From 8193ecc0758145635e09ed2d277474b71a8badb2 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 11 Mar 2022 17:10:09 -0800 Subject: [PATCH 01/21] Initial functionality --- .../src/ApiResponseTypeProvider.cs | 5 +- src/Mvc/Mvc.Core/src/ActionResultOfT.cs | 6 ++- .../Mvc.Core/src/HttpResultsActionResult.cs | 31 ++++++++++++ .../Infrastructure/ActionResultTypeMapper.cs | 7 +++ .../src/Microsoft.AspNetCore.Mvc.Core.csproj | 1 + src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt | 4 ++ src/Mvc/Mvc.Core/test/ActionResultOfTTest.cs | 17 +++++++ .../Mvc.Core/test/HttpResultsResultTest.cs | 50 +++++++++++++++++++ .../ActionResultTypeMapperTest.cs | 19 ++++++- 9 files changed, 135 insertions(+), 5 deletions(-) create mode 100644 src/Mvc/Mvc.Core/src/HttpResultsActionResult.cs create mode 100644 src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs diff --git a/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs index 0c58956eccce..1d2df05819b1 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs @@ -284,9 +284,10 @@ internal static void CalculateResponseFormatForType(ApiResponseType apiResponse, unwrappedType = declaredReturnType.GetGenericArguments()[0]; } - // If the method is declared to return IActionResult or a derived class, that information + // If the method is declared to return IActionResult, IResult or a derived class, that information // isn't valuable to the formatter. - if (typeof(IActionResult).IsAssignableFrom(unwrappedType)) + if (typeof(IActionResult).IsAssignableFrom(unwrappedType) || + typeof(IResult).IsAssignableFrom(unwrappedType)) { return null; } diff --git a/src/Mvc/Mvc.Core/src/ActionResultOfT.cs b/src/Mvc/Mvc.Core/src/ActionResultOfT.cs index cc942b2aa3d5..8086cadfd0ae 100644 --- a/src/Mvc/Mvc.Core/src/ActionResultOfT.cs +++ b/src/Mvc/Mvc.Core/src/ActionResultOfT.cs @@ -21,7 +21,8 @@ public sealed class ActionResult : IConvertToActionResult /// The value. public ActionResult(TValue value) { - if (typeof(IActionResult).IsAssignableFrom(typeof(TValue))) + if (typeof(IActionResult).IsAssignableFrom(typeof(TValue)) || + typeof(IResult).IsAssignableFrom(typeof(TValue))) { var error = Resources.FormatInvalidTypeTForActionResultOfT(typeof(TValue), "ActionResult"); throw new ArgumentException(error); @@ -36,7 +37,8 @@ public ActionResult(TValue value) /// The . public ActionResult(ActionResult result) { - if (typeof(IActionResult).IsAssignableFrom(typeof(TValue))) + if (typeof(IActionResult).IsAssignableFrom(typeof(TValue)) || + typeof(IResult).IsAssignableFrom(typeof(TValue))) { var error = Resources.FormatInvalidTypeTForActionResultOfT(typeof(TValue), "ActionResult"); throw new ArgumentException(error); diff --git a/src/Mvc/Mvc.Core/src/HttpResultsActionResult.cs b/src/Mvc/Mvc.Core/src/HttpResultsActionResult.cs new file mode 100644 index 000000000000..deeef8e2a796 --- /dev/null +++ b/src/Mvc/Mvc.Core/src/HttpResultsActionResult.cs @@ -0,0 +1,31 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Mvc; + +/// +/// An that when executed will produce a response based on the provided. +/// +public sealed class HttpResultsActionResult : ActionResult +{ + /// + /// Gets the instance of the current . + /// + public IResult Result { get; } + + /// + /// Initializes a new instance of the class with the + /// provided. + /// + /// The instance to be used during the invocation. + public HttpResultsActionResult(IResult result) + { + Result = result; + } + + /// + public override Task ExecuteResultAsync(ActionContext context) + => Result.ExecuteAsync(context.HttpContext); +} diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/ActionResultTypeMapper.cs b/src/Mvc/Mvc.Core/src/Infrastructure/ActionResultTypeMapper.cs index 97645af4704e..8f233b9a4f35 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/ActionResultTypeMapper.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/ActionResultTypeMapper.cs @@ -3,6 +3,8 @@ #nullable enable +using Microsoft.AspNetCore.Http; + namespace Microsoft.AspNetCore.Mvc.Infrastructure; internal class ActionResultTypeMapper : IActionResultTypeMapper @@ -35,6 +37,11 @@ public IActionResult Convert(object? value, Type returnType) return converter.Convert(); } + if (value is IResult result) + { + return new HttpResultsActionResult(result); + } + return new ObjectResult(value) { DeclaredType = returnType, diff --git a/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj b/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj index 9d9b2ac58e91..e572494c1d14 100644 --- a/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj +++ b/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj @@ -46,6 +46,7 @@ Microsoft.AspNetCore.Mvc.RouteAttribute + diff --git a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt index 80b80b352063..0c949c990354 100644 --- a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt @@ -3,6 +3,9 @@ Microsoft.AspNetCore.Mvc.ApiBehaviorOptions.DisableImplicitFromServicesParameters.get -> bool Microsoft.AspNetCore.Mvc.ApiBehaviorOptions.DisableImplicitFromServicesParameters.set -> void Microsoft.AspNetCore.Mvc.ApplicationModels.InferParameterBindingInfoConvention.InferParameterBindingInfoConvention(Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider! modelMetadataProvider, Microsoft.Extensions.DependencyInjection.IServiceProviderIsService! serviceProviderIsService) -> void +Microsoft.AspNetCore.Mvc.HttpResultsActionResult +Microsoft.AspNetCore.Mvc.HttpResultsActionResult.HttpResultsActionResult(Microsoft.AspNetCore.Http.IResult! result) -> void +Microsoft.AspNetCore.Mvc.HttpResultsActionResult.Result.get -> Microsoft.AspNetCore.Http.IResult! Microsoft.AspNetCore.Mvc.ModelBinding.Binders.TryParseModelBinderProvider Microsoft.AspNetCore.Mvc.ModelBinding.Binders.TryParseModelBinderProvider.GetBinder(Microsoft.AspNetCore.Mvc.ModelBinding.ModelBinderProviderContext! context) -> Microsoft.AspNetCore.Mvc.ModelBinding.IModelBinder? Microsoft.AspNetCore.Mvc.ModelBinding.Binders.TryParseModelBinderProvider.TryParseModelBinderProvider() -> void @@ -13,6 +16,7 @@ Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataP Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.SystemTextJsonValidationMetadataProvider(System.Text.Json.JsonNamingPolicy! namingPolicy) -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ValidationMetadata.ValidationModelName.get -> string? Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ValidationMetadata.ValidationModelName.set -> void +override Microsoft.AspNetCore.Mvc.HttpResultsActionResult.ExecuteResultAsync(Microsoft.AspNetCore.Mvc.ActionContext! context) -> System.Threading.Tasks.Task! virtual Microsoft.AspNetCore.Mvc.Infrastructure.ConfigureCompatibilityOptions.PostConfigure(string? name, TOptions! options) -> void *REMOVED*~Microsoft.AspNetCore.Mvc.Formatters.FormatFilter.FormatFilter(Microsoft.Extensions.Options.IOptions! options, Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void Microsoft.AspNetCore.Mvc.Formatters.FormatFilter.FormatFilter(Microsoft.Extensions.Options.IOptions! options, Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void diff --git a/src/Mvc/Mvc.Core/test/ActionResultOfTTest.cs b/src/Mvc/Mvc.Core/test/ActionResultOfTTest.cs index 768725145524..e6a4c78cac5e 100644 --- a/src/Mvc/Mvc.Core/test/ActionResultOfTTest.cs +++ b/src/Mvc/Mvc.Core/test/ActionResultOfTTest.cs @@ -30,6 +30,17 @@ public void Constructor_WithActionResult_ThrowsForInvalidType() Assert.Equal($"Invalid type parameter '{typeof(FileStreamResult)}' specified for 'ActionResult'.", ex.Message); } + [Fact] + public void Constructor_WithIResult_ThrowsForInvalidType() + { + // Arrange + var result = new TestResult(); + + // Act & Assert + var ex = Assert.Throws(() => new ActionResult(value: result)); + Assert.Equal($"Invalid type parameter '{typeof(TestResult)}' specified for 'ActionResult'.", ex.Message); + } + [Fact] public void Convert_ReturnsResultIfSet() { @@ -106,4 +117,10 @@ private class BaseItem private class DerivedItem : BaseItem { } + + private class TestResult : IResult + { + public Task ExecuteAsync(HttpContext httpContext) + => Task.CompletedTask; + } } diff --git a/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs b/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs new file mode 100644 index 000000000000..96326710f9e1 --- /dev/null +++ b/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs @@ -0,0 +1,50 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; + +namespace Microsoft.AspNetCore.Mvc; + +public class HttpResultsResultTest +{ + [Fact] + public void HttpResultsResult_InitializesWithResultsStaticMethods() + { + // Arrange & Act + var result = new HttpResultsActionResult(Results.Ok()); + + // Assert + Assert.NotNull(result.Result); + } + + [Fact] + public async Task HttpResultsResult_SetsStatusCode() + { + // Arrange + var httpContext = new DefaultHttpContext + { + RequestServices = CreateServices().BuildServiceProvider() + }; + + var context = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + var result = new HttpResultsActionResult(Results.StatusCode(StatusCodes.Status400BadRequest)); + + // Act + await result.ExecuteResultAsync(context); + + // Assert + Assert.Equal(StatusCodes.Status400BadRequest, context.HttpContext.Response.StatusCode); + } + + private static IServiceCollection CreateServices() + { + var services = new ServiceCollection(); + services.AddSingleton(NullLoggerFactory.Instance); + return services; + } +} diff --git a/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs b/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs index b739e829f6d2..05a6d1140166 100644 --- a/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs +++ b/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs @@ -1,6 +1,7 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.AspNetCore.Http; using Moq; namespace Microsoft.AspNetCore.Mvc.Infrastructure; @@ -23,6 +24,22 @@ public void Convert_WithIConvertToActionResult_DelegatesToInterface() Assert.Same(expected, result); } + [Fact] + public void Convert_WithIResult_DelegatesToInterface() + { + // Arrange + var mapper = new ActionResultTypeMapper(); + + var returnValue = Mock.Of(); + + // Act + var result = mapper.Convert(returnValue, returnValue.GetType()); + + // Assert + var httpResultsResult = Assert.IsType(result); + Assert.Same(returnValue, httpResultsResult.Result); + } + [Fact] public void Convert_WithRegularType_CreatesObjectResult() { From 97dab79738904e941fc2942f28a9369907649f5c Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 14 Mar 2022 16:45:21 -0700 Subject: [PATCH 02/21] Adding functional test --- .../Mvc.FunctionalTests/HttpResultsTests.cs | 62 +++++++++++++++++++ .../Controllers/ContactApiController.cs | 15 +++++ 2 files changed, 77 insertions(+) create mode 100644 src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs diff --git a/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs b/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs new file mode 100644 index 000000000000..8943e3338cdb --- /dev/null +++ b/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs @@ -0,0 +1,62 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Net; +using System.Net.Http; +using BasicWebSite.Models; +using Newtonsoft.Json; + +namespace Microsoft.AspNetCore.Mvc.FunctionalTests; + +public class HttpResultsTests : IClassFixture> +{ + public HttpResultsTests(MvcTestFixture fixture) + { + Client = fixture.CreateDefaultClient(); + } + + public HttpClient Client { get; } + + [Fact] + public async Task ActionCanReturnHttpResultsActionResult() + { + // Arrange + var id = 1; + var url = $"/contact/{nameof(BasicWebSite.ContactApiController.ActionReturningHttpResultsActionResult)}/{id}"; + var response = await Client.GetAsync(url); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + var result = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); + Assert.NotNull(result); + Assert.Equal(id, result.ContactId); + } + + [Fact] + public async Task ActionCanReturnIResultWithContent() + { + // Arrange + var id = 1; + var url = $"/contact/{nameof(BasicWebSite.ContactApiController.ActionReturningObjectIResult)}/{id}"; + var response = await Client.GetAsync(url); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + var result = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); + Assert.NotNull(result); + Assert.Equal(id, result.ContactId); + } + + [Fact] + public async Task ActionCanReturnIResultWithStatusCodeOnly() + { + // Arrange + var url = $"/contact/{nameof(BasicWebSite.ContactApiController.ActionReturningStatusCodeIResult)}"; + var response = await Client.GetAsync(url); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.NoContent); + + } + +} diff --git a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs index 18cdc433222c..0a5e4719f592 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs @@ -128,6 +128,21 @@ public ActionResult ActionReturningValidationProblemDetails() }); } + [HttpGet("[action]/{id}")] + public IResult ActionReturningObjectIResult(int id) + => Results.Ok(new Contact() { ContactId = id }); + + [HttpGet("[action]/{id}")] + public ActionResult ActionReturningHttpResultsActionResult(int id) + { + var result = Results.Ok(new Contact() { ContactId = id }); + return new HttpResultsActionResult(result); + } + + [HttpGet("[action]")] + public IResult ActionReturningStatusCodeIResult() + => Results.NoContent(); + private class TestModelBinder : IModelBinder { public Task BindModelAsync(ModelBindingContext bindingContext) From 796aff3aaae6eac4adb22467c4311f5cfa1076dc Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Tue, 15 Mar 2022 16:56:05 -0700 Subject: [PATCH 03/21] Adding a test with filter --- .../Mvc.FunctionalTests/HttpResultsTests.cs | 23 +++++++++++++++++++ .../Controllers/FiltersController.cs | 6 +++++ 2 files changed, 29 insertions(+) diff --git a/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs b/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs index 8943e3338cdb..5b930c6abba5 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs @@ -56,7 +56,30 @@ public async Task ActionCanReturnIResultWithStatusCodeOnly() // Assert await response.AssertStatusCodeAsync(HttpStatusCode.NoContent); + } + [Fact] + public async Task ActionCanHaveAFilterForIResult() + { + // Arrange + var url = $"/filters/{nameof(BasicWebSite.Controllers.FiltersController.HttpResultsFilterForIResult)}"; + var response = await Client.GetAsync(url); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + Assert.Single(response.Headers.GetValues("X-HttpResultType")); + } + + [Fact] + public async Task ActionCanHaveAFilterForHttpResultsActionResult() + { + // Arrange + var url = $"/filters/{nameof(BasicWebSite.Controllers.FiltersController.HttpResultsFilter)}"; + var response = await Client.GetAsync(url); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + Assert.Single(response.Headers.GetValues("X-HttpResultType")); } } diff --git a/src/Mvc/test/WebSites/BasicWebSite/Controllers/FiltersController.cs b/src/Mvc/test/WebSites/BasicWebSite/Controllers/FiltersController.cs index dbb4e760e0a2..e1b7d23bc2a2 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/Controllers/FiltersController.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/Controllers/FiltersController.cs @@ -27,4 +27,10 @@ public IActionResult MiddlewareFilterTest() { return Content($"CurrentCulture:{CultureInfo.CurrentCulture.Name},CurrentUICulture:{CultureInfo.CurrentUICulture.Name}"); } + + [HttpResultsFilter] + public IResult HttpResultsFilterForIResult() => Results.NoContent(); + + [HttpResultsFilter] + public IActionResult HttpResultsFilter() => new HttpResultsActionResult(Results.NoContent()); } From 6a26ae13dbdf1a9367fbda5cfc126277532cebe4 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Tue, 15 Mar 2022 17:07:54 -0700 Subject: [PATCH 04/21] adding test filter --- .../Filters/HttpResultsResultFilter.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 src/Mvc/test/WebSites/BasicWebSite/Filters/HttpResultsResultFilter.cs diff --git a/src/Mvc/test/WebSites/BasicWebSite/Filters/HttpResultsResultFilter.cs b/src/Mvc/test/WebSites/BasicWebSite/Filters/HttpResultsResultFilter.cs new file mode 100644 index 000000000000..46e6c29c1fc6 --- /dev/null +++ b/src/Mvc/test/WebSites/BasicWebSite/Filters/HttpResultsResultFilter.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; + +namespace BasicWebSite; + +[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)] +public class HttpResultsFilter : Attribute, IResultFilter +{ + public void OnResultExecuted(ResultExecutedContext context) + { + } + + public void OnResultExecuting(ResultExecutingContext context) + { + if (context is { Result: HttpResultsActionResult { Result: IResult result } }) + { + context.Result = new StatusCodeResult(StatusCodes.Status200OK); + context.HttpContext.Response.Headers["X-HttpResultType"] = result.GetType().Name; + } + } +} From cf0d3653b5b01e37b65a123ef9ed520c9f747351 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 17 Mar 2022 13:17:40 -0700 Subject: [PATCH 05/21] PR feedback --- src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs | 10 ++++++++-- src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs | 6 +++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs b/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs index 96326710f9e1..2cd16424983d 100644 --- a/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs +++ b/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs @@ -7,6 +7,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Moq; namespace Microsoft.AspNetCore.Mvc; @@ -32,13 +33,18 @@ public async Task HttpResultsResult_SetsStatusCode() }; var context = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); - var result = new HttpResultsActionResult(Results.StatusCode(StatusCodes.Status400BadRequest)); + + var httpResult = new Mock(); + httpResult.Setup(s => s.ExecuteAsync(httpContext)) + .Returns(() => Task.CompletedTask) + .Verifiable(); + var result = new HttpResultsActionResult(httpResult.Object); // Act await result.ExecuteResultAsync(context); // Assert - Assert.Equal(StatusCodes.Status400BadRequest, context.HttpContext.Response.StatusCode); + httpResult.Verify(); } private static IServiceCollection CreateServices() diff --git a/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs b/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs index 5b930c6abba5..4fec01144ef0 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs @@ -3,8 +3,8 @@ using System.Net; using System.Net.Http; +using System.Net.Http.Json; using BasicWebSite.Models; -using Newtonsoft.Json; namespace Microsoft.AspNetCore.Mvc.FunctionalTests; @@ -27,7 +27,7 @@ public async Task ActionCanReturnHttpResultsActionResult() // Assert await response.AssertStatusCodeAsync(HttpStatusCode.OK); - var result = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); + var result = await response.Content.ReadFromJsonAsync(); Assert.NotNull(result); Assert.Equal(id, result.ContactId); } @@ -42,7 +42,7 @@ public async Task ActionCanReturnIResultWithContent() // Assert await response.AssertStatusCodeAsync(HttpStatusCode.OK); - var result = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); + var result = await response.Content.ReadFromJsonAsync(); Assert.NotNull(result); Assert.Equal(id, result.ContactId); } From 9e5a1d41554e5d74924c4c231d368dfc90c8d891 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 17 Mar 2022 13:18:51 -0700 Subject: [PATCH 06/21] remove empty line --- src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs b/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs index 4fec01144ef0..9a952f66cb30 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs @@ -81,5 +81,4 @@ public async Task ActionCanHaveAFilterForHttpResultsActionResult() await response.AssertStatusCodeAsync(HttpStatusCode.OK); Assert.Single(response.Headers.GetValues("X-HttpResultType")); } - } From d1f16f7202b9c1958ead9f893e6b961e512a13a0 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Thu, 17 Mar 2022 16:32:11 -0700 Subject: [PATCH 07/21] Adding tests and IResult default metadata --- .../src/ApiResponseTypeProvider.cs | 3 +- .../test/ApiResponseTypeProviderTest.cs | 18 ++++++- .../ApiBehaviorApplicationModelProvider.cs | 2 + .../HttpResultMetadataConvention.cs | 49 +++++++++++++++++++ ...ApiBehaviorApplicationModelProviderTest.cs | 13 ++++- 5 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 src/Mvc/Mvc.Core/src/ApplicationModels/HttpResultMetadataConvention.cs diff --git a/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs index 1d2df05819b1..35bd0b8ce7f8 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs @@ -286,8 +286,7 @@ internal static void CalculateResponseFormatForType(ApiResponseType apiResponse, // If the method is declared to return IActionResult, IResult or a derived class, that information // isn't valuable to the formatter. - if (typeof(IActionResult).IsAssignableFrom(unwrappedType) || - typeof(IResult).IsAssignableFrom(unwrappedType)) + if (typeof(IActionResult).IsAssignableFrom(unwrappedType)) { return null; } diff --git a/src/Mvc/Mvc.ApiExplorer/test/ApiResponseTypeProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/ApiResponseTypeProviderTest.cs index 096445358b96..2d4adc6d0269 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/ApiResponseTypeProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/ApiResponseTypeProviderTest.cs @@ -1,7 +1,8 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Reflection; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Formatters; @@ -749,6 +750,20 @@ public void GetApiResponseTypes_HandlesActionWithMultipleContentTypesAndProduces }); } + [Fact] + public void GetApiResponseTypes_ReturnNoResponseTypes_IfActionWithIResultReturnType() + { + // Arrange + var actionDescriptor = GetControllerActionDescriptor(typeof(TestController), nameof(TestController.GetIResult)); + var provider = new ApiResponseTypeProvider(new EmptyModelMetadataProvider(), new ActionResultTypeMapper(), new MvcOptions()); + + // Act + var result = provider.GetApiResponseTypes(actionDescriptor); + + // Assert + Assert.False(result.Any()); + } + private static ApiResponseTypeProvider GetProvider() { var mvcOptions = new MvcOptions @@ -794,6 +809,7 @@ public class TestController public ActionResult GetUserLocation(int a, int b) => null; public ActionResult PutModel(string userId, DerivedModel model) => null; + public IResult GetIResult(int id) => null; } private class TestOutputFormatter : OutputFormatter diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs index d42b6b3f6ed7..8bc2607d3fe7 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs @@ -52,6 +52,8 @@ public ApiBehaviorApplicationModelProvider( new InferParameterBindingInfoConvention(modelMetadataProvider, serviceProviderIsService); ActionModelConventions.Add(convention); } + + ActionModelConventions.Add(new HttpResultMetadataConvention()); } /// diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/HttpResultMetadataConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/HttpResultMetadataConvention.cs new file mode 100644 index 000000000000..4acecc3add86 --- /dev/null +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/HttpResultMetadataConvention.cs @@ -0,0 +1,49 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Mvc.ApplicationModels; + +/// +/// An that adds metadata to actions returning . +/// +/// +/// The following metadata are applied: +/// +/// A with application/json content-type. +/// +/// +internal class HttpResultMetadataConvention : IActionModelConvention +{ + public void Apply(ActionModel action) + { + if (action == null) + { + throw new ArgumentNullException(nameof(action)); + } + + if (!ShouldApply(action)) + { + return; + } + + AddProducesAttribute(action); + } + + /// + /// Determines if this instance of applies to a specified . + /// + /// The . + /// + /// if the convention applies, otherwise . + /// Derived types may override this method to selectively apply this convention. + /// + protected virtual bool ShouldApply(ActionModel action) => typeof(IResult).IsAssignableFrom(action!.ActionMethod.ReturnType); + + private static void AddProducesAttribute(ActionModel action) + { + // Currently all IResult is written using HttpResponse.WriteAsJsonAsync(T). + action.Filters.Add(new ProducesAttribute("application/json")); + } +} diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs index 74108673950c..b27478642832 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Reflection; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.ApiExplorer; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.Extensions.Logging.Abstractions; @@ -47,12 +49,18 @@ public void OnProvidersExecuting_AppliesConventions() }; var method = typeof(TestApiController).GetMethod(nameof(TestApiController.TestAction)); + var methodWithIResult = typeof(TestApiController).GetMethod(nameof(TestApiController.TestActionWithIResult)); var actionModel = new ActionModel(method, Array.Empty()) { Controller = controllerModel, }; + var actionWitIResultModel = new ActionModel(methodWithIResult, Array.Empty()) + { + Controller = controllerModel, + }; controllerModel.Actions.Add(actionModel); + controllerModel.Actions.Add(actionWitIResultModel); var parameter = method.GetParameters()[0]; var parameterModel = new ParameterModel(parameter, Array.Empty()) @@ -75,6 +83,7 @@ public void OnProvidersExecuting_AppliesConventions() Assert.NotEmpty(actionModel.Filters.OfType()); Assert.NotEmpty(actionModel.Filters.OfType()); Assert.Equal(BindingSource.Body, parameterModel.BindingInfo.BindingSource); + Assert.NotEmpty(actionWitIResultModel.Filters.OfType()); } [Fact] @@ -95,7 +104,8 @@ public void Constructor_SetsUpConventions() var convention = Assert.IsType(c); Assert.Equal(typeof(ProblemDetails), convention.DefaultErrorResponseType.Type); }, - c => Assert.IsType(c)); + c => Assert.IsType(c), + c => Assert.IsType(c)); } [Fact] @@ -178,5 +188,6 @@ private static ApiBehaviorApplicationModelProvider GetProvider( private class TestApiController : ControllerBase { public IActionResult TestAction(object value) => null; + public IResult TestActionWithIResult() => null; } } From 61a880825356da2a89e2e67908382f23b71a9282 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 18 Mar 2022 10:42:48 -0700 Subject: [PATCH 08/21] Adding test for IResult convention --- ...ApiBehaviorApplicationModelProviderTest.cs | 45 ++++++++++++++++--- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs index b27478642832..4c46be8d9d9c 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs @@ -49,18 +49,53 @@ public void OnProvidersExecuting_AppliesConventions() }; var method = typeof(TestApiController).GetMethod(nameof(TestApiController.TestAction)); - var methodWithIResult = typeof(TestApiController).GetMethod(nameof(TestApiController.TestActionWithIResult)); var actionModel = new ActionModel(method, Array.Empty()) { Controller = controllerModel, }; - var actionWitIResultModel = new ActionModel(methodWithIResult, Array.Empty()) + controllerModel.Actions.Add(actionModel); + + var parameter = method.GetParameters()[0]; + var parameterModel = new ParameterModel(parameter, Array.Empty()) + { + Action = actionModel, + }; + actionModel.Parameters.Add(parameterModel); + + var context = new ApplicationModelProviderContext(new[] { controllerModel.ControllerType }); + context.Result.Controllers.Add(controllerModel); + + var provider = GetProvider(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + // Verify some of the side-effects of executing API behavior conventions. + Assert.True(actionModel.ApiExplorer.IsVisible); + Assert.NotEmpty(actionModel.Filters.OfType()); + Assert.NotEmpty(actionModel.Filters.OfType()); + Assert.Equal(BindingSource.Body, parameterModel.BindingInfo.BindingSource); + Assert.Empty(actionModel.Filters.OfType()); + } + + [Fact] + public void OnProvidersExecuting_AppliesConventionsForIResult() + { + // Arrange + var controllerModel = new ControllerModel(typeof(TestApiController).GetTypeInfo(), new[] { new ApiControllerAttribute() }) + { + Selectors = { new SelectorModel { AttributeRouteModel = new AttributeRouteModel() } }, + }; + + var method = typeof(TestApiController).GetMethod(nameof(TestApiController.TestActionWithIResult)); + + var actionModel = new ActionModel(method, Array.Empty()) { Controller = controllerModel, }; controllerModel.Actions.Add(actionModel); - controllerModel.Actions.Add(actionWitIResultModel); var parameter = method.GetParameters()[0]; var parameterModel = new ParameterModel(parameter, Array.Empty()) @@ -83,7 +118,7 @@ public void OnProvidersExecuting_AppliesConventions() Assert.NotEmpty(actionModel.Filters.OfType()); Assert.NotEmpty(actionModel.Filters.OfType()); Assert.Equal(BindingSource.Body, parameterModel.BindingInfo.BindingSource); - Assert.NotEmpty(actionWitIResultModel.Filters.OfType()); + Assert.NotEmpty(actionModel.Filters.OfType()); } [Fact] @@ -188,6 +223,6 @@ private static ApiBehaviorApplicationModelProvider GetProvider( private class TestApiController : ControllerBase { public IActionResult TestAction(object value) => null; - public IResult TestActionWithIResult() => null; + public IResult TestActionWithIResult(object value) => null; } } From 608262f58fd269288da48052d706a32f19023e59 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 18 Mar 2022 10:59:01 -0700 Subject: [PATCH 09/21] Adding new unit test --- .../HttpResultMetadataConventionTest.cs | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 src/Mvc/Mvc.Core/test/ApplicationModels/HttpResultMetadataConventionTest.cs diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/HttpResultMetadataConventionTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/HttpResultMetadataConventionTest.cs new file mode 100644 index 000000000000..989b7d626149 --- /dev/null +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/HttpResultMetadataConventionTest.cs @@ -0,0 +1,82 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Reflection; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.ApiExplorer; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Mvc.ApplicationModels; + +public class HttpResultMetadataConventionTest +{ + [Fact] + public void Apply_AddsFilter() + { + // Arrange + var action = GetActionModel(typeof(TestController), nameof(TestController.IResultAction)); + var convention = GetConvention(); + + // Act + convention.Apply(action); + + // Assert + Assert.Single(action.Filters.OfType()); + } + + [Fact] + public void Apply_DoesNotAddFilter_ForActionResult() + { + // Arrange + var action = GetActionModel(typeof(TestController), nameof(TestController.IActionResultAction)); + var convention = GetConvention(); + + // Act + convention.Apply(action); + + // Assert + Assert.Empty(action.Filters.OfType()); + } + + [Fact] + public void Apply_DoesNotAddFilter_ForUserDefinedType() + { + // Arrange + var action = GetActionModel(typeof(TestController), nameof(TestController.UserDefinedTypeAction)); + var convention = GetConvention(); + + // Act + convention.Apply(action); + + // Assert + Assert.Empty(action.Filters.OfType()); + } + + private HttpResultMetadataConvention GetConvention() => new HttpResultMetadataConvention(); + + private static ApplicationModelProviderContext GetContext(Type type) + { + var context = new ApplicationModelProviderContext(new[] { type.GetTypeInfo() }); + var convention = new DefaultApplicationModelProvider(Options.Create(new MvcOptions()), new EmptyModelMetadataProvider()); + convention.OnProvidersExecuting(context); + + return context; + } + + private static ActionModel GetActionModel(Type controllerType, string actionName) + { + var context = GetContext(controllerType); + var controller = Assert.Single(context.Result.Controllers); + return Assert.Single(controller.Actions, m => m.ActionName == actionName); + } + + private class TestController + { + public record Todo(int id, string title); + + public IResult IResultAction(object value) => null; + public IActionResult IActionResultAction(object value) => null; + public Todo UserDefinedTypeAction(object value) => default(Todo); + } +} From e51544133f786293dbeafdf8431f458ef089b8ca Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 18 Mar 2022 11:00:49 -0700 Subject: [PATCH 10/21] Rollback change --- src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs index 35bd0b8ce7f8..1d2df05819b1 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs @@ -286,7 +286,8 @@ internal static void CalculateResponseFormatForType(ApiResponseType apiResponse, // If the method is declared to return IActionResult, IResult or a derived class, that information // isn't valuable to the formatter. - if (typeof(IActionResult).IsAssignableFrom(unwrappedType)) + if (typeof(IActionResult).IsAssignableFrom(unwrappedType) || + typeof(IResult).IsAssignableFrom(unwrappedType)) { return null; } From 803b548558401198a069bba19d2db284882db7c7 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 18 Mar 2022 11:05:48 -0700 Subject: [PATCH 11/21] Remove unnecessary change --- src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj b/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj index e572494c1d14..9d9b2ac58e91 100644 --- a/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj +++ b/src/Mvc/Mvc.Core/src/Microsoft.AspNetCore.Mvc.Core.csproj @@ -46,7 +46,6 @@ Microsoft.AspNetCore.Mvc.RouteAttribute - From c9c4b89df13d793a018f9cbd2abdefedabba7811 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Fri, 18 Mar 2022 11:10:52 -0700 Subject: [PATCH 12/21] Updating unit tests --- .../HttpResultMetadataConventionTest.cs | 22 +++++++++++++++++++ .../Mvc.Core/test/HttpResultsResultTest.cs | 5 +++-- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/HttpResultMetadataConventionTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/HttpResultMetadataConventionTest.cs index 989b7d626149..e6a0a50840df 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/HttpResultMetadataConventionTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/HttpResultMetadataConventionTest.cs @@ -24,6 +24,19 @@ public void Apply_AddsFilter() // Assert Assert.Single(action.Filters.OfType()); } + [Fact] + public void Apply_AddsFilter_ForCustomIResult() + { + // Arrange + var action = GetActionModel(typeof(TestController), nameof(TestController.CustomIResultAction)); + var convention = GetConvention(); + + // Act + convention.Apply(action); + + // Assert + Assert.Single(action.Filters.OfType()); + } [Fact] public void Apply_DoesNotAddFilter_ForActionResult() @@ -73,9 +86,18 @@ private static ActionModel GetActionModel(Type controllerType, string actionName private class TestController { + public class CustomIResult : IResult + { + public Task ExecuteAsync(HttpContext httpContext) + { + throw new NotImplementedException(); + } + } + public record Todo(int id, string title); public IResult IResultAction(object value) => null; + public CustomIResult CustomIResultAction(object value) => null; public IActionResult IActionResultAction(object value) => null; public Todo UserDefinedTypeAction(object value) => default(Todo); } diff --git a/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs b/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs index 2cd16424983d..7c4612576716 100644 --- a/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs +++ b/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs @@ -17,10 +17,11 @@ public class HttpResultsResultTest public void HttpResultsResult_InitializesWithResultsStaticMethods() { // Arrange & Act - var result = new HttpResultsActionResult(Results.Ok()); + var httpResult = Mock.Of(); + var result = new HttpResultsActionResult(httpResult); // Assert - Assert.NotNull(result.Result); + Assert.Equal(httpResult, result.Result); } [Fact] From 73c70a169e1d5a3dffa5480b09f3f126f0fa8f26 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 21 Mar 2022 11:24:50 -0700 Subject: [PATCH 13/21] Renaming to HttpActionResult and making it internal --- .../src/{HttpResultsActionResult.cs => HttpActionResult.cs} | 6 +++--- .../Mvc.Core/src/Infrastructure/ActionResultTypeMapper.cs | 2 +- src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt | 5 +---- src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs | 4 ++-- .../test/Infrastructure/ActionResultTypeMapperTest.cs | 2 +- src/Mvc/test/WebSites/BasicWebSite/BasicWebSite.csproj | 4 +++- .../BasicWebSite/Controllers/ContactApiController.cs | 2 +- .../WebSites/BasicWebSite/Controllers/FiltersController.cs | 2 +- .../BasicWebSite/Filters/HttpResultsResultFilter.cs | 2 +- 9 files changed, 14 insertions(+), 15 deletions(-) rename src/Mvc/Mvc.Core/src/{HttpResultsActionResult.cs => HttpActionResult.cs} (81%) diff --git a/src/Mvc/Mvc.Core/src/HttpResultsActionResult.cs b/src/Mvc/Mvc.Core/src/HttpActionResult.cs similarity index 81% rename from src/Mvc/Mvc.Core/src/HttpResultsActionResult.cs rename to src/Mvc/Mvc.Core/src/HttpActionResult.cs index deeef8e2a796..e8218cfec2c4 100644 --- a/src/Mvc/Mvc.Core/src/HttpResultsActionResult.cs +++ b/src/Mvc/Mvc.Core/src/HttpActionResult.cs @@ -8,7 +8,7 @@ namespace Microsoft.AspNetCore.Mvc; /// /// An that when executed will produce a response based on the provided. /// -public sealed class HttpResultsActionResult : ActionResult +internal sealed class HttpActionResult : ActionResult { /// /// Gets the instance of the current . @@ -16,11 +16,11 @@ public sealed class HttpResultsActionResult : ActionResult public IResult Result { get; } /// - /// Initializes a new instance of the class with the + /// Initializes a new instance of the class with the /// provided. /// /// The instance to be used during the invocation. - public HttpResultsActionResult(IResult result) + public HttpActionResult(IResult result) { Result = result; } diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/ActionResultTypeMapper.cs b/src/Mvc/Mvc.Core/src/Infrastructure/ActionResultTypeMapper.cs index 8f233b9a4f35..2e6afec44aa7 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/ActionResultTypeMapper.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/ActionResultTypeMapper.cs @@ -39,7 +39,7 @@ public IActionResult Convert(object? value, Type returnType) if (value is IResult result) { - return new HttpResultsActionResult(result); + return new HttpActionResult(result); } return new ObjectResult(value) diff --git a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt index 0c949c990354..cfefd22810dc 100644 --- a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt @@ -3,9 +3,6 @@ Microsoft.AspNetCore.Mvc.ApiBehaviorOptions.DisableImplicitFromServicesParameters.get -> bool Microsoft.AspNetCore.Mvc.ApiBehaviorOptions.DisableImplicitFromServicesParameters.set -> void Microsoft.AspNetCore.Mvc.ApplicationModels.InferParameterBindingInfoConvention.InferParameterBindingInfoConvention(Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider! modelMetadataProvider, Microsoft.Extensions.DependencyInjection.IServiceProviderIsService! serviceProviderIsService) -> void -Microsoft.AspNetCore.Mvc.HttpResultsActionResult -Microsoft.AspNetCore.Mvc.HttpResultsActionResult.HttpResultsActionResult(Microsoft.AspNetCore.Http.IResult! result) -> void -Microsoft.AspNetCore.Mvc.HttpResultsActionResult.Result.get -> Microsoft.AspNetCore.Http.IResult! Microsoft.AspNetCore.Mvc.ModelBinding.Binders.TryParseModelBinderProvider Microsoft.AspNetCore.Mvc.ModelBinding.Binders.TryParseModelBinderProvider.GetBinder(Microsoft.AspNetCore.Mvc.ModelBinding.ModelBinderProviderContext! context) -> Microsoft.AspNetCore.Mvc.ModelBinding.IModelBinder? Microsoft.AspNetCore.Mvc.ModelBinding.Binders.TryParseModelBinderProvider.TryParseModelBinderProvider() -> void @@ -16,7 +13,7 @@ Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataP Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.SystemTextJsonValidationMetadataProvider(System.Text.Json.JsonNamingPolicy! namingPolicy) -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ValidationMetadata.ValidationModelName.get -> string? Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ValidationMetadata.ValidationModelName.set -> void -override Microsoft.AspNetCore.Mvc.HttpResultsActionResult.ExecuteResultAsync(Microsoft.AspNetCore.Mvc.ActionContext! context) -> System.Threading.Tasks.Task! +override Microsoft.AspNetCore.Mvc.HttpActionResult.ExecuteResultAsync(Microsoft.AspNetCore.Mvc.ActionContext! context) -> System.Threading.Tasks.Task! virtual Microsoft.AspNetCore.Mvc.Infrastructure.ConfigureCompatibilityOptions.PostConfigure(string? name, TOptions! options) -> void *REMOVED*~Microsoft.AspNetCore.Mvc.Formatters.FormatFilter.FormatFilter(Microsoft.Extensions.Options.IOptions! options, Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void Microsoft.AspNetCore.Mvc.Formatters.FormatFilter.FormatFilter(Microsoft.Extensions.Options.IOptions! options, Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void diff --git a/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs b/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs index 7c4612576716..074d34c92722 100644 --- a/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs +++ b/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs @@ -18,7 +18,7 @@ public void HttpResultsResult_InitializesWithResultsStaticMethods() { // Arrange & Act var httpResult = Mock.Of(); - var result = new HttpResultsActionResult(httpResult); + var result = new HttpActionResult(httpResult); // Assert Assert.Equal(httpResult, result.Result); @@ -39,7 +39,7 @@ public async Task HttpResultsResult_SetsStatusCode() httpResult.Setup(s => s.ExecuteAsync(httpContext)) .Returns(() => Task.CompletedTask) .Verifiable(); - var result = new HttpResultsActionResult(httpResult.Object); + var result = new HttpActionResult(httpResult.Object); // Act await result.ExecuteResultAsync(context); diff --git a/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs b/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs index 05a6d1140166..b7cf2a25b67e 100644 --- a/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs +++ b/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs @@ -36,7 +36,7 @@ public void Convert_WithIResult_DelegatesToInterface() var result = mapper.Convert(returnValue, returnValue.GetType()); // Assert - var httpResultsResult = Assert.IsType(result); + var httpResultsResult = Assert.IsType(result); Assert.Same(returnValue, httpResultsResult.Result); } diff --git a/src/Mvc/test/WebSites/BasicWebSite/BasicWebSite.csproj b/src/Mvc/test/WebSites/BasicWebSite/BasicWebSite.csproj index d57b78a2bd3b..fcd96bbb5f1f 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/BasicWebSite.csproj +++ b/src/Mvc/test/WebSites/BasicWebSite/BasicWebSite.csproj @@ -1,4 +1,4 @@ - + $(DefaultNetCoreTargetFramework) @@ -9,12 +9,14 @@ + + diff --git a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs index 0a5e4719f592..aa5c8ac9381f 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs @@ -136,7 +136,7 @@ public IResult ActionReturningObjectIResult(int id) public ActionResult ActionReturningHttpResultsActionResult(int id) { var result = Results.Ok(new Contact() { ContactId = id }); - return new HttpResultsActionResult(result); + return new HttpActionResult(result); } [HttpGet("[action]")] diff --git a/src/Mvc/test/WebSites/BasicWebSite/Controllers/FiltersController.cs b/src/Mvc/test/WebSites/BasicWebSite/Controllers/FiltersController.cs index e1b7d23bc2a2..379546a62a9b 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/Controllers/FiltersController.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/Controllers/FiltersController.cs @@ -32,5 +32,5 @@ public IActionResult MiddlewareFilterTest() public IResult HttpResultsFilterForIResult() => Results.NoContent(); [HttpResultsFilter] - public IActionResult HttpResultsFilter() => new HttpResultsActionResult(Results.NoContent()); + public IActionResult HttpResultsFilter() => new HttpActionResult(Results.NoContent()); } diff --git a/src/Mvc/test/WebSites/BasicWebSite/Filters/HttpResultsResultFilter.cs b/src/Mvc/test/WebSites/BasicWebSite/Filters/HttpResultsResultFilter.cs index 46e6c29c1fc6..1931368bd852 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/Filters/HttpResultsResultFilter.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/Filters/HttpResultsResultFilter.cs @@ -15,7 +15,7 @@ public void OnResultExecuted(ResultExecutedContext context) public void OnResultExecuting(ResultExecutingContext context) { - if (context is { Result: HttpResultsActionResult { Result: IResult result } }) + if (context is { Result: HttpActionResult { Result: IResult result } }) { context.Result = new StatusCodeResult(StatusCodes.Status200OK); context.HttpContext.Response.Headers["X-HttpResultType"] = result.GetType().Name; From 66084813b364bb06061461828ed871aefb70be72 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 21 Mar 2022 11:31:57 -0700 Subject: [PATCH 14/21] Removing public API --- src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt index cfefd22810dc..80b80b352063 100644 --- a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt @@ -13,7 +13,6 @@ Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataP Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.SystemTextJsonValidationMetadataProvider(System.Text.Json.JsonNamingPolicy! namingPolicy) -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ValidationMetadata.ValidationModelName.get -> string? Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ValidationMetadata.ValidationModelName.set -> void -override Microsoft.AspNetCore.Mvc.HttpActionResult.ExecuteResultAsync(Microsoft.AspNetCore.Mvc.ActionContext! context) -> System.Threading.Tasks.Task! virtual Microsoft.AspNetCore.Mvc.Infrastructure.ConfigureCompatibilityOptions.PostConfigure(string? name, TOptions! options) -> void *REMOVED*~Microsoft.AspNetCore.Mvc.Formatters.FormatFilter.FormatFilter(Microsoft.Extensions.Options.IOptions! options, Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void Microsoft.AspNetCore.Mvc.Formatters.FormatFilter.FormatFilter(Microsoft.Extensions.Options.IOptions! options, Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void From 8805a53cc96567efe5178c0a2404d1e6adf08df1 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 21 Mar 2022 11:32:18 -0700 Subject: [PATCH 15/21] Removing tests --- .../Mvc.FunctionalTests/HttpResultsTests.cs | 39 ------------------- .../Controllers/ContactApiController.cs | 7 ---- .../Controllers/FiltersController.cs | 6 --- .../Filters/HttpResultsResultFilter.cs | 24 ------------ 4 files changed, 76 deletions(-) delete mode 100644 src/Mvc/test/WebSites/BasicWebSite/Filters/HttpResultsResultFilter.cs diff --git a/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs b/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs index 9a952f66cb30..7df2510809d9 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs @@ -17,21 +17,6 @@ public HttpResultsTests(MvcTestFixture f public HttpClient Client { get; } - [Fact] - public async Task ActionCanReturnHttpResultsActionResult() - { - // Arrange - var id = 1; - var url = $"/contact/{nameof(BasicWebSite.ContactApiController.ActionReturningHttpResultsActionResult)}/{id}"; - var response = await Client.GetAsync(url); - - // Assert - await response.AssertStatusCodeAsync(HttpStatusCode.OK); - var result = await response.Content.ReadFromJsonAsync(); - Assert.NotNull(result); - Assert.Equal(id, result.ContactId); - } - [Fact] public async Task ActionCanReturnIResultWithContent() { @@ -57,28 +42,4 @@ public async Task ActionCanReturnIResultWithStatusCodeOnly() // Assert await response.AssertStatusCodeAsync(HttpStatusCode.NoContent); } - - [Fact] - public async Task ActionCanHaveAFilterForIResult() - { - // Arrange - var url = $"/filters/{nameof(BasicWebSite.Controllers.FiltersController.HttpResultsFilterForIResult)}"; - var response = await Client.GetAsync(url); - - // Assert - await response.AssertStatusCodeAsync(HttpStatusCode.OK); - Assert.Single(response.Headers.GetValues("X-HttpResultType")); - } - - [Fact] - public async Task ActionCanHaveAFilterForHttpResultsActionResult() - { - // Arrange - var url = $"/filters/{nameof(BasicWebSite.Controllers.FiltersController.HttpResultsFilter)}"; - var response = await Client.GetAsync(url); - - // Assert - await response.AssertStatusCodeAsync(HttpStatusCode.OK); - Assert.Single(response.Headers.GetValues("X-HttpResultType")); - } } diff --git a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs index aa5c8ac9381f..ef218911ccaf 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs @@ -132,13 +132,6 @@ public ActionResult ActionReturningValidationProblemDetails() public IResult ActionReturningObjectIResult(int id) => Results.Ok(new Contact() { ContactId = id }); - [HttpGet("[action]/{id}")] - public ActionResult ActionReturningHttpResultsActionResult(int id) - { - var result = Results.Ok(new Contact() { ContactId = id }); - return new HttpActionResult(result); - } - [HttpGet("[action]")] public IResult ActionReturningStatusCodeIResult() => Results.NoContent(); diff --git a/src/Mvc/test/WebSites/BasicWebSite/Controllers/FiltersController.cs b/src/Mvc/test/WebSites/BasicWebSite/Controllers/FiltersController.cs index 379546a62a9b..dbb4e760e0a2 100644 --- a/src/Mvc/test/WebSites/BasicWebSite/Controllers/FiltersController.cs +++ b/src/Mvc/test/WebSites/BasicWebSite/Controllers/FiltersController.cs @@ -27,10 +27,4 @@ public IActionResult MiddlewareFilterTest() { return Content($"CurrentCulture:{CultureInfo.CurrentCulture.Name},CurrentUICulture:{CultureInfo.CurrentUICulture.Name}"); } - - [HttpResultsFilter] - public IResult HttpResultsFilterForIResult() => Results.NoContent(); - - [HttpResultsFilter] - public IActionResult HttpResultsFilter() => new HttpActionResult(Results.NoContent()); } diff --git a/src/Mvc/test/WebSites/BasicWebSite/Filters/HttpResultsResultFilter.cs b/src/Mvc/test/WebSites/BasicWebSite/Filters/HttpResultsResultFilter.cs deleted file mode 100644 index 1931368bd852..000000000000 --- a/src/Mvc/test/WebSites/BasicWebSite/Filters/HttpResultsResultFilter.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.Filters; - -namespace BasicWebSite; - -[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)] -public class HttpResultsFilter : Attribute, IResultFilter -{ - public void OnResultExecuted(ResultExecutedContext context) - { - } - - public void OnResultExecuting(ResultExecutingContext context) - { - if (context is { Result: HttpActionResult { Result: IResult result } }) - { - context.Result = new StatusCodeResult(StatusCodes.Status200OK); - context.HttpContext.Response.Headers["X-HttpResultType"] = result.GetType().Name; - } - } -} From dc0b88b4682e5b12baf006012b00498899f55918 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 21 Mar 2022 11:33:08 -0700 Subject: [PATCH 16/21] Renaming unit tests --- .../{HttpResultsTests.cs => HttpActionResultTests.cs} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename src/Mvc/test/Mvc.FunctionalTests/{HttpResultsTests.cs => HttpActionResultTests.cs} (86%) diff --git a/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs b/src/Mvc/test/Mvc.FunctionalTests/HttpActionResultTests.cs similarity index 86% rename from src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs rename to src/Mvc/test/Mvc.FunctionalTests/HttpActionResultTests.cs index 7df2510809d9..4314dd4060d5 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/HttpResultsTests.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/HttpActionResultTests.cs @@ -8,9 +8,9 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests; -public class HttpResultsTests : IClassFixture> +public class HttpActionResultTests : IClassFixture> { - public HttpResultsTests(MvcTestFixture fixture) + public HttpActionResultTests(MvcTestFixture fixture) { Client = fixture.CreateDefaultClient(); } From b56abcbe27bae8b52a78704702e6704c596d643c Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Mon, 21 Mar 2022 11:42:58 -0700 Subject: [PATCH 17/21] Renaming types --- .../Mvc.Core/src/Infrastructure/ActionResultTypeMapper.cs | 4 ++-- .../{HttpResultsResultTest.cs => HttpActionResultTests.cs} | 6 +++--- .../test/Infrastructure/ActionResultTypeMapperTest.cs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) rename src/Mvc/Mvc.Core/test/{HttpResultsResultTest.cs => HttpActionResultTests.cs} (90%) diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/ActionResultTypeMapper.cs b/src/Mvc/Mvc.Core/src/Infrastructure/ActionResultTypeMapper.cs index 2e6afec44aa7..79d012c251c8 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/ActionResultTypeMapper.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/ActionResultTypeMapper.cs @@ -37,9 +37,9 @@ public IActionResult Convert(object? value, Type returnType) return converter.Convert(); } - if (value is IResult result) + if (value is IResult httpResult) { - return new HttpActionResult(result); + return new HttpActionResult(httpResult); } return new ObjectResult(value) diff --git a/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs b/src/Mvc/Mvc.Core/test/HttpActionResultTests.cs similarity index 90% rename from src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs rename to src/Mvc/Mvc.Core/test/HttpActionResultTests.cs index 074d34c92722..bff2ca58ab32 100644 --- a/src/Mvc/Mvc.Core/test/HttpResultsResultTest.cs +++ b/src/Mvc/Mvc.Core/test/HttpActionResultTests.cs @@ -11,10 +11,10 @@ namespace Microsoft.AspNetCore.Mvc; -public class HttpResultsResultTest +public class HttpActionResultTests { [Fact] - public void HttpResultsResult_InitializesWithResultsStaticMethods() + public void HttpActionResult_InitializesWithResultsStaticMethods() { // Arrange & Act var httpResult = Mock.Of(); @@ -25,7 +25,7 @@ public void HttpResultsResult_InitializesWithResultsStaticMethods() } [Fact] - public async Task HttpResultsResult_SetsStatusCode() + public async Task HttpActionResult_SetsStatusCode() { // Arrange var httpContext = new DefaultHttpContext diff --git a/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs b/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs index b7cf2a25b67e..21205f3fde9f 100644 --- a/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs +++ b/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs @@ -36,8 +36,8 @@ public void Convert_WithIResult_DelegatesToInterface() var result = mapper.Convert(returnValue, returnValue.GetType()); // Assert - var httpResultsResult = Assert.IsType(result); - Assert.Same(returnValue, httpResultsResult.Result); + var httpResult = Assert.IsType(result); + Assert.Same(returnValue, httpResult.Result); } [Fact] From bb0a8055c17739cb5b046556fdf9841503850ad9 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Tue, 22 Mar 2022 09:34:29 -0700 Subject: [PATCH 18/21] PR Feedback --- src/Mvc/Mvc.ApiExplorer/test/ApiResponseTypeProviderTest.cs | 1 + src/Mvc/Mvc.Core/test/HttpActionResultTests.cs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.ApiExplorer/test/ApiResponseTypeProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/ApiResponseTypeProviderTest.cs index 2d4adc6d0269..b65ef5cc4846 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/ApiResponseTypeProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/ApiResponseTypeProviderTest.cs @@ -809,6 +809,7 @@ public class TestController public ActionResult GetUserLocation(int a, int b) => null; public ActionResult PutModel(string userId, DerivedModel model) => null; + public IResult GetIResult(int id) => null; } diff --git a/src/Mvc/Mvc.Core/test/HttpActionResultTests.cs b/src/Mvc/Mvc.Core/test/HttpActionResultTests.cs index bff2ca58ab32..b94695646fe8 100644 --- a/src/Mvc/Mvc.Core/test/HttpActionResultTests.cs +++ b/src/Mvc/Mvc.Core/test/HttpActionResultTests.cs @@ -25,7 +25,7 @@ public void HttpActionResult_InitializesWithResultsStaticMethods() } [Fact] - public async Task HttpActionResult_SetsStatusCode() + public async Task HttpActionResult_InvokesInternalHttpResult() { // Arrange var httpContext = new DefaultHttpContext From 6047c974c94e5e7d7bc53ac2a4d3fc265f57b014 Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Tue, 22 Mar 2022 09:49:38 -0700 Subject: [PATCH 19/21] PR Feeback --- .../src/ApplicationModels/HttpResultMetadataConvention.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/HttpResultMetadataConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/HttpResultMetadataConvention.cs index 4acecc3add86..be9c5fc6e82c 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/HttpResultMetadataConvention.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/HttpResultMetadataConvention.cs @@ -39,7 +39,7 @@ public void Apply(ActionModel action) /// if the convention applies, otherwise . /// Derived types may override this method to selectively apply this convention. /// - protected virtual bool ShouldApply(ActionModel action) => typeof(IResult).IsAssignableFrom(action!.ActionMethod.ReturnType); + protected virtual bool ShouldApply(ActionModel action) => typeof(IResult).IsAssignableFrom(action.ActionMethod.ReturnType); private static void AddProducesAttribute(ActionModel action) { From 939c542472d247a969a2bbe10d37235a1b6abebc Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Wed, 23 Mar 2022 13:32:34 -0700 Subject: [PATCH 20/21] Removing HttpResultMetadataConvention --- .../ApiBehaviorApplicationModelProvider.cs | 2 - .../HttpResultMetadataConvention.cs | 49 --------- ...ApiBehaviorApplicationModelProviderTest.cs | 5 +- .../HttpResultMetadataConventionTest.cs | 104 ------------------ 4 files changed, 1 insertion(+), 159 deletions(-) delete mode 100644 src/Mvc/Mvc.Core/src/ApplicationModels/HttpResultMetadataConvention.cs delete mode 100644 src/Mvc/Mvc.Core/test/ApplicationModels/HttpResultMetadataConventionTest.cs diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs index 8bc2607d3fe7..d42b6b3f6ed7 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/ApiBehaviorApplicationModelProvider.cs @@ -52,8 +52,6 @@ public ApiBehaviorApplicationModelProvider( new InferParameterBindingInfoConvention(modelMetadataProvider, serviceProviderIsService); ActionModelConventions.Add(convention); } - - ActionModelConventions.Add(new HttpResultMetadataConvention()); } /// diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/HttpResultMetadataConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/HttpResultMetadataConvention.cs deleted file mode 100644 index be9c5fc6e82c..000000000000 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/HttpResultMetadataConvention.cs +++ /dev/null @@ -1,49 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using Microsoft.AspNetCore.Http; - -namespace Microsoft.AspNetCore.Mvc.ApplicationModels; - -/// -/// An that adds metadata to actions returning . -/// -/// -/// The following metadata are applied: -/// -/// A with application/json content-type. -/// -/// -internal class HttpResultMetadataConvention : IActionModelConvention -{ - public void Apply(ActionModel action) - { - if (action == null) - { - throw new ArgumentNullException(nameof(action)); - } - - if (!ShouldApply(action)) - { - return; - } - - AddProducesAttribute(action); - } - - /// - /// Determines if this instance of applies to a specified . - /// - /// The . - /// - /// if the convention applies, otherwise . - /// Derived types may override this method to selectively apply this convention. - /// - protected virtual bool ShouldApply(ActionModel action) => typeof(IResult).IsAssignableFrom(action.ActionMethod.ReturnType); - - private static void AddProducesAttribute(ActionModel action) - { - // Currently all IResult is written using HttpResponse.WriteAsJsonAsync(T). - action.Filters.Add(new ProducesAttribute("application/json")); - } -} diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs index 4c46be8d9d9c..16117d17db96 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs @@ -77,7 +77,6 @@ public void OnProvidersExecuting_AppliesConventions() Assert.NotEmpty(actionModel.Filters.OfType()); Assert.NotEmpty(actionModel.Filters.OfType()); Assert.Equal(BindingSource.Body, parameterModel.BindingInfo.BindingSource); - Assert.Empty(actionModel.Filters.OfType()); } [Fact] @@ -118,7 +117,6 @@ public void OnProvidersExecuting_AppliesConventionsForIResult() Assert.NotEmpty(actionModel.Filters.OfType()); Assert.NotEmpty(actionModel.Filters.OfType()); Assert.Equal(BindingSource.Body, parameterModel.BindingInfo.BindingSource); - Assert.NotEmpty(actionModel.Filters.OfType()); } [Fact] @@ -139,8 +137,7 @@ public void Constructor_SetsUpConventions() var convention = Assert.IsType(c); Assert.Equal(typeof(ProblemDetails), convention.DefaultErrorResponseType.Type); }, - c => Assert.IsType(c), - c => Assert.IsType(c)); + c => Assert.IsType(c)); } [Fact] diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/HttpResultMetadataConventionTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/HttpResultMetadataConventionTest.cs deleted file mode 100644 index e6a0a50840df..000000000000 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/HttpResultMetadataConventionTest.cs +++ /dev/null @@ -1,104 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Reflection; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Mvc.ApiExplorer; -using Microsoft.AspNetCore.Mvc.ModelBinding; -using Microsoft.Extensions.Options; - -namespace Microsoft.AspNetCore.Mvc.ApplicationModels; - -public class HttpResultMetadataConventionTest -{ - [Fact] - public void Apply_AddsFilter() - { - // Arrange - var action = GetActionModel(typeof(TestController), nameof(TestController.IResultAction)); - var convention = GetConvention(); - - // Act - convention.Apply(action); - - // Assert - Assert.Single(action.Filters.OfType()); - } - [Fact] - public void Apply_AddsFilter_ForCustomIResult() - { - // Arrange - var action = GetActionModel(typeof(TestController), nameof(TestController.CustomIResultAction)); - var convention = GetConvention(); - - // Act - convention.Apply(action); - - // Assert - Assert.Single(action.Filters.OfType()); - } - - [Fact] - public void Apply_DoesNotAddFilter_ForActionResult() - { - // Arrange - var action = GetActionModel(typeof(TestController), nameof(TestController.IActionResultAction)); - var convention = GetConvention(); - - // Act - convention.Apply(action); - - // Assert - Assert.Empty(action.Filters.OfType()); - } - - [Fact] - public void Apply_DoesNotAddFilter_ForUserDefinedType() - { - // Arrange - var action = GetActionModel(typeof(TestController), nameof(TestController.UserDefinedTypeAction)); - var convention = GetConvention(); - - // Act - convention.Apply(action); - - // Assert - Assert.Empty(action.Filters.OfType()); - } - - private HttpResultMetadataConvention GetConvention() => new HttpResultMetadataConvention(); - - private static ApplicationModelProviderContext GetContext(Type type) - { - var context = new ApplicationModelProviderContext(new[] { type.GetTypeInfo() }); - var convention = new DefaultApplicationModelProvider(Options.Create(new MvcOptions()), new EmptyModelMetadataProvider()); - convention.OnProvidersExecuting(context); - - return context; - } - - private static ActionModel GetActionModel(Type controllerType, string actionName) - { - var context = GetContext(controllerType); - var controller = Assert.Single(context.Result.Controllers); - return Assert.Single(controller.Actions, m => m.ActionName == actionName); - } - - private class TestController - { - public class CustomIResult : IResult - { - public Task ExecuteAsync(HttpContext httpContext) - { - throw new NotImplementedException(); - } - } - - public record Todo(int id, string title); - - public IResult IResultAction(object value) => null; - public CustomIResult CustomIResultAction(object value) => null; - public IActionResult IActionResultAction(object value) => null; - public Todo UserDefinedTypeAction(object value) => default(Todo); - } -} From 9e19ea5bf3a43f0aa93a59360b659963c0ea5cca Mon Sep 17 00:00:00 2001 From: Bruno Lins de Oliveira Date: Wed, 23 Mar 2022 15:12:20 -0700 Subject: [PATCH 21/21] Unit testing IResult and IConvertToActionResult at same type --- .../ActionResultTypeMapperTest.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs b/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs index 21205f3fde9f..280a866c3f8a 100644 --- a/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs +++ b/src/Mvc/Mvc.Core/test/Infrastructure/ActionResultTypeMapperTest.cs @@ -40,6 +40,21 @@ public void Convert_WithIResult_DelegatesToInterface() Assert.Same(returnValue, httpResult.Result); } + [Fact] + public void Convert_WithIConvertToActionResultAndIResult_DelegatesToInterface() + { + // Arrange + var mapper = new ActionResultTypeMapper(); + + var returnValue = new CustomConvertibleIResult(); + + // Act + var result = mapper.Convert(returnValue, returnValue.GetType()); + + // Assert + Assert.IsType(result); + } + [Fact] public void Convert_WithRegularType_CreatesObjectResult() { @@ -86,4 +101,11 @@ public void GetResultDataType_WithRegularType_ReturnsType() // Assert Assert.Equal(typeof(string), result); } + + private class CustomConvertibleIResult : IConvertToActionResult, IResult + { + public IActionResult Convert() => new EmptyResult(); + + public Task ExecuteAsync(HttpContext httpContext) => throw new NotImplementedException(); + } }