From 9b4d3aac8fcfdf14a97191f83c9b5208602305b7 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 7 Jun 2021 18:26:06 -0700 Subject: [PATCH 01/24] WIP --- ...malActionEndpointRouteBuilderExtensions.cs | 8 ++- ...ndpointMethodInfoApiDescriptionProvider.cs | 68 +++++++++++++++++++ .../src/EndpointMethodInfoModelMetadata.cs | 57 ++++++++++++++++ ...icrosoft.AspNetCore.Mvc.ApiExplorer.csproj | 4 ++ 4 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs create mode 100644 src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoModelMetadata.cs diff --git a/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs b/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs index fb538f4d5283..9d8c6a7c33d7 100644 --- a/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs @@ -61,7 +61,7 @@ public static MinimalActionEndpointConventionBuilder MapPost( /// The to add the route to. /// The route pattern. /// The delegate executed when the endpoint is matched. - /// A that canaction be used to further customize the endpoint. + /// A that can be used to further customize the endpoint. public static MinimalActionEndpointConventionBuilder MapPut( this IEndpointRouteBuilder endpoints, string pattern, @@ -166,6 +166,12 @@ public static MinimalActionEndpointConventionBuilder Map( DisplayName = pattern.RawText ?? pattern.DebuggerToString(), }; + // REVIEW: Should we add an IActionMethodMetadata with just MethodInfo on it so we are + // explicit about the MethodInfo representing the "action" and not the RequestDelegate? + + // Add MethodInfo as metadata to assist with OpenAPI generation for the endpoint. + builder.Metadata.Add(action.Method); + // Add delegate attributes as metadata var attributes = action.Method.GetCustomAttributes(); diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs new file mode 100644 index 000000000000..3f127d3d6e62 --- /dev/null +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs @@ -0,0 +1,68 @@ +// 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.Reflection; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.Routing.Patterns; + +namespace Microsoft.AspNetCore.Mvc.ApiExplorer +{ + internal class EndpointMethodInfoApiDescriptionProvider : IApiDescriptionProvider + { + private readonly EndpointDataSource _endpointDataSource; + + // Executes before MVC's DefaultApiDescriptionProvider and GrpcHttpApiDescriptionProvider + public int Order => -1100; + + public EndpointMethodInfoApiDescriptionProvider(EndpointDataSource endpointDataSource) + { + _endpointDataSource = endpointDataSource; + } + + public void OnProvidersExecuting(ApiDescriptionProviderContext context) + { + foreach (var endpoint in _endpointDataSource.Endpoints) + { + if (endpoint is RouteEndpoint routeEndpoint + && routeEndpoint.Metadata.GetMetadata() is { } methodInfo + && routeEndpoint.Metadata.GetMetadata() is { } httpMethodMetadata) + { + foreach (var httpMethod in httpMethodMetadata.HttpMethods) + { + context.Results.Add(CreateApiDescription(routeEndpoint.RoutePattern, httpMethod, methodInfo)); + } + } + } + } + + public void OnProvidersExecuted(ApiDescriptionProviderContext context) + { + } + + private static ApiDescription CreateApiDescription(RoutePattern pattern, string httpMethod, MethodInfo actionMethodInfo) + { + var apiDescription = new ApiDescription + { + HttpMethod = httpMethod, + RelativePath = pattern.ToString(), + ActionDescriptor = new ActionDescriptor + { + RouteValues = new Dictionary + { + // Swagger uses this to group endpoints together. + // For now, put all endpoints configured with Map(Delegate) together. + // TODO: Use some other metadata for this. + ["controller"] = "Map" + }, + }, + }; + + //var returnType = actionMethodInfo.ReturnType. + //if (actionMethodInfo.) + + return apiDescription; + } + } +} diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoModelMetadata.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoModelMetadata.cs new file mode 100644 index 000000000000..5104b53fe76f --- /dev/null +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoModelMetadata.cs @@ -0,0 +1,57 @@ +// 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.Collections.Immutable; +using System.Linq; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; + +namespace Microsoft.AspNetCore.Mvc.ApiExplorer +{ + internal class EndpointMethodInfoModelMetadata : ModelMetadata + { + public EndpointMethodInfoModelMetadata(ModelMetadataIdentity identity) : base(identity) + { + IsBindingAllowed = true; + } + + public override IReadOnlyDictionary AdditionalValues { get; } = ImmutableDictionary.Empty; + public override string? BinderModelName { get; } + public override Type? BinderType { get; } + public override BindingSource? BindingSource { get; } + public override bool ConvertEmptyStringToNull { get; } + public override string? DataTypeName { get; } + public override string? Description { get; } + public override string? DisplayFormatString { get; } + public override string? DisplayName { get; } + public override string? EditFormatString { get; } + public override ModelMetadata? ElementMetadata { get; } + public override IEnumerable>? EnumGroupedDisplayNamesAndValues { get; } + public override IReadOnlyDictionary? EnumNamesAndValues { get; } + public override bool HasNonDefaultEditFormat { get; } + public override bool HideSurroundingHtml { get; } + public override bool HtmlEncode { get; } + public override bool IsBindingAllowed { get; } + public override bool IsBindingRequired { get; } + public override bool IsEnum { get; } + public override bool IsFlagsEnum { get; } + public override bool IsReadOnly { get; } + public override bool IsRequired { get; } + public override ModelBindingMessageProvider ModelBindingMessageProvider { get; } = new DefaultModelBindingMessageProvider(); + public override string? NullDisplayText { get; } + public override int Order { get; } + public override string? Placeholder { get; } + public override ModelPropertyCollection Properties { get; } = new(Enumerable.Empty()); + public override IPropertyFilterProvider? PropertyFilterProvider { get; } + public override Func? PropertyGetter { get; } + public override Action? PropertySetter { get; } + public override bool ShowForDisplay { get; } + public override bool ShowForEdit { get; } + public override string? SimpleDisplayProperty { get; } + public override string? TemplateHint { get; } + public override bool ValidateChildren { get; } + public override IReadOnlyList ValidatorMetadata { get; } = Array.Empty(); + } +} diff --git a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj index a3ee0007eedc..ac27f6442e1b 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj +++ b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj @@ -9,6 +9,10 @@ false + + + + From 6aefae52b6e713390fa8f6aa3399d1b0dd27e1db Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 8 Jun 2021 11:18:54 -0700 Subject: [PATCH 02/24] Add to slnf --- src/Mvc/Mvc.slnf | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Mvc/Mvc.slnf b/src/Mvc/Mvc.slnf index b6e6c9fbf539..0d133ef4b69f 100644 --- a/src/Mvc/Mvc.slnf +++ b/src/Mvc/Mvc.slnf @@ -32,6 +32,7 @@ "src\\Http\\Routing.Abstractions\\src\\Microsoft.AspNetCore.Routing.Abstractions.csproj", "src\\Http\\Routing\\src\\Microsoft.AspNetCore.Routing.csproj", "src\\Http\\WebUtilities\\src\\Microsoft.AspNetCore.WebUtilities.csproj", + "src\\Http\\samples\\MinimalSample\\MinimalSample.csproj", "src\\JSInterop\\Microsoft.JSInterop\\src\\Microsoft.JSInterop.csproj", "src\\Localization\\Abstractions\\src\\Microsoft.Extensions.Localization.Abstractions.csproj", "src\\Localization\\Localization\\src\\Microsoft.Extensions.Localization.csproj", From 01e31bff21b64f02f0f935ceb2ad373c65cbd566 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 8 Jun 2021 13:30:48 -0700 Subject: [PATCH 03/24] WIP --- ...oApiExplorerServiceCollectionExtensions.cs | 12 ++++ ...ndpointMethodInfoApiDescriptionProvider.cs | 60 +++++++++++++++++-- ...icrosoft.AspNetCore.Mvc.ApiExplorer.csproj | 4 -- 3 files changed, 68 insertions(+), 8 deletions(-) create mode 100644 src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs diff --git a/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs b/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs new file mode 100644 index 000000000000..e658744fcc47 --- /dev/null +++ b/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs @@ -0,0 +1,12 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.Mvc.ApiExplorer.DependencyInjection +{ + class EndpointMethodInfoApiExplorerServiceCollectionExtensions + { + } +} diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs index 3f127d3d6e62..193aa17c7ed4 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs @@ -1,11 +1,14 @@ // 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; using System.Reflection; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Patterns; +using Microsoft.Extensions.Internal; namespace Microsoft.AspNetCore.Mvc.ApiExplorer { @@ -49,7 +52,7 @@ private static ApiDescription CreateApiDescription(RoutePattern pattern, string RelativePath = pattern.ToString(), ActionDescriptor = new ActionDescriptor { - RouteValues = new Dictionary + RouteValues = { // Swagger uses this to group endpoints together. // For now, put all endpoints configured with Map(Delegate) together. @@ -59,10 +62,59 @@ private static ApiDescription CreateApiDescription(RoutePattern pattern, string }, }; - //var returnType = actionMethodInfo.ReturnType. - //if (actionMethodInfo.) + var responseType = actionMethodInfo.ReturnType; + + if (AwaitableInfo.IsTypeAwaitable(responseType, out var awaitableInfo)) + { + responseType = awaitableInfo.ResultType; + } + + if (CreateApiResponseType(responseType) is { } apiResponseType) + { + apiDescription.SupportedResponseTypes.Add(apiResponseType); + } return apiDescription; } + + private static ApiResponseType? CreateApiResponseType(Type responseType) + { + if (typeof(IResult).IsAssignableFrom(responseType)) + { + // Can't determine anything about IResults yet. IResult could help here. + // REVIEW: Is there any value in returning an ApiResponseType with StatusCode = 200 and that's it? + return null; + } + + if (responseType == typeof(void)) + { + return new ApiResponseType + { + ModelMetadata = new EndpointMethodInfoModelMetadata(ModelMetadataIdentity.ForType(typeof(void))), + StatusCode = 200, + }; + } + + if (responseType == typeof(string)) + { + // This uses HttpResponse.WriteAsync(string) method which doesn't set a content type. It could be anything, + // but I think "text/plain" is a reasonable assumption. + + return new ApiResponseType + { + ApiResponseFormats = { new ApiResponseFormat { MediaType = "text/plain" } }, + ModelMetadata = new EndpointMethodInfoModelMetadata(ModelMetadataIdentity.ForType(typeof(string))), + StatusCode = 200, + }; + } + + // Everything else is written using HttpResponse.WriteAsJsonAsync(T). + return new ApiResponseType + { + ApiResponseFormats = { new ApiResponseFormat { MediaType = "application/json" } }, + ModelMetadata = new EndpointMethodInfoModelMetadata(ModelMetadataIdentity.ForType(responseType)), + StatusCode = 200, + }; + } } } diff --git a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj index ac27f6442e1b..a3ee0007eedc 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj +++ b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj @@ -9,10 +9,6 @@ false - - - - From 681138f77bf9e86e5a78b28257b755f2dfc5565f Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 8 Jun 2021 16:19:25 -0700 Subject: [PATCH 04/24] AddMethodInfoApiExplorerServices --- ...oApiExplorerServiceCollectionExtensions.cs | 29 +++++++++++++++---- ...ndpointMethodInfoApiDescriptionProvider.cs | 2 +- .../src/PublicAPI.Unshipped.txt | 2 ++ 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs b/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs index e658744fcc47..6dcc45bc9cdd 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs @@ -1,12 +1,29 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; +// 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.Infrastructure; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; namespace Microsoft.AspNetCore.Mvc.ApiExplorer.DependencyInjection { - class EndpointMethodInfoApiExplorerServiceCollectionExtensions + /// + /// + /// + public static class EndpointMethodInfoApiExplorerServiceCollectionExtensions { + /// + /// + /// + /// + public static void AddMethodInfoApiExplorerServices(this IServiceCollection services) + { + // Try to add default services in case MVC services aren't added. + services.TryAddSingleton(); + services.TryAddSingleton(); + + services.TryAddEnumerable( + ServiceDescriptor.Transient()); + } } } diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs index 193aa17c7ed4..acc26313ad74 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs @@ -49,7 +49,7 @@ private static ApiDescription CreateApiDescription(RoutePattern pattern, string var apiDescription = new ApiDescription { HttpMethod = httpMethod, - RelativePath = pattern.ToString(), + RelativePath = pattern.RawText?.TrimStart('/'), ActionDescriptor = new ActionDescriptor { RouteValues = diff --git a/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt index 069adc68ef65..266bf91a891e 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt @@ -22,6 +22,8 @@ Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollection.ApiDescriptio Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollection.Items.get -> System.Collections.Generic.IReadOnlyList! Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider.ApiDescriptionGroupCollectionProvider(Microsoft.AspNetCore.Mvc.Infrastructure.IActionDescriptorCollectionProvider! actionDescriptorCollectionProvider, System.Collections.Generic.IEnumerable! apiDescriptionProviders) -> void Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider.ApiDescriptionGroups.get -> Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollection! +Microsoft.AspNetCore.Mvc.ApiExplorer.DependencyInjection.EndpointMethodInfoApiExplorerServiceCollectionExtensions +static Microsoft.AspNetCore.Mvc.ApiExplorer.DependencyInjection.EndpointMethodInfoApiExplorerServiceCollectionExtensions.AddMethodInfoApiExplorerServices(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> void ~Microsoft.AspNetCore.Mvc.ApiExplorer.DefaultApiDescriptionProvider.DefaultApiDescriptionProvider(Microsoft.Extensions.Options.IOptions! optionsAccessor, Microsoft.AspNetCore.Routing.IInlineConstraintResolver! constraintResolver, Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider! modelMetadataProvider, Microsoft.AspNetCore.Mvc.Infrastructure.IActionResultTypeMapper! mapper, Microsoft.Extensions.Options.IOptions! routeOptions) -> void Microsoft.AspNetCore.Mvc.ApiExplorer.DefaultApiDescriptionProvider.OnProvidersExecuted(Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionProviderContext! context) -> void Microsoft.AspNetCore.Mvc.ApiExplorer.DefaultApiDescriptionProvider.OnProvidersExecuting(Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionProviderContext! context) -> void From c96eb6691cffe04b515e45fdcb4e108125e02f7a Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 9 Jun 2021 19:52:20 -0700 Subject: [PATCH 05/24] Start of params --- ...ndpointMethodInfoApiDescriptionProvider.cs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs index acc26313ad74..87e394bdb18b 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs @@ -5,6 +5,7 @@ using System.Reflection; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Patterns; @@ -32,6 +33,9 @@ public void OnProvidersExecuting(ApiDescriptionProviderContext context) && routeEndpoint.Metadata.GetMetadata() is { } methodInfo && routeEndpoint.Metadata.GetMetadata() is { } httpMethodMetadata) { + // REVIEW: Should we add an ApiDescription for endpoints without IHttpMethodMetadata? Swagger doesn't handle + // a null HttpMethod even though it's nullable on ApiDescription, so we'd need to define "default" HTTP methods. + // In practice, the Delegate will be called for any HTTP method if there is no IHttpMethodMetadata. foreach (var httpMethod in httpMethodMetadata.HttpMethods) { context.Results.Add(CreateApiDescription(routeEndpoint.RoutePattern, httpMethod, methodInfo)); @@ -62,6 +66,11 @@ private static ApiDescription CreateApiDescription(RoutePattern pattern, string }, }; + foreach (var parameter in actionMethodInfo.GetParameters()) + { + apiDescription.ParameterDescriptions.Add(CreateApiParameterDescription(parameter)); + } + var responseType = actionMethodInfo.ReturnType; if (AwaitableInfo.IsTypeAwaitable(responseType, out var awaitableInfo)) @@ -77,6 +86,19 @@ private static ApiDescription CreateApiDescription(RoutePattern pattern, string return apiDescription; } + private static ApiParameterDescription CreateApiParameterDescription(ParameterInfo parameterInfo) + { + var parameterType = parameterInfo.ParameterType; + + return new ApiParameterDescription + { + Name = parameterInfo.Name ?? parameterType.Name, + ModelMetadata = new EndpointMethodInfoModelMetadata(ModelMetadataIdentity.ForType(parameterType)), + Source = BindingSource.Path, + DefaultValue = parameterInfo.DefaultValue, + }; + } + private static ApiResponseType? CreateApiResponseType(Type responseType) { if (typeof(IResult).IsAssignableFrom(responseType)) From c0d3e0996460bacfeb80bc3c98f8fa7e17bcfc0d Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 10 Jun 2021 08:13:47 -0700 Subject: [PATCH 06/24] Add RequestDelegateFactoryUtilities --- ...icrosoft.AspNetCore.Http.Extensions.csproj | 3 +- .../src/RequestDelegateFactory.cs | 77 +---------------- .../src/ModelBinding/BindingSource.cs | 9 ++ .../src/PublicAPI.Unshipped.txt | 1 + ...ndpointMethodInfoApiDescriptionProvider.cs | 81 ++++++++++++++---- ...icrosoft.AspNetCore.Mvc.ApiExplorer.csproj | 4 + src/Shared/RequestDelegateFactoryUtilities.cs | 84 +++++++++++++++++++ 7 files changed, 167 insertions(+), 92 deletions(-) create mode 100644 src/Shared/RequestDelegateFactoryUtilities.cs diff --git a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj index 94a3e73732a3..a79d4d207cfc 100644 --- a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj +++ b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj @@ -12,7 +12,8 @@ - + + diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index b14ef5fb3d40..e4835a0763de 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -2,9 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Concurrent; using System.Collections.Generic; -using System.Globalization; using System.IO; using System.Linq; using System.Linq.Expressions; @@ -34,7 +32,6 @@ public static class RequestDelegateFactory private static readonly MethodInfo ResultWriteResponseAsyncMethod = typeof(RequestDelegateFactory).GetMethod(nameof(ExecuteResultWriteResponse), BindingFlags.NonPublic | BindingFlags.Static)!; private static readonly MethodInfo StringResultWriteResponseAsyncMethod = GetMethodInfo>((response, text) => HttpResponseWritingExtensions.WriteAsync(response, text, default)); private static readonly MethodInfo JsonResultWriteResponseAsyncMethod = GetMethodInfo>((response, value) => HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, default)); - private static readonly MethodInfo EnumTryParseMethod = GetEnumTryParseMethod(); private static readonly MethodInfo LogParameterBindingFailureMethod = GetMethodInfo>((httpContext, parameterType, parameterName, sourceValue) => Log.ParameterBindingFailed(httpContext, parameterType, parameterName, sourceValue)); @@ -56,8 +53,6 @@ public static class RequestDelegateFactory private static readonly BinaryExpression TempSourceStringNotNullExpr = Expression.NotEqual(TempSourceStringExpr, Expression.Constant(null)); - private static readonly ConcurrentDictionary TryParseMethodCache = new(); - /// /// Creates a implementation for . /// @@ -197,7 +192,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext { if (parameter.Name is null) { - throw new InvalidOperationException("A parameter does not have a name! Was it genererated? All parameters must be named."); + throw new InvalidOperationException("A parameter does not have a name! Was it generated? All parameters must be named."); } var parameterCustomAttributes = parameter.GetCustomAttributes(); @@ -230,7 +225,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext { return RequestAbortedExpr; } - else if (parameter.ParameterType == typeof(string) || HasTryParseMethod(parameter)) + else if (parameter.ParameterType == typeof(string) || RequestDelegateFactoryUtilities.HasTryParseMethod(parameter)) { return BindParameterFromRouteValueOrQueryString(parameter, parameter.Name, factoryContext); } @@ -477,72 +472,6 @@ private static Expression AddResponseWritingToMethodCall(Expression methodCall, }; } - private static MethodInfo GetEnumTryParseMethod() - { - var staticEnumMethods = typeof(Enum).GetMethods(BindingFlags.Public | BindingFlags.Static); - - foreach (var method in staticEnumMethods) - { - if (!method.IsGenericMethod || method.Name != "TryParse" || method.ReturnType != typeof(bool)) - { - continue; - } - - var tryParseParameters = method.GetParameters(); - - if (tryParseParameters.Length == 2 && - tryParseParameters[0].ParameterType == typeof(string) && - tryParseParameters[1].IsOut) - { - return method; - } - } - - throw new Exception("static bool System.Enum.TryParse(string? value, out TEnum result) does not exist!!?!?"); - } - - // TODO: Use InvariantCulture where possible? Or is CurrentCulture fine because it's more flexible? - private static MethodInfo? FindTryParseMethod(Type type) - { - static MethodInfo? Finder(Type type) - { - if (type.IsEnum) - { - return EnumTryParseMethod.MakeGenericMethod(type); - } - - var staticMethods = type.GetMethods(BindingFlags.Public | BindingFlags.Static); - - foreach (var method in staticMethods) - { - if (method.Name != "TryParse" || method.ReturnType != typeof(bool)) - { - continue; - } - - var tryParseParameters = method.GetParameters(); - - if (tryParseParameters.Length == 2 && - tryParseParameters[0].ParameterType == typeof(string) && - tryParseParameters[1].IsOut && - tryParseParameters[1].ParameterType == type.MakeByRefType()) - { - return method; - } - } - - return null; - } - - return TryParseMethodCache.GetOrAdd(type, Finder); - } - - private static bool HasTryParseMethod(ParameterInfo parameter) - { - var nonNullableParameterType = Nullable.GetUnderlyingType(parameter.ParameterType) ?? parameter.ParameterType; - return FindTryParseMethod(nonNullableParameterType) is not null; - } - private static Expression GetValueFromProperty(Expression sourceExpression, string key) { var itemProperty = sourceExpression.Type.GetProperty("Item"); @@ -574,7 +503,7 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres var isNotNullable = underlyingNullableType is null; var nonNullableParameterType = underlyingNullableType ?? parameter.ParameterType; - var tryParseMethod = FindTryParseMethod(nonNullableParameterType); + var tryParseMethod = RequestDelegateFactoryUtilities.FindTryParseMethod(nonNullableParameterType); if (tryParseMethod is null) { diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingSource.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingSource.cs index ea06036844ff..928a69101c09 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingSource.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingSource.cs @@ -78,6 +78,15 @@ public class BindingSource : IEquatable isGreedy: false, isFromRequest: true); + /// + /// A for the request url path or query string. + /// + public static readonly BindingSource PathOrQuery = new BindingSource( + "PathOrQuery", + "PathOrQuery", + isGreedy: false, + isFromRequest: true); + /// /// A for request services. /// diff --git a/src/Mvc/Mvc.Abstractions/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Abstractions/src/PublicAPI.Unshipped.txt index 04c7237435da..155a20ec2e25 100644 --- a/src/Mvc/Mvc.Abstractions/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.Abstractions/src/PublicAPI.Unshipped.txt @@ -102,6 +102,7 @@ abstract Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata.TemplateHint.get -> static Microsoft.AspNetCore.Mvc.Abstractions.ActionDescriptorExtensions.GetProperty(this Microsoft.AspNetCore.Mvc.Abstractions.ActionDescriptor! actionDescriptor) -> T? static Microsoft.AspNetCore.Mvc.Formatters.InputFormatterResult.Success(object? model) -> Microsoft.AspNetCore.Mvc.Formatters.InputFormatterResult! static Microsoft.AspNetCore.Mvc.Formatters.InputFormatterResult.SuccessAsync(object? model) -> System.Threading.Tasks.Task! +static readonly Microsoft.AspNetCore.Mvc.ModelBinding.BindingSource.PathOrQuery -> Microsoft.AspNetCore.Mvc.ModelBinding.BindingSource! virtual Microsoft.AspNetCore.Mvc.Filters.ActionExecutedContext.Result.get -> Microsoft.AspNetCore.Mvc.IActionResult? virtual Microsoft.AspNetCore.Mvc.Filters.ActionExecutingContext.ActionArguments.get -> System.Collections.Generic.IDictionary! virtual Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata.BoundConstructorInvoker.get -> System.Func? diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs index 87e394bdb18b..7af3d37594e8 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs @@ -2,8 +2,11 @@ // 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 System.Threading; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Metadata; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; @@ -78,6 +81,8 @@ private static ApiDescription CreateApiDescription(RoutePattern pattern, string responseType = awaitableInfo.ResultType; } + responseType = Nullable.GetUnderlyingType(responseType) ?? responseType; + if (CreateApiResponseType(responseType) is { } apiResponseType) { apiDescription.SupportedResponseTypes.Add(apiResponseType); @@ -86,19 +91,62 @@ private static ApiDescription CreateApiDescription(RoutePattern pattern, string return apiDescription; } - private static ApiParameterDescription CreateApiParameterDescription(ParameterInfo parameterInfo) + private static ApiParameterDescription CreateApiParameterDescription(ParameterInfo parameter) { - var parameterType = parameterInfo.ParameterType; + var parameterType = parameter.ParameterType; + + var (source, name) = GetBindingSourceAndName(parameter); return new ApiParameterDescription { - Name = parameterInfo.Name ?? parameterType.Name, + Name = name, ModelMetadata = new EndpointMethodInfoModelMetadata(ModelMetadataIdentity.ForType(parameterType)), - Source = BindingSource.Path, - DefaultValue = parameterInfo.DefaultValue, + Source = source, + DefaultValue = parameter.DefaultValue, }; } + // TODO: Share more of this logic with RequestDelegateFactory.CreateArgument(...) using RequestDelegateFactoryUtilities + // which is shared source. + private static (BindingSource, string) GetBindingSourceAndName(ParameterInfo parameter) + { + var attributes = parameter.GetCustomAttributes(); + + if (attributes.OfType().FirstOrDefault() is { } routeAttribute) + { + return (BindingSource.Path, routeAttribute.Name ?? parameter.Name ?? string.Empty); + } + else if (attributes.OfType().FirstOrDefault() is { } queryAttribute) + { + return (BindingSource.Query, queryAttribute.Name ?? parameter.Name ?? string.Empty); + } + else if (attributes.OfType().FirstOrDefault() is { } headerAttribute) + { + return (BindingSource.Header, headerAttribute.Name ?? parameter.Name ?? string.Empty); + } + else if (parameter.CustomAttributes.Any(a => typeof(IFromBodyMetadata).IsAssignableFrom(a.AttributeType))) + { + return (BindingSource.Body, parameter.Name ?? string.Empty); + } + else if (parameter.CustomAttributes.Any(a => typeof(IFromServiceMetadata).IsAssignableFrom(a.AttributeType)) || + parameter.ParameterType == typeof(HttpContext) || + parameter.ParameterType == typeof(CancellationToken) || + parameter.ParameterType.IsInterface) + { + return (BindingSource.Body, parameter.Name ?? string.Empty); + } + else if (parameter.ParameterType == typeof(string) || RequestDelegateFactoryUtilities.HasTryParseMethod(parameter)) + { + // TODO: Look at the pattern and infer whether it's really the path or query. + // This cannot be done by RequestDelegateFactory currently because of the layering, but could be done here. + return (BindingSource.PathOrQuery, parameter.Name ?? string.Empty); + } + else + { + return (BindingSource.Body, parameter.Name ?? string.Empty); + } + } + private static ApiResponseType? CreateApiResponseType(Type responseType) { if (typeof(IResult).IsAssignableFrom(responseType)) @@ -107,8 +155,7 @@ private static ApiParameterDescription CreateApiParameterDescription(ParameterIn // REVIEW: Is there any value in returning an ApiResponseType with StatusCode = 200 and that's it? return null; } - - if (responseType == typeof(void)) + else if (responseType == typeof(void)) { return new ApiResponseType { @@ -116,12 +163,10 @@ private static ApiParameterDescription CreateApiParameterDescription(ParameterIn StatusCode = 200, }; } - - if (responseType == typeof(string)) + else if (responseType == typeof(string)) { // This uses HttpResponse.WriteAsync(string) method which doesn't set a content type. It could be anything, // but I think "text/plain" is a reasonable assumption. - return new ApiResponseType { ApiResponseFormats = { new ApiResponseFormat { MediaType = "text/plain" } }, @@ -129,14 +174,16 @@ private static ApiParameterDescription CreateApiParameterDescription(ParameterIn StatusCode = 200, }; } - - // Everything else is written using HttpResponse.WriteAsJsonAsync(T). - return new ApiResponseType + else { - ApiResponseFormats = { new ApiResponseFormat { MediaType = "application/json" } }, - ModelMetadata = new EndpointMethodInfoModelMetadata(ModelMetadataIdentity.ForType(responseType)), - StatusCode = 200, - }; + // Everything else is written using HttpResponse.WriteAsJsonAsync(T). + return new ApiResponseType + { + ApiResponseFormats = { new ApiResponseFormat { MediaType = "application/json" } }, + ModelMetadata = new EndpointMethodInfoModelMetadata(ModelMetadataIdentity.ForType(responseType)), + StatusCode = 200, + }; + } } } } diff --git a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj index a3ee0007eedc..15dc95e039bf 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj +++ b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj @@ -9,6 +9,10 @@ false + + + + diff --git a/src/Shared/RequestDelegateFactoryUtilities.cs b/src/Shared/RequestDelegateFactoryUtilities.cs new file mode 100644 index 000000000000..619e1df84fd7 --- /dev/null +++ b/src/Shared/RequestDelegateFactoryUtilities.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.Concurrent; +using System.Reflection; + +namespace Microsoft.AspNetCore.Http +{ + // REVIEW: Better name? + internal static class RequestDelegateFactoryUtilities + { + private static readonly MethodInfo EnumTryParseMethod = GetEnumTryParseMethod(); + + // Since this is shared source the cache won't be shared between RequestDelegateFactory and the ApiDescriptionProvider sadly :( + private static readonly ConcurrentDictionary TryParseMethodCache = new(); + + public static bool HasTryParseMethod(ParameterInfo parameter) + { + var nonNullableParameterType = Nullable.GetUnderlyingType(parameter.ParameterType) ?? parameter.ParameterType; + return FindTryParseMethod(nonNullableParameterType) is not null; + } + + // TODO: Use InvariantCulture where possible? Or is CurrentCulture fine because it's more flexible? + public static MethodInfo? FindTryParseMethod(Type type) + { + static MethodInfo? Finder(Type type) + { + if (type.IsEnum) + { + return EnumTryParseMethod.MakeGenericMethod(type); + } + + var staticMethods = type.GetMethods(BindingFlags.Public | BindingFlags.Static); + + foreach (var method in staticMethods) + { + if (method.Name != "TryParse" || method.ReturnType != typeof(bool)) + { + continue; + } + + var tryParseParameters = method.GetParameters(); + + if (tryParseParameters.Length == 2 && + tryParseParameters[0].ParameterType == typeof(string) && + tryParseParameters[1].IsOut && + tryParseParameters[1].ParameterType == type.MakeByRefType()) + { + return method; + } + } + + return null; + } + + return TryParseMethodCache.GetOrAdd(type, Finder); + } + + private static MethodInfo GetEnumTryParseMethod() + { + var staticEnumMethods = typeof(Enum).GetMethods(BindingFlags.Public | BindingFlags.Static); + + foreach (var method in staticEnumMethods) + { + if (!method.IsGenericMethod || method.Name != "TryParse" || method.ReturnType != typeof(bool)) + { + continue; + } + + var tryParseParameters = method.GetParameters(); + + if (tryParseParameters.Length == 2 && + tryParseParameters[0].ParameterType == typeof(string) && + tryParseParameters[1].IsOut) + { + return method; + } + } + + throw new Exception("static bool System.Enum.TryParse(string? value, out TEnum result) does not exist!!?!?"); + } + } +} From 3f3f0a100f7f799cf682a4b5b2fa51618afd894e Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 10 Jun 2021 08:25:49 -0700 Subject: [PATCH 07/24] Remove BindingSource.PathOrQuery --- .../src/ModelBinding/BindingSource.cs | 9 -------- .../src/PublicAPI.Unshipped.txt | 1 - ...ndpointMethodInfoApiDescriptionProvider.cs | 21 ++++++++++++------- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingSource.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingSource.cs index 928a69101c09..ea06036844ff 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingSource.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingSource.cs @@ -78,15 +78,6 @@ public class BindingSource : IEquatable isGreedy: false, isFromRequest: true); - /// - /// A for the request url path or query string. - /// - public static readonly BindingSource PathOrQuery = new BindingSource( - "PathOrQuery", - "PathOrQuery", - isGreedy: false, - isFromRequest: true); - /// /// A for request services. /// diff --git a/src/Mvc/Mvc.Abstractions/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Abstractions/src/PublicAPI.Unshipped.txt index 155a20ec2e25..04c7237435da 100644 --- a/src/Mvc/Mvc.Abstractions/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.Abstractions/src/PublicAPI.Unshipped.txt @@ -102,7 +102,6 @@ abstract Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata.TemplateHint.get -> static Microsoft.AspNetCore.Mvc.Abstractions.ActionDescriptorExtensions.GetProperty(this Microsoft.AspNetCore.Mvc.Abstractions.ActionDescriptor! actionDescriptor) -> T? static Microsoft.AspNetCore.Mvc.Formatters.InputFormatterResult.Success(object? model) -> Microsoft.AspNetCore.Mvc.Formatters.InputFormatterResult! static Microsoft.AspNetCore.Mvc.Formatters.InputFormatterResult.SuccessAsync(object? model) -> System.Threading.Tasks.Task! -static readonly Microsoft.AspNetCore.Mvc.ModelBinding.BindingSource.PathOrQuery -> Microsoft.AspNetCore.Mvc.ModelBinding.BindingSource! virtual Microsoft.AspNetCore.Mvc.Filters.ActionExecutedContext.Result.get -> Microsoft.AspNetCore.Mvc.IActionResult? virtual Microsoft.AspNetCore.Mvc.Filters.ActionExecutingContext.ActionArguments.get -> System.Collections.Generic.IDictionary! virtual Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata.BoundConstructorInvoker.get -> System.Func? diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs index 7af3d37594e8..1e02cfd0b5d3 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs @@ -21,6 +21,7 @@ internal class EndpointMethodInfoApiDescriptionProvider : IApiDescriptionProvide private readonly EndpointDataSource _endpointDataSource; // Executes before MVC's DefaultApiDescriptionProvider and GrpcHttpApiDescriptionProvider + // REVIEW: Does order matter here? Should this run after MVC? public int Order => -1100; public EndpointMethodInfoApiDescriptionProvider(EndpointDataSource endpointDataSource) @@ -71,7 +72,7 @@ private static ApiDescription CreateApiDescription(RoutePattern pattern, string foreach (var parameter in actionMethodInfo.GetParameters()) { - apiDescription.ParameterDescriptions.Add(CreateApiParameterDescription(parameter)); + apiDescription.ParameterDescriptions.Add(CreateApiParameterDescription(parameter, pattern)); } var responseType = actionMethodInfo.ReturnType; @@ -91,11 +92,11 @@ private static ApiDescription CreateApiDescription(RoutePattern pattern, string return apiDescription; } - private static ApiParameterDescription CreateApiParameterDescription(ParameterInfo parameter) + private static ApiParameterDescription CreateApiParameterDescription(ParameterInfo parameter, RoutePattern pattern) { var parameterType = parameter.ParameterType; - var (source, name) = GetBindingSourceAndName(parameter); + var (source, name) = GetBindingSourceAndName(parameter, pattern); return new ApiParameterDescription { @@ -108,7 +109,7 @@ private static ApiParameterDescription CreateApiParameterDescription(ParameterIn // TODO: Share more of this logic with RequestDelegateFactory.CreateArgument(...) using RequestDelegateFactoryUtilities // which is shared source. - private static (BindingSource, string) GetBindingSourceAndName(ParameterInfo parameter) + private static (BindingSource, string) GetBindingSourceAndName(ParameterInfo parameter, RoutePattern pattern) { var attributes = parameter.GetCustomAttributes(); @@ -137,9 +138,15 @@ private static (BindingSource, string) GetBindingSourceAndName(ParameterInfo par } else if (parameter.ParameterType == typeof(string) || RequestDelegateFactoryUtilities.HasTryParseMethod(parameter)) { - // TODO: Look at the pattern and infer whether it's really the path or query. - // This cannot be done by RequestDelegateFactory currently because of the layering, but could be done here. - return (BindingSource.PathOrQuery, parameter.Name ?? string.Empty); + // Path vs query cannot be determined by RequestDelegateFactory at startup currently because of the layering, but can be done here. + if (parameter.Name is { } name && pattern.GetParameter(name) is not null) + { + return (BindingSource.Path, name); + } + else + { + return (BindingSource.Query, parameter.Name ?? string.Empty); + } } else { From 7863bd5e1ef852ff6141e21fee9c2964406a1e7c Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 10 Jun 2021 09:01:04 -0700 Subject: [PATCH 08/24] Fix handling of "FromBody" params --- ...ndpointMethodInfoApiDescriptionProvider.cs | 27 +++++++++++++++---- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs index 1e02cfd0b5d3..aa93376940b8 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs @@ -52,7 +52,7 @@ public void OnProvidersExecuted(ApiDescriptionProviderContext context) { } - private static ApiDescription CreateApiDescription(RoutePattern pattern, string httpMethod, MethodInfo actionMethodInfo) + private static ApiDescription CreateApiDescription(RoutePattern pattern, string httpMethod, MethodInfo methodInfo) { var apiDescription = new ApiDescription { @@ -65,17 +65,34 @@ private static ApiDescription CreateApiDescription(RoutePattern pattern, string // Swagger uses this to group endpoints together. // For now, put all endpoints configured with Map(Delegate) together. // TODO: Use some other metadata for this. - ["controller"] = "Map" + ["controller"] = "Map", }, }, }; - foreach (var parameter in actionMethodInfo.GetParameters()) + var hasJsonBody = false; + + foreach (var parameter in methodInfo.GetParameters()) { + var parameterDescription = CreateApiParameterDescription(parameter, pattern); + + if (parameterDescription.Source == BindingSource.Body) + { + hasJsonBody = true; + } + apiDescription.ParameterDescriptions.Add(CreateApiParameterDescription(parameter, pattern)); } - var responseType = actionMethodInfo.ReturnType; + if (hasJsonBody) + { + apiDescription.SupportedRequestFormats.Add(new ApiRequestFormat + { + MediaType = "application/json", + }); + } + + var responseType = methodInfo.ReturnType; if (AwaitableInfo.IsTypeAwaitable(responseType, out var awaitableInfo)) { @@ -134,7 +151,7 @@ private static (BindingSource, string) GetBindingSourceAndName(ParameterInfo par parameter.ParameterType == typeof(CancellationToken) || parameter.ParameterType.IsInterface) { - return (BindingSource.Body, parameter.Name ?? string.Empty); + return (BindingSource.Services, parameter.Name ?? string.Empty); } else if (parameter.ParameterType == typeof(string) || RequestDelegateFactoryUtilities.HasTryParseMethod(parameter)) { From 8354c658f40ec7c10361095080065d9bb8d1cf4c Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 10 Jun 2021 09:46:44 -0700 Subject: [PATCH 09/24] Start adding EndpointMethodInfoApiDescriptionProviderTests --- ...ndpointMethodInfoApiDescriptionProvider.cs | 2 +- ...intMethodInfoApiDescriptionProviderTest.cs | 62 +++++++++++++++++++ src/Shared/RequestDelegateFactoryUtilities.cs | 2 +- 3 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 src/Mvc/Mvc.ApiExplorer/test/EndpointMethodInfoApiDescriptionProviderTest.cs diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs index aa93376940b8..4cb646852870 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs @@ -52,7 +52,7 @@ public void OnProvidersExecuted(ApiDescriptionProviderContext context) { } - private static ApiDescription CreateApiDescription(RoutePattern pattern, string httpMethod, MethodInfo methodInfo) + internal static ApiDescription CreateApiDescription(RoutePattern pattern, string httpMethod, MethodInfo methodInfo) { var apiDescription = new ApiDescription { diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMethodInfoApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMethodInfoApiDescriptionProviderTest.cs new file mode 100644 index 000000000000..4aff3aa29d53 --- /dev/null +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMethodInfoApiDescriptionProviderTest.cs @@ -0,0 +1,62 @@ +// 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.Reflection; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.Routing.Patterns; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.ApiExplorer +{ + public class EndpointMethodInfoApiDescriptionProviderTest + { + [Fact] + public void ApiDescription_MultipleCreatedForMultipleHttpMethods() + { + Action action = () => { }; + var httpMethods = new string[] { "FOO", "BAR" }; + + var apiDescriptions = GetApiDescriptions(action.Method, httpMethods); + + Assert.Equal(httpMethods.Length, apiDescriptions.Count); + } + + [Fact] + public void ApiDescription_NotCreatedIfNoHttpMethods() + { + Action action = () => { }; + var emptyHttpMethods = new string[0]; + + var apiDescriptions = GetApiDescriptions(action.Method, emptyHttpMethods); + + Assert.Empty(apiDescriptions); + } + + private IList GetApiDescriptions( + MethodInfo methodInfo, + IEnumerable httpMethods = null, + string pattern = null) + { + var context = new ApiDescriptionProviderContext(Array.Empty()); + + var httpMethodMetadata = new HttpMethodMetadata(httpMethods ?? new[] { "GET" }); + var endpointMetadata = new EndpointMetadataCollection(methodInfo, httpMethodMetadata); + var routePattern = RoutePatternFactory.Pattern(pattern ?? "/"); + + var endpoint = new RouteEndpoint(httpContext => Task.CompletedTask, routePattern, 0, endpointMetadata, null); + var endpointDataSource = new DefaultEndpointDataSource(endpoint); + + var provider = new EndpointMethodInfoApiDescriptionProvider(endpointDataSource); + + provider.OnProvidersExecuting(context); + provider.OnProvidersExecuted(context); + + return context.Results; + } + } +} diff --git a/src/Shared/RequestDelegateFactoryUtilities.cs b/src/Shared/RequestDelegateFactoryUtilities.cs index 619e1df84fd7..d43c62d7a582 100644 --- a/src/Shared/RequestDelegateFactoryUtilities.cs +++ b/src/Shared/RequestDelegateFactoryUtilities.cs @@ -12,7 +12,7 @@ internal static class RequestDelegateFactoryUtilities { private static readonly MethodInfo EnumTryParseMethod = GetEnumTryParseMethod(); - // Since this is shared source the cache won't be shared between RequestDelegateFactory and the ApiDescriptionProvider sadly :( + // Since this is shared source, the cache won't be shared between RequestDelegateFactory and the ApiDescriptionProvider sadly :( private static readonly ConcurrentDictionary TryParseMethodCache = new(); public static bool HasTryParseMethod(ParameterInfo parameter) From 68b9b647901ecc43f4401f9e0e16ae1d1efa63de Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Thu, 10 Jun 2021 17:30:41 -0700 Subject: [PATCH 10/24] rename and cleanup --- ...oApiExplorerServiceCollectionExtensions.cs | 6 ++-- ...EndpointMetadataApiDescriptionProvider.cs} | 35 ++++++++++++------- ...elMetadata.cs => EndpointModelMetadata.cs} | 4 +-- .../src/PublicAPI.Unshipped.txt | 4 +-- ...intMethodInfoApiDescriptionProviderTest.cs | 2 +- 5 files changed, 31 insertions(+), 20 deletions(-) rename src/Mvc/Mvc.ApiExplorer/src/{EndpointMethodInfoApiDescriptionProvider.cs => EndpointMetadataApiDescriptionProvider.cs} (85%) rename src/Mvc/Mvc.ApiExplorer/src/{EndpointMethodInfoModelMetadata.cs => EndpointModelMetadata.cs} (94%) diff --git a/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs b/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs index 6dcc45bc9cdd..4faaff9a2a93 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs @@ -10,20 +10,20 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer.DependencyInjection /// /// /// - public static class EndpointMethodInfoApiExplorerServiceCollectionExtensions + public static class EndpointMetadataApiExplorerServiceCollectionExtensions { /// /// /// /// - public static void AddMethodInfoApiExplorerServices(this IServiceCollection services) + public static void AddEndpointMetadataApiExplorer(this IServiceCollection services) { // Try to add default services in case MVC services aren't added. services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddEnumerable( - ServiceDescriptor.Transient()); + ServiceDescriptor.Transient()); } } } diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs similarity index 85% rename from src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs rename to src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 4cb646852870..a4b76406e815 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -2,12 +2,14 @@ // 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.Reflection; using System.Threading; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Metadata; using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Routing; @@ -16,15 +18,16 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer { - internal class EndpointMethodInfoApiDescriptionProvider : IApiDescriptionProvider + internal class EndpointMetadataApiDescriptionProvider : + IApiDescriptionProvider, IApiResponseTypeMetadataProvider, IApiRequestFormatMetadataProvider { + // IApiResponseMetadataProvider, private readonly EndpointDataSource _endpointDataSource; - // Executes before MVC's DefaultApiDescriptionProvider and GrpcHttpApiDescriptionProvider - // REVIEW: Does order matter here? Should this run after MVC? + // Executes before MVC's DefaultApiDescriptionProvider and GrpcHttpApiDescriptionProvider for no particular reason :D public int Order => -1100; - public EndpointMethodInfoApiDescriptionProvider(EndpointDataSource endpointDataSource) + public EndpointMetadataApiDescriptionProvider(EndpointDataSource endpointDataSource) { _endpointDataSource = endpointDataSource; } @@ -52,7 +55,7 @@ public void OnProvidersExecuted(ApiDescriptionProviderContext context) { } - internal static ApiDescription CreateApiDescription(RoutePattern pattern, string httpMethod, MethodInfo methodInfo) + private static ApiDescription CreateApiDescription(RoutePattern pattern, string httpMethod, MethodInfo methodInfo) { var apiDescription = new ApiDescription { @@ -81,7 +84,7 @@ internal static ApiDescription CreateApiDescription(RoutePattern pattern, string hasJsonBody = true; } - apiDescription.ParameterDescriptions.Add(CreateApiParameterDescription(parameter, pattern)); + apiDescription.ParameterDescriptions.Add(parameterDescription); } if (hasJsonBody) @@ -111,14 +114,12 @@ internal static ApiDescription CreateApiDescription(RoutePattern pattern, string private static ApiParameterDescription CreateApiParameterDescription(ParameterInfo parameter, RoutePattern pattern) { - var parameterType = parameter.ParameterType; - var (source, name) = GetBindingSourceAndName(parameter, pattern); return new ApiParameterDescription { Name = name, - ModelMetadata = new EndpointMethodInfoModelMetadata(ModelMetadataIdentity.ForType(parameterType)), + ModelMetadata = new EndpointModelMetadata(ModelMetadataIdentity.ForType(parameter.ParameterType)), Source = source, DefaultValue = parameter.DefaultValue, }; @@ -183,7 +184,7 @@ private static (BindingSource, string) GetBindingSourceAndName(ParameterInfo par { return new ApiResponseType { - ModelMetadata = new EndpointMethodInfoModelMetadata(ModelMetadataIdentity.ForType(typeof(void))), + ModelMetadata = new EndpointModelMetadata(ModelMetadataIdentity.ForType(typeof(void))), StatusCode = 200, }; } @@ -194,7 +195,7 @@ private static (BindingSource, string) GetBindingSourceAndName(ParameterInfo par return new ApiResponseType { ApiResponseFormats = { new ApiResponseFormat { MediaType = "text/plain" } }, - ModelMetadata = new EndpointMethodInfoModelMetadata(ModelMetadataIdentity.ForType(typeof(string))), + ModelMetadata = new EndpointModelMetadata(ModelMetadataIdentity.ForType(typeof(string))), StatusCode = 200, }; } @@ -204,10 +205,20 @@ private static (BindingSource, string) GetBindingSourceAndName(ParameterInfo par return new ApiResponseType { ApiResponseFormats = { new ApiResponseFormat { MediaType = "application/json" } }, - ModelMetadata = new EndpointMethodInfoModelMetadata(ModelMetadataIdentity.ForType(responseType)), + ModelMetadata = new EndpointModelMetadata(ModelMetadataIdentity.ForType(responseType)), StatusCode = 200, }; } } + + public IReadOnlyList? GetSupportedContentTypes(string contentType, Type objectType) + { + throw new NotImplementedException(); + } + + public void SetContentTypes(MediaTypeCollection contentTypes) + { + throw new NotImplementedException(); + } } } diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoModelMetadata.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointModelMetadata.cs similarity index 94% rename from src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoModelMetadata.cs rename to src/Mvc/Mvc.ApiExplorer/src/EndpointModelMetadata.cs index 5104b53fe76f..fc5fc4267632 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMethodInfoModelMetadata.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointModelMetadata.cs @@ -10,9 +10,9 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer { - internal class EndpointMethodInfoModelMetadata : ModelMetadata + internal class EndpointModelMetadata : ModelMetadata { - public EndpointMethodInfoModelMetadata(ModelMetadataIdentity identity) : base(identity) + public EndpointModelMetadata(ModelMetadataIdentity identity) : base(identity) { IsBindingAllowed = true; } diff --git a/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt index 266bf91a891e..0baaebcbfcfc 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt @@ -22,8 +22,8 @@ Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollection.ApiDescriptio Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollection.Items.get -> System.Collections.Generic.IReadOnlyList! Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider.ApiDescriptionGroupCollectionProvider(Microsoft.AspNetCore.Mvc.Infrastructure.IActionDescriptorCollectionProvider! actionDescriptorCollectionProvider, System.Collections.Generic.IEnumerable! apiDescriptionProviders) -> void Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider.ApiDescriptionGroups.get -> Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollection! -Microsoft.AspNetCore.Mvc.ApiExplorer.DependencyInjection.EndpointMethodInfoApiExplorerServiceCollectionExtensions -static Microsoft.AspNetCore.Mvc.ApiExplorer.DependencyInjection.EndpointMethodInfoApiExplorerServiceCollectionExtensions.AddMethodInfoApiExplorerServices(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> void +Microsoft.AspNetCore.Mvc.ApiExplorer.DependencyInjection.EndpointMetadataApiExplorerServiceCollectionExtensions +static Microsoft.AspNetCore.Mvc.ApiExplorer.DependencyInjection.EndpointMetadataApiExplorerServiceCollectionExtensions.AddEndpointMetadataApiExplorer(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> void ~Microsoft.AspNetCore.Mvc.ApiExplorer.DefaultApiDescriptionProvider.DefaultApiDescriptionProvider(Microsoft.Extensions.Options.IOptions! optionsAccessor, Microsoft.AspNetCore.Routing.IInlineConstraintResolver! constraintResolver, Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider! modelMetadataProvider, Microsoft.AspNetCore.Mvc.Infrastructure.IActionResultTypeMapper! mapper, Microsoft.Extensions.Options.IOptions! routeOptions) -> void Microsoft.AspNetCore.Mvc.ApiExplorer.DefaultApiDescriptionProvider.OnProvidersExecuted(Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionProviderContext! context) -> void Microsoft.AspNetCore.Mvc.ApiExplorer.DefaultApiDescriptionProvider.OnProvidersExecuting(Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionProviderContext! context) -> void diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMethodInfoApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMethodInfoApiDescriptionProviderTest.cs index 4aff3aa29d53..b2e0cbd8f3dc 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMethodInfoApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMethodInfoApiDescriptionProviderTest.cs @@ -51,7 +51,7 @@ private IList GetApiDescriptions( var endpoint = new RouteEndpoint(httpContext => Task.CompletedTask, routePattern, 0, endpointMetadata, null); var endpointDataSource = new DefaultEndpointDataSource(endpoint); - var provider = new EndpointMethodInfoApiDescriptionProvider(endpointDataSource); + var provider = new EndpointMetadataApiDescriptionProvider(endpointDataSource); provider.OnProvidersExecuting(context); provider.OnProvidersExecuted(context); From 155f205a3ffa56b0796035e3a9f49a4ba0b2e269 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 14 Jun 2021 17:42:27 -0700 Subject: [PATCH 11/24] Support response metadata attributes --- .../src/ApiResponseTypeProvider.cs | 61 ++++++++----- .../EndpointMetadataApiDescriptionProvider.cs | 89 ++++++++++++------- 2 files changed, 93 insertions(+), 57 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs index 330ff630384d..d99e90157644 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs @@ -77,13 +77,48 @@ private ICollection GetApiResponseTypes( IReadOnlyList responseMetadataAttributes, Type? type, Type defaultErrorType) + { + var contentTypes = new MediaTypeCollection(); + + var responseTypes = ReadResponseMetadata(responseMetadataAttributes, type, defaultErrorType, contentTypes); + + // Set the default status only when no status has already been set explicitly + if (responseTypes.Count == 0 && type != null) + { + responseTypes.Add(new ApiResponseType + { + StatusCode = StatusCodes.Status200OK, + Type = type, + }); + } + + if (contentTypes.Count == 0) + { + // None of the IApiResponseMetadataProvider specified a content type. This is common for actions that + // specify one or more ProducesResponseType but no ProducesAttribute. In this case, formatters will participate in conneg + // and respond to the incoming request. + // Querying IApiResponseTypeMetadataProvider.GetSupportedContentTypes with "null" should retrieve all supported + // content types that each formatter may respond in. + contentTypes.Add((string)null!); + } + + CalculateResponseFormats(responseTypes, contentTypes); + + return responseTypes; + } + + // Shared with EndpointMetadataApiDescriptionProvider + internal static List ReadResponseMetadata( + IReadOnlyList responseMetadataAttributes, + Type? type, + Type defaultErrorType, + MediaTypeCollection contentTypes) { var results = new Dictionary(); // Get the content type that the action explicitly set to support. // Walk through all 'filter' attributes in order, and allow each one to see or override // the results of the previous ones. This is similar to the execution path for content-negotiation. - var contentTypes = new MediaTypeCollection(); if (responseMetadataAttributes != null) { foreach (var metadataAttribute in responseMetadataAttributes) @@ -129,29 +164,7 @@ private ICollection GetApiResponseTypes( } } - // Set the default status only when no status has already been set explicitly - if (results.Count == 0 && type != null) - { - results[StatusCodes.Status200OK] = new ApiResponseType - { - StatusCode = StatusCodes.Status200OK, - Type = type, - }; - } - - if (contentTypes.Count == 0) - { - // None of the IApiResponseMetadataProvider specified a content type. This is common for actions that - // specify one or more ProducesResponseType but no ProducesAttribute. In this case, formatters will participate in conneg - // and respond to the incoming request. - // Querying IApiResponseTypeMetadataProvider.GetSupportedContentTypes with "null" should retrieve all supported - // content types that each formatter may respond in. - contentTypes.Add((string)null!); - } - - var responseTypes = results.Values; - CalculateResponseFormats(responseTypes, contentTypes); - return responseTypes; + return results.Values.ToList(); } private void CalculateResponseFormats(ICollection responseTypes, MediaTypeCollection declaredContentTypes) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index a4b76406e815..d0322bb53221 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -18,8 +18,7 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer { - internal class EndpointMetadataApiDescriptionProvider : - IApiDescriptionProvider, IApiResponseTypeMetadataProvider, IApiRequestFormatMetadataProvider + internal class EndpointMetadataApiDescriptionProvider : IApiDescriptionProvider { // IApiResponseMetadataProvider, private readonly EndpointDataSource _endpointDataSource; @@ -45,7 +44,7 @@ public void OnProvidersExecuting(ApiDescriptionProviderContext context) // In practice, the Delegate will be called for any HTTP method if there is no IHttpMethodMetadata. foreach (var httpMethod in httpMethodMetadata.HttpMethods) { - context.Results.Add(CreateApiDescription(routeEndpoint.RoutePattern, httpMethod, methodInfo)); + context.Results.Add(CreateApiDescription(routeEndpoint, httpMethod, methodInfo)); } } } @@ -55,12 +54,12 @@ public void OnProvidersExecuted(ApiDescriptionProviderContext context) { } - private static ApiDescription CreateApiDescription(RoutePattern pattern, string httpMethod, MethodInfo methodInfo) + private static ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string httpMethod, MethodInfo methodInfo) { var apiDescription = new ApiDescription { HttpMethod = httpMethod, - RelativePath = pattern.RawText?.TrimStart('/'), + RelativePath = routeEndpoint.RoutePattern.RawText?.TrimStart('/'), ActionDescriptor = new ActionDescriptor { RouteValues = @@ -77,7 +76,7 @@ private static ApiDescription CreateApiDescription(RoutePattern pattern, string foreach (var parameter in methodInfo.GetParameters()) { - var parameterDescription = CreateApiParameterDescription(parameter, pattern); + var parameterDescription = CreateApiParameterDescription(parameter, routeEndpoint.RoutePattern); if (parameterDescription.Source == BindingSource.Body) { @@ -95,19 +94,7 @@ private static ApiDescription CreateApiDescription(RoutePattern pattern, string }); } - var responseType = methodInfo.ReturnType; - - if (AwaitableInfo.IsTypeAwaitable(responseType, out var awaitableInfo)) - { - responseType = awaitableInfo.ResultType; - } - - responseType = Nullable.GetUnderlyingType(responseType) ?? responseType; - - if (CreateApiResponseType(responseType) is { } apiResponseType) - { - apiDescription.SupportedResponseTypes.Add(apiResponseType); - } + AddSupportedResponseTypes(apiDescription.SupportedResponseTypes, methodInfo.ReturnType, routeEndpoint.Metadata); return apiDescription; } @@ -172,16 +159,51 @@ private static (BindingSource, string) GetBindingSourceAndName(ParameterInfo par } } - private static ApiResponseType? CreateApiResponseType(Type responseType) + private static void AddSupportedResponseTypes(IList supportedResponseTypes, Type returnType, EndpointMetadataCollection endpointMetadata) { - if (typeof(IResult).IsAssignableFrom(responseType)) + var responseType = returnType; + + if (AwaitableInfo.IsTypeAwaitable(responseType, out var awaitableInfo)) + { + responseType = awaitableInfo.ResultType; + } + + var responseMetadata = endpointMetadata.GetOrderedMetadata(); + var errorMetadata = endpointMetadata.GetMetadata(); + var defaultErrorType = errorMetadata?.Type ?? typeof(void); + var contentTypes = new MediaTypeCollection(); + + var responseMetadataTypes = ApiResponseTypeProvider.ReadResponseMetadata(responseMetadata, responseType, defaultErrorType, contentTypes); + + if (responseMetadataTypes.Count > 0) + { + foreach (var apiResponseType in responseMetadataTypes) + { + AddApiResponseFormats(apiResponseType.ApiResponseFormats, contentTypes); + supportedResponseTypes.Add(apiResponseType); + } + } + else { - // Can't determine anything about IResults yet. IResult could help here. - // REVIEW: Is there any value in returning an ApiResponseType with StatusCode = 200 and that's it? - return null; + // Set the default response type only when none has already been set explicitly with metadata. + var defaultApiResponseType = CreateDefaultApiResponseType(responseType); + + if (contentTypes.Count > 0) + { + // If metadata provided us with response formats, use that instead of the defaults. + defaultApiResponseType.ApiResponseFormats.Clear(); + AddApiResponseFormats(defaultApiResponseType.ApiResponseFormats, contentTypes); + } + + supportedResponseTypes.Add(defaultApiResponseType); } - else if (responseType == typeof(void)) + } + + private static ApiResponseType CreateDefaultApiResponseType(Type responseType) + { + if (responseType == typeof(void) || typeof(IResult).IsAssignableFrom(responseType)) { + // Can't determine anything about IResults yet that's not from extra metadata. IResult could help here. return new ApiResponseType { ModelMetadata = new EndpointModelMetadata(ModelMetadataIdentity.ForType(typeof(void))), @@ -191,7 +213,7 @@ private static (BindingSource, string) GetBindingSourceAndName(ParameterInfo par else if (responseType == typeof(string)) { // This uses HttpResponse.WriteAsync(string) method which doesn't set a content type. It could be anything, - // but I think "text/plain" is a reasonable assumption. + // but I think "text/plain" is a reasonable assumption if nothing else is specified with metadata. return new ApiResponseType { ApiResponseFormats = { new ApiResponseFormat { MediaType = "text/plain" } }, @@ -211,14 +233,15 @@ private static (BindingSource, string) GetBindingSourceAndName(ParameterInfo par } } - public IReadOnlyList? GetSupportedContentTypes(string contentType, Type objectType) - { - throw new NotImplementedException(); - } - - public void SetContentTypes(MediaTypeCollection contentTypes) + private static void AddApiResponseFormats(IList apiResponseFormats, MediaTypeCollection contentTypes) { - throw new NotImplementedException(); + foreach (var contentType in contentTypes) + { + apiResponseFormats.Add(new ApiResponseFormat + { + MediaType = contentType, + }); + } } } } From 54b224538561bd7505f13d4cc5d5ea563321950c Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 14 Jun 2021 18:33:04 -0700 Subject: [PATCH 12/24] Use all content-types --- .../src/ApiResponseTypeProvider.cs | 17 ++++- .../EndpointMetadataApiDescriptionProvider.cs | 65 ++++++++++--------- 2 files changed, 49 insertions(+), 33 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs index d99e90157644..8baa425128e9 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs @@ -140,7 +140,7 @@ internal static List ReadResponseMetadata( { // ProducesResponseTypeAttribute's constructor defaults to setting "Type" to void when no value is specified. // In this event, use the action's return type for 200 or 201 status codes. This lets you decorate an action with a - // [ProducesResponseType(201)] instead of [ProducesResponseType(201, typeof(Person)] when typeof(Person) can be inferred + // [ProducesResponseType(201)] instead of [ProducesResponseType(typeof(Person), 201] when typeof(Person) can be inferred // from the return type. apiResponseType.Type = type; } @@ -159,6 +159,7 @@ internal static List ReadResponseMetadata( if (apiResponseType.Type != null) { + AddApiResponseFormats(apiResponseType.ApiResponseFormats, contentTypes); results[apiResponseType.StatusCode] = apiResponseType; } } @@ -167,6 +168,17 @@ internal static List ReadResponseMetadata( return results.Values.ToList(); } + internal static void AddApiResponseFormats(IList apiResponseFormats, IReadOnlyList contentTypes) + { + foreach (var contentType in contentTypes) + { + apiResponseFormats.Add(new ApiResponseFormat + { + MediaType = contentType, + }); + } + } + private void CalculateResponseFormats(ICollection responseTypes, MediaTypeCollection declaredContentTypes) { var responseTypeMetadataProviders = _mvcOptions.OutputFormatters.OfType(); @@ -182,6 +194,9 @@ private void CalculateResponseFormats(ICollection responseTypes foreach (var apiResponse in responseTypes) { + // ApiResponseFormats has been preset for EndpointMetadataApiDescriptionProvider's use but MVC has more inputs to consider. + apiResponse.ApiResponseFormats.Clear(); + var responseType = apiResponse.Type; if (responseType == null || responseType == typeof(void)) { diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index d0322bb53221..be0a15a4e492 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -168,6 +168,12 @@ private static void AddSupportedResponseTypes(IList supportedRe responseType = awaitableInfo.ResultType; } + // Can't determine anything about IResults yet that's not from extra metadata. IResult could help here. + if (typeof(IResult).IsAssignableFrom(responseType)) + { + responseType = typeof(void); + } + var responseMetadata = endpointMetadata.GetOrderedMetadata(); var errorMetadata = endpointMetadata.GetMetadata(); var defaultErrorType = errorMetadata?.Type ?? typeof(void); @@ -179,7 +185,12 @@ private static void AddSupportedResponseTypes(IList supportedRe { foreach (var apiResponseType in responseMetadataTypes) { - AddApiResponseFormats(apiResponseType.ApiResponseFormats, contentTypes); + if (apiResponseType.ApiResponseFormats.Count == 0 && + CreateDefaultApiResponseFormat(responseType) is { } defaultResponseFormat) + { + apiResponseType.ApiResponseFormats.Add(defaultResponseFormat); + } + supportedResponseTypes.Add(apiResponseType); } } @@ -190,9 +201,9 @@ private static void AddSupportedResponseTypes(IList supportedRe if (contentTypes.Count > 0) { - // If metadata provided us with response formats, use that instead of the defaults. + // If metadata provided us with response formats, use that instead of the default. defaultApiResponseType.ApiResponseFormats.Clear(); - AddApiResponseFormats(defaultApiResponseType.ApiResponseFormats, contentTypes); + ApiResponseTypeProvider.AddApiResponseFormats(defaultApiResponseType.ApiResponseFormats, contentTypes); } supportedResponseTypes.Add(defaultApiResponseType); @@ -200,47 +211,37 @@ private static void AddSupportedResponseTypes(IList supportedRe } private static ApiResponseType CreateDefaultApiResponseType(Type responseType) + { + var apiResponseType = new ApiResponseType + { + ModelMetadata = new EndpointModelMetadata(ModelMetadataIdentity.ForType(responseType)), + StatusCode = 200, + }; + + if (CreateDefaultApiResponseFormat(responseType) is { } responseFormat) + { + apiResponseType.ApiResponseFormats.Add(responseFormat); + } + + return apiResponseType; + } + + private static ApiResponseFormat? CreateDefaultApiResponseFormat(Type responseType) { if (responseType == typeof(void) || typeof(IResult).IsAssignableFrom(responseType)) { - // Can't determine anything about IResults yet that's not from extra metadata. IResult could help here. - return new ApiResponseType - { - ModelMetadata = new EndpointModelMetadata(ModelMetadataIdentity.ForType(typeof(void))), - StatusCode = 200, - }; + return null; } else if (responseType == typeof(string)) { // This uses HttpResponse.WriteAsync(string) method which doesn't set a content type. It could be anything, // but I think "text/plain" is a reasonable assumption if nothing else is specified with metadata. - return new ApiResponseType - { - ApiResponseFormats = { new ApiResponseFormat { MediaType = "text/plain" } }, - ModelMetadata = new EndpointModelMetadata(ModelMetadataIdentity.ForType(typeof(string))), - StatusCode = 200, - }; + return new ApiResponseFormat { MediaType = "text/plain" }; } else { // Everything else is written using HttpResponse.WriteAsJsonAsync(T). - return new ApiResponseType - { - ApiResponseFormats = { new ApiResponseFormat { MediaType = "application/json" } }, - ModelMetadata = new EndpointModelMetadata(ModelMetadataIdentity.ForType(responseType)), - StatusCode = 200, - }; - } - } - - private static void AddApiResponseFormats(IList apiResponseFormats, MediaTypeCollection contentTypes) - { - foreach (var contentType in contentTypes) - { - apiResponseFormats.Add(new ApiResponseFormat - { - MediaType = contentType, - }); + return new ApiResponseFormat { MediaType = "application/json" }; } } } From 48ee0975a78df627af2ea85bb7468a28eb8d3a9c Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 14 Jun 2021 19:08:22 -0700 Subject: [PATCH 13/24] Set ModelMetadata when given response type for attribute --- .../src/ApiResponseTypeProvider.cs | 15 --------- .../EndpointMetadataApiDescriptionProvider.cs | 32 ++++++++++++++++--- ...ointMetadataApiDescriptionProviderTest.cs} | 2 +- 3 files changed, 28 insertions(+), 21 deletions(-) rename src/Mvc/Mvc.ApiExplorer/test/{EndpointMethodInfoApiDescriptionProviderTest.cs => EndpointMetadataApiDescriptionProviderTest.cs} (97%) diff --git a/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs index 8baa425128e9..fcf7dfcf05ab 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs @@ -159,7 +159,6 @@ internal static List ReadResponseMetadata( if (apiResponseType.Type != null) { - AddApiResponseFormats(apiResponseType.ApiResponseFormats, contentTypes); results[apiResponseType.StatusCode] = apiResponseType; } } @@ -168,17 +167,6 @@ internal static List ReadResponseMetadata( return results.Values.ToList(); } - internal static void AddApiResponseFormats(IList apiResponseFormats, IReadOnlyList contentTypes) - { - foreach (var contentType in contentTypes) - { - apiResponseFormats.Add(new ApiResponseFormat - { - MediaType = contentType, - }); - } - } - private void CalculateResponseFormats(ICollection responseTypes, MediaTypeCollection declaredContentTypes) { var responseTypeMetadataProviders = _mvcOptions.OutputFormatters.OfType(); @@ -194,9 +182,6 @@ private void CalculateResponseFormats(ICollection responseTypes foreach (var apiResponse in responseTypes) { - // ApiResponseFormats has been preset for EndpointMetadataApiDescriptionProvider's use but MVC has more inputs to consider. - apiResponse.ApiResponseFormats.Clear(); - var responseType = apiResponse.Type; if (responseType == null || responseType == typeof(void)) { diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index be0a15a4e492..d0b83b315792 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Linq; using System.Reflection; using System.Threading; @@ -106,7 +107,7 @@ private static ApiParameterDescription CreateApiParameterDescription(ParameterIn return new ApiParameterDescription { Name = name, - ModelMetadata = new EndpointModelMetadata(ModelMetadataIdentity.ForType(parameter.ParameterType)), + ModelMetadata = CreateModelMetadata(parameter.ParameterType), Source = source, DefaultValue = parameter.DefaultValue, }; @@ -185,8 +186,15 @@ private static void AddSupportedResponseTypes(IList supportedRe { foreach (var apiResponseType in responseMetadataTypes) { - if (apiResponseType.ApiResponseFormats.Count == 0 && - CreateDefaultApiResponseFormat(responseType) is { } defaultResponseFormat) + Debug.Assert(apiResponseType.Type is not null, "ApiResponseTypeProvider gave us a null Type!?"); + + apiResponseType.ModelMetadata = CreateModelMetadata(apiResponseType.Type); + + if (contentTypes.Count > 0) + { + AddResponseContentTypes(apiResponseType.ApiResponseFormats, contentTypes); + } + else if (CreateDefaultApiResponseFormat(responseType) is { } defaultResponseFormat) { apiResponseType.ApiResponseFormats.Add(defaultResponseFormat); } @@ -203,7 +211,7 @@ private static void AddSupportedResponseTypes(IList supportedRe { // If metadata provided us with response formats, use that instead of the default. defaultApiResponseType.ApiResponseFormats.Clear(); - ApiResponseTypeProvider.AddApiResponseFormats(defaultApiResponseType.ApiResponseFormats, contentTypes); + AddResponseContentTypes(defaultApiResponseType.ApiResponseFormats, contentTypes); } supportedResponseTypes.Add(defaultApiResponseType); @@ -214,7 +222,7 @@ private static ApiResponseType CreateDefaultApiResponseType(Type responseType) { var apiResponseType = new ApiResponseType { - ModelMetadata = new EndpointModelMetadata(ModelMetadataIdentity.ForType(responseType)), + ModelMetadata = CreateModelMetadata(responseType), StatusCode = 200, }; @@ -244,5 +252,19 @@ private static ApiResponseType CreateDefaultApiResponseType(Type responseType) return new ApiResponseFormat { MediaType = "application/json" }; } } + + private static EndpointModelMetadata CreateModelMetadata(Type type) => + new EndpointModelMetadata(ModelMetadataIdentity.ForType(type)); + + private static void AddResponseContentTypes(IList apiResponseFormats, IReadOnlyList contentTypes) + { + foreach (var contentType in contentTypes) + { + apiResponseFormats.Add(new ApiResponseFormat + { + MediaType = contentType, + }); + } + } } } diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMethodInfoApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs similarity index 97% rename from src/Mvc/Mvc.ApiExplorer/test/EndpointMethodInfoApiDescriptionProviderTest.cs rename to src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index b2e0cbd8f3dc..5d9d7299c563 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMethodInfoApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -13,7 +13,7 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer { - public class EndpointMethodInfoApiDescriptionProviderTest + public class EndpointMetadataApiDescriptionProviderTest { [Fact] public void ApiDescription_MultipleCreatedForMultipleHttpMethods() From de645fef5c5687c58b4f3a9d8e92b81e58e31f7f Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 15 Jun 2021 10:24:52 -0700 Subject: [PATCH 14/24] AddSupportedRequestFormats --- .../src/DefaultApiDescriptionProvider.cs | 2 +- .../EndpointMetadataApiDescriptionProvider.cs | 44 ++++++++++++++----- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/DefaultApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/DefaultApiDescriptionProvider.cs index 843216b21a9b..ea388cdfbb36 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/DefaultApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/DefaultApiDescriptionProvider.cs @@ -429,7 +429,7 @@ private IReadOnlyList GetSupportedFormats(MediaTypeCollection return results; } - private static MediaTypeCollection GetDeclaredContentTypes(IApiRequestMetadataProvider[]? requestMetadataAttributes) + internal static MediaTypeCollection GetDeclaredContentTypes(IReadOnlyList? requestMetadataAttributes) { // Walk through all 'filter' attributes in order, and allow each one to see or override // the results of the previous ones. This is similar to the execution path for content-negotiation. diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index d0b83b315792..22ffdf5ca1ee 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -87,14 +87,7 @@ private static ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, apiDescription.ParameterDescriptions.Add(parameterDescription); } - if (hasJsonBody) - { - apiDescription.SupportedRequestFormats.Add(new ApiRequestFormat - { - MediaType = "application/json", - }); - } - + AddSupportedRequestFormats(apiDescription.SupportedRequestFormats, hasJsonBody, routeEndpoint.Metadata); AddSupportedResponseTypes(apiDescription.SupportedResponseTypes, methodInfo.ReturnType, routeEndpoint.Metadata); return apiDescription; @@ -160,7 +153,35 @@ private static (BindingSource, string) GetBindingSourceAndName(ParameterInfo par } } - private static void AddSupportedResponseTypes(IList supportedResponseTypes, Type returnType, EndpointMetadataCollection endpointMetadata) + private static void AddSupportedRequestFormats( + IList supportedRequestFormats, + bool hasJsonBody, + EndpointMetadataCollection endpointMetadata) + { + // If RequestDelegateFactory thinks the API supports a JSON body, it does. + if (hasJsonBody) + { + supportedRequestFormats.Add(new ApiRequestFormat + { + MediaType = "application/json", + }); + } + + var requestMetadata = endpointMetadata.GetOrderedMetadata(); + + foreach (var contentType in DefaultApiDescriptionProvider.GetDeclaredContentTypes(requestMetadata)) + { + supportedRequestFormats.Add(new ApiRequestFormat + { + MediaType = contentType, + }); + } + } + + private static void AddSupportedResponseTypes( + IList supportedResponseTypes, + Type returnType, + EndpointMetadataCollection endpointMetadata) { var responseType = returnType; @@ -180,7 +201,8 @@ private static void AddSupportedResponseTypes(IList supportedRe var defaultErrorType = errorMetadata?.Type ?? typeof(void); var contentTypes = new MediaTypeCollection(); - var responseMetadataTypes = ApiResponseTypeProvider.ReadResponseMetadata(responseMetadata, responseType, defaultErrorType, contentTypes); + var responseMetadataTypes = ApiResponseTypeProvider.ReadResponseMetadata( + responseMetadata, responseType, defaultErrorType, contentTypes); if (responseMetadataTypes.Count > 0) { @@ -236,7 +258,7 @@ private static ApiResponseType CreateDefaultApiResponseType(Type responseType) private static ApiResponseFormat? CreateDefaultApiResponseFormat(Type responseType) { - if (responseType == typeof(void) || typeof(IResult).IsAssignableFrom(responseType)) + if (responseType == typeof(void)) { return null; } From 8a5913e6bf6a86fa475e76b2060cf4e3a9ba205c Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 15 Jun 2021 11:22:30 -0700 Subject: [PATCH 15/24] Use non-compiler-generated declaring types for "controller" name --- .../EndpointMetadataApiDescriptionProvider.cs | 28 +++++++++++++++---- ...pointMetadataApiDescriptionProviderTest.cs | 26 +++++++++++++++++ 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 22ffdf5ca1ee..150eaf202ec6 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -6,6 +6,7 @@ using System.Diagnostics; using System.Linq; using System.Reflection; +using System.Runtime.CompilerServices; using System.Threading; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Metadata; @@ -21,7 +22,6 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer { internal class EndpointMetadataApiDescriptionProvider : IApiDescriptionProvider { - // IApiResponseMetadataProvider, private readonly EndpointDataSource _endpointDataSource; // Executes before MVC's DefaultApiDescriptionProvider and GrpcHttpApiDescriptionProvider for no particular reason :D @@ -57,6 +57,21 @@ public void OnProvidersExecuted(ApiDescriptionProviderContext context) private static ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string httpMethod, MethodInfo methodInfo) { + // Swagger uses the "controller" name to group endpoints together. + // For now, put all methods defined the same declaring type together. + string controllerName; + + if (methodInfo.DeclaringType is not null && !IsCompilerGenerated(methodInfo.DeclaringType)) + { + controllerName = methodInfo.DeclaringType.Name; + } + else + { + // If the declaring type is null or compiler-generated (e.g. lambdas), + // group the methods under a "Map" controller. + controllerName = "Map"; + } + var apiDescription = new ApiDescription { HttpMethod = httpMethod, @@ -65,10 +80,7 @@ private static ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, { RouteValues = { - // Swagger uses this to group endpoints together. - // For now, put all endpoints configured with Map(Delegate) together. - // TODO: Use some other metadata for this. - ["controller"] = "Map", + ["controller"] = controllerName, }, }, }; @@ -288,5 +300,11 @@ private static void AddResponseContentTypes(IList apiResponse }); } } + + // The CompilerGeneratedAttribute doesn't always get added so we also check if the type name starts with "<" + // For example,w "<>c" is a "declaring" type the C# compiler will generate without the attribute for a top-level lambda + // REVIEW: Is there a better way to do this? + private static bool IsCompilerGenerated(Type type) => + Attribute.IsDefined(type, typeof(CompilerGeneratedAttribute)) || type.Name.StartsWith('<'); } } diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 5d9d7299c563..4916237f7472 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -37,6 +37,28 @@ public void ApiDescription_NotCreatedIfNoHttpMethods() Assert.Empty(apiDescriptions); } + + [Fact] + public void ApiDescription_UsesDeclaringTypeAsControllerName() + { + var apiDescriptions = GetApiDescriptions(((Action)TestAction).Method); + + var apiDescription = Assert.Single(apiDescriptions); + var declaringTypeName = typeof(EndpointMetadataApiDescriptionProviderTest).Name; + Assert.Equal(declaringTypeName, apiDescription.ActionDescriptor.RouteValues["controller"]); + } + + [Fact] + public void ApiDescription_UsesMapAsControllerNameIfNoDeclaringType() + { + Action action = () => { }; + + var apiDescriptions = GetApiDescriptions(action.Method); + + var apiDescription = Assert.Single(apiDescriptions); + Assert.Equal("Map", apiDescription.ActionDescriptor.RouteValues["controller"]); + } + private IList GetApiDescriptions( MethodInfo methodInfo, IEnumerable httpMethods = null, @@ -58,5 +80,9 @@ private IList GetApiDescriptions( return context.Results; } + + private static void TestAction() + { + } } } From 25bb72f5c60dbddf9d03538ced11673e2e7235fe Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 15 Jun 2021 12:59:28 -0700 Subject: [PATCH 16/24] Add request/response format tests --- .../EndpointMetadataApiDescriptionProvider.cs | 23 ++-- ...pointMetadataApiDescriptionProviderTest.cs | 124 +++++++++++++++--- ...oft.AspNetCore.Mvc.ApiExplorer.Test.csproj | 1 + 3 files changed, 118 insertions(+), 30 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 150eaf202ec6..08dcaeb4f2bb 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -170,22 +170,24 @@ private static void AddSupportedRequestFormats( bool hasJsonBody, EndpointMetadataCollection endpointMetadata) { - // If RequestDelegateFactory thinks the API supports a JSON body, it does. - if (hasJsonBody) + var requestMetadata = endpointMetadata.GetOrderedMetadata(); + var declaredContentTypes = DefaultApiDescriptionProvider.GetDeclaredContentTypes(requestMetadata); + + if (declaredContentTypes.Count > 0) { - supportedRequestFormats.Add(new ApiRequestFormat + foreach (var contentType in declaredContentTypes) { - MediaType = "application/json", - }); + supportedRequestFormats.Add(new ApiRequestFormat + { + MediaType = contentType, + }); + } } - - var requestMetadata = endpointMetadata.GetOrderedMetadata(); - - foreach (var contentType in DefaultApiDescriptionProvider.GetDeclaredContentTypes(requestMetadata)) + else if (hasJsonBody) { supportedRequestFormats.Add(new ApiRequestFormat { - MediaType = contentType, + MediaType = "application/json", }); } } @@ -258,6 +260,7 @@ private static ApiResponseType CreateDefaultApiResponseType(Type responseType) { ModelMetadata = CreateModelMetadata(responseType), StatusCode = 200, + Type = responseType, }; if (CreateDefaultApiResponseFormat(responseType) is { } responseFormat) diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 4916237f7472..47b0a1b617fb 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -18,32 +18,24 @@ public class EndpointMetadataApiDescriptionProviderTest [Fact] public void ApiDescription_MultipleCreatedForMultipleHttpMethods() { - Action action = () => { }; - var httpMethods = new string[] { "FOO", "BAR" }; + var apiDescriptions = GetApiDescriptions(() => { }, "/", new string[] { "FOO", "BAR" }); - var apiDescriptions = GetApiDescriptions(action.Method, httpMethods); - - Assert.Equal(httpMethods.Length, apiDescriptions.Count); + Assert.Equal(2, apiDescriptions.Count); } [Fact] public void ApiDescription_NotCreatedIfNoHttpMethods() { - Action action = () => { }; - var emptyHttpMethods = new string[0]; - - var apiDescriptions = GetApiDescriptions(action.Method, emptyHttpMethods); + var apiDescriptions = GetApiDescriptions(() => { }, "/", Array.Empty()); Assert.Empty(apiDescriptions); } - [Fact] public void ApiDescription_UsesDeclaringTypeAsControllerName() { - var apiDescriptions = GetApiDescriptions(((Action)TestAction).Method); + var apiDescription = GetApiDescription(TestAction); - var apiDescription = Assert.Single(apiDescriptions); var declaringTypeName = typeof(EndpointMetadataApiDescriptionProviderTest).Name; Assert.Equal(declaringTypeName, apiDescription.ActionDescriptor.RouteValues["controller"]); } @@ -51,23 +43,104 @@ public void ApiDescription_UsesDeclaringTypeAsControllerName() [Fact] public void ApiDescription_UsesMapAsControllerNameIfNoDeclaringType() { - Action action = () => { }; + var apiDescription = GetApiDescription(() => { }); - var apiDescriptions = GetApiDescriptions(action.Method); - - var apiDescription = Assert.Single(apiDescriptions); Assert.Equal("Map", apiDescription.ActionDescriptor.RouteValues["controller"]); } + [Fact] + public void ApiDescription_AddsJsonRequestFormatWhenFromBodyInferred() + { + static void AssertApiDescriptionHasJsonRequestFormat(ApiDescription apiDescription) + { + var requestFormat = Assert.Single(apiDescription.SupportedRequestFormats); + Assert.Equal("application/json", requestFormat.MediaType); + Assert.Null(requestFormat.Formatter); + } + + AssertApiDescriptionHasJsonRequestFormat(GetApiDescription( + (InferredJsonType fromBody) => { })); + + AssertApiDescriptionHasJsonRequestFormat(GetApiDescription(( + [FromBody] int fromBody) => { })); + } + + [Fact] + public void ApiDescription_UsesMetadadataInsteadOfDefaultJsonRequestFormat() + { + static void AssertApiDescriptionHasCustomRequestFormat(ApiDescription apiDescription) + { + var requestFormat = Assert.Single(apiDescription.SupportedRequestFormats); + Assert.Equal("application/custom", requestFormat.MediaType); + Assert.Null(requestFormat.Formatter); + } + + AssertApiDescriptionHasCustomRequestFormat(GetApiDescription( + [Consumes("application/custom")] + (InferredJsonType fromBody) => { })); + + AssertApiDescriptionHasCustomRequestFormat(GetApiDescription( + [Consumes("application/custom")] + ([FromBody] int fromBody) => { })); + } + + [Fact] + public void ApiDescription_AddsJsonResponseFormatWhenFromBodyInferred() + { + var apiDescription = GetApiDescription(() => new InferredJsonType()); + + var responseType = Assert.Single(apiDescription.SupportedResponseTypes); + Assert.Equal(typeof(InferredJsonType), responseType.Type); + Assert.Equal(typeof(InferredJsonType), responseType.ModelMetadata.ModelType); + + var responseFormat = Assert.Single(responseType.ApiResponseFormats); + Assert.Equal("application/json", responseFormat.MediaType); + Assert.Null(responseFormat.Formatter); + } + + [Fact] + public void ApiDescription_AddsTextResponseFormatWhenFromBodyInferred() + { + var apiDescription = GetApiDescription(() => "foo"); + + var responseType = Assert.Single(apiDescription.SupportedResponseTypes); + Assert.Equal(typeof(string), responseType.Type); + Assert.Equal(typeof(string), responseType.ModelMetadata.ModelType); + + var responseFormat = Assert.Single(responseType.ApiResponseFormats); + Assert.Equal("text/plain", responseFormat.MediaType); + Assert.Null(responseFormat.Formatter); + } + + [Fact] + public void ApiDescription_AddsNoResponseFormatWhenItCannotBeInferredAndTheresNoMetadata() + { + static void AssertApiDescriptionIsVoid(ApiDescription apiDescription) + { + var responseType = Assert.Single(apiDescription.SupportedResponseTypes); + Assert.Equal(typeof(void), responseType.Type); + Assert.Equal(typeof(void), responseType.ModelMetadata.ModelType); + + Assert.Empty(responseType.ApiResponseFormats); + } + + AssertApiDescriptionIsVoid(GetApiDescription(() => { })); + AssertApiDescriptionIsVoid(GetApiDescription(() => Task.CompletedTask)); + AssertApiDescriptionIsVoid(GetApiDescription(() => new ValueTask())); + } + private IList GetApiDescriptions( - MethodInfo methodInfo, - IEnumerable httpMethods = null, - string pattern = null) + Delegate action, + string pattern = null, + IEnumerable httpMethods = null) { + var methodInfo = action.Method; + var attributes = methodInfo.GetCustomAttributes(); var context = new ApiDescriptionProviderContext(Array.Empty()); var httpMethodMetadata = new HttpMethodMetadata(httpMethods ?? new[] { "GET" }); - var endpointMetadata = new EndpointMetadataCollection(methodInfo, httpMethodMetadata); + var metadataItems = new List(attributes) { methodInfo, httpMethodMetadata }; + var endpointMetadata = new EndpointMetadataCollection(metadataItems.ToArray()); var routePattern = RoutePatternFactory.Pattern(pattern ?? "/"); var endpoint = new RouteEndpoint(httpContext => Task.CompletedTask, routePattern, 0, endpointMetadata, null); @@ -81,8 +154,19 @@ private IList GetApiDescriptions( return context.Results; } + private ApiDescription GetApiDescription(Delegate action, string pattern = null) => + Assert.Single(GetApiDescriptions(action, pattern)); + private static void TestAction() { } + + private class InferredJsonType + { + } + + private interface IInferredServiceType + { + } } } diff --git a/src/Mvc/Mvc.ApiExplorer/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test.csproj b/src/Mvc/Mvc.ApiExplorer/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test.csproj index 3f1a6d737463..dce16986fb56 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test.csproj +++ b/src/Mvc/Mvc.ApiExplorer/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test.csproj @@ -2,6 +2,7 @@ $(DefaultNetCoreTargetFramework) + Preview From 842b18a8cf7bc54bcdd03b22796e45127ca96a75 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 15 Jun 2021 14:48:26 -0700 Subject: [PATCH 17/24] Add BindingSource tests --- .../EndpointMetadataApiDescriptionProvider.cs | 1 + ...pointMetadataApiDescriptionProviderTest.cs | 152 +++++++++++++++--- 2 files changed, 132 insertions(+), 21 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 08dcaeb4f2bb..b8402c3264a0 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -115,6 +115,7 @@ private static ApiParameterDescription CreateApiParameterDescription(ParameterIn ModelMetadata = CreateModelMetadata(parameter.ParameterType), Source = source, DefaultValue = parameter.DefaultValue, + Type = parameter.ParameterType, }; } diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 47b0a1b617fb..bf51a95e4f8d 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -4,9 +4,11 @@ using System; using System.Collections.Generic; using System.Reflection; +using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Patterns; using Xunit; @@ -16,7 +18,7 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer public class EndpointMetadataApiDescriptionProviderTest { [Fact] - public void ApiDescription_MultipleCreatedForMultipleHttpMethods() + public void MultipleApiDescriptionsCreatedForMultipleHttpMethods() { var apiDescriptions = GetApiDescriptions(() => { }, "/", new string[] { "FOO", "BAR" }); @@ -24,7 +26,7 @@ public void ApiDescription_MultipleCreatedForMultipleHttpMethods() } [Fact] - public void ApiDescription_NotCreatedIfNoHttpMethods() + public void ApiDescriptionNotCreatedIfNoHttpMethods() { var apiDescriptions = GetApiDescriptions(() => { }, "/", Array.Empty()); @@ -32,7 +34,7 @@ public void ApiDescription_NotCreatedIfNoHttpMethods() } [Fact] - public void ApiDescription_UsesDeclaringTypeAsControllerName() + public void UsesDeclaringTypeAsControllerName() { var apiDescription = GetApiDescription(TestAction); @@ -41,7 +43,7 @@ public void ApiDescription_UsesDeclaringTypeAsControllerName() } [Fact] - public void ApiDescription_UsesMapAsControllerNameIfNoDeclaringType() + public void UsesMapAsControllerNameIfNoDeclaringType() { var apiDescription = GetApiDescription(() => { }); @@ -49,43 +51,43 @@ public void ApiDescription_UsesMapAsControllerNameIfNoDeclaringType() } [Fact] - public void ApiDescription_AddsJsonRequestFormatWhenFromBodyInferred() + public void AddsJsonRequestFormatWhenFromBodyInferred() { - static void AssertApiDescriptionHasJsonRequestFormat(ApiDescription apiDescription) + static void AssertJsonRequestFormat(ApiDescription apiDescription) { var requestFormat = Assert.Single(apiDescription.SupportedRequestFormats); Assert.Equal("application/json", requestFormat.MediaType); Assert.Null(requestFormat.Formatter); } - AssertApiDescriptionHasJsonRequestFormat(GetApiDescription( + AssertJsonRequestFormat(GetApiDescription( (InferredJsonType fromBody) => { })); - AssertApiDescriptionHasJsonRequestFormat(GetApiDescription(( - [FromBody] int fromBody) => { })); + AssertJsonRequestFormat(GetApiDescription( + ([FromBody] int fromBody) => { })); } [Fact] - public void ApiDescription_UsesMetadadataInsteadOfDefaultJsonRequestFormat() + public void UsesMetadadataInsteadOfDefaultJsonRequestFormat() { - static void AssertApiDescriptionHasCustomRequestFormat(ApiDescription apiDescription) + static void AssertustomRequestFormat(ApiDescription apiDescription) { var requestFormat = Assert.Single(apiDescription.SupportedRequestFormats); Assert.Equal("application/custom", requestFormat.MediaType); Assert.Null(requestFormat.Formatter); } - AssertApiDescriptionHasCustomRequestFormat(GetApiDescription( + AssertustomRequestFormat(GetApiDescription( [Consumes("application/custom")] (InferredJsonType fromBody) => { })); - AssertApiDescriptionHasCustomRequestFormat(GetApiDescription( + AssertustomRequestFormat(GetApiDescription( [Consumes("application/custom")] ([FromBody] int fromBody) => { })); } [Fact] - public void ApiDescription_AddsJsonResponseFormatWhenFromBodyInferred() + public void AddsJsonResponseFormatWhenFromBodyInferred() { var apiDescription = GetApiDescription(() => new InferredJsonType()); @@ -99,7 +101,7 @@ public void ApiDescription_AddsJsonResponseFormatWhenFromBodyInferred() } [Fact] - public void ApiDescription_AddsTextResponseFormatWhenFromBodyInferred() + public void AddsTextResponseFormatWhenFromBodyInferred() { var apiDescription = GetApiDescription(() => "foo"); @@ -113,9 +115,9 @@ public void ApiDescription_AddsTextResponseFormatWhenFromBodyInferred() } [Fact] - public void ApiDescription_AddsNoResponseFormatWhenItCannotBeInferredAndTheresNoMetadata() + public void AddsNoResponseFormatWhenItCannotBeInferredAndTheresNoMetadata() { - static void AssertApiDescriptionIsVoid(ApiDescription apiDescription) + static void AssertVoid(ApiDescription apiDescription) { var responseType = Assert.Single(apiDescription.SupportedResponseTypes); Assert.Equal(typeof(void), responseType.Type); @@ -124,9 +126,113 @@ static void AssertApiDescriptionIsVoid(ApiDescription apiDescription) Assert.Empty(responseType.ApiResponseFormats); } - AssertApiDescriptionIsVoid(GetApiDescription(() => { })); - AssertApiDescriptionIsVoid(GetApiDescription(() => Task.CompletedTask)); - AssertApiDescriptionIsVoid(GetApiDescription(() => new ValueTask())); + AssertVoid(GetApiDescription(() => { })); + AssertVoid(GetApiDescription(() => Task.CompletedTask)); + AssertVoid(GetApiDescription(() => new ValueTask())); + } + + [Fact] + public void AddsFromRouteParameterAsPath() + { + static void AssertPathParameter(ApiDescription apiDescription) + { + var param = Assert.Single(apiDescription.ParameterDescriptions); + Assert.Equal(typeof(int), param.Type); + Assert.Equal(typeof(int), param.ModelMetadata.ModelType); + Assert.Equal(BindingSource.Path, param.Source); + } + + AssertPathParameter(GetApiDescription((int foo) => { }, "/{foo}")); + AssertPathParameter(GetApiDescription(([FromRoute] int foo) => { })); + } + + [Fact] + public void AddsFromQueryParameterAsQuery() + { + static void AssertQueryParameter(ApiDescription apiDescription) + { + var param = Assert.Single(apiDescription.ParameterDescriptions); + Assert.Equal(typeof(int), param.Type); + Assert.Equal(typeof(int), param.ModelMetadata.ModelType); + Assert.Equal(BindingSource.Query, param.Source); + } + + AssertQueryParameter(GetApiDescription((int foo) => { }, "/")); + AssertQueryParameter(GetApiDescription(([FromQuery] int foo) => { })); + } + + [Fact] + public void AddsFromHeaderParameterAsHeader() + { + var apiDescription = GetApiDescription(([FromHeader] int foo) => { }); + var param = Assert.Single(apiDescription.ParameterDescriptions); + + Assert.Equal(typeof(int), param.Type); + Assert.Equal(typeof(int), param.ModelMetadata.ModelType); + Assert.Equal(BindingSource.Header, param.Source); + } + + [Fact] + public void AddsFromServiceParameterAsService() + { + static void AssertServiceParameter(ApiDescription apiDescription, Type expectedType) + { + var param = Assert.Single(apiDescription.ParameterDescriptions); + Assert.Equal(expectedType, param.Type); + Assert.Equal(expectedType, param.ModelMetadata.ModelType); + Assert.Equal(BindingSource.Services, param.Source); + } + + AssertServiceParameter(GetApiDescription((IInferredServiceType foo) => { }), typeof(IInferredServiceType)); + AssertServiceParameter(GetApiDescription(([FromServices] int foo) => { }), typeof(int)); + AssertServiceParameter(GetApiDescription((HttpContext context) => { }), typeof(HttpContext)); + AssertServiceParameter(GetApiDescription((CancellationToken token) => { }), typeof(CancellationToken)); + } + + [Fact] + public void AddsFromBodyParameterAsBody() + { + static void AssertBodyParameter(ApiDescription apiDescription, Type expectedType) + { + var param = Assert.Single(apiDescription.ParameterDescriptions); + Assert.Equal(expectedType, param.Type); + Assert.Equal(expectedType, param.ModelMetadata.ModelType); + Assert.Equal(BindingSource.Body, param.Source); + } + + AssertBodyParameter(GetApiDescription((InferredJsonType foo) => { }), typeof(InferredJsonType)); + AssertBodyParameter(GetApiDescription(([FromBody] int foo) => { }), typeof(int)); + } + + [Fact] + public void AddsDefaultValueFromParameters() + { + var apiDescription = GetApiDescription(TestActionWithDefaultValue); + + var param = Assert.Single(apiDescription.ParameterDescriptions); + Assert.Equal(42, param.DefaultValue); + } + + [Fact] + public void AddsMultipleParameters() + { + var apiDescription = GetApiDescription(([FromRoute] int foo, int bar, InferredJsonType fromBody) => { }); + Assert.Equal(3, apiDescription.ParameterDescriptions.Count); + + var fooParam = apiDescription.ParameterDescriptions[0]; + Assert.Equal(typeof(int), fooParam.Type); + Assert.Equal(typeof(int), fooParam.ModelMetadata.ModelType); + Assert.Equal(BindingSource.Path, fooParam.Source); + + var barParam = apiDescription.ParameterDescriptions[1]; + Assert.Equal(typeof(int), barParam.Type); + Assert.Equal(typeof(int), barParam.ModelMetadata.ModelType); + Assert.Equal(BindingSource.Query, barParam.Source); + + var fromBodyParam = apiDescription.ParameterDescriptions[2]; + Assert.Equal(typeof(InferredJsonType), fromBodyParam.Type); + Assert.Equal(typeof(InferredJsonType), fromBodyParam.ModelMetadata.ModelType); + Assert.Equal(BindingSource.Body, fromBodyParam.Source); } private IList GetApiDescriptions( @@ -141,7 +247,7 @@ private IList GetApiDescriptions( var httpMethodMetadata = new HttpMethodMetadata(httpMethods ?? new[] { "GET" }); var metadataItems = new List(attributes) { methodInfo, httpMethodMetadata }; var endpointMetadata = new EndpointMetadataCollection(metadataItems.ToArray()); - var routePattern = RoutePatternFactory.Pattern(pattern ?? "/"); + var routePattern = RoutePatternFactory.Parse(pattern ?? "/"); var endpoint = new RouteEndpoint(httpContext => Task.CompletedTask, routePattern, 0, endpointMetadata, null); var endpointDataSource = new DefaultEndpointDataSource(endpoint); @@ -161,6 +267,10 @@ private static void TestAction() { } + private static void TestActionWithDefaultValue(int foo = 42) + { + } + private class InferredJsonType { } From c09aaba7cb4e48d281649c5f9ae7074aae490b60 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 15 Jun 2021 17:05:24 -0700 Subject: [PATCH 18/24] Add more response/request format tests --- .../EndpointMetadataApiDescriptionProvider.cs | 6 ++ ...pointMetadataApiDescriptionProviderTest.cs | 70 ++++++++++++++++++- 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index b8402c3264a0..53f72ff94a5f 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -225,6 +225,12 @@ private static void AddSupportedResponseTypes( { Debug.Assert(apiResponseType.Type is not null, "ApiResponseTypeProvider gave us a null Type!?"); + // void means no response type was specified by the metadata, so use whatever we inferred. + if (apiResponseType.Type == typeof(void)) + { + apiResponseType.Type = responseType; + } + apiResponseType.ModelMetadata = CreateModelMetadata(apiResponseType.Type); if (contentTypes.Count > 0) diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index bf51a95e4f8d..8bf30ec27a28 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -68,7 +68,7 @@ static void AssertJsonRequestFormat(ApiDescription apiDescription) } [Fact] - public void UsesMetadadataInsteadOfDefaultJsonRequestFormat() + public void AddsRequestFormatFromMetadata() { static void AssertustomRequestFormat(ApiDescription apiDescription) { @@ -86,12 +86,31 @@ static void AssertustomRequestFormat(ApiDescription apiDescription) ([FromBody] int fromBody) => { })); } + [Fact] + public void AddsMultipleRequestFormatsFromMetadata() + { + var apiDescription = GetApiDescription( + [Consumes("application/custom0", "application/custom1")] + (InferredJsonType fromBody) => { }); + + Assert.Equal(2, apiDescription.SupportedRequestFormats.Count); + + var requestFormat0 = apiDescription.SupportedRequestFormats[0]; + Assert.Equal("application/custom0", requestFormat0.MediaType); + Assert.Null(requestFormat0.Formatter); + + var requestFormat1 = apiDescription.SupportedRequestFormats[1]; + Assert.Equal("application/custom1", requestFormat1.MediaType); + Assert.Null(requestFormat1.Formatter); + } + [Fact] public void AddsJsonResponseFormatWhenFromBodyInferred() { var apiDescription = GetApiDescription(() => new InferredJsonType()); var responseType = Assert.Single(apiDescription.SupportedResponseTypes); + Assert.Equal(200, responseType.StatusCode); Assert.Equal(typeof(InferredJsonType), responseType.Type); Assert.Equal(typeof(InferredJsonType), responseType.ModelMetadata.ModelType); @@ -106,6 +125,7 @@ public void AddsTextResponseFormatWhenFromBodyInferred() var apiDescription = GetApiDescription(() => "foo"); var responseType = Assert.Single(apiDescription.SupportedResponseTypes); + Assert.Equal(200, responseType.StatusCode); Assert.Equal(typeof(string), responseType.Type); Assert.Equal(typeof(string), responseType.ModelMetadata.ModelType); @@ -120,6 +140,7 @@ public void AddsNoResponseFormatWhenItCannotBeInferredAndTheresNoMetadata() static void AssertVoid(ApiDescription apiDescription) { var responseType = Assert.Single(apiDescription.SupportedResponseTypes); + Assert.Equal(200, responseType.StatusCode); Assert.Equal(typeof(void), responseType.Type); Assert.Equal(typeof(void), responseType.ModelMetadata.ModelType); @@ -131,6 +152,53 @@ static void AssertVoid(ApiDescription apiDescription) AssertVoid(GetApiDescription(() => new ValueTask())); } + [Fact] + public void AddsResponseFormatFromMetadata() + { + var apiDescription = GetApiDescription( + [ProducesResponseType(typeof(TimeSpan), StatusCodes.Status201Created)] + [Produces("application/custom")] + () => new InferredJsonType()); + + var responseType = Assert.Single(apiDescription.SupportedResponseTypes); + + Assert.Equal(201, responseType.StatusCode); + Assert.Equal(typeof(TimeSpan), responseType.Type); + Assert.Equal(typeof(TimeSpan), responseType.ModelMetadata.ModelType); + + var responseFormat = Assert.Single(responseType.ApiResponseFormats); + Assert.Equal("application/custom", responseFormat.MediaType); + } + + [Fact] + public void AddsMultipleResponseFormatsFromMetadata() + { + var apiDescription = GetApiDescription( + [ProducesResponseType(typeof(TimeSpan), StatusCodes.Status201Created)] + [ProducesResponseType(StatusCodes.Status400BadRequest)] + () => new InferredJsonType()); + + Assert.Equal(2, apiDescription.SupportedResponseTypes.Count); + + var createdResponseType = apiDescription.SupportedResponseTypes[0]; + + Assert.Equal(201, createdResponseType.StatusCode); + Assert.Equal(typeof(TimeSpan), createdResponseType.Type); + Assert.Equal(typeof(TimeSpan), createdResponseType.ModelMetadata.ModelType); + + var createdResponseFormat = Assert.Single(createdResponseType.ApiResponseFormats); + Assert.Equal("application/json", createdResponseFormat.MediaType); + + var badRequestResponseType = apiDescription.SupportedResponseTypes[1]; + + Assert.Equal(400, badRequestResponseType.StatusCode); + Assert.Equal(typeof(InferredJsonType), badRequestResponseType.Type); + Assert.Equal(typeof(InferredJsonType), badRequestResponseType.ModelMetadata.ModelType); + + var badRequestResponseFormat = Assert.Single(badRequestResponseType.ApiResponseFormats); + Assert.Equal("application/json", badRequestResponseFormat.MediaType); + } + [Fact] public void AddsFromRouteParameterAsPath() { From 9491d49083066c2a632e0f78fd775ab7b2ffb6fd Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 15 Jun 2021 17:42:31 -0700 Subject: [PATCH 19/24] Use IServiceProviderIsService in EndpointMetadataApiDescriptionProvider --- .../EndpointMetadataApiDescriptionProvider.cs | 22 ++++--- ...pointMetadataApiDescriptionProviderTest.cs | 60 ++++++++++++------- 2 files changed, 51 insertions(+), 31 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 53f72ff94a5f..ae67834146d3 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; @@ -16,6 +15,7 @@ using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Patterns; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Internal; namespace Microsoft.AspNetCore.Mvc.ApiExplorer @@ -23,13 +23,20 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer internal class EndpointMetadataApiDescriptionProvider : IApiDescriptionProvider { private readonly EndpointDataSource _endpointDataSource; + private readonly IServiceProviderIsService? _serviceProviderIsService; // Executes before MVC's DefaultApiDescriptionProvider and GrpcHttpApiDescriptionProvider for no particular reason :D public int Order => -1100; public EndpointMetadataApiDescriptionProvider(EndpointDataSource endpointDataSource) + : this(endpointDataSource, null) + { + } + + public EndpointMetadataApiDescriptionProvider(EndpointDataSource endpointDataSource, IServiceProviderIsService? serviceProviderIsService) { _endpointDataSource = endpointDataSource; + _serviceProviderIsService = serviceProviderIsService; } public void OnProvidersExecuting(ApiDescriptionProviderContext context) @@ -55,7 +62,7 @@ public void OnProvidersExecuted(ApiDescriptionProviderContext context) { } - private static ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string httpMethod, MethodInfo methodInfo) + private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string httpMethod, MethodInfo methodInfo) { // Swagger uses the "controller" name to group endpoints together. // For now, put all methods defined the same declaring type together. @@ -105,7 +112,7 @@ private static ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, return apiDescription; } - private static ApiParameterDescription CreateApiParameterDescription(ParameterInfo parameter, RoutePattern pattern) + private ApiParameterDescription CreateApiParameterDescription(ParameterInfo parameter, RoutePattern pattern) { var (source, name) = GetBindingSourceAndName(parameter, pattern); @@ -121,7 +128,7 @@ private static ApiParameterDescription CreateApiParameterDescription(ParameterIn // TODO: Share more of this logic with RequestDelegateFactory.CreateArgument(...) using RequestDelegateFactoryUtilities // which is shared source. - private static (BindingSource, string) GetBindingSourceAndName(ParameterInfo parameter, RoutePattern pattern) + private (BindingSource, string) GetBindingSourceAndName(ParameterInfo parameter, RoutePattern pattern) { var attributes = parameter.GetCustomAttributes(); @@ -144,7 +151,7 @@ private static (BindingSource, string) GetBindingSourceAndName(ParameterInfo par else if (parameter.CustomAttributes.Any(a => typeof(IFromServiceMetadata).IsAssignableFrom(a.AttributeType)) || parameter.ParameterType == typeof(HttpContext) || parameter.ParameterType == typeof(CancellationToken) || - parameter.ParameterType.IsInterface) + _serviceProviderIsService?.IsService(parameter.ParameterType) == true) { return (BindingSource.Services, parameter.Name ?? string.Empty); } @@ -223,10 +230,9 @@ private static void AddSupportedResponseTypes( { foreach (var apiResponseType in responseMetadataTypes) { - Debug.Assert(apiResponseType.Type is not null, "ApiResponseTypeProvider gave us a null Type!?"); - // void means no response type was specified by the metadata, so use whatever we inferred. - if (apiResponseType.Type == typeof(void)) + // ApiResponseTypeProvider should never return ApiResponseTypes with null Type, but it doesn't hurt to check. + if (apiResponseType.Type is null || apiResponseType.Type == typeof(void)) { apiResponseType.Type = responseType; } diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 8bf30ec27a28..62299cd0fb95 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Patterns; +using Microsoft.Extensions.DependencyInjection; using Xunit; namespace Microsoft.AspNetCore.Mvc.ApiExplorer @@ -61,7 +62,7 @@ static void AssertJsonRequestFormat(ApiDescription apiDescription) } AssertJsonRequestFormat(GetApiDescription( - (InferredJsonType fromBody) => { })); + (InferredJsonClass fromBody) => { })); AssertJsonRequestFormat(GetApiDescription( ([FromBody] int fromBody) => { })); @@ -79,7 +80,7 @@ static void AssertustomRequestFormat(ApiDescription apiDescription) AssertustomRequestFormat(GetApiDescription( [Consumes("application/custom")] - (InferredJsonType fromBody) => { })); + (InferredJsonClass fromBody) => { })); AssertustomRequestFormat(GetApiDescription( [Consumes("application/custom")] @@ -91,7 +92,7 @@ public void AddsMultipleRequestFormatsFromMetadata() { var apiDescription = GetApiDescription( [Consumes("application/custom0", "application/custom1")] - (InferredJsonType fromBody) => { }); + (InferredJsonClass fromBody) => { }); Assert.Equal(2, apiDescription.SupportedRequestFormats.Count); @@ -107,16 +108,20 @@ public void AddsMultipleRequestFormatsFromMetadata() [Fact] public void AddsJsonResponseFormatWhenFromBodyInferred() { - var apiDescription = GetApiDescription(() => new InferredJsonType()); + static void AssertJsonResponse(ApiDescription apiDescription, Type expectedType) + { + var responseType = Assert.Single(apiDescription.SupportedResponseTypes); + Assert.Equal(200, responseType.StatusCode); + Assert.Equal(expectedType, responseType.Type); + Assert.Equal(expectedType, responseType.ModelMetadata.ModelType); - var responseType = Assert.Single(apiDescription.SupportedResponseTypes); - Assert.Equal(200, responseType.StatusCode); - Assert.Equal(typeof(InferredJsonType), responseType.Type); - Assert.Equal(typeof(InferredJsonType), responseType.ModelMetadata.ModelType); + var responseFormat = Assert.Single(responseType.ApiResponseFormats); + Assert.Equal("application/json", responseFormat.MediaType); + Assert.Null(responseFormat.Formatter); + } - var responseFormat = Assert.Single(responseType.ApiResponseFormats); - Assert.Equal("application/json", responseFormat.MediaType); - Assert.Null(responseFormat.Formatter); + AssertJsonResponse(GetApiDescription(() => new InferredJsonClass()), typeof(InferredJsonClass)); + AssertJsonResponse(GetApiDescription(() => (IInferredJsonInterface)null), typeof(IInferredJsonInterface)); } [Fact] @@ -158,7 +163,7 @@ public void AddsResponseFormatFromMetadata() var apiDescription = GetApiDescription( [ProducesResponseType(typeof(TimeSpan), StatusCodes.Status201Created)] [Produces("application/custom")] - () => new InferredJsonType()); + () => new InferredJsonClass()); var responseType = Assert.Single(apiDescription.SupportedResponseTypes); @@ -176,7 +181,7 @@ public void AddsMultipleResponseFormatsFromMetadata() var apiDescription = GetApiDescription( [ProducesResponseType(typeof(TimeSpan), StatusCodes.Status201Created)] [ProducesResponseType(StatusCodes.Status400BadRequest)] - () => new InferredJsonType()); + () => new InferredJsonClass()); Assert.Equal(2, apiDescription.SupportedResponseTypes.Count); @@ -192,8 +197,8 @@ public void AddsMultipleResponseFormatsFromMetadata() var badRequestResponseType = apiDescription.SupportedResponseTypes[1]; Assert.Equal(400, badRequestResponseType.StatusCode); - Assert.Equal(typeof(InferredJsonType), badRequestResponseType.Type); - Assert.Equal(typeof(InferredJsonType), badRequestResponseType.ModelMetadata.ModelType); + Assert.Equal(typeof(InferredJsonClass), badRequestResponseType.Type); + Assert.Equal(typeof(InferredJsonClass), badRequestResponseType.ModelMetadata.ModelType); var badRequestResponseFormat = Assert.Single(badRequestResponseType.ApiResponseFormats); Assert.Equal("application/json", badRequestResponseFormat.MediaType); @@ -251,7 +256,7 @@ static void AssertServiceParameter(ApiDescription apiDescription, Type expectedT Assert.Equal(BindingSource.Services, param.Source); } - AssertServiceParameter(GetApiDescription((IInferredServiceType foo) => { }), typeof(IInferredServiceType)); + AssertServiceParameter(GetApiDescription((IInferredServiceInterface foo) => { }), typeof(IInferredServiceInterface)); AssertServiceParameter(GetApiDescription(([FromServices] int foo) => { }), typeof(int)); AssertServiceParameter(GetApiDescription((HttpContext context) => { }), typeof(HttpContext)); AssertServiceParameter(GetApiDescription((CancellationToken token) => { }), typeof(CancellationToken)); @@ -268,7 +273,7 @@ static void AssertBodyParameter(ApiDescription apiDescription, Type expectedType Assert.Equal(BindingSource.Body, param.Source); } - AssertBodyParameter(GetApiDescription((InferredJsonType foo) => { }), typeof(InferredJsonType)); + AssertBodyParameter(GetApiDescription((InferredJsonClass foo) => { }), typeof(InferredJsonClass)); AssertBodyParameter(GetApiDescription(([FromBody] int foo) => { }), typeof(int)); } @@ -284,7 +289,7 @@ public void AddsDefaultValueFromParameters() [Fact] public void AddsMultipleParameters() { - var apiDescription = GetApiDescription(([FromRoute] int foo, int bar, InferredJsonType fromBody) => { }); + var apiDescription = GetApiDescription(([FromRoute] int foo, int bar, InferredJsonClass fromBody) => { }); Assert.Equal(3, apiDescription.ParameterDescriptions.Count); var fooParam = apiDescription.ParameterDescriptions[0]; @@ -298,8 +303,8 @@ public void AddsMultipleParameters() Assert.Equal(BindingSource.Query, barParam.Source); var fromBodyParam = apiDescription.ParameterDescriptions[2]; - Assert.Equal(typeof(InferredJsonType), fromBodyParam.Type); - Assert.Equal(typeof(InferredJsonType), fromBodyParam.ModelMetadata.ModelType); + Assert.Equal(typeof(InferredJsonClass), fromBodyParam.Type); + Assert.Equal(typeof(InferredJsonClass), fromBodyParam.ModelMetadata.ModelType); Assert.Equal(BindingSource.Body, fromBodyParam.Source); } @@ -320,7 +325,7 @@ private IList GetApiDescriptions( var endpoint = new RouteEndpoint(httpContext => Task.CompletedTask, routePattern, 0, endpointMetadata, null); var endpointDataSource = new DefaultEndpointDataSource(endpoint); - var provider = new EndpointMetadataApiDescriptionProvider(endpointDataSource); + var provider = new EndpointMetadataApiDescriptionProvider(endpointDataSource, new ServiceProviderIsService()); provider.OnProvidersExecuting(context); provider.OnProvidersExecuted(context); @@ -339,12 +344,21 @@ private static void TestActionWithDefaultValue(int foo = 42) { } - private class InferredJsonType + private class InferredJsonClass + { + } + + private interface IInferredServiceInterface + { + } + + private interface IInferredJsonInterface { } - private interface IInferredServiceType + private class ServiceProviderIsService : IServiceProviderIsService { + public bool IsService(Type serviceType) => serviceType is IInferredServiceInterface; } } } From 1520e4cb3f50d1ad443501c2385b5ac19758f437 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 15 Jun 2021 18:18:26 -0700 Subject: [PATCH 20/24] cleanup --- .../src/EndpointMetadataApiDescriptionProvider.cs | 8 ++++---- .../test/EndpointMetadataApiDescriptionProviderTest.cs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index ae67834146d3..ac483743ee83 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -43,9 +43,9 @@ public void OnProvidersExecuting(ApiDescriptionProviderContext context) { foreach (var endpoint in _endpointDataSource.Endpoints) { - if (endpoint is RouteEndpoint routeEndpoint - && routeEndpoint.Metadata.GetMetadata() is { } methodInfo - && routeEndpoint.Metadata.GetMetadata() is { } httpMethodMetadata) + if (endpoint is RouteEndpoint routeEndpoint && + routeEndpoint.Metadata.GetMetadata() is { } methodInfo && + routeEndpoint.Metadata.GetMetadata() is { } httpMethodMetadata) { // REVIEW: Should we add an ApiDescription for endpoints without IHttpMethodMetadata? Swagger doesn't handle // a null HttpMethod even though it's nullable on ApiDescription, so we'd need to define "default" HTTP methods. @@ -64,7 +64,7 @@ public void OnProvidersExecuted(ApiDescriptionProviderContext context) private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string httpMethod, MethodInfo methodInfo) { - // Swagger uses the "controller" name to group endpoints together. + // Swashbuckle uses the "controller" name to group endpoints together. // For now, put all methods defined the same declaring type together. string controllerName; diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 62299cd0fb95..949895872dbe 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -358,7 +358,7 @@ private interface IInferredJsonInterface private class ServiceProviderIsService : IServiceProviderIsService { - public bool IsService(Type serviceType) => serviceType is IInferredServiceInterface; + public bool IsService(Type serviceType) => serviceType == typeof(IInferredServiceInterface); } } } From 03ee0a579f9fd4a08f864f921bb061162034eb9f Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 16 Jun 2021 12:02:35 -0700 Subject: [PATCH 21/24] Rename AddEndpointsApiExplorer and fix namespace --- ...hodInfoApiExplorerServiceCollectionExtensions.cs | 13 +++++++------ src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs b/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs index 4faaff9a2a93..5d635ac5428e 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs @@ -1,22 +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 Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.ApiExplorer; using Microsoft.AspNetCore.Mvc.Infrastructure; -using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; -namespace Microsoft.AspNetCore.Mvc.ApiExplorer.DependencyInjection +namespace Microsoft.Extensions.DependencyInjection { /// - /// + /// Extensions for configuring ApiExplorer using . /// public static class EndpointMetadataApiExplorerServiceCollectionExtensions { /// - /// + /// Configures ApiExplorer using . /// - /// - public static void AddEndpointMetadataApiExplorer(this IServiceCollection services) + /// The . + public static void AddEndpointsApiExplorer(this IServiceCollection services) { // Try to add default services in case MVC services aren't added. services.TryAddSingleton(); diff --git a/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt index 0baaebcbfcfc..a14472ae6b96 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt @@ -22,8 +22,8 @@ Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollection.ApiDescriptio Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollection.Items.get -> System.Collections.Generic.IReadOnlyList! Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider.ApiDescriptionGroupCollectionProvider(Microsoft.AspNetCore.Mvc.Infrastructure.IActionDescriptorCollectionProvider! actionDescriptorCollectionProvider, System.Collections.Generic.IEnumerable! apiDescriptionProviders) -> void Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider.ApiDescriptionGroups.get -> Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollection! -Microsoft.AspNetCore.Mvc.ApiExplorer.DependencyInjection.EndpointMetadataApiExplorerServiceCollectionExtensions -static Microsoft.AspNetCore.Mvc.ApiExplorer.DependencyInjection.EndpointMetadataApiExplorerServiceCollectionExtensions.AddEndpointMetadataApiExplorer(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> void +Microsoft.Extensions.DependencyInjection.EndpointMetadataApiExplorerServiceCollectionExtensions +static Microsoft.Extensions.DependencyInjection.EndpointMetadataApiExplorerServiceCollectionExtensions.AddEndpointsApiExplorer(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> void ~Microsoft.AspNetCore.Mvc.ApiExplorer.DefaultApiDescriptionProvider.DefaultApiDescriptionProvider(Microsoft.Extensions.Options.IOptions! optionsAccessor, Microsoft.AspNetCore.Routing.IInlineConstraintResolver! constraintResolver, Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider! modelMetadataProvider, Microsoft.AspNetCore.Mvc.Infrastructure.IActionResultTypeMapper! mapper, Microsoft.Extensions.Options.IOptions! routeOptions) -> void Microsoft.AspNetCore.Mvc.ApiExplorer.DefaultApiDescriptionProvider.OnProvidersExecuted(Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionProviderContext! context) -> void Microsoft.AspNetCore.Mvc.ApiExplorer.DefaultApiDescriptionProvider.OnProvidersExecuting(Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionProviderContext! context) -> void From 1b53e2ebf229bf4108e1e5de55f127fb41b9df29 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 16 Jun 2021 12:34:28 -0700 Subject: [PATCH 22/24] fix tests --- .../test/RequestDelegateFactoryTests.cs | 2 +- .../MinimalActionEndpointRouteBuilderExtensionsTest.cs | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index bac3c83201dd..8ecbae2c39a6 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -479,7 +479,7 @@ public void CreateThrowsInvalidOperationExceptionGivenUnnamedArgument() var unnamedParameter = Expression.Parameter(typeof(int)); var lambda = Expression.Lambda(Expression.Block(), unnamedParameter); var ex = Assert.Throws(() => RequestDelegateFactory.Create((Action)lambda.Compile(), new EmptyServiceProvdier())); - Assert.Equal("A parameter does not have a name! Was it genererated? All parameters must be named.", ex.Message); + Assert.Equal("A parameter does not have a name! Was it generated? All parameters must be named.", ex.Message); } [Fact] diff --git a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs index fec8da0ce7cc..f43d9d97d55d 100644 --- a/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs @@ -42,7 +42,9 @@ void TestAction() var dataSource = Assert.Single(builder.DataSources); var endpoint = Assert.Single(dataSource.Endpoints); - var metadataArray = endpoint.Metadata.Where(m => m is not CompilerGeneratedAttribute).ToArray(); + var metadataArray = endpoint.Metadata.OfType().ToArray(); + + static string GetMethod(IHttpMethodMetadata metadata) => Assert.Single(metadata.HttpMethods); Assert.Equal(3, metadataArray.Length); Assert.Equal("ATTRIBUTE", GetMethod(metadataArray[0])); @@ -50,12 +52,6 @@ void TestAction() Assert.Equal("BUILDER", GetMethod(metadataArray[2])); Assert.Equal("BUILDER", endpoint.Metadata.GetMetadata()!.HttpMethods.Single()); - - string GetMethod(object metadata) - { - var httpMethodMetadata = Assert.IsAssignableFrom(metadata); - return Assert.Single(httpMethodMetadata.HttpMethods); - } } [Fact] From 3368d23891746dde8a026021c0d793f57b16303e Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Wed, 16 Jun 2021 12:45:19 -0700 Subject: [PATCH 23/24] Use ApplicationName instead of "Map" if no declaring type --- .../EndpointMetadataApiDescriptionProvider.cs | 14 +++++++++---- ...pointMetadataApiDescriptionProviderTest.cs | 20 ++++++++++++++++--- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index ac483743ee83..5e0710a13c3f 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -16,6 +16,7 @@ using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Internal; namespace Microsoft.AspNetCore.Mvc.ApiExplorer @@ -23,19 +24,24 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer internal class EndpointMetadataApiDescriptionProvider : IApiDescriptionProvider { private readonly EndpointDataSource _endpointDataSource; + private readonly IHostEnvironment _environment; private readonly IServiceProviderIsService? _serviceProviderIsService; // Executes before MVC's DefaultApiDescriptionProvider and GrpcHttpApiDescriptionProvider for no particular reason :D public int Order => -1100; - public EndpointMetadataApiDescriptionProvider(EndpointDataSource endpointDataSource) - : this(endpointDataSource, null) + public EndpointMetadataApiDescriptionProvider(EndpointDataSource endpointDataSource, IHostEnvironment environment) + : this(endpointDataSource, environment, null) { } - public EndpointMetadataApiDescriptionProvider(EndpointDataSource endpointDataSource, IServiceProviderIsService? serviceProviderIsService) + public EndpointMetadataApiDescriptionProvider( + EndpointDataSource endpointDataSource, + IHostEnvironment environment, + IServiceProviderIsService? serviceProviderIsService) { _endpointDataSource = endpointDataSource; + _environment = environment; _serviceProviderIsService = serviceProviderIsService; } @@ -76,7 +82,7 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string { // If the declaring type is null or compiler-generated (e.g. lambdas), // group the methods under a "Map" controller. - controllerName = "Map"; + controllerName = _environment.ApplicationName; } var apiDescription = new ApiDescription diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 949895872dbe..cd39f51ca3b0 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -12,6 +12,8 @@ using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.FileProviders; +using Microsoft.Extensions.Hosting; using Xunit; namespace Microsoft.AspNetCore.Mvc.ApiExplorer @@ -44,11 +46,11 @@ public void UsesDeclaringTypeAsControllerName() } [Fact] - public void UsesMapAsControllerNameIfNoDeclaringType() + public void UsesApplicationNameAsControllerNameIfNoDeclaringType() { var apiDescription = GetApiDescription(() => { }); - Assert.Equal("Map", apiDescription.ActionDescriptor.RouteValues["controller"]); + Assert.Equal(nameof(EndpointMetadataApiDescriptionProviderTest), apiDescription.ActionDescriptor.RouteValues["controller"]); } [Fact] @@ -324,8 +326,12 @@ private IList GetApiDescriptions( var endpoint = new RouteEndpoint(httpContext => Task.CompletedTask, routePattern, 0, endpointMetadata, null); var endpointDataSource = new DefaultEndpointDataSource(endpoint); + var hostEnvironment = new HostEnvironment + { + ApplicationName = nameof(EndpointMetadataApiDescriptionProviderTest) + }; - var provider = new EndpointMetadataApiDescriptionProvider(endpointDataSource, new ServiceProviderIsService()); + var provider = new EndpointMetadataApiDescriptionProvider(endpointDataSource, hostEnvironment, new ServiceProviderIsService()); provider.OnProvidersExecuting(context); provider.OnProvidersExecuted(context); @@ -360,5 +366,13 @@ private class ServiceProviderIsService : IServiceProviderIsService { public bool IsService(Type serviceType) => serviceType == typeof(IInferredServiceInterface); } + + private class HostEnvironment : IHostEnvironment + { + public string EnvironmentName { get; set; } + public string ApplicationName { get; set; } + public string ContentRootPath { get; set; } + public IFileProvider ContentRootFileProvider { get; set; } + } } } From dc80513d1854f1adb334e3f83000dabcf3ab69ca Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Fri, 18 Jun 2021 11:14:29 -0700 Subject: [PATCH 24/24] address pr feedback --- .../src/Microsoft.AspNetCore.Http.Extensions.csproj | 2 +- .../Http.Extensions/src/RequestDelegateFactory.cs | 4 ++-- ...hodInfoApiExplorerServiceCollectionExtensions.cs | 4 +++- .../src/EndpointMetadataApiDescriptionProvider.cs | 6 +++--- .../src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj | 2 +- src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt | 2 +- ...teFactoryUtilities.cs => TryParseMethodCache.cs} | 13 +++++++------ 7 files changed, 18 insertions(+), 15 deletions(-) rename src/Shared/{RequestDelegateFactoryUtilities.cs => TryParseMethodCache.cs} (86%) diff --git a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj index a79d4d207cfc..b9961e96de26 100644 --- a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj +++ b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj @@ -12,7 +12,7 @@ - + diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index e4835a0763de..91f5660e9051 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -225,7 +225,7 @@ private static Expression CreateArgument(ParameterInfo parameter, FactoryContext { return RequestAbortedExpr; } - else if (parameter.ParameterType == typeof(string) || RequestDelegateFactoryUtilities.HasTryParseMethod(parameter)) + else if (parameter.ParameterType == typeof(string) || TryParseMethodCache.HasTryParseMethod(parameter)) { return BindParameterFromRouteValueOrQueryString(parameter, parameter.Name, factoryContext); } @@ -503,7 +503,7 @@ private static Expression BindParameterFromValue(ParameterInfo parameter, Expres var isNotNullable = underlyingNullableType is null; var nonNullableParameterType = underlyingNullableType ?? parameter.ParameterType; - var tryParseMethod = RequestDelegateFactoryUtilities.FindTryParseMethod(nonNullableParameterType); + var tryParseMethod = TryParseMethodCache.FindTryParseMethod(nonNullableParameterType); if (tryParseMethod is null) { diff --git a/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs b/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs index 5d635ac5428e..0be4f4c89410 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs @@ -17,7 +17,7 @@ public static class EndpointMetadataApiExplorerServiceCollectionExtensions /// Configures ApiExplorer using . /// /// The . - public static void AddEndpointsApiExplorer(this IServiceCollection services) + public static IServiceCollection AddEndpointsApiExplorer(this IServiceCollection services) { // Try to add default services in case MVC services aren't added. services.TryAddSingleton(); @@ -25,6 +25,8 @@ public static void AddEndpointsApiExplorer(this IServiceCollection services) services.TryAddEnumerable( ServiceDescriptor.Transient()); + + return services; } } } diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 5e0710a13c3f..e4bef2e33fa7 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -27,7 +27,7 @@ internal class EndpointMetadataApiDescriptionProvider : IApiDescriptionProvider private readonly IHostEnvironment _environment; private readonly IServiceProviderIsService? _serviceProviderIsService; - // Executes before MVC's DefaultApiDescriptionProvider and GrpcHttpApiDescriptionProvider for no particular reason :D + // Executes before MVC's DefaultApiDescriptionProvider and GrpcHttpApiDescriptionProvider for no particular reason. public int Order => -1100; public EndpointMetadataApiDescriptionProvider(EndpointDataSource endpointDataSource, IHostEnvironment environment) @@ -81,7 +81,7 @@ private ApiDescription CreateApiDescription(RouteEndpoint routeEndpoint, string else { // If the declaring type is null or compiler-generated (e.g. lambdas), - // group the methods under a "Map" controller. + // group the methods under the application name. controllerName = _environment.ApplicationName; } @@ -161,7 +161,7 @@ private ApiParameterDescription CreateApiParameterDescription(ParameterInfo para { return (BindingSource.Services, parameter.Name ?? string.Empty); } - else if (parameter.ParameterType == typeof(string) || RequestDelegateFactoryUtilities.HasTryParseMethod(parameter)) + else if (parameter.ParameterType == typeof(string) || TryParseMethodCache.HasTryParseMethod(parameter)) { // Path vs query cannot be determined by RequestDelegateFactory at startup currently because of the layering, but can be done here. if (parameter.Name is { } name && pattern.GetParameter(name) is not null) diff --git a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj index 15dc95e039bf..4b86355d1087 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj +++ b/src/Mvc/Mvc.ApiExplorer/src/Microsoft.AspNetCore.Mvc.ApiExplorer.csproj @@ -10,7 +10,7 @@ - + diff --git a/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt index a14472ae6b96..e06fd7f7a795 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt @@ -23,7 +23,7 @@ Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollection.Items.get -> Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider.ApiDescriptionGroupCollectionProvider(Microsoft.AspNetCore.Mvc.Infrastructure.IActionDescriptorCollectionProvider! actionDescriptorCollectionProvider, System.Collections.Generic.IEnumerable! apiDescriptionProviders) -> void Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollectionProvider.ApiDescriptionGroups.get -> Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionGroupCollection! Microsoft.Extensions.DependencyInjection.EndpointMetadataApiExplorerServiceCollectionExtensions -static Microsoft.Extensions.DependencyInjection.EndpointMetadataApiExplorerServiceCollectionExtensions.AddEndpointsApiExplorer(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> void +static Microsoft.Extensions.DependencyInjection.EndpointMetadataApiExplorerServiceCollectionExtensions.AddEndpointsApiExplorer(this Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection! ~Microsoft.AspNetCore.Mvc.ApiExplorer.DefaultApiDescriptionProvider.DefaultApiDescriptionProvider(Microsoft.Extensions.Options.IOptions! optionsAccessor, Microsoft.AspNetCore.Routing.IInlineConstraintResolver! constraintResolver, Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider! modelMetadataProvider, Microsoft.AspNetCore.Mvc.Infrastructure.IActionResultTypeMapper! mapper, Microsoft.Extensions.Options.IOptions! routeOptions) -> void Microsoft.AspNetCore.Mvc.ApiExplorer.DefaultApiDescriptionProvider.OnProvidersExecuted(Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionProviderContext! context) -> void Microsoft.AspNetCore.Mvc.ApiExplorer.DefaultApiDescriptionProvider.OnProvidersExecuting(Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescriptionProviderContext! context) -> void diff --git a/src/Shared/RequestDelegateFactoryUtilities.cs b/src/Shared/TryParseMethodCache.cs similarity index 86% rename from src/Shared/RequestDelegateFactoryUtilities.cs rename to src/Shared/TryParseMethodCache.cs index d43c62d7a582..ba378a59d08e 100644 --- a/src/Shared/RequestDelegateFactoryUtilities.cs +++ b/src/Shared/TryParseMethodCache.cs @@ -3,17 +3,17 @@ using System; using System.Collections.Concurrent; +using System.Diagnostics; using System.Reflection; namespace Microsoft.AspNetCore.Http { - // REVIEW: Better name? - internal static class RequestDelegateFactoryUtilities + internal static class TryParseMethodCache { private static readonly MethodInfo EnumTryParseMethod = GetEnumTryParseMethod(); // Since this is shared source, the cache won't be shared between RequestDelegateFactory and the ApiDescriptionProvider sadly :( - private static readonly ConcurrentDictionary TryParseMethodCache = new(); + private static readonly ConcurrentDictionary Cache = new(); public static bool HasTryParseMethod(ParameterInfo parameter) { @@ -54,7 +54,7 @@ public static bool HasTryParseMethod(ParameterInfo parameter) return null; } - return TryParseMethodCache.GetOrAdd(type, Finder); + return Cache.GetOrAdd(type, Finder); } private static MethodInfo GetEnumTryParseMethod() @@ -63,7 +63,7 @@ private static MethodInfo GetEnumTryParseMethod() foreach (var method in staticEnumMethods) { - if (!method.IsGenericMethod || method.Name != "TryParse" || method.ReturnType != typeof(bool)) + if (!method.IsGenericMethod || method.Name != nameof(Enum.TryParse) || method.ReturnType != typeof(bool)) { continue; } @@ -78,7 +78,8 @@ private static MethodInfo GetEnumTryParseMethod() } } - throw new Exception("static bool System.Enum.TryParse(string? value, out TEnum result) does not exist!!?!?"); + Debug.Fail("static bool System.Enum.TryParse(string? value, out TEnum result) not found."); + throw new Exception("static bool System.Enum.TryParse(string? value, out TEnum result) not found."); } } }