Serialize types implementing only IReadOnlyDictionary<,> as mappings - #1127
Open
youdie006 wants to merge 1 commit into
Open
Serialize types implementing only IReadOnlyDictionary<,> as mappings#1127youdie006 wants to merge 1 commit into
youdie006 wants to merge 1 commit into
Conversation
A type that implements IReadOnlyDictionary<TKey, TValue> but neither the
non-generic IDictionary nor IDictionary<TKey, TValue> was serialized as a
sequence of {Key, Value} mappings instead of a single YAML mapping,
because ObjectFactoryBase.GetDictionary only recognised IDictionary<,>
and the object graph traversal then fell through to the IEnumerable
(list) branch.
Detect IReadOnlyDictionary<,> in GetDictionary and wrap it in a new
GenericReadOnlyDictionaryToNonGenericAdapter (mirroring the existing
IDictionary<,> adapter; only enumeration is needed for traversal) so it
is traversed as a mapping. The regular IDictionary<,> path is unchanged.
Fixes aaubry#606
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.
Motivation
Fixes #606. A type that implements only
IReadOnlyDictionary<TKey, TValue>(and neither the non-genericIDictionarynorIDictionary<TKey, TValue>) is serialized as a sequence of{ Key, Value }mappings instead of a single YAML mapping:FullObjectGraphTraversalStrategy.TraverseObjectdispatches dictionaries viaObjectFactory.GetDictionary, which only recognisesIDictionary<,>. A read-only-only dictionary therefore returnsfalsethere and falls through to theIEnumerablebranch (IReadOnlyDictionary<,>isIEnumerable<KeyValuePair<,>>), so it is written as a list. TheBCL ReadOnlyDictionary<,>happens to also implementIDictionary, so only custom read-only dictionary types are affected. The asymmetry is real: the deserialize side already handles non-IDictionarydictionary types (see the existingGenericDictionaryThatDoesNotImplementIDictionaryCanBeDeserializedtest).Solution
ObjectFactoryBase.GetDictionarynow also detectsIReadOnlyDictionary<,>(after the existingIDictionary<,>check, so types implementing both keep the current path) and wraps the value in a newGenericReadOnlyDictionaryToNonGenericAdapter<TKey, TValue>.GenericDictionaryToNonGenericAdapterbut takes anIReadOnlyDictionary<,>. Object-graph traversal (TraverseDictionary) only enumerates the adapter, so onlyGetEnumeratoris implemented; the rest throwNotSupportedException, exactly like the existing adapter's unused members.The regular
IDictionary<,>path is unchanged.Test
Added
SerializeGenericReadOnlyDictionary, plus aGenericTestReadOnlyDictionary<,>test helper (implements onlyIReadOnlyDictionary<,>, mirroring the existingGenericTestDictionary). It asserts the read-only dictionary serializes to the same YAML as an equivalentDictionary<,>.Verified locally in Docker:
dotnet test— the fullYamlDotNet.Testsuite passes (the only 2 failures,ConformsWithYamlSpec, are pre-existing and caused by the YAML spec test-suite submodule not being present in the shallow checkout; they fail identically without this change). The library also builds cleanly fornetstandard2.0andnet8.0(0 warnings).Disclosure: this fix was prepared with AI assistance (Claude). I reviewed it, confirmed the root cause in the traversal dispatch, and verified the red-green test and full suite myself.