diff --git a/build/dependencies.props b/build/dependencies.props index 6835b744f0..89607d37e5 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -48,8 +48,8 @@ 3.0.0-alpha1-10321 3.0.0-alpha1-10321 3.0.0-alpha1-10321 - 3.0.0-alpha1-10321 - 3.0.0-alpha1-10321 + 3.0.0-a-alpha1-master-routing-refactor-16895 + 3.0.0-a-alpha1-master-routing-refactor-16895 3.0.0-alpha1-10321 3.0.0-alpha1-10321 3.0.0-alpha1-10321 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/Internal/MvcEndpointDataSource.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs index 9e4ff9b92c..5955b3fe04 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs @@ -6,11 +6,11 @@ using System.Collections.Generic; using System.Linq; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Routing; -using Microsoft.AspNetCore.Routing.Matching; using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.AspNetCore.Routing.Template; using Microsoft.Extensions.Primitives; @@ -262,7 +262,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal return false; } - private MatcherEndpoint CreateEndpoint( + private RouteEndpoint CreateEndpoint( ActionDescriptor action, string routeName, string template, @@ -271,9 +271,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal object source, bool suppressLinkGeneration) { - RequestDelegate invokerDelegate = (context) => + RequestDelegate requestDelegate = (context) => { - var values = context.Features.Get().Values; + var values = context.Features.Get().RouteValues; var routeData = new RouteData(); foreach (var kvp in values) { @@ -299,9 +299,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal source, suppressLinkGeneration); - var endpoint = new MatcherEndpoint( - next => invokerDelegate, - RoutePatternFactory.Parse(template, defaults, constraints: null), + var endpoint = new RouteEndpoint( + requestDelegate, + RoutePatternFactory.Parse(template, defaults, parameterPolicies: null), order, metadataCollection, action.DisplayName); 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/Routing/ConsumesMatcherPolicy.cs b/src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMatcherPolicy.cs index 3adacf3c38..507d3bcf68 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMatcherPolicy.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Routing/ConsumesMatcherPolicy.cs @@ -135,8 +135,8 @@ namespace Microsoft.AspNetCore.Mvc.Routing private Endpoint CreateRejectionEndpoint() { - return new MatcherEndpoint( - (next) => (context) => + return new RouteEndpoint( + (context) => { context.Response.StatusCode = StatusCodes.Status415UnsupportedMediaType; return Task.CompletedTask; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Routing/UrlHelperFactory.cs b/src/Microsoft.AspNetCore.Mvc.Core/Routing/UrlHelperFactory.cs index 933f339f30..dfe16c2421 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Routing/UrlHelperFactory.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Routing/UrlHelperFactory.cs @@ -3,6 +3,7 @@ using System; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; 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.Core.Test/Internal/MvcEndpointDataSourceTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs index bc8d0dbcdc..ca09424a52 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs @@ -64,7 +64,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert var endpoint = Assert.Single(endpoints); - var matcherEndpoint = Assert.IsType(endpoint); + var matcherEndpoint = Assert.IsType(endpoint); var routeValuesAddressMetadata = matcherEndpoint.Metadata.GetMetadata(); Assert.NotNull(routeValuesAddressMetadata); @@ -77,14 +77,17 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public void Endpoints_InvokeReturnedEndpoint_ActionInvokerProviderCalled() + public async Task Endpoints_InvokeReturnedEndpoint_ActionInvokerProviderCalled() { // Arrange - var featureCollection = new FeatureCollection(); - featureCollection.Set(new EndpointFeature + var endpointFeature = new EndpointFeature { - Values = new RouteValueDictionary() - }); + RouteValues = new RouteValueDictionary() + }; + + var featureCollection = new FeatureCollection(); + featureCollection.Set(endpointFeature); + featureCollection.Set(endpointFeature); var httpContextMock = new Mock(); httpContextMock.Setup(m => m.Features).Returns(featureCollection); @@ -122,11 +125,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert var endpoint = Assert.Single(endpoints); - var matcherEndpoint = Assert.IsType(endpoint); + var matcherEndpoint = Assert.IsType(endpoint); - var invokerDelegate = matcherEndpoint.Invoker((next) => Task.CompletedTask); - - invokerDelegate(httpContextMock.Object); + await matcherEndpoint.RequestDelegate(httpContextMock.Object); Assert.True(actionInvokerCalled); } @@ -138,7 +139,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var featureCollection = new FeatureCollection(); featureCollection.Set(new EndpointFeature { - Values = new RouteValueDictionary() + RouteValues = new RouteValueDictionary() }); var httpContextMock = new Mock(); @@ -199,7 +200,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert var inspectors = finalEndpointTemplates - .Select(t => new Action(e => Assert.Equal(t, Assert.IsType(e).RoutePattern.RawText))) + .Select(t => new Action(e => Assert.Equal(t, Assert.IsType(e).RoutePattern.RawText))) .ToArray(); // Assert @@ -226,7 +227,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert var inspectors = finalEndpointTemplates - .Select(t => new Action(e => Assert.Equal(t, Assert.IsType(e).RoutePattern.RawText))) + .Select(t => new Action(e => Assert.Equal(t, Assert.IsType(e).RoutePattern.RawText))) .ToArray(); // Assert @@ -250,8 +251,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert Assert.Collection(endpoints, - (e) => Assert.Equal("TestController", Assert.IsType(e).RoutePattern.RawText), - (e) => Assert.Equal("TestController/TestAction", Assert.IsType(e).RoutePattern.RawText)); + (e) => Assert.Equal("TestController", Assert.IsType(e).RoutePattern.RawText), + (e) => Assert.Equal("TestController/TestAction", Assert.IsType(e).RoutePattern.RawText)); } [Fact] @@ -278,8 +279,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert Assert.Collection(endpoints1, - (e) => Assert.Equal("TestController", Assert.IsType(e).RoutePattern.RawText), - (e) => Assert.Equal("TestController/TestAction", Assert.IsType(e).RoutePattern.RawText)); + (e) => Assert.Equal("TestController", Assert.IsType(e).RoutePattern.RawText), + (e) => Assert.Equal("TestController/TestAction", Assert.IsType(e).RoutePattern.RawText)); Assert.Same(endpoints1, endpoints2); actionDescriptorCollectionProviderMock.VerifyGet(m => m.ActionDescriptors, Times.Once); @@ -320,8 +321,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal var endpoints = dataSource.Endpoints; Assert.Collection(endpoints, - (e) => Assert.Equal("TestController", Assert.IsType(e).RoutePattern.RawText), - (e) => Assert.Equal("TestController/TestAction", Assert.IsType(e).RoutePattern.RawText)); + (e) => Assert.Equal("TestController", Assert.IsType(e).RoutePattern.RawText), + (e) => Assert.Equal("TestController/TestAction", Assert.IsType(e).RoutePattern.RawText)); actionDescriptorCollectionProviderMock .Setup(m => m.ActionDescriptors) @@ -337,7 +338,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.NotSame(endpoints, newEndpoints); Assert.Collection(newEndpoints, - (e) => Assert.Equal("NewTestController/NewTestAction", Assert.IsType(e).RoutePattern.RawText)); + (e) => Assert.Equal("NewTestController/NewTestAction", Assert.IsType(e).RoutePattern.RawText)); } [Fact] @@ -359,8 +360,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert Assert.Collection(endpoints, - (e) => Assert.Equal("TestController/TestAction1", Assert.IsType(e).RoutePattern.RawText), - (e) => Assert.Equal("TestController/TestAction2", Assert.IsType(e).RoutePattern.RawText)); + (e) => Assert.Equal("TestController/TestAction1", Assert.IsType(e).RoutePattern.RawText), + (e) => Assert.Equal("TestController/TestAction2", Assert.IsType(e).RoutePattern.RawText)); } [Theory] @@ -383,7 +384,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var endpoints = dataSource.Endpoints; var inspectors = finalEndpointTemplates - .Select(t => new Action(e => Assert.Equal(t, Assert.IsType(e).RoutePattern.RawText))) + .Select(t => new Action(e => Assert.Equal(t, Assert.IsType(e).RoutePattern.RawText))) .ToArray(); // Assert @@ -405,7 +406,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert var endpoint = Assert.Single(endpoints); - var matcherEndpoint = Assert.IsType(endpoint); + var matcherEndpoint = Assert.IsType(endpoint); var routeValuesAddressNameMetadata = matcherEndpoint.Metadata.GetMetadata(); Assert.NotNull(routeValuesAddressNameMetadata); Assert.Equal(string.Empty, routeValuesAddressNameMetadata.Name); @@ -430,7 +431,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal endpoints, (ep) => { - var matcherEndpoint = Assert.IsType(ep); + var matcherEndpoint = Assert.IsType(ep); var routeValuesAddressMetadata = matcherEndpoint.Metadata.GetMetadata(); Assert.NotNull(routeValuesAddressMetadata); Assert.Equal("namedRoute", routeValuesAddressMetadata.Name); @@ -438,7 +439,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal }, (ep) => { - var matcherEndpoint = Assert.IsType(ep); + var matcherEndpoint = Assert.IsType(ep); var routeValuesAddressMetadata = matcherEndpoint.Metadata.GetMetadata(); Assert.NotNull(routeValuesAddressMetadata); Assert.Equal("namedRoute", routeValuesAddressMetadata.Name); @@ -469,25 +470,25 @@ namespace Microsoft.AspNetCore.Mvc.Internal endpoints, (ep) => { - var matcherEndpoint = Assert.IsType(ep); + var matcherEndpoint = Assert.IsType(ep); Assert.Equal("Home/Index/{id?}", matcherEndpoint.RoutePattern.RawText); Assert.Equal(1, matcherEndpoint.Order); }, (ep) => { - var matcherEndpoint = Assert.IsType(ep); + var matcherEndpoint = Assert.IsType(ep); Assert.Equal("named/Home/Index/{id?}", matcherEndpoint.RoutePattern.RawText); Assert.Equal(2, matcherEndpoint.Order); }, (ep) => { - var matcherEndpoint = Assert.IsType(ep); + var matcherEndpoint = Assert.IsType(ep); Assert.Equal("Products/Details/{id?}", matcherEndpoint.RoutePattern.RawText); Assert.Equal(1, matcherEndpoint.Order); }, (ep) => { - var matcherEndpoint = Assert.IsType(ep); + var matcherEndpoint = Assert.IsType(ep); Assert.Equal("named/Products/Details/{id?}", matcherEndpoint.RoutePattern.RawText); Assert.Equal(2, matcherEndpoint.Order); }); @@ -589,7 +590,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert var endpoint = Assert.Single(endpoints); - var matcherEndpoint = Assert.IsType(endpoint); + var matcherEndpoint = Assert.IsType(endpoint); Assert.Equal("Foo/Bar", matcherEndpoint.RoutePattern.RawText); AssertIsSubset(expectedDefaults, matcherEndpoint.RoutePattern.Defaults); } @@ -611,7 +612,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert var endpoint = Assert.Single(endpoints); - var matcherEndpoint = Assert.IsType(endpoint); + var matcherEndpoint = Assert.IsType(endpoint); Assert.Equal("Foo/Bar", matcherEndpoint.RoutePattern.RawText); AssertIsSubset(expectedDefaults, matcherEndpoint.RoutePattern.Defaults); } @@ -634,7 +635,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert var endpoint = Assert.Single(endpoints); - var matcherEndpoint = Assert.IsType(endpoint); + var matcherEndpoint = Assert.IsType(endpoint); Assert.Equal("Foo/Bar/{subscription=general}", matcherEndpoint.RoutePattern.RawText); AssertIsSubset(expectedDefaults, matcherEndpoint.RoutePattern.Defaults); } @@ -656,7 +657,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert var endpoint = Assert.Single(endpoints); - var matcherEndpoint = Assert.IsType(endpoint); + var matcherEndpoint = Assert.IsType(endpoint); Assert.Equal("Foo/Bar", matcherEndpoint.RoutePattern.RawText); AssertIsSubset(expectedDefaults, matcherEndpoint.RoutePattern.Defaults); } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionConstraintMatcherPolicyTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionConstraintMatcherPolicyTest.cs index 31e3dd8653..a423dfd2e3 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionConstraintMatcherPolicyTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ActionConstraintMatcherPolicyTest.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ActionConstraints; @@ -369,11 +370,11 @@ namespace Microsoft.AspNetCore.Mvc.Routing return httpContext; } - private static MatcherEndpoint CreateEndpoint(ActionDescriptor action) + private static RouteEndpoint CreateEndpoint(ActionDescriptor action) { var metadata = new List() { action, }; - return new MatcherEndpoint( - (r) => null, + return new RouteEndpoint( + (context) => Task.CompletedTask, RoutePatternFactory.Parse("/"), 0, new EndpointMetadataCollection(metadata), diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ConsumesMatcherPolicyTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ConsumesMatcherPolicyTest.cs index 48a49e465e..cad3edc0b9 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ConsumesMatcherPolicyTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/ConsumesMatcherPolicyTest.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Matching; @@ -212,7 +213,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing Assert.Equal(expected, actual); } - private static MatcherEndpoint CreateEndpoint(string template, ConsumesMetadata consumesMetadata) + private static RouteEndpoint CreateEndpoint(string template, ConsumesMetadata consumesMetadata) { var metadata = new List(); if (consumesMetadata != null) @@ -220,8 +221,8 @@ namespace Microsoft.AspNetCore.Mvc.Routing metadata.Add(consumesMetadata); } - return new MatcherEndpoint( - (next) => null, + return new RouteEndpoint( + (context) => Task.CompletedTask, RoutePatternFactory.Parse(template), 0, new EndpointMetadataCollection(metadata), diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/EndpointRoutingUrlHelperTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/EndpointRoutingUrlHelperTest.cs index e02dd3c5c9..6853dd7180 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/EndpointRoutingUrlHelperTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/EndpointRoutingUrlHelperTest.cs @@ -5,6 +5,8 @@ using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Matching; using Microsoft.AspNetCore.Routing.Patterns; @@ -60,8 +62,9 @@ namespace Microsoft.AspNetCore.Mvc.Routing // Set the endpoint feature and current context just as a normal request to MVC app would be var endpointFeature = new EndpointFeature(); urlHelper.ActionContext.HttpContext.Features.Set(endpointFeature); + urlHelper.ActionContext.HttpContext.Features.Set(endpointFeature); endpointFeature.Endpoint = endpoint1; - endpointFeature.Values = new RouteValueDictionary + endpointFeature.RouteValues = new RouteValueDictionary { ["controller"] = "Orders", ["action"] = "GetById", @@ -123,7 +126,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing protected override IUrlHelper CreateUrlHelper(string appRoot, string host, string protocol) { - return CreateUrlHelper(Enumerable.Empty(), appRoot, host, protocol); + return CreateUrlHelper(Enumerable.Empty(), appRoot, host, protocol); } protected override IUrlHelper CreateUrlHelperWithDefaultRoutes(string appRoot, string host, string protocol) @@ -139,8 +142,8 @@ namespace Microsoft.AspNetCore.Mvc.Routing string template) { var endpoints = GetDefaultEndpoints(); - endpoints.Add(new MatcherEndpoint( - next => httpContext => Task.CompletedTask, + endpoints.Add(new RouteEndpoint( + httpContext => Task.CompletedTask, RoutePatternFactory.Parse(template), 0, EndpointMetadataCollection.Empty, @@ -153,10 +156,8 @@ namespace Microsoft.AspNetCore.Mvc.Routing var httpContext = actionContext.HttpContext; httpContext.Features.Set(new EndpointFeature() { - Endpoint = new MatcherEndpoint( - next => cntxt => Task.CompletedTask, - RoutePatternFactory.Parse("/"), - 0, + Endpoint = new Endpoint( + context => Task.CompletedTask, EndpointMetadataCollection.Empty, null) }); @@ -187,7 +188,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing return CreateUrlHelper(actionContext); } - private IUrlHelper CreateUrlHelper(IEnumerable endpoints, ActionContext actionContext = null) + private IUrlHelper CreateUrlHelper(IEnumerable endpoints, ActionContext actionContext = null) { var serviceProvider = CreateServices(endpoints); var httpContext = CreateHttpContext(serviceProvider, null, null, "http"); @@ -196,7 +197,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing } private IUrlHelper CreateUrlHelper( - IEnumerable endpoints, + IEnumerable endpoints, string appRoot, string host, string protocol) @@ -207,9 +208,9 @@ namespace Microsoft.AspNetCore.Mvc.Routing return CreateUrlHelper(actionContext); } - private List GetDefaultEndpoints() + private List GetDefaultEndpoints() { - var endpoints = new List(); + var endpoints = new List(); endpoints.Add( CreateEndpoint( "home/newaction/{id}", @@ -278,7 +279,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing return endpoints; } - private MatcherEndpoint CreateEndpoint( + private RouteEndpoint CreateEndpoint( string template, object defaults = null, object requiredValues = null, @@ -292,9 +293,9 @@ namespace Microsoft.AspNetCore.Mvc.Routing new RouteValuesAddressMetadata(routeName, new RouteValueDictionary(requiredValues))); } - return new MatcherEndpoint( - next => (httpContext) => Task.CompletedTask, - RoutePatternFactory.Parse(template, defaults, constraints: null), + return new RouteEndpoint( + (httpContext) => Task.CompletedTask, + RoutePatternFactory.Parse(template, defaults, parameterPolicies: null), order, metadataCollection, null); @@ -315,11 +316,11 @@ namespace Microsoft.AspNetCore.Mvc.Routing return services.BuildServiceProvider(); } - private MatcherEndpoint GetEndpoint(string name, string template, RouteValueDictionary defaults) + private RouteEndpoint GetEndpoint(string name, string template, RouteValueDictionary defaults) { - return new MatcherEndpoint( - next => c => Task.CompletedTask, - RoutePatternFactory.Parse(template, defaults, constraints: null), + return new RouteEndpoint( + c => Task.CompletedTask, + RoutePatternFactory.Parse(template, defaults, parameterPolicies: null), 0, EndpointMetadataCollection.Empty, null); 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/Controllers/RoutingController.cs b/test/WebSites/BasicWebSite/Controllers/RoutingController.cs index afe914f572..e2c416505d 100644 --- a/test/WebSites/BasicWebSite/Controllers/RoutingController.cs +++ b/test/WebSites/BasicWebSite/Controllers/RoutingController.cs @@ -1,8 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Routing; namespace BasicWebSite { 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(); + } + } +} diff --git a/test/WebSites/RoutingWebSite/Controllers/RoutingController.cs b/test/WebSites/RoutingWebSite/Controllers/RoutingController.cs index 354ff5539a..ba7df5622b 100644 --- a/test/WebSites/RoutingWebSite/Controllers/RoutingController.cs +++ b/test/WebSites/RoutingWebSite/Controllers/RoutingController.cs @@ -1,8 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Routing; namespace RoutingWebSite { diff --git a/test/WebSites/VersioningWebSite/Controllers/RoutingController.cs b/test/WebSites/VersioningWebSite/Controllers/RoutingController.cs index aad2099d1d..709fb51494 100644 --- a/test/WebSites/VersioningWebSite/Controllers/RoutingController.cs +++ b/test/WebSites/VersioningWebSite/Controllers/RoutingController.cs @@ -1,8 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Routing; namespace VersioningWebSite {