Capability to close ODataBinaryStreamValue stream after serialization (…

…#2597) * Capability for JsonLightSerializer to close ODataBinaryStreamValue streams once processed. * Use of DisposeAsync in runtimes where it's available. * Update public API * Undo change to Unshipped.txt * Update PublicAPI.Shipped.txt * Update PublicAPI.shipped.txt for 3.1 Co-authored-by: Sylvain Prieur <syprieur@microsoft.com> Co-authored-by: Clement Habinshuti <clhabins@microsoft.com>
OData · Jan 25, 2023 · 0770ead · 0770ead
1 parent ecd0850
commit 0770ead
Show file tree

Hide file tree

Showing 10 changed files with 64 additions and 6 deletions.
diff --git a/src/Microsoft.OData.Core/JsonLight/ODataJsonLightValueSerializer.cs b/src/Microsoft.OData.Core/JsonLight/ODataJsonLightValueSerializer.cs
@@ -390,6 +390,11 @@ public virtual void WriteStreamValue(ODataBinaryStreamValue streamValue)
                 stream.Dispose();
                 streamWriter.EndStreamValueScope();
             }
+
+            if (!streamValue.LeaveOpen)
+            {
+                streamValue.Stream.Dispose();
+            }
         }
 
         /// <summary>
@@ -720,7 +725,6 @@ public virtual async Task WriteStreamValueAsync(ODataBinaryStreamValue streamVal
             IJsonStreamWriterAsync streamWriter = this.AsynchronousJsonWriter as IJsonStreamWriterAsync;
             if (streamWriter == null)
             {
-                // write as a string
                 byte[] value = await streamValue.Stream.ReadAllBytesAsync().ConfigureAwait(false);
                 await this.AsynchronousJsonWriter.WritePrimitiveValueAsync(value).ConfigureAwait(false);
             }
@@ -729,9 +733,22 @@ public virtual async Task WriteStreamValueAsync(ODataBinaryStreamValue streamVal
                 Stream stream = await streamWriter.StartStreamValueScopeAsync().ConfigureAwait(false);
                 await streamValue.Stream.CopyToAsync(stream).ConfigureAwait(false);
                 await stream.FlushAsync().ConfigureAwait(false);
+#if NETCOREAPP3_1_OR_GREATER
+                await stream.DisposeAsync();
+#else
                 stream.Dispose();
+#endif
                 await streamWriter.EndStreamValueScopeAsync().ConfigureAwait(false);
             }
+
+            if (!streamValue.LeaveOpen)
+            {
+#if NETCOREAPP3_1_OR_GREATER
+                await streamValue.Stream.DisposeAsync();
+#else
+                streamValue.Stream.Dispose();
+#endif
+            }
         }
 
         /// <summary>

diff --git a/src/Microsoft.OData.Core/PublicAPI/net45/PublicAPI.Shipped.txt b/src/Microsoft.OData.Core/PublicAPI/net45/PublicAPI.Shipped.txt
@@ -373,6 +373,7 @@ Microsoft.OData.ODataBatchWriter.WriteStartChangesetAsync() -> System.Threading.
 Microsoft.OData.ODataBatchWriter.WriteStartChangesetAsync(string changesetId) -> System.Threading.Tasks.Task
 Microsoft.OData.ODataBinaryStreamValue
 Microsoft.OData.ODataBinaryStreamValue.ODataBinaryStreamValue(System.IO.Stream stream) -> void
+Microsoft.OData.ODataBinaryStreamValue.ODataBinaryStreamValue(System.IO.Stream stream, bool leaveOpen) -> void
 Microsoft.OData.ODataBinaryStreamValue.Stream.get -> System.IO.Stream
 Microsoft.OData.ODataCollectionReader
 Microsoft.OData.ODataCollectionReader.ODataCollectionReader() -> void

