Address review comments

OData · Feb 23, 2022 · e4aa83a · e4aa83a
1 parent a983a2b
commit e4aa83a
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 20 deletions.
diff --git a/src/Microsoft.OData.Core/ODataNotificationReader.cs b/src/Microsoft.OData.Core/ODataNotificationReader.cs
@@ -28,7 +28,7 @@ internal sealed class ODataNotificationReader : TextReader
         internal ODataNotificationReader(TextReader textReader, IODataStreamListener listener, bool synchronous = true)
         {
             Debug.Assert(textReader != null, "Creating a notification reader for a null textReader.");
-            Debug.Assert(listener != null, "Creating a notification reader with a null textReader.");
+            Debug.Assert(listener != null, "Creating a notification reader with a null listener.");
 
             this.textReader = textReader;
             this.listener = listener;
@@ -126,16 +126,12 @@ protected override void Dispose(bool disposing)
                 // Tell the listener that the stream is being disposed.
                 if (synchronous)
                 {
-                    this.listener?.StreamDisposed();
+                    this.listener.StreamDisposed();
                 }
                 else
                 {
-                    this.listener?.StreamDisposedAsync().Wait();
+                    this.listener.StreamDisposedAsync().Wait();
                 }
-
-                this.listener = null;
-                // NOTE: Do not dispose the text reader since this instance does not own it.
-                this.textReader = null;
             }
 
             this.disposed = true;
@@ -145,14 +141,10 @@ protected override void Dispose(bool disposing)
 #if NETSTANDARD2_0
         public async ValueTask DisposeAsync()
         {
-            if (!this.disposed && this.listener != null)
+            if (!this.disposed)
             {
                 await this.listener.StreamDisposedAsync()
                     .ConfigureAwait(false);
-
-                this.listener = null;
-                // NOTE: Do not dispose the text reader since this instance does not own it.
-                this.textReader = null;
             }
 
             // Dispose unmanaged resources

diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ODataNotificationReaderTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ODataNotificationReaderTests.cs
@@ -6,22 +6,25 @@
 
 using System;
 using System.IO;
+using System.Text;
 using System.Threading.Tasks;
 using Xunit;
 
 namespace Microsoft.OData.Tests
 {
-    public class ODataNotificationReaderTests
+    public class ODataNotificationReaderTests : IDisposable
     {
         private MemoryStream stream;
         private TextReader reader;
+        private TextWriter streamListenerWriter;
         private IODataStreamListener streamListener;
 
         public ODataNotificationReaderTests()
         {
             this.stream = new MemoryStream();
-            this.reader = new StreamReader(this.stream);
-            this.streamListener = new MockODataStreamListener(new StreamWriter(this.stream));
+            this.reader = new StreamReader(this.stream, encoding: Encoding.UTF8, detectEncodingFromByteOrderMarks: true, bufferSize: 1024, leaveOpen: true);
+            this.streamListenerWriter = new StreamWriter(this.stream, encoding: Encoding.UTF8, bufferSize: 1024, leaveOpen: true);
+            this.streamListener = new MockODataStreamListener(this.streamListenerWriter);
         }
 
         [Theory]
@@ -114,16 +117,49 @@ public async Task NotificationReaderDisposeShouldInvokeStreamDisposedAsync()
         }
 #endif
 
+        public void Dispose() // Fired after every test is ran
+        {
+            this.streamListenerWriter.Dispose();
+            this.reader.Dispose();
+            this.stream.Dispose();
+        }
+
         private string ReadStreamContents()
         {
-            this.stream.Position = 0;
-            return new StreamReader(this.stream).ReadToEnd();
+            string streamContents;
+
+            using (var reader = new StreamReader(
+                this.stream,
+                encoding: Encoding.UTF8,
+                detectEncodingFromByteOrderMarks: true,
+                bufferSize: 1024,
+                leaveOpen: true))
+            {
+
+                this.stream.Position = 0;
+                streamContents = reader.ReadToEnd();
+            }
+
+            return streamContents;
         }
 
-        private Task<string> ReadStreamContentsAsync()
+        private async Task<string> ReadStreamContentsAsync()
         {
-            this.stream.Position = 0;
-            return new StreamReader(this.stream).ReadToEndAsync();
+            string  streamContents;
+
+            using (var reader = new StreamReader(
+                this.stream,
+                encoding: Encoding.UTF8,
+                detectEncodingFromByteOrderMarks: true,
+                bufferSize: 1024,
+                leaveOpen: true))
+            {
+
+                this.stream.Position = 0;
+                streamContents = await reader.ReadToEndAsync();
+            }
+
+            return streamContents;
         }
 
         private class MockODataStreamListener : IODataStreamListener