From 94a14f42b69628cbf8cd88854bfff592fdfd95fd Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Sun, 2 Aug 2026 20:11:44 +0200 Subject: [PATCH 01/15] docs(aot): plan the Native AOT compatibility checks Records the triage decision for the 68 IL2026/IL3050 warnings that appear once IsAotCompatible is enabled: route the reflection-based JsonSerializer calls through JsonTypeInfo, following the existing write-path pattern, rather than annotating the public entry points. Closes #78 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EMMdhU9uijRHcmmuynn2Ve --- ai-plans/0078-aot-compatibility-checks.md | 81 +++++++++++++++++++++++ 1 file changed, 81 insertions(+) create mode 100644 ai-plans/0078-aot-compatibility-checks.md diff --git a/ai-plans/0078-aot-compatibility-checks.md b/ai-plans/0078-aot-compatibility-checks.md new file mode 100644 index 0000000..2701633 --- /dev/null +++ b/ai-plans/0078-aot-compatibility-checks.md @@ -0,0 +1,81 @@ +# Make the Native AOT Compatibility Claim Verifiable + +## Rationale + +`Light.PortableResults` advertises "Compatible with .NET Native AOT" in its package description, and the README repeats the claim for the base, validation, and Minimal APIs packages. The base package never sets `IsAotCompatible`, so the trim and AOT analyzers have never run over it. Enabling them for the `net10.0` asset surfaces 68 IL2026/IL3050 warnings, all in the CloudEvents write path and the two read paths. + +The warnings are not cosmetic. Both `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` build a `JsonSerializerOptions` with converters but no `TypeInfoResolver`. Under `JsonSerializerIsReflectionEnabledByDefault=false`, the switch Native AOT sets, `result.ToCloudEvent(...)`, `Result.Ok().ToCloudEvent(...)`, and `response.ReadResultAsync()` all throw `InvalidOperationException: Reflection-based serialization has been disabled for this application`. The first two are the README's "Publish to RabbitMQ" quick start, and the second involves no user type at all. Because the annotations are never propagated, the consumer gets no compile-time warning and discovers this when the AOT binary throws. + +The underlying design is sound: registering `CloudEventsEnvelopeForWriting`, `CloudEventsEnvelopeForWriting`, and the `HttpRead*Payload` types in a `JsonSerializerContext` makes every one of those calls succeed. What is missing is the plumbing that makes that enforceable, discoverable, and regression-proof. Do this before 0.7.0: the corrective work touches the public surface and is cheap while breaking changes are still allowed. + +## Acceptance Criteria + +- [ ] The `net10.0` assets of `Light.PortableResults` and `Light.PortableResults.Validation` build with `IsAotCompatible`, and the `netstandard2.0` assets build without `NETSDK1210`. +- [ ] Both packages build clean under `Release`, where `TreatWarningsAsErrors` turns any future IL2026/IL3050 into a build failure. No warning is resolved by disabling a rule in `.editorconfig` or `NoWarn`. +- [ ] The core package ships public source-generated `JsonSerializerContext` types covering its own CloudEvents write, CloudEvents read, and HTTP read payload types, so a consumer never has to name library-internal payload types in their own context. +- [ ] Options constructed from the shipped contexts round-trip a non-generic `Result` and a `Result` over CloudEvents and HTTP without a reflection-based resolver present, proving the paths need no runtime code generation. Tests assert this in-process by resolver composition, not by toggling the reflection switch. +- [ ] Behavior for consumers who pass reflection-backed options is unchanged: the existing test suites pass without modification beyond additions, and `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` keep serializing arbitrary `T` as they do today. +- [ ] The Native AOT sample exercises the CloudEvents write path and the HTTP read path, so ILC covers the code this issue is about, and its blanket `SuppressTrimAnalysisWarnings` no longer hides warnings from Light.PortableResults assemblies. +- [ ] CI publishes the sample with `PublishAot=true` and fails on any trim or AOT warning originating in a Light.PortableResults assembly. +- [ ] The README documents how to compose options for Native AOT, and the `` of every affected package describe their respective changes. + +## Technical Details + +### Enabling the analyzers + +`IsAotCompatible` is unsupported on `netstandard2.0` and raises `NETSDK1210` when set unconditionally. Use the SDK's own recommended form in both multi-targeted projects: + +```xml +true +``` + +This is independent of the existing `PublishAot=false` and `TreatAsLocalProperty="PublishAot"` settings, which stay as they are. `Light.PortableResults.Validation` is clean today; enabling it there buys regression protection only, at no triage cost. + +Measured warning distribution in `Light.PortableResults` before any fix — use it to confirm the triage is complete: + +| File | IL2026 | IL3050 | +| --- | ---: | ---: | +| `CloudEvents/Reading/ReadOnlyMemoryCloudEventsExtensions.cs` | 16 | 16 | +| `Http/Reading/HttpResponseMessageExtensions.cs` | 12 | 12 | +| `CloudEvents/Writing/CloudEventsResultExtensions.cs` | 4 | 4 | +| `Http/Reading/Json/ResultJsonReader.cs` | 2 | 2 | + +### Resolving the warnings: route through `JsonTypeInfo`, do not annotate + +Every warning is a call to a `JsonSerializer.Serialize`/`Deserialize(…, JsonSerializerOptions)` overload. The payload type is statically known at each site; only the resolver is the caller's. The library already has the correct pattern for this and it produces no warnings: `SystemTextJsonWritingExtensions` resolves `options.GetTypeInfo(typeof(T))`, casts to `JsonTypeInfo`, and calls the `JsonTypeInfo` overload; `LightResult` and `LightActionResult` do the same and throw an actionable message when the cast fails. Apply that pattern to the four files above. + +This is the decision that shapes the rest of the work, so record why the alternative was rejected. Annotating the public entry points with `[RequiresDynamicCode]`/`[RequiresUnreferencedCode]` would be permanently wrong: these methods do not intrinsically require dynamic code, they require the caller's options to carry a resolver. Annotating would force a suppression on every correctly configured AOT consumer while doing nothing for an incorrectly configured one. Blanket `UnconditionalSuppressMessage` at the call sites is equally wrong here — it silences a diagnostic that is currently accurate. The two existing suppressions in `CloudEventsEnvelopeForWritingJsonConverterFactory` and its HTTP counterpart stay: `MakeGenericType` over a resolved generic is genuinely safe when the closed type is registered, which is what their justifications already state. + +Where a resolved `JsonTypeInfo` cannot be obtained, throw `InvalidOperationException` naming the missing type and pointing at the shipped context, matching the wording style already used in `LightResult`. This replaces `System.Text.Json`'s generic "reflection has been disabled" message, which does not tell the caller which type to register. + +### Shipped contexts and the default options + +Ship public `JsonSerializerContext` types in the core package, one per direction, mirroring `PortableResultsMinimalApiJsonContext`: the CloudEvents write envelopes, the CloudEvents read payloads, and the HTTP read payloads, plus `MetadataObject` and `MetadataValue`. `GenerationMode = JsonSourceGenerationMode.Metadata` is the right mode; serialization mode buys nothing for converter-backed types. Verify the S.T.J. source generator behaves on the `netstandard2.0` asset — if it does not, condition the contexts to the `net10.0` asset, since Native AOT only exists there. + +Leave `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` reflection-backed. They cannot be made AOT-complete regardless: a generic `Result` needs `CloudEventsEnvelopeForWriting` or `HttpReadAutoSuccessResultPayload` closed over the consumer's own type, which only the consumer's context can supply. Setting a source-generated resolver on the shared defaults would silently stop arbitrary `T` from serializing for the majority of consumers who do not use AOT — a far worse trade than requiring explicit opt-in. Instead, expose the composition explicitly, for example a `Create…SerializerOptions(IJsonTypeInfoResolver consumerResolver)` helper per direction that combines the shipped context with the consumer's via `JsonTypeInfoResolver.Combine` and applies the existing `AddDefault…Converters` call. Consumers who prefer to wire it by hand can keep doing so; the helper exists so the README has one line to point at. + +Confirmed working shape, for reference — this succeeds today with reflection disabled, without any library change: + +```csharp +var options = new JsonSerializerOptions(JsonSerializerDefaults.Web) { TypeInfoResolver = MyContext.Default }; +options.AddDefaultPortableResultsCloudEventsWriteJsonConverters(); +``` + +where `MyContext` declares `[JsonSerializable(typeof(CloudEventsEnvelopeForWriting))]` and `[JsonSerializable(typeof(CloudEventsEnvelopeForWriting))]`. The work here is to make the library half of that automatic. + +### Proving it, in tests and in CI + +Two layers, because neither is sufficient alone. + +In-process tests compose options whose only resolver is a source-generated context — the shipped one combined with a test-local context for the value type — and assert that a non-generic `Result` and a `Result` round-trip over CloudEvents and HTTP. A path that completes with no reflection-based resolver in the graph cannot have needed runtime code generation, which is the property under test. Do not toggle `System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault` from a test: the value is read once and cached per process, and the test hosts are shared and parallel. The switch remains the right manual reproduction, via a standalone console app with `false`, and that is how the failures in the Rationale were confirmed. + +The ILC publish is the end-to-end gate, and it only means something once two things change in `samples/NativeAotMovieRating`. First, the sample is a server that exercises the Minimal API write path exclusively — the path that already works. Extend it to publish a CloudEvents message and to read a `Result` back from an `HttpResponseMessage`, so ILC and the trim analyzer actually walk the code this issue is about; the in-memory database module is a reasonable place to hang an outbox-style publish, and the sample's own endpoints can serve as the source for a typed-client read. Second, the sample sets `SuppressTrimAnalysisWarnings=true` to silence Serilog's unannotated assembly rollup, which would equally hide every warning this work is meant to catch. Replace it with a targeted suppression scoped to the Serilog assemblies, and set `TrimmerSingleWarn=false` so individual warnings are reported rather than collapsed into one per assembly. + +Add the publish to `build-and-test.yml` as a separate step after the existing test steps, on the runner's own RID. It is the slowest step in the workflow; keep it out of the matrix and do not gate the coverage jobs on it. + +### Deliberately out of scope + +- **`Light.PortableResults.AspNetCore.Mvc`.** MVC is not Native AOT compatible, and its package description does not claim otherwise. +- **Making the shared `Default` options AOT-complete.** Rejected above; the generic payload types make it impossible without the consumer's context. +- **Trim-only (`PublishTrimmed`) verification as a separate gate.** The AOT publish subsumes the trim analyzer for this code; a dedicated trimmed-but-not-AOT configuration would add a second slow CI step for no additional signal. +- **The `default(Result)` write guard** and **`EnablePackageValidation`.** Tracked separately as items 2 and 3 of #77. From 5a050f9de2123ba646b83363b3fa08dfdcf2c25b Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Sun, 2 Aug 2026 20:31:14 +0200 Subject: [PATCH 02/15] docs(aot): defer the sample extension out of the plan The ILC publish gate stays, but it covers the Minimal API write path only; the in-process resolver-composition tests carry the proof for the CloudEvents write and HTTP read paths. The sample keeps its narrowed trim-warning suppression so the gate can report warnings from this library. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EMMdhU9uijRHcmmuynn2Ve --- ai-plans/0078-aot-compatibility-checks.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/ai-plans/0078-aot-compatibility-checks.md b/ai-plans/0078-aot-compatibility-checks.md index 2701633..860e884 100644 --- a/ai-plans/0078-aot-compatibility-checks.md +++ b/ai-plans/0078-aot-compatibility-checks.md @@ -15,8 +15,7 @@ The underlying design is sound: registering `CloudEventsEnvelopeForWriting`, `Cl - [ ] The core package ships public source-generated `JsonSerializerContext` types covering its own CloudEvents write, CloudEvents read, and HTTP read payload types, so a consumer never has to name library-internal payload types in their own context. - [ ] Options constructed from the shipped contexts round-trip a non-generic `Result` and a `Result` over CloudEvents and HTTP without a reflection-based resolver present, proving the paths need no runtime code generation. Tests assert this in-process by resolver composition, not by toggling the reflection switch. - [ ] Behavior for consumers who pass reflection-backed options is unchanged: the existing test suites pass without modification beyond additions, and `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` keep serializing arbitrary `T` as they do today. -- [ ] The Native AOT sample exercises the CloudEvents write path and the HTTP read path, so ILC covers the code this issue is about, and its blanket `SuppressTrimAnalysisWarnings` no longer hides warnings from Light.PortableResults assemblies. -- [ ] CI publishes the sample with `PublishAot=true` and fails on any trim or AOT warning originating in a Light.PortableResults assembly. +- [ ] CI publishes the Native AOT sample with `PublishAot=true` and fails on any trim or AOT warning originating in a Light.PortableResults assembly, which requires that the sample's blanket `SuppressTrimAnalysisWarnings` no longer hide them. - [ ] The README documents how to compose options for Native AOT, and the `` of every affected package describe their respective changes. ## Technical Details @@ -65,11 +64,13 @@ where `MyContext` declares `[JsonSerializable(typeof(CloudEventsEnvelopeForWriti ### Proving it, in tests and in CI -Two layers, because neither is sufficient alone. +The in-process tests are the primary proof for this issue; the ILC publish is regression protection for the paths the sample already walks. In-process tests compose options whose only resolver is a source-generated context — the shipped one combined with a test-local context for the value type — and assert that a non-generic `Result` and a `Result` round-trip over CloudEvents and HTTP. A path that completes with no reflection-based resolver in the graph cannot have needed runtime code generation, which is the property under test. Do not toggle `System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault` from a test: the value is read once and cached per process, and the test hosts are shared and parallel. The switch remains the right manual reproduction, via a standalone console app with `false`, and that is how the failures in the Rationale were confirmed. -The ILC publish is the end-to-end gate, and it only means something once two things change in `samples/NativeAotMovieRating`. First, the sample is a server that exercises the Minimal API write path exclusively — the path that already works. Extend it to publish a CloudEvents message and to read a `Result` back from an `HttpResponseMessage`, so ILC and the trim analyzer actually walk the code this issue is about; the in-memory database module is a reasonable place to hang an outbox-style publish, and the sample's own endpoints can serve as the source for a typed-client read. Second, the sample sets `SuppressTrimAnalysisWarnings=true` to silence Serilog's unannotated assembly rollup, which would equally hide every warning this work is meant to catch. Replace it with a targeted suppression scoped to the Serilog assemblies, and set `TrimmerSingleWarn=false` so individual warnings are reported rather than collapsed into one per assembly. +The ILC publish covers `samples/NativeAotMovieRating`, which exercises the Minimal API write path exclusively — the path that already works. It therefore does not verify the CloudEvents write or HTTP read paths in this release; extending the sample to walk them is deferred to a follow-up the maintainer will drive. Keep the gate anyway: it proves ILC still succeeds after the `IsAotCompatible` and `JsonTypeInfo` changes, and it is the seam the later sample work plugs into. + +For the gate to report anything at all from this library, one sample setting has to change. The sample sets `SuppressTrimAnalysisWarnings=true` to silence Serilog's unannotated assembly rollup, which equally hides every warning originating in Light.PortableResults. Replace it with a targeted suppression scoped to the Serilog assemblies, and set `TrimmerSingleWarn=false` so individual warnings are reported rather than collapsed into one per assembly. This is the only change the sample needs here, and it adds no new code paths. Add the publish to `build-and-test.yml` as a separate step after the existing test steps, on the runner's own RID. It is the slowest step in the workflow; keep it out of the matrix and do not gate the coverage jobs on it. @@ -77,5 +78,6 @@ Add the publish to `build-and-test.yml` as a separate step after the existing te - **`Light.PortableResults.AspNetCore.Mvc`.** MVC is not Native AOT compatible, and its package description does not claim otherwise. - **Making the shared `Default` options AOT-complete.** Rejected above; the generic payload types make it impossible without the consumer's context. +- **Extending the Native AOT sample to exercise the CloudEvents write and HTTP read paths.** Deferred to a follow-up the maintainer will drive. Until then the ILC gate covers the Minimal API write path only, and the in-process resolver-composition tests carry the proof for the paths this issue fixes. - **Trim-only (`PublishTrimmed`) verification as a separate gate.** The AOT publish subsumes the trim analyzer for this code; a dedicated trimmed-but-not-AOT configuration would add a second slow CI step for no additional signal. - **The `default(Result)` write guard** and **`EnablePackageValidation`.** Tracked separately as items 2 and 3 of #77. From 9768bae9cf2ed6ced461640545b847c56d367ad5 Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Sun, 2 Aug 2026 20:48:52 +0200 Subject: [PATCH 03/15] docs(aot): let the library own its own JSON contracts Resolver composition cannot synthesize the closed wrapper contracts, so combining a shipped library context with the consumer's would still force consumers to declare CloudEventsEnvelopeForWriting and the HttpRead* payloads themselves. Take the other route from the review: the write path calls the existing public Utf8JsonWriter extension directly instead of detouring through JsonSerializer, and the read paths build their wrapper contracts from the library's own converters via JsonMetadataServices.CreateValueInfo. The consumer context then only ever declares the result value type. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EMMdhU9uijRHcmmuynn2Ve --- ai-plans/0078-aot-compatibility-checks.md | 39 +++++++++++++++-------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/ai-plans/0078-aot-compatibility-checks.md b/ai-plans/0078-aot-compatibility-checks.md index 860e884..43ad4b6 100644 --- a/ai-plans/0078-aot-compatibility-checks.md +++ b/ai-plans/0078-aot-compatibility-checks.md @@ -6,14 +6,15 @@ The warnings are not cosmetic. Both `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` build a `JsonSerializerOptions` with converters but no `TypeInfoResolver`. Under `JsonSerializerIsReflectionEnabledByDefault=false`, the switch Native AOT sets, `result.ToCloudEvent(...)`, `Result.Ok().ToCloudEvent(...)`, and `response.ReadResultAsync()` all throw `InvalidOperationException: Reflection-based serialization has been disabled for this application`. The first two are the README's "Publish to RabbitMQ" quick start, and the second involves no user type at all. Because the annotations are never propagated, the consumer gets no compile-time warning and discovers this when the AOT binary throws. -The underlying design is sound: registering `CloudEventsEnvelopeForWriting`, `CloudEventsEnvelopeForWriting`, and the `HttpRead*Payload` types in a `JsonSerializerContext` makes every one of those calls succeed. What is missing is the plumbing that makes that enforceable, discoverable, and regression-proof. Do this before 0.7.0: the corrective work touches the public surface and is cheap while breaking changes are still allowed. +The underlying design is sound. Every failing site serializes a library-owned type through a library-owned converter, so the library can supply the contract itself and leave the consumer responsible for their own value type alone. What is missing is the plumbing that makes that so, enforceably and regression-proof. Do this before 0.7.0: the corrective work touches the public surface and is cheap while breaking changes are still allowed. ## Acceptance Criteria - [ ] The `net10.0` assets of `Light.PortableResults` and `Light.PortableResults.Validation` build with `IsAotCompatible`, and the `netstandard2.0` assets build without `NETSDK1210`. - [ ] Both packages build clean under `Release`, where `TreatWarningsAsErrors` turns any future IL2026/IL3050 into a build failure. No warning is resolved by disabling a rule in `.editorconfig` or `NoWarn`. -- [ ] The core package ships public source-generated `JsonSerializerContext` types covering its own CloudEvents write, CloudEvents read, and HTTP read payload types, so a consumer never has to name library-internal payload types in their own context. -- [ ] Options constructed from the shipped contexts round-trip a non-generic `Result` and a `Result` over CloudEvents and HTTP without a reflection-based resolver present, proving the paths need no runtime code generation. Tests assert this in-process by resolver composition, not by toggling the reflection switch. +- [ ] A consumer never names a library-owned type in their own `JsonSerializerContext`. Registering the result value type is sufficient for CloudEvents write, CloudEvents read, and HTTP read; the library resolves contracts for its own envelope and payload types itself. +- [ ] A non-generic `Result` and a `Result` round-trip over CloudEvents and HTTP against options whose only resolver is a consumer context declaring the value type alone, proving the paths need no runtime code generation. Tests assert this in-process by resolver composition, not by toggling the reflection switch. +- [ ] Library-owned contracts are created once per `JsonSerializerOptions` instance, not per serialization call, and creation works against options that are already read-only. - [ ] Behavior for consumers who pass reflection-backed options is unchanged: the existing test suites pass without modification beyond additions, and `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` keep serializing arbitrary `T` as they do today. - [ ] CI publishes the Native AOT sample with `PublishAot=true` and fails on any trim or AOT warning originating in a Light.PortableResults assembly, which requires that the sample's blanket `SuppressTrimAnalysisWarnings` no longer hide them. - [ ] The README documents how to compose options for Native AOT, and the `` of every affected package describe their respective changes. @@ -45,28 +46,39 @@ Every warning is a call to a `JsonSerializer.Serialize`/`Deserialize(…, Jso This is the decision that shapes the rest of the work, so record why the alternative was rejected. Annotating the public entry points with `[RequiresDynamicCode]`/`[RequiresUnreferencedCode]` would be permanently wrong: these methods do not intrinsically require dynamic code, they require the caller's options to carry a resolver. Annotating would force a suppression on every correctly configured AOT consumer while doing nothing for an incorrectly configured one. Blanket `UnconditionalSuppressMessage` at the call sites is equally wrong here — it silences a diagnostic that is currently accurate. The two existing suppressions in `CloudEventsEnvelopeForWritingJsonConverterFactory` and its HTTP counterpart stay: `MakeGenericType` over a resolved generic is genuinely safe when the closed type is registered, which is what their justifications already state. -Where a resolved `JsonTypeInfo` cannot be obtained, throw `InvalidOperationException` naming the missing type and pointing at the shipped context, matching the wording style already used in `LightResult`. This replaces `System.Text.Json`'s generic "reflection has been disabled" message, which does not tell the caller which type to register. +Where a resolved `JsonTypeInfo` cannot be obtained for the consumer's value type, throw `InvalidOperationException` naming the missing type, matching the wording style already used in `LightResult`. This replaces `System.Text.Json`'s generic "reflection has been disabled" message, which does not tell the caller which type to register. -### Shipped contexts and the default options +### Library-owned contracts, not resolver composition -Ship public `JsonSerializerContext` types in the core package, one per direction, mirroring `PortableResultsMinimalApiJsonContext`: the CloudEvents write envelopes, the CloudEvents read payloads, and the HTTP read payloads, plus `MetadataObject` and `MetadataValue`. `GenerationMode = JsonSourceGenerationMode.Metadata` is the right mode; serialization mode buys nothing for converter-backed types. Verify the S.T.J. source generator behaves on the `netstandard2.0` asset — if it does not, condition the contexts to the `net10.0` asset, since Native AOT only exists there. +Resolver composition cannot carry this. `JsonTypeInfoResolver.Combine` queries each resolver in order and returns the first contract that matches the requested type; it never synthesizes `CloudEventsEnvelopeForWriting` from a contract for the envelope and another for `MyDto`. Verified: with a consumer context declaring only `string`, the current write path throws `NotSupportedException` for `CloudEventsEnvelopeForWriting`. Shipping library contexts and combining them would therefore not spare consumers from declaring every closed wrapper themselves, in the right direction-specific shape — a requirement that leaks library-internal types into consumer code and breaks whenever a wrapper is added. -Leave `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` reflection-backed. They cannot be made AOT-complete regardless: a generic `Result` needs `CloudEventsEnvelopeForWriting` or `HttpReadAutoSuccessResultPayload` closed over the consumer's own type, which only the consumer's context can supply. Setting a source-generated resolver on the shared defaults would silently stop arbitrary `T` from serializing for the majority of consumers who do not use AOT — a far worse trade than requiring explicit opt-in. Instead, expose the composition explicitly, for example a `Create…SerializerOptions(IJsonTypeInfoResolver consumerResolver)` helper per direction that combines the shipped context with the consumer's via `JsonTypeInfoResolver.Combine` and applies the existing `AddDefault…Converters` call. Consumers who prefer to wire it by hand can keep doing so; the helper exists so the README has one line to point at. +Take the other route: never ask `System.Text.Json` to resolve a contract for a library-owned type. Every one of those types is already serialized by a library-owned converter, so the library can supply the contract itself, and the consumer's context then only ever needs their own value type. Both mechanisms are verified against a consumer context declaring `string` alone, with reflection disabled. -Confirmed working shape, for reference — this succeeds today with reflection disabled, without any library change: +**Write.** Replace `JsonSerializer.Serialize(writer, envelope, options.SerializerOptions)` in `CloudEventsResultExtensions` with a direct call to the existing public `writer.WriteCloudEvents(envelope, options)`. This is not new machinery: the converter that the current call dispatches into already delegates to that extension, so the `JsonSerializer` hop is a detour through contract resolution for a type the library fully controls. Removing it also removes the warnings at those sites. Metadata is unaffected — it is written through the manual `Utf8JsonWriter` extensions and needs no contract. + +**Read.** Build the wrapper's contract from the library's own converter and use the `JsonTypeInfo` overloads: ```csharp -var options = new JsonSerializerOptions(JsonSerializerDefaults.Web) { TypeInfoResolver = MyContext.Default }; -options.AddDefaultPortableResultsCloudEventsWriteJsonConverters(); +var wrapperInfo = JsonMetadataServices.CreateValueInfo>( + options, + new HttpReadAutoSuccessResultPayloadJsonConverter() +); +var payload = await JsonSerializer.DeserializeAsync(contentStream, wrapperInfo).ConfigureAwait(false); ``` -where `MyContext` declares `[JsonSerializable(typeof(CloudEventsEnvelopeForWriting))]` and `[JsonSerializable(typeof(CloudEventsEnvelopeForWriting))]`. The work here is to make the library half of that automatic. +`CreateValueInfo` is the source generator's own building block, involves no reflection, and is analyzer-clean: a project compiled with `IsAotCompatible=true` reports zero IL warnings for the write and read shapes above. The same treatment applies to the non-generic payloads and to the CloudEvents read path. + +Two constraints on the implementation. `CreateValueInfo` allocates a `JsonTypeInfo` per call, so cache per `JsonSerializerOptions` instance — a static per-closed-type cache keyed by options, not a new contract on every serialization; this library does not allocate on hot paths. Creation against already-read-only options is safe, so the cache may be lazy: verified after an explicit `MakeReadOnly()`, which is the state a long-lived `Default` instance reaches after first use. + +`CloudEventsEnvelopeForWritingJsonConverterFactory` and `HttpReadSuccessResultPayloadJsonConverterFactory` stay public and unchanged, with their existing `MakeGenericType` suppressions, for consumers who serialize the wrappers themselves through reflection-backed options. They simply leave the library's own path. + +This also improves the defaults. `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` stay reflection-backed for the consumer's `T` — no library can supply that contract, and putting a source-generated resolver on the shared defaults would silently stop arbitrary `T` from serializing for the non-AOT majority. But once the library owns its own contracts, the defaults stop failing on library-owned types, so a non-generic `Result` should need no consumer registration at all. Confirm that during implementation and state it in the README; it is the difference between "AOT needs setup" and "AOT needs setup proportional to your own payload types". ### Proving it, in tests and in CI The in-process tests are the primary proof for this issue; the ILC publish is regression protection for the paths the sample already walks. -In-process tests compose options whose only resolver is a source-generated context — the shipped one combined with a test-local context for the value type — and assert that a non-generic `Result` and a `Result` round-trip over CloudEvents and HTTP. A path that completes with no reflection-based resolver in the graph cannot have needed runtime code generation, which is the property under test. Do not toggle `System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault` from a test: the value is read once and cached per process, and the test hosts are shared and parallel. The switch remains the right manual reproduction, via a standalone console app with `false`, and that is how the failures in the Rationale were confirmed. +In-process tests compose options whose only resolver is a test-local source-generated context declaring the value type and nothing else, and assert that a non-generic `Result` and a `Result` round-trip over CloudEvents and HTTP. That the context declares no library-owned type is itself part of the assertion. A path that completes with no reflection-based resolver in the graph cannot have needed runtime code generation, which is the property under test. Do not toggle `System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault` from a test: the value is read once and cached per process, and the test hosts are shared and parallel. The switch remains the right manual reproduction, via a standalone console app with `false`, and that is how the failures in the Rationale were confirmed. The ILC publish covers `samples/NativeAotMovieRating`, which exercises the Minimal API write path exclusively — the path that already works. It therefore does not verify the CloudEvents write or HTTP read paths in this release; extending the sample to walk them is deferred to a follow-up the maintainer will drive. Keep the gate anyway: it proves ILC still succeeds after the `IsAotCompatible` and `JsonTypeInfo` changes, and it is the seam the later sample work plugs into. @@ -77,7 +89,8 @@ Add the publish to `build-and-test.yml` as a separate step after the existing te ### Deliberately out of scope - **`Light.PortableResults.AspNetCore.Mvc`.** MVC is not Native AOT compatible, and its package description does not claim otherwise. -- **Making the shared `Default` options AOT-complete.** Rejected above; the generic payload types make it impossible without the consumer's context. +- **Putting a source-generated resolver on the shared `Default` options.** Rejected above: only the consumer's context can supply a contract for their `T`, and a source-generated resolver on the shared defaults would break arbitrary `T` for the non-AOT majority. +- **Shipping `JsonSerializerContext` types for library-owned payloads.** Superseded — with the library building its own contracts from its own converters, there is nothing left for a shipped context to declare. Add one only if triage finds a site where converter-backed contract creation is impractical, and record why. - **Extending the Native AOT sample to exercise the CloudEvents write and HTTP read paths.** Deferred to a follow-up the maintainer will drive. Until then the ILC gate covers the Minimal API write path only, and the in-process resolver-composition tests carry the proof for the paths this issue fixes. - **Trim-only (`PublishTrimmed`) verification as a separate gate.** The AOT publish subsumes the trim analyzer for this code; a dedicated trimmed-but-not-AOT configuration would add a second slow CI step for no additional signal. - **The `default(Result)` write guard** and **`EnablePackageValidation`.** Tracked separately as items 2 and 3 of #77. From 4b910710ae3efe61236c5a11bd06055508aa0318 Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Sun, 2 Aug 2026 21:00:06 +0200 Subject: [PATCH 04/15] docs(aot): pin the exact trim-warning configuration for the CI gate TrimmerSingleWarn=false expands Serilog into individual diagnostics, and Release TreatWarningsAsErrors turns IL2067 into a failing publish. Keep single-warn mode and demote only the IL2104 package rollup: the SDK applies single-warn to package references while project references keep reporting in detail, so an injected regression in Light.PortableResults still fails the publish with file and line. Also records why the CI step must not pass PublishAot on the command line. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EMMdhU9uijRHcmmuynn2Ve --- ai-plans/0078-aot-compatibility-checks.md | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/ai-plans/0078-aot-compatibility-checks.md b/ai-plans/0078-aot-compatibility-checks.md index 43ad4b6..8f9c074 100644 --- a/ai-plans/0078-aot-compatibility-checks.md +++ b/ai-plans/0078-aot-compatibility-checks.md @@ -16,7 +16,7 @@ The underlying design is sound. Every failing site serializes a library-owned ty - [ ] A non-generic `Result` and a `Result` round-trip over CloudEvents and HTTP against options whose only resolver is a consumer context declaring the value type alone, proving the paths need no runtime code generation. Tests assert this in-process by resolver composition, not by toggling the reflection switch. - [ ] Library-owned contracts are created once per `JsonSerializerOptions` instance, not per serialization call, and creation works against options that are already read-only. - [ ] Behavior for consumers who pass reflection-backed options is unchanged: the existing test suites pass without modification beyond additions, and `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` keep serializing arbitrary `T` as they do today. -- [ ] CI publishes the Native AOT sample with `PublishAot=true` and fails on any trim or AOT warning originating in a Light.PortableResults assembly, which requires that the sample's blanket `SuppressTrimAnalysisWarnings` no longer hide them. +- [ ] CI publishes the Native AOT sample and fails on any trim or AOT diagnostic originating in a Light.PortableResults assembly, while the sample's unannotated package dependencies stay non-fatal. The sample's blanket `SuppressTrimAnalysisWarnings` is gone, and the replacement is demonstrated to fail the publish on an injected library-side regression. - [ ] The README documents how to compose options for Native AOT, and the `` of every affected package describe their respective changes. ## Technical Details @@ -82,9 +82,26 @@ In-process tests compose options whose only resolver is a test-local source-gene The ILC publish covers `samples/NativeAotMovieRating`, which exercises the Minimal API write path exclusively — the path that already works. It therefore does not verify the CloudEvents write or HTTP read paths in this release; extending the sample to walk them is deferred to a follow-up the maintainer will drive. Keep the gate anyway: it proves ILC still succeeds after the `IsAotCompatible` and `JsonTypeInfo` changes, and it is the seam the later sample work plugs into. -For the gate to report anything at all from this library, one sample setting has to change. The sample sets `SuppressTrimAnalysisWarnings=true` to silence Serilog's unannotated assembly rollup, which equally hides every warning originating in Light.PortableResults. Replace it with a targeted suppression scoped to the Serilog assemblies, and set `TrimmerSingleWarn=false` so individual warnings are reported rather than collapsed into one per assembly. This is the only change the sample needs here, and it adds no new code paths. +For the gate to report anything at all from this library, one sample setting has to change. The sample sets `SuppressTrimAnalysisWarnings=true` to silence Serilog's unannotated assembly rollup, which equally hides every warning originating in Light.PortableResults. Drop it, and keep single-warning mode rather than turning it off: -Add the publish to `build-and-test.yml` as a separate step after the existing test steps, on the runner's own RID. It is the slowest step in the workflow; keep it out of the matrix and do not gate the coverage jobs on it. +```xml + +$(WarningsNotAsErrors);IL2104 +``` + +Do not set `TrimmerSingleWarn=false`. Verified on this sample: it expands Serilog into individual diagnostics, `Serilog.Capturing.PropertyValueConverter.TryConvertStructure` raises `IL2067`, the repository's Release `TreatWarningsAsErrors` promotes it to an error, and ILC fails with `MSB3077`. Suppressing that through `NoWarn` is not an option either — `IL2067` is not Serilog-specific, and silencing it globally would hide the same diagnostic in Light assemblies. + +The configuration above is not a blunt suppression, because the SDK applies single-warn to package references while project references always report in detail. Verified end to end by injecting a `JsonSerializer.Serialize(value, value.GetType())` call into `Light.PortableResults` and publishing the sample: + +| Origin | Reported as | Fatal | +| --- | --- | --- | +| `Serilog.dll` (package reference) | one `IL2104` rollup | no | +| `Light.PortableResults` (project reference) | `IL3050` and `IL2026`, with file and line | yes | + +The injected regression failed the publish while Serilog stayed quiet, which is exactly the asymmetry the gate needs. Note this also holds today, before the analyzers are enabled: `IsAotCompatible` governs compile-time analysis, so ILC is the only thing that would have caught such a call in the current codebase. + +Add the publish to `build-and-test.yml` as a separate step after the existing test steps, on the runner's own RID. Publish the sample project directly and do not pass `/p:PublishAot=true` on the command line: as a global property it flows into every project reference and fails the `netstandard2.0` source generator with `NETSDK1207`. The sample already sets `PublishAot` internally and strips it from its references via `GlobalPropertiesToRemove`, which is why `dotnet publish -c Release -r ` is the correct invocation. It is the slowest step in the workflow; keep it out of the matrix and do not gate the coverage jobs on it. ### Deliberately out of scope From c107f05c1e1fb3934ffa934687ef8ee797c3dc29 Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Sun, 2 Aug 2026 21:03:18 +0200 Subject: [PATCH 05/15] docs(aot): resolve type info with TryGetTypeInfo GetTypeInfo throws NotSupportedException for an unknown type rather than returning null, so a null check or failed-cast check after it is unreachable for the only case that occurs in practice. That makes the promised actionable exception impossible and leaves three existing guards as dead code. Prescribe TryGetTypeInfo with explicit translation, require a negative test per site, and fold the repair of the existing guards into this work so the broken pattern is not copied further. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EMMdhU9uijRHcmmuynn2Ve --- ai-plans/0078-aot-compatibility-checks.md | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/ai-plans/0078-aot-compatibility-checks.md b/ai-plans/0078-aot-compatibility-checks.md index 8f9c074..28ba99a 100644 --- a/ai-plans/0078-aot-compatibility-checks.md +++ b/ai-plans/0078-aot-compatibility-checks.md @@ -15,6 +15,7 @@ The underlying design is sound. Every failing site serializes a library-owned ty - [ ] A consumer never names a library-owned type in their own `JsonSerializerContext`. Registering the result value type is sufficient for CloudEvents write, CloudEvents read, and HTTP read; the library resolves contracts for its own envelope and payload types itself. - [ ] A non-generic `Result` and a `Result` round-trip over CloudEvents and HTTP against options whose only resolver is a consumer context declaring the value type alone, proving the paths need no runtime code generation. Tests assert this in-process by resolver composition, not by toggling the reflection switch. - [ ] Library-owned contracts are created once per `JsonSerializerOptions` instance, not per serialization call, and creation works against options that are already read-only. +- [ ] When the consumer's value type is unresolvable, every affected entry point throws an exception that names the unresolved type and states the remedy, and a negative test pins that behavior per site. The unreachable guards in `SystemTextJsonWritingExtensions`, `LightResult`, and `LightActionResult` are converted so they can fire. - [ ] Behavior for consumers who pass reflection-backed options is unchanged: the existing test suites pass without modification beyond additions, and `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` keep serializing arbitrary `T` as they do today. - [ ] CI publishes the Native AOT sample and fails on any trim or AOT diagnostic originating in a Light.PortableResults assembly, while the sample's unannotated package dependencies stay non-fatal. The sample's blanket `SuppressTrimAnalysisWarnings` is gone, and the replacement is demonstrated to fail the publish on an injected library-side regression. - [ ] The README documents how to compose options for Native AOT, and the `` of every affected package describe their respective changes. @@ -42,11 +43,27 @@ Measured warning distribution in `Light.PortableResults` before any fix — use ### Resolving the warnings: route through `JsonTypeInfo`, do not annotate -Every warning is a call to a `JsonSerializer.Serialize`/`Deserialize(…, JsonSerializerOptions)` overload. The payload type is statically known at each site; only the resolver is the caller's. The library already has the correct pattern for this and it produces no warnings: `SystemTextJsonWritingExtensions` resolves `options.GetTypeInfo(typeof(T))`, casts to `JsonTypeInfo`, and calls the `JsonTypeInfo` overload; `LightResult` and `LightActionResult` do the same and throw an actionable message when the cast fails. Apply that pattern to the four files above. +Every warning is a call to a `JsonSerializer.Serialize`/`Deserialize(…, JsonSerializerOptions)` overload. The payload type is statically known at each site; only the resolver is the caller's. The fix is to resolve a `JsonTypeInfo` from the options and call the `JsonTypeInfo` overload, which is analyzer-clean. `SystemTextJsonWritingExtensions`, `LightResult`, and `LightActionResult` already do this, but resolve through `options.GetTypeInfo(typeof(T))` — see the next section before copying them. This is the decision that shapes the rest of the work, so record why the alternative was rejected. Annotating the public entry points with `[RequiresDynamicCode]`/`[RequiresUnreferencedCode]` would be permanently wrong: these methods do not intrinsically require dynamic code, they require the caller's options to carry a resolver. Annotating would force a suppression on every correctly configured AOT consumer while doing nothing for an incorrectly configured one. Blanket `UnconditionalSuppressMessage` at the call sites is equally wrong here — it silences a diagnostic that is currently accurate. The two existing suppressions in `CloudEventsEnvelopeForWritingJsonConverterFactory` and its HTTP counterpart stay: `MakeGenericType` over a resolved generic is genuinely safe when the closed type is registered, which is what their justifications already state. -Where a resolved `JsonTypeInfo` cannot be obtained for the consumer's value type, throw `InvalidOperationException` naming the missing type, matching the wording style already used in `LightResult`. This replaces `System.Text.Json`'s generic "reflection has been disabled" message, which does not tell the caller which type to register. +### Resolve with `TryGetTypeInfo`, and repair the existing guards + +`JsonSerializerOptions.GetTypeInfo` does not return `null` for an unknown type — it throws `NotSupportedException` first. Verified against a source-generated resolver declaring only `string`: + +``` +GetTypeInfo(typeof(HttpResultForWriting)) + -> NotSupportedException: JsonTypeInfo metadata for type '…HttpResultForWriting' was not provided by + TypeInfoResolver of type 'OnlyStringContext'. … +TryGetTypeInfo(typeof(HttpResultForWriting), out var info) + -> false, info is null +``` + +So a `GetTypeInfo` call followed by a null check or a failed-cast check can never produce the intended exception; the guard is unreachable for the missing-type case, which is the only case that occurs in practice. Three existing guards are dead code for this reason and must be converted as part of this work, not copied: `SystemTextJsonWritingExtensions` (the `is null` check after `GetTypeInfo`; the `TryGetTypeInfo` call later in the same method is already correct and is the model), `LightResult`, and `LightActionResult`. + +Resolve through `options.TryGetTypeInfo(typeof(T), out var typeInfo)` and translate the negative result into an `InvalidOperationException` that names the unresolved type and states the remedy. Keep `InvalidOperationException` for consistency with the existing wording in `LightResult`, and keep the message specific to what the caller must do — register their value type with the context supplied to these options. `System.Text.Json`'s own message names the type and the resolver but cannot mention the library-specific setup, and the "reflection has been disabled" variant names no type at all. + +Every converted site needs a negative test asserting the exception type and that the message names the unresolved type; without it the guards stay untested, which is how the current three came to be unreachable. ### Library-owned contracts, not resolver composition @@ -105,7 +122,7 @@ Add the publish to `build-and-test.yml` as a separate step after the existing te ### Deliberately out of scope -- **`Light.PortableResults.AspNetCore.Mvc`.** MVC is not Native AOT compatible, and its package description does not claim otherwise. +- **Native AOT support for `Light.PortableResults.AspNetCore.Mvc`.** MVC is not Native AOT compatible, and its package description does not claim otherwise, so it gets no `IsAotCompatible` and no analyzer triage. The unreachable guard in `LightActionResult` is still repaired: it is the same defect as the other two and is not an AOT concern. - **Putting a source-generated resolver on the shared `Default` options.** Rejected above: only the consumer's context can supply a contract for their `T`, and a source-generated resolver on the shared defaults would break arbitrary `T` for the non-AOT majority. - **Shipping `JsonSerializerContext` types for library-owned payloads.** Superseded — with the library building its own contracts from its own converters, there is nothing left for a shipped context to declare. Add one only if triage finds a site where converter-backed contract creation is impractical, and record why. - **Extending the Native AOT sample to exercise the CloudEvents write and HTTP read paths.** Deferred to a follow-up the maintainer will drive. Until then the ILC gate covers the Minimal API write path only, and the in-process resolver-composition tests carry the proof for the paths this issue fixes. From 9344bc7dc08b79e9f1fa91cd49ac1fd80af1ae40 Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Sun, 2 Aug 2026 21:11:55 +0200 Subject: [PATCH 06/15] docs(aot): make the library contract a fallback, not the first choice Building the wrapper contract from the library's own converter bypasses converters supplied through JsonSerializerOptions, which existing read tests depend on. Resolve from the options first and fall back only when the configured resolver cannot supply a contract. The options must have their resolver materialized before that check: the library's default options never assign a TypeInfoResolver, so TryGetTypeInfo reports false on them and every reflection-backed caller would otherwise lose its replacement converters. Also corrects the direct write call to pass SerializerOptions and the count of suppressed converter factories. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EMMdhU9uijRHcmmuynn2Ve --- ai-plans/0078-aot-compatibility-checks.md | 27 +++++++++++------------ 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/ai-plans/0078-aot-compatibility-checks.md b/ai-plans/0078-aot-compatibility-checks.md index 28ba99a..0dc6f8e 100644 --- a/ai-plans/0078-aot-compatibility-checks.md +++ b/ai-plans/0078-aot-compatibility-checks.md @@ -17,6 +17,7 @@ The underlying design is sound. Every failing site serializes a library-owned ty - [ ] Library-owned contracts are created once per `JsonSerializerOptions` instance, not per serialization call, and creation works against options that are already read-only. - [ ] When the consumer's value type is unresolvable, every affected entry point throws an exception that names the unresolved type and states the remedy, and a negative test pins that behavior per site. The unreachable guards in `SystemTextJsonWritingExtensions`, `LightResult`, and `LightActionResult` are converted so they can fire. - [ ] Behavior for consumers who pass reflection-backed options is unchanged: the existing test suites pass without modification beyond additions, and `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` keep serializing arbitrary `T` as they do today. +- [ ] Converters supplied through `JsonSerializerOptions` still take precedence over the library's own, including replacements inserted ahead of the defaults. The library-created contract is reached only when the configured resolver cannot supply one. - [ ] CI publishes the Native AOT sample and fails on any trim or AOT diagnostic originating in a Light.PortableResults assembly, while the sample's unannotated package dependencies stay non-fatal. The sample's blanket `SuppressTrimAnalysisWarnings` is gone, and the replacement is demonstrated to fail the publish on an injected library-side regression. - [ ] The README documents how to compose options for Native AOT, and the `` of every affected package describe their respective changes. @@ -45,7 +46,7 @@ Measured warning distribution in `Light.PortableResults` before any fix — use Every warning is a call to a `JsonSerializer.Serialize`/`Deserialize(…, JsonSerializerOptions)` overload. The payload type is statically known at each site; only the resolver is the caller's. The fix is to resolve a `JsonTypeInfo` from the options and call the `JsonTypeInfo` overload, which is analyzer-clean. `SystemTextJsonWritingExtensions`, `LightResult`, and `LightActionResult` already do this, but resolve through `options.GetTypeInfo(typeof(T))` — see the next section before copying them. -This is the decision that shapes the rest of the work, so record why the alternative was rejected. Annotating the public entry points with `[RequiresDynamicCode]`/`[RequiresUnreferencedCode]` would be permanently wrong: these methods do not intrinsically require dynamic code, they require the caller's options to carry a resolver. Annotating would force a suppression on every correctly configured AOT consumer while doing nothing for an incorrectly configured one. Blanket `UnconditionalSuppressMessage` at the call sites is equally wrong here — it silences a diagnostic that is currently accurate. The two existing suppressions in `CloudEventsEnvelopeForWritingJsonConverterFactory` and its HTTP counterpart stay: `MakeGenericType` over a resolved generic is genuinely safe when the closed type is registered, which is what their justifications already state. +This is the decision that shapes the rest of the work, so record why the alternative was rejected. Annotating the public entry points with `[RequiresDynamicCode]`/`[RequiresUnreferencedCode]` would be permanently wrong: these methods do not intrinsically require dynamic code, they require the caller's options to carry a resolver. Annotating would force a suppression on every correctly configured AOT consumer while doing nothing for an incorrectly configured one. Blanket `UnconditionalSuppressMessage` at the call sites is equally wrong here — it silences a diagnostic that is currently accurate. The four existing factory suppressions stay untouched: `MakeGenericType` over a resolved generic is genuinely safe when the closed type is registered, which is what their justifications already state. ### Resolve with `TryGetTypeInfo`, and repair the existing guards @@ -69,25 +70,23 @@ Every converted site needs a negative test asserting the exception type and that Resolver composition cannot carry this. `JsonTypeInfoResolver.Combine` queries each resolver in order and returns the first contract that matches the requested type; it never synthesizes `CloudEventsEnvelopeForWriting` from a contract for the envelope and another for `MyDto`. Verified: with a consumer context declaring only `string`, the current write path throws `NotSupportedException` for `CloudEventsEnvelopeForWriting`. Shipping library contexts and combining them would therefore not spare consumers from declaring every closed wrapper themselves, in the right direction-specific shape — a requirement that leaks library-internal types into consumer code and breaks whenever a wrapper is added. -Take the other route: never ask `System.Text.Json` to resolve a contract for a library-owned type. Every one of those types is already serialized by a library-owned converter, so the library can supply the contract itself, and the consumer's context then only ever needs their own value type. Both mechanisms are verified against a consumer context declaring `string` alone, with reflection disabled. +Take the other route: when `System.Text.Json` cannot resolve a contract for a library-owned type, let the library supply one. Every such type is already serialized by a library-owned converter, so the consumer's context only ever needs their own value type. Verified against a consumer context declaring `string` alone, with reflection disabled: a direct `writer.WriteCloudEvents(envelope, options.SerializerOptions)` writes a full CloudEvent including metadata, and a contract built with `JsonMetadataServices.CreateValueInfo>(options, new HttpReadAutoSuccessResultPayloadJsonConverter())` deserializes correctly. `CreateValueInfo` is the source generator's own building block, involves no reflection, and both shapes are analyzer-clean under `IsAotCompatible=true`. -**Write.** Replace `JsonSerializer.Serialize(writer, envelope, options.SerializerOptions)` in `CloudEventsResultExtensions` with a direct call to the existing public `writer.WriteCloudEvents(envelope, options)`. This is not new machinery: the converter that the current call dispatches into already delegates to that extension, so the `JsonSerializer` hop is a detour through contract resolution for a type the library fully controls. Removing it also removes the warnings at those sites. Metadata is unaffected — it is written through the manual `Utf8JsonWriter` extensions and needs no contract. +**The library-created contract must be the fallback, never the first choice.** Hardcoding the library converter would ignore converters supplied through `JsonSerializerOptions`, and existing tests depend on exactly that: `HttpResponseMessageExtensionsTests` inserts `NullBareStringPayloadConverter` and `EmptyFailurePayloadConverter` at index 0 and asserts the read path uses them. Resolve in this order at every converted site: -**Read.** Build the wrapper's contract from the library's own converter and use the `JsonTypeInfo` overloads: +1. If `options.TypeInfoResolver is null` and `JsonSerializer.IsReflectionEnabledByDefault`, call `options.MakeReadOnly(populateMissingResolver: true)`. +2. If `options.TryGetTypeInfo(typeof(TWrapper), out var info)` and `info is JsonTypeInfo typed`, serialize with `typed`. +3. Otherwise use the cached library-created contract, or the direct writer on the CloudEvents write path. -```csharp -var wrapperInfo = JsonMetadataServices.CreateValueInfo>( - options, - new HttpReadAutoSuccessResultPayloadJsonConverter() -); -var payload = await JsonSerializer.DeserializeAsync(contentStream, wrapperInfo).ConfigureAwait(false); -``` +Step 1 is not optional, and it is the step that is easy to miss. The library's own default options never assign a `TypeInfoResolver` — `CreateDefaultSerializerOptions` only adds converters — so `TryGetTypeInfo` returns `false` on them until a resolver is materialized, and without step 1 every reflection-backed caller would silently route to the fallback and lose their replacement converters. Verified: on default HTTP read options with a replacement converter at index 0, `TryGetTypeInfo` returns `false` beforehand and `true` afterwards, and the replacement converter is the one that runs. + +`MakeReadOnly(bool)` carries IL2026 and IL3050 because it may construct the reflection resolver. Guarding the call with `JsonSerializer.IsReflectionEnabledByDefault` makes it unreachable when reflection is off, which is precisely what that property exists for, so suppress both codes there with that justification. This is the one place in the new design where a suppression is correct. Freezing the options is not a behavior change: the paths that reach step 1 are about to serialize with them, which freezes them anyway. -`CreateValueInfo` is the source generator's own building block, involves no reflection, and is analyzer-clean: a project compiled with `IsAotCompatible=true` reports zero IL warnings for the write and read shapes above. The same treatment applies to the non-generic payloads and to the CloudEvents read path. +Under AOT the sequence degrades exactly as intended: step 1 is skipped, step 2 fails for a consumer context declaring only `T`, and step 3 carries the call. Under reflection, step 2 always wins and behavior is unchanged. -Two constraints on the implementation. `CreateValueInfo` allocates a `JsonTypeInfo` per call, so cache per `JsonSerializerOptions` instance — a static per-closed-type cache keyed by options, not a new contract on every serialization; this library does not allocate on hot paths. Creation against already-read-only options is safe, so the cache may be lazy: verified after an explicit `MakeReadOnly()`, which is the state a long-lived `Default` instance reaches after first use. +Two constraints on step 3. `CreateValueInfo` allocates a `JsonTypeInfo` per call, so cache per `JsonSerializerOptions` instance — a static per-closed-type cache keyed by options, not a new contract on every serialization; this library does not allocate on hot paths. Creation against already-read-only options is safe, so the cache may be lazy: verified after an explicit `MakeReadOnly()`, which is the state a long-lived `Default` instance reaches after first use. And even in the fallback, prefer a converter registered on `options.Converters` for the wrapper type over the library default, so a consumer who replaces a converter is honored under AOT too. -`CloudEventsEnvelopeForWritingJsonConverterFactory` and `HttpReadSuccessResultPayloadJsonConverterFactory` stay public and unchanged, with their existing `MakeGenericType` suppressions, for consumers who serialize the wrappers themselves through reflection-backed options. They simply leave the library's own path. +Four factories carry `MakeGenericType` suppressions and all stay public and unchanged: `CloudEventsEnvelopeForWritingJsonConverterFactory`, `CloudEventsSuccessPayloadJsonConverterFactory`, and `HttpReadSuccessResultPayloadJsonConverterFactory` sit on the three paths this work touches and continue to serve step 2, while `HttpResultForWritingJsonConverterFactory` is on the HTTP write path and is not involved at all. This also improves the defaults. `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` stay reflection-backed for the consumer's `T` — no library can supply that contract, and putting a source-generated resolver on the shared defaults would silently stop arbitrary `T` from serializing for the non-AOT majority. But once the library owns its own contracts, the defaults stop failing on library-owned types, so a non-generic `Result` should need no consumer registration at all. Confirm that during implementation and state it in the README; it is the difference between "AOT needs setup" and "AOT needs setup proportional to your own payload types". From 66883f0adcd94d65e7b091f36519c7e9e0baf5ce Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Sun, 2 Aug 2026 21:21:12 +0200 Subject: [PATCH 07/15] docs(aot): condense the plan without dropping decisions Restores three points lost in the condensation: the existing read tests that pin converter precedence and would fail if the library contract came first, the release-timing argument for doing this before 0.7.0, and the to-be-confirmed status of non-generic results needing no consumer registration. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01EMMdhU9uijRHcmmuynn2Ve --- ai-plans/0078-aot-compatibility-checks.md | 125 ++++++++-------------- 1 file changed, 45 insertions(+), 80 deletions(-) diff --git a/ai-plans/0078-aot-compatibility-checks.md b/ai-plans/0078-aot-compatibility-checks.md index 0dc6f8e..4ec3dd3 100644 --- a/ai-plans/0078-aot-compatibility-checks.md +++ b/ai-plans/0078-aot-compatibility-checks.md @@ -2,38 +2,35 @@ ## Rationale -`Light.PortableResults` advertises "Compatible with .NET Native AOT" in its package description, and the README repeats the claim for the base, validation, and Minimal APIs packages. The base package never sets `IsAotCompatible`, so the trim and AOT analyzers have never run over it. Enabling them for the `net10.0` asset surfaces 68 IL2026/IL3050 warnings, all in the CloudEvents write path and the two read paths. +`Light.PortableResults` advertises Native AOT compatibility, but its `net10.0` asset does not set `IsAotCompatible`. Enabling the trim and AOT analyzers exposes 68 IL2026/IL3050 warnings across CloudEvents writing and the CloudEvents and HTTP read paths. -The warnings are not cosmetic. Both `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` build a `JsonSerializerOptions` with converters but no `TypeInfoResolver`. Under `JsonSerializerIsReflectionEnabledByDefault=false`, the switch Native AOT sets, `result.ToCloudEvent(...)`, `Result.Ok().ToCloudEvent(...)`, and `response.ReadResultAsync()` all throw `InvalidOperationException: Reflection-based serialization has been disabled for this application`. The first two are the README's "Publish to RabbitMQ" quick start, and the second involves no user type at all. Because the annotations are never propagated, the consumer gets no compile-time warning and discovers this when the AOT binary throws. - -The underlying design is sound. Every failing site serializes a library-owned type through a library-owned converter, so the library can supply the contract itself and leave the consumer responsible for their own value type alone. What is missing is the plumbing that makes that so, enforceably and regression-proof. Do this before 0.7.0: the corrective work touches the public surface and is cheap while breaking changes are still allowed. +The warnings represent runtime failures: with reflection serialization disabled, the default options cannot resolve library-owned envelope and payload types, so even non-generic operations throw. The library already owns converters for those types and should supply their contracts while leaving consumers responsible only for their result value types. Make that behavior enforceable and regression-protected before 0.7.0: the corrective work changes public surface, which is cheap while breaking changes are still allowed and expensive afterwards. ## Acceptance Criteria -- [ ] The `net10.0` assets of `Light.PortableResults` and `Light.PortableResults.Validation` build with `IsAotCompatible`, and the `netstandard2.0` assets build without `NETSDK1210`. -- [ ] Both packages build clean under `Release`, where `TreatWarningsAsErrors` turns any future IL2026/IL3050 into a build failure. No warning is resolved by disabling a rule in `.editorconfig` or `NoWarn`. -- [ ] A consumer never names a library-owned type in their own `JsonSerializerContext`. Registering the result value type is sufficient for CloudEvents write, CloudEvents read, and HTTP read; the library resolves contracts for its own envelope and payload types itself. -- [ ] A non-generic `Result` and a `Result` round-trip over CloudEvents and HTTP against options whose only resolver is a consumer context declaring the value type alone, proving the paths need no runtime code generation. Tests assert this in-process by resolver composition, not by toggling the reflection switch. -- [ ] Library-owned contracts are created once per `JsonSerializerOptions` instance, not per serialization call, and creation works against options that are already read-only. -- [ ] When the consumer's value type is unresolvable, every affected entry point throws an exception that names the unresolved type and states the remedy, and a negative test pins that behavior per site. The unreachable guards in `SystemTextJsonWritingExtensions`, `LightResult`, and `LightActionResult` are converted so they can fire. -- [ ] Behavior for consumers who pass reflection-backed options is unchanged: the existing test suites pass without modification beyond additions, and `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` keep serializing arbitrary `T` as they do today. -- [ ] Converters supplied through `JsonSerializerOptions` still take precedence over the library's own, including replacements inserted ahead of the defaults. The library-created contract is reached only when the configured resolver cannot supply one. -- [ ] CI publishes the Native AOT sample and fails on any trim or AOT diagnostic originating in a Light.PortableResults assembly, while the sample's unannotated package dependencies stay non-fatal. The sample's blanket `SuppressTrimAnalysisWarnings` is gone, and the replacement is demonstrated to fail the publish on an injected library-side regression. -- [ ] The README documents how to compose options for Native AOT, and the `` of every affected package describe their respective changes. +- [ ] The `net10.0` assets of `Light.PortableResults` and `Light.PortableResults.Validation` build with `IsAotCompatible`; their `netstandard2.0` assets build without `NETSDK1210`. +- [ ] Both packages build clean in `Release`, where future IL2026/IL3050 diagnostics fail the build. No rule is disabled through `.editorconfig` or `NoWarn`. +- [ ] Consumers register only their result value types in `JsonSerializerContext`; the library resolves its own CloudEvents write, CloudEvents read, and HTTP read types. +- [ ] With a source-generated resolver declaring only the value type, non-generic `Result` and generic `Result` round-trip over CloudEvents and HTTP in process without a reflection-backed resolver. +- [ ] Library-owned contracts are created once per `JsonSerializerOptions` instance and can be created after the options become read-only. +- [ ] Missing consumer value-type metadata produces an exception that names the type and remedy at every affected entry point, with a negative test per site. The unreachable guards in `SystemTextJsonWritingExtensions`, `LightResult`, and `LightActionResult` are repaired. +- [ ] Reflection-backed behavior remains unchanged: existing tests require additions only, the shared default options continue to support arbitrary `T`, and converters configured ahead of the library defaults retain precedence. Library-created contracts are used only when the configured resolver cannot supply one. +- [ ] CI publishes the Native AOT sample, fails on trim or AOT diagnostics from Light.PortableResults assemblies, and keeps diagnostics from unannotated package dependencies non-fatal. The replacement for `SuppressTrimAnalysisWarnings` is proven with an injected library-side regression. +- [ ] The README documents Native AOT option composition, and every affected package updates ``. ## Technical Details -### Enabling the analyzers +### Analyzer configuration and warning triage -`IsAotCompatible` is unsupported on `netstandard2.0` and raises `NETSDK1210` when set unconditionally. Use the SDK's own recommended form in both multi-targeted projects: +Set `IsAotCompatible` only on compatible target frameworks in both multi-targeted projects: ```xml true ``` -This is independent of the existing `PublishAot=false` and `TreatAsLocalProperty="PublishAot"` settings, which stay as they are. `Light.PortableResults.Validation` is clean today; enabling it there buys regression protection only, at no triage cost. +Keep the existing `PublishAot=false` and `TreatAsLocalProperty="PublishAot"` settings. `Light.PortableResults.Validation` is currently clean; enabling its analyzers provides regression protection. -Measured warning distribution in `Light.PortableResults` before any fix — use it to confirm the triage is complete: +Use the measured baseline to confirm complete triage: | File | IL2026 | IL3050 | | --- | ---: | ---: | @@ -42,88 +39,56 @@ Measured warning distribution in `Light.PortableResults` before any fix — use | `CloudEvents/Writing/CloudEventsResultExtensions.cs` | 4 | 4 | | `Http/Reading/Json/ResultJsonReader.cs` | 2 | 2 | -### Resolving the warnings: route through `JsonTypeInfo`, do not annotate - -Every warning is a call to a `JsonSerializer.Serialize`/`Deserialize(…, JsonSerializerOptions)` overload. The payload type is statically known at each site; only the resolver is the caller's. The fix is to resolve a `JsonTypeInfo` from the options and call the `JsonTypeInfo` overload, which is analyzer-clean. `SystemTextJsonWritingExtensions`, `LightResult`, and `LightActionResult` already do this, but resolve through `options.GetTypeInfo(typeof(T))` — see the next section before copying them. - -This is the decision that shapes the rest of the work, so record why the alternative was rejected. Annotating the public entry points with `[RequiresDynamicCode]`/`[RequiresUnreferencedCode]` would be permanently wrong: these methods do not intrinsically require dynamic code, they require the caller's options to carry a resolver. Annotating would force a suppression on every correctly configured AOT consumer while doing nothing for an incorrectly configured one. Blanket `UnconditionalSuppressMessage` at the call sites is equally wrong here — it silences a diagnostic that is currently accurate. The four existing factory suppressions stay untouched: `MakeGenericType` over a resolved generic is genuinely safe when the closed type is registered, which is what their justifications already state. - -### Resolve with `TryGetTypeInfo`, and repair the existing guards - -`JsonSerializerOptions.GetTypeInfo` does not return `null` for an unknown type — it throws `NotSupportedException` first. Verified against a source-generated resolver declaring only `string`: - -``` -GetTypeInfo(typeof(HttpResultForWriting)) - -> NotSupportedException: JsonTypeInfo metadata for type '…HttpResultForWriting' was not provided by - TypeInfoResolver of type 'OnlyStringContext'. … -TryGetTypeInfo(typeof(HttpResultForWriting), out var info) - -> false, info is null -``` - -So a `GetTypeInfo` call followed by a null check or a failed-cast check can never produce the intended exception; the guard is unreachable for the missing-type case, which is the only case that occurs in practice. Three existing guards are dead code for this reason and must be converted as part of this work, not copied: `SystemTextJsonWritingExtensions` (the `is null` check after `GetTypeInfo`; the `TryGetTypeInfo` call later in the same method is already correct and is the model), `LightResult`, and `LightActionResult`. - -Resolve through `options.TryGetTypeInfo(typeof(T), out var typeInfo)` and translate the negative result into an `InvalidOperationException` that names the unresolved type and states the remedy. Keep `InvalidOperationException` for consistency with the existing wording in `LightResult`, and keep the message specific to what the caller must do — register their value type with the context supplied to these options. `System.Text.Json`'s own message names the type and the resolver but cannot mention the library-specific setup, and the "reflection has been disabled" variant names no type at all. - -Every converted site needs a negative test asserting the exception type and that the message names the unresolved type; without it the guards stay untested, which is how the current three came to be unreachable. - -### Library-owned contracts, not resolver composition +Route each warned serialization call through a `JsonTypeInfo` overload. Do not annotate public entry points with `[RequiresDynamicCode]` or `[RequiresUnreferencedCode]`: the operations require a configured resolver, not intrinsic dynamic code. Do not suppress these call sites either. The four existing, justified `MakeGenericType` factory suppressions remain unchanged. -Resolver composition cannot carry this. `JsonTypeInfoResolver.Combine` queries each resolver in order and returns the first contract that matches the requested type; it never synthesizes `CloudEventsEnvelopeForWriting` from a contract for the envelope and another for `MyDto`. Verified: with a consumer context declaring only `string`, the current write path throws `NotSupportedException` for `CloudEventsEnvelopeForWriting`. Shipping library contexts and combining them would therefore not spare consumers from declaring every closed wrapper themselves, in the right direction-specific shape — a requirement that leaks library-internal types into consumer code and breaks whenever a wrapper is added. +### Contract resolution and error behavior -Take the other route: when `System.Text.Json` cannot resolve a contract for a library-owned type, let the library supply one. Every such type is already serialized by a library-owned converter, so the consumer's context only ever needs their own value type. Verified against a consumer context declaring `string` alone, with reflection disabled: a direct `writer.WriteCloudEvents(envelope, options.SerializerOptions)` writes a full CloudEvent including metadata, and a contract built with `JsonMetadataServices.CreateValueInfo>(options, new HttpReadAutoSuccessResultPayloadJsonConverter())` deserializes correctly. `CreateValueInfo` is the source generator's own building block, involves no reflection, and both shapes are analyzer-clean under `IsAotCompatible=true`. +Use `JsonSerializerOptions.TryGetTypeInfo`; `GetTypeInfo` throws `NotSupportedException` for a missing contract before a null or failed-cast guard can run. Convert the existing unreachable guards in `SystemTextJsonWritingExtensions`, `LightResult`, and `LightActionResult`, and use the same pattern at new sites. A failed lookup becomes `InvalidOperationException` naming the unresolved consumer type and instructing the caller to register it in the context supplied to the options. Negative tests assert the exception type and unresolved type at every converted site. -**The library-created contract must be the fallback, never the first choice.** Hardcoding the library converter would ignore converters supplied through `JsonSerializerOptions`, and existing tests depend on exactly that: `HttpResponseMessageExtensionsTests` inserts `NullBareStringPayloadConverter` and `EmptyFailurePayloadConverter` at index 0 and asserts the read path uses them. Resolve in this order at every converted site: +Combining source-generated resolvers cannot synthesize a closed contract such as `CloudEventsEnvelopeForWriting` from separate contracts for the wrapper and `MyDto`. Do not ship library contexts that force consumers to declare closed library wrappers. Instead, resolve every library-owned wrapper in this order: -1. If `options.TypeInfoResolver is null` and `JsonSerializer.IsReflectionEnabledByDefault`, call `options.MakeReadOnly(populateMissingResolver: true)`. -2. If `options.TryGetTypeInfo(typeof(TWrapper), out var info)` and `info is JsonTypeInfo typed`, serialize with `typed`. -3. Otherwise use the cached library-created contract, or the direct writer on the CloudEvents write path. +1. If `options.TypeInfoResolver` is null and `JsonSerializer.IsReflectionEnabledByDefault` is true, call `options.MakeReadOnly(populateMissingResolver: true)`. +2. Use a typed contract returned by `options.TryGetTypeInfo(typeof(TWrapper), ...)` when available. +3. Otherwise, select a registered converter using normal `JsonSerializerOptions` precedence, including replacements inserted before the defaults; fall back to the library converter only when none matches. Cache the resulting library-owned contract or converter per `JsonSerializerOptions` instance and closed wrapper type. -Step 1 is not optional, and it is the step that is easy to miss. The library's own default options never assign a `TypeInfoResolver` — `CreateDefaultSerializerOptions` only adds converters — so `TryGetTypeInfo` returns `false` on them until a resolver is materialized, and without step 1 every reflection-backed caller would silently route to the fallback and lose their replacement converters. Verified: on default HTTP read options with a replacement converter at index 0, `TryGetTypeInfo` returns `false` beforehand and `true` afterwards, and the replacement converter is the one that runs. +The ordering is load-bearing, not a preference: `HttpResponseMessageExtensionsTests` inserts `NullBareStringPayloadConverter` and `EmptyFailurePayloadConverter` at index 0 and asserts the read path uses them, so reaching for a library-created contract first fails those tests. -`MakeReadOnly(bool)` carries IL2026 and IL3050 because it may construct the reflection resolver. Guarding the call with `JsonSerializer.IsReflectionEnabledByDefault` makes it unreachable when reflection is off, which is precisely what that property exists for, so suppress both codes there with that justification. This is the one place in the new design where a suppression is correct. Freezing the options is not a behavior change: the paths that reach step 1 are about to serialize with them, which freezes them anyway. +Step 1 preserves current reflection-backed behavior: default options contain converters but no explicit resolver, and `TryGetTypeInfo` cannot expose their contracts until the reflection resolver is materialized. `MakeReadOnly(bool)` carries IL2026 and IL3050; narrowly suppress both at this guarded call because `JsonSerializer.IsReflectionEnabledByDefault` is a link-time constant and makes the call unreachable when reflection is disabled. Freezing here is not an additional behavior change because serialization would freeze the options immediately afterward. -Under AOT the sequence degrades exactly as intended: step 1 is skipped, step 2 fails for a consumer context declaring only `T`, and step 3 carries the call. Under reflection, step 2 always wins and behavior is unchanged. +For read fallbacks, build the cached contract with `JsonMetadataServices.CreateValueInfo(options, converter)` and use the `JsonTypeInfo` serializer overload. Contract creation is reflection-free, analyzer-clean, and works with read-only options. For CloudEvents write fallback, invoke the selected converter directly; the library converter already delegates to `writer.WriteCloudEvents(envelope, options.SerializerOptions)`. This preserves custom converter precedence without resolving a wrapper contract. -Two constraints on step 3. `CreateValueInfo` allocates a `JsonTypeInfo` per call, so cache per `JsonSerializerOptions` instance — a static per-closed-type cache keyed by options, not a new contract on every serialization; this library does not allocate on hot paths. Creation against already-read-only options is safe, so the cache may be lazy: verified after an explicit `MakeReadOnly()`, which is the state a long-lived `Default` instance reaches after first use. And even in the fallback, prefer a converter registered on `options.Converters` for the wrapper type over the library default, so a consumer who replaces a converter is honored under AOT too. +With reflection-backed options, step 2 preserves existing behavior. With a source-generated resolver that declares only the consumer's `T`, wrapper lookup fails and step 3 handles the library type; nested value serialization still resolves `T` from the consumer context. The shared defaults remain reflection-backed so arbitrary consumer types continue to work, and non-generic results should then require no consumer registration at all — confirm that during implementation rather than assuming it, and document the outcome in the README. -Four factories carry `MakeGenericType` suppressions and all stay public and unchanged: `CloudEventsEnvelopeForWritingJsonConverterFactory`, `CloudEventsSuccessPayloadJsonConverterFactory`, and `HttpReadSuccessResultPayloadJsonConverterFactory` sit on the three paths this work touches and continue to serve step 2, while `HttpResultForWritingJsonConverterFactory` is on the HTTP write path and is not involved at all. +All four factory types and their suppressions remain public and unchanged: the CloudEvents write, CloudEvents read, and HTTP read factories continue serving resolver-backed contracts, while the unrelated HTTP write factory is unaffected. -This also improves the defaults. `PortableResultsCloudEventsWriteOptions.Default` and `PortableResultsHttpReadOptions.Default` stay reflection-backed for the consumer's `T` — no library can supply that contract, and putting a source-generated resolver on the shared defaults would silently stop arbitrary `T` from serializing for the non-AOT majority. But once the library owns its own contracts, the defaults stop failing on library-owned types, so a non-generic `Result` should need no consumer registration at all. Confirm that during implementation and state it in the README; it is the difference between "AOT needs setup" and "AOT needs setup proportional to your own payload types". +### Tests and CI -### Proving it, in tests and in CI +In-process tests use options whose only resolver is a test-local source-generated context declaring the value type. Round-trip non-generic and generic results over CloudEvents and HTTP, verify configured replacement converters still win, verify contract caching and read-only options, and cover the missing-type errors. Do not toggle `JsonSerializer.IsReflectionEnabledByDefault` inside the shared parallel test process; standalone reproduction may use `false`. -The in-process tests are the primary proof for this issue; the ILC publish is regression protection for the paths the sample already walks. +The Native AOT sample currently exercises only the already-working Minimal API write path. Its publish remains an ILC regression gate for this change; extending it to CloudEvents write and HTTP read is deferred to a maintainer-driven follow-up. -In-process tests compose options whose only resolver is a test-local source-generated context declaring the value type and nothing else, and assert that a non-generic `Result` and a `Result` round-trip over CloudEvents and HTTP. That the context declares no library-owned type is itself part of the assertion. A path that completes with no reflection-based resolver in the graph cannot have needed runtime code generation, which is the property under test. Do not toggle `System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault` from a test: the value is read once and cached per process, and the test hosts are shared and parallel. The switch remains the right manual reproduction, via a standalone console app with `false`, and that is how the failures in the Rationale were confirmed. - -The ILC publish covers `samples/NativeAotMovieRating`, which exercises the Minimal API write path exclusively — the path that already works. It therefore does not verify the CloudEvents write or HTTP read paths in this release; extending the sample to walk them is deferred to a follow-up the maintainer will drive. Keep the gate anyway: it proves ILC still succeeds after the `IsAotCompatible` and `JsonTypeInfo` changes, and it is the seam the later sample work plugs into. - -For the gate to report anything at all from this library, one sample setting has to change. The sample sets `SuppressTrimAnalysisWarnings=true` to silence Serilog's unannotated assembly rollup, which equally hides every warning originating in Light.PortableResults. Drop it, and keep single-warning mode rather than turning it off: +Remove `SuppressTrimAnalysisWarnings=true` from the sample and retain single-warning mode for package references: ```xml - + $(WarningsNotAsErrors);IL2104 ``` -Do not set `TrimmerSingleWarn=false`. Verified on this sample: it expands Serilog into individual diagnostics, `Serilog.Capturing.PropertyValueConverter.TryConvertStructure` raises `IL2067`, the repository's Release `TreatWarningsAsErrors` promotes it to an error, and ILC fails with `MSB3077`. Suppressing that through `NoWarn` is not an option either — `IL2067` is not Serilog-specific, and silencing it globally would hide the same diagnostic in Light assemblies. +Do not set `TrimmerSingleWarn=false`: it expands Serilog to IL2067, which Release promotes to an error, while globally suppressing IL2067 could hide the same defect in Light assemblies. The proposed configuration was verified by injecting a reflection-based serialization call into `Light.PortableResults`: Serilog remained a non-fatal IL2104 rollup, while the Light project reference emitted fatal IL2026/IL3050 diagnostics with source locations. -The configuration above is not a blunt suppression, because the SDK applies single-warn to package references while project references always report in detail. Verified end to end by injecting a `JsonSerializer.Serialize(value, value.GetType())` call into `Light.PortableResults` and publishing the sample: +Add a separate post-test step to `build-and-test.yml` that publishes the sample on the runner RID: -| Origin | Reported as | Fatal | -| --- | --- | --- | -| `Serilog.dll` (package reference) | one `IL2104` rollup | no | -| `Light.PortableResults` (project reference) | `IL3050` and `IL2026`, with file and line | yes | - -The injected regression failed the publish while Serilog stayed quiet, which is exactly the asymmetry the gate needs. Note this also holds today, before the analyzers are enabled: `IsAotCompatible` governs compile-time analysis, so ILC is the only thing that would have caught such a call in the current codebase. +```shell +dotnet publish -c Release -r +``` -Add the publish to `build-and-test.yml` as a separate step after the existing test steps, on the runner's own RID. Publish the sample project directly and do not pass `/p:PublishAot=true` on the command line: as a global property it flows into every project reference and fails the `netstandard2.0` source generator with `NETSDK1207`. The sample already sets `PublishAot` internally and strips it from its references via `GlobalPropertiesToRemove`, which is why `dotnet publish -c Release -r ` is the correct invocation. It is the slowest step in the workflow; keep it out of the matrix and do not gate the coverage jobs on it. +Do not pass `/p:PublishAot=true`: as a global property it reaches project references and causes `NETSDK1207` in the `netstandard2.0` source generator. The sample already sets `PublishAot` and removes it from project references through `GlobalPropertiesToRemove`. Keep this slow publish outside any matrix and do not make coverage jobs depend on it. -### Deliberately out of scope +Deliberately out of scope: -- **Native AOT support for `Light.PortableResults.AspNetCore.Mvc`.** MVC is not Native AOT compatible, and its package description does not claim otherwise, so it gets no `IsAotCompatible` and no analyzer triage. The unreachable guard in `LightActionResult` is still repaired: it is the same defect as the other two and is not an AOT concern. -- **Putting a source-generated resolver on the shared `Default` options.** Rejected above: only the consumer's context can supply a contract for their `T`, and a source-generated resolver on the shared defaults would break arbitrary `T` for the non-AOT majority. -- **Shipping `JsonSerializerContext` types for library-owned payloads.** Superseded — with the library building its own contracts from its own converters, there is nothing left for a shipped context to declare. Add one only if triage finds a site where converter-backed contract creation is impractical, and record why. -- **Extending the Native AOT sample to exercise the CloudEvents write and HTTP read paths.** Deferred to a follow-up the maintainer will drive. Until then the ILC gate covers the Minimal API write path only, and the in-process resolver-composition tests carry the proof for the paths this issue fixes. -- **Trim-only (`PublishTrimmed`) verification as a separate gate.** The AOT publish subsumes the trim analyzer for this code; a dedicated trimmed-but-not-AOT configuration would add a second slow CI step for no additional signal. -- **The `default(Result)` write guard** and **`EnablePackageValidation`.** Tracked separately as items 2 and 3 of #77. +- Native AOT support for `Light.PortableResults.AspNetCore.Mvc`; only its shared unreachable guard is repaired. +- A source-generated resolver on the shared default options. +- Shipped `JsonSerializerContext` types for library-owned payloads. +- Extending the Native AOT sample to exercise CloudEvents write and HTTP read. +- A separate trim-only publish gate. +- The `default(Result)` write guard and `EnablePackageValidation`, tracked as items 2 and 3 of #77. From 465dc97348e717568183f0b4829c757143ef6a7b Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Sun, 2 Aug 2026 22:18:44 +0200 Subject: [PATCH 08/15] feat: introduce Native AOT compatibility checks Signed-off-by: Kenny Pflug --- .github/workflows/build-and-test.yml | 8 + README.md | 74 ++++ ai-plans/0078-aot-compatibility-checks.md | 20 +- .../NativeAotMovieRating.csproj | 10 +- ...tableResults.AspNetCore.MinimalApis.csproj | 3 + .../LightResult.cs | 15 +- .../packages.lock.json | 2 +- ...ight.PortableResults.AspNetCore.Mvc.csproj | 4 + .../LightActionResult.cs | 15 +- .../packages.lock.json | 2 +- .../packages.lock.json | 6 +- .../Light.PortableResults.Validation.csproj | 10 + .../PortableResultsValidationModule.cs | 12 +- .../packages.lock.json | 6 + .../ReadOnlyMemoryCloudEventsExtensions.cs | 73 ++-- .../Writing/CloudEventsResultExtensions.cs | 16 +- .../Reading/HttpResponseMessageExtensions.cs | 58 ++- .../Http/Reading/Json/ResultJsonReader.cs | 6 +- .../Light.PortableResults.csproj | 12 + .../PortableResultsJsonContracts.cs | 352 +++++++++++++++++ .../Writing/ErrorsExtensions.cs | 15 +- .../SystemTextJsonWritingExtensions.cs | 16 +- src/Light.PortableResults/packages.lock.json | 6 + .../LightResultContractGuardTests.cs | 82 ++++ .../LightActionResultContractGuardTests.cs | 87 +++++ .../PortableResultsJsonContractsTests.cs | 366 ++++++++++++++++++ .../ConverterPrecedenceTests.cs | 181 +++++++++ .../MissingValueTypeMetadataTests.cs | 140 +++++++ .../SourceGeneratedSerialization/MovieDto.cs | 7 + .../MovieJsonContext.cs | 24 ++ .../SourceGeneratedRoundTripTests.cs | 293 ++++++++++++++ 31 files changed, 1832 insertions(+), 89 deletions(-) create mode 100644 src/Light.PortableResults/SharedJsonSerialization/PortableResultsJsonContracts.cs create mode 100644 tests/Light.PortableResults.AspNetCore.MinimalApis.Tests/LightResultContractGuardTests.cs create mode 100644 tests/Light.PortableResults.AspNetCore.Mvc.Tests/LightActionResultContractGuardTests.cs create mode 100644 tests/Light.PortableResults.Tests/SharedJsonSerialization/PortableResultsJsonContractsTests.cs create mode 100644 tests/Light.PortableResults.Tests/SourceGeneratedSerialization/ConverterPrecedenceTests.cs create mode 100644 tests/Light.PortableResults.Tests/SourceGeneratedSerialization/MissingValueTypeMetadataTests.cs create mode 100644 tests/Light.PortableResults.Tests/SourceGeneratedSerialization/MovieDto.cs create mode 100644 tests/Light.PortableResults.Tests/SourceGeneratedSerialization/MovieJsonContext.cs create mode 100644 tests/Light.PortableResults.Tests/SourceGeneratedSerialization/SourceGeneratedRoundTripTests.cs diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index b0f45e8..8624dbf 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -61,6 +61,14 @@ jobs: --configuration Release -p:PortableResultsAssetTargetFramework=netstandard2.0 -p:ContinuousIntegrationBuild=true + # ILC regression gate for the Native AOT compatibility claim. Trim and AOT diagnostics from the + # Light.PortableResults project references are fatal in Release, while rollups from unannotated package + # dependencies stay non-fatal through the WarningsNotAsErrors setting in the sample project. + # PublishAot must not be passed on the command line: as a global property it reaches project references + # and causes NETSDK1207 in the netstandard2.0 source generator. The sample sets it itself and removes it + # from its project references through GlobalPropertiesToRemove. + - name: Publish Native AOT sample + run: dotnet publish ./samples/NativeAotMovieRating/NativeAotMovieRating.csproj -c Release -r linux-x64 coverage-comment: if: always() && github.event_name == 'pull_request' diff --git a/README.md b/README.md index c00b3b0..e3d25e0 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,7 @@ Most Result Pattern libraries stop at the application boundary. Light.PortableRe - [CloudEvents Quick Start](#cloudevents-quick-start) - [Validation In Depth](#validation-in-depth) - [OpenAPI Support](#openapi-support) +- [Native AOT and Trimming](#native-aot-and-trimming) - [Configuration Reference](#configuration-reference) @@ -1315,6 +1316,79 @@ app.MapPut("/api/movieRatings", handler) ); ``` + + +## 🛩️ Native AOT and Trimming + +`Light.PortableResults` and `Light.PortableResults.Validation` ship `net10.0` assets that are built with +`IsAotCompatible`, and CI publishes a Native AOT sample so that trim and AOT regressions fail the build. + +### What you have to register + +You only register **your own result value types** with a `JsonSerializerContext`. Every envelope and payload type +that Light.PortableResults owns is resolved by the library itself, so you never declare closed library wrappers such +as `CloudEventsEnvelopeForWriting` or `HttpReadAutoSuccessResultPayload`. Combining +source-generated resolvers cannot synthesize those closed generics from separate contracts anyway. + +```csharp +using System.Text.Json; +using System.Text.Json.Serialization; + +[JsonSourceGenerationOptions(JsonSerializerDefaults.Web)] +[JsonSerializable(typeof(MovieRating))] +public sealed partial class MovieRatingJsonContext : JsonSerializerContext; +``` + +Compose the options by setting the resolver and then adding the Light.PortableResults converters: + +```csharp +using Light.PortableResults.CloudEvents.Reading; +using Light.PortableResults.CloudEvents.Writing; +using Light.PortableResults.Http.Reading; + +var serializerOptions = new JsonSerializerOptions(JsonSerializerDefaults.Web) +{ + TypeInfoResolver = MovieRatingJsonContext.Default +}; +serializerOptions.AddDefaultPortableResultsCloudEventsWriteJsonConverters(); + +var writeOptions = new PortableResultsCloudEventsWriteOptions +{ + SerializerOptions = serializerOptions, + Source = "urn:movies:rating-service" +}; + +byte[] cloudEvent = result.ToCloudEvent(successType: "movie.rated", options: writeOptions); +``` + +The same composition applies to `AddDefaultPortableResultsCloudEventsReadJsonConverters` and +`AddDefaultPortableResultsHttpReadJsonConverters`. Non-generic `Result` values require **no** consumer registration +at all: every type involved in writing and reading them belongs to Light.PortableResults. + +### How a contract is resolved + +For each library-owned type, `PortableResultsJsonContracts` resolves in this order: + +1. When the options carry no resolver and reflection-based serialization is enabled, the reflection resolver is + materialized so that the shared default options keep working exactly as before. +2. A contract supplied by the configured `TypeInfoResolver` wins. Declaring a library type in your own context + therefore still overrides the library. +3. Otherwise the converter that the options select for the type is used, following the normal + `JsonSerializerOptions` precedence — the first entry in `Converters` that can convert the type. A converter you + insert **before** the Light.PortableResults defaults keeps its precedence; the library converter is only reached + when nothing else matches. Contracts created this way are cached per `JsonSerializerOptions` instance and can be + created after the options became read-only. + +If a value type is missing from your context, the affected entry point throws an `InvalidOperationException` that +names the unresolved type and tells you to register it, instead of failing deep inside `System.Text.Json`. + +### HTTP writing with ASP.NET Core + +The HTTP write path is the exception: `LightResult`/`LightActionResult` serialize `HttpResultForWriting` and +`HttpResultForWriting` through the configured resolver, so those wrappers still have to be declared in your +context — see the `samples/NativeAotMovieRating` project. `Light.PortableResults.AspNetCore.Mvc` is not Native AOT +compatible. + ## ⚙️ Configuration Reference diff --git a/ai-plans/0078-aot-compatibility-checks.md b/ai-plans/0078-aot-compatibility-checks.md index 4ec3dd3..2ddf8af 100644 --- a/ai-plans/0078-aot-compatibility-checks.md +++ b/ai-plans/0078-aot-compatibility-checks.md @@ -8,15 +8,15 @@ The warnings represent runtime failures: with reflection serialization disabled, ## Acceptance Criteria -- [ ] The `net10.0` assets of `Light.PortableResults` and `Light.PortableResults.Validation` build with `IsAotCompatible`; their `netstandard2.0` assets build without `NETSDK1210`. -- [ ] Both packages build clean in `Release`, where future IL2026/IL3050 diagnostics fail the build. No rule is disabled through `.editorconfig` or `NoWarn`. -- [ ] Consumers register only their result value types in `JsonSerializerContext`; the library resolves its own CloudEvents write, CloudEvents read, and HTTP read types. -- [ ] With a source-generated resolver declaring only the value type, non-generic `Result` and generic `Result` round-trip over CloudEvents and HTTP in process without a reflection-backed resolver. -- [ ] Library-owned contracts are created once per `JsonSerializerOptions` instance and can be created after the options become read-only. -- [ ] Missing consumer value-type metadata produces an exception that names the type and remedy at every affected entry point, with a negative test per site. The unreachable guards in `SystemTextJsonWritingExtensions`, `LightResult`, and `LightActionResult` are repaired. -- [ ] Reflection-backed behavior remains unchanged: existing tests require additions only, the shared default options continue to support arbitrary `T`, and converters configured ahead of the library defaults retain precedence. Library-created contracts are used only when the configured resolver cannot supply one. -- [ ] CI publishes the Native AOT sample, fails on trim or AOT diagnostics from Light.PortableResults assemblies, and keeps diagnostics from unannotated package dependencies non-fatal. The replacement for `SuppressTrimAnalysisWarnings` is proven with an injected library-side regression. -- [ ] The README documents Native AOT option composition, and every affected package updates ``. +- [x] The `net10.0` assets of `Light.PortableResults` and `Light.PortableResults.Validation` build with `IsAotCompatible`; their `netstandard2.0` assets build without `NETSDK1210`. +- [x] Both packages build clean in `Release`, where future IL2026/IL3050 diagnostics fail the build. No rule is disabled through `.editorconfig` or `NoWarn`. +- [x] Consumers register only their result value types in `JsonSerializerContext`; the library resolves its own CloudEvents write, CloudEvents read, and HTTP read types. +- [x] With a source-generated resolver declaring only the value type, non-generic `Result` and generic `Result` round-trip over CloudEvents and HTTP in process without a reflection-backed resolver. +- [x] Library-owned contracts are created once per `JsonSerializerOptions` instance and can be created after the options become read-only. +- [x] Missing consumer value-type metadata produces an exception that names the type and remedy at every affected entry point, with a negative test per site. The unreachable guards in `SystemTextJsonWritingExtensions`, `LightResult`, and `LightActionResult` are repaired. +- [x] Reflection-backed behavior remains unchanged: existing tests require additions only, the shared default options continue to support arbitrary `T`, and converters configured ahead of the library defaults retain precedence. Library-created contracts are used only when the configured resolver cannot supply one. +- [x] CI publishes the Native AOT sample, fails on trim or AOT diagnostics from Light.PortableResults assemblies, and keeps diagnostics from unannotated package dependencies non-fatal. The replacement for `SuppressTrimAnalysisWarnings` is proven with an injected library-side regression. +- [x] The README documents Native AOT option composition, and every affected package updates ``. ## Technical Details @@ -43,7 +43,7 @@ Route each warned serialization call through a `JsonTypeInfo` overload. Do no ### Contract resolution and error behavior -Use `JsonSerializerOptions.TryGetTypeInfo`; `GetTypeInfo` throws `NotSupportedException` for a missing contract before a null or failed-cast guard can run. Convert the existing unreachable guards in `SystemTextJsonWritingExtensions`, `LightResult`, and `LightActionResult`, and use the same pattern at new sites. A failed lookup becomes `InvalidOperationException` naming the unresolved consumer type and instructing the caller to register it in the context supplied to the options. Negative tests assert the exception type and unresolved type at every converted site. +Use `JsonSerializerOptions.TryGetTypeInfo`; `GetTypeInfo` throws `NotSupportedException` for a missing contract before a null or failed -cast guard can run. Convert the existing unreachable guards in `SystemTextJsonWritingExtensions`, `LightResult`, and `LightActionResult`, and use the same pattern at new sites. A failed lookup becomes `InvalidOperationException` naming the unresolved consumer type and instructing the caller to register it in the context supplied to the options. Negative tests assert the exception type and unresolved type at every converted site. Combining source-generated resolvers cannot synthesize a closed contract such as `CloudEventsEnvelopeForWriting` from separate contracts for the wrapper and `MyDto`. Do not ship library contexts that force consumers to declare closed library wrappers. Instead, resolve every library-owned wrapper in this order: diff --git a/samples/NativeAotMovieRating/NativeAotMovieRating.csproj b/samples/NativeAotMovieRating/NativeAotMovieRating.csproj index e081e88..01aa9cd 100644 --- a/samples/NativeAotMovieRating/NativeAotMovieRating.csproj +++ b/samples/NativeAotMovieRating/NativeAotMovieRating.csproj @@ -5,8 +5,14 @@ true false $(InterceptorsNamespaces);Microsoft.AspNetCore.OpenApi.Generated - - true + + $(WarningsNotAsErrors);IL2104 false diff --git a/src/Light.PortableResults.AspNetCore.MinimalApis/Light.PortableResults.AspNetCore.MinimalApis.csproj b/src/Light.PortableResults.AspNetCore.MinimalApis/Light.PortableResults.AspNetCore.MinimalApis.csproj index 4ce2423..874036c 100644 --- a/src/Light.PortableResults.AspNetCore.MinimalApis/Light.PortableResults.AspNetCore.MinimalApis.csproj +++ b/src/Light.PortableResults.AspNetCore.MinimalApis/Light.PortableResults.AspNetCore.MinimalApis.csproj @@ -11,6 +11,9 @@ - Easy integration into ASP.NET Core's composition root via IServiceCollection.AddPortableResultsForMinimalApis. - OpenAPI helpers and schema-only CLR surrogate types were removed; use Light.PortableResults.AspNetCore.OpenApi for OpenAPI integration. - Compatible with .NET Native AOT. + - LightResult and LightResult<T> now report an unresolvable HttpResultForWriting contract as an + InvalidOperationException naming the closed wrapper type. The previous guard was unreachable because + JsonSerializerOptions.GetTypeInfo threw a NotSupportedException first. diff --git a/src/Light.PortableResults.AspNetCore.MinimalApis/LightResult.cs b/src/Light.PortableResults.AspNetCore.MinimalApis/LightResult.cs index 0dfa8a6..534386b 100644 --- a/src/Light.PortableResults.AspNetCore.MinimalApis/LightResult.cs +++ b/src/Light.PortableResults.AspNetCore.MinimalApis/LightResult.cs @@ -5,6 +5,7 @@ using System.Threading.Tasks; using Light.PortableResults.AspNetCore.MinimalApis.Serialization; using Light.PortableResults.Http.Writing; +using Light.PortableResults.SharedJsonSerialization; using Microsoft.AspNetCore.Http; namespace Light.PortableResults.AspNetCore.MinimalApis; @@ -47,11 +48,12 @@ ResolvedHttpWriteOptions resolvedOptions var serializerOptions = httpContext.RequestServices.ResolveJsonSerializerOptions(SerializerOptions); var wrapper = enrichedResult.ToHttpResultForWriting(resolvedOptions); - var typeInfo = serializerOptions.GetTypeInfo(typeof(HttpResultForWriting)); - if (typeInfo is not JsonTypeInfo castTypeInfo) + PortableResultsJsonContracts.EnsureTypeInfoResolver(serializerOptions); + if (!serializerOptions.TryGetTypeInfo(typeof(HttpResultForWriting), out var typeInfo) || + typeInfo is not JsonTypeInfo castTypeInfo) { throw new InvalidOperationException( - "Could not resolve 'JsonTypeInfo'. Please ensure that your JsonSerializerOptions are configured correctly. The AddDefaultLightResultsHttpWriteJsonConverters extension method can help you with this." + $"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'. Please ensure that your JsonSerializerOptions are configured correctly. The AddDefaultLightResultsHttpWriteJsonConverters extension method can help you with this." ); } @@ -103,11 +105,12 @@ ResolvedHttpWriteOptions resolvedOptions var serializerOptions = httpContext.RequestServices.ResolveJsonSerializerOptions(SerializerOptions); var wrapper = enrichedResult.ToHttpResultForWriting(resolvedOptions); - var typeInfo = serializerOptions.GetTypeInfo(typeof(HttpResultForWriting)); - if (typeInfo is not JsonTypeInfo> castTypeInfo) + PortableResultsJsonContracts.EnsureTypeInfoResolver(serializerOptions); + if (!serializerOptions.TryGetTypeInfo(typeof(HttpResultForWriting), out var typeInfo) || + typeInfo is not JsonTypeInfo> castTypeInfo) { throw new InvalidOperationException( - $"Could not resolve 'JsonTypeInfo>'. Please ensure that your JsonSerializerOptions are configured correctly. The AddDefaultLightResultsHttpWriteJsonConverters extension method can help you with this." + $"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'. Please ensure that your JsonSerializerOptions are configured correctly. The AddDefaultLightResultsHttpWriteJsonConverters extension method can help you with this." ); } diff --git a/src/Light.PortableResults.AspNetCore.MinimalApis/packages.lock.json b/src/Light.PortableResults.AspNetCore.MinimalApis/packages.lock.json index 315c35f..05fbdb1 100644 --- a/src/Light.PortableResults.AspNetCore.MinimalApis/packages.lock.json +++ b/src/Light.PortableResults.AspNetCore.MinimalApis/packages.lock.json @@ -47,7 +47,7 @@ "light.portableresults.aspnetcore.shared": { "type": "Project", "dependencies": { - "Light.PortableResults": "[0.6.0, )" + "Light.PortableResults": "[0.7.0, )" } }, "Microsoft.Bcl.HashCode": { diff --git a/src/Light.PortableResults.AspNetCore.Mvc/Light.PortableResults.AspNetCore.Mvc.csproj b/src/Light.PortableResults.AspNetCore.Mvc/Light.PortableResults.AspNetCore.Mvc.csproj index b200f44..5628f99 100644 --- a/src/Light.PortableResults.AspNetCore.Mvc/Light.PortableResults.AspNetCore.Mvc.csproj +++ b/src/Light.PortableResults.AspNetCore.Mvc/Light.PortableResults.AspNetCore.Mvc.csproj @@ -9,6 +9,10 @@ - LightActionResult and LightActionResult<T> and corresponding extension methods to turn result instances into HTTP success responses or RFC 9457 (and RFC 7807) compatible Problem Details responses. - Easy integration into ASP.NET Core's composition root via IServiceCollection.AddPortableResultsForMvc. - OpenAPI attributes and schema-only CLR surrogate types were removed; use Light.PortableResults.AspNetCore.OpenApi for OpenAPI integration. + - LightActionResult and LightActionResult<T> now report an unresolvable HttpResultForWriting contract as an + InvalidOperationException naming the closed wrapper type. The previous guard was unreachable because + JsonSerializerOptions.GetTypeInfo threw a NotSupportedException first. This package remains incompatible + with .NET Native AOT. diff --git a/src/Light.PortableResults.AspNetCore.Mvc/LightActionResult.cs b/src/Light.PortableResults.AspNetCore.Mvc/LightActionResult.cs index e160705..621bad9 100644 --- a/src/Light.PortableResults.AspNetCore.Mvc/LightActionResult.cs +++ b/src/Light.PortableResults.AspNetCore.Mvc/LightActionResult.cs @@ -5,6 +5,7 @@ using System.Threading.Tasks; using Light.PortableResults.AspNetCore.Mvc.Serialization; using Light.PortableResults.Http.Writing; +using Light.PortableResults.SharedJsonSerialization; using Microsoft.AspNetCore.Http; namespace Light.PortableResults.AspNetCore.Mvc; @@ -47,11 +48,12 @@ ResolvedHttpWriteOptions resolvedOptions var serializerOptions = httpContext.RequestServices.ResolveMvcJsonSerializerOptions(SerializerOptions); var wrapper = enrichedResult.ToHttpResultForWriting(resolvedOptions); - var typeInfo = serializerOptions.GetTypeInfo(typeof(HttpResultForWriting)); - if (typeInfo is not JsonTypeInfo castTypeInfo) + PortableResultsJsonContracts.EnsureTypeInfoResolver(serializerOptions); + if (!serializerOptions.TryGetTypeInfo(typeof(HttpResultForWriting), out var typeInfo) || + typeInfo is not JsonTypeInfo castTypeInfo) { throw new InvalidOperationException( - "Could not resolve 'JsonTypeInfo'. Please ensure that your JsonSerializerOptions are configured correctly. The AddDefaultLightResultsHttpWriteJsonConverters extension method can help you with this." + $"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'. Please ensure that your JsonSerializerOptions are configured correctly. The AddDefaultLightResultsHttpWriteJsonConverters extension method can help you with this." ); } @@ -103,11 +105,12 @@ ResolvedHttpWriteOptions resolvedOptions var serializerOptions = httpContext.RequestServices.ResolveMvcJsonSerializerOptions(SerializerOptions); var wrapper = enrichedResult.ToHttpResultForWriting(resolvedOptions); - var typeInfo = serializerOptions.GetTypeInfo(typeof(HttpResultForWriting)); - if (typeInfo is not JsonTypeInfo> castTypeInfo) + PortableResultsJsonContracts.EnsureTypeInfoResolver(serializerOptions); + if (!serializerOptions.TryGetTypeInfo(typeof(HttpResultForWriting), out var typeInfo) || + typeInfo is not JsonTypeInfo> castTypeInfo) { throw new InvalidOperationException( - $"Could not resolve 'JsonTypeInfo>'. Please ensure that your JsonSerializerOptions are configured correctly. The AddDefaultLightResultsHttpWriteJsonConverters extension method can help you with this." + $"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'. Please ensure that your JsonSerializerOptions are configured correctly. The AddDefaultLightResultsHttpWriteJsonConverters extension method can help you with this." ); } diff --git a/src/Light.PortableResults.AspNetCore.OpenApi/packages.lock.json b/src/Light.PortableResults.AspNetCore.OpenApi/packages.lock.json index 48391ea..7a499d8 100644 --- a/src/Light.PortableResults.AspNetCore.OpenApi/packages.lock.json +++ b/src/Light.PortableResults.AspNetCore.OpenApi/packages.lock.json @@ -56,7 +56,7 @@ "light.portableresults.aspnetcore.shared": { "type": "Project", "dependencies": { - "Light.PortableResults": "[0.6.0, )" + "Light.PortableResults": "[0.7.0, )" } }, "Microsoft.Bcl.HashCode": { diff --git a/src/Light.PortableResults.Validation.OpenApi/packages.lock.json b/src/Light.PortableResults.Validation.OpenApi/packages.lock.json index 1f8db2d..d963998 100644 --- a/src/Light.PortableResults.Validation.OpenApi/packages.lock.json +++ b/src/Light.PortableResults.Validation.OpenApi/packages.lock.json @@ -54,20 +54,20 @@ "light.portableresults.aspnetcore.openapi": { "type": "Project", "dependencies": { - "Light.PortableResults.AspNetCore.Shared": "[0.6.0, )", + "Light.PortableResults.AspNetCore.Shared": "[0.7.0, )", "Microsoft.AspNetCore.OpenApi": "[10.0.10, )" } }, "light.portableresults.aspnetcore.shared": { "type": "Project", "dependencies": { - "Light.PortableResults": "[0.6.0, )" + "Light.PortableResults": "[0.7.0, )" } }, "light.portableresults.validation": { "type": "Project", "dependencies": { - "Light.PortableResults": "[0.6.0, )" + "Light.PortableResults": "[0.7.0, )" } }, "Microsoft.AspNetCore.OpenApi": { diff --git a/src/Light.PortableResults.Validation/Light.PortableResults.Validation.csproj b/src/Light.PortableResults.Validation/Light.PortableResults.Validation.csproj index 01536fc..59f1045 100644 --- a/src/Light.PortableResults.Validation/Light.PortableResults.Validation.csproj +++ b/src/Light.PortableResults.Validation/Light.PortableResults.Validation.csproj @@ -3,6 +3,7 @@ netstandard2.0;net10.0 false + true Framework-agnostic validation foundations for Light.PortableResults, including validation contexts, low-allocation checks, validator base classes, and validated value pipelines. Light.PortableResults.Validation 0.7.0 @@ -20,6 +21,15 @@ Unspecified error codes and customizable message templates. They assert the DateTime.Kind of the checked value; with the default System.Text.Json converter, IsUtc requires a trailing 'Z' because a numeric offset - including +00:00 - deserializes as Local. + - Builds the net10.0 asset with IsAotCompatible, so trim and Native AOT analyzer diagnostics fail the build. + + Breaking changes + --------------------------------- + + - The TValidator type parameter of OptionsBuilder<T>.ValidateWithPortableResults is annotated with + DynamicallyAccessedMemberTypes.PublicConstructors, because the DI container instantiates it. Callers that + forward their own unannotated generic parameter now get a trim analysis warning and have to add the same + annotation. diff --git a/src/Light.PortableResults.Validation/PortableResultsValidationModule.cs b/src/Light.PortableResults.Validation/PortableResultsValidationModule.cs index 6f50e03..495a8ab 100644 --- a/src/Light.PortableResults.Validation/PortableResultsValidationModule.cs +++ b/src/Light.PortableResults.Validation/PortableResultsValidationModule.cs @@ -1,4 +1,5 @@ using System; +using System.Diagnostics.CodeAnalysis; using Light.PortableResults.Validation.ConfigurationIntegration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; @@ -17,7 +18,7 @@ public static class PortableResultsValidationModule /// /// The service collection that holds all registrations. /// - /// The optional delegate that creates the default instance. The created + /// The optional delegate that creates the default instance. The created /// options are registered as a singleton (as the itself is registered /// as a singleton). /// @@ -51,11 +52,16 @@ public static IServiceCollection AddValidationForPortableResults( /// pipeline from Microsoft.Extensions.Options. /// /// The options type to validate. - /// The validator type that implements . + /// + /// The validator type that implements . It is instantiated by the DI container, + /// so its public constructors must be preserved when trimming. + /// /// The options builder to configure. /// The for further chaining. /// Thrown when is null. - public static OptionsBuilder ValidateWithPortableResults( + public static OptionsBuilder ValidateWithPortableResults< + TOptions, + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors)] TValidator>( this OptionsBuilder builder ) where TOptions : class diff --git a/src/Light.PortableResults.Validation/packages.lock.json b/src/Light.PortableResults.Validation/packages.lock.json index 192a7c8..8f5b6b4 100644 --- a/src/Light.PortableResults.Validation/packages.lock.json +++ b/src/Light.PortableResults.Validation/packages.lock.json @@ -206,6 +206,12 @@ } }, "net10.0": { + "Microsoft.NET.ILLink.Tasks": { + "type": "Direct", + "requested": "[10.0.10, )", + "resolved": "10.0.10", + "contentHash": "f5VCIE7AJpd5YvzNTeMGVzQIgyE9tX+AreTYwQF+REbu+DZo/2Ae+jNSwhPEYrVz6RRkd7y8ubXjk6Nn6Ka+Cg==" + }, "Microsoft.SourceLink.GitHub": { "type": "Direct", "requested": "[10.0.301, )", diff --git a/src/Light.PortableResults/CloudEvents/Reading/ReadOnlyMemoryCloudEventsExtensions.cs b/src/Light.PortableResults/CloudEvents/Reading/ReadOnlyMemoryCloudEventsExtensions.cs index 4a40cde..1c3eb31 100644 --- a/src/Light.PortableResults/CloudEvents/Reading/ReadOnlyMemoryCloudEventsExtensions.cs +++ b/src/Light.PortableResults/CloudEvents/Reading/ReadOnlyMemoryCloudEventsExtensions.cs @@ -3,6 +3,7 @@ using Light.PortableResults.CloudEvents.Reading.Json; using Light.PortableResults.Http.Reading.Json; using Light.PortableResults.Metadata; +using Light.PortableResults.SharedJsonSerialization; namespace Light.PortableResults.CloudEvents.Reading; @@ -71,10 +72,7 @@ public static CloudEventsEnvelope ReadResultWithCloudEventsEnvelope( ) { options ??= PortableResultsCloudEventsReadOptions.Default; - var parsedEnvelope = JsonSerializer.Deserialize( - cloudEvent.Span, - options.SerializerOptions - ); + var parsedEnvelope = ParseEnvelope(cloudEvent, options); var isFailure = DetermineIsFailure(parsedEnvelope, options); var dataSegment = parsedEnvelope is { HasData: true, IsDataNull: false } ? @@ -114,10 +112,7 @@ public static CloudEventsEnvelope ReadResultWithCloudEventsEnvelope( ) { options ??= PortableResultsCloudEventsReadOptions.Default; - var parsedEnvelope = JsonSerializer.Deserialize( - cloudEvent.Span, - options.SerializerOptions - ); + var parsedEnvelope = ParseEnvelope(cloudEvent, options); var isFailure = DetermineIsFailure(parsedEnvelope, options); var dataSegment = parsedEnvelope is { HasData: true, IsDataNull: false } ? @@ -140,6 +135,18 @@ public static CloudEventsEnvelope ReadResultWithCloudEventsEnvelope( } + private static CloudEventsEnvelopePayload ParseEnvelope( + ReadOnlyMemory cloudEvent, + PortableResultsCloudEventsReadOptions options + ) => + JsonSerializer.Deserialize( + cloudEvent.Span, + PortableResultsJsonContracts.GetLibraryTypeInfo( + options.SerializerOptions, + static () => new CloudEventsEnvelopePayloadJsonConverter() + ) + ); + private static Result ParseResultPayload( ReadOnlyMemory dataSegment, bool isFailure, @@ -160,16 +167,16 @@ PortableResultsCloudEventsReadOptions options if (isFailure) { - var failurePayload = JsonSerializer.Deserialize( - dataSegment.Span, - options.SerializerOptions - ); + var failurePayload = ParseFailurePayload(dataSegment, options); return Result.Fail(failurePayload.Errors, failurePayload.Metadata); } - var successPayload = JsonSerializer.Deserialize( + var successPayload = JsonSerializer.Deserialize( dataSegment.Span, - options.SerializerOptions + PortableResultsJsonContracts.GetLibraryTypeInfo( + options.SerializerOptions, + static () => new CloudEventsSuccessPayloadJsonConverter() + ) ); var metadata = successPayload.Metadata; if (metadata is not null) @@ -183,6 +190,18 @@ PortableResultsCloudEventsReadOptions options return Result.Ok(metadata); } + private static CloudEventsFailurePayload ParseFailurePayload( + ReadOnlyMemory dataSegment, + PortableResultsCloudEventsReadOptions options + ) => + JsonSerializer.Deserialize( + dataSegment.Span, + PortableResultsJsonContracts.GetLibraryTypeInfo( + options.SerializerOptions, + static () => new CloudEventsFailurePayloadJsonConverter() + ) + ); + private static Result ParseGenericResultPayload( ReadOnlyMemory dataSegment, // ReSharper disable once ParameterOnlyUsedForPreconditionCheck.Local - this is not a precondition check. @@ -199,10 +218,7 @@ PortableResultsCloudEventsReadOptions options if (isFailure) { - var failurePayload = JsonSerializer.Deserialize( - dataSegment.Span, - options.SerializerOptions - ); + var failurePayload = ParseFailurePayload(dataSegment, options); return Result.Fail(failurePayload.Errors, failurePayload.Metadata); } @@ -213,18 +229,24 @@ PortableResultsCloudEventsReadOptions options if (normalizedPreference == PreferSuccessPayload.BareValue) { - var payload = JsonSerializer.Deserialize>( + var payload = JsonSerializer.Deserialize( dataSegment.Span, - options.SerializerOptions + PortableResultsJsonContracts.GetLibraryTypeInfo( + options.SerializerOptions, + static () => new CloudEventsBareSuccessPayloadJsonConverter() + ) ); return CreateSuccessfulGenericResult(payload.Value, metadata: null); } if (normalizedPreference == PreferSuccessPayload.WrappedValue) { - var payload = JsonSerializer.Deserialize>( + var payload = JsonSerializer.Deserialize( dataSegment.Span, - options.SerializerOptions + PortableResultsJsonContracts.GetLibraryTypeInfo( + options.SerializerOptions, + static () => new CloudEventsWrappedSuccessPayloadJsonConverter() + ) ); var metadata = payload.Metadata; if (metadata is not null) @@ -238,9 +260,12 @@ PortableResultsCloudEventsReadOptions options return CreateSuccessfulGenericResult(payload.Value, metadata); } - var autoPayload = JsonSerializer.Deserialize>( + var autoPayload = JsonSerializer.Deserialize( dataSegment.Span, - options.SerializerOptions + PortableResultsJsonContracts.GetLibraryTypeInfo( + options.SerializerOptions, + static () => new CloudEventsAutoSuccessPayloadJsonConverter() + ) ); var autoMetadata = autoPayload.Metadata; if (autoMetadata is not null) diff --git a/src/Light.PortableResults/CloudEvents/Writing/CloudEventsResultExtensions.cs b/src/Light.PortableResults/CloudEvents/Writing/CloudEventsResultExtensions.cs index cac1e2e..127098a 100644 --- a/src/Light.PortableResults/CloudEvents/Writing/CloudEventsResultExtensions.cs +++ b/src/Light.PortableResults/CloudEvents/Writing/CloudEventsResultExtensions.cs @@ -2,7 +2,9 @@ using System.Globalization; using System.Text.Json; using Light.PortableResults.Buffers; +using Light.PortableResults.CloudEvents.Writing.Json; using Light.PortableResults.Metadata; +using Light.PortableResults.SharedJsonSerialization; namespace Light.PortableResults.CloudEvents.Writing; @@ -176,7 +178,12 @@ public static void WriteCloudEvent( options ); - JsonSerializer.Serialize(writer, envelope, options.SerializerOptions); + PortableResultsJsonContracts.WriteLibraryValue( + writer, + envelope, + options.SerializerOptions, + static () => new CloudEventsEnvelopeForWritingJsonConverter() + ); } /// @@ -412,7 +419,12 @@ public static void WriteCloudEvent( resolvedOptions ); - JsonSerializer.Serialize(writer, envelope, resolvedOptions.SerializerOptions); + PortableResultsJsonContracts.WriteLibraryValue( + writer, + envelope, + resolvedOptions.SerializerOptions, + static () => new CloudEventsEnvelopeForWritingJsonConverter() + ); } /// diff --git a/src/Light.PortableResults/Http/Reading/HttpResponseMessageExtensions.cs b/src/Light.PortableResults/Http/Reading/HttpResponseMessageExtensions.cs index 7e80caf..aa11160 100644 --- a/src/Light.PortableResults/Http/Reading/HttpResponseMessageExtensions.cs +++ b/src/Light.PortableResults/Http/Reading/HttpResponseMessageExtensions.cs @@ -2,10 +2,12 @@ using System.IO; using System.Net.Http; using System.Text.Json; +using System.Text.Json.Serialization.Metadata; using System.Threading; using System.Threading.Tasks; using Light.PortableResults.Http.Reading.Json; using Light.PortableResults.Metadata; +using Light.PortableResults.SharedJsonSerialization; namespace Light.PortableResults.Http.Reading; @@ -124,19 +126,30 @@ CancellationToken cancellationToken return HandleEmptyBody(isFailure, static () => Result.Ok(), successEmptyBodyMessage: null); } +#if NET10_0_OR_GREATER + await using (contentStream) +#else using (contentStream) +#endif { if (isFailure) { var failurePayload = await JsonSerializer - .DeserializeAsync(contentStream, serializerOptions, cancellationToken) + .DeserializeAsync(contentStream, GetFailurePayloadTypeInfo(serializerOptions), cancellationToken) .ConfigureAwait(false); EnsureFailurePayloadHasErrors(failurePayload.Errors, NonGenericFailureMustDeserializeToFailedMessage); return Result.Fail(failurePayload.Errors, failurePayload.Metadata); } var successPayload = await JsonSerializer - .DeserializeAsync(contentStream, serializerOptions, cancellationToken) + .DeserializeAsync( + contentStream, + PortableResultsJsonContracts.GetLibraryTypeInfo( + serializerOptions, + static () => new HttpReadSuccessResultPayloadJsonConverter() + ), + cancellationToken + ) .ConfigureAwait(false); return Result.Ok(successPayload.Metadata); } @@ -160,12 +173,16 @@ CancellationToken cancellationToken ); } +#if NET10_0_OR_GREATER + await using (contentStream) +#else using (contentStream) +#endif { if (isFailure) { var failurePayload = await JsonSerializer - .DeserializeAsync(contentStream, serializerOptions, cancellationToken) + .DeserializeAsync(contentStream, GetFailurePayloadTypeInfo(serializerOptions), cancellationToken) .ConfigureAwait(false); EnsureFailurePayloadHasErrors(failurePayload.Errors, GenericFailureMustDeserializeToFailedMessage); return Result.Fail(failurePayload.Errors, failurePayload.Metadata); @@ -198,9 +215,12 @@ CancellationToken cancellationToken if (preferSuccessPayload == PreferSuccessPayload.BareValue) { var barePayload = await JsonSerializer - .DeserializeAsync>( + .DeserializeAsync( contentStream, - serializerOptions, + PortableResultsJsonContracts.GetLibraryTypeInfo( + serializerOptions, + static () => new HttpReadBareSuccessResultPayloadJsonConverter() + ), cancellationToken ) .ConfigureAwait(false); @@ -210,9 +230,12 @@ CancellationToken cancellationToken if (preferSuccessPayload == PreferSuccessPayload.WrappedValue) { var wrappedPayload = await JsonSerializer - .DeserializeAsync>( + .DeserializeAsync( contentStream, - serializerOptions, + PortableResultsJsonContracts.GetLibraryTypeInfo( + serializerOptions, + static () => new HttpReadWrappedSuccessResultPayloadJsonConverter() + ), cancellationToken ) .ConfigureAwait(false); @@ -220,11 +243,26 @@ CancellationToken cancellationToken } var autoPayload = await JsonSerializer - .DeserializeAsync>(contentStream, serializerOptions, cancellationToken) + .DeserializeAsync( + contentStream, + PortableResultsJsonContracts.GetLibraryTypeInfo( + serializerOptions, + static () => new HttpReadAutoSuccessResultPayloadJsonConverter() + ), + cancellationToken + ) .ConfigureAwait(false); return CreateSuccessfulGenericResult(autoPayload.Value, autoPayload.Metadata); } + private static JsonTypeInfo GetFailurePayloadTypeInfo( + JsonSerializerOptions serializerOptions + ) => + PortableResultsJsonContracts.GetLibraryTypeInfo( + serializerOptions, + static () => new HttpReadFailureResultPayloadJsonConverter() + ); + private static Result CreateSuccessfulGenericResult(T value, MetadataObject? metadata) { try @@ -281,12 +319,12 @@ CancellationToken cancellationToken } cancellationToken.ThrowIfCancellationRequested(); - var stream = await response.Content.ReadAsStreamAsync(); + var stream = await response.Content.ReadAsStreamAsync(cancellationToken); switch (stream.CanSeek) { case true when stream.Length == 0L: - stream.Dispose(); + await stream.DisposeAsync(); return null; default: return stream; } diff --git a/src/Light.PortableResults/Http/Reading/Json/ResultJsonReader.cs b/src/Light.PortableResults/Http/Reading/Json/ResultJsonReader.cs index fe22e09..60883b6 100644 --- a/src/Light.PortableResults/Http/Reading/Json/ResultJsonReader.cs +++ b/src/Light.PortableResults/Http/Reading/Json/ResultJsonReader.cs @@ -3,6 +3,7 @@ using System.Globalization; using System.Text.Json; using Light.PortableResults.Metadata; +using Light.PortableResults.SharedJsonSerialization; using Light.PortableResults.SharedJsonSerialization.Reading; namespace Light.PortableResults.Http.Reading.Json; @@ -663,7 +664,10 @@ private static Error CreateFallbackError(string title, string? detail, int statu private static T? ReadGenericValue(ref Utf8JsonReader reader, JsonSerializerOptions serializerOptions) { - return JsonSerializer.Deserialize(ref reader, serializerOptions); + return JsonSerializer.Deserialize( + ref reader, + PortableResultsJsonContracts.GetRequiredTypeInfo(serializerOptions) + ); } private static bool IsWrappedSuccessPayloadCandidate(ref Utf8JsonReader reader) diff --git a/src/Light.PortableResults/Light.PortableResults.csproj b/src/Light.PortableResults/Light.PortableResults.csproj index abf731b..fc426a0 100644 --- a/src/Light.PortableResults/Light.PortableResults.csproj +++ b/src/Light.PortableResults/Light.PortableResults.csproj @@ -3,6 +3,7 @@ netstandard2.0;net10.0 false + true true true The Light.PortableResults package implements the core functionality: Results, Errors, Metadata, Functional Extensions, and serialization support for various formats like HTTP and CloudEvents. Compatible with Native AOT. Check out the integration packages Light.PortableResults.AspNetCore.MinimalApis or Light.PortableResults.AspNetCore.Mvc. @@ -25,10 +26,21 @@ and CloudEvents extension-attribute writing for bounded primitive values. - Emits primitive metadata arrays as ordered, separate HTTP header values. Custom header converters can reuse the new public HttpHeaderValueFormatter. + - Builds the net10.0 asset with IsAotCompatible, so trim and Native AOT analyzer diagnostics fail the build. + - Resolves its own CloudEvents write, CloudEvents read, and HTTP read envelope and payload contracts through + the new public PortableResultsJsonContracts. Consumers only register their result value types with a + JsonSerializerContext; closed library wrappers such as CloudEventsEnvelopeForWriting<T> no longer have to + be declared. Non-generic results need no consumer registration at all. + - Keeps the precedence of converters registered before the Light.PortableResults defaults, and caches + library-created contracts per JsonSerializerOptions instance, including read-only instances. Breaking changes --------------------------------- + - Missing serialization metadata for a result value type now throws an InvalidOperationException that names the + type and the remedy at every CloudEvents and HTTP entry point, instead of the NotSupportedException that + System.Text.Json raised for the unresolved contract. + - Values of the ten newly typed BCL metadata types no longer flatten to MetadataKind.String. Their JSON representations now use the canonical encoding of their dedicated kind. - Whole-number Double and Single metadata values serialize with a trailing .0. diff --git a/src/Light.PortableResults/SharedJsonSerialization/PortableResultsJsonContracts.cs b/src/Light.PortableResults/SharedJsonSerialization/PortableResultsJsonContracts.cs new file mode 100644 index 0000000..85b0829 --- /dev/null +++ b/src/Light.PortableResults/SharedJsonSerialization/PortableResultsJsonContracts.cs @@ -0,0 +1,352 @@ +using System; +using System.Collections.Concurrent; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; +using System.Text.Json; +using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; + +namespace Light.PortableResults.SharedJsonSerialization; + +/// +/// +/// Provides the JSON contract resolution that Light.PortableResults uses instead of the reflection-based +/// overloads. All members are safe to call from trimmed and Native AOT applications. +/// +/// +/// Consumers only need to register their own result value types with a . +/// The envelope and payload types owned by Light.PortableResults are resolved by this class: it prefers a contract +/// supplied by the configured and otherwise creates a contract +/// from the converter that the options select for the type. Created contracts are cached per +/// instance and can be created after the options became read-only. +/// +/// +public static class PortableResultsJsonContracts +{ + private static readonly ConditionalWeakTable ContractsPerOptions = new (); + + /// + /// Creates the message of the that is thrown when the configured + /// cannot supply serialization metadata for a consumer-owned type. + /// + /// The type whose metadata could not be resolved. + /// The exception message naming and the required remedy. + /// Thrown when is . + public static string CreateMissingTypeInfoMessage(Type type) + { + if (type is null) + { + throw new ArgumentNullException(nameof(type)); + } + + return + $"No JSON serialization metadata was found for type '{type}'. Register this type with the " + + "JsonSerializerContext that is supplied to the JsonSerializerOptions used by Light.PortableResults."; + } + + /// + /// Creates the message of the that is thrown when neither a contract nor + /// a converter can be resolved for a type owned by Light.PortableResults. + /// + /// The Light.PortableResults type whose contract could not be resolved. + /// The exception message naming and the required remedy. + /// Thrown when is . + public static string CreateMissingLibraryContractMessage(Type type) + { + if (type is null) + { + throw new ArgumentNullException(nameof(type)); + } + + return + $"No JSON serialization metadata or converter was found for the Light.PortableResults type '{type}'. " + + "Call the corresponding AddDefaultPortableResults... extension method on your JsonSerializerOptions."; + } + + /// + /// Materializes the reflection-based type info resolver when the specified options have no resolver and + /// reflection-based serialization is enabled. This keeps the behavior of the shared default options - which carry + /// converters but no explicit resolver - unchanged, and is a no-op when reflection is disabled. + /// + /// The serializer options to inspect. + /// Thrown when is . + public static void EnsureTypeInfoResolver(JsonSerializerOptions options) + { + if (options is null) + { + throw new ArgumentNullException(nameof(options)); + } + + if (options.TypeInfoResolver is not null || !JsonSerializer.IsReflectionEnabledByDefault) + { + return; + } + + PopulateReflectionTypeInfoResolver(options); + } + + /// + /// Resolves the contract for a consumer-owned type, e.g. the value type of a . + /// + /// The consumer-owned type. + /// The serializer options that must supply the contract. + /// The resolved contract. + /// Thrown when is . + /// + /// Thrown when cannot supply serialization metadata for . + /// + public static JsonTypeInfo GetRequiredTypeInfo(JsonSerializerOptions options) + { + if (options is null) + { + throw new ArgumentNullException(nameof(options)); + } + + EnsureTypeInfoResolver(options); + if (options.TryGetTypeInfo(typeof(T), out var typeInfo) && typeInfo is JsonTypeInfo typedTypeInfo) + { + return typedTypeInfo; + } + + throw new InvalidOperationException(CreateMissingTypeInfoMessage(typeof(T))); + } + + /// + /// Resolves the contract for a type owned by Light.PortableResults. A contract supplied by the configured + /// resolver takes precedence; otherwise a contract is created from the converter that + /// select for , falling back to + /// when no converter is registered. + /// + /// The Light.PortableResults type to resolve. + /// The serializer options. + /// + /// An optional factory for the library-owned converter that is used when have no + /// converter for . + /// + /// The resolved contract. + /// Thrown when is . + /// + /// Thrown when neither a contract nor a converter can be resolved for . + /// + public static JsonTypeInfo GetLibraryTypeInfo( + JsonSerializerOptions options, + Func>? createLibraryConverter = null + ) + { + if (options is null) + { + throw new ArgumentNullException(nameof(options)); + } + + if (TryGetCachedTypeInfo(options, out var cachedTypeInfo)) + { + return cachedTypeInfo; + } + + EnsureTypeInfoResolver(options); + if (options.TryGetTypeInfo(typeof(TLibraryType), out var resolvedTypeInfo) && + resolvedTypeInfo is JsonTypeInfo typedTypeInfo) + { + return typedTypeInfo; + } + + var converter = ResolveLibraryConverter(options, createLibraryConverter); + var createdTypeInfo = JsonMetadataServices.CreateValueInfo(options, converter); + return CacheTypeInfo(options, createdTypeInfo); + } + + /// + /// Writes a value of a type owned by Light.PortableResults. A contract supplied by the configured resolver takes + /// precedence; otherwise the converter that select for + /// is invoked directly, falling back to + /// when no converter is registered. + /// + /// The Light.PortableResults type to write. + /// The JSON writer receiving the value. + /// The value to write. + /// The serializer options. + /// + /// An optional factory for the library-owned converter that is used when have no + /// converter for . + /// + /// + /// Thrown when or is . + /// + /// + /// Thrown when neither a contract nor a converter can be resolved for . + /// + public static void WriteLibraryValue( + Utf8JsonWriter writer, + TLibraryType value, + JsonSerializerOptions options, + Func>? createLibraryConverter = null + ) + { + if (writer is null) + { + throw new ArgumentNullException(nameof(writer)); + } + + if (options is null) + { + throw new ArgumentNullException(nameof(options)); + } + + if (TryGetCachedConverter(options, out var cachedConverter)) + { + cachedConverter.Write(writer, value, options); + return; + } + + EnsureTypeInfoResolver(options); + if (options.TryGetTypeInfo(typeof(TLibraryType), out var resolvedTypeInfo) && + resolvedTypeInfo is JsonTypeInfo typedTypeInfo) + { + JsonSerializer.Serialize(writer, value, typedTypeInfo); + return; + } + + var converter = CacheConverter(options, ResolveLibraryConverter(options, createLibraryConverter)); + converter.Write(writer, value, options); + } + + // JsonSerializer.IsReflectionEnabledByDefault is a link-time constant: when reflection-based serialization is + // disabled - which is the case in trimmed and Native AOT apps that opt out - the call below is removed entirely + // and the trimmer never has to preserve the reflection-based resolver. Freezing the options here is not an + // additional behavior change, because the serialization that follows immediately would freeze them anyway. + [UnconditionalSuppressMessage( + "Trimming", + "IL2026:RequiresUnreferencedCode", + Justification = + "The call is guarded by JsonSerializer.IsReflectionEnabledByDefault and is unreachable when reflection-based serialization is disabled." + )] + [UnconditionalSuppressMessage( + "AOT", + "IL3050:RequiresDynamicCode", + Justification = + "The call is guarded by JsonSerializer.IsReflectionEnabledByDefault and is unreachable when reflection-based serialization is disabled." + )] + private static void PopulateReflectionTypeInfoResolver(JsonSerializerOptions options) => + options.MakeReadOnly(populateMissingResolver: true); + + private static JsonConverter ResolveLibraryConverter( + JsonSerializerOptions options, + Func>? createLibraryConverter + ) + { + var registeredConverter = FindRegisteredConverter(options); + if (registeredConverter is not null) + { + return registeredConverter; + } + + if (createLibraryConverter is not null) + { + return createLibraryConverter(); + } + + throw new InvalidOperationException(CreateMissingLibraryContractMessage(typeof(TLibraryType))); + } + + // Mirrors how JsonSerializerOptions select a converter from their converter list: the first entry that can + // convert the type wins. Converters that a caller inserted before the Light.PortableResults defaults therefore + // keep their precedence, and the library converter is only reached when no other entry matches. + private static JsonConverter? FindRegisteredConverter(JsonSerializerOptions options) + { + var converters = options.Converters; + for (var i = 0; i < converters.Count; i++) + { + var candidate = converters[i]; + if (!candidate.CanConvert(typeof(TLibraryType))) + { + continue; + } + + if (candidate is JsonConverterFactory factory) + { + if (factory.CreateConverter(typeof(TLibraryType), options) is JsonConverter created) + { + return created; + } + + continue; + } + + if (candidate is JsonConverter converter) + { + return converter; + } + } + + return null; + } + + private static bool TryGetCachedTypeInfo( + JsonSerializerOptions options, + [NotNullWhen(true)] out JsonTypeInfo? typeInfo + ) + { + if (ContractsPerOptions.TryGetValue(options, out var contracts) && + contracts.TypeInfos.TryGetValue(typeof(TLibraryType), out var cached)) + { + typeInfo = (JsonTypeInfo) cached; + return true; + } + + typeInfo = null; + return false; + } + + // Contracts are only cached once the options are frozen. A mutable instance can still gain a converter that + // would change the outcome of the resolution, and freezing happens at the latest when the value is written or + // read with the resolved contract. + private static JsonTypeInfo CacheTypeInfo( + JsonSerializerOptions options, + JsonTypeInfo typeInfo + ) + { + if (!options.IsReadOnly) + { + return typeInfo; + } + + var contracts = ContractsPerOptions.GetValue(options, static _ => new LibraryContracts()); + return (JsonTypeInfo) contracts.TypeInfos.GetOrAdd(typeof(TLibraryType), typeInfo); + } + + private static bool TryGetCachedConverter( + JsonSerializerOptions options, + [NotNullWhen(true)] out JsonConverter? converter + ) + { + if (ContractsPerOptions.TryGetValue(options, out var contracts) && + contracts.Converters.TryGetValue(typeof(TLibraryType), out var cached)) + { + converter = (JsonConverter) cached; + return true; + } + + converter = null; + return false; + } + + private static JsonConverter CacheConverter( + JsonSerializerOptions options, + JsonConverter converter + ) + { + if (!options.IsReadOnly) + { + return converter; + } + + var contracts = ContractsPerOptions.GetValue(options, static _ => new LibraryContracts()); + return (JsonConverter) contracts.Converters.GetOrAdd(typeof(TLibraryType), converter); + } + + private sealed class LibraryContracts + { + public ConcurrentDictionary TypeInfos { get; } = new (); + public ConcurrentDictionary Converters { get; } = new (); + } +} diff --git a/src/Light.PortableResults/SharedJsonSerialization/Writing/ErrorsExtensions.cs b/src/Light.PortableResults/SharedJsonSerialization/Writing/ErrorsExtensions.cs index 5f23979..45dab55 100644 --- a/src/Light.PortableResults/SharedJsonSerialization/Writing/ErrorsExtensions.cs +++ b/src/Light.PortableResults/SharedJsonSerialization/Writing/ErrorsExtensions.cs @@ -1,6 +1,5 @@ using System; using System.Text.Json; -using System.Text.Json.Serialization.Metadata; using Light.PortableResults.Metadata; namespace Light.PortableResults.SharedJsonSerialization.Writing; @@ -69,16 +68,14 @@ JsonSerializerOptions serializerOptions if (error.Metadata.HasValue) { - var metadataTypeInfo = serializerOptions.GetTypeInfo(typeof(MetadataObject)); - if (metadataTypeInfo is not JsonTypeInfo castTypeInfo) - { - throw new InvalidOperationException( - "Could not resolve 'JsonTypeInfo'. Please ensure that your JsonSerializerOptions are configured correctly." - ); - } + // The MetadataObject converter differs per transport - CloudEvents writing and HTTP writing register + // their own - so no library-owned fallback converter is supplied here. The one configured on the + // options is used, and a missing registration is reported instead of silently reflecting. + var metadataTypeInfo = + PortableResultsJsonContracts.GetLibraryTypeInfo(serializerOptions); writer.WritePropertyName("metadata"); - JsonSerializer.Serialize(writer, error.Metadata.Value, castTypeInfo); + JsonSerializer.Serialize(writer, error.Metadata.Value, metadataTypeInfo); } writer.WriteEndObject(); diff --git a/src/Light.PortableResults/SharedJsonSerialization/Writing/SystemTextJsonWritingExtensions.cs b/src/Light.PortableResults/SharedJsonSerialization/Writing/SystemTextJsonWritingExtensions.cs index 0022064..e580e64 100644 --- a/src/Light.PortableResults/SharedJsonSerialization/Writing/SystemTextJsonWritingExtensions.cs +++ b/src/Light.PortableResults/SharedJsonSerialization/Writing/SystemTextJsonWritingExtensions.cs @@ -67,28 +67,22 @@ public static void WriteGenericValue(this Utf8JsonWriter writer, T? value, Js return; } - var valueTypeInfo = options.GetTypeInfo(typeof(T)); - if (valueTypeInfo is null) - { - throw new InvalidOperationException( - $"Could not find JsonTypeInfo for type '{nameof(T)}'. Please ensure that your JsonSerializerOptions are configured correctly." - ); - } + var valueTypeInfo = PortableResultsJsonContracts.GetRequiredTypeInfo(options); var runtimeType = value.GetType(); if (valueTypeInfo.ShouldUseWith(runtimeType)) { - JsonSerializer.Serialize(writer, value, (JsonTypeInfo) valueTypeInfo); + JsonSerializer.Serialize(writer, value, valueTypeInfo); return; } - if (!options.TryGetTypeInfo(runtimeType, out valueTypeInfo)) + if (!options.TryGetTypeInfo(runtimeType, out var runtimeTypeInfo)) { throw new InvalidOperationException( - $"No JSON serialization metadata was found for type '{runtimeType}' - please ensure that JsonOptions are configured properly" + PortableResultsJsonContracts.CreateMissingTypeInfoMessage(runtimeType) ); } - JsonSerializer.Serialize(writer, value, valueTypeInfo); + JsonSerializer.Serialize(writer, value, runtimeTypeInfo); } } diff --git a/src/Light.PortableResults/packages.lock.json b/src/Light.PortableResults/packages.lock.json index 84ec8ac..ba5d68f 100644 --- a/src/Light.PortableResults/packages.lock.json +++ b/src/Light.PortableResults/packages.lock.json @@ -217,6 +217,12 @@ "resolved": "10.0.10", "contentHash": "5wu/GrYVd8mG2DVUw3vFJzF+O336TyTGg/Kmcgw9bfwYhCoFiV5lR5QeEmKecJyrW4W54nMfD3p3589E8a7czQ==" }, + "Microsoft.NET.ILLink.Tasks": { + "type": "Direct", + "requested": "[10.0.10, )", + "resolved": "10.0.10", + "contentHash": "f5VCIE7AJpd5YvzNTeMGVzQIgyE9tX+AreTYwQF+REbu+DZo/2Ae+jNSwhPEYrVz6RRkd7y8ubXjk6Nn6Ka+Cg==" + }, "Microsoft.SourceLink.GitHub": { "type": "Direct", "requested": "[10.0.301, )", diff --git a/tests/Light.PortableResults.AspNetCore.MinimalApis.Tests/LightResultContractGuardTests.cs b/tests/Light.PortableResults.AspNetCore.MinimalApis.Tests/LightResultContractGuardTests.cs new file mode 100644 index 0000000..f0b2e44 --- /dev/null +++ b/tests/Light.PortableResults.AspNetCore.MinimalApis.Tests/LightResultContractGuardTests.cs @@ -0,0 +1,82 @@ +using System; +using System.IO; +using System.Text.Json; +using System.Text.Json.Serialization.Metadata; +using System.Threading.Tasks; +using FluentAssertions; +using Light.PortableResults.Http.Writing; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Light.PortableResults.AspNetCore.MinimalApis.Tests; + +/// +/// Verifies that the Minimal API results report the unresolved wrapper type instead of failing inside +/// System.Text.Json when the configured resolver cannot supply its contract. +/// +public sealed class LightResultContractGuardTests +{ + [Fact] + public async Task ExecuteAsyncShouldNameTheUnresolvedWrapperType() + { + await using var provider = CreateServiceProvider(); + var httpContext = CreateHttpContext(provider); + var lightResult = new LightResult(Result.Ok(), serializerOptions: CreateSerializerOptions()); + + var act = async () => await lightResult.ExecuteAsync(httpContext); + + await act.Should().ThrowAsync() + .WithMessage($"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'*"); + } + + [Fact] + public async Task ExecuteAsyncShouldNameTheUnresolvedGenericWrapperType() + { + await using var provider = CreateServiceProvider(); + var httpContext = CreateHttpContext(provider); + var lightResult = new LightResult( + Result.Ok("hello"), + serializerOptions: CreateSerializerOptions() + ); + + var act = async () => await lightResult.ExecuteAsync(httpContext); + + await act.Should().ThrowAsync() + .WithMessage($"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'*"); + } + + private static ServiceProvider CreateServiceProvider() + { + var services = new ServiceCollection(); + services.AddPortableResultsForMinimalApis(); + return services.BuildServiceProvider(); + } + + private static DefaultHttpContext CreateHttpContext(IServiceProvider provider) => + new () + { + RequestServices = provider, + Response = { Body = new MemoryStream() } + }; + + private static JsonSerializerOptions CreateSerializerOptions() + { + var serializerOptions = new JsonSerializerOptions + { + TypeInfoResolver = NoContractsJsonTypeInfoResolver.Instance + }; + serializerOptions.AddDefaultPortableResultsHttpWriteJsonConverters(); + return serializerOptions; + } +} + +/// +/// A resolver that supplies no contract for any type. +/// +public sealed class NoContractsJsonTypeInfoResolver : IJsonTypeInfoResolver +{ + public static NoContractsJsonTypeInfoResolver Instance { get; } = new (); + + public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options) => null; +} diff --git a/tests/Light.PortableResults.AspNetCore.Mvc.Tests/LightActionResultContractGuardTests.cs b/tests/Light.PortableResults.AspNetCore.Mvc.Tests/LightActionResultContractGuardTests.cs new file mode 100644 index 0000000..4a0d4b7 --- /dev/null +++ b/tests/Light.PortableResults.AspNetCore.Mvc.Tests/LightActionResultContractGuardTests.cs @@ -0,0 +1,87 @@ +using System; +using System.IO; +using System.Text.Json; +using System.Text.Json.Serialization.Metadata; +using System.Threading.Tasks; +using FluentAssertions; +using Light.PortableResults.Http.Writing; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Light.PortableResults.AspNetCore.Mvc.Tests; + +/// +/// Verifies that the MVC action results report the unresolved wrapper type instead of failing inside +/// System.Text.Json when the configured resolver cannot supply its contract. +/// +public sealed class LightActionResultContractGuardTests +{ + [Fact] + public async Task ExecuteResultAsyncShouldNameTheUnresolvedWrapperType() + { + await using var provider = CreateServiceProvider(); + var actionResult = new LightActionResult(Result.Ok(), serializerOptions: CreateSerializerOptions()); + + var act = async () => await actionResult.ExecuteResultAsync(CreateActionContext(provider)); + + await act.Should().ThrowAsync() + .WithMessage($"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'*"); + } + + [Fact] + public async Task ExecuteResultAsyncShouldNameTheUnresolvedGenericWrapperType() + { + await using var provider = CreateServiceProvider(); + var actionResult = new LightActionResult( + Result.Ok("hello"), + serializerOptions: CreateSerializerOptions() + ); + + var act = async () => await actionResult.ExecuteResultAsync(CreateActionContext(provider)); + + await act.Should().ThrowAsync() + .WithMessage($"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'*"); + } + + private static ServiceProvider CreateServiceProvider() + { + var services = new ServiceCollection(); + services.AddPortableResultsForMvc(); + return services.BuildServiceProvider(); + } + + private static ActionContext CreateActionContext(IServiceProvider provider) => + new ( + new DefaultHttpContext + { + RequestServices = provider, + Response = { Body = new MemoryStream() } + }, + new RouteData(), + new ActionDescriptor() + ); + + private static JsonSerializerOptions CreateSerializerOptions() + { + var serializerOptions = new JsonSerializerOptions + { + TypeInfoResolver = NoContractsJsonTypeInfoResolver.Instance + }; + serializerOptions.AddDefaultPortableResultsHttpWriteJsonConverters(); + return serializerOptions; + } +} + +/// +/// A resolver that supplies no contract for any type. +/// +public sealed class NoContractsJsonTypeInfoResolver : IJsonTypeInfoResolver +{ + public static NoContractsJsonTypeInfoResolver Instance { get; } = new (); + + public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options) => null; +} diff --git a/tests/Light.PortableResults.Tests/SharedJsonSerialization/PortableResultsJsonContractsTests.cs b/tests/Light.PortableResults.Tests/SharedJsonSerialization/PortableResultsJsonContractsTests.cs new file mode 100644 index 0000000..3365bdb --- /dev/null +++ b/tests/Light.PortableResults.Tests/SharedJsonSerialization/PortableResultsJsonContractsTests.cs @@ -0,0 +1,366 @@ +using System; +using System.IO; +using System.Text; +using System.Text.Json; +using System.Text.Json.Serialization; +using FluentAssertions; +using Light.PortableResults.CloudEvents.Reading; +using Light.PortableResults.CloudEvents.Reading.Json; +using Light.PortableResults.Metadata; +using Light.PortableResults.SharedJsonSerialization; +using Light.PortableResults.Tests.SourceGeneratedSerialization; +using Xunit; + +namespace Light.PortableResults.Tests.SharedJsonSerialization; + +public sealed class PortableResultsJsonContractsTests +{ + [Fact] + public void CreateMissingTypeInfoMessageShouldRejectNullType() + { + var act = () => PortableResultsJsonContracts.CreateMissingTypeInfoMessage(null!); + + act.Should().Throw().WithParameterName("type"); + } + + [Fact] + public void CreateMissingLibraryContractMessageShouldRejectNullType() + { + var act = () => PortableResultsJsonContracts.CreateMissingLibraryContractMessage(null!); + + act.Should().Throw().WithParameterName("type"); + } + + [Fact] + public void EnsureTypeInfoResolverShouldRejectNullOptions() + { + var act = () => PortableResultsJsonContracts.EnsureTypeInfoResolver(null!); + + act.Should().Throw().WithParameterName("options"); + } + + [Fact] + public void GetRequiredTypeInfoShouldRejectNullOptions() + { + var act = () => PortableResultsJsonContracts.GetRequiredTypeInfo(null!); + + act.Should().Throw().WithParameterName("options"); + } + + [Fact] + public void GetLibraryTypeInfoShouldRejectNullOptions() + { + var act = () => PortableResultsJsonContracts.GetLibraryTypeInfo(null!); + + act.Should().Throw().WithParameterName("options"); + } + + [Fact] + public void WriteLibraryValueShouldRejectNullWriter() + { + var act = () => PortableResultsJsonContracts.WriteLibraryValue( + null!, + new CloudEventsSuccessPayload(null), + new JsonSerializerOptions() + ); + + act.Should().Throw().WithParameterName("writer"); + } + + [Fact] + public void WriteLibraryValueShouldRejectNullOptions() + { + using var stream = new MemoryStream(); + using var writer = new Utf8JsonWriter(stream); + + var act = () => PortableResultsJsonContracts.WriteLibraryValue( + writer, + new CloudEventsSuccessPayload(null), + null! + ); + + act.Should().Throw().WithParameterName("options"); + } + + [Fact] + public void EnsureTypeInfoResolverShouldMaterializeTheReflectionResolver() + { + var options = PortableResultsCloudEventsReadingModule.CreateDefaultSerializerOptions(); + options.TypeInfoResolver.Should().BeNull(); + + PortableResultsJsonContracts.EnsureTypeInfoResolver(options); + + options.TypeInfoResolver.Should().NotBeNull(); + } + + [Fact] + public void EnsureTypeInfoResolverShouldKeepAConfiguredResolver() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(); + options.AddDefaultPortableResultsCloudEventsReadJsonConverters(); + + PortableResultsJsonContracts.EnsureTypeInfoResolver(options); + + options.TypeInfoResolver.Should().BeSameAs(MovieJsonContext.Default); + } + + [Fact] + public void GetLibraryTypeInfoShouldCreateTheContractAfterTheOptionsBecameReadOnly() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(); + options.AddDefaultPortableResultsCloudEventsReadJsonConverters(); + options.MakeReadOnly(); + + var typeInfo = PortableResultsJsonContracts.GetLibraryTypeInfo(options); + + typeInfo.Should().NotBeNull(); + typeInfo.Type.Should().Be(typeof(CloudEventsSuccessPayload)); + } + + [Fact] + public void GetLibraryTypeInfoShouldCreateTheContractOnlyOncePerOptionsInstance() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(); + options.AddDefaultPortableResultsCloudEventsReadJsonConverters(); + options.MakeReadOnly(); + + var first = PortableResultsJsonContracts.GetLibraryTypeInfo(options); + var second = PortableResultsJsonContracts.GetLibraryTypeInfo(options); + + second.Should().BeSameAs(first); + } + + [Fact] + public void GetLibraryTypeInfoShouldCreateSeparateContractsPerOptionsInstance() + { + var firstOptions = SourceGeneratedOptions.CreateSerializerOptions(); + firstOptions.AddDefaultPortableResultsCloudEventsReadJsonConverters(); + firstOptions.MakeReadOnly(); + var secondOptions = SourceGeneratedOptions.CreateSerializerOptions(); + secondOptions.AddDefaultPortableResultsCloudEventsReadJsonConverters(); + secondOptions.MakeReadOnly(); + + var first = PortableResultsJsonContracts.GetLibraryTypeInfo(firstOptions); + var second = PortableResultsJsonContracts.GetLibraryTypeInfo(secondOptions); + + second.Should().NotBeSameAs(first); + } + + [Fact] + public void GetLibraryTypeInfoShouldPreferTheContractOfTheConfiguredResolver() + { + var options = PortableResultsCloudEventsReadingModule.CreateDefaultSerializerOptions(); + + var typeInfo = PortableResultsJsonContracts.GetLibraryTypeInfo(options); + + typeInfo.Should().BeSameAs(options.GetTypeInfo(typeof(CloudEventsSuccessPayload))); + } + + [Fact] + public void GetLibraryTypeInfoShouldUseTheConverterRegisteredBeforeTheDefaults() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(NoContractsJsonTypeInfoResolver.Instance); + options.Converters.Add(new FixedSuccessPayloadConverter()); + options.AddDefaultPortableResultsCloudEventsReadJsonConverters(); + + var typeInfo = PortableResultsJsonContracts.GetLibraryTypeInfo(options); + var payload = JsonSerializer.Deserialize("{}"u8, typeInfo); + + payload.Metadata.Should().Be(FixedSuccessPayloadConverter.Metadata); + } + + [Fact] + public void GetLibraryTypeInfoShouldFallBackToTheLibraryConverterWhenNoneIsRegistered() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(NoContractsJsonTypeInfoResolver.Instance); + + var typeInfo = PortableResultsJsonContracts.GetLibraryTypeInfo( + options, + static () => new CloudEventsSuccessPayloadJsonConverter() + ); + var payload = JsonSerializer.Deserialize("""{"metadata":{"note":"body"}}"""u8, typeInfo); + + payload.Metadata.Should().Be(MetadataObject.Create(("note", MetadataValue.FromString("body")))); + } + + [Fact] + public void GetLibraryTypeInfoShouldSkipConvertersThatCannotProduceAMatchingConverter() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(NoContractsJsonTypeInfoResolver.Instance); + options.Converters.Add(new NullReturningConverterFactory()); + options.Converters.Add(new UntypedGreedyConverter()); + options.Converters.Add(new FixedSuccessPayloadConverter()); + + var typeInfo = PortableResultsJsonContracts.GetLibraryTypeInfo(options); + var payload = JsonSerializer.Deserialize("{}"u8, typeInfo); + + payload.Metadata.Should().Be(FixedSuccessPayloadConverter.Metadata); + } + + [Fact] + public void GetLibraryTypeInfoShouldThrowWhenNeitherContractNorConverterIsAvailable() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(NoContractsJsonTypeInfoResolver.Instance); + + var act = () => PortableResultsJsonContracts.GetLibraryTypeInfo(options); + + act.Should().Throw() + .WithMessage( + PortableResultsJsonContracts.CreateMissingLibraryContractMessage(typeof(CloudEventsSuccessPayload)) + ); + } + + [Fact] + public void GetRequiredTypeInfoShouldThrowWhenTheConsumerTypeIsNotRegistered() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(NoContractsJsonTypeInfoResolver.Instance); + + var act = () => PortableResultsJsonContracts.GetRequiredTypeInfo(options); + + act.Should().Throw() + .WithMessage(PortableResultsJsonContracts.CreateMissingTypeInfoMessage(typeof(MovieDto))); + } + + [Fact] + public void WriteLibraryValueShouldUseTheConverterRegisteredBeforeTheDefaults() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(NoContractsJsonTypeInfoResolver.Instance); + options.Converters.Add(new FixedSuccessPayloadConverter()); + using var stream = new MemoryStream(); + + using (var writer = new Utf8JsonWriter(stream)) + { + PortableResultsJsonContracts.WriteLibraryValue(writer, new CloudEventsSuccessPayload(null), options); + } + + Encoding.UTF8.GetString(stream.ToArray()).Should().Be(FixedSuccessPayloadConverter.WrittenJson); + } + + [Fact] + public void WriteLibraryValueShouldUseTheContractOfTheConfiguredResolver() + { + var options = PortableResultsCloudEventsReadingModule.CreateDefaultSerializerOptions(); + options.Converters.Insert(0, new FixedSuccessPayloadConverter()); + using var stream = new MemoryStream(); + + using (var writer = new Utf8JsonWriter(stream)) + { + PortableResultsJsonContracts.WriteLibraryValue(writer, new CloudEventsSuccessPayload(null), options); + } + + options.TypeInfoResolver.Should().NotBeNull(); + Encoding.UTF8.GetString(stream.ToArray()).Should().Be(FixedSuccessPayloadConverter.WrittenJson); + } + + [Fact] + public void WriteLibraryValueShouldReuseTheResolvedConverter() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(NoContractsJsonTypeInfoResolver.Instance); + var converter = new CountingSuccessPayloadConverter(); + options.Converters.Add(converter); + options.MakeReadOnly(); + + WriteSuccessPayload(options); + WriteSuccessPayload(options); + + converter.CanConvertCallCount.Should().Be(1); + } + + [Fact] + public void WriteLibraryValueShouldThrowWhenNeitherContractNorConverterIsAvailable() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(NoContractsJsonTypeInfoResolver.Instance); + using var stream = new MemoryStream(); + using var writer = new Utf8JsonWriter(stream); + + var act = () => PortableResultsJsonContracts.WriteLibraryValue( + writer, + new CloudEventsSuccessPayload(null), + options + ); + + act.Should().Throw() + .WithMessage( + PortableResultsJsonContracts.CreateMissingLibraryContractMessage(typeof(CloudEventsSuccessPayload)) + ); + } + + private static void WriteSuccessPayload(JsonSerializerOptions options) + { + using var stream = new MemoryStream(); + using var writer = new Utf8JsonWriter(stream); + PortableResultsJsonContracts.WriteLibraryValue(writer, new CloudEventsSuccessPayload(null), options); + } + + private sealed class FixedSuccessPayloadConverter : JsonConverter + { + public const string WrittenJson = """{"fixed":true}"""; + + public static MetadataObject Metadata { get; } = + MetadataObject.Create(("origin", MetadataValue.FromString("custom-converter"))); + + public override CloudEventsSuccessPayload Read( + ref Utf8JsonReader reader, + Type typeToConvert, + JsonSerializerOptions options + ) + { + reader.Skip(); + return new CloudEventsSuccessPayload(Metadata); + } + + public override void Write( + Utf8JsonWriter writer, + CloudEventsSuccessPayload value, + JsonSerializerOptions options + ) + { + writer.WriteStartObject(); + writer.WriteBoolean("fixed", true); + writer.WriteEndObject(); + } + } + + private sealed class NullReturningConverterFactory : JsonConverterFactory + { + public override bool CanConvert(Type typeToConvert) => typeToConvert == typeof(CloudEventsSuccessPayload); + + public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options) => null; + } + + private sealed class UntypedGreedyConverter : JsonConverter + { + public override bool CanConvert(Type typeToConvert) => typeToConvert == typeof(CloudEventsSuccessPayload); + + public override string Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => + throw new NotSupportedException(); + + public override void Write(Utf8JsonWriter writer, string value, JsonSerializerOptions options) => + throw new NotSupportedException(); + } + + private sealed class CountingSuccessPayloadConverter : JsonConverter + { + public int CanConvertCallCount { get; private set; } + + public override bool CanConvert(Type typeToConvert) + { + CanConvertCallCount++; + return base.CanConvert(typeToConvert); + } + + public override CloudEventsSuccessPayload Read( + ref Utf8JsonReader reader, + Type typeToConvert, + JsonSerializerOptions options + ) => + throw new NotSupportedException(); + + public override void Write( + Utf8JsonWriter writer, + CloudEventsSuccessPayload value, + JsonSerializerOptions options + ) => + writer.WriteNullValue(); + } +} diff --git a/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/ConverterPrecedenceTests.cs b/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/ConverterPrecedenceTests.cs new file mode 100644 index 0000000..91dae72 --- /dev/null +++ b/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/ConverterPrecedenceTests.cs @@ -0,0 +1,181 @@ +using System; +using System.Text; +using System.Text.Json; +using System.Text.Json.Serialization; +using System.Threading.Tasks; +using FluentAssertions; +using Light.PortableResults.CloudEvents.Reading; +using Light.PortableResults.CloudEvents.Reading.Json; +using Light.PortableResults.CloudEvents.Writing; +using Light.PortableResults.Http.Reading; +using Light.PortableResults.Http.Reading.Json; +using Xunit; + +namespace Light.PortableResults.Tests.SourceGeneratedSerialization; + +/// +/// Verifies that converters registered before the Light.PortableResults defaults keep their precedence, both with a +/// reflection-backed resolver and with a source-generated resolver that cannot supply the wrapper contract. +/// +public sealed class ConverterPrecedenceTests +{ + private const string SuccessCloudEvent = + """ + { + "specversion": "1.0", + "type": "movie.created", + "source": "urn:test:source", + "id": "evt-1", + "lproutcome": "success", + "datacontenttype": "application/json", + "data": { "value": { "title": "Blade Runner", "year": 1982 } } + } + """; + + private const string SuccessHttpBody = """{"value":{"title":"Blade Runner","year":1982}}"""; + private static readonly MovieDto ReplacementMovie = new ("Replaced by converter", 2026); + + [Fact] + public void CloudEventsReadShouldUseTheConverterRegisteredBeforeTheDefaults() + { + var options = SourceGeneratedOptions.CreateCloudEventsReadOptions( + configureBeforeDefaults: serializerOptions => + serializerOptions.Converters.Add(new ReplacingCloudEventsAutoSuccessPayloadConverter()) + ); + var cloudEvent = new ReadOnlyMemory(Encoding.UTF8.GetBytes(SuccessCloudEvent)); + + var result = cloudEvent.ReadResult(options); + + result.Should().Be(Result.Ok(ReplacementMovie)); + } + + [Fact] + public void CloudEventsReadShouldUseTheConverterRegisteredBeforeTheDefaultsWithReflection() + { + var serializerOptions = PortableResultsCloudEventsReadingModule.CreateDefaultSerializerOptions(); + serializerOptions.Converters.Insert(0, new ReplacingCloudEventsAutoSuccessPayloadConverter()); + var options = new PortableResultsCloudEventsReadOptions { SerializerOptions = serializerOptions }; + var cloudEvent = new ReadOnlyMemory(Encoding.UTF8.GetBytes(SuccessCloudEvent)); + + var result = cloudEvent.ReadResult(options); + + result.Should().Be(Result.Ok(ReplacementMovie)); + } + + [Fact] + public async Task HttpReadShouldUseTheConverterRegisteredBeforeTheDefaults() + { + var cancellationToken = TestContext.Current.CancellationToken; + var options = SourceGeneratedOptions.CreateHttpReadOptions( + configureBeforeDefaults: serializerOptions => + serializerOptions.Converters.Add(new ReplacingHttpReadAutoSuccessResultPayloadConverter()) + ); + using var response = SourceGeneratedOptions.CreateResponse(Encoding.UTF8.GetBytes(SuccessHttpBody)); + + var result = await response.ReadResultAsync(options, cancellationToken); + + result.Should().Be(Result.Ok(ReplacementMovie)); + } + + [Fact] + public void CloudEventsWriteShouldUseTheConverterRegisteredBeforeTheDefaults() + { + var serializerOptions = SourceGeneratedOptions.CreateSerializerOptions(); + serializerOptions.Converters.Add(new SentinelCloudEventsEnvelopeForWritingConverter()); + serializerOptions.AddDefaultPortableResultsCloudEventsWriteJsonConverters(); + var options = new PortableResultsCloudEventsWriteOptions + { + SerializerOptions = serializerOptions, + Source = "urn:test:source" + }; + + var cloudEvent = Result.Ok(new MovieDto("Blade Runner", 1982)) + .ToCloudEvent(successType: "movie.created", id: "evt-1", options: options); + + Encoding.UTF8.GetString(cloudEvent).Should().Be(SentinelCloudEventsEnvelopeForWritingConverter.WrittenJson); + } + + [Fact] + public void CloudEventsWriteShouldUseTheConverterRegisteredBeforeTheDefaultsWithReflection() + { + var serializerOptions = new JsonSerializerOptions(JsonSerializerDefaults.Web); + serializerOptions.Converters.Add(new SentinelCloudEventsEnvelopeForWritingConverter()); + serializerOptions.AddDefaultPortableResultsCloudEventsWriteJsonConverters(); + var options = new PortableResultsCloudEventsWriteOptions + { + SerializerOptions = serializerOptions, + Source = "urn:test:source" + }; + + var cloudEvent = Result.Ok(new MovieDto("Blade Runner", 1982)) + .ToCloudEvent(successType: "movie.created", id: "evt-1", options: options); + + Encoding.UTF8.GetString(cloudEvent).Should().Be(SentinelCloudEventsEnvelopeForWritingConverter.WrittenJson); + } + + private sealed class ReplacingCloudEventsAutoSuccessPayloadConverter + : JsonConverter> + { + public override CloudEventsAutoSuccessPayload Read( + ref Utf8JsonReader reader, + Type typeToConvert, + JsonSerializerOptions options + ) + { + reader.Skip(); + return new CloudEventsAutoSuccessPayload(ReplacementMovie, metadata: null); + } + + public override void Write( + Utf8JsonWriter writer, + CloudEventsAutoSuccessPayload value, + JsonSerializerOptions options + ) => + throw new NotSupportedException(); + } + + private sealed class ReplacingHttpReadAutoSuccessResultPayloadConverter + : JsonConverter> + { + public override HttpReadAutoSuccessResultPayload Read( + ref Utf8JsonReader reader, + Type typeToConvert, + JsonSerializerOptions options + ) + { + reader.Skip(); + return new HttpReadAutoSuccessResultPayload(ReplacementMovie, metadata: null); + } + + public override void Write( + Utf8JsonWriter writer, + HttpReadAutoSuccessResultPayload value, + JsonSerializerOptions options + ) => + throw new NotSupportedException(); + } + + private sealed class SentinelCloudEventsEnvelopeForWritingConverter + : JsonConverter> + { + public const string WrittenJson = """{"replacedBy":"custom-converter"}"""; + + public override CloudEventsEnvelopeForWriting Read( + ref Utf8JsonReader reader, + Type typeToConvert, + JsonSerializerOptions options + ) => + throw new NotSupportedException(); + + public override void Write( + Utf8JsonWriter writer, + CloudEventsEnvelopeForWriting value, + JsonSerializerOptions options + ) + { + writer.WriteStartObject(); + writer.WriteString("replacedBy", "custom-converter"); + writer.WriteEndObject(); + } + } +} diff --git a/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/MissingValueTypeMetadataTests.cs b/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/MissingValueTypeMetadataTests.cs new file mode 100644 index 0000000..d9d3ef6 --- /dev/null +++ b/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/MissingValueTypeMetadataTests.cs @@ -0,0 +1,140 @@ +using System; +using System.IO; +using System.Text; +using System.Text.Json; +using System.Threading.Tasks; +using FluentAssertions; +using Light.PortableResults.CloudEvents.Reading; +using Light.PortableResults.CloudEvents.Writing; +using Light.PortableResults.Http.Reading; +using Light.PortableResults.Http.Reading.Json; +using Light.PortableResults.Http.Writing; +using Light.PortableResults.Http.Writing.Json; +using Light.PortableResults.Metadata; +using Light.PortableResults.SharedJsonSerialization; +using Light.PortableResults.SharedJsonSerialization.Writing; +using Xunit; + +namespace Light.PortableResults.Tests.SourceGeneratedSerialization; + +/// +/// Verifies that every entry point which resolves a consumer-owned type reports the unresolved type and the remedy +/// instead of failing with a System.Text.Json error about missing metadata. +/// +public sealed class MissingValueTypeMetadataTests +{ + private const string CloudEventJson = + """ + { + "specversion": "1.0", + "type": "movie.created", + "source": "urn:test:source", + "id": "evt-1", + "lproutcome": "success", + "datacontenttype": "application/json", + "data": { "value": { "title": "Blade Runner", "year": 1982 } } + } + """; + + private const string HttpBodyJson = """{"value":{"title":"Blade Runner","year":1982}}"""; + + private static string ExpectedMessage => PortableResultsJsonContracts.CreateMissingTypeInfoMessage(typeof(MovieDto)); + + [Theory] + [InlineData(PreferSuccessPayload.Auto)] + [InlineData(PreferSuccessPayload.BareValue)] + [InlineData(PreferSuccessPayload.WrappedValue)] + public void CloudEventsReadShouldNameTheUnresolvedValueType(PreferSuccessPayload preferSuccessPayload) + { + var options = SourceGeneratedOptions.CreateCloudEventsReadOptions(NoContractsJsonTypeInfoResolver.Instance) + with + { + PreferSuccessPayload = preferSuccessPayload + }; + var cloudEvent = new ReadOnlyMemory(Encoding.UTF8.GetBytes(CloudEventJson)); + + var act = () => cloudEvent.ReadResult(options); + + act.Should().Throw().WithMessage(ExpectedMessage); + } + + [Theory] + [InlineData(PreferSuccessPayload.Auto)] + [InlineData(PreferSuccessPayload.BareValue)] + [InlineData(PreferSuccessPayload.WrappedValue)] + public async Task HttpReadShouldNameTheUnresolvedValueType(PreferSuccessPayload preferSuccessPayload) + { + var cancellationToken = TestContext.Current.CancellationToken; + var options = SourceGeneratedOptions.CreateHttpReadOptions(NoContractsJsonTypeInfoResolver.Instance) with + { + PreferSuccessPayload = preferSuccessPayload + }; + using var response = SourceGeneratedOptions.CreateResponse(Encoding.UTF8.GetBytes(HttpBodyJson)); + + // ReSharper disable once AccessToDisposedClosure -- act is called before disposal + var act = async () => await response.ReadResultAsync(options, cancellationToken); + + await act.Should().ThrowAsync().WithMessage(ExpectedMessage); + } + + [Fact] + public void CloudEventsWriteShouldNameTheUnresolvedValueType() + { + var result = Result.Ok(new MovieDto("Blade Runner", 1982)); + var options = SourceGeneratedOptions.CreateCloudEventsWriteOptions(NoContractsJsonTypeInfoResolver.Instance); + + var act = () => result.ToCloudEvent(successType: "movie.created", id: "evt-1", options: options); + + act.Should().Throw().WithMessage(ExpectedMessage); + } + + [Fact] + public void HttpWriteShouldNameTheUnresolvedValueType() + { + var result = Result.Ok(new MovieDto("Blade Runner", 1982)); + var options = SourceGeneratedOptions.CreateHttpWriteOptions(NoContractsJsonTypeInfoResolver.Instance); + using var stream = new MemoryStream(); + using var writer = new Utf8JsonWriter(stream); + + var act = () => new HttpResultForWritingJsonConverter().Write( + writer, + result.ToHttpResultForWriting(new PortableResultsHttpWriteOptions()), + options + ); + + act.Should().Throw().WithMessage(ExpectedMessage); + } + + [Fact] + public void WriteGenericValueShouldNameTheUnresolvedValueType() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(NoContractsJsonTypeInfoResolver.Instance); + using var stream = new MemoryStream(); + using var writer = new Utf8JsonWriter(stream); + + var act = () => writer.WriteGenericValue(new MovieDto("Blade Runner", 1982), options); + + act.Should().Throw().WithMessage(ExpectedMessage); + } + + [Fact] + public void WriteRichErrorsShouldNameTheUnresolvedMetadataContract() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(NoContractsJsonTypeInfoResolver.Instance); + var errors = new Errors( + new Error + { + Message = "The movie was rejected.", + Metadata = MetadataObject.Create(("attempt", MetadataValue.FromString("1"))) + } + ); + using var stream = new MemoryStream(); + using var writer = new Utf8JsonWriter(stream); + writer.WriteStartObject(); + + var act = () => writer.WriteRichErrors(errors, isValidationResponse: false, options); + + act.Should().Throw() + .WithMessage(PortableResultsJsonContracts.CreateMissingLibraryContractMessage(typeof(MetadataObject))); + } +} diff --git a/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/MovieDto.cs b/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/MovieDto.cs new file mode 100644 index 0000000..04a40fe --- /dev/null +++ b/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/MovieDto.cs @@ -0,0 +1,7 @@ +namespace Light.PortableResults.Tests.SourceGeneratedSerialization; + +/// +/// The consumer-owned result value type used by the source-generation tests. It is the only type the test contexts +/// declare - every Light.PortableResults envelope and payload type must be resolved by the library itself. +/// +public sealed record MovieDto(string Title, int Year); diff --git a/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/MovieJsonContext.cs b/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/MovieJsonContext.cs new file mode 100644 index 0000000..d4c7c5c --- /dev/null +++ b/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/MovieJsonContext.cs @@ -0,0 +1,24 @@ +using System; +using System.Text.Json; +using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; + +namespace Light.PortableResults.Tests.SourceGeneratedSerialization; + +/// +/// A source-generated resolver that declares the consumer's result value type and nothing else. +/// +[JsonSourceGenerationOptions(JsonSerializerDefaults.Web)] +[JsonSerializable(typeof(MovieDto))] +public sealed partial class MovieJsonContext : JsonSerializerContext; + +/// +/// A resolver that supplies no contract for any type. It proves that non-generic results require no consumer +/// registration, because every type involved is resolved by Light.PortableResults itself. +/// +public sealed class NoContractsJsonTypeInfoResolver : IJsonTypeInfoResolver +{ + public static NoContractsJsonTypeInfoResolver Instance { get; } = new (); + + public JsonTypeInfo? GetTypeInfo(Type type, JsonSerializerOptions options) => null; +} diff --git a/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/SourceGeneratedRoundTripTests.cs b/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/SourceGeneratedRoundTripTests.cs new file mode 100644 index 0000000..560afab --- /dev/null +++ b/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/SourceGeneratedRoundTripTests.cs @@ -0,0 +1,293 @@ +using System; +using System.IO; +using System.Net; +using System.Net.Http; +using System.Text; +using System.Text.Json; +using System.Text.Json.Serialization.Metadata; +using System.Threading.Tasks; +using FluentAssertions; +using Light.PortableResults.CloudEvents.Reading; +using Light.PortableResults.CloudEvents.Writing; +using Light.PortableResults.Http.Reading; +using Light.PortableResults.Http.Writing; +using Light.PortableResults.Http.Writing.Json; +using Light.PortableResults.Metadata; +using Light.PortableResults.SharedJsonSerialization; +using Xunit; + +namespace Light.PortableResults.Tests.SourceGeneratedSerialization; + +/// +/// Verifies that results round-trip when the configured have no +/// reflection-backed resolver and the source-generated context declares nothing but the result value type. +/// +public sealed class SourceGeneratedRoundTripTests +{ + private static readonly MovieDto Movie = new ("Blade Runner", 1982); + + [Fact] + public void GenericResultShouldRoundTripOverCloudEvents() + { + var result = Result.Ok(Movie, MetadataObject.Create(("note", MetadataValue.FromString("body")))); + + var cloudEvent = result.ToCloudEvent( + successType: "movie.created", + failureType: "movie.rejected", + id: "evt-1", + options: SourceGeneratedOptions.CreateCloudEventsWriteOptions() + ); + + var readResult = new ReadOnlyMemory(cloudEvent) + .ReadResult(SourceGeneratedOptions.CreateCloudEventsReadOptions()); + + readResult.Should().Be(result); + } + + [Fact] + public void FailedGenericResultShouldRoundTripOverCloudEvents() + { + var result = Result.Fail(CreateFailure()); + + var cloudEvent = result.ToCloudEvent( + successType: "movie.created", + failureType: "movie.rejected", + id: "evt-2", + options: SourceGeneratedOptions.CreateCloudEventsWriteOptions() + ); + + var readResult = new ReadOnlyMemory(cloudEvent) + .ReadResult(SourceGeneratedOptions.CreateCloudEventsReadOptions()); + + readResult.Should().Be(result); + } + + [Fact] + public void NonGenericResultShouldRoundTripOverCloudEventsWithoutAnyConsumerRegistration() + { + var result = Result.Ok(MetadataObject.Create(("note", MetadataValue.FromString("body")))); + + var cloudEvent = result.ToCloudEvent( + successType: "note.created", + failureType: "note.rejected", + id: "evt-3", + options: SourceGeneratedOptions.CreateCloudEventsWriteOptions(NoContractsJsonTypeInfoResolver.Instance) + ); + + var readResult = new ReadOnlyMemory(cloudEvent) + .ReadResult(SourceGeneratedOptions.CreateCloudEventsReadOptions(NoContractsJsonTypeInfoResolver.Instance)); + + readResult.Should().Be(result); + } + + [Fact] + public void FailedNonGenericResultShouldRoundTripOverCloudEventsWithoutAnyConsumerRegistration() + { + var result = Result.Fail(CreateFailure()); + + var cloudEvent = result.ToCloudEvent( + successType: "note.created", + failureType: "note.rejected", + id: "evt-4", + options: SourceGeneratedOptions.CreateCloudEventsWriteOptions(NoContractsJsonTypeInfoResolver.Instance) + ); + + var readResult = new ReadOnlyMemory(cloudEvent) + .ReadResult(SourceGeneratedOptions.CreateCloudEventsReadOptions(NoContractsJsonTypeInfoResolver.Instance)); + + readResult.Should().Be(result); + } + + [Fact] + public async Task GenericResultShouldRoundTripOverHttp() + { + var cancellationToken = TestContext.Current.CancellationToken; + var result = Result.Ok(Movie, MetadataObject.Create(("note", MetadataValue.FromString("body")))); + + using var response = SourceGeneratedOptions.CreateResponse(WriteGenericHttpBody(result)); + + var readResult = await response.ReadResultAsync( + SourceGeneratedOptions.CreateHttpReadOptions(), + cancellationToken + ); + + readResult.Should().Be(result); + } + + [Fact] + public async Task FailedGenericResultShouldRoundTripOverHttp() + { + var cancellationToken = TestContext.Current.CancellationToken; + var result = Result.Fail(CreateFailure()); + + using var response = SourceGeneratedOptions.CreateResponse(WriteGenericHttpBody(result)); + + var readResult = await response.ReadResultAsync( + SourceGeneratedOptions.CreateHttpReadOptions(), + cancellationToken + ); + + readResult.Should().Be(result); + } + + [Fact] + public async Task NonGenericResultShouldRoundTripOverHttpWithoutAnyConsumerRegistration() + { + var cancellationToken = TestContext.Current.CancellationToken; + var result = Result.Ok(MetadataObject.Create(("note", MetadataValue.FromString("body")))); + + using var response = SourceGeneratedOptions.CreateResponse(WriteNonGenericHttpBody(result)); + + var readResult = await response.ReadResultAsync( + SourceGeneratedOptions.CreateHttpReadOptions(NoContractsJsonTypeInfoResolver.Instance), + cancellationToken + ); + + readResult.Should().Be(result); + } + + [Fact] + public async Task FailedNonGenericResultShouldRoundTripOverHttpWithoutAnyConsumerRegistration() + { + var cancellationToken = TestContext.Current.CancellationToken; + var result = Result.Fail(CreateFailure()); + + using var response = SourceGeneratedOptions.CreateResponse(WriteNonGenericHttpBody(result)); + + var readResult = await response.ReadResultAsync( + SourceGeneratedOptions.CreateHttpReadOptions(NoContractsJsonTypeInfoResolver.Instance), + cancellationToken + ); + + readResult.Should().Be(result); + } + + private static PortableResultsHttpWriteOptions CreateHttpWriteOptions() => + new () { MetadataSerializationMode = MetadataSerializationMode.Always }; + + private static Errors CreateFailure() => + new ( + new Error + { + Message = "The movie was rejected.", + Code = "MOVIE_REJECTED", + Target = "title", + Category = ErrorCategory.Validation, + Metadata = MetadataObject.Create(("attempt", MetadataValue.FromString("1"))) + } + ); + + // The HTTP write path resolves HttpResultForWriting through the configured resolver, which is deliberately + // left to the consumer. The converter is therefore invoked directly so that the source-generated context only + // has to declare the value type, which is what the HTTP read path under test requires. + private static byte[] WriteGenericHttpBody(Result result) => + SourceGeneratedOptions.WriteHttpBody( + writer => new HttpResultForWritingJsonConverter().Write( + writer, + result.ToHttpResultForWriting(CreateHttpWriteOptions()), + SourceGeneratedOptions.CreateHttpWriteOptions() + ) + ); + + private static byte[] WriteNonGenericHttpBody(Result result) => + SourceGeneratedOptions.WriteHttpBody( + writer => HttpResultForWritingJsonConverter.Instance.Write( + writer, + result.ToHttpResultForWriting(CreateHttpWriteOptions()), + SourceGeneratedOptions.CreateHttpWriteOptions(NoContractsJsonTypeInfoResolver.Instance) + ) + ); +} + +/// +/// Creates Light.PortableResults options whose only type info resolver is a source-generated context. +/// +public static class SourceGeneratedOptions +{ + public static PortableResultsCloudEventsWriteOptions CreateCloudEventsWriteOptions( + IJsonTypeInfoResolver? resolver = null + ) + { + var serializerOptions = CreateSerializerOptions(resolver); + serializerOptions.AddDefaultPortableResultsCloudEventsWriteJsonConverters(); + return new PortableResultsCloudEventsWriteOptions + { + SerializerOptions = serializerOptions, + Source = "urn:test:source" + }; + } + + public static PortableResultsCloudEventsReadOptions CreateCloudEventsReadOptions( + IJsonTypeInfoResolver? resolver = null, + Action? configureBeforeDefaults = null + ) + { + var serializerOptions = CreateSerializerOptions(resolver); + configureBeforeDefaults?.Invoke(serializerOptions); + serializerOptions.AddDefaultPortableResultsCloudEventsReadJsonConverters(); + return new PortableResultsCloudEventsReadOptions { SerializerOptions = serializerOptions }; + } + + public static JsonSerializerOptions CreateHttpWriteOptions(IJsonTypeInfoResolver? resolver = null) + { + var serializerOptions = CreateSerializerOptions(resolver); + serializerOptions.AddDefaultPortableResultsHttpWriteJsonConverters(); + return serializerOptions; + } + + public static PortableResultsHttpReadOptions CreateHttpReadOptions( + IJsonTypeInfoResolver? resolver = null, + Action? configureBeforeDefaults = null + ) + { + var serializerOptions = CreateSerializerOptions(resolver); + configureBeforeDefaults?.Invoke(serializerOptions); + serializerOptions.AddDefaultPortableResultsHttpReadJsonConverters(); + return new PortableResultsHttpReadOptions { SerializerOptions = serializerOptions }; + } + + public static JsonSerializerOptions CreateSerializerOptions(IJsonTypeInfoResolver? resolver = null) => + new (JsonSerializerDefaults.Web) + { + TypeInfoResolver = resolver ?? MovieJsonContext.Default + }; + + public static byte[] WriteHttpBody(Action write) + { + using var stream = new MemoryStream(); + using (var writer = new Utf8JsonWriter(stream)) + { + write(writer); + } + + return stream.ToArray(); + } + + // The status code and content type are derived from the written body so that the response mirrors what the + // Light.PortableResults ASP.NET Core integrations would emit for the same result. + public static HttpResponseMessage CreateResponse(byte[] body) + { + var json = Encoding.UTF8.GetString(body); + var statusCode = HttpStatusCode.OK; + var isProblemDetails = false; + + using (var document = JsonDocument.Parse(json)) + { + if (document.RootElement.ValueKind == JsonValueKind.Object && + document.RootElement.TryGetProperty("status", out var status)) + { + isProblemDetails = true; + statusCode = (HttpStatusCode) status.GetInt32(); + } + } + + return new HttpResponseMessage(statusCode) + { + Content = new StringContent( + json, + Encoding.UTF8, + isProblemDetails ? "application/problem+json" : "application/json" + ) + }; + } +} From 2268b4655e42abc1603a9d30ae325b00c856e933 Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Sun, 2 Aug 2026 22:25:58 +0200 Subject: [PATCH 09/15] fix(aot): flush the writer and cache contracts on the fallback path WriteLibraryValue invoked the resolved converter directly, which does not flush the writer, while the resolver path went through JsonSerializer, which does. The public WriteCloudEvent overloads taking a Utf8JsonWriter therefore left the event buffered whenever the envelope contract came from a library-owned converter: whether a caller saw output before flushing or disposing the writer depended on how they had configured their resolver. Contracts and converters were also only cached against read-only options. Invoking a converter directly never freezes them, so the CloudEvents write path re-ran TryGetTypeInfo and the full converter scan on every write and never populated the cache. Freezing before caching - after the converter is resolved, so that a failed lookup does not freeze the caller's options - makes the cache effective and keeps a converter registered later from invalidating it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VsEiLdTSnUPsGV1UFBxnrg --- .../PortableResultsJsonContracts.cs | 45 ++++++---- .../PortableResultsJsonContractsTests.cs | 25 ++++++ .../CloudEventsWriteFlushTests.cs | 89 +++++++++++++++++++ 3 files changed, 142 insertions(+), 17 deletions(-) create mode 100644 tests/Light.PortableResults.Tests/SourceGeneratedSerialization/CloudEventsWriteFlushTests.cs diff --git a/src/Light.PortableResults/SharedJsonSerialization/PortableResultsJsonContracts.cs b/src/Light.PortableResults/SharedJsonSerialization/PortableResultsJsonContracts.cs index 85b0829..fa40149 100644 --- a/src/Light.PortableResults/SharedJsonSerialization/PortableResultsJsonContracts.cs +++ b/src/Light.PortableResults/SharedJsonSerialization/PortableResultsJsonContracts.cs @@ -18,7 +18,8 @@ namespace Light.PortableResults.SharedJsonSerialization; /// The envelope and payload types owned by Light.PortableResults are resolved by this class: it prefers a contract /// supplied by the configured and otherwise creates a contract /// from the converter that the options select for the type. Created contracts are cached per -/// instance and can be created after the options became read-only. +/// instance and can be created after the options became read-only. Creating one +/// makes the options read-only, so that the cached contract cannot be invalidated by a converter registered later. /// /// public static class PortableResultsJsonContracts @@ -151,6 +152,7 @@ public static JsonTypeInfo GetLibraryTypeInfo( } var converter = ResolveLibraryConverter(options, createLibraryConverter); + FreezeBeforeCaching(options); var createdTypeInfo = JsonMetadataServices.CreateValueInfo(options, converter); return CacheTypeInfo(options, createdTypeInfo); } @@ -159,7 +161,8 @@ public static JsonTypeInfo GetLibraryTypeInfo( /// Writes a value of a type owned by Light.PortableResults. A contract supplied by the configured resolver takes /// precedence; otherwise the converter that select for /// is invoked directly, falling back to - /// when no converter is registered. + /// when no converter is registered. The writer is flushed once the + /// value has been written, regardless of which of the two paths was taken. /// /// The Light.PortableResults type to write. /// The JSON writer receiving the value. @@ -194,7 +197,7 @@ public static void WriteLibraryValue( if (TryGetCachedConverter(options, out var cachedConverter)) { - cachedConverter.Write(writer, value, options); + WriteWithConverter(writer, value, options, cachedConverter); return; } @@ -206,8 +209,23 @@ public static void WriteLibraryValue( return; } - var converter = CacheConverter(options, ResolveLibraryConverter(options, createLibraryConverter)); + var resolvedConverter = ResolveLibraryConverter(options, createLibraryConverter); + FreezeBeforeCaching(options); + WriteWithConverter(writer, value, options, CacheConverter(options, resolvedConverter)); + } + + // JsonSerializer.Serialize(Utf8JsonWriter, ...) flushes the writer once it has written the value, while a + // converter invoked directly does not. Flushing here keeps the observable behavior of the public write APIs + // independent of whether the contract came from the configured resolver or from a library-owned converter. + private static void WriteWithConverter( + Utf8JsonWriter writer, + TLibraryType value, + JsonSerializerOptions options, + JsonConverter converter + ) + { converter.Write(writer, value, options); + writer.Flush(); } // JsonSerializer.IsReflectionEnabledByDefault is a link-time constant: when reflection-based serialization is @@ -297,19 +315,17 @@ private static bool TryGetCachedTypeInfo( return false; } - // Contracts are only cached once the options are frozen. A mutable instance can still gain a converter that - // would change the outcome of the resolution, and freezing happens at the latest when the value is written or - // read with the resolved contract. + // Only a frozen instance may be cached against: a mutable one can still gain a converter that would change the + // outcome of the resolution, which would leave the cache serving a stale contract. Freezing here is not an + // additional behavior change - reading or writing with the resolved contract freezes the options anyway - but it + // has to happen explicitly, because invoking a converter directly does not freeze them. + private static void FreezeBeforeCaching(JsonSerializerOptions options) => options.MakeReadOnly(); + private static JsonTypeInfo CacheTypeInfo( JsonSerializerOptions options, JsonTypeInfo typeInfo ) { - if (!options.IsReadOnly) - { - return typeInfo; - } - var contracts = ContractsPerOptions.GetValue(options, static _ => new LibraryContracts()); return (JsonTypeInfo) contracts.TypeInfos.GetOrAdd(typeof(TLibraryType), typeInfo); } @@ -335,11 +351,6 @@ private static JsonConverter CacheConverter( JsonConverter converter ) { - if (!options.IsReadOnly) - { - return converter; - } - var contracts = ContractsPerOptions.GetValue(options, static _ => new LibraryContracts()); return (JsonConverter) contracts.Converters.GetOrAdd(typeof(TLibraryType), converter); } diff --git a/tests/Light.PortableResults.Tests/SharedJsonSerialization/PortableResultsJsonContractsTests.cs b/tests/Light.PortableResults.Tests/SharedJsonSerialization/PortableResultsJsonContractsTests.cs index 3365bdb..141a813 100644 --- a/tests/Light.PortableResults.Tests/SharedJsonSerialization/PortableResultsJsonContractsTests.cs +++ b/tests/Light.PortableResults.Tests/SharedJsonSerialization/PortableResultsJsonContractsTests.cs @@ -130,6 +130,18 @@ public void GetLibraryTypeInfoShouldCreateTheContractOnlyOncePerOptionsInstance( second.Should().BeSameAs(first); } + [Fact] + public void GetLibraryTypeInfoShouldCreateTheContractOnlyOnceForOptionsTheCallerNeverFroze() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(NoContractsJsonTypeInfoResolver.Instance); + options.Converters.Add(new FixedSuccessPayloadConverter()); + + var first = PortableResultsJsonContracts.GetLibraryTypeInfo(options); + var second = PortableResultsJsonContracts.GetLibraryTypeInfo(options); + + second.Should().BeSameAs(first); + } + [Fact] public void GetLibraryTypeInfoShouldCreateSeparateContractsPerOptionsInstance() { @@ -266,6 +278,19 @@ public void WriteLibraryValueShouldReuseTheResolvedConverter() converter.CanConvertCallCount.Should().Be(1); } + [Fact] + public void WriteLibraryValueShouldReuseTheResolvedConverterForOptionsTheCallerNeverFroze() + { + var options = SourceGeneratedOptions.CreateSerializerOptions(NoContractsJsonTypeInfoResolver.Instance); + var converter = new CountingSuccessPayloadConverter(); + options.Converters.Add(converter); + + WriteSuccessPayload(options); + WriteSuccessPayload(options); + + converter.CanConvertCallCount.Should().Be(1); + } + [Fact] public void WriteLibraryValueShouldThrowWhenNeitherContractNorConverterIsAvailable() { diff --git a/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/CloudEventsWriteFlushTests.cs b/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/CloudEventsWriteFlushTests.cs new file mode 100644 index 0000000..71c61a3 --- /dev/null +++ b/tests/Light.PortableResults.Tests/SourceGeneratedSerialization/CloudEventsWriteFlushTests.cs @@ -0,0 +1,89 @@ +using System; +using System.IO; +using System.Text.Json; +using FluentAssertions; +using Light.PortableResults.CloudEvents.Writing; +using Xunit; + +namespace Light.PortableResults.Tests.SourceGeneratedSerialization; + +/// +/// Verifies that the public CloudEvents write overloads taking a leave the complete +/// event in the output before the caller flushes or disposes the writer. The envelope contract can come either from +/// the configured resolver or from a library-owned converter, and the caller must not have to know which. +/// +public sealed class CloudEventsWriteFlushTests +{ + [Fact] + public void NonGenericWriteShouldFlushWithASourceGeneratedResolver() + { + var options = SourceGeneratedOptions.CreateCloudEventsWriteOptions(NoContractsJsonTypeInfoResolver.Instance); + + var written = WriteWithoutFlushing( + writer => Result.Ok().WriteCloudEvent(writer, successType: "note.created", id: "evt-1", options: options) + ); + + GetId(written).Should().Be("evt-1"); + } + + [Fact] + public void GenericWriteShouldFlushWithASourceGeneratedResolver() + { + var options = SourceGeneratedOptions.CreateCloudEventsWriteOptions(); + var result = Result.Ok(new MovieDto("Blade Runner", 1982)); + + var written = WriteWithoutFlushing( + writer => result.WriteCloudEvent(writer, successType: "movie.created", id: "evt-2", options: options) + ); + + GetId(written).Should().Be("evt-2"); + } + + [Fact] + public void NonGenericWriteShouldFlushWithAReflectionBackedResolver() + { + var options = CreateReflectionBackedOptions(); + + var written = WriteWithoutFlushing( + writer => Result.Ok().WriteCloudEvent(writer, successType: "note.created", id: "evt-3", options: options) + ); + + GetId(written).Should().Be("evt-3"); + } + + [Fact] + public void GenericWriteShouldFlushWithAReflectionBackedResolver() + { + var options = CreateReflectionBackedOptions(); + var result = Result.Ok(new MovieDto("Blade Runner", 1982)); + + var written = WriteWithoutFlushing( + writer => result.WriteCloudEvent(writer, successType: "movie.created", id: "evt-4", options: options) + ); + + GetId(written).Should().Be("evt-4"); + } + + private static PortableResultsCloudEventsWriteOptions CreateReflectionBackedOptions() => + new () + { + SerializerOptions = PortableResultsCloudEventsWritingModule.CreateDefaultSerializerOptions(), + Source = "urn:test:source" + }; + + // The writer is deliberately neither flushed nor disposed: everything the caller can observe at this point must + // already have reached the stream. + private static byte[] WriteWithoutFlushing(Action write) + { + using var stream = new MemoryStream(); + var writer = new Utf8JsonWriter(stream); + write(writer); + return stream.ToArray(); + } + + private static string? GetId(byte[] cloudEvent) + { + using var document = JsonDocument.Parse(cloudEvent); + return document.RootElement.GetProperty("id").GetString(); + } +} From e60811691d98ba3f33ae4e18439e6392345701e0 Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Sun, 2 Aug 2026 22:27:58 +0200 Subject: [PATCH 10/15] fix(aot): name the correct remedy in the HTTP write guards The guards in LightResult and LightActionResult pointed at AddDefaultLightResultsHttpWriteJsonConverters, which was renamed to AddDefaultPortableResultsHttpWriteJsonConverters. The stale name was harmless while the guards were unreachable; repairing them is what makes it reach users. They also offered converter registration as the only remedy, which helps just the reflection-backed case. A source-generated resolver cannot supply the closed HttpResultForWriting contract at all - which is what makes these guards reachable in the first place - so the message now also asks for the type to be declared in the JsonSerializerContext. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VsEiLdTSnUPsGV1UFBxnrg --- .../LightResult.cs | 4 ++-- src/Light.PortableResults.AspNetCore.Mvc/LightActionResult.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Light.PortableResults.AspNetCore.MinimalApis/LightResult.cs b/src/Light.PortableResults.AspNetCore.MinimalApis/LightResult.cs index 534386b..21e06e7 100644 --- a/src/Light.PortableResults.AspNetCore.MinimalApis/LightResult.cs +++ b/src/Light.PortableResults.AspNetCore.MinimalApis/LightResult.cs @@ -53,7 +53,7 @@ ResolvedHttpWriteOptions resolvedOptions typeInfo is not JsonTypeInfo castTypeInfo) { throw new InvalidOperationException( - $"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'. Please ensure that your JsonSerializerOptions are configured correctly. The AddDefaultLightResultsHttpWriteJsonConverters extension method can help you with this." + $"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'. Please ensure that your JsonSerializerOptions are configured correctly: the AddDefaultPortableResultsHttpWriteJsonConverters extension method registers the required converters, and a source-generated resolver must declare this type in its JsonSerializerContext." ); } @@ -110,7 +110,7 @@ ResolvedHttpWriteOptions resolvedOptions typeInfo is not JsonTypeInfo> castTypeInfo) { throw new InvalidOperationException( - $"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'. Please ensure that your JsonSerializerOptions are configured correctly. The AddDefaultLightResultsHttpWriteJsonConverters extension method can help you with this." + $"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'. Please ensure that your JsonSerializerOptions are configured correctly: the AddDefaultPortableResultsHttpWriteJsonConverters extension method registers the required converters, and a source-generated resolver must declare this type in its JsonSerializerContext." ); } diff --git a/src/Light.PortableResults.AspNetCore.Mvc/LightActionResult.cs b/src/Light.PortableResults.AspNetCore.Mvc/LightActionResult.cs index 621bad9..0bcf67a 100644 --- a/src/Light.PortableResults.AspNetCore.Mvc/LightActionResult.cs +++ b/src/Light.PortableResults.AspNetCore.Mvc/LightActionResult.cs @@ -53,7 +53,7 @@ ResolvedHttpWriteOptions resolvedOptions typeInfo is not JsonTypeInfo castTypeInfo) { throw new InvalidOperationException( - $"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'. Please ensure that your JsonSerializerOptions are configured correctly. The AddDefaultLightResultsHttpWriteJsonConverters extension method can help you with this." + $"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'. Please ensure that your JsonSerializerOptions are configured correctly: the AddDefaultPortableResultsHttpWriteJsonConverters extension method registers the required converters, and a source-generated resolver must declare this type in its JsonSerializerContext." ); } @@ -110,7 +110,7 @@ ResolvedHttpWriteOptions resolvedOptions typeInfo is not JsonTypeInfo> castTypeInfo) { throw new InvalidOperationException( - $"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'. Please ensure that your JsonSerializerOptions are configured correctly. The AddDefaultLightResultsHttpWriteJsonConverters extension method can help you with this." + $"Could not resolve 'JsonTypeInfo<{typeof(HttpResultForWriting)}>'. Please ensure that your JsonSerializerOptions are configured correctly: the AddDefaultPortableResultsHttpWriteJsonConverters extension method registers the required converters, and a source-generated resolver must declare this type in its JsonSerializerContext." ); } From 18aa1762bc619e8b33aff9740f782d646acd2f5b Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Sun, 2 Aug 2026 22:29:39 +0200 Subject: [PATCH 11/15] docs(aot): record where converter selection diverges from System.Text.Json FindRegisteredConverter claimed to mirror how JsonSerializerOptions pick a converter. It follows their list order, but it skips an entry whose CanConvert claims the type while it cannot supply a JsonConverter, where JsonSerializerOptions would select that entry and then throw. The skip is intended - it keeps such an entry from making a library-owned type unwritable and unreadable - so the comment now states it instead of implying equivalence. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VsEiLdTSnUPsGV1UFBxnrg --- .../PortableResultsJsonContracts.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Light.PortableResults/SharedJsonSerialization/PortableResultsJsonContracts.cs b/src/Light.PortableResults/SharedJsonSerialization/PortableResultsJsonContracts.cs index fa40149..6f2e8bf 100644 --- a/src/Light.PortableResults/SharedJsonSerialization/PortableResultsJsonContracts.cs +++ b/src/Light.PortableResults/SharedJsonSerialization/PortableResultsJsonContracts.cs @@ -266,9 +266,13 @@ private static JsonConverter ResolveLibraryConverter throw new InvalidOperationException(CreateMissingLibraryContractMessage(typeof(TLibraryType))); } - // Mirrors how JsonSerializerOptions select a converter from their converter list: the first entry that can - // convert the type wins. Converters that a caller inserted before the Light.PortableResults defaults therefore - // keep their precedence, and the library converter is only reached when no other entry matches. + // Follows the order in which JsonSerializerOptions search their converter list - the first entry that can convert + // the type wins - so converters a caller inserted before the Light.PortableResults defaults keep their precedence + // and the library converter is only reached when no other entry matches. + // + // One deliberate difference: an entry whose CanConvert claims the type but that cannot supply a + // JsonConverter is skipped here, whereas JsonSerializerOptions would select it and then throw. + // Skipping keeps such an entry from making a library-owned type unwritable and unreadable. private static JsonConverter? FindRegisteredConverter(JsonSerializerOptions options) { var converters = options.Converters; From 7a42c9b87c6c5bd0064b6256331e76dd352ec0d3 Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Mon, 3 Aug 2026 06:25:49 +0200 Subject: [PATCH 12/15] ci(aot): publish the Native AOT sample in its own job The publish was the last step of build-and-test, so coverage-comment - which needs that job - waited for a slow native compilation it has nothing to do with. Moving it into an independent job lets it run in parallel with the tests and returns the coverage comment to its previous latency. The job is deliberately not gated on build-and-test: it restores and builds what it needs on its own, so a dependency would only serialize two unrelated pieces of work. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VsEiLdTSnUPsGV1UFBxnrg --- .github/workflows/build-and-test.yml | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 8624dbf..90575f5 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -61,12 +61,26 @@ jobs: --configuration Release -p:PortableResultsAssetTargetFramework=netstandard2.0 -p:ContinuousIntegrationBuild=true - # ILC regression gate for the Native AOT compatibility claim. Trim and AOT diagnostics from the - # Light.PortableResults project references are fatal in Release, while rollups from unannotated package - # dependencies stay non-fatal through the WarningsNotAsErrors setting in the sample project. + + # ILC regression gate for the Native AOT compatibility claim. Trim and AOT diagnostics from the + # Light.PortableResults project references are fatal in Release, while rollups from unannotated package + # dependencies stay non-fatal through the WarningsNotAsErrors setting in the sample project. + # This runs as its own job because the native compilation is slow: it must not delay build-and-test or the + # coverage-comment job that depends on it. + native-aot-publish: + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v7 + - name: Set up .NET + uses: actions/setup-dotnet@v6 + with: + global-json-file: ./global.json + - name: Cache NuGet packages + uses: ./.github/actions/cache-nuget # PublishAot must not be passed on the command line: as a global property it reaches project references # and causes NETSDK1207 in the netstandard2.0 source generator. The sample sets it itself and removes it - # from its project references through GlobalPropertiesToRemove. + # from its project references through GlobalPropertiesToRemove. The RID matches the runner. - name: Publish Native AOT sample run: dotnet publish ./samples/NativeAotMovieRating/NativeAotMovieRating.csproj -c Release -r linux-x64 From ae7d22a7fb6b191467ead910167630a3d85b65c5 Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Mon, 3 Aug 2026 07:11:51 +0200 Subject: [PATCH 13/15] test: run the allocation measurements in a non-parallel collection The tests that assert exact GC.GetAllocatedBytesForCurrentThread() deltas were flaky. The counter is per-thread, so concurrent tests cannot add to it directly, but a GC triggered by any other thread retires and refills the measuring thread's allocation context, and that accounting boundary shifts the observed value by up to the size of a context. Every deviation stayed below 8 KB, and one landed in the baseline rather than the measured value, which is why a one-sided tolerance would not have covered it. A tolerance was the alternative. It would have to be two-sided and roughly 8 KB wide, which on the span test asserting zero allocations would let a regression of hundreds of bytes per iteration through unnoticed. Scheduling the four affected classes into one collection with DisableParallelization removes the cause instead. Measured in Debug, where the suite reproduces this most readily: four failures in twelve runs before, none in twenty-four runs after, for roughly three percent of the runtime. Release did not reproduce it in thirty-three runs, nor in eighteen under CPU contention, so this mainly affected Debug and Stryker runs rather than the Release CI gate. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VsEiLdTSnUPsGV1UFBxnrg --- .../AllocationMeasurementCollection.cs | 21 +++++++++++++++++++ .../Writing/JsonCloudEventsExtensionsTests.cs | 1 + .../Metadata/CanonicalTextFormatterTests.cs | 1 + ...anonicalTextFormatterFloatingPointTests.cs | 1 + .../Writing/SharedWritingExtensionsTests.cs | 1 + 5 files changed, 25 insertions(+) create mode 100644 tests/Light.PortableResults.Tests/AllocationMeasurementCollection.cs diff --git a/tests/Light.PortableResults.Tests/AllocationMeasurementCollection.cs b/tests/Light.PortableResults.Tests/AllocationMeasurementCollection.cs new file mode 100644 index 0000000..3e1a4cf --- /dev/null +++ b/tests/Light.PortableResults.Tests/AllocationMeasurementCollection.cs @@ -0,0 +1,21 @@ +using Xunit; + +namespace Light.PortableResults.Tests; + +/// +/// Groups the test classes that assert exact GC.GetAllocatedBytesForCurrentThread() deltas. +/// +/// +/// DisableParallelization is load-bearing, not tidiness. The counter is per-thread, but a GC triggered by any other +/// thread retires and refills the measuring thread's allocation context, and that accounting boundary shifts the +/// observed value by up to the size of a context. Deviations therefore stay below 8 KB and can fall on either side, +/// because the perturbation lands in the baseline measurement as easily as in the measured one. A plain Collection +/// attribute would not help: it only serializes the tests inside the collection, while the GC pressure comes from +/// everything scheduled beside it. Measured in Debug, where the suite reproduces this most readily: four failures in +/// twelve runs without this attribute, none in twenty-four runs with it, for about three percent of the runtime. +/// +[CollectionDefinition(Name, DisableParallelization = true)] +public sealed class AllocationMeasurementCollection +{ + public const string Name = "Allocation measurement"; +} diff --git a/tests/Light.PortableResults.Tests/CloudEvents/Writing/JsonCloudEventsExtensionsTests.cs b/tests/Light.PortableResults.Tests/CloudEvents/Writing/JsonCloudEventsExtensionsTests.cs index b457dde..072493c 100644 --- a/tests/Light.PortableResults.Tests/CloudEvents/Writing/JsonCloudEventsExtensionsTests.cs +++ b/tests/Light.PortableResults.Tests/CloudEvents/Writing/JsonCloudEventsExtensionsTests.cs @@ -9,6 +9,7 @@ namespace Light.PortableResults.Tests.CloudEvents.Writing; +[Collection(AllocationMeasurementCollection.Name)] public sealed class JsonCloudEventsExtensionsTests { private const int AllocationSampleCount = 5; diff --git a/tests/Light.PortableResults.Tests/Metadata/CanonicalTextFormatterTests.cs b/tests/Light.PortableResults.Tests/Metadata/CanonicalTextFormatterTests.cs index 1083161..1220242 100644 --- a/tests/Light.PortableResults.Tests/Metadata/CanonicalTextFormatterTests.cs +++ b/tests/Light.PortableResults.Tests/Metadata/CanonicalTextFormatterTests.cs @@ -11,6 +11,7 @@ namespace Light.PortableResults.Tests.Metadata; +[Collection(AllocationMeasurementCollection.Name)] public sealed class CanonicalTextFormatterTests { private const int AllocationIterations = 1_000; diff --git a/tests/Light.PortableResults.Tests/Numbers/CanonicalTextFormatterFloatingPointTests.cs b/tests/Light.PortableResults.Tests/Numbers/CanonicalTextFormatterFloatingPointTests.cs index c055e46..17943ea 100644 --- a/tests/Light.PortableResults.Tests/Numbers/CanonicalTextFormatterFloatingPointTests.cs +++ b/tests/Light.PortableResults.Tests/Numbers/CanonicalTextFormatterFloatingPointTests.cs @@ -11,6 +11,7 @@ namespace Light.PortableResults.Tests.Numbers; +[Collection(AllocationMeasurementCollection.Name)] public sealed class CanonicalTextFormatterFloatingPointTests { private const int CorpusSize = 50_000; diff --git a/tests/Light.PortableResults.Tests/SharedJsonSerialization/Writing/SharedWritingExtensionsTests.cs b/tests/Light.PortableResults.Tests/SharedJsonSerialization/Writing/SharedWritingExtensionsTests.cs index a63bdc7..223d9d2 100644 --- a/tests/Light.PortableResults.Tests/SharedJsonSerialization/Writing/SharedWritingExtensionsTests.cs +++ b/tests/Light.PortableResults.Tests/SharedJsonSerialization/Writing/SharedWritingExtensionsTests.cs @@ -13,6 +13,7 @@ namespace Light.PortableResults.Tests.SharedJsonSerialization.Writing; +[Collection(AllocationMeasurementCollection.Name)] public sealed class SharedWritingExtensionsTests { private const int AllocationIterations = 1_000; From 1e9a85d2099a158311eac7a143e13d152a349e62 Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Mon, 3 Aug 2026 07:26:11 +0200 Subject: [PATCH 14/15] docs(aot): record the deviations from plan 0078 Four differences between the plan and the implementation: the fourth unreachable guard in ErrorsExtensions, which the plan did not list; the DynamicallyAccessedMembers annotation that Light.PortableResults.Validation needed although the plan recorded it as clean; the publish gate becoming its own job, because the plan asked both for a step in build-and-test and for no coverage job to depend on it; and step 3 of the contract resolution freezing the options, without which the caching the plan required never took effect. Adds the zero-based sequence to the plan file now that the issue has more than one document. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VsEiLdTSnUPsGV1UFBxnrg --- ....md => 0078-0-aot-compatibility-checks.md} | 0 ai-plans/0078-1-plan-deviations.md | 67 +++++++++++++++++++ 2 files changed, 67 insertions(+) rename ai-plans/{0078-aot-compatibility-checks.md => 0078-0-aot-compatibility-checks.md} (100%) create mode 100644 ai-plans/0078-1-plan-deviations.md diff --git a/ai-plans/0078-aot-compatibility-checks.md b/ai-plans/0078-0-aot-compatibility-checks.md similarity index 100% rename from ai-plans/0078-aot-compatibility-checks.md rename to ai-plans/0078-0-aot-compatibility-checks.md diff --git a/ai-plans/0078-1-plan-deviations.md b/ai-plans/0078-1-plan-deviations.md new file mode 100644 index 0000000..e1ff6e1 --- /dev/null +++ b/ai-plans/0078-1-plan-deviations.md @@ -0,0 +1,67 @@ +# Plan Deviations for Native AOT Compatibility Checks + +## Referenced Plans + +- `0078-0-aot-compatibility-checks.md` enabled `IsAotCompatible` on the `net10.0` assets of + `Light.PortableResults` and `Light.PortableResults.Validation`, routed every warned serialization call through a + `JsonTypeInfo` overload, introduced library-owned contract resolution for the CloudEvents write, CloudEvents + read, and HTTP read wrappers, and added an ILC publish gate to CI. + +## Deviations + +### A fourth unreachable guard in `ErrorsExtensions` + +The plan listed `SystemTextJsonWritingExtensions`, `LightResult`, and `LightActionResult` as the sites whose +unreachable guards had to be converted. `ErrorsExtensions.WriteRichErrors` resolved `JsonTypeInfo` +through the same pattern — `GetTypeInfo` followed by a failed-cast guard that `NotSupportedException` reached +first — but the plan did not name it. It is converted with the same contract resolution. + +It is the one converted site that supplies no library fallback converter: `GetLibraryTypeInfo` is +called without a `createLibraryConverter` argument, so only a converter configured on the options is used. The +correct `MetadataObject` converter differs per transport, because CloudEvents writing and HTTP writing register +their own, and the library cannot pick one without knowing the transport. + +Without this change, a failure result carrying error metadata could not be written under a source-generated +resolver, and acceptance criterion 4 would not hold for failure results. + +### `Light.PortableResults.Validation` was not clean + +The plan recorded the package as "currently clean" and enabled its analyzers purely for regression protection. +Enabling `IsAotCompatible` instead produced `IL2091` on +`PortableResultsValidationModule.ValidateWithPortableResults`. The framework annotates the +`TService` parameter of `ServiceCollectionDescriptorExtensions.TryAddSingleton` with +`DynamicallyAccessedMemberTypes.PublicConstructors`, and the method forwards its own open `TValidator` into it +without declaring the same requirement. + +`TValidator` is therefore annotated with `DynamicallyAccessedMemberTypes.PublicConstructors`. The annotation is +load-bearing rather than cosmetic: the container constructs the validator reflectively, no IL calls its +constructor, and a trimmed constructor fails at runtime when the service is resolved. + +This is a source-breaking change for callers that keep their own `TValidator` parameter open and forward it, +which is why it is recorded under `Breaking changes` in the package release notes. Applications passing a concrete +validator type are unaffected: the annotation is discharged at the call site, exactly as it already is for +`AddSingleton`. Enabling trim analyzers on a package believed to be clean is therefore not free — it +can cost public API surface. + +### The publish gate is a separate job, not a post-test step + +The plan asked for "a separate post-test step to `build-and-test.yml`" and, in the same section, that no coverage +job depend on it. Those two cannot both hold: `coverage-comment` declares `needs: build-and-test`, so any step +added to that job also gates the coverage comment behind the native compilation. + +The publish runs as an independent `native-aot-publish` job instead. It preserves every constraint the plan gave +the step — runner RID, no `PublishAot` on the command line, outside any matrix — while satisfying the intent +behind the coverage constraint, and it now runs in parallel with the tests rather than after them. + +### Contract resolution freezes the options on the fallback path + +The plan called for freezing only in step 1, where `MakeReadOnly(populateMissingResolver: true)` materializes the +reflection resolver, and separately required library-owned contracts to be created once per `JsonSerializerOptions` +instance. Those two do not compose on the CloudEvents write path: invoking a converter directly never freezes the +options, so nothing marked them read-only and the cache was never populated. + +Step 3 therefore calls `MakeReadOnly()` once the converter has been resolved and before the result is cached. The +observable consequence is that resolving a library-owned contract makes the options read-only, so converters +registered afterwards throw. This matches what the plan already accepted for step 1 — serializing with the +resolved contract would freeze the options immediately afterwards — and it is what keeps a later registration from +invalidating a cached contract. From f1f9a8b187be42d6f0164b56e3c00eeb01dd0730 Mon Sep 17 00:00:00 2001 From: Kenny Pflug Date: Mon, 3 Aug 2026 07:58:47 +0200 Subject: [PATCH 15/15] docs: note that resolving a library contract freezes the options Step 3 of the contract resolution calls MakeReadOnly so that the contract it caches cannot be invalidated by a converter registered afterwards. The README described the caching but not the freeze, leaving a caller to discover it as an InvalidOperationException from a late Converters.Add. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VsEiLdTSnUPsGV1UFBxnrg --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index e3d25e0..d94c349 100644 --- a/README.md +++ b/README.md @@ -1377,7 +1377,8 @@ For each library-owned type, `PortableResultsJsonContracts` resolves in this ord `JsonSerializerOptions` precedence — the first entry in `Converters` that can convert the type. A converter you insert **before** the Light.PortableResults defaults keeps its precedence; the library converter is only reached when nothing else matches. Contracts created this way are cached per `JsonSerializerOptions` instance and can be - created after the options became read-only. + created after the options became read-only. Creating one also makes the options read-only, so register every + converter before the first read or write. If a value type is missing from your context, the affected entry point throws an `InvalidOperationException` that names the unresolved type and tells you to register it, instead of failing deep inside `System.Text.Json`.