[Fixes #1331] Register object for dispose in ObjectResult and

FileStreamResult

src/Microsoft.AspNet.Mvc.Core/Controller.cs

@@ -376,6 +376,12 @@ namespace Microsoft.AspNet.Mvc
         [NonAction]
         public virtual JsonResult Json(object data)
         {
+            var disposableValue = data as IDisposable;
+            if (disposableValue != null)
+            {
+                Response.OnResponseCompleted(_ => disposableValue.Dispose(), state: null);
+            }
+
             return new JsonResult(data);
         }
 
@@ -659,6 +665,11 @@ namespace Microsoft.AspNet.Mvc
         [NonAction]
         public virtual FileStreamResult File(Stream fileStream, string contentType, string fileDownloadName)
         {
+            if (fileStream != null)
+            {
+                Response.OnResponseCompleted(_ => fileStream.Dispose(), state: null);
+            }
+
             return new FileStreamResult(fileStream, contentType) { FileDownloadName = fileDownloadName };
         }
 
@@ -717,6 +728,12 @@ namespace Microsoft.AspNet.Mvc
         [NonAction]
         public virtual HttpNotFoundObjectResult HttpNotFound(object value)
         {
+            var disposableValue = value as IDisposable;
+            if (disposableValue != null)
+            {
+                Response.OnResponseCompleted(_ => disposableValue.Dispose(), state: null);
+            }
+
             return new HttpNotFoundObjectResult(value);
         }
 
@@ -737,6 +754,12 @@ namespace Microsoft.AspNet.Mvc
         [NonAction]
         public virtual BadRequestObjectResult HttpBadRequest(object error)
         {
+            var disposableValue = error as IDisposable;
+            if (disposableValue != null)
+            {
+                Response.OnResponseCompleted(_ => disposableValue.Dispose(), state: null);
+            }
+
             return new BadRequestObjectResult(error);
         }
 
@@ -759,6 +782,12 @@ namespace Microsoft.AspNet.Mvc
         [NonAction]
         public virtual CreatedResult Created([NotNull] string uri, object value)
         {
+            var disposableValue = value as IDisposable;
+            if (disposableValue != null)
+            {
+                Response.OnResponseCompleted(_ => disposableValue.Dispose(), state: null);
+            }
+
             return new CreatedResult(uri, value);
         }
 
@@ -780,7 +809,8 @@ namespace Microsoft.AspNet.Mvc
             {
                 location = uri.GetComponents(UriComponents.SerializationInfoString, UriFormat.UriEscaped);
             }
-            return new CreatedResult(location, value);
+
+            return Created(location, value);
         }
 
         /// <summary>
@@ -822,6 +852,12 @@ namespace Microsoft.AspNet.Mvc
                                                              object routeValues,
                                                              object value)
         {
+            var disposableValue = value as IDisposable;
+            if (disposableValue != null)
+            {
+                Response.OnResponseCompleted(_ => disposableValue.Dispose(), state: null);
+            }
+
             return new CreatedAtActionResult(actionName, controllerName, routeValues, value);
         }
 
@@ -859,6 +895,12 @@ namespace Microsoft.AspNet.Mvc
         [NonAction]
         public virtual CreatedAtRouteResult CreatedAtRoute(string routeName, object routeValues, object value)
         {
+            var disposableValue = value as IDisposable;
+            if (disposableValue != null)
+            {
+                Response.OnResponseCompleted(_ => disposableValue.Dispose(), state: null);
+            }
+
             return new CreatedAtRouteResult(routeName, routeValues, value);
         }
 

test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs

@@ -423,6 +423,33 @@ namespace Microsoft.AspNet.Mvc.Test
             Assert.Equal(uri.OriginalString, result.Location);
         }
 
