diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs index e955f25768..54614a2465 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs @@ -2,7 +2,10 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections; +using System.Collections.Generic; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; namespace Microsoft.AspNetCore.Mvc @@ -10,17 +13,31 @@ namespace Microsoft.AspNetCore.Mvc /// /// Options used to configure behavior for types annotated with . /// - public class ApiBehaviorOptions + public class ApiBehaviorOptions : IEnumerable { + private readonly CompatibilitySwitch _suppressUseClientErrorFactory; + private readonly CompatibilitySwitch _suppressUseValidationProblemDetailsForInvalidModelStateResponses; + private readonly ICompatibilitySwitch[] _switches; + private Func _invalidModelStateResponseFactory; + /// + /// Creates a new instance of . + /// + public ApiBehaviorOptions() + { + _suppressUseClientErrorFactory = new CompatibilitySwitch(nameof(SuppressUseClientErrorFactory)); + _suppressUseValidationProblemDetailsForInvalidModelStateResponses = new CompatibilitySwitch(nameof(SuppressUseValidationProblemDetailsForInvalidModelStateResponses)); + _switches = new[] + { + _suppressUseClientErrorFactory, + _suppressUseValidationProblemDetailsForInvalidModelStateResponses, + }; + } + /// /// Delegate invoked on actions annotated with to convert invalid /// into an - /// - /// By default, the delegate produces a that wraps a serialized form - /// of . - /// /// public Func InvalidModelStateResponseFactory { @@ -52,5 +69,96 @@ namespace Microsoft.AspNetCore.Mvc /// that are bound from form data. /// public bool SuppressConsumesConstraintForFormFileParameters { get; set; } + + /// + /// Gets or sets a value that determines if controllers with use + /// to transform certain certain client errors. + /// + /// When false, is used to transform to the value + /// specified by the factory. In the default case, this converts instances to an + /// with . + /// + /// + /// + /// The default value is if the version is + /// or later; otherwise. + /// + /// + /// + /// This property is associated with a compatibility switch and can provide a different behavior depending on + /// the configured compatibility version for the application. See for + /// guidance and examples of setting the application's compatibility version. + /// + /// + /// Configuring the desired value of the compatibility switch by calling this property's setter will take + /// precedence over the value implied by the application's . + /// + /// + /// If the application's compatibility version is set to or + /// lower then this setting will have the value unless explicitly configured. + /// + /// + /// If the application's compatibility version is set to or + /// higher then this setting will have the value unless explicitly configured. + /// + /// + public bool SuppressUseClientErrorFactory + { + // Note: When compatibility switches are removed in 3.0, this property should be retained as a regular boolean property. + get => _suppressUseClientErrorFactory.Value; + set => _suppressUseClientErrorFactory.Value = value; + } + + /// + /// Gets or sets a value that determines if controllers annotated with respond using + /// in . + /// + /// When , returns errors in + /// as a . Otherwise, returns the errors + /// in the format determined by . + /// + /// + /// + /// The default value is if the version is + /// or later; otherwise. + /// + /// + /// + /// This property is associated with a compatibility switch and can provide a different behavior depending on + /// the configured compatibility version for the application. See for + /// guidance and examples of setting the application's compatibility version. + /// + /// + /// Configuring the desired value of the compatibility switch by calling this property's setter will take + /// precedence over the value implied by the application's . + /// + /// + /// If the application's compatibility version is set to or + /// lower then this setting will have the value unless explicitly configured. + /// + /// + /// If the application's compatibility version is set to or + /// higher then this setting will have the value unless explicitly configured. + /// + /// + public bool SuppressUseValidationProblemDetailsForInvalidModelStateResponses + { + get => _suppressUseValidationProblemDetailsForInvalidModelStateResponses.Value; + set => _suppressUseValidationProblemDetailsForInvalidModelStateResponses.Value = value; + } + + /// + /// Gets a map of HTTP status codes to factories. + /// Configured factories are used when is . + /// + public IDictionary> ClientErrorFactory { get; } = + new Dictionary>(); + + IEnumerator IEnumerable.GetEnumerator() + { + return ((IEnumerable)_switches).GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() => _switches.GetEnumerator(); } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index e7e7d5e0dd..3338870e73 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -148,6 +148,8 @@ namespace Microsoft.Extensions.DependencyInjection ServiceDescriptor.Transient, MvcOptionsConfigureCompatibilityOptions>()); services.TryAddEnumerable( ServiceDescriptor.Transient, ApiBehaviorOptionsSetup>()); + services.TryAddEnumerable( + ServiceDescriptor.Transient, ApiBehaviorOptionsSetup>()); services.TryAddEnumerable( ServiceDescriptor.Transient, MvcCoreRouteOptionsSetup>()); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ClientErrorResultFilter.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ClientErrorResultFilter.cs new file mode 100644 index 0000000000..a6482dfe09 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ClientErrorResultFilter.cs @@ -0,0 +1,53 @@ +// 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.Collections.Generic; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Mvc.Internal; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + internal class ClientErrorResultFilter : IAlwaysRunResultFilter, IOrderedFilter + { + private readonly IDictionary> _clientErrorFactory; + private readonly ILogger _logger; + + /// + /// Gets the filter order. Defaults to -2000 so that it runs early. + /// + public int Order => -2000; + + public ClientErrorResultFilter( + ApiBehaviorOptions apiBehaviorOptions, + ILogger logger) + { + _clientErrorFactory = apiBehaviorOptions?.ClientErrorFactory ?? throw new ArgumentNullException(nameof(apiBehaviorOptions)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + } + + public void OnResultExecuted(ResultExecutedContext context) + { + } + + public void OnResultExecuting(ResultExecutingContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (context.Result is IClientErrorActionResult clientErrorActionResult && + clientErrorActionResult.StatusCode is int statusCode && + _clientErrorFactory.TryGetValue(statusCode, out var factory)) + { + var result = factory(context); + + _logger.TransformingClientError(context.Result.GetType(), result?.GetType(), statusCode); + + context.Result = factory(context); + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IClientErrorActionResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IClientErrorActionResult.cs new file mode 100644 index 0000000000..6d9926b50c --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IClientErrorActionResult.cs @@ -0,0 +1,12 @@ +// 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. + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + /// + /// An that can be transformed to a more descriptive client error. + /// + public interface IClientErrorActionResult : IStatusCodeActionResult + { + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs index f5930f40a6..bc1ac18c79 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs @@ -21,6 +21,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal private readonly ApiBehaviorOptions _apiBehaviorOptions; private readonly IModelMetadataProvider _modelMetadataProvider; private readonly ModelStateInvalidFilter _modelStateInvalidFilter; + private readonly ClientErrorResultFilter _clientErrorResultFilter; private readonly ILogger _logger; public ApiBehaviorApplicationModelProvider( @@ -42,6 +43,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal _modelStateInvalidFilter = new ModelStateInvalidFilter( apiBehaviorOptions.Value, loggerFactory.CreateLogger()); + + _clientErrorResultFilter = new ClientErrorResultFilter( + _apiBehaviorOptions, + loggerFactory.CreateLogger()); } /// @@ -90,6 +95,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal AddInvalidModelStateFilter(actionModel); + AddClientErrorFilter(actionModel); + InferParameterBindingSources(actionModel); InferParameterModelPrefixes(actionModel); @@ -149,6 +156,16 @@ namespace Microsoft.AspNetCore.Mvc.Internal actionModel.Filters.Add(_modelStateInvalidFilter); } + private void AddClientErrorFilter(ActionModel actionModel) + { + if (_apiBehaviorOptions.SuppressUseClientErrorFactory) + { + return; + } + + actionModel.Filters.Add(_clientErrorResultFilter); + } + // Internal for unit testing internal void InferParameterBindingSources(ActionModel actionModel) { diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs index 48f50f7980..15c4ef8892 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs @@ -2,17 +2,44 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; +using Microsoft.AspNetCore.Mvc.Core; +using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.Internal { - public class ApiBehaviorOptionsSetup : IConfigureOptions + public class ApiBehaviorOptionsSetup : + ConfigureCompatibilityOptions, + IConfigureOptions { + internal static readonly Func DefaultFactory = DefaultInvalidModelStateResponse; + internal static readonly Func ProblemDetailsFactory = ProblemDetailsInvalidModelStateResponse; - public ApiBehaviorOptionsSetup() + public ApiBehaviorOptionsSetup( + ILoggerFactory loggerFactory, + IOptions compatibilityOptions) + : base(loggerFactory, compatibilityOptions) { } + protected override IReadOnlyDictionary DefaultValues + { + get + { + var dictionary = new Dictionary(); + + if (Version < CompatibilityVersion.Version_2_2) + { + dictionary[nameof(ApiBehaviorOptions.SuppressUseClientErrorFactory)] = true; + dictionary[nameof(ApiBehaviorOptions.SuppressUseValidationProblemDetailsForInvalidModelStateResponses)] = true; + } + + return dictionary; + } + } + public void Configure(ApiBehaviorOptions options) { if (options == null) @@ -20,17 +47,117 @@ namespace Microsoft.AspNetCore.Mvc.Internal throw new ArgumentNullException(nameof(options)); } - options.InvalidModelStateResponseFactory = GetInvalidModelStateResponse; + options.InvalidModelStateResponseFactory = DefaultFactory; + ConfigureClientErrorFactories(options); + } - IActionResult GetInvalidModelStateResponse(ActionContext context) + public override void PostConfigure(string name, ApiBehaviorOptions options) + { + // Let compatibility switches do their thing. + base.PostConfigure(name, options); + + // We want to use problem details factory only if + // (a) it has not been opted out of (SuppressUseClientErrorFactory = true) + // (b) a different factory was configured + if (!options.SuppressUseClientErrorFactory && + object.ReferenceEquals(options.InvalidModelStateResponseFactory, DefaultFactory)) { - var result = new BadRequestObjectResult(context.ModelState); - - result.ContentTypes.Add("application/json"); - result.ContentTypes.Add("application/xml"); - - return result; + options.InvalidModelStateResponseFactory = ProblemDetailsFactory; } } + + // Internal for unit testing + internal static void ConfigureClientErrorFactories(ApiBehaviorOptions options) + { + AddClientErrorFactory(new ProblemDetails + { + Status = 400, + Type = "https://tools.ietf.org/html/rfc7231#section-6.5.1", + Title = Resources.ApiConventions_Title_400, + }); + + AddClientErrorFactory(new ProblemDetails + { + Status = 401, + Type = "https://tools.ietf.org/html/rfc7235#section-3.1", + Title = Resources.ApiConventions_Title_401, + }); + + AddClientErrorFactory(new ProblemDetails + { + Status = 403, + Type = "https://tools.ietf.org/html/rfc7231#section-6.5.3", + Title = Resources.ApiConventions_Title_403, + }); + + AddClientErrorFactory(new ProblemDetails + { + Status = 404, + Type = "https://tools.ietf.org/html/rfc7231#section-6.5.4", + Title = Resources.ApiConventions_Title_404, + }); + + AddClientErrorFactory(new ProblemDetails + { + Status = 406, + Type = "https://tools.ietf.org/html/rfc7231#section-6.5.6", + Title = Resources.ApiConventions_Title_406, + }); + + AddClientErrorFactory(new ProblemDetails + { + Status = 409, + Type = "https://tools.ietf.org/html/rfc7231#section-6.5.8", + Title = Resources.ApiConventions_Title_409, + }); + + AddClientErrorFactory(new ProblemDetails + { + Status = 415, + Type = "https://tools.ietf.org/html/rfc7231#section-6.5.13", + Title = Resources.ApiConventions_Title_415, + }); + + AddClientErrorFactory(new ProblemDetails + { + Status = 422, + Type = "https://tools.ietf.org/html/rfc4918#section-11.2", + Title = Resources.ApiConventions_Title_422, + }); + + void AddClientErrorFactory(ProblemDetails problemDetails) + { + var statusCode = problemDetails.Status.Value; + options.ClientErrorFactory[statusCode] = _ => new ObjectResult(problemDetails) + { + StatusCode = statusCode, + ContentTypes = + { + "application/problem+json", + "application/problem+xml", + }, + }; + } + } + + private static IActionResult DefaultInvalidModelStateResponse(ActionContext context) + { + var result = new BadRequestObjectResult(context.ModelState); + + result.ContentTypes.Add("application/json"); + result.ContentTypes.Add("application/xml"); + + return result; + } + + private static IActionResult ProblemDetailsInvalidModelStateResponse(ActionContext context) + { + var result = new BadRequestObjectResult(new ValidationProblemDetails(context.ModelState)); + + result.ContentTypes.Add("application/problem+json"); + result.ContentTypes.Add("application/problem+xml"); + + return result; + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs index db626eb4a5..4c2d80972e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs @@ -151,6 +151,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal private static readonly Action _notMostEffectiveFilter; private static readonly Action, Exception> _registeredOutputFormatters; + private static readonly Action _transformingClientError; + static MvcCoreLoggerExtensions() { _actionExecuting = LoggerMessage.Define( @@ -648,6 +650,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal LogLevel.Debug, 48, "Skipped binding parameter '{ParameterName}' since its binding information disallowed it for the current request."); + + _transformingClientError = LoggerMessage.Define( + LogLevel.Trace, + new EventId(49, nameof(Infrastructure.ClientErrorResultFilter)), + "Replacing {InitialActionResultType} with status code {StatusCode} with {ReplacedActionResultType} produced from ClientErrorFactory'."); } public static void RegisteredOutputFormatters(this ILogger logger, IEnumerable outputFormatters) @@ -1578,6 +1585,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } + public static void TransformingClientError(this ILogger logger, Type initialType, Type replacedType, int statusCode) + { + _transformingClientError(logger, initialType, replacedType, statusCode, null); + } + private static void LogFilterExecutionPlan( ILogger logger, string filterType, diff --git a/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs index 34a55b4ce4..dd8e716d18 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs @@ -95,7 +95,7 @@ namespace Microsoft.AspNetCore.Mvc /// lower then this setting will have the value unless explicitly configured. /// /// - /// If the application's compatibility version is set to or + /// If the application's compatibility version is set to or /// higher then this setting will have the value unless explicitly configured. /// /// diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs index 354ce64258..d70a92f526 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs @@ -1578,6 +1578,118 @@ namespace Microsoft.AspNetCore.Mvc.Core internal static string FormatValidationVisitor_ExceededMaxPropertyDepth(object p0, object p1, object p2, object p3) => string.Format(CultureInfo.CurrentCulture, GetString("ValidationVisitor_ExceededMaxPropertyDepth"), p0, p1, p2, p3); + /// + /// Bad Request + /// + internal static string ApiConventions_Title_400 + { + get => GetString("ApiConventions_Title_400"); + } + + /// + /// Bad Request + /// + internal static string FormatApiConventions_Title_400() + => GetString("ApiConventions_Title_400"); + + /// + /// Unauthorized + /// + internal static string ApiConventions_Title_401 + { + get => GetString("ApiConventions_Title_401"); + } + + /// + /// Unauthorized + /// + internal static string FormatApiConventions_Title_401() + => GetString("ApiConventions_Title_401"); + + /// + /// Forbidden + /// + internal static string ApiConventions_Title_403 + { + get => GetString("ApiConventions_Title_403"); + } + + /// + /// Forbidden + /// + internal static string FormatApiConventions_Title_403() + => GetString("ApiConventions_Title_403"); + + /// + /// Not Found + /// + internal static string ApiConventions_Title_404 + { + get => GetString("ApiConventions_Title_404"); + } + + /// + /// Not Found + /// + internal static string FormatApiConventions_Title_404() + => GetString("ApiConventions_Title_404"); + + /// + /// Not Acceptable + /// + internal static string ApiConventions_Title_406 + { + get => GetString("ApiConventions_Title_406"); + } + + /// + /// Not Acceptable + /// + internal static string FormatApiConventions_Title_406() + => GetString("ApiConventions_Title_406"); + + /// + /// Conflict + /// + internal static string ApiConventions_Title_409 + { + get => GetString("ApiConventions_Title_409"); + } + + /// + /// Conflict + /// + internal static string FormatApiConventions_Title_409() + => GetString("ApiConventions_Title_409"); + + /// + /// Unsupported Media Type + /// + internal static string ApiConventions_Title_415 + { + get => GetString("ApiConventions_Title_415"); + } + + /// + /// Unsupported Media Type + /// + internal static string FormatApiConventions_Title_415() + => GetString("ApiConventions_Title_415"); + + /// + /// Unprocessable Entity + /// + internal static string ApiConventions_Title_422 + { + get => GetString("ApiConventions_Title_422"); + } + + /// + /// Unprocessable Entity + /// + internal static string FormatApiConventions_Title_422() + => GetString("ApiConventions_Title_422"); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx b/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx index 4916783a8c..cc88759296 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx @@ -466,4 +466,28 @@ {0} exceeded the maximum configured validation depth '{1}' when validating property '{2}' on type '{3}'. + + Bad Request + + + Unauthorized + + + Forbidden + + + Not Found + + + Not Acceptable + + + Conflict + + + Unsupported Media Type + + + Unprocessable Entity + \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Core/StatusCodeResult.cs b/src/Microsoft.AspNetCore.Mvc.Core/StatusCodeResult.cs index 305d279372..c3ec5614c0 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/StatusCodeResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/StatusCodeResult.cs @@ -13,7 +13,7 @@ namespace Microsoft.AspNetCore.Mvc /// Represents an that when executed will /// produce an HTTP response with the given response status code. /// - public class StatusCodeResult : ActionResult, IStatusCodeActionResult + public class StatusCodeResult : ActionResult, IClientErrorActionResult { /// /// Initializes a new instance of the class diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs index f7223852de..ba70d85c6e 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs @@ -264,6 +264,13 @@ namespace Microsoft.AspNetCore.Mvc typeof(ApiBehaviorOptionsSetup), } }, + { + typeof(IPostConfigureOptions), + new Type[] + { + typeof(ApiBehaviorOptionsSetup), + } + }, { typeof(IActionConstraintProvider), new Type[] diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ClientErrorResultFilterTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ClientErrorResultFilterTest.cs new file mode 100644 index 0000000000..7c3f55858d --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ClientErrorResultFilterTest.cs @@ -0,0 +1,104 @@ +// 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 Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + public class ClientErrorResultFilterTest + { + private static readonly IActionResult Result = new EmptyResult(); + + [Fact] + public void OnResultExecuting_DoesNothing_IfActionIsNotClientErrorActionResult() + { + // Arrange + var actionResult = new NotFoundObjectResult(new object()); + var context = GetContext(actionResult); + var filter = GetFilter(); + + // Act + filter.OnResultExecuting(context); + + // Assert + Assert.Same(actionResult, context.Result); + } + + [Fact] + public void OnResultExecuting_DoesNothing_IfStatusCodeDoesNotExistInApiBehaviorOptions() + { + // Arrange + var actionResult = new NotFoundResult(); + var context = GetContext(actionResult); + var filter = GetFilter(new ApiBehaviorOptions()); + + // Act + filter.OnResultExecuting(context); + + // Assert + Assert.Same(actionResult, context.Result); + } + + [Fact] + public void OnResultExecuting_DoesNothing_IfResultDoesNotHaveStatusCode() + { + // Arrange + var actionResult = new Mock() + .As() + .Object; + var context = GetContext(actionResult); + var filter = GetFilter(new ApiBehaviorOptions()); + + // Act + filter.OnResultExecuting(context); + + // Assert + Assert.Same(actionResult, context.Result); + } + + [Fact] + public void OnResultExecuting_TransformsClientErrors() + { + // Arrange + var actionResult = new NotFoundResult(); + var context = GetContext(actionResult); + var filter = GetFilter(); + + // Act + filter.OnResultExecuting(context); + + // Assert + Assert.Same(Result, context.Result); + } + + private static ClientErrorResultFilter GetFilter(ApiBehaviorOptions options = null) + { + var apiBehaviorOptions = options ?? GetOptions(); + var filter = new ClientErrorResultFilter(apiBehaviorOptions, NullLogger.Instance); + return filter; + } + + private static ApiBehaviorOptions GetOptions() + { + var apiBehaviorOptions = new ApiBehaviorOptions(); + apiBehaviorOptions.ClientErrorFactory[404] = _ => Result; + return apiBehaviorOptions; + } + + private static ResultExecutingContext GetContext(IActionResult actionResult) + { + return new ResultExecutingContext( + new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()), + Array.Empty(), + actionResult, + new object()); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs index fc47f2abd0..3b5d4415de 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs @@ -35,7 +35,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert var actionModel = Assert.Single(Assert.Single(context.Result.Controllers).Actions); - Assert.IsType(actionModel.Filters.Last()); + Assert.Single(actionModel.Filters.OfType()); } [Fact] @@ -54,8 +54,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal provider.OnProvidersExecuting(context); // Assert - var controllerModel = Assert.Single(context.Result.Controllers); - Assert.DoesNotContain(typeof(ModelStateInvalidFilter), controllerModel.Filters.Select(f => f.GetType())); + var actionModel = Assert.Single(Assert.Single(context.Result.Controllers).Actions); + Assert.Empty(actionModel.Filters.OfType()); } [Fact] @@ -73,11 +73,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Single(context.Result.Controllers).Actions.OrderBy(a => a.ActionName), action => { - Assert.Contains(typeof(ModelStateInvalidFilter), action.Filters.Select(f => f.GetType())); + Assert.Single(action.Filters.OfType()); }, action => { - Assert.DoesNotContain(typeof(ModelStateInvalidFilter), action.Filters.Select(f => f.GetType())); + Assert.Empty(action.Filters.OfType()); }); } @@ -101,11 +101,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Single(context.Result.Controllers).Actions.OrderBy(a => a.ActionName), action => { - Assert.DoesNotContain(typeof(ModelStateInvalidFilter), action.Filters.Select(f => f.GetType())); + Assert.Empty(action.Filters.OfType()); }, action => { - Assert.DoesNotContain(typeof(ModelStateInvalidFilter), action.Filters.Select(f => f.GetType())); + Assert.Empty(action.Filters.OfType()); }); } @@ -1059,6 +1059,41 @@ Environment.NewLine + "int b"; }); } + [Fact] + public void OnProvidersExecuting_AddsClientErrorResultFilter() + { + // Arrange + var context = GetContext(typeof(TestApiController)); + var provider = GetProvider(); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var actionModel = Assert.Single(Assert.Single(context.Result.Controllers).Actions); + Assert.Single(actionModel.Filters.OfType()); + } + + [Fact] + public void OnProvidersExecuting_DoesNotAddClientErrorResultFilter_IfFeatureIsDisabled() + { + // Arrange + var context = GetContext(typeof(TestApiController)); + var options = new ApiBehaviorOptions + { + SuppressUseClientErrorFactory = true, + InvalidModelStateResponseFactory = _ => null, + }; + var provider = GetProvider(options); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var actionModel = Assert.Single(Assert.Single(context.Result.Controllers).Actions); + Assert.Empty(actionModel.Filters.OfType()); + } + // A dynamically generated type in an assembly that has an ApiConventionAttribute. private static TypeBuilder CreateTestControllerType() { diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorOptionsSetupTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorOptionsSetupTest.cs new file mode 100644 index 0000000000..66f4f7933b --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorOptionsSetupTest.cs @@ -0,0 +1,101 @@ +// 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 Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Internal +{ + public class ApiBehaviorOptionsSetupTest + { + [Fact] + public void Configure_AssignsInvalidModelStateResponseFactory() + { + // Arrange + var optionsSetup = new ApiBehaviorOptionsSetup( + NullLoggerFactory.Instance, + Options.Create(new MvcCompatibilityOptions())); + var options = new ApiBehaviorOptions(); + + // Act + optionsSetup.Configure(options); + + // Assert + Assert.Same(ApiBehaviorOptionsSetup.DefaultFactory, options.InvalidModelStateResponseFactory); + } + + [Fact] + public void Configure_AddsClientErrorFactories() + { + // Arrange + var expected = new[] { 400, 401, 403, 404, 406, 409, 415, 422, }; + var optionsSetup = new ApiBehaviorOptionsSetup( + NullLoggerFactory.Instance, + Options.Create(new MvcCompatibilityOptions())); + var options = new ApiBehaviorOptions(); + + // Act + optionsSetup.Configure(options); + + // Assert + Assert.Equal(expected, options.ClientErrorFactory.Keys); + } + + [Fact] + public void PostConfigure_SetProblemDetailsModelStateResponseFactory() + { + // Arrange + var optionsSetup = new ApiBehaviorOptionsSetup( + NullLoggerFactory.Instance, + Options.Create(new MvcCompatibilityOptions { CompatibilityVersion = CompatibilityVersion.Latest })); + var options = new ApiBehaviorOptions(); + + // Act + optionsSetup.Configure(options); + optionsSetup.PostConfigure(string.Empty, options); + + // Assert + Assert.Same(ApiBehaviorOptionsSetup.ProblemDetailsFactory, options.InvalidModelStateResponseFactory); + } + + [Fact] + public void PostConfigure_DoesNotSetProblemDetailsFactoryWithLegacyCompatBehavior() + { + // Arrange + var optionsSetup = new ApiBehaviorOptionsSetup( + NullLoggerFactory.Instance, + Options.Create(new MvcCompatibilityOptions { CompatibilityVersion = CompatibilityVersion.Version_2_1 })); + var options = new ApiBehaviorOptions(); + + // Act + optionsSetup.Configure(options); + optionsSetup.PostConfigure(string.Empty, options); + + // Assert + Assert.Same(ApiBehaviorOptionsSetup.DefaultFactory, options.InvalidModelStateResponseFactory); + } + + [Fact] + public void PostConfigure_DoesNotSetProblemDetailsFactory_IfValueWasModified() + { + // Arrange + var optionsSetup = new ApiBehaviorOptionsSetup( + NullLoggerFactory.Instance, + Options.Create(new MvcCompatibilityOptions { CompatibilityVersion = CompatibilityVersion.Latest })); + var options = new ApiBehaviorOptions(); + Func expected = _ => null; + + // Act + optionsSetup.Configure(options); + // This is equivalent to user code updating the value via ConfigureOptions + options.InvalidModelStateResponseFactory = expected; + optionsSetup.PostConfigure(string.Empty, options); + + // Assert + Assert.Same(expected, options.InvalidModelStateResponseFactory); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs index cfb5df0887..0d0a85999f 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs @@ -8,6 +8,7 @@ using System.Net.Http; using System.Text; using System.Threading.Tasks; using BasicWebSite.Models; +using Microsoft.AspNetCore.Hosting; using Newtonsoft.Json; using Xunit; @@ -18,9 +19,16 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public ApiBehaviorTest(MvcTestFixture fixture) { Client = fixture.CreateDefaultClient(); + + var factory = fixture.WithWebHostBuilder(ConfigureWebHostBuilder); + CustomInvalidModelStateClient = factory.CreateDefaultClient(); } + private static void ConfigureWebHostBuilder(IWebHostBuilder builder) => + builder.UseStartup(); + public HttpClient Client { get; } + public HttpClient CustomInvalidModelStateClient { get; } [Fact] public async Task ActionsReturnBadRequest_WhenModelStateIsInvalid() @@ -39,11 +47,11 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests var response = await Client.PostAsJsonAsync("/contact", contactModel); // Assert - Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); - Assert.Equal("application/json", response.Content.Headers.ContentType.MediaType); - var actual = JsonConvert.DeserializeObject>(await response.Content.ReadAsStringAsync()); + await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest); + Assert.Equal("application/problem+json", response.Content.Headers.ContentType.MediaType); + var problemDetails = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); Assert.Collection( - actual.OrderBy(kvp => kvp.Key), + problemDetails.Errors.OrderBy(kvp => kvp.Key), kvp => { Assert.Equal("Name", kvp.Key); @@ -110,10 +118,10 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests var contactString = JsonConvert.SerializeObject(contactModel); // Act - var response = await Client.PostAsJsonAsync("/contact/PostWithVnd", contactModel); + var response = await CustomInvalidModelStateClient.PostAsJsonAsync("/contact/PostWithVnd", contactModel); // Assert - Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest); Assert.Equal("application/vnd.error+json", response.Content.Headers.ContentType.MediaType); var content = await response.Content.ReadAsStringAsync(); var actual = JsonConvert.DeserializeObject>(content); @@ -236,5 +244,18 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests var result = await response.Content.ReadAsStringAsync(); Assert.Equal(expected, result); } + + [Fact] + public async Task ClientErrorResultFilterExecutesForStatusCodeResults() + { + // Act + var response = await Client.GetAsync("/contact/ActionReturningStatusCodeResult"); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.NotFound); + var content = await response.Content.ReadAsStringAsync(); + var problemDetails = JsonConvert.DeserializeObject(content); + Assert.Equal(404, problemDetails.Status); + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs index f5a19379d0..813c148bfe 100644 --- a/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs @@ -31,6 +31,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest var mvcOptions = services.GetRequiredService>().Value; var jsonOptions = services.GetRequiredService>().Value; var razorPagesOptions = services.GetRequiredService>().Value; + var apiBehaviorOptions = services.GetRequiredService>().Value; // Assert Assert.False(mvcOptions.AllowCombiningAuthorizeFilters); @@ -41,6 +42,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest Assert.False(razorPagesOptions.AllowAreas); Assert.False(mvcOptions.EnableEndpointRouting); Assert.Null(mvcOptions.MaxValidationDepth); + Assert.True(apiBehaviorOptions.SuppressUseValidationProblemDetailsForInvalidModelStateResponses); + Assert.True(apiBehaviorOptions.SuppressUseClientErrorFactory); } [Fact] @@ -57,6 +60,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest var mvcOptions = services.GetRequiredService>().Value; var jsonOptions = services.GetRequiredService>().Value; var razorPagesOptions = services.GetRequiredService>().Value; + var apiBehaviorOptions = services.GetRequiredService>().Value; // Assert Assert.True(mvcOptions.AllowCombiningAuthorizeFilters); @@ -67,6 +71,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest Assert.True(razorPagesOptions.AllowAreas); Assert.False(mvcOptions.EnableEndpointRouting); Assert.Null(mvcOptions.MaxValidationDepth); + Assert.True(apiBehaviorOptions.SuppressUseValidationProblemDetailsForInvalidModelStateResponses); + Assert.True(apiBehaviorOptions.SuppressUseClientErrorFactory); } [Fact] @@ -83,6 +89,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest var mvcOptions = services.GetRequiredService>().Value; var jsonOptions = services.GetRequiredService>().Value; var razorPagesOptions = services.GetRequiredService>().Value; + var apiBehaviorOptions = services.GetRequiredService>().Value; // Assert Assert.True(mvcOptions.AllowCombiningAuthorizeFilters); @@ -93,6 +100,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest Assert.True(razorPagesOptions.AllowAreas); Assert.True(mvcOptions.EnableEndpointRouting); Assert.Equal(32, mvcOptions.MaxValidationDepth); + Assert.False(apiBehaviorOptions.SuppressUseValidationProblemDetailsForInvalidModelStateResponses); + Assert.False(apiBehaviorOptions.SuppressUseClientErrorFactory); } [Fact] @@ -109,6 +118,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest var mvcOptions = services.GetRequiredService>().Value; var jsonOptions = services.GetRequiredService>().Value; var razorPagesOptions = services.GetRequiredService>().Value; + var apiBehaviorOptions = services.GetRequiredService>().Value; // Assert Assert.True(mvcOptions.AllowCombiningAuthorizeFilters); @@ -119,6 +129,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest Assert.True(razorPagesOptions.AllowAreas); Assert.True(mvcOptions.EnableEndpointRouting); Assert.Equal(32, mvcOptions.MaxValidationDepth); + Assert.False(apiBehaviorOptions.SuppressUseValidationProblemDetailsForInvalidModelStateResponses); + Assert.False(apiBehaviorOptions.SuppressUseClientErrorFactory); } // This just does the minimum needed to be able to resolve these options. diff --git a/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs b/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs index 77a757f1ff..a2cd22afc5 100644 --- a/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs +++ b/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs @@ -1,9 +1,6 @@ // 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.Collections.Generic; -using System.Linq; using System.Threading; using System.Threading.Tasks; using BasicWebSite.Models; @@ -88,6 +85,12 @@ namespace BasicWebSite return foo; } + [HttpGet("[action]")] + public ActionResult ActionReturningStatusCodeResult() + { + return NotFound(); + } + private class TestModelBinder : IModelBinder { public Task BindModelAsync(ModelBindingContext bindingContext) diff --git a/test/WebSites/BasicWebSite/RequestIdService.cs b/test/WebSites/BasicWebSite/Filters/RequestIdService.cs similarity index 100% rename from test/WebSites/BasicWebSite/RequestIdService.cs rename to test/WebSites/BasicWebSite/Filters/RequestIdService.cs diff --git a/test/WebSites/BasicWebSite/Startup.cs b/test/WebSites/BasicWebSite/Startup.cs index 1c3218266e..86d5bab150 100644 --- a/test/WebSites/BasicWebSite/Startup.cs +++ b/test/WebSites/BasicWebSite/Startup.cs @@ -1,7 +1,6 @@ // 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.Linq; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Builder; @@ -33,22 +32,6 @@ namespace BasicWebSite .SetCompatibilityVersion(CompatibilityVersion.Latest) .AddXmlDataContractSerializerFormatters(); - services.Configure(options => - { - var previous = options.InvalidModelStateResponseFactory; - options.InvalidModelStateResponseFactory = context => - { - var result = (BadRequestObjectResult)previous(context); - if (context.ActionDescriptor.FilterDescriptors.Any(f => f.Filter is VndErrorAttribute)) - { - result.ContentTypes.Clear(); - result.ContentTypes.Add("application/vnd.error+json"); - } - - return result; - }; - }); - services.ConfigureBaseWebSiteAuthPolicies(); services.AddTransient(); diff --git a/test/WebSites/BasicWebSite/StartupWithCustomInvalidModelStateFactory.cs b/test/WebSites/BasicWebSite/StartupWithCustomInvalidModelStateFactory.cs new file mode 100644 index 0000000000..27c7b58761 --- /dev/null +++ b/test/WebSites/BasicWebSite/StartupWithCustomInvalidModelStateFactory.cs @@ -0,0 +1,52 @@ +// 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.Linq; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.DependencyInjection; + +namespace BasicWebSite +{ + public class StartupWithCustomInvalidModelStateFactory + { + // Set up application services + public void ConfigureServices(IServiceCollection services) + { + services.AddAuthentication() + .AddScheme("Api", _ => { }); + + services + .AddMvc() + .SetCompatibilityVersion(CompatibilityVersion.Latest); + + services.Configure(options => + { + var previous = options.InvalidModelStateResponseFactory; + options.InvalidModelStateResponseFactory = context => + { + var result = (BadRequestObjectResult)previous(context); + if (context.ActionDescriptor.FilterDescriptors.Any(f => f.Filter is VndErrorAttribute)) + { + result.ContentTypes.Clear(); + result.ContentTypes.Add("application/vnd.error+json"); + } + + return result; + }; + }); + + services.ConfigureBaseWebSiteAuthPolicies(); + + services.AddLogging(); + services.AddSingleton(); + } + + public void Configure(IApplicationBuilder app) + { + app.UseDeveloperExceptionPage(); + app.UseMvc(); + } + } +}