diff --git a/src/Microsoft.OData.Core/PublicAPI/netcoreapp3.1/PublicAPI.Shipped.txt b/src/Microsoft.OData.Core/PublicAPI/netcoreapp3.1/PublicAPI.Shipped.txt
@@ -373,6 +373,7 @@ Microsoft.OData.ODataBatchWriter.WriteStartChangesetAsync() -> System.Threading.
 Microsoft.OData.ODataBatchWriter.WriteStartChangesetAsync(string changesetId) -> System.Threading.Tasks.Task
 Microsoft.OData.ODataBinaryStreamValue
 Microsoft.OData.ODataBinaryStreamValue.ODataBinaryStreamValue(System.IO.Stream stream) -> void
+Microsoft.OData.ODataBinaryStreamValue.ODataBinaryStreamValue(System.IO.Stream stream, bool leaveOpen) -> void
 Microsoft.OData.ODataBinaryStreamValue.Stream.get -> System.IO.Stream
 Microsoft.OData.ODataCollectionReader
 Microsoft.OData.ODataCollectionReader.ODataCollectionReader() -> void

diff --git a/src/Microsoft.OData.Core/PublicAPI/netstandard1.1/PublicAPI.Shipped.txt b/src/Microsoft.OData.Core/PublicAPI/netstandard1.1/PublicAPI.Shipped.txt
@@ -373,6 +373,7 @@ Microsoft.OData.ODataBatchWriter.WriteStartChangesetAsync() -> System.Threading.
 Microsoft.OData.ODataBatchWriter.WriteStartChangesetAsync(string changesetId) -> System.Threading.Tasks.Task
 Microsoft.OData.ODataBinaryStreamValue
 Microsoft.OData.ODataBinaryStreamValue.ODataBinaryStreamValue(System.IO.Stream stream) -> void
+Microsoft.OData.ODataBinaryStreamValue.ODataBinaryStreamValue(System.IO.Stream stream, bool leaveOpen) -> void
 Microsoft.OData.ODataBinaryStreamValue.Stream.get -> System.IO.Stream
 Microsoft.OData.ODataCollectionReader
 Microsoft.OData.ODataCollectionReader.ODataCollectionReader() -> void

diff --git a/src/Microsoft.OData.Core/PublicAPI/netstandard2.0/PublicAPI.Shipped.txt b/src/Microsoft.OData.Core/PublicAPI/netstandard2.0/PublicAPI.Shipped.txt
@@ -373,6 +373,7 @@ Microsoft.OData.ODataBatchWriter.WriteStartChangesetAsync() -> System.Threading.
 Microsoft.OData.ODataBatchWriter.WriteStartChangesetAsync(string changesetId) -> System.Threading.Tasks.Task
 Microsoft.OData.ODataBinaryStreamValue
 Microsoft.OData.ODataBinaryStreamValue.ODataBinaryStreamValue(System.IO.Stream stream) -> void
+Microsoft.OData.ODataBinaryStreamValue.ODataBinaryStreamValue(System.IO.Stream stream, bool leaveOpen) -> void
 Microsoft.OData.ODataBinaryStreamValue.Stream.get -> System.IO.Stream
 Microsoft.OData.ODataCollectionReader
 Microsoft.OData.ODataCollectionReader.ODataCollectionReader() -> void

diff --git a/src/Microsoft.OData.Core/Value/ODataBinaryStreamValue.cs b/src/Microsoft.OData.Core/Value/ODataBinaryStreamValue.cs
@@ -19,16 +19,32 @@ public sealed class ODataBinaryStreamValue : ODataValue
         /// Constructor
         /// </summary>
         /// <param name="stream">Input stream</param>
-        public ODataBinaryStreamValue(Stream stream)
+        public ODataBinaryStreamValue(Stream stream) :
+            this(stream, true) // Leaves the stream open by default, for backward compatibility.
+        {
+        }
+
+        /// <summary>
+        /// Constructor
+        /// </summary>
+        /// <param name="stream">Input stream</param>
+        /// <param name="leaveOpen">Whether the provided stream should be left open.</param>
+        public ODataBinaryStreamValue(Stream stream, bool leaveOpen)
         {
             ExceptionUtils.CheckArgumentNotNull(stream, "stream");
 
             this.Stream = stream;
+            this.LeaveOpen = leaveOpen;
         }
 
         /// <summary>
         /// The Stream wrapped by the ODataBinaryStreamValue
         /// </summary>
         public Stream Stream { get; private set; }
