Repository navigation
(MINOR): OpenAPI contract generation for IPatchObject<T> - #73
Merged
Merged
Conversation
…t<T>
Routes taking [FromBody] IPatchObject<T> are documented from the interface,
which has no properties, so the request body currently emits as
{ "type": "object", "additionalProperties": false } — a body that strictly
permits nothing.
The proposal recommends deriving the patch schema from the source model's
own schema (drop required, drop non-writable properties, add a description)
rather than describing the generated Optional-valued class, and ships that
as two small opt-in integration packages: an ISchemaFilter for Swashbuckle
and an IOpenApiSchemaTransformer for the built-in .NET 9+ generator. Both
approaches were prototyped and their output is recorded in the document.
Also records the core-library prerequisites: a shared PatchClassBuilder
cache, a public TryGetPatchSourceType helper, and a fix for PatchClassBuilder
throwing CS1001 for source types declared in the global namespace.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v
Swagger UI renders from the emitted document, so the proposed schema filter fixes it with no UI-specific work. Verified in a headless browser against the sample app: the request body goes from an empty model to the full model with constraints and description, and name renders without the required marker while the PUT body built from the same model still shows name*. The one thing the schema alone cannot fix is the Example Value tab. Swagger UI synthesizes examples from properties and ignores required, so a PATCH example lists every property and Try it out sends a full replacement. Setting Example on the patch schema overrides that, verified, so SimplePatchSchemaOptions gains an Example factory — off by default, since any property the library picked would be arbitrary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v
Share the generated patch classes: the cache in PatchClassBuilder is now static, so PatchJsonConverterFactory and anything else that needs a patch type — the OpenAPI integrations, user code — resolve a source type to the same generated type instead of each compiling its own dynamic assembly for it. PatchClassBuilder.Shared is the instance to reach for. Make TypeExtensions public and add TryGetPatchSourceType, which answers the question an OpenAPI schema filter actually asks: it accepts the open interface as well as a concrete implementation. The two existing helpers stay internal. Fix patch class generation for source types in the global namespace, which failed with "error CS1001: Identifier expected" because the generated namespace came out as ".Optionals". Minimal-API apps declaring their models in Program.cs hit this immediately. Add SimplePatchSchemaOptions, the configuration both integration packages take. It lives here so they configure identically and can be referenced side by side. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v
Swashbuckle documents a [FromBody] IPatchObject<T> parameter from the
interface, which has no properties, so the request body came out as
{ "type": "object", "additionalProperties": false } — a body that strictly
permits nothing, which a validator or generated client will reject.
AddSimplePatchSchemas registers a schema filter that rewrites that into the
patched model's schema with every property optional. The schema is derived
from the model's own generated schema rather than from the Optional-valued
class PatchClassBuilder emits, so validation constraints, custom converters,
property naming and any tuning already applied to the write model all carry
over, and the PUT and PATCH contracts cannot describe different shapes.
Properties the patch object cannot set — get-only ones, which the model's
schema marks readOnly — are dropped. Their JSON names come from asking
Swashbuckle to describe the generated patch class into a throwaway schema
repository, so the names agree with whatever JsonSerializerOptions the
application configured; Swashbuckle resolves those internally and exposes
them to neither filters nor DI. The throwaway repository keeps the Optional<T>
schemas that produces out of the real document.
The transform itself is a linked source file shared with the forthcoming
built-in-OpenAPI package, so the two cannot drift. It is internal, so both
packages can be referenced together.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v
Same transform as the Swashbuckle package, as an IOpenApiSchemaTransformer, registered with AddOpenApi(options => options.AddSimplePatchSchemas()). Here the patchable property names come from JsonTypeInfo.Options — the very JsonSerializerOptions the document generator is using — so they match the names in the source model's schema exactly. net10.0 only. Obtaining the source model's schema from a transformer needs OpenApiSchemaTransformerContext.GetOrCreateSchemaAsync, which .NET 10 added and .NET 9 does not have. Swashbuckle covers .NET 8 and 9 in the meantime. Microsoft.AspNetCore.OpenApi is pinned to 10.0.12 rather than 10.0.1 because earlier 10.0.x pull Microsoft.OpenApi 2.0.0, which carries a known high severity advisory (GHSA-v5pm-xwqc-g5wc). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v
Wire the sample app to PTrampert.SimplePatch.Swashbuckle, including an explicit example. That last part is worth showing: Swagger UI builds its example from the schema's properties and ignores "required", so the default PATCH example lists every property, which reads as "send all of these" and is what "Try it out" submits. Add the new projects to docfx's API metadata, and record in the proposal that it shipped, with the one deviation — net10.0 rather than net9.0 for the built-in generator. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v
PaulTrampert
commented
Sep 11, 2026
Review feedback: don't link source files across csproj files. PatchSchemaTransform moves into PTrampert.SimplePatch, which both integration packages already reference, and the <Compile Include="../Shared/..."> items are gone along with the Shared/ folder. It stays internal via InternalsVisibleTo rather than becoming public API, since it is an implementation detail of the integration packages. This costs the core package a Microsoft.OpenApi dependency, pinned to 2.12.0 — 2.3.0 and below carry GHSA-v5pm-xwqc-g5wc. Swashbuckle 10.2.3 declares 2.3.0 but runs fine against 2.12.0; the Swashbuckle tests exercise the real schema generator and still pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v
PaulTrampert
commented
Sep 11, 2026
Review feedback: the core project should not carry that reference. It was only there because PatchSchemaTransform had moved in, and the transform takes Microsoft.OpenApi types — so hosting it in the core package handed a Microsoft.OpenApi dependency to every consumer of PTrampert.SimplePatch, OpenAPI user or not. Each integration package now carries its own copy of the transform as a private method: about a dozen mechanical lines, kept in step by the two mirrored test suites. No linked source files, and the core package's dependency list is back to what it was before. What the integrations do share through their project reference to the core package is SimplePatchSchemaOptions, which needs nothing beyond System.Text.Json. The Swashbuckle package now pins Microsoft.OpenApi 2.12.0 itself, since Swashbuckle pulls 2.3.0 and that carries GHSA-v5pm-xwqc-g5wc. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v
Review decision: rather than duplicating the transform across the two
integration packages, it gets its own package. PatchSchemaTransform and
SimplePatchSchemaOptions move there, and both integration packages reference
it, so there is one copy of the transform and the core package's dependency
list stays as it was.
PatchSchemaTransform becomes public — it is now this package's surface rather
than an implementation detail behind InternalsVisibleTo.
The package targets net8.0 so that both consumers can take it: the Swashbuckle
integration targets net8.0 and the built-in one targets net10.0.
Resulting graph, from the packed nuspecs:
PTrampert.SimplePatch Microsoft.CodeAnalysis.CSharp, System.CodeDom,
System.Text.Json, DotNetCompilerPlatform
...Schema + Microsoft.OpenApi 2.12.0
...Swashbuckle + Swashbuckle.AspNetCore.SwaggerGen
...OpenApi + Microsoft.AspNetCore.OpenApi
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v
PaulTrampert
commented
Sep 12, 2026
Review feedback on the README. The same construction appeared twice more in this PR — "Worth setting" in the sample's comment and in the SimplePatchSchemaOptions remarks — so all three go, and the prose is reflowed. The substance the reader needs is the Swagger UI behaviour itself, which is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v
The Swashbuckle integration carried its own Microsoft.OpenApi 2.12.0 pin alongside the schema package's. Once the transform moved into PTrampert.SimplePatch.Schema, that became redundant: the schema package's direct reference already outranks the 2.3.0 that Swashbuckle.AspNetCore brings, so the vulnerable version never resolves. Verified both sides. In this repo, restoring the Swashbuckle project without the pin resolves Microsoft.OpenApi 2.12.0 and raises no NU1903. For consumers, a scratch project referencing only PTrampert.SimplePatch.Swashbuckle from a local feed resolves 2.12.0 the same way, so the floor still holds through the package graph. One pin, in the package that actually uses the types. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v
PaulTrampert
commented
Sep 12, 2026
…uctor Review feedback. The singleton is now PatchClassBuilder.Instance, and the constructor is internal, so consumers reach the builder only through it — verified: a scratch project referencing the package fails to compile "new PatchClassBuilder()" while PatchClassBuilder.Instance builds fine. The tests do construct builders directly, to prove separate instances still resolve a source type to the same generated patch type, so the core assembly grants InternalsVisibleTo to PTrampert.SimplePatch.Test. The proposal document's snippets named the old member; updated so they match the shipped API. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v
Backs out the internal constructor from ea9bd58, keeping only the rename to PatchClassBuilder.Instance. PatchClassBuilder is a public type that has shipped with an implicit public constructor, so hiding it removes a member consumers could be calling — a breaking change, and this release is not a major one. The rename itself is safe: Shared was introduced in this branch and never shipped. InternalsVisibleTo goes with it; the tests construct builders through the public constructor again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v
Points callers at PatchClassBuilder.Instance and says the constructor will be made internal in the next major version, linking #75 which tracks that. Still a warning, not an error: a consumer calling new PatchClassBuilder() compiles, with CS0618. Two tests that only needed a builder now use Instance; GetPatchClassFor_SharesGeneratedTypesAcrossBuilders keeps the constructor behind a scoped pragma, since separately constructed builders sharing one cache is exactly what it asserts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v
✅ PR Title Formatted CorrectlyThe title of this PR has been updated to match the correct format. Thank you! |
PaulTrampert
enabled auto-merge (squash)
September 12, 2026 04:58
PaulTrampert
disabled auto-merge
September 12, 2026 04:59
PaulTrampert
enabled auto-merge (squash)
September 12, 2026 04:59
This was referenced Sep 12, 2026
PaulTrampert
added a commit
that referenced
this pull request
Oct 4, 2026
**Targets `release/2.0`**, the staging branch for the 2.0 major release, not `main`. ## Cause Since #73, `PatchClassBuilder`'s cache is static, so every instance resolves a source type to the same generated patch type. Constructing your own builder buys nothing over `PatchClassBuilder.Instance` except an allocation. The public constructor was kept only for compatibility, marked `[Obsolete]` and pointing at this issue for the next major version. ## Fix - `PatchClassBuilder()` is now `internal`, and its `[Obsolete]` attribute is gone. `PatchClassBuilder.Instance` is the only way for callers to get a builder. - The `CS0618` suppression around `Instance`'s initializer is removed, since it is no longer needed. - `RoslynPatchClassBuilderTest.GetPatchClassFor_SharesGeneratedTypesAcrossBuilders` still constructs two builders, to show that the static cache is shared. Its `CS0618` suppression is removed. The test project already had `InternalsVisibleTo` from `PTrampert.SimplePatch.csproj`, so no project change was needed. - AGENTS.md now says the constructor is internal, not obsolete. README.md and `docs/` never mentioned the constructor. This is a **breaking change**: callers of `new PatchClassBuilder()` no longer compile, and existing binaries get a `MissingMethodException`. Hence `(MAJOR)`. The type of `Instance` (#135), the Roslyn builder and `UseExperimentalDynamicClassBuilder` (#126), and target frameworks (#76) are out of scope and unchanged here. ## Alternatives rejected - **Make the constructor `private`.** That would break the shared-cache test. The test is still worth keeping while the cache is static and per-type rather than per-instance. - **Make the class `static`.** That would remove the `IPatchClassBuilder` implementation and `Instance`, which is a much larger break than the issue asks for. - **Delete the shared-cache test.** Making the constructor internal doesn't change the guarantee that the test checks. ## Test results - `dotnet build`: 0 errors and 21 warnings, the same count as the base branch. - `dotnet test`: PTrampert.SimplePatch.Test 129/129, Swashbuckle.Test 18/18, OpenApi.Test 12/12, all passing. The net8.0 test hosts were run with `DOTNET_ROLL_FORWARD=Major`, because only the .NET 10 runtime is installed locally. Closes #75 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the proposal in
docs/proposals/openapi-contract-generation.md(also added on this branch, along with the Swagger UI findings).The problem
A route taking
[FromBody] IPatchObject<PersonWriteModel>is documented from the interface, which has no properties. The sample app emitted:That's not just uninformative —
additionalProperties: falsewith nopropertiesdescribes a body that permits nothing, so a strict validator, contract test, or generated client rejects every legal PATCH. In Swagger UI the model renders as an empty box and "Try it out" prefills{}.The approach
The patch schema is derived from the source model's own schema — clear
required, drop properties the patch object can't set, add a description — rather than built by describing theOptional-valued classPatchClassBuilderemits.Describing the generated class would mean re-solving four things the generator already gets right for
T:Optional<T>documents as{hasValue, value}; the schema id becomes the randomPersonWriteModel_Optionals_xj1k2h3g, which changes every run;[StringLength]has been rewritten to[OptionalValidation(typeof(StringLengthAttribute))], which no generator recognizes, so constraints are lost; and[JsonConverter]has become[OptionalConverter].Deriving from
Tinherits validation, converters, naming policy, and anyMapType/filter already applied to the write model — and makes it impossible for the PUT and PATCH contracts to describe different shapes.What's here
Core (
PTrampert.SimplePatch), net8.0 — no new dependencies; its dependency list is unchanged by this PR.PatchClassBuilder's cache is now static, with aPatchClassBuilder.Sharedinstance. PreviouslyPatchJsonConverterFactoryhad its own private builder; a second builder compiled a second dynamic assembly for the same source type.TypeExtensionsis public, withTryGetPatchSourceType, which accepts the open interface as well as a concrete implementation. The two existing helpers stay internal.error CS1001: Identifier expectedfor source types in the global namespace — the generated namespace came out as".Optionals". Minimal-API apps declaring models inProgram.cshit this immediately.PTrampert.SimplePatch.Schema, net8.0 — the one copy ofPatchSchemaTransformplusSimplePatchSchemaOptions. Both integrations reference it, so the transform cannot drift between them. It lives here rather than in core because it takesMicrosoft.OpenApitypes, and core shouldn't hand that dependency to consumers who never generate OpenAPI. net8.0 so both integrations can consume it.PTrampert.SimplePatch.Swashbuckle, net8.0 —AddSwaggerGen(o => o.AddSimplePatchSchemas()), anISchemaFilter.PTrampert.SimplePatch.OpenApi, net10.0 —AddOpenApi(o => o.AddSimplePatchSchemas()), anIOpenApiSchemaTransformer.Options:
Example,SchemaId(t => t.Name + "Patch"forPersonWriteModelPatch),DescriptionFormat,ClearRequired.Dependency graph
Read back from the packed nuspecs. Each package's
lib/holds only its own assembly; the siblings are NuGet dependencies, pinned to the same build version.PTrampert.SimplePatchMicrosoft.CodeAnalysis.CSharp,Microsoft.CodeDom.Providers.DotNetCompilerPlatform,System.CodeDom,System.Text.JsonPTrampert.SimplePatch.SchemaMicrosoft.OpenApi2.12.0PTrampert.SimplePatch.SwashbuckleSwashbuckle.AspNetCore.SwaggerGen10.2.3PTrampert.SimplePatch.OpenApiMicrosoft.AspNetCore.OpenApi10.0.12Result
Verified against the sample app:
In Swagger UI,
namerenders without the red*while the PUT body built from the same model still showsname*— so a reader can see at a glance that PATCH takes a subset. PATCH round-trips still work, and validation still rejects{"name":"ab"}with a 400.Two decisions worth a look
Only
PTrampert.SimplePatch.OpenApiis .NET 10 — core,.Schemaand.Swashbuckleare all net8.0. The proposal called for net9.0 and net10.0 on that one package. Obtaining the source model's schema from a transformer needsOpenApiSchemaTransformerContext.GetOrCreateSchemaAsync, which .NET 10 added and .NET 9 does not have (verified by compiling against 9); .NET 9's OpenAPI.NET 1.6 object model would also fork the shared transform. .NET 8 and 9 apps still get full support through the Swashbuckle package, so the narrowing only affects apps on the built-in generator below .NET 10.Microsoft.OpenApiis pinned to 2.12.0 in one place:PTrampert.SimplePatch.Schema. Versions at or below 2.3.0 carry GHSA-v5pm-xwqc-g5wc and raiseNU1903.Swashbuckle.AspNetCore.SwaggerGenbrings 2.3.0, but the schema package's direct reference outranks it, so the single pin covers both integrations — checked by restoring a scratch project that references onlyPTrampert.SimplePatch.Swashbucklefrom a local feed, which resolvesMicrosoft.OpenApi/2.12.0with no advisory. Separately,Microsoft.AspNetCore.OpenApiis on 10.0.12 rather than 10.0.1 because earlier 10.0.x pullMicrosoft.OpenApi2.0.0.Testing
46 tests pass across the three test projects (28 core, 10 Swashbuckle, 8 OpenApi). The integration tests drive the real DI container and schema generator rather than the filter in isolation, and cover: properties and constraints carried over,
requiredcleared while the source model keeps its own, get-only and[JsonIgnore]properties dropped,[JsonPropertyName]honoured, each option, and that noOptional<T>components leak into the document.All four packages
dotnet packcleanly, with noNU1903.Release pipeline
No workflow change needed — verified by running the reusable workflow's own commands locally against this branch.
project_namenames the solution, not a project (dotnet pack ${{inputs.project_name}}.sln), so packing the solution produces all four packages plus symbols; the test projects areIsPackable=falseand the sample is a Web SDK project, so neither ships.dotnet nuget push **/*.nupkgaccepts the multi-file expansion and pushes each in turn. Push order is alphabetical, so.OpenApigoes up before the.Schemait depends on — harmless, since nuget.org does not resolve dependencies at push time and all four carry the same version from one run.Notes for you
README.mdanddocs/getting-started.mdrefer toIPatchObjectFor<T>andpatch.ApplyTo(person), neither of which exists. Left alone to keep this PR focused.🤖 Generated with Claude Code
https://claude.ai/code/session_01RtsmxAyPuVHce2nkVDZh2v