Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Commit fc9e1ca

Browse files
ajaybhargavbajaybhargavb
committed
[Fixes #1331] Register object for dispose in ObjectResult and
FileStreamResult
1 parent 1c00cfe commit fc9e1ca

File tree

7 files changed

+317
-7
lines changed

7 files changed

+317
-7
lines changed

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

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,12 @@ public virtual ContentResult Content(string content, string contentType, Encodin
376376
[NonAction]
377377
public virtual JsonResult Json(object data)
378378
{
379+
var disposableValue = data as IDisposable;
380+
if (disposableValue != null)
381+
{
382+
Response.OnResponseCompleted(_ => disposableValue.Dispose(), state: null);
383+
}
384+
379385
return new JsonResult(data);
380386
}
381387

@@ -659,6 +665,11 @@ public virtual FileStreamResult File(Stream fileStream, string contentType)
659665
[NonAction]
660666
public virtual FileStreamResult File(Stream fileStream, string contentType, string fileDownloadName)
661667
{
668+
if (fileStream != null)
669+
{
670+
Response.OnResponseCompleted(_ => fileStream.Dispose(), state: null);
671+
}
672+
662673
return new FileStreamResult(fileStream, contentType) { FileDownloadName = fileDownloadName };
663674
}
664675

@@ -717,6 +728,12 @@ public virtual HttpNotFoundResult HttpNotFound()
717728
[NonAction]
718729
public virtual HttpNotFoundObjectResult HttpNotFound(object value)
719730
{
731+
var disposableValue = value as IDisposable;
732+
if (disposableValue != null)
733+
{
734+
Response.OnResponseCompleted(_ => disposableValue.Dispose(), state: null);
735+
}
736+
720737
return new HttpNotFoundObjectResult(value);
721738
}
722739

@@ -737,6 +754,12 @@ public virtual BadRequestResult HttpBadRequest()
737754
[NonAction]
738755
public virtual BadRequestObjectResult HttpBadRequest(object error)
739756
{
757+
var disposableValue = error as IDisposable;
758+
if (disposableValue != null)
759+
{
760+
Response.OnResponseCompleted(_ => disposableValue.Dispose(), state: null);
761+
}
762+
740763
return new BadRequestObjectResult(error);
741764
}
742765

@@ -759,6 +782,12 @@ public virtual BadRequestObjectResult HttpBadRequest([NotNull] ModelStateDiction
759782
[NonAction]
760783
public virtual CreatedResult Created([NotNull] string uri, object value)
761784
{
785+
var disposableValue = value as IDisposable;
786+
if (disposableValue != null)
787+
{
788+
Response.OnResponseCompleted(_ => disposableValue.Dispose(), state: null);
789+
}
790+
762791
return new CreatedResult(uri, value);
763792
}
764793

@@ -780,7 +809,8 @@ public virtual CreatedResult Created([NotNull] Uri uri, object value)
780809
{
781810
location = uri.GetComponents(UriComponents.SerializationInfoString, UriFormat.UriEscaped);
782811
}
783-
return new CreatedResult(location, value);
812+
813+
return Created(location, value);
784814
}
785815

786816
/// <summary>
@@ -822,6 +852,12 @@ public virtual CreatedAtActionResult CreatedAtAction(string actionName,
822852
object routeValues,
823853
object value)
824854
{
855+
var disposableValue = value as IDisposable;
856+
if (disposableValue != null)
857+
{
858+
Response.OnResponseCompleted(_ => disposableValue.Dispose(), state: null);
859+
}
860+
825861
return new CreatedAtActionResult(actionName, controllerName, routeValues, value);
826862
}
827863

@@ -859,6 +895,12 @@ public virtual CreatedAtRouteResult CreatedAtRoute(object routeValues, object va
859895
[NonAction]
860896
public virtual CreatedAtRouteResult CreatedAtRoute(string routeName, object routeValues, object value)
861897
{
898+
var disposableValue = value as IDisposable;
899+
if (disposableValue != null)
900+
{
901+
Response.OnResponseCompleted(_ => disposableValue.Dispose(), state: null);
902+
}
903+
862904
return new CreatedAtRouteResult(routeName, routeValues, value);
863905
}
864906

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

Lines changed: 178 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,33 @@ public void Created_WithRelativeUriParameter_SetsCreatedLocation()
423423
Assert.Equal(uri.OriginalString, result.Location);
424424
}
425425

426+
[Fact]
427+
public void Created_IDisposableObject_RegistersForDispose()
428+
{
429+
// Arrange
430+
var mockHttpContext = new Mock<DefaultHttpContext>();
431+
mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
432+
var uri = new Uri("/test/url", UriKind.Relative);
433+
434+
var controller = new TestableController()
435+
{
436+
ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
437+
};
438+
var input = new DisposableObject();
439+
440+
// Act
441+
var result = controller.Created(uri, input);
442+
443+
// Assert
444+
Assert.IsType<CreatedResult>(result);
445+
Assert.Equal(StatusCodes.Status201Created, result.StatusCode);
446+
Assert.Equal(uri.OriginalString, result.Location);
447+
Assert.Same(input, result.Value);
448+
mockHttpContext.Verify(
449+
x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
450+
Times.Once());
451+
}
452+
426453
[Fact]
427454
public void CreatedAtAction_WithParameterActionName_SetsResultActionName()
428455
{
@@ -483,6 +510,32 @@ public void CreatedAtAction_WithActionControllerRouteValues_SetsSameValues()
483510
Assert.Equal(expected, result.RouteValues);
484511
}
485512

513+
[Fact]
514+
public void CreatedAtAction_IDisposableObject_RegistersForDispose()
515+
{
516+
// Arrange
517+
var mockHttpContext = new Mock<DefaultHttpContext>();
518+
mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
519+
520+
var controller = new TestableController()
521+
{
522+
ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
523+
};
524+
var input = new DisposableObject();
525+
526+
// Act
527+
var result = controller.CreatedAtAction("SampleAction", input);
528+
529+
// Assert
530+
Assert.IsType<CreatedAtActionResult>(result);
531+
Assert.Equal(StatusCodes.Status201Created, result.StatusCode);
532+
Assert.Equal("SampleAction", result.ActionName);
533+
Assert.Same(input, result.Value);
534+
mockHttpContext.Verify(
535+
x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
536+
Times.Once());
537+
}
538+
486539
[Fact]
487540
public void CreatedAtRoute_WithParameterRouteName_SetsResultSameRouteName()
488541
{
@@ -540,6 +593,32 @@ public void CreatedAtRoute_WithParameterRouteNameAndValues_SetsResultSamePropert
540593
Assert.Equal(expected, result.RouteValues);
541594
}
542595

596+
[Fact]
597+
public void CreatedAtRoute_IDisposableObject_RegistersForDispose()
598+
{
599+
// Arrange
600+
var mockHttpContext = new Mock<DefaultHttpContext>();
601+
mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
602+
603+
var controller = new TestableController()
604+
{
605+
ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
606+
};
607+
var input = new DisposableObject();
608+
609+
// Act
610+
var result = controller.CreatedAtRoute("SampleRoute", input);
611+
612+
// Assert
613+
Assert.IsType<CreatedAtRouteResult>(result);
614+
Assert.Equal(StatusCodes.Status201Created, result.StatusCode);
615+
Assert.Equal("SampleRoute", result.RouteName);
616+
Assert.Same(input, result.Value);
617+
mockHttpContext.Verify(
618+
x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
619+
Times.Once());
620+
}
621+
543622
[Fact]
544623
public void File_WithContents()
545624
{
@@ -612,7 +691,12 @@ public void File_WithPathAndFileDownloadName()
612691
public void File_WithStream()
613692
{
614693
// Arrange
615-
var controller = new TestableController();
694+
var mockHttpContext = new Mock<DefaultHttpContext>();
695+
mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
696+
var controller = new TestableController()
697+
{
698+
ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
699+
};
616700
var fileStream = Stream.Null;
617701

618702
// Act
@@ -629,7 +713,13 @@ public void File_WithStream()
629713
public void File_WithStreamAndFileDownloadName()
630714
{
631715
// Arrange
632-
var controller = new TestableController();
716+
var mockHttpContext = new Mock<DefaultHttpContext>();
717+
mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
718+
719+
var controller = new TestableController()
720+
{
721+
ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
722+
};
633723
var fileStream = Stream.Null;
634724

635725
// Act
@@ -640,6 +730,9 @@ public void File_WithStreamAndFileDownloadName()
640730
Assert.Same(fileStream, result.FileStream);
641731
Assert.Equal("someContentType", result.ContentType);
642732
Assert.Equal("someDownloadName", result.FileDownloadName);
733+
mockHttpContext.Verify(
734+
x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
735+
Times.Once());
643736
}
644737

645738
[Fact]
@@ -685,6 +778,31 @@ public void HttpNotFound_SetsStatusCodeAndResponseContent()
685778
Assert.Equal("Test Content", result.Value);
686779
}
687780

781+
[Fact]
782+
public void HttpNotFound_IDisposableObject_RegistersForDispose()
783+
{
784+
// Arrange
785+
var mockHttpContext = new Mock<DefaultHttpContext>();
786+
mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
787+
788+
var controller = new TestableController()
789+
{
790+
ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
791+
};
792+
var input = new DisposableObject();
793+
794+
// Act
795+
var result = controller.HttpNotFound(input);
796+
797+
// Assert
798+
Assert.IsType<HttpNotFoundObjectResult>(result);
799+
Assert.Equal(StatusCodes.Status404NotFound, result.StatusCode);
800+
Assert.Same(input, result.Value);
801+
mockHttpContext.Verify(
802+
x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
803+
Times.Once());
804+
}
805+
688806
[Fact]
689807
public void BadRequest_SetsStatusCode()
690808
{
@@ -715,6 +833,31 @@ public void BadRequest_SetsStatusCodeAndValue_Object()
715833
Assert.Equal(obj, result.Value);
716834
}
717835

836+
[Fact]
837+
public void BadRequest_IDisposableObject_RegistersForDispose()
838+
{
839+
// Arrange
840+
var mockHttpContext = new Mock<DefaultHttpContext>();
841+
mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
842+
843+
var controller = new TestableController()
844+
{
845+
ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
846+
};
847+
var input = new DisposableObject();
848+
849+
// Act
850+
var result = controller.HttpBadRequest(input);
851+
852+
// Assert
853+
Assert.IsType<BadRequestObjectResult>(result);
854+
Assert.Equal(StatusCodes.Status400BadRequest, result.StatusCode);
855+
Assert.Same(input, result.Value);
856+
mockHttpContext.Verify(
857+
x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
858+
Times.Once());
859+
}
860+
718861
[Fact]
719862
public void BadRequest_SetsStatusCodeAndValue_ModelState()
720863
{
@@ -888,6 +1031,30 @@ public void Controller_Json_WithParameterValue_SetsResultData()
8881031
Assert.Same(data, actualJsonResult.Value);
8891032
}
8901033

1034+
[Fact]
1035+
public void Controller_Json_IDisposableObject_RegistersForDispose()
1036+
{
1037+
// Arrange
1038+
var mockHttpContext = new Mock<DefaultHttpContext>();
1039+
mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
1040+
1041+
var controller = new TestableController()
1042+
{
1043+
ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
1044+
};
1045+
var input = new DisposableObject();
1046+
1047+
// Act
1048+
var result = controller.Json(input);
1049+
1050+
// Assert
1051+
Assert.IsType<JsonResult>(result);
1052+
Assert.Same(input, result.Value);
1053+
mockHttpContext.Verify(
1054+
x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()),
1055+
Times.Once());
1056+
}
1057+
8911058
public static IEnumerable<object[]> RedirectTestData
8921059
{
8931060
get
@@ -1402,7 +1569,7 @@ public void TryValidateModelWithInvalidModelWithPrefix_ReturnsFalse()
14021569
{
14031570
// Arrange
14041571
var model = new TryValidateModelModel();
1405-
var validationResult = new []
1572+
var validationResult = new[]
14061573
{
14071574
new ModelValidationResult(string.Empty, "Out of range!")
14081575
};
@@ -1569,5 +1736,13 @@ private class TestableController : Controller
15691736
{
15701737

15711738
}
1739+
1740+
private class DisposableObject : IDisposable
1741+
{
1742+
public void Dispose()
1743+
{
1744+
throw new NotImplementedException();
1745+
}
1746+
}
15721747
}
15731748
}

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.IO;
77
using System.Text;
88
using Microsoft.AspNet.Http;
9+
using Microsoft.AspNet.Http.Core;
910
using Microsoft.AspNet.Mvc.ModelBinding;
1011
using Microsoft.AspNet.Routing;
1112
using Microsoft.AspNet.WebUtilities;
@@ -144,7 +145,12 @@ public void ControllerFileContent_InvokedInUnitTests(string content, string cont
144145
public void ControllerFileStream_InvokedInUnitTests(string content, string contentType, string fileName)
145146
{
146147
// Arrange
147-
var controller = new TestabilityController();
148+
var mockHttpContext = new Mock<DefaultHttpContext>();
149+
mockHttpContext.Setup(x => x.Response.OnResponseCompleted(It.IsAny<Action<object>>(), It.IsAny<object>()));
150+
var controller = new TestabilityController()
151+
{
152+
ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor())
153+
};
148154

149155
// Act
150156
var result = controller.FileStream_Action(content, contentType, fileName);

0 commit comments

Comments
 (0)