CR feedback

Author: Yishai Galatzer

src/Microsoft.AspNet.Mvc.Core/Formatters/StreamOutputFormatter.cs

@@ -44,7 +44,6 @@ public IReadOnlyList<MediaTypeHeaderValue> GetSupportedContentTypes(
         public bool CanWriteResult([NotNull] OutputFormatterContext context, MediaTypeHeaderValue contentType)
         {
             // Ignore the passed in content type, if the object is a Stream.
-            // always return it as a text/plain format.
             if (context.Object is Stream)
             {
                 context.SelectedContentType = contentType;
@@ -57,16 +56,17 @@ public bool CanWriteResult([NotNull] OutputFormatterContext context, MediaTypeHe
         /// <inheritdoc />
         public async Task WriteAsync([NotNull] OutputFormatterContext context)
         {
-            var valueAsStream = ((Stream)context.Object);
+            using (var valueAsStream = ((Stream)context.Object))
+            {
+                var response = context.ActionContext.HttpContext.Response;
 
-            var response = context.ActionContext.HttpContext.Response;
+                if (context.SelectedContentType != null)
+                {
+                    response.ContentType = context.SelectedContentType.ToString();
+                }
 
-            if (context.SelectedContentType != null)
-            {
-                response.ContentType = context.SelectedContentType.ToString();
+                await valueAsStream.CopyToAsync(response.Body);
             }
-
-            await valueAsStream.CopyToAsync(response.Body);
         }
     }
 }

test/Microsoft.AspNet.Mvc.Core.Test/Formatters/StreamOutputFormatterTest.cs

@@ -3,14 +3,14 @@
 
 using System;
 using System.IO;
-using Microsoft.AspNet.Mvc;
 using Microsoft.Net.Http.Headers;
 using Xunit;
 
 namespace Microsoft.AspNet.Mvc
 {
     public class StreamOutputFormatterTest
     {
+        [Theory]
         [InlineData(typeof(Stream), typeof(FileStream), "text/plain", "text/plain")]
         [InlineData(typeof(object), typeof(FileStream), "text/plain", "text/plain")]
         [InlineData(typeof(object), typeof(MemoryStream), "text/plain", "text/plain")]
@@ -19,9 +19,10 @@ public class StreamOutputFormatterTest
         [InlineData(typeof(object), null, "text/plain", null)]
         [InlineData(typeof(IActionResult), null, "text/plain", null)]
         [InlineData(typeof(IActionResult), typeof(IActionResult), "text/plain", null)]
-        [Theory]
-        public void GetSupportedContentTypes_ReturnsAppropriateValues(Type declaredType, Type runtimeType,
-                                                                      string contentType, string expected)
+        public void GetSupportedContentTypes_ReturnsAppropriateValues(Type declaredType,
+                                                                      Type runtimeType,
+                                                                      string contentType,
+                                                                      string expected)
         {
             // Arrange
             var formatter = new StreamOutputFormatter();
@@ -42,10 +43,10 @@ public void GetSupportedContentTypes_ReturnsAppropriateValues(Type declaredType,
             }
         }
 
+        [Theory]
         [InlineData(typeof(object))]
         [InlineData(typeof(SimplePOCO))]
         [InlineData(null)]
-        [Theory]
         public void CanWriteResult_OnlyActsOnStreams(Type type)
         {
             // Arrange

test/Microsoft.AspNet.Mvc.FunctionalTests/StreamOutputFormatterTest.cs

@@ -2,8 +2,6 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Net;
-using System.Net.Http.Headers;
 using System.Threading.Tasks;
 using FormatterWebSite;
 using Microsoft.AspNet.Builder;
@@ -17,12 +15,12 @@ public class StreamOutputFormatterTest
         private readonly IServiceProvider _provider = TestHelper.CreateServices(nameof(FormatterWebSite));
         private readonly Action<IApplicationBuilder> _app = new Startup().Configure;
 
+        [Theory]
         [InlineData("SimpleMemoryStream", null)]
         [InlineData("MemoryStreamWithContentType", "text/html")]
         [InlineData("MemoryStreamWithContentTypeFromProduces", "text/plain")]
         [InlineData("MemoryStreamWithContentTypeFromProducesWithMultipleValues", "text/html")]
         [InlineData("MemoryStreamOverridesContentTypeWithProduces", "text/plain")]
-        [Theory]
         public async Task StreamOutputFormatter_ReturnsAppropriateContentAndContentType(string actionName, string contentType)
         {
             // Arrange

…ft.AspNet.Mvc.Test/MvcOptionSetupTest.cs …t.AspNet.Mvc.Test/MvcOptionsSetupTest.cstest/Microsoft.AspNet.Mvc.Test/MvcOptionSetupTest.cs renamed to test/Microsoft.AspNet.Mvc.Test/MvcOptionsSetupTest.cs test/Microsoft.AspNet.Mvc.Test/MvcOptionSetupTest.cs renamed to test/Microsoft.AspNet.Mvc.Test/MvcOptionsSetupTest.cs

@@ -10,7 +10,7 @@
 
 namespace Microsoft.AspNet.Mvc
 {
-    public class MvcOptionSetupTest
+    public class MvcOptionsSetupTest
     {
         [Fact]
         public void Setup_SetsUpViewEngines()
@@ -86,8 +86,8 @@ public void Setup_SetsUpOutputFormatters()
             Assert.Equal(4, mvcOptions.OutputFormatters.Count);
             Assert.IsType<HttpNoContentOutputFormatter>(mvcOptions.OutputFormatters[0].Instance);
             Assert.IsType<StringOutputFormatter>(mvcOptions.OutputFormatters[1].Instance);
-            Assert.IsType<StreamOutputFormatter>(mvcOptions.OutputFormatters[1].Instance);
-            Assert.IsType<JsonOutputFormatter>(mvcOptions.OutputFormatters[2].Instance);
+            Assert.IsType<StreamOutputFormatter>(mvcOptions.OutputFormatters[2].Instance);
+            Assert.IsType<JsonOutputFormatter>(mvcOptions.OutputFormatters[3].Instance);
         }
 
         [Fact]

test/WebSites/FormatterWebSite/Controllers/StreamController.cs

@@ -3,7 +3,6 @@
 
 using System.IO;
 using Microsoft.AspNet.Mvc;
-using Microsoft.Net.Http.Headers;
 
 namespace FormatterWebSite
 {
@@ -40,7 +39,7 @@ public Stream MemoryStreamWithContentTypeFromProducesWithMultipleValues()
         [Produces("text/plain")]
         public Stream MemoryStreamOverridesContentTypeWithProduces()
         {
-            // Produces will set a ContentType on the implicit ObjecResult and
+            // Produces will set a ContentType on the implicit ObjectResult and
             // ContentType on response are overriden by content types from ObjectResult.
             Response.ContentType = "text/html";
 
@@ -55,7 +54,7 @@ private static Stream CreateDefaultStream()
             writer.Flush();
             stream.Seek(0, SeekOrigin.Begin);
 
-            return stream;            
+            return stream;
         }
     }
 }