Repository navigation
fix(csharp): emit value-object null-omission only on nullable members - #448
Merged
Merged
Conversation
…llable members, gate required value-type members cross-port
…and invariant wording; no unresolved doc gaps.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Changed
EntityGeneratoremits[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]on value-object members only when the member is nullable. System.Text.Json throws on that condition for non-nullable value types (double,DateTimeOffset,int,bool), which made generated routes answer 500 when deserializing such a value object.Telemetryvalue object with requiredfield.timestampandfield.doublemembers plus an optionalfield.int, held by atelemetryjsonb field on the document entity. New scenariojsonb-value-object-required-value-types.yamlgates this shape in every port.ValueObjectWireIgnoreTestscompiles the generated value object and asserts the JSON behavior. Reference servers and generated harnesses in C#, Java, Kotlin, Python and TypeScript are updated to carry the new field.Validation
Risk Assessment
✅ Low: The generator change is a narrow gate (emit WhenWritingNull only on nullable members), the new C# test now compiles the generated value object and round-trips it through System.Text.Json (which throws without the fix), and the shared jsonb corpus extension is wired consistently across all five ports' reference and generated servers.
Testing
Baseline TS lanes already green. The new C# regression test passes on the fix and fails with the exact System.Text.Json 500 on the pre-fix generator, so it reproduces the reported bug. Its pass is an in-process Roslyn compile plus System.Text.Json test, not a live run, so that scenario is recorded as untested. Live jsonb API contract lanes (generated and reference, real Postgres) pass all 5 scenarios each, including the required-value-types scenario (POST, GET, PATCH, optional tempC omitted). Full C# codegen (983), conformance (2075) and integration (357) suites pass. Overall: pass on live scenarios. Gap: the regression test runs in-process, not through a live HTTP request; live HTTP coverage comes from the integration lane.
Pipeline
Updates from git push no-mistakes
✅ **Intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
server/csharp/MetaObjects.Codegen.Tests/ValueObjectWireIgnoreTests.cs:53- The new C# regression test only asserts attribute text in the generated Telemetry.g.cs. It does not reproduce the reported 500, which is System.Text.Json throwing InvalidOperationException when it builds type info for a type with WhenWritingNull on a non-nullable value type. A generator that dropped the throwing condition by some other emission, or emitted a different but still invalid one, would be judged by string shape instead of behavior. The real behavioral gate (the new jsonb scenario) needs Docker and runs only in the integration lane. A semantic oracle would compile the generated Telemetry source (or serialize an equivalent POCO) and call JsonSerializer.Serialize/Deserialize, asserting no exception and that a null optional member is omitted. System.Text.Json's documented behavior is the oracle.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
scripts/ci-local.sh --only ts-fast --only ts-unit --strict-toolchainsBaseline (given): scripts/ci-local.sh --only ts-fast --only ts-unit --strict-toolchainsdotnet test server/csharp/MetaObjects.Codegen.Tests --filter FullyQualifiedName~ValueObjectWireIgnoreTests (passes on fix)Reproduction: temporarily restored server/csharp/MetaObjects.Codegen/Generators/EntityGenerator.cs from c59838e74 and re-ran the same test. It failed with InvalidOperationException: ignore condition 'JsonIgnoreCondition.WhenWritingNull' is not valid on value-type member 'CapturedAt'. File restored with git checkout; worktree clean.dotnet test server/csharp/MetaObjects.IntegrationTests --filter FullyQualifiedName~ApiContractJsonbConformanceTest (live: real generated C# server and reference server against Testcontainers Postgres; 5 generated + 5 reference scenarios incl. jsonb-value-object-required-value-types, all pass)dotnet test server/csharp/MetaObjects.Codegen.Tests (983 passed, 1 skipped)dotnet test server/csharp/MetaObjects.Conformance.Tests (2075 passed)dotnet test server/csharp/MetaObjects.IntegrationTests (full: 357 passed; Testcontainers Postgres)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.
Validation: 1 no-mistakes run (one review-gate fix round); full scripts/ci-local.sh passed beforehand.