Migration guide, API renames and read-path fixes - #210
Open
manuc66 wants to merge 8 commits into
Open
Conversation
MIGRATION.md is an actionable before/after recipe: the mechanical renames, the behaviors that actually differ between the Newtonsoft and STJ packages, and the capability ceilings when moving between the converter, resolver and generator. Also fix the outdated test count in the STJ status note and link the guide from the README.
…SubTypes.Text.Json The STJ package is still a release candidate, so now is the time to fix the capitalization inherited from the Newtonsoft API: FallBackSubTypeAttribute -> FallbackSubTypeAttribute and FallBackToNearestAncestor() -> FallbackToNearestAncestor(). The Newtonsoft package keeps its historical names. The generator matches attributes by namespace + short name, so it now looks up FallbackSubTypeAttribute; the native JsonUnknownDerivedTypeHandling enum member is untouched. All STJ (196), AOT (79) and Newtonsoft (153) tests pass.
…e path
When a nested hierarchy's discriminators are also native properties of the type
(e.g. [JsonPropertyName("$PayloadKind")]), the generated nested-chain writer
emitted every payload property without excluding the discriminator names, so the
injected discriminators were written twice. The runtime converter already
excluded them; the generator now skips any property whose name matches a
discriminator in the chain.
Adds a dedicated test fixture pinning the single-write and the round-trip to the
deepest subtype.
Converter_Serialize/Generated_Deserialize etc. collided across the single-object and base-as-leaf classes, so a full-suite run produced ambiguous rows. Each class now uses a unique scenario prefix (Single_, Col_, Nested_, Pres_, Leaf_, Nw_) and PERFORMANCE.md documents the mapping and how to filter a scenario.
ReadObject parsed the JSON once to resolve the type, then ReadPlainObject parsed it again from the reader to materialize the base object. Reuse the already-parsed RootElement instead, matching how the subtype path deserializes. Measured (BenchmarkDotNet, net10, DefaultJob): Leaf_Converter_Deserialize 1.58us / 560 B before, 1.16us / 360 B after. All STJ and AOT tests pass.
The nested-chain writer now skips properties whose name matches a discriminator in the chain; the committed PayloadJsonSubTypesConverter still carried the old output without the guard, failing the golden-master test.
…ribute-based discriminator
The JsonSubTypeConverterAttribute kept passing a closed converter type to
JsonConverterAttribute, so System.Text.Json built the converter through its
parameterless constructor and the CreateConverter override was never called.
The attribute now leaves ConverterType null for the JsonSubtypes<T> forms and
routes through CreateConverter, which closes the generic over the annotated
type (new [JsonSubTypeConverter("Kind")] convenience constructors) and passes
the discriminator to the converter. As a result the attribute-based write path
now injects the discriminator for registered subtypes, matching the behaviour
the README and MIGRATION.md already documented. GetTypeResolver and
CreateTypeResolver close JsonSubtypes<> over the target type when the attribute
carries no converter type, so the resolver dance keeps working for nested
hierarchies.
Property-presence has no discriminator to inject, so serialization writes the plain runtime contract. Add a parity test pinning that the runtime converter and the generated converter emit the same output for a property-presence subtype (all properties once, nothing injected), and strengthen the attribute-based runtime test to assert the exact property set so migrating never loses a property nor duplicates the discriminator.
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
1 participant
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.
Problem
Fix
Tests
Honest note(s)