Repository navigation
Read HttpGraph instead of EndpointDataSource in WolverineApiDescriptionProvider. Closes GH-3371 - #3373
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an ASP.NET Core ApiExplorer caching edge case (GH-3371) by making Wolverine’s IApiDescriptionProvider read from Wolverine’s HttpGraph (WolverineHttpOptions.Endpoints) instead of the runtime EndpointDataSource, ensuring Wolverine endpoint descriptions are available on the first ApiExplorer read after MapWolverineEndpoints() even before host start.
Changes:
- Update
WolverineApiDescriptionProviderto enumerateHttpGraph.Chainsand buildApiDescriptionentries directly from the already-builtRouteEndpointon each chain. - Add a regression test ensuring Wolverine ApiExplorer descriptions are complete prior to
app.StartAsync(). - Document the “pre-start ApiExplorer read” behavior and the remaining limitation for minimal APIs in hybrid apps.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Http/Wolverine.Http/WolverineApiDescriptionProvider.cs | Switch ApiExplorer enumeration from EndpointDataSource to HttpGraph to avoid start-time population/caching issues. |
| src/Http/Wolverine.Http.Tests/api_explorer_before_host_start.cs | Add regression test verifying descriptions are present before host start (after mapping). |
| docs/guide/http/metadata.md | Document start-independent Wolverine descriptions and hybrid-app caveat for minimal APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| using JasperFx.Core.Reflection; | ||
| using Microsoft.AspNetCore.Mvc.ApiExplorer; | ||
| using Microsoft.AspNetCore.Routing; |
jeremydmiller
left a comment
jeremydmiller
left a comment
There was a problem hiding this comment.
Reviewed in depth — this is exactly right, and the key insight makes it safely behavior-preserving: even in the old code, the composite EndpointDataSource enumeration was only ever used to rediscover the HttpChain from endpoint metadata; the ApiDescription itself was always built by chain.CreateApiDescription() off chain.Endpoint. Since the data-source RouteEndpoint for a Wolverine endpoint is the same instance as chain.Endpoint (both from chain.BuildEndpoint() in HttpGraph), switching discovery to WolverineHttpOptions.Endpoints changes nothing post-start — it just makes the provider complete from the moment MapWolverineEndpoints() returns. Ordering, the ExcludeFromDescription filter, and the QUERY-verb skip all carry over verbatim, and AspVersioning's document discovery was already graph-based, so this makes the provider consistent with it.
Also verified as correct calls:
- Leaving the CLI
ComposeEndpointDataSourcespath untouched — still needed for minimal-API endpoints in hybrid CLI generation. - Documenting (not fixing) the hybrid-host limitation; that half is ASP.NET's version-keyed cache.
- The new
chain.Endpoint == nullguard, which matches prior behavior (such chains never reached the composite source either). - The disclosed double-
MapWolverineEndpoints()semantic (only the last graph is described) — double-mapping was never supported, acceptable.
One non-blocking suggestion: the regression test asserts a complete pre-start read; adding a start-then-re-read assertion in the same test would lock in the full GH-3371 loop (the cached collection staying complete across StartAsync). Happy to take that as a follow-up if you'd rather not touch the PR again.
Merging once CI is green. Thanks for an unusually well-researched fix — the root-cause analysis on the version-keyed cache saved us real time, and this also unblocks a monitoring scenario on our side (CritterWatch's capability snapshot is an in-box pre-start ApiExplorer reader).
GH-3373 made Wolverine's own endpoint descriptions start-independent, but that only half-closed the freeze. ASP.NET Core surfaces endpoints to its own description providers through an EndpointDataSource that UseEndpoints() does not fill until the server starts, and it caches the first ApiExplorer read for the lifetime of the host. So on a hybrid host, a pre-start read cached a document carrying every Wolverine route and silently omitting every minimal API and MVC one — a plausible-looking subset, which is worse than the empty document it replaced. Publish the application's endpoints ahead of that read rather than refusing to serve it. It is the same merge UseEndpoints() performs at startup, with the same data source instances, so UseEndpoints() de-dupes it away and the router is untouched. The `openapi` command already did this merge privately (GH-2903, which is why it was never affected); lift it into HostEndpointDataSources so the in-process read shares it, and fail that command loudly instead of writing a partial spec at exit 0 if the merge ever stops working. Publish only from the root route builder: a route group's DataSources are its inner, un-prefixed sources that ASP.NET Core already publishes via the outer GroupDataSource, so republishing them would register every endpoint in the group a second time without its prefix or conventions. Wolverine warns rather than serve a partial document for that shape.
Fixes GH-3371: ASP.NET Core caches the first ApiExplorer read for the lifetime of the host (keyed on
ActionDescriptorCollection.Version, which endpoint routing never bumps), and Wolverine endpoints only reach the DIEndpointDataSourceat server start — so any earlier read permanently empties every OpenAPI document.The change
WolverineApiDescriptionProviderenumeratesWolverineHttpOptions.Endpoints(theHttpGraph) instead of the injectedEndpointDataSource. The graph — including each chain's builtRouteEndpoint— is complete as soon asMapWolverineEndpoints()returns, so the provider's answer is the same before and after server start.This is behavior-preserving: every value in the produced
ApiDescriptionalready comes fromchain.Endpoint; the data source enumeration only ever recovered the chain from endpoint metadata. TheExcludeFromDescriptionfilter and the QUERY-verb skip are unchanged. Chains are enumerated rather thangraph.Endpointsbecause the chain is what descriptions are built from — happy to switch if you'd rather keep the endpoint-shaped loop.OpenApiCommand.ComposeEndpointDataSourcesis intentionally untouched: ASP.NET's own minimal-API provider still reads the composite source, so the CLI's merge is still needed for non-Wolverine endpoints.Behavior notes
MapWolverineEndpoints()returns still sees nothing — unchanged.MapWolverineEndpoints()twice now describes only the last graph. Double-mapping duplicates every route and was never a supported configuration.chain.Endpoint == nullguard skips chains added to the graph after mapping — they never reached the data source either.Testing
api_explorer_before_host_start: maps endpoints, never starts the host, asserts descriptions are complete on the first ApiExplorer read. Fails on the old provider, passes on the new one.Wolverine.Http.Tests818/818 andWolverine.Http.AspVersioning.Tests66/66, includinggenerate_openapi_without_database(Wolverine centric OpenAPI generation command that isn't as problematic as Microsoft's #2903). Theopenapicommand emits the same document with no database running.