diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ApiExplorer/ApiDescription.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ApiExplorer/ApiDescription.cs index 40107e4e8a..827712f1bc 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ApiExplorer/ApiDescription.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ApiExplorer/ApiDescription.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.Collections.Generic; +using System.Diagnostics; using Microsoft.AspNetCore.Mvc.Abstractions; namespace Microsoft.AspNetCore.Mvc.ApiExplorer @@ -9,6 +10,7 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer /// /// Represents an API exposed by this application. /// + [DebuggerDisplay("{ActionDescriptor.DisplayName,nq}")] public class ApiDescription { /// diff --git a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DependencyInjection/MvcApiExplorerMvcCoreBuilderExtensions.cs b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DependencyInjection/MvcApiExplorerMvcCoreBuilderExtensions.cs index 0d3b74c378..9f7668d194 100644 --- a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DependencyInjection/MvcApiExplorerMvcCoreBuilderExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DependencyInjection/MvcApiExplorerMvcCoreBuilderExtensions.cs @@ -26,6 +26,8 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddSingleton(); services.TryAddEnumerable( ServiceDescriptor.Transient()); + services.TryAddEnumerable( + ServiceDescriptor.Transient()); } } } diff --git a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/ProblemDetailsApiDescriptionProvider.cs b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/ProblemDetailsApiDescriptionProvider.cs new file mode 100644 index 0000000000..917588f86c --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/ProblemDetailsApiDescriptionProvider.cs @@ -0,0 +1,77 @@ +// 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 Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.ModelBinding; + +namespace Microsoft.AspNetCore.Mvc.ApiExplorer +{ + public class ProblemDetailsApiDescriptionProvider : IApiDescriptionProvider + { + private readonly IModelMetadataProvider _modelMetadaProvider; + + public ProblemDetailsApiDescriptionProvider(IModelMetadataProvider modelMetadataProvider) + { + _modelMetadaProvider = modelMetadataProvider; + } + + /// + /// The order is set to execute after the . + /// + public int Order => -1000 + 10; + + public void OnProvidersExecuted(ApiDescriptionProviderContext context) + { + } + + public void OnProvidersExecuting(ApiDescriptionProviderContext context) + { + foreach (var apiDescription in context.Results) + { + if (!apiDescription.ActionDescriptor.FilterDescriptors.Any(f => f.Filter is ProblemDetailsAttribute)) + { + continue; + } + + var parameters = apiDescription.ActionDescriptor.Parameters.Concat(apiDescription.ActionDescriptor.BoundProperties); + if (parameters.Any()) + { + apiDescription.SupportedResponseTypes.Add(CreateProblemResponse(StatusCodes.Status400BadRequest)); + + if (parameters.Any(p => p.Name.EndsWith("id", StringComparison.OrdinalIgnoreCase))) + { + apiDescription.SupportedResponseTypes.Add(CreateProblemResponse(StatusCodes.Status404NotFound)); + } + } + + // We don't have a good way to signal a "default" response type. We'll use 0 to indicate this until we come up + // with something better. + apiDescription.SupportedResponseTypes.Add(CreateProblemResponse(statusCode: 0)); + } + } + + private ApiResponseType CreateProblemResponse(int statusCode) + { + return new ApiResponseType + { + ApiResponseFormats = new List + { + new ApiResponseFormat + { + MediaType = "application/problem+json", + }, + new ApiResponseFormat + { + MediaType = "application/problem+xml", + }, + }, + ModelMetadata = _modelMetadaProvider.GetMetadataForType(typeof(ProblemDetails)), + StatusCode = statusCode, + Type = typeof(ProblemDetails), + }; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs b/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs index 946bef7d71..692627c6e8 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs @@ -1503,7 +1503,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// The created for the response. [NonAction] - public virtual BadRequestObjectResult ValidationProblem(ValidationProblemDescription descriptor) + public virtual ActionResult ValidationProblem(ValidationProblemDetails descriptor) { if (descriptor == null) { @@ -1518,14 +1518,14 @@ namespace Microsoft.AspNetCore.Mvc /// /// The created for the response. [NonAction] - public virtual BadRequestObjectResult ValidationProblem(ModelStateDictionary modelStateDictionary) + public virtual ActionResult ValidationProblem(ModelStateDictionary modelStateDictionary) { if (modelStateDictionary == null) { throw new ArgumentNullException(nameof(modelStateDictionary)); } - var validationProblem = new ValidationProblemDescription(modelStateDictionary); + var validationProblem = new ValidationProblemDetails(modelStateDictionary); return new BadRequestObjectResult(validationProblem); } @@ -1535,9 +1535,9 @@ namespace Microsoft.AspNetCore.Mvc /// /// The created for the response. [NonAction] - public virtual BadRequestObjectResult ValidationProblem() + public virtual ActionResult ValidationProblem() { - var validationProblem = new ValidationProblemDescription(ModelState); + var validationProblem = new ValidationProblemDetails(ModelState); return new BadRequestObjectResult(validationProblem); } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index 1e38d3c00f..cb00dfca6c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -204,6 +204,9 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddTransient(); services.TryAddTransient(); + // Error description + services.TryAddSingleton(); + // // ModelBinding, Validation // diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ErrorDescriptionContext.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ErrorDescriptionContext.cs new file mode 100644 index 0000000000..10370fb514 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/ErrorDescriptionContext.cs @@ -0,0 +1,19 @@ +// 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.Mvc.Abstractions; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + public class ErrorDescriptionContext + { + public ErrorDescriptionContext(ActionDescriptor actionDescriptor) + { + ActionDescriptor = actionDescriptor; + } + + public ActionDescriptor ActionDescriptor { get; } + + public object Result { get; set; } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IErrorDescriptionFactory.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IErrorDescriptionFactory.cs new file mode 100644 index 0000000000..7cddc853ff --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IErrorDescriptionFactory.cs @@ -0,0 +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 Microsoft.AspNetCore.Mvc.Abstractions; + +namespace Microsoft.AspNetCore.Mvc.Infrastructure +{ + /// + /// Defines a contract for creating or modifying an error response. + /// + public interface IErrorDescriptionFactory + { + object CreateErrorDescription(ActionDescriptor actionDescriptor, object result); + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IErrorDescriptorProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IErrorDescriptorProvider.cs new file mode 100644 index 0000000000..9ca38e8254 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IErrorDescriptorProvider.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 +{ + public interface IErrorDescriptorProvider + { + int Order { get; } + + void OnProvidersExecuting(ErrorDescriptionContext context); + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultErrorDescriptorFactory.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultErrorDescriptorFactory.cs new file mode 100644 index 0000000000..667e3199b1 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultErrorDescriptorFactory.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 Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Infrastructure; + +namespace Microsoft.AspNetCore.Mvc.Internal +{ + public class DefaultErrorDescriptorFactory : IErrorDescriptionFactory + { + private readonly IErrorDescriptorProvider[] _providers; + + public DefaultErrorDescriptorFactory(IEnumerable providers) + { + if (providers == null) + { + throw new ArgumentNullException(nameof(providers)); + } + + _providers = providers.OrderBy(p => p.Order).ToArray(); + } + + public object CreateErrorDescription(ActionDescriptor actionDescriptor, object result) + { + if (actionDescriptor == null) + { + throw new ArgumentNullException(nameof(actionDescriptor)); + } + + if (result == null) + { + throw new ArgumentNullException(nameof(result)); + } + + var context = new ErrorDescriptionContext(actionDescriptor) + { + Result = result, + }; + + for (var i = 0; i < _providers.Length; i++) + { + _providers[i].OnProvidersExecuting(context); + } + + return context.Result ?? result; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ProblemDescription.cs b/src/Microsoft.AspNetCore.Mvc.Core/ProblemDetails.cs similarity index 98% rename from src/Microsoft.AspNetCore.Mvc.Core/ProblemDescription.cs rename to src/Microsoft.AspNetCore.Mvc.Core/ProblemDetails.cs index dedd6a0c48..35ac215562 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ProblemDescription.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ProblemDetails.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// A machine-readable format for specifying errors in HTTP API responses based on https://tools.ietf.org/html/rfc7807. /// - public class ProblemDescription + public class ProblemDetails { /// /// A URI reference [RFC3986] that identifies the problem type. This specification encourages that, when diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ProblemDetailsAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/ProblemDetailsAttribute.cs new file mode 100644 index 0000000000..e837470761 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/ProblemDetailsAttribute.cs @@ -0,0 +1,16 @@ +// 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 Microsoft.AspNetCore.Mvc +{ + /// + /// Adds an that indicates to the framework that the current action conforms to well-known API behavior. + /// + [AttributeUsage(AttributeTargets.Method | AttributeTargets.Class, AllowMultiple = false, Inherited = true)] + public class ProblemDetailsAttribute : Attribute, IFilterMetadata + { + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx b/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx index fb875f3667..eceaf33576 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx @@ -1,17 +1,17 @@  - diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ValidationProblemDescription.cs b/src/Microsoft.AspNetCore.Mvc.Core/ValidationProblemDetails.cs similarity index 86% rename from src/Microsoft.AspNetCore.Mvc.Core/ValidationProblemDescription.cs rename to src/Microsoft.AspNetCore.Mvc.Core/ValidationProblemDetails.cs index 610707566a..b27e790a90 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ValidationProblemDescription.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ValidationProblemDetails.cs @@ -9,19 +9,19 @@ using Microsoft.AspNetCore.Mvc.ModelBinding; namespace Microsoft.AspNetCore.Mvc { /// - /// A for validation errors. + /// A for validation errors. /// - public class ValidationProblemDescription : ProblemDescription + public class ValidationProblemDetails : ProblemDetails { /// - /// Intializes a new instance of . + /// Intializes a new instance of . /// - public ValidationProblemDescription() + public ValidationProblemDetails() { Title = Resources.ValidationProblemDescription_Title; } - public ValidationProblemDescription(ModelStateDictionary modelState) + public ValidationProblemDetails(ModelStateDictionary modelState) : this() { if (modelState == null) @@ -62,7 +62,7 @@ namespace Microsoft.AspNetCore.Mvc } /// - /// Gets or sets the validation errors associated with this instance of . + /// Gets or sets the validation errors associated with this instance of . /// public IDictionary Errors { get; } = new Dictionary(StringComparer.Ordinal); } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ValidationProblemDescriptionTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ValidationProblemDetailsTest.cs similarity index 88% rename from test/Microsoft.AspNetCore.Mvc.Core.Test/ValidationProblemDescriptionTest.cs rename to test/Microsoft.AspNetCore.Mvc.Core.Test/ValidationProblemDetailsTest.cs index 7d6e062005..2fb53be244 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ValidationProblemDescriptionTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ValidationProblemDetailsTest.cs @@ -6,13 +6,13 @@ using Xunit; namespace Microsoft.AspNetCore.Mvc { - public class ValidationProblemDescriptionTest + public class ValidationProblemDetailsTest { [Fact] public void Constructor_SetsTitle() { // Arrange & Act - var problemDescription = new ValidationProblemDescription(); + var problemDescription = new ValidationProblemDetails(); // Assert Assert.Equal("One or more validation errors occured.", problemDescription.Title); @@ -30,7 +30,7 @@ namespace Microsoft.AspNetCore.Mvc modelStateDictionary.AddModelError("key3", "error3"); // Act - var problemDescription = new ValidationProblemDescription(modelStateDictionary); + var problemDescription = new ValidationProblemDetails(modelStateDictionary); // Assert Assert.Equal("One or more validation errors occured.", problemDescription.Title); diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs index 936a56fd89..322ca76658 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiExplorerTest.cs @@ -1065,6 +1065,88 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal("ApiExplorerInboundOutbound/SuppressedForLinkGeneration", description.RelativePath); } + [Fact] + public async Task ProblemDetails_AddsProblemAsDefaultErrorResult() + { + // Act + var body = await Client.GetStringAsync("ApiExplorerProblemDetails/ActionWithoutParameters"); + var result = JsonConvert.DeserializeObject>(body); + + // Assert + var description = Assert.Single(result); + Assert.Collection( + description.SupportedResponseTypes, + response => + { + Assert.Equal(0, response.StatusCode); + AssertProblemDetails(response); + }); + } + + [Fact] + public async Task ProblemDetails_AddsProblemAsErrorResultForBadResult_WhenActionHasParameters() + { + // Act + var body = await Client.GetStringAsync("ApiExplorerProblemDetails/ActionWithSomeParameters"); + var result = JsonConvert.DeserializeObject>(body); + + // Assert + var description = Assert.Single(result); + Assert.Collection( + description.SupportedResponseTypes.OrderBy(r => r.StatusCode), + response => + { + Assert.Equal(0, response.StatusCode); + AssertProblemDetails(response); + }, + response => Assert.Equal(200, response.StatusCode), + response => + { + Assert.Equal(400, response.StatusCode); + AssertProblemDetails(response); + }); + } + + [Theory] + [InlineData("ApiExplorerProblemDetails/ActionWithIdParameter")] + [InlineData("ApiExplorerProblemDetails/ActionWithIdSuffixParameter")] + public async Task ProblemDetails_AddsProblemAsErrorResultForNotFoundResult_WhenActionHasAnIdParameters(string url) + { + // Act + var body = await Client.GetStringAsync(url); + var result = JsonConvert.DeserializeObject>(body); + + // Assert + var description = Assert.Single(result); + Assert.Collection( + description.SupportedResponseTypes.OrderBy(r => r.StatusCode), + response => + { + Assert.Equal(0, response.StatusCode); + AssertProblemDetails(response); + }, + response => Assert.Equal(200, response.StatusCode), + response => + { + Assert.Equal(400, response.StatusCode); + AssertProblemDetails(response); + }, + response => + { + Assert.Equal(404, response.StatusCode); + AssertProblemDetails(response); + }); + } + + private void AssertProblemDetails(ApiExplorerResponseType response) + { + Assert.Equal("Microsoft.AspNetCore.Mvc.ProblemDetails", response.ResponseType); + Assert.Collection( + GetSortedMediaTypes(response), + mediaType => Assert.Equal("application/problem+json", mediaType), + mediaType => Assert.Equal("application/problem+xml", mediaType)); + } + private IEnumerable GetSortedMediaTypes(ApiExplorerResponseType apiResponseType) { return apiResponseType.ResponseFormats diff --git a/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs b/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs index 58164f6c8f..2d982e334f 100644 --- a/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs @@ -23,7 +23,6 @@ using Microsoft.AspNetCore.Mvc.Razor; using Microsoft.AspNetCore.Mvc.Razor.Compilation; using Microsoft.AspNetCore.Mvc.Razor.Internal; using Microsoft.AspNetCore.Mvc.Razor.TagHelpers; -using Microsoft.AspNetCore.Mvc.RazorPages; using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure; using Microsoft.AspNetCore.Mvc.RazorPages.Internal; using Microsoft.AspNetCore.Mvc.TagHelpers; @@ -418,6 +417,7 @@ namespace Microsoft.AspNetCore.Mvc new Type[] { typeof(DefaultApiDescriptionProvider), + typeof(ProblemDetailsApiDescriptionProvider), typeof(JsonPatchOperationsArrayProvider), } }, diff --git a/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerProblemDetailsController.cs b/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerProblemDetailsController.cs new file mode 100644 index 0000000000..cdf82b6336 --- /dev/null +++ b/test/WebSites/ApiExplorerWebSite/Controllers/ApiExplorerProblemDetailsController.cs @@ -0,0 +1,26 @@ +// 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.Mvc; + +namespace ApiExplorerWebSite +{ + [Route("ApiExplorerProblemDetails/[action]")] + [ProblemDetails] + public class ApiExplorerProblemDetailsController : Controller + { + public IActionResult ActionWithoutParameters() => Ok(); + + public void ActionWithSomeParameters(object input) + { + } + + public void ActionWithIdParameter(int id, string name) + { + } + + public void ActionWithIdSuffixParameter(int personId, string personName) + { + } + } +} \ No newline at end of file