Repository navigation
Use Minimal APIs JSON options when MVC is not registered - #4116
martincostello merged 3 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4116 +/- ##
=======================================
Coverage 95.24% 95.24%
=======================================
Files 111 111
Lines 4142 4143 +1
Branches 848 848
=======================================
+ Hits 3945 3946 +1
Misses 197 197
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
7a38cf1 to
6c2b100
Compare
IOptions<Mvc.JsonOptions> resolves in any application, whether or not MVC was registered, so the fallback to the Minimal APIs options never ran and MVC's own camelCase default silently won. An application that sets a naming policy through ConfigureHttpJsonOptions therefore got a document that contradicted what its endpoints actually serialize. Only prefer the MVC options when MVC is registered, detected through IControllerFactory, which AddControllers, AddMvcCore and AddMvc all register and nothing in a Minimal APIs application does. When both are registered MVC keeps precedence, as before.
Each path now sets a policy that suffixes the property name, so an assertion names the source of the options directly instead of relying on a null policy looking different from the camelCase default. The both-registered test sets a different policy on each side, so it shows MVC winning rather than only showing camelCase.
The WebApi sample registers JsonStringEnumConverter through the Minimal APIs JSON options, which are now used because the sample does not register MVC. Regenerated from test runs.
|
The branch is rebased onto current master and the change is now limited to One set of snapshots still changes, and it is a direct result of the fix rather than a wider code change. The If you prefer that the sample stays unchanged, the alternative is to add |
6c2b100 to
fdd4aa1
Compare
|
Thanks for your contribution @RaphaelFakhri - the changes from this pull request have been published as part of version 10.3.0 📦, which is now available from NuGet.org 🚀 |
Fixes #3097.
IOptions<Microsoft.AspNetCore.Mvc.JsonOptions>resolves in any application, whether or not MVC was ever registered, so the existing fallback toIOptions<Microsoft.AspNetCore.Http.Json.JsonOptions>never ran: in a Minimal APIs application the MVC defaults were returned and the options set throughConfigureHttpJsonOptionswere ignored. The generated document then contradicted what the endpoints actually serialize, which is the case in the report: the endpoint returns{"HelloWorld":"hi"}while the schema documentshelloWorld.The MVC options are now preferred only when MVC is actually registered, detected through
IControllerFactory. I checked that it is registered byAddControllers(),AddMvcCore()andAddMvc(), and by nothing a Minimal APIs application uses, includingAddEndpointsApiExplorer(), which is whyIActionDescriptorCollectionProvideris not the marker here.When an application registers both, MVC keeps precedence exactly as before: one document is generated with one set of serializer options, so a tie-break is unavoidable and this is the one that does not change existing behaviour. An application that configures neither is unaffected, since both option types default to the same web defaults.
Four tests in
SwaggerGenServiceCollectionExtensionsTests: Minimal APIs only, MVC only, both registered, and neither configured.Swashbuckle.AspNetCore.SwaggerGen.Testpasses on net8.0, net9.0 and net10.0 with 764 tests, 0 failed.It turns out the repo's own Minimal APIs sample already demonstrates the bug:
test/WebSites/WebApiadds aJsonStringEnumConverterthroughMicrosoft.AspNetCore.Http.Json.JsonOptionsand never registers MVC, but the committed snapshots documentDateTimeKindas an int32 enum (0, 1, 2) while the endpoints actually serialize "Unspecified", "Utc" and "Local". With this change the generated document matches what the sample really returns, and the six snapshots are updated accordingly; every changed hunk is that one schema.This deliberately takes the no-new-public-API route rather than reviving what #3378 attempted, since that PR was closed with an invitation to start fresh.