Skip to content

Conversation

@halter73
Copy link
Member

image

Addresses #30662

@halter73 halter73 added this to the 6.0-preview6 milestone Jun 10, 2021
@BrennanConroy BrennanConroy added the feature-minimal-actions Controller-like actions for endpoint routing label Jun 10, 2021
&& routeEndpoint.Metadata.GetMetadata<MethodInfo>() is { } methodInfo
&& routeEndpoint.Metadata.GetMetadata<IHttpMethodMetadata>() is { } httpMethodMetadata)
{
// REVIEW: Should we add an ApiDescription for endpoints without IHttpMethodMetadata? Swagger doesn't handle
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting q - MVC does add an entry for items that do not specify a method constraint (with a null value), so maybe it's fine to add one:

private IEnumerable<string?> GetHttpMethods(ControllerActionDescriptor action)
{
if (action.ActionConstraints != null && action.ActionConstraints.Count > 0)
{
return action.ActionConstraints.OfType<HttpMethodActionConstraint>().SelectMany(c => c.HttpMethods);
}
else
{
return new string?[] { null };
}
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried null initially since ApiDescription.HttpMethod is indeed nullable, but I get this when trying this with Swashbuckle 5.6.3.

fail: Microsoft.AspNetCore.Server.Kestrel[13]
      Swashbuckle.AspNetCore.SwaggerGen.SwaggerGeneratorException: Ambiguous HTTP method for action - . Actions require an explicit HttpMethod binding for Swagger/OpenAPI 3.0
         at Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GenerateOperations(IEnumerable`1 apiDescriptions, SchemaRepository schemaRepository) in Swashbuckle.AspNetCore.SwaggerGen.dll:token 0x60000f0+0x76
         at Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GeneratePaths(IEnumerable`1 apiDescriptions, SchemaRepository schemaRepository) in Swashbuckle.AspNetCore.SwaggerGen.dll:token 0x60000ef+0x60
         at Swashbuckle.AspNetCore.SwaggerGen.SwaggerGenerator.GetSwagger(String documentName, String host, String basePath) in Swashbuckle.AspNetCore.SwaggerGen.dll:token 0x60000ed+0xe0
         at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider) in Swashbuckle.AspNetCore.Swagger.dll:token 0x6000009+0xa9
         at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application) in Microsoft.AspNetCore.Server.Kestrel.Core.dll:token 0x6000a9f+0x1b8

Is there a workaround?

@bradygaster
Copy link
Member

OH SO EXCITING

@Tratcher Tratcher removed their request for review June 10, 2021 21:43
@halter73 halter73 marked this pull request as ready for review June 15, 2021 20:00
// 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('<');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaredpar Is there a better way to do this?

@halter73 halter73 force-pushed the halter73/30662 branch 2 times, most recently from 3f3e634 to 3f9451c Compare June 16, 2021 00:42
@halter73 halter73 added the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label Jun 16, 2021
@ghost
Copy link

ghost commented Jun 16, 2021

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@halter73
Copy link
Member Author

/backport to release/6.0-preview6

@github-actions
Copy link
Contributor

Started backporting to release/6.0-preview6: https://github.com/dotnet/aspnetcore/actions/runs/941202939

@halter73 halter73 enabled auto-merge (squash) June 18, 2021 18:15
@halter73 halter73 merged commit fd19f92 into main Jun 18, 2021
@halter73 halter73 deleted the halter73/30662 branch June 18, 2021 19:52
@ghost ghost modified the milestones: 6.0-preview6, 6.0-preview7 Jun 18, 2021
@BrennanConroy
Copy link
Member

API Review:
AddMinimalActionsApiExplorer()

That would make MinimalActions more of a public name, whereas right now it's mostly a code-name.

AddEndpointsApiExplorer() uses the now overloaded word "Endpoints" and it doesn't apply to all Endpoints.

@pranavkm pranavkm added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews labels Jul 6, 2021
@amcasey amcasey added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-runtime labels Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-approved API was approved in API review, it can be implemented area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-minimal-actions Controller-like actions for endpoint routing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants