Add OpenAPI/Swagger support for minimal actions (#33433)

Author: halter73

src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj

@@ -12,7 +12,8 @@
 
   <ItemGroup>
     <Compile Include="$(SharedSourceRoot)ObjectMethodExecutor\**\*.cs" />
-    <Compile Include="..\..\Shared\StreamCopyOperationInternal.cs" Link="StreamCopyOperationInternal.cs" />
+    <Compile Include="$(SharedSourceRoot)TryParseMethodCache.cs" />
+    <Compile Include="..\..\Shared\StreamCopyOperationInternal.cs" />
   </ItemGroup>
 
   <ItemGroup>

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<Func<HttpResponse, string, Task>>((response, text) => HttpResponseWritingExtensions.WriteAsync(response, text, default));
         private static readonly MethodInfo JsonResultWriteResponseAsyncMethod = GetMethodInfo<Func<HttpResponse, object, Task>>((response, value) => HttpResponseJsonExtensions.WriteAsJsonAsync(response, value, default));
-        private static readonly MethodInfo EnumTryParseMethod = GetEnumTryParseMethod();
         private static readonly MethodInfo LogParameterBindingFailureMethod = GetMethodInfo<Action<HttpContext, string, string, string>>((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<Type, MethodInfo?> TryParseMethodCache = new();
-
         /// <summary>
         /// Creates a <see cref="RequestDelegate"/> implementation for <paramref name="action"/>.
         /// </summary>
@@ -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) || TryParseMethodCache.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<TEnum>(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 = TryParseMethodCache.FindTryParseMethod(nonNullableParameterType);
 
             if (tryParseMethod is null)
             {

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<InvalidOperationException>(() => RequestDelegateFactory.Create((Action<int>)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]

src/Http/Routing/src/Builder/MinimalActionEndpointRouteBuilderExtensions.cs

@@ -61,7 +61,7 @@ public static MinimalActionEndpointConventionBuilder MapPost(
         /// <param name="endpoints">The <see cref="IEndpointRouteBuilder"/> to add the route to.</param>
         /// <param name="pattern">The route pattern.</param>
         /// <param name="action">The delegate executed when the endpoint is matched.</param>
-        /// <returns>A <see cref="IEndpointConventionBuilder"/> that canaction be used to further customize the endpoint.</returns>
+        /// <returns>A <see cref="IEndpointConventionBuilder"/> that can be used to further customize the endpoint.</returns>
         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();
 

src/Http/Routing/test/UnitTests/Builder/MinimalActionEndpointRouteBuilderExtensionsTest.cs

@@ -42,20 +42,16 @@ 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<IHttpMethodMetadata>().ToArray();
+
+            static string GetMethod(IHttpMethodMetadata metadata) => Assert.Single(metadata.HttpMethods);
 
             Assert.Equal(3, metadataArray.Length);
             Assert.Equal("ATTRIBUTE", GetMethod(metadataArray[0]));
             Assert.Equal("METHOD", GetMethod(metadataArray[1]));
             Assert.Equal("BUILDER", GetMethod(metadataArray[2]));
 
             Assert.Equal("BUILDER", endpoint.Metadata.GetMetadata<IHttpMethodMetadata>()!.HttpMethods.Single());
-
-            string GetMethod(object metadata)
-            {
-                var httpMethodMetadata = Assert.IsAssignableFrom<IHttpMethodMetadata>(metadata);
-                return Assert.Single(httpMethodMetadata.HttpMethods);
-            }
         }
 
         [Fact]

src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs

@@ -77,13 +77,48 @@ private ICollection<ApiResponseType> GetApiResponseTypes(
            IReadOnlyList<IApiResponseMetadataProvider> 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<ApiResponseType> ReadResponseMetadata(
+            IReadOnlyList<IApiResponseMetadataProvider> responseMetadataAttributes,
+            Type? type,
+            Type defaultErrorType,
+            MediaTypeCollection contentTypes)
         {
             var results = new Dictionary<int, ApiResponseType>();
 
             // 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)
@@ -105,7 +140,7 @@ private ICollection<ApiResponseType> GetApiResponseTypes(
                         {
                             // 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;
                         }
@@ -129,29 +164,7 @@ private ICollection<ApiResponseType> 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<ApiResponseType> responseTypes, MediaTypeCollection declaredContentTypes)

src/Mvc/Mvc.ApiExplorer/src/DefaultApiDescriptionProvider.cs

@@ -429,7 +429,7 @@ private IReadOnlyList<ApiRequestFormat> GetSupportedFormats(MediaTypeCollection
             return results;
         }
 
-        private static MediaTypeCollection GetDeclaredContentTypes(IApiRequestMetadataProvider[]? requestMetadataAttributes)
+        internal static MediaTypeCollection GetDeclaredContentTypes(IReadOnlyList<IApiRequestMetadataProvider>? 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.

src/Mvc/Mvc.ApiExplorer/src/DependencyInjection/EndpointMethodInfoApiExplorerServiceCollectionExtensions.cs

@@ -0,0 +1,32 @@
+// 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.Extensions;
+
+namespace Microsoft.Extensions.DependencyInjection
+{
+    /// <summary>
+    /// Extensions for configuring ApiExplorer using <see cref="Endpoint.Metadata"/>.
+    /// </summary>
+    public static class EndpointMetadataApiExplorerServiceCollectionExtensions
+    {
+        /// <summary>
+        /// Configures ApiExplorer using <see cref="Endpoint.Metadata"/>.
+        /// </summary>
+        /// <param name="services">The <see cref="IServiceCollection"/>.</param>
+        public static IServiceCollection AddEndpointsApiExplorer(this IServiceCollection services)
+        {
+            // Try to add default services in case MVC services aren't added.
+            services.TryAddSingleton<IActionDescriptorCollectionProvider, DefaultActionDescriptorCollectionProvider>();
+            services.TryAddSingleton<IApiDescriptionGroupCollectionProvider, ApiDescriptionGroupCollectionProvider>();
+
+            services.TryAddEnumerable(
+                ServiceDescriptor.Transient<IApiDescriptionProvider, EndpointMetadataApiDescriptionProvider>());
+
+            return services;
+        }
+    }
+}