YamlDotNet icon indicating copy to clipboard operation
YamlDotNet copied to clipboard

`WithDuplicateChecking()` conflicts with `MergingParser` operation

Open dougbu opened this issue 1 year ago • 4 comments
trafficstars

Describe the bug

As far as I can tell, YamlDotNet correctly handles duplicate key checking with YAML-spec-compliant anchors and aliases. However, the MergingParser is unable to deserialize YAML using the proposed merge key when DeserializerBuilder.WithDuplicateKeyChecking() has been called.

My initial thought for a potential fix would be tracking the anchor from which a key was previously set. While expanding a higher-level alias or a new map, duplicate keys should be ignored and handled as they are today when using MergingParser. I see YamlNode.Anchor exists but am not sure the MergingParser, DictionaryDeserializer, and ObjectNodeDeserializer can use this property to handle keys during merges correctly.  

To Reproduce

Add a test like SerializationTests.ExampleFromSpecificationIsHandledCorrectly() but with a new name and using

var deserializer = DeserializerBuilder.WithDuplicateKeyChecking().Build();

instead of the inherited Deserializer property. Execute the new test.

Expected

The test should work perfectly.

Actual

The test fails. In my branch, the error is

[xUnit.net 00:00:03.91]     ExampleFromSpecificationIsHandledCorrectlyWithDuplicateChecking [FAIL]
  Failed ExampleFromSpecificationIsHandledCorrectlyWithDuplicateChecking [6 ms]
  Error Message:
   YamlDotNet.Core.YamlException : Encountered duplicate key r
  Stack Trace:
     at YamlDotNet.Serialization.NodeDeserializers.DictionaryDeserializer.TryAssign(IDictionary result, Object key, Object value, MappingStart propertyName) in C:\dd\dnx\other\YamlDotNet\YamlDotNet\Serialization\NodeDeserializers\DictionaryDeserializer.cs:line 44
...

Debugging shows the incorrect detection occurs at the << : [ *BIG, *LEFT, *SMALL ] line in the sample. I'm not positive (because I'm not familiar enough w/ the code) but suspect it's currently expanding the *BIG alias.

dougbu avatar Jan 07 '24 20:01 dougbu

@aaubry, this one is important in something I'd like to change in .NET build infrastructure. if you have a minute to suggest an approach, I'm happy to put some time into it :grin:

dougbu avatar Jan 23 '24 03:01 dougbu

I’d have to double check but if the merging parser uses yamlstream under the hood then that makes sense because the stream calls dictionary.add without checking for duplicate keys. My suspicion is so that it will follow the spec. It was requested not to long ago to have that feature in the stream where duplicate keys would override the previous value, pretty sure that’s what it was.

EdwardCooke avatar Jan 26 '24 16:01 EdwardCooke

Ignore my last comment. Way off base.

EdwardCooke avatar Jan 26 '24 16:01 EdwardCooke

I’m going to work on fixing this in the next little bit.

EdwardCooke avatar Jul 08 '24 03:07 EdwardCooke