From 6106375306f626c1f08323333602a4d5075e19c0 Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Tue, 21 Jul 2015 19:22:32 -0700 Subject: [PATCH] Add `HttpOkResult`, `HttpOkObjectResult`, and `Ok()` methods in `Controller` - #2825 - new class names align with existing types such as `HttpNotFoundResult` and `HttpNotFoundObjectResult` - remove similar types from WebApiCompatShim and use replacements in `ApiController` - `NegotiatedContentResult` remains because Core doesn't have an exact replacement nits: - add missing periods in some `Controller` doc comments --- .../HttpOkObjectResult.cs | 24 +++++ src/Microsoft.AspNet.Mvc.Core/HttpOkResult.cs | 22 +++++ .../Controller.cs | 59 ++++++++--- .../OkNegotiatedContentResult.cs | 27 ----- .../ActionResults/OkResult.cs | 22 ----- .../ApiController.cs | 16 +-- .../HttpOkObjectResultTest.cs | 99 +++++++++++++++++++ .../HttpOkResultTest.cs} | 21 ++-- .../ControllerTest.cs | 41 +++++++- .../OkNegotiatedContentResultTest.cs | 77 --------------- .../ApiControllerTest.cs | 6 +- .../Controllers/HomeController.cs | 2 +- 12 files changed, 255 insertions(+), 161 deletions(-) create mode 100644 src/Microsoft.AspNet.Mvc.Core/HttpOkObjectResult.cs create mode 100644 src/Microsoft.AspNet.Mvc.Core/HttpOkResult.cs delete mode 100644 src/Microsoft.AspNet.Mvc.WebApiCompatShim/ActionResults/OkNegotiatedContentResult.cs delete mode 100644 src/Microsoft.AspNet.Mvc.WebApiCompatShim/ActionResults/OkResult.cs create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/HttpOkObjectResultTest.cs rename test/{Microsoft.AspNet.Mvc.WebApiCompatShimTest/ActionResults/OkResultTest.cs => Microsoft.AspNet.Mvc.Core.Test/HttpOkResultTest.cs} (56%) delete mode 100644 test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/ActionResults/OkNegotiatedContentResultTest.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/HttpOkObjectResult.cs b/src/Microsoft.AspNet.Mvc.Core/HttpOkObjectResult.cs new file mode 100644 index 0000000000..5efefa2173 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/HttpOkObjectResult.cs @@ -0,0 +1,24 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNet.Http; + +namespace Microsoft.AspNet.Mvc +{ + /// + /// An that when executed performs content negotiation, formats the entity body, and + /// will produce a response if negotiation and formatting succeed. + /// + public class HttpOkObjectResult : ObjectResult + { + /// + /// Initializes a new instance of the class. + /// + /// The content to format into the entity body. + public HttpOkObjectResult(object value) + : base(value) + { + StatusCode = StatusCodes.Status200OK; + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/HttpOkResult.cs b/src/Microsoft.AspNet.Mvc.Core/HttpOkResult.cs new file mode 100644 index 0000000000..51482ca450 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/HttpOkResult.cs @@ -0,0 +1,22 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNet.Http; + +namespace Microsoft.AspNet.Mvc +{ + /// + /// An that when executed will produce an empty + /// response. + /// + public class HttpOkResult : HttpStatusCodeResult + { + /// + /// Initializes a new instance of the class. + /// + public HttpOkResult() + : base(StatusCodes.Status200OK) + { + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Controller.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/Controller.cs index c34773f95a..50c15a411d 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/Controller.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Controller.cs @@ -463,6 +463,33 @@ namespace Microsoft.AspNet.Mvc return new JsonResult(data, serializerSettings); } + /// + /// Creates a object that produces an empty OK (200) response. + /// + /// The created for the response. + [NonAction] + public virtual HttpOkResult Ok() + { + return new HttpOkResult(); + } + + /// + /// Creates an object that produces an OK (200) response. + /// + /// The content value to format in the entity body. + /// The created for the response. + [NonAction] + public virtual HttpOkObjectResult Ok(object value) + { + var disposableValue = value as IDisposable; + if (disposableValue != null) + { + Response.RegisterForDispose(disposableValue); + } + + return new HttpOkObjectResult(value); + } + /// /// Creates a object that redirects to the specified . /// @@ -1025,7 +1052,7 @@ namespace Microsoft.AspNet.Mvc /// /// The type of the model object. /// The model instance to update. - /// A that on completion returns true if the update is successful + /// A that on completion returns true if the update is successful. [NonAction] public virtual Task TryUpdateModelAsync([NotNull] TModel model) where TModel : class @@ -1039,9 +1066,9 @@ namespace Microsoft.AspNet.Mvc /// /// The type of the model object. /// The model instance to update. - /// The prefix to use when looking up values in the current + /// The prefix to use when looking up values in the current . /// - /// A that on completion returns true if the update is successful + /// A that on completion returns true if the update is successful. [NonAction] public virtual async Task TryUpdateModelAsync([NotNull] TModel model, [NotNull] string prefix) @@ -1067,7 +1094,7 @@ namespace Microsoft.AspNet.Mvc /// The prefix to use when looking up values in the . /// /// The used for looking up values. - /// A that on completion returns true if the update is successful + /// A that on completion returns true if the update is successful. [NonAction] public virtual async Task TryUpdateModelAsync([NotNull] TModel model, [NotNull] string prefix, @@ -1105,7 +1132,7 @@ namespace Microsoft.AspNet.Mvc /// /// (s) which represent top-level properties /// which need to be included for the current model. - /// A that on completion returns true if the update is successful + /// A that on completion returns true if the update is successful. [NonAction] public async Task TryUpdateModelAsync( [NotNull] TModel model, @@ -1144,7 +1171,7 @@ namespace Microsoft.AspNet.Mvc /// The prefix to use when looking up values in the current . /// /// A predicate which can be used to filter properties at runtime. - /// A that on completion returns true if the update is successful + /// A that on completion returns true if the update is successful. [NonAction] public async Task TryUpdateModelAsync( [NotNull] TModel model, @@ -1180,12 +1207,12 @@ namespace Microsoft.AspNet.Mvc /// /// The type of the model object. /// The model instance to update. - /// The prefix to use when looking up values in the + /// The prefix to use when looking up values in the . /// /// The used for looking up values. /// (s) which represent top-level properties /// which need to be included for the current model. - /// A that on completion returns true if the update is successful + /// A that on completion returns true if the update is successful. [NonAction] public async Task TryUpdateModelAsync( [NotNull] TModel model, @@ -1222,11 +1249,11 @@ namespace Microsoft.AspNet.Mvc /// /// The type of the model object. /// The model instance to update. - /// The prefix to use when looking up values in the + /// The prefix to use when looking up values in the . /// /// The used for looking up values. /// A predicate which can be used to filter properties at runtime. - /// A that on completion returns true if the update is successful + /// A that on completion returns true if the update is successful. [NonAction] public async Task TryUpdateModelAsync( [NotNull] TModel model, @@ -1263,9 +1290,9 @@ namespace Microsoft.AspNet.Mvc /// /// The model instance to update. /// The type of model instance to update. - /// The prefix to use when looking up values in the current + /// The prefix to use when looking up values in the current . /// - /// A that on completion returns true if the update is successful + /// A that on completion returns true if the update is successful. [NonAction] public virtual async Task TryUpdateModelAsync([NotNull] object model, [NotNull] Type modelType, @@ -1299,11 +1326,11 @@ namespace Microsoft.AspNet.Mvc /// /// The model instance to update. /// The type of model instance to update. - /// The prefix to use when looking up values in the + /// The prefix to use when looking up values in the . /// /// The used for looking up values. /// A predicate which can be used to filter properties at runtime. - /// A that on completion returns true if the update is successful + /// A that on completion returns true if the update is successful. [NonAction] public async Task TryUpdateModelAsync( [NotNull] object model, @@ -1339,7 +1366,7 @@ namespace Microsoft.AspNet.Mvc /// Validates the specified instance. /// /// The model to validate. - /// true if the is valid; false otherwise. + /// true if the is valid; false otherwise. [NonAction] public virtual bool TryValidateModel([NotNull] object model) { @@ -1352,7 +1379,7 @@ namespace Microsoft.AspNet.Mvc /// The model to validate. /// The key to use when looking up information in . /// - /// true if the is valid;false otherwise. + /// true if the is valid;false otherwise. [NonAction] public virtual bool TryValidateModel([NotNull] object model, string prefix) { diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ActionResults/OkNegotiatedContentResult.cs b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ActionResults/OkNegotiatedContentResult.cs deleted file mode 100644 index be288e604b..0000000000 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ActionResults/OkNegotiatedContentResult.cs +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Net; -using Microsoft.AspNet.Mvc; -using Microsoft.Framework.Internal; - -namespace System.Web.Http -{ - /// - /// Represents an action result that performs content negotiation and returns an - /// response when it succeeds. - /// - /// The type of content in the entity body. - public class OkNegotiatedContentResult : NegotiatedContentResult - { - /// - /// Initializes a new instance of the class with the values - /// provided. - /// - /// The content value to negotiate and format in the entity body. - public OkNegotiatedContentResult([NotNull] T content) - : base(HttpStatusCode.OK, content) - { - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ActionResults/OkResult.cs b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ActionResults/OkResult.cs deleted file mode 100644 index 17989ad863..0000000000 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ActionResults/OkResult.cs +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.AspNet.Http; -using Microsoft.AspNet.Mvc; - -namespace System.Web.Http -{ - /// - /// An action result that returns an empty response. - /// - public class OkResult : HttpStatusCodeResult - { - /// - /// Initializes a new instance of the class. - /// - public OkResult() - : base(StatusCodes.Status200OK) - { - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs index 51ee047790..778f8dc1a0 100644 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs +++ b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs @@ -295,25 +295,25 @@ namespace System.Web.Http } /// - /// Creates an (200 OK). + /// Creates an (200 OK). /// - /// An . + /// An . [NonAction] - public virtual OkResult Ok() + public virtual HttpOkResult Ok() { - return new OkResult(); + return new HttpOkResult(); } /// - /// Creates an (200 OK) with the specified values. + /// Creates an (200 OK) with the specified values. /// /// The type of content in the entity body. /// The content value to negotiate and format in the entity body. - /// An with the specified values. + /// An with the specified values. [NonAction] - public virtual OkNegotiatedContentResult Ok([NotNull] T content) + public virtual HttpOkObjectResult Ok(T content) { - return new OkNegotiatedContentResult(content); + return new HttpOkObjectResult(content); } /// diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/HttpOkObjectResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/HttpOkObjectResultTest.cs new file mode 100644 index 0000000000..ec1f3b7d36 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/HttpOkObjectResultTest.cs @@ -0,0 +1,99 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Threading.Tasks; +using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Internal; +using Microsoft.AspNet.Routing; +using Microsoft.Framework.DependencyInjection; +using Microsoft.Framework.Logging; +using Microsoft.Framework.Logging.Testing; +using Microsoft.Framework.OptionsModel; +using Xunit; + +namespace Microsoft.AspNet.Mvc +{ + public class HttpOkObjectResultTest + { + public static TheoryData ValuesData + { + get + { + return new TheoryData + { + null, + "Test string", + new Person + { + Id = 274, + Name = "George", + } + }; + } + } + + [Theory] + [MemberData(nameof(ValuesData))] + public void HttpOkObjectResult_InitializesStatusCodeAndValue(object value) + { + // Arrange & Act + var result = new HttpOkObjectResult(value); + + // Assert + Assert.Equal(StatusCodes.Status200OK, result.StatusCode); + Assert.Same(value, result.Value); + } + + [Theory] + [MemberData(nameof(ValuesData))] + public async Task HttpOkObjectResult_SetsStatusCode(object value) + { + // Arrange + var result = new HttpOkObjectResult(value); + + var httpContext = new DefaultHttpContext + { + RequestServices = CreateServices(), + }; + var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + + // Act + await result.ExecuteResultAsync(actionContext); + + // Assert + Assert.Equal(StatusCodes.Status200OK, httpContext.Response.StatusCode); + } + + private IServiceProvider CreateServices() + { + var services = new ServiceCollection(); + services.Add(new ServiceDescriptor( + typeof(ILogger), + new Logger(NullLoggerFactory.Instance))); + + var optionsAccessor = new MockMvcOptionsAccessor(); + optionsAccessor.Options.OutputFormatters.Add(new JsonOutputFormatter()); + services.Add(new ServiceDescriptor(typeof(IOptions), optionsAccessor)); + + var bindingContext = new ActionBindingContext + { + OutputFormatters = optionsAccessor.Options.OutputFormatters, + }; + var bindingContextAccessor = new MockScopedInstance + { + Value = bindingContext, + }; + services.Add(new ServiceDescriptor(typeof(IScopedInstance), bindingContextAccessor)); + + return services.BuildServiceProvider(); + } + + private class Person + { + public int Id { get; set; } + + public string Name { get; set; } + } + } +} diff --git a/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/ActionResults/OkResultTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/HttpOkResultTest.cs similarity index 56% rename from test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/ActionResults/OkResultTest.cs rename to test/Microsoft.AspNet.Mvc.Core.Test/HttpOkResultTest.cs index 0b1679d75a..5f59e26dce 100644 --- a/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/ActionResults/OkResultTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/HttpOkResultTest.cs @@ -1,23 +1,32 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Http.Internal; -using Microsoft.AspNet.Mvc; using Microsoft.AspNet.Routing; using Xunit; -namespace System.Web.Http +namespace Microsoft.AspNet.Mvc { - public class OkResultTest + public class HttpOkResultTest { [Fact] - public async Task OkResult_SetsStatusCode() + public void HttpOkResult_InitializesStatusCode() + { + // Arrange & Act + var result = new HttpOkResult(); + + // Assert + Assert.Equal(StatusCodes.Status200OK, result.StatusCode); + } + + [Fact] + public async Task HttpOkResult_SetsStatusCode() { // Arrange var context = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()); - var result = new OkResult(); + var result = new HttpOkResult(); // Act await result.ExecuteResultAsync(context); diff --git a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ControllerTest.cs b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ControllerTest.cs index acfbcf3044..d132c719cf 100644 --- a/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ControllerTest.cs +++ b/test/Microsoft.AspNet.Mvc.ViewFeatures.Test/ControllerTest.cs @@ -739,7 +739,7 @@ namespace Microsoft.AspNet.Mvc.Test [Fact] public void HttpUnauthorized_SetsStatusCode() { - // Arrange + // Arrange var controller = new TestableController(); // Act @@ -804,6 +804,45 @@ namespace Microsoft.AspNet.Mvc.Test Times.Once()); } + [Fact] + public void Ok_SetsStatusCode() + { + // Arrange + var controller = new TestableController(); + + // Act + var result = controller.Ok(); + + // Assert + Assert.IsType(result); + Assert.Equal(StatusCodes.Status200OK, result.StatusCode); + } + + [Fact] + public void Ok_WithIDisposableObject_RegistersForDispose() + { + // Arrange + var mockHttpContext = new Mock(); + mockHttpContext.Setup(x => x.Response.RegisterForDispose(It.IsAny())); + + var controller = new TestableController() + { + ActionContext = new ActionContext(mockHttpContext.Object, new RouteData(), new ActionDescriptor()) + }; + var input = new DisposableObject(); + + // Act + var result = controller.Ok(input); + + // Assert + Assert.IsType(result); + Assert.Equal(StatusCodes.Status200OK, result.StatusCode); + Assert.Same(input, result.Value); + mockHttpContext.Verify( + x => x.Response.RegisterForDispose(It.IsAny()), + Times.Once()); + } + [Fact] public void BadRequest_SetsStatusCode() { diff --git a/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/ActionResults/OkNegotiatedContentResultTest.cs b/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/ActionResults/OkNegotiatedContentResultTest.cs deleted file mode 100644 index a52f2baf1e..0000000000 --- a/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/ActionResults/OkNegotiatedContentResultTest.cs +++ /dev/null @@ -1,77 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -#if DNX451 -using System.IO; -using System.Threading.Tasks; -using Microsoft.AspNet.Http; -using Microsoft.AspNet.Http.Internal; -using Microsoft.AspNet.Mvc; -using Microsoft.AspNet.Routing; -using Microsoft.Framework.Logging; -using Microsoft.Framework.OptionsModel; -using Moq; -using Xunit; - -namespace System.Web.Http -{ - public class OkNegotiatedContentResultTest - { - [Fact] - public async Task OkNegotiatedContentResult_SetsStatusCode() - { - // Arrange - var httpContext = new DefaultHttpContext(); - httpContext.RequestServices = CreateServices(); - - var stream = new MemoryStream(); - httpContext.Response.Body = stream; - - var context = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); - var result = new OkNegotiatedContentResult(new Product()); - - // Act - await result.ExecuteResultAsync(context); - - // Assert - Assert.Equal(StatusCodes.Status200OK, context.HttpContext.Response.StatusCode); - } - - private IServiceProvider CreateServices() - { - var services = new Mock(MockBehavior.Strict); - - var options = new MvcOptions(); - options.OutputFormatters.Add(new JsonOutputFormatter()); - - var optionsAccessor = new Mock>(); - optionsAccessor.SetupGet(o => o.Options) - .Returns(options); - - var mockActionBindingContext = new Mock>(); - var bindingContext = new ActionBindingContext { OutputFormatters = options.OutputFormatters }; - mockActionBindingContext - .SetupGet(o => o.Value) - .Returns(bindingContext); - - services.Setup(o => o.GetService(typeof(IScopedInstance))) - .Returns(mockActionBindingContext.Object); - - services.Setup(s => s.GetService(typeof(IOptions))) - .Returns(optionsAccessor.Object); - - services.Setup(s => s.GetService(typeof(ILogger))) - .Returns(new Mock>().Object); - - return services.Object; - } - - private class Product - { - public int Id { get; set; } - - public string Name { get; set; } - }; - } -} -#endif diff --git a/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/ApiControllerTest.cs b/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/ApiControllerTest.cs index 48dc8e8c11..804b99850c 100644 --- a/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/ApiControllerTest.cs +++ b/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/ApiControllerTest.cs @@ -290,7 +290,7 @@ namespace System.Web.Http var result = controller.Ok(); // Assert - Assert.Equal(200, Assert.IsType(result).StatusCode); + Assert.Equal(200, Assert.IsType(result).StatusCode); } @@ -305,8 +305,8 @@ namespace System.Web.Http var result = controller.Ok(product); // Assert - var okResult = Assert.IsType>(result); - Assert.Same(product, okResult.Content); + var okResult = Assert.IsType(result); + Assert.Same(product, okResult.Value); } [Fact] diff --git a/test/WebSites/BasicWebSite/Controllers/HomeController.cs b/test/WebSites/BasicWebSite/Controllers/HomeController.cs index cf36d62216..35f4a4da7d 100644 --- a/test/WebSites/BasicWebSite/Controllers/HomeController.cs +++ b/test/WebSites/BasicWebSite/Controllers/HomeController.cs @@ -51,7 +51,7 @@ namespace BasicWebSite.Controllers [RequireHttps] public IActionResult HttpsOnlyAction() { - return new HttpStatusCodeResult(StatusCodes.Status200OK); + return Ok(); } public Task ActionReturningTask()