+
+        /// <summary>
+        /// Whether this instance's <see cref="Stream"/> should be left open.
+        /// </summary>
+        internal bool LeaveOpen { get; }
     }
 }
diff --git a/...unctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightValueSerializerTests.cs b/...unctionalTests/Microsoft.OData.Core.Tests/JsonLight/ODataJsonLightValueSerializerTests.cs
@@ -314,15 +314,17 @@ public void WritingMultipleInstanceAnnotationInResourceValueShouldWrite(string f
             Assert.Equal(expect, result);
         }
 
-        [Fact]
-        public void WriteStreamValue_WritesStreamValue()
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void WriteStreamValue_WritesStreamValue(bool leaveOpen)
         {
-            var stream = new MemoryStream();
+            var stream = new DisposeTrackingMemoryStream();
             var writer = new BinaryWriter(stream);
             writer.Write("1234567890");
             writer.Flush();
             stream.Position = 0;
-            var streamValue = new ODataBinaryStreamValue(stream);
+            var streamValue = new ODataBinaryStreamValue(stream, leaveOpen);
 
             var result = this.SetupSerializerAndRunTest(
                 (jsonLightValueSerializer) =>
@@ -331,6 +333,7 @@ public void WriteStreamValue_WritesStreamValue()
                 });
 
             Assert.Equal("\"CjEyMzQ1Njc4OTA=\"", result);
+            Assert.Equal(leaveOpen, !stream.Disposed);
         }
 
 #if NETCOREAPP3_1_OR_GREATER
@@ -395,5 +398,20 @@ private string SetupSerializerAndRunTest(Action<ODataJsonLightValueSerializer> a
             var streamReader = new StreamReader(stream);
             return streamReader.ReadToEnd();
         }
+
+        private class DisposeTrackingMemoryStream : MemoryStream
+        {
+            public bool Disposed { get; private set; } = false;
+
+            protected override void Dispose(bool disposing)
+            {
+                if (disposing)
+                {
+                    this.Disposed = true;
+                }
+
+                base.Dispose(disposing);
+            }
+        }
     }
 }
diff --git a/test/PublicApiTests/BaseLine/Microsoft.OData.PublicApi.net45.bsl b/test/PublicApiTests/BaseLine/Microsoft.OData.PublicApi.net45.bsl
@@ -5222,6 +5222,7 @@ public sealed class Microsoft.OData.ODataBatchOperationResponseMessage : IContai
 
 public sealed class Microsoft.OData.ODataBinaryStreamValue : Microsoft.OData.ODataValue {
     public ODataBinaryStreamValue (System.IO.Stream stream)
+    public ODataBinaryStreamValue (System.IO.Stream stream, bool leaveOpen)
 
     System.IO.Stream Stream  { public get; }
 }

diff --git a/test/PublicApiTests/BaseLine/Microsoft.OData.PublicApi.netstandard1.1.bsl b/test/PublicApiTests/BaseLine/Microsoft.OData.PublicApi.netstandard1.1.bsl
@@ -5222,6 +5222,7 @@ public sealed class Microsoft.OData.ODataBatchOperationResponseMessage : IContai
 
 public sealed class Microsoft.OData.ODataBinaryStreamValue : Microsoft.OData.ODataValue {
     public ODataBinaryStreamValue (System.IO.Stream stream)
+    public ODataBinaryStreamValue (System.IO.Stream stream, bool leaveOpen)
 
     System.IO.Stream Stream  { public get; }
 }

diff --git a/test/PublicApiTests/BaseLine/Microsoft.OData.PublicApi.netstandard2.0.bsl b/test/PublicApiTests/BaseLine/Microsoft.OData.PublicApi.netstandard2.0.bsl
@@ -5222,6 +5222,7 @@ public sealed class Microsoft.OData.ODataBatchOperationResponseMessage : IContai
 
 public sealed class Microsoft.OData.ODataBinaryStreamValue : Microsoft.OData.ODataValue {
     public ODataBinaryStreamValue (System.IO.Stream stream)
+    public ODataBinaryStreamValue (System.IO.Stream stream, bool leaveOpen)
 
     System.IO.Stream Stream  { public get; }
 }