+        [Fact]
+        public void Created_IDisposableObject_RegistersForDispose()
+        {
+            // Arrange
+            var mockHttpContext = new Mock<DefaultHttpContext>();
+            mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
+            var uri = new Uri("/test/url", UriKind.Relative);
+
+            var controller = new TestableController()
+            {
+                ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
+            };
+            var input = new DisposableObject();
+
+            // Act
+            var result = controller.Created(uri, input);
+
+            // Assert
+            Assert.IsType<CreatedResult>(result);
+            Assert.Equal(StatusCodes.Status201Created, result.StatusCode);
+            Assert.Equal(uri.OriginalString, result.Location);
+            Assert.Same(input, result.Value);
+            mockHttpContext.Verify(
+                x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
+                Times.Once());
+        }
+
         [Fact]
         public void CreatedAtAction_WithParameterActionName_SetsResultActionName()
         {
@@ -483,6 +510,32 @@ namespace Microsoft.AspNet.Mvc.Test
             Assert.Equal(expected, result.RouteValues);
         }
 
+        [Fact]
+        public void CreatedAtAction_IDisposableObject_RegistersForDispose()
+        {
+            // Arrange
+            var mockHttpContext = new Mock<DefaultHttpContext>();
+            mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
+
+            var controller = new TestableController()
+            {
+                ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
+            };
+            var input = new DisposableObject();
+
+            // Act
+            var result = controller.CreatedAtAction("SampleAction", input);
+
+            // Assert
+            Assert.IsType<CreatedAtActionResult>(result);
+            Assert.Equal(StatusCodes.Status201Created, result.StatusCode);
+            Assert.Equal("SampleAction", result.ActionName);
+            Assert.Same(input, result.Value);
+            mockHttpContext.Verify(
+                x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
+                Times.Once());
+        }
+
         [Fact]
         public void CreatedAtRoute_WithParameterRouteName_SetsResultSameRouteName()
         {
@@ -540,6 +593,32 @@ namespace Microsoft.AspNet.Mvc.Test
             Assert.Equal(expected, result.RouteValues);
         }
 
