diff --git a/src/Microsoft.AspNet.Mvc.Core/Controller.cs b/src/Microsoft.AspNet.Mvc.Core/Controller.cs index f33342ef2c..3510736fb1 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Controller.cs +++ b/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); } /// @@ -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); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs index 5304c27099..662f0b231d 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs +++ b/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(); + mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny>(), It.IsAny())); + 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(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>(), It.IsAny()), + 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(); + mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny>(), It.IsAny())); + + 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(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>(), It.IsAny()), + 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(); + mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny>(), It.IsAny())); + + 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(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>(), It.IsAny()), + 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(); + mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny>(), It.IsAny())); + 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(); + mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny>(), It.IsAny())); + + 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>(), It.IsAny()), + 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(); + mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny>(), It.IsAny())); + + 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(result); + Assert.Equal(StatusCodes.Status404NotFound, result.StatusCode); + Assert.Same(input, result.Value); + mockHttpContext.Verify( + x => x.Response.OnResponseCompleted(It.IsAny>(), It.IsAny()), + 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(); + mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny>(), It.IsAny())); + + 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(result); + Assert.Equal(StatusCodes.Status400BadRequest, result.StatusCode); + Assert.Same(input, result.Value); + mockHttpContext.Verify( + x => x.Response.OnResponseCompleted(It.IsAny>(), It.IsAny()), + 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(); + mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny>(), It.IsAny())); + + 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(result); + Assert.Same(input, result.Value); + mockHttpContext.Verify( + x => x.Response.OnResponseCompleted(It.IsAny>(), It.IsAny()), + Times.Once()); + } + public static IEnumerable 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(); + } + } } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerUnitTestabilityTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerUnitTestabilityTests.cs index 9b77a73ebf..c127337a7a 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerUnitTestabilityTests.cs +++ b/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(); + mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny>(), It.IsAny())); + var controller = new TestabilityController() + { + ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor()) + }; // Act var result = controller.FileStream_Action(content, contentType, fileName); diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ActionResultTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ActionResultTests.cs index 58cec78c25..bb5469f07c 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ActionResultTests.cs +++ b/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> + { + new KeyValuePair("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); + } } } \ No newline at end of file diff --git a/test/WebSites/ActionResultsWebSite/Controllers/ActionResultsVerificationController.cs b/test/WebSites/ActionResultsWebSite/Controllers/ActionResultsVerificationController.cs index 8fddb320c3..2b5dbe4cda 100644 --- a/test/WebSites/ActionResultsWebSite/Controllers/ActionResultsVerificationController.cs +++ b/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; + } + } } } \ No newline at end of file diff --git a/test/WebSites/ActionResultsWebSite/GuidLookupService.cs b/test/WebSites/ActionResultsWebSite/GuidLookupService.cs new file mode 100644 index 0000000000..4d3a238efe --- /dev/null +++ b/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 IsDisposed { get; } = new Dictionary(); + } +} \ No newline at end of file diff --git a/test/WebSites/ActionResultsWebSite/Startup.cs b/test/WebSites/ActionResultsWebSite/Startup.cs index 767a359806..e8cef5ca62 100644 --- a/test/WebSites/ActionResultsWebSite/Startup.cs +++ b/test/WebSites/ActionResultsWebSite/Startup.cs @@ -16,6 +16,7 @@ namespace ActionResultsWebSite app.UseServices(services => { services.AddMvc(); + services.AddInstance(new GuidLookupService()); services.Configure(options => {