From 7f214492b80a562b2f537dc535a77ee49f19c6a3 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Thu, 21 Sep 2017 11:09:32 -0700 Subject: [PATCH] Introduce a filter to send bad request results with details when ModelState is invalid (#6849) * Introduce a filter to send bad request results with details when ModelState is invalid Fixes #6789 --- .../Filters/FilterDescriptor.cs | 5 +- .../ApiBehaviorOptions.cs | 37 +++++ .../ApiControllerAttribute.cs | 4 +- .../MvcCoreServiceCollectionExtensions.cs | 5 + .../Infrastructure/ModelStateInvalidFilter.cs | 60 ++++++++ .../Internal/ApiBehaviorOptionsSetup.cs | 45 ++++++ .../ApiControllerApplicationModelProvider.cs | 73 +++++++++ .../Internal/MvcCoreLoggerExtensions.cs | 11 +- .../MvcCoreServiceCollectionExtensionsTest.cs | 8 + .../ModelStateInvalidFilterTest.cs | 84 +++++++++++ ...iControllerApplicationModelProviderTest.cs | 139 ++++++++++++++++++ .../ApiControllerAttributeTests.cs | 101 +++++++++++++ .../MvcServiceCollectionExtensionsTest.cs | 8 + .../BasicWebSite/ContactsRepository.cs | 25 ++++ .../Controllers/ContactApiController.cs | 51 +++++++ test/WebSites/BasicWebSite/Models/Contact.cs | 4 + test/WebSites/BasicWebSite/Startup.cs | 24 ++- test/WebSites/BasicWebSite/VndError.cs | 23 +++ .../BasicWebSite/VndErrorAttribute.cs | 13 ++ .../VndErrorDescriptionProvider.cs | 43 ++++++ 20 files changed, 756 insertions(+), 7 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ModelStateInvalidFilter.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiControllerApplicationModelProvider.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ModelStateInvalidFilterTest.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiControllerApplicationModelProviderTest.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiControllerAttributeTests.cs create mode 100644 test/WebSites/BasicWebSite/ContactsRepository.cs create mode 100644 test/WebSites/BasicWebSite/Controllers/ContactApiController.cs create mode 100644 test/WebSites/BasicWebSite/VndError.cs create mode 100644 test/WebSites/BasicWebSite/VndErrorAttribute.cs create mode 100644 test/WebSites/BasicWebSite/VndErrorDescriptionProvider.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/FilterDescriptor.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/FilterDescriptor.cs index bcdb7eca53..a84ea30133 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/FilterDescriptor.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/Filters/FilterDescriptor.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Diagnostics; namespace Microsoft.AspNetCore.Mvc.Filters { @@ -21,6 +22,7 @@ namespace Microsoft.AspNetCore.Mvc.Filters /// For implementations, the filter runs only after an exception has occurred, /// and so the observed order of execution will be opposite that of other filters. /// + [DebuggerDisplay("Filter = {Filter.ToString(),nq}, Order = {Order}")] public class FilterDescriptor { /// @@ -43,9 +45,8 @@ namespace Microsoft.AspNetCore.Mvc.Filters Filter = filter; Scope = filterScope; - var orderedFilter = Filter as IOrderedFilter; - if (orderedFilter != null) + if (Filter is IOrderedFilter orderedFilter) { Order = orderedFilter.Order; } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs new file mode 100644 index 0000000000..2a827c0141 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs @@ -0,0 +1,37 @@ +// 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.ModelBinding; + +namespace Microsoft.AspNetCore.Mvc +{ + /// + /// Options used to configure behavior for types annotated with . + /// + public class ApiBehaviorOptions + { + private Func _invalidModelStateResponseFactory; + + /// + /// Delegate invoked on actions annotated with to convert invalid + /// into an + /// + /// By default, the delegate produces a using + /// as the problem format. + /// + /// + public Func InvalidModelStateResponseFactory + { + get => _invalidModelStateResponseFactory; + set => _invalidModelStateResponseFactory = value ?? throw new ArgumentNullException(nameof(value)); + } + + /// + /// Disables the filter that returns an when + /// is invalid. + /// . + /// + public bool EnableModelStateInvalidFilter { get; set; } = true; + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApiControllerAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApiControllerAttribute.cs index 98b6f5c084..0d328333f7 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApiControllerAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApiControllerAttribute.cs @@ -11,8 +11,8 @@ namespace Microsoft.AspNetCore.Mvc /// this attribute can be used to target conventions, filters and other behaviors based on the purpose /// of the controller. /// - [AttributeUsage(AttributeTargets.Class)] - public class ApiControllerAttribute : ControllerAttribute , IApiBehaviorMetadata + [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)] + public class ApiControllerAttribute : ControllerAttribute, IApiBehaviorMetadata { } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index 6fe3f0f254..a6dfebfc1e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -147,6 +147,8 @@ namespace Microsoft.Extensions.DependencyInjection // services.TryAddEnumerable( ServiceDescriptor.Transient, MvcCoreMvcOptionsSetup>()); + services.TryAddEnumerable( + ServiceDescriptor.Transient, ApiBehaviorOptionsSetup>()); services.TryAddEnumerable( ServiceDescriptor.Transient, MvcCoreRouteOptionsSetup>()); @@ -157,8 +159,11 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddEnumerable( ServiceDescriptor.Transient()); + services.TryAddEnumerable( + ServiceDescriptor.Transient()); services.TryAddEnumerable( ServiceDescriptor.Transient()); + services.TryAddSingleton(); // diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ModelStateInvalidFilter.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ModelStateInvalidFilter.cs new file mode 100644 index 0000000000..43aa96daec --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ModelStateInvalidFilter.cs @@ -0,0 +1,60 @@ +// 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.Filters; +using Microsoft.AspNetCore.Mvc.Internal; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + /// + /// A that responds to invalid . This filter is + /// added to all types and actions annotated with . + /// See for ways to configure this filter. + /// + public class ModelStateInvalidFilter : IActionFilter, IOrderedFilter + { + private readonly ApiBehaviorOptions _apiBehaviorOptions; + private readonly ILogger _logger; + + public ModelStateInvalidFilter(ApiBehaviorOptions apiBehaviorOptions, ILogger logger) + { + _apiBehaviorOptions = apiBehaviorOptions ?? throw new ArgumentNullException(nameof(apiBehaviorOptions)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + } + + /// + /// Gets the order value for determining the order of execution of filters. Filters execute in + /// ascending numeric value of the property. + /// + /// + /// + /// Filters are executed in a sequence determined by an ascending sort of the property. + /// + /// + /// The default Order for this attribute is -2000 so that it runs early in the pipeline. + /// + /// + /// Look at for more detailed info. + /// + /// + public int Order => -2000; + + /// + public bool IsReusable => true; + + public void OnActionExecuted(ActionExecutedContext context) + { + } + + public void OnActionExecuting(ActionExecutingContext context) + { + if (context.Result == null && !context.ModelState.IsValid) + { + _logger.ModelStateInvalidFilterExecuting(); + context.Result = _apiBehaviorOptions.InvalidModelStateResponseFactory(context); + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs new file mode 100644 index 0000000000..52be8d2e0a --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs @@ -0,0 +1,45 @@ +// 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.Options; + +namespace Microsoft.AspNetCore.Mvc.Internal +{ + public class ApiBehaviorOptionsSetup : IConfigureOptions + { + private readonly IErrorDescriptionFactory _errorDescriptionFactory; + + public ApiBehaviorOptionsSetup(IErrorDescriptionFactory errorDescriptionFactory) + { + _errorDescriptionFactory = errorDescriptionFactory; + } + + public void Configure(ApiBehaviorOptions options) + { + if (options == null) + { + throw new ArgumentNullException(nameof(options)); + } + + options.InvalidModelStateResponseFactory = GetInvalidModelStateResponse; + + IActionResult GetInvalidModelStateResponse(ActionContext context) + { + var errorDetails = _errorDescriptionFactory.CreateErrorDescription( + context.ActionDescriptor, + new ValidationProblemDetails(context.ModelState)); + + return new BadRequestObjectResult(errorDetails) + { + ContentTypes = + { + "application/problem+json", + "application/problem+xml", + }, + }; + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiControllerApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiControllerApplicationModelProvider.cs new file mode 100644 index 0000000000..27d83200b2 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiControllerApplicationModelProvider.cs @@ -0,0 +1,73 @@ +// 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.Diagnostics; +using System.Linq; +using Microsoft.AspNetCore.Mvc.ApplicationModels; +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 ApiControllerApplicationModelProvider : IApplicationModelProvider + { + private readonly ApiBehaviorOptions _apiBehaviorOptions; + private readonly ModelStateInvalidFilter _modelStateInvalidFilter; + + public ApiControllerApplicationModelProvider(IOptions apiBehaviorOptions, ILoggerFactory loggerFactory) + { + _apiBehaviorOptions = apiBehaviorOptions.Value; + if (_apiBehaviorOptions.EnableModelStateInvalidFilter && _apiBehaviorOptions.InvalidModelStateResponseFactory == null) + { + throw new ArgumentException(Resources.FormatPropertyOfTypeCannotBeNull( + typeof(ApiBehaviorOptions), + nameof(ApiBehaviorOptions.InvalidModelStateResponseFactory))); + } + + _modelStateInvalidFilter = new ModelStateInvalidFilter( + apiBehaviorOptions.Value, + loggerFactory.CreateLogger()); + } + + /// + /// Order is set to execute after the . + /// + public int Order => -1000 + 10; + + public void OnProvidersExecuted(ApplicationModelProviderContext context) + { + } + + public void OnProvidersExecuting(ApplicationModelProviderContext context) + { + foreach (var controllerModel in context.Result.Controllers) + { + if (controllerModel.Attributes.OfType().Any()) + { + if (_apiBehaviorOptions.EnableModelStateInvalidFilter) + { + Debug.Assert(_apiBehaviorOptions.InvalidModelStateResponseFactory != null); + controllerModel.Filters.Add(_modelStateInvalidFilter); + } + + continue; + } + + foreach (var actionModel in controllerModel.Actions) + { + if (actionModel.Attributes.OfType().Any()) + { + if (_apiBehaviorOptions.EnableModelStateInvalidFilter) + { + Debug.Assert(_apiBehaviorOptions.InvalidModelStateResponseFactory != null); + actionModel.Filters.Add(_modelStateInvalidFilter); + } + } + } + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs index c50d9b5a22..c691df1ea9 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs @@ -79,7 +79,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal private static readonly Action _cannotApplyRequestFormLimits; private static readonly Action _appliedRequestFormLimits; - + + private static readonly Action _modelStateInvalidFilterExecuting; static MvcCoreLoggerExtensions() { @@ -282,6 +283,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal LogLevel.Debug, 2, "Applied the configured form options on the current request."); + + _modelStateInvalidFilterExecuting = LoggerMessage.Define( + LogLevel.Debug, + 1, + "The request has model state errors, returning an error response."); + } public static IDisposable ActionScope(this ILogger logger, ActionDescriptor action) @@ -592,6 +599,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal _appliedRequestFormLimits(logger, null); } + public static void ModelStateInvalidFilterExecuting(this ILogger logger) => _modelStateInvalidFilterExecuting(logger, null); + private class ActionLogScope : IReadOnlyList> { private readonly ActionDescriptor _action; diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs index 7e1888b3f9..5340478ff7 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs @@ -248,6 +248,13 @@ namespace Microsoft.AspNetCore.Mvc typeof(MvcCoreRouteOptionsSetup), } }, + { + typeof(IConfigureOptions), + new Type[] + { + typeof(ApiBehaviorOptionsSetup), + } + }, { typeof(IActionConstraintProvider), new Type[] @@ -288,6 +295,7 @@ namespace Microsoft.AspNetCore.Mvc new Type[] { typeof(DefaultApplicationModelProvider), + typeof(ApiControllerApplicationModelProvider), } }, }; diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ModelStateInvalidFilterTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ModelStateInvalidFilterTest.cs new file mode 100644 index 0000000000..fb5c0111c1 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ModelStateInvalidFilterTest.cs @@ -0,0 +1,84 @@ +// 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.Http; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + public class ModelStateInvalidFilterTest + { + [Fact] + public void OnActionExecuting_NoOpsIfResultIsAlreadySet() + { + // Arrange + var options = new ApiBehaviorOptions + { + InvalidModelStateResponseFactory = _ => new BadRequestResult(), + }; + var filter = new ModelStateInvalidFilter(options, NullLogger.Instance); + var context = GetActionExecutingContext(); + var expected = new OkResult(); + context.Result = expected; + + // Act + filter.OnActionExecuting(context); + + // Assert + Assert.Same(expected, context.Result); + } + + [Fact] + public void OnActionExecuting_NoOpsIfModelStateIsValid() + { + // Arrange + var options = new ApiBehaviorOptions + { + InvalidModelStateResponseFactory = _ => new BadRequestResult(), + }; + var filter = new ModelStateInvalidFilter(options, NullLogger.Instance); + var context = GetActionExecutingContext(); + + // Act + filter.OnActionExecuting(context); + + // Assert + Assert.Null(context.Result); + } + + [Fact] + public void OnActionExecuting_InvokesResponseFactoryIfModelStateIsInvalid() + { + // Arrange + var expected = new BadRequestResult(); + var options = new ApiBehaviorOptions + { + InvalidModelStateResponseFactory = _ => expected, + }; + var filter = new ModelStateInvalidFilter(options, NullLogger.Instance); + var context = GetActionExecutingContext(); + context.ModelState.AddModelError("some-key", "some-error"); + + // Act + filter.OnActionExecuting(context); + + // Assert + Assert.Same(expected, context.Result); + } + + private static ActionExecutingContext GetActionExecutingContext() + { + return new ActionExecutingContext( + new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()), + Array.Empty(), + new Dictionary(), + new object()); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiControllerApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiControllerApplicationModelProviderTest.cs new file mode 100644 index 0000000000..dc78ee56d7 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiControllerApplicationModelProviderTest.cs @@ -0,0 +1,139 @@ +// 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.Linq; +using System.Reflection; +using Microsoft.AspNetCore.Mvc.ApplicationModels; +using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.Extensions.Logging.Abstractions; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.Internal +{ + public class ApiControllerApplicationModelProviderTest + { + [Fact] + public void OnProvidersExecuting_AddsModelStateInvalidFilter_IfTypeIsAnnotatedWithAttribute() + { + // Arrange + var context = GetContext(typeof(TestApiController)); + var options = new TestOptionsManager(new ApiBehaviorOptions + { + InvalidModelStateResponseFactory = _ => null, + }); + + var provider = new ApiControllerApplicationModelProvider(options, NullLoggerFactory.Instance); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var controllerModel = Assert.Single(context.Result.Controllers); + Assert.IsType(controllerModel.Filters.Last()); + } + + [Fact] + public void OnProvidersExecuting_DoesNotAddModelStateInvalidFilterToController_IfFeatureIsDisabledViaOptions() + { + // Arrange + var context = GetContext(typeof(TestApiController)); + var options = new TestOptionsManager(new ApiBehaviorOptions + { + EnableModelStateInvalidFilter = false, + }); + + var provider = new ApiControllerApplicationModelProvider(options, NullLoggerFactory.Instance); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + var controllerModel = Assert.Single(context.Result.Controllers); + Assert.DoesNotContain(typeof(ModelStateInvalidFilter), controllerModel.Filters.Select(f => f.GetType())); + } + + [Fact] + public void OnProvidersExecuting_AddsModelStateInvalidFilter_IfActionIsAnnotatedWithAttribute() + { + // Arrange + var context = GetContext(typeof(SimpleController)); + var options = new TestOptionsManager(new ApiBehaviorOptions + { + InvalidModelStateResponseFactory = _ => null, + }); + + var provider = new ApiControllerApplicationModelProvider(options, NullLoggerFactory.Instance); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + Assert.Collection( + Assert.Single(context.Result.Controllers).Actions.OrderBy(a => a.ActionName), + action => + { + Assert.Contains(typeof(ModelStateInvalidFilter), action.Filters.Select(f => f.GetType())); + }, + action => + { + Assert.DoesNotContain(typeof(ModelStateInvalidFilter), action.Filters.Select(f => f.GetType())); + }); + } + + [Fact] + public void OnProvidersExecuting_SkipsAddingFilterToActionIfFeatureIsDisabledUsingOptions() + { + // Arrange + var context = GetContext(typeof(SimpleController)); + var options = new TestOptionsManager(new ApiBehaviorOptions + { + EnableModelStateInvalidFilter = false, + }); + + var provider = new ApiControllerApplicationModelProvider(options, NullLoggerFactory.Instance); + + // Act + provider.OnProvidersExecuting(context); + + // Assert + Assert.Collection( + Assert.Single(context.Result.Controllers).Actions.OrderBy(a => a.ActionName), + action => + { + Assert.DoesNotContain(typeof(ModelStateInvalidFilter), action.Filters.Select(f => f.GetType())); + }, + action => + { + Assert.DoesNotContain(typeof(ModelStateInvalidFilter), action.Filters.Select(f => f.GetType())); + }); + } + + private static ApplicationModelProviderContext GetContext(Type type) + { + var context = new ApplicationModelProviderContext(new[] { type.GetTypeInfo() }); + new DefaultApplicationModelProvider(new TestOptionsManager()).OnProvidersExecuting(context); + return context; + } + + [ApiController] + private class TestApiController : Controller + { + public IActionResult TestAction() => null; + } + + + private class SimpleController : Controller + { + public IActionResult ActionWithoutFilter() => null; + + [TestApiBehavior] + public IActionResult ActionWithFilter() => null; + } + + [AttributeUsage(AttributeTargets.Method)] + private class TestApiBehavior : Attribute, IApiBehaviorMetadata + { + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiControllerAttributeTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiControllerAttributeTests.cs new file mode 100644 index 0000000000..61183259ce --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiControllerAttributeTests.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.Linq; +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using BasicWebSite; +using BasicWebSite.Models; +using Newtonsoft.Json; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.FunctionalTests +{ + public class ApiControllerAttributeTests : IClassFixture> + { + public ApiControllerAttributeTests(MvcTestFixture fixture) + { + Client = fixture.Client; + } + + public HttpClient Client { get; } + + [Fact] + public async Task ActionsReturnBadRequest_WhenModelStateIsInvalid() + { + // Arrange + var contactModel = new Contact + { + Name = "Abc", + City = "Redmond", + State = "WA", + Zip = "Invalid", + }; + var expected = new ValidationProblemDetails + { + Errors = + { + ["Zip"] = new[] { @"The field Zip must match the regular expression '\d{5}'." }, + ["Name"] = new[] { "The field Name must be a string with a minimum length of 5 and a maximum length of 30." }, + }, + }; + var contactString = JsonConvert.SerializeObject(contactModel); + + // Act + var response = await Client.PostAsJsonAsync("/contact", contactModel); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + Assert.Equal("application/problem+json", response.Content.Headers.ContentType.MediaType); + var actual = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); + Assert.Equal(expected.Errors.Count, actual.Errors.Count); + foreach (var error in expected.Errors) + { + Assert.Equal(error.Value, actual.Errors[error.Key]); + } + } + + [Fact] + public async Task ActionsReturnBadRequest_UsesProblemDescriptionProviderAndApiConventionsToConfigureErrorResponse() + { + // Arrange + var contactModel = new Contact + { + Name = "Abc", + City = "Redmond", + State = "WA", + Zip = "Invalid", + }; + var expected = new[] + { + new VndError + { + Path = "Name", + Message = "The field Name must be a string with a minimum length of 5 and a maximum length of 30.", + }, + new VndError + { + Path = "Zip", + Message = @"The field Zip must match the regular expression '\d{5}'.", + }, + }; + var contactString = JsonConvert.SerializeObject(contactModel); + + // Act + var response = await Client.PostAsJsonAsync("/contact/PostWithVnd", contactModel); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + Assert.Equal("application/vnd.error+json", response.Content.Headers.ContentType.MediaType); + var actual = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); + actual = actual.OrderBy(e => e.Path).ToArray(); + Assert.Equal(expected.Length, actual.Length); + for (var i = 0; i < expected.Length; i++) + { + Assert.Equal(expected[i].Path, expected[i].Path); + Assert.Equal(expected[i].Message, expected[i].Message); + } + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs b/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs index 2d982e334f..8f34600cc9 100644 --- a/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs @@ -348,6 +348,13 @@ namespace Microsoft.AspNetCore.Mvc typeof(MvcCoreRouteOptionsSetup), } }, + { + typeof(IConfigureOptions), + new Type[] + { + typeof(ApiBehaviorOptionsSetup), + } + }, { typeof(IConfigureOptions), new Type[] @@ -410,6 +417,7 @@ namespace Microsoft.AspNetCore.Mvc typeof(CorsApplicationModelProvider), typeof(AuthorizationApplicationModelProvider), typeof(TempDataApplicationModelProvider), + typeof(ApiControllerApplicationModelProvider), } }, { diff --git a/test/WebSites/BasicWebSite/ContactsRepository.cs b/test/WebSites/BasicWebSite/ContactsRepository.cs new file mode 100644 index 0000000000..49589b48af --- /dev/null +++ b/test/WebSites/BasicWebSite/ContactsRepository.cs @@ -0,0 +1,25 @@ +// 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.Collections.Generic; +using System.Linq; +using BasicWebSite.Models; + +namespace BasicWebSite +{ + public class ContactsRepository + { + private readonly List _contacts = new List(); + + public Contact GetContact(int id) + { + return _contacts.FirstOrDefault(f => f.ContactId == id); + } + + public void Add(Contact contact) + { + contact.ContactId = _contacts.Count + 1; + _contacts.Add(contact); + } + } +} \ No newline at end of file diff --git a/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs b/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs new file mode 100644 index 0000000000..43006e7966 --- /dev/null +++ b/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs @@ -0,0 +1,51 @@ +// 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.Tasks; +using BasicWebSite.Models; +using Microsoft.AspNetCore.Mvc; + +namespace BasicWebSite +{ + [ApiController] + [Route("/contact")] + public class ContactApiController : Controller + { + private readonly ContactsRepository _repository; + + public ContactApiController(ContactsRepository repository) + { + _repository = repository; + } + + [HttpGet("{id}")] + public ActionResult Get(int id) + { + var contact = _repository.GetContact(id); + if (contact == null) + { + return NotFound(); + } + + return contact; + } + + [HttpPost] + public ActionResult Post([FromBody] Contact contact) + { + _repository.Add(contact); + return CreatedAtAction(nameof(Get), new { id = contact.ContactId }, contact); + } + + [VndError] + [HttpPost("PostWithVnd")] + public ActionResult PostWithVnd([FromBody] Contact contact) + { + _repository.Add(contact); + return CreatedAtAction(nameof(Get), new { id = contact.ContactId }, contact); + } + } +} \ No newline at end of file diff --git a/test/WebSites/BasicWebSite/Models/Contact.cs b/test/WebSites/BasicWebSite/Models/Contact.cs index 3c4190a807..a6514e7e10 100644 --- a/test/WebSites/BasicWebSite/Models/Contact.cs +++ b/test/WebSites/BasicWebSite/Models/Contact.cs @@ -1,12 +1,15 @@ // 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.ComponentModel.DataAnnotations; + namespace BasicWebSite.Models { public class Contact { public int ContactId { get; set; } + [StringLength(30, MinimumLength = 5)] public string Name { get; set; } public GenderType Gender { get; set; } @@ -17,6 +20,7 @@ namespace BasicWebSite.Models public string State { get; set; } + [RegularExpression(@"\d{5}")] public string Zip { get; set; } public string Email { get; set; } diff --git a/test/WebSites/BasicWebSite/Startup.cs b/test/WebSites/BasicWebSite/Startup.cs index 158b3604c1..4aea85a78f 100644 --- a/test/WebSites/BasicWebSite/Startup.cs +++ b/test/WebSites/BasicWebSite/Startup.cs @@ -2,10 +2,13 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.IO; +using System.Linq; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.Extensions.DependencyInjection; namespace BasicWebSite @@ -16,13 +19,30 @@ namespace BasicWebSite public void ConfigureServices(IServiceCollection services) { services - .AddMvc( - options => { options.Conventions.Add(new ApplicationDescription("This is a basic website.")); }) + .AddMvc(options => options.Conventions.Add(new ApplicationDescription("This is a basic website."))) .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.AddLogging(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); + services.AddSingleton(); services.AddScoped(); } diff --git a/test/WebSites/BasicWebSite/VndError.cs b/test/WebSites/BasicWebSite/VndError.cs new file mode 100644 index 0000000000..e134062067 --- /dev/null +++ b/test/WebSites/BasicWebSite/VndError.cs @@ -0,0 +1,23 @@ +// 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.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Controllers; +using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.Extensions.DependencyInjection; + +namespace BasicWebSite +{ + public class VndError + { + public string LogRef { get; set; } + + public string Path { get; set; } + + public string Message { get; set; } + } +} \ No newline at end of file diff --git a/test/WebSites/BasicWebSite/VndErrorAttribute.cs b/test/WebSites/BasicWebSite/VndErrorAttribute.cs new file mode 100644 index 0000000000..c5b3f973e5 --- /dev/null +++ b/test/WebSites/BasicWebSite/VndErrorAttribute.cs @@ -0,0 +1,13 @@ +// 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.Filters; + +namespace BasicWebSite +{ + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)] + public class VndErrorAttribute : Attribute, IFilterMetadata + { + } +} \ No newline at end of file diff --git a/test/WebSites/BasicWebSite/VndErrorDescriptionProvider.cs b/test/WebSites/BasicWebSite/VndErrorDescriptionProvider.cs new file mode 100644 index 0000000000..e63cb90bbd --- /dev/null +++ b/test/WebSites/BasicWebSite/VndErrorDescriptionProvider.cs @@ -0,0 +1,43 @@ +// 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.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Controllers; +using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.Extensions.DependencyInjection; + +namespace BasicWebSite +{ + public class VndErrorDescriptionProvider : IErrorDescriptorProvider + { + public int Order => 0; + + public void OnProvidersExecuting(ErrorDescriptionContext context) + { + if (context.ActionDescriptor.FilterDescriptors.Any(f => f.Filter is VndErrorAttribute) && + context.Result is ValidationProblemDetails problemDetails) + { + var vndErrors = new List(); + foreach (var item in problemDetails.Errors) + { + foreach (var message in item.Value) + { + vndErrors.Add(new VndError + { + LogRef = problemDetails.Title, + Path = item.Key, + Message = message, + }); + } + } + + context.Result = vndErrors; + } + } + } +} \ No newline at end of file