Repository navigation
[Refactor] #621 - [Serializer.Json] Move to System.Text.Json and cover roundtrip - #624
Merged
Merged
Conversation
…n and cover the round trip The JSON serializer could not round trip a layout with a nested pane tree, which is every real layout. Deserializing failed with "Unknown DTO type discriminator '' for base type 'LayoutPositionableGroupDto'". Newtonsoft picks a converter by the runtime type when writing and by the declared type when reading, so the exact-type test in CanConvert attached the converter on the reading side only: nothing ever wrote a discriminator, while reading a list declared as the abstract base demanded one. Even once that ran, the converter serialized each value through a copy of the serializer it had been removed from, so the whole subtree below it was written without discriminators too. System.Text.Json supports this directly. The three abstract bases that appear as a declared type - LayoutPositionableGroupDto, LayoutContentDto and LayoutFloatingWindowDto - declare their concrete types through JsonPolymorphismOptions, and the custom converter is gone. The declaration lives in a type info resolver modifier rather than in attributes on the DTOs, so AvalonDock.Core keeps its dependency-free netstandard2.0 surface. Newtonsoft.Json is no longer referenced. A second bug goes with it: DefaultValueHandling.Ignore dropped every member holding the default of its CLR type. CanHide, CanFloat, CanAutoHide, CanDockAsTabbedDocument, CanMove and CanShowOnHover all default to true, so every false was silently restored as true, and PreviousContainerIndex 0 came back as -1. System.Text.Json does not honour ShouldSerializeXxx() either, so every value is now written explicitly and the whole bug class is gone; only nulls are skipped, which value types cannot be. Tests: the round trip had five of them. JsonLayoutSerializerTests now covers the content flags in their switched-off state, selection flags, floating bounds, auto-hide sizes, absolute and star dock lengths, dock minimums, pane ids, previous container id with index zero, the hidden list, auto-hidden anchorables on each of the four sides and both floating window types. Three more pin the discriminator design: written where the declared type is abstract and nowhere else, naming the concrete type, and refusing a discriminator this version does not know. SerializerParityTests round-trips one layout through both serializers and compares the restored trees, which is where any drift between the two formats will surface. The optional constructor parameter changes from JsonSerializerSettings to JsonSerializerOptions. The JSON serializer has not shipped, so nothing depends on the old signature or on the previous file contents, which could not be read back at all.
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.
The JSON serializer could not round trip a layout with a nested pane tree, which is every real layout. Deserializing failed with "Unknown DTO type discriminator '' for base type 'LayoutPositionableGroupDto'".
Newtonsoft picks a converter by the runtime type when writing and by the declared type when reading, so the exact-type test in CanConvert attached the converter on the reading side only: nothing ever wrote a discriminator, while reading a list declared as the abstract base demanded one. Even once that ran, the converter serialized each value through a copy of the serializer it had been removed from, so the whole subtree below it was written without discriminators too.
System.Text.Json supports this directly. The three abstract bases that appear as a declared type - LayoutPositionableGroupDto, LayoutContentDto and LayoutFloatingWindowDto - declare their concrete types through JsonPolymorphismOptions, and the custom converter is gone. The declaration lives in a type info resolver modifier rather than in attributes on the DTOs, so AvalonDock.Core keeps its dependency-free netstandard2.0 surface. Newtonsoft.Json is no longer referenced.
A second bug goes with it: DefaultValueHandling.Ignore dropped every member holding the default of its CLR type. CanHide, CanFloat, CanAutoHide, CanDockAsTabbedDocument, CanMove and CanShowOnHover all default to true, so every false was silently restored as true, and PreviousContainerIndex 0 came back as -1. System.Text.Json does not honour ShouldSerializeXxx() either, so every value is now written explicitly and the whole bug class is gone; only nulls are skipped, which value types cannot be.
Tests: the round trip had five of them. JsonLayoutSerializerTests now covers the content flags in their switched-off state, selection flags, floating bounds, auto-hide sizes, absolute and star dock lengths, dock minimums, pane ids, previous container id with index zero, the hidden list, auto-hidden anchorables on each of the four sides and both floating window types. Three more pin the discriminator design: written where the declared type is abstract and nowhere else, naming the concrete type, and refusing a discriminator this version does not know. SerializerParityTests round-trips one layout through both serializers and compares the restored trees, which is where any drift between the two formats will surface.
The optional constructor parameter changes from JsonSerializerSettings to JsonSerializerOptions. The JSON serializer has not shipped, so nothing depends on the old signature or on the previous file contents, which could not be read back at all.