+        [Fact]
+        public void CreatedAtRoute_IDisposableObject_RegistersForDispose()
+        {
+            // Arrange
+            var mockHttpContext = new Mock<DefaultHttpContext>();
+            mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
+
+            var controller = new TestableController()
+            {
+                ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
+            };
+            var input = new DisposableObject();
+
+            // Act
+            var result = controller.CreatedAtRoute("SampleRoute", input);
+
+            // Assert
+            Assert.IsType<CreatedAtRouteResult>(result);
+            Assert.Equal(StatusCodes.Status201Created, result.StatusCode);
+            Assert.Equal("SampleRoute", result.RouteName);
+            Assert.Same(input, result.Value);
+            mockHttpContext.Verify(
+                x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
+                Times.Once());
+        }
+
         [Fact]
         public void File_WithContents()
         {
@@ -612,7 +691,12 @@ namespace Microsoft.AspNet.Mvc.Test
         public void File_WithStream()
         {
             // Arrange
-            var controller = new TestableController();
+            var mockHttpContext = new Mock<DefaultHttpContext>();
+            mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
+            var controller = new TestableController()
+            {
+                ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
+            };
             var fileStream = Stream.Null;
 
             // Act
@@ -629,7 +713,13 @@ namespace Microsoft.AspNet.Mvc.Test
         public void File_WithStreamAndFileDownloadName()
         {
             // Arrange
-            var controller = new TestableController();
+            var mockHttpContext = new Mock<DefaultHttpContext>();
+            mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
+
+            var controller = new TestableController()
+            {
+                ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
+            };
             var fileStream = Stream.Null;
 
             // Act
@@ -640,6 +730,9 @@ namespace Microsoft.AspNet.Mvc.Test
             Assert.Same(fileStream, result.FileStream);
             Assert.Equal("someContentType", result.ContentType);
             Assert.Equal("someDownloadName", result.FileDownloadName);
+            mockHttpContext.Verify(
+                x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
+                Times.Once());
         }
 
         [Fact]
@@ -685,6 +778,31 @@ namespace Microsoft.AspNet.Mvc.Test
             Assert.Equal("Test Content", result.Value);
         }
 
+        [Fact]
+        public void HttpNotFound_IDisposableObject_RegistersForDispose()
+        {
+            // Arrange
+            var mockHttpContext = new Mock<DefaultHttpContext>();
+            mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
+
+            var controller = new TestableController()
+            {
+                ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
+            };
+            var input = new DisposableObject();
+
+            // Act
+            var result = controller.HttpNotFound(input);
+
+            // Assert
+            Assert.IsType<HttpNotFoundObjectResult>(result);
+            Assert.Equal(StatusCodes.Status404NotFound, result.StatusCode);
+            Assert.Same(input, result.Value);
+            mockHttpContext.Verify(
+                x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
+                Times.Once());
+        }
+
         [Fact]
         public void BadRequest_SetsStatusCode()
         {
@@ -715,6 +833,31 @@ namespace Microsoft.AspNet.Mvc.Test
             Assert.Equal(obj, result.Value);
         }
 
+        [Fact]
+        public void BadRequest_IDisposableObject_RegistersForDispose()
+        {
+            // Arrange
+            var mockHttpContext = new Mock<DefaultHttpContext>();
+            mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
+
+            var controller = new TestableController()
+            {
+                ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
+            };
+            var input = new DisposableObject();
+
+            // Act
+            var result = controller.HttpBadRequest(input);
+
+            // Assert
+            Assert.IsType<BadRequestObjectResult>(result);
+            Assert.Equal(StatusCodes.Status400BadRequest, result.StatusCode);
+            Assert.Same(input, result.Value);
+            mockHttpContext.Verify(
+                x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
+                Times.Once());
+        }
+
         [Fact]
         public void BadRequest_SetsStatusCodeAndValue_ModelState()
         {
@@ -888,6 +1031,30 @@ namespace Microsoft.AspNet.Mvc.Test
             Assert.Same(data, actualJsonResult.Value);
         }
 
+        [Fact]
+        public void Controller_Json_IDisposableObject_RegistersForDispose()
+        {
+            // Arrange
+            var mockHttpContext = new Mock<DefaultHttpContext>();
+            mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
+
+            var controller = new TestableController()
+            {
+                ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
+            };
+            var input = new DisposableObject();
+
+            // Act
+            var result = controller.Json(input);
+
+            // Assert
+            Assert.IsType<JsonResult>(result);
+            Assert.Same(input, result.Value);
+            mockHttpContext.Verify(
+                x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
+                Times.Once());
+        }
+
         public static IEnumerable<object[]> RedirectTestData
         {
             get
@@ -1402,7 +1569,7 @@ namespace Microsoft.AspNet.Mvc.Test
         {
             // Arrange
             var model = new TryValidateModelModel();
-            var validationResult = new []
+            var validationResult = new[]
             {
                 new ModelValidationResult(string.Empty, "Out of range!")
             };
@@ -1569,5 +1736,13 @@ namespace Microsoft.AspNet.Mvc.Test
         {
 
         }
+
+        private class DisposableObject : IDisposable
+        {
+            public void Dispose()
+            {
+                throw new NotImplementedException();
+            }
+        }
     }
 }

test/Microsoft.AspNet.Mvc.Core.Test/ControllerUnitTestabilityTests.cs

@@ -6,6 +6,7 @@ using System.Collections.Generic;
 using System.IO;
 using System.Text;
 using Microsoft.AspNet.Http;
+using Microsoft.AspNet.Http.Core;
 using Microsoft.AspNet.Mvc.ModelBinding;
 using Microsoft.AspNet.Routing;
 using Microsoft.AspNet.WebUtilities;
@@ -144,7 +145,12 @@ namespace Microsoft.AspNet.Mvc
         public void ControllerFileStream_InvokedInUnitTests(string content, string contentType, string fileName)
         {
             // Arrange
-            var controller = new TestabilityController();
+            var mockHttpContext = new Mock<DefaultHttpContext>();
+            mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
+            var controller = new TestabilityController()
+            {
+                ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
+            };
 
             // Act
             var result = controller.FileStream_Action(content, contentType, fileName);

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

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Collections.Generic;
 using System.Net;
 using System.Net.Http;
 using System.Net.Http.Headers;
@@ -307,12 +308,10 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
             // Arrange
             var server = TestHelper.CreateServer(_app, SiteName);
             var client = server.CreateClient();
-            var input = "{\"SampleInt\":10}";
 
             var request = new HttpRequestMessage(
                 HttpMethod.Post,
                 "http://localhost/ActionResultsVerification/GetNotFoundObjectResultWithContent");
-            request.Content = new StringContent(input, Encoding.UTF8, "application/json");
 
             // Act
             var response = await client.SendAsync(request);
@@ -321,5 +320,37 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
             Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
             Assert.Equal("{\"SampleInt\":10,\"SampleString\":\"Foo\"}", await response.Content.ReadAsStringAsync());
         }
+
+        [Fact]
+        public async Task HttpNotFoundObjectResult_WithDisposableObject()
+        {
+            // Arrange
+            var server = TestHelper.CreateServer(_app, SiteName);
+            var client = server.CreateClient();
+            var nameValueCollection = new List<KeyValuePair<string, string>>
+            {
+                new KeyValuePair<string, string>("guid", Guid.NewGuid().ToString()),
+            };
+
+            // Act
+            var response1 = await client.PostAsync(
+                "/ActionResultsVerification/GetDisposeCalled",
+                new FormUrlEncodedContent(nameValueCollection));
+
+            await client.PostAsync(
+                "/ActionResultsVerification/GetNotFoundObjectResultWithDisposableObject",
+                new FormUrlEncodedContent(nameValueCollection));
+
+            var response2 = await client.PostAsync(
+                "/ActionResultsVerification/GetDisposeCalled",
+                new FormUrlEncodedContent(nameValueCollection));
+
+            // Assert
+            var isDisposed = Convert.ToBoolean(await response1.Content.ReadAsStringAsync());
+            Assert.False(isDisposed);
+
+            isDisposed = Convert.ToBoolean(await response2.Content.ReadAsStringAsync());
+            Assert.True(isDisposed);
+        }
     }
 }

test/WebSites/ActionResultsWebSite/Controllers/ActionResultsVerificationController.cs

@@ -10,6 +10,10 @@ namespace ActionResultsWebSite
 {
     public class ActionResultsVerificationController : Controller
     {
+
+        [FromServices]
+        public GuidLookupService Service { get; set; }
+
         public IActionResult Index([FromBody]DummyClass test)
         {
             if (!ModelState.IsValid)
@@ -95,6 +99,22 @@ namespace ActionResultsWebSite
             return HttpNotFound(CreateDummy());
         }
 
+        public IActionResult GetNotFoundObjectResultWithDisposableObject(string guid)
+        {
+            return HttpNotFound(CreateDisposableType(guid));
+        }
+
+        public bool GetDisposeCalled(string guid)
+        {
+            bool value;
+            if (Service.IsDisposed.TryGetValue(guid, out value))
+            {
+                return value;
+            }
+
+            return false;
+        }
+
         public DummyClass GetDummy(int id)
         {
             return CreateDummy();
@@ -108,5 +128,28 @@ namespace ActionResultsWebSite
                 SampleString = "Foo"
             };
         }
+
+        private DisposableType CreateDisposableType(string guid)
+        {
+            return new DisposableType(Service, guid);
+        }
+
+        private class DisposableType : IDisposable
+        {
+            private GuidLookupService _service;
+            private string _guid;
+
+            public DisposableType(GuidLookupService service, string guid)
+            {
+                _service = service;
+                _guid = guid;
+                _service.IsDisposed[_guid] = false;
+            }
+
+            public void Dispose()
+            {
+                _service.IsDisposed[_guid] = true;
+            }
+        }
     }
 }

test/WebSites/ActionResultsWebSite/GuidLookupService.cs

@@ -0,0 +1,12 @@
+// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Collections.Generic;
+
+namespace ActionResultsWebSite
+{
+    public class GuidLookupService
+    {
+        public Dictionary<string, bool> IsDisposed { get; } = new Dictionary<string, bool>();
+    }
+}

test/WebSites/ActionResultsWebSite/Startup.cs

@@ -16,6 +16,7 @@ namespace ActionResultsWebSite
             app.UseServices(services =>
             {
                 services.AddMvc();
+                services.AddInstance(new GuidLookupService());
 
                 services.Configure<MvcOptions>(options =>
                 {