Add Microsoft.AspNetCore.OpenApi package#41197
Conversation
| <Reference Include="Microsoft.OpenApi" /> | ||
| <Reference Include="Microsoft.AspNetCore.Http.Abstractions" /> | ||
| <Reference Include="Microsoft.AspNetCore.Routing" /> | ||
| <Reference Include="Microsoft.AspNetCore.Mvc.Core" /> |
There was a problem hiding this comment.
Organization question: Why put this under /Http/ when it depends on higher level components like MVC?
| { | ||
| return httpMethod switch | ||
| { | ||
| "GET" => OperationType.Get, |
There was a problem hiding this comment.
nit: Don't we have constants or static readonly properties somewhere for all of these strings❔
| <GenerateDocumentationFile>true</GenerateDocumentationFile> | ||
| <PackageTags>aspnetcore;openapi</PackageTags> | ||
| <Nullable>enable</Nullable> | ||
| <Trimmable>true</Trimmable> |
There was a problem hiding this comment.
Has trim-ability been tested❔
| <PropertyGroup> | ||
| <TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework> | ||
| <GenerateDocumentationFile>true</GenerateDocumentationFile> | ||
| <PackageTags>aspnetcore;openapi</PackageTags> |
There was a problem hiding this comment.
The package deserves a <Description>...</Description>. Otherwise people will need to guess its point from the package name and these tags. Suggest also adding more information to the commit and maybe the PR descriptions.
captainsafia
force-pushed
the
openapi-package
branch
from
be6e50f to
84913ff
Compare
| var pattern = routeEndpointBuilder.RoutePattern; | ||
| var metadata = new EndpointMetadataCollection(routeEndpointBuilder.Metadata); | ||
| var methodInfo = metadata.OfType<MethodInfo>().SingleOrDefault(); | ||
| var serviceProvider = routeEndpointBuilder.Metadata.OfType<IServiceProvider>().SingleOrDefault(); |
There was a problem hiding this comment.
This is kinda jank. The IServiceProvider shouldn't be metadata. I guess there's no other way to get the IServiceProvider? Can we defer the need for it later?
There was a problem hiding this comment.
I guess there's no other way to get the IServiceProvider?
Not that I can reason about.
Can we defer the need for it later?
There's some alternatives. We mostly need DI to access the services we need to determine what parameters are what. We can punt on checking those parameters and make a parameter-less constructor for OpenApiGenerator that we instantiate directly here without relying on DI.
But I think we have to solve that problem eventually...
| // Add the ServiceProvider to metadata so that it can be resolved | ||
| // by extension methods that need accesses to the same services that | ||
| // are registered by the target endpoint. | ||
| if (endpoints.ServiceProvider is not null) | ||
| { | ||
| builder.Metadata.Add(endpoints.ServiceProvider); | ||
| } | ||
|
|
There was a problem hiding this comment.
Pilchie
added
the
old-area-web-frameworks-do-not-use
|
Did you mean to close this @captainsafia❔ |
Yep, I ended up cleaning the commit history and opening a new PR. |
5 participants
Closes #40676.