Merge pull request #771 from stan-sz/WithDuplicateKeyChecking

EdwardCooke · web-flow · commit 65c2aa9db0c8 · 2023-01-28T13:24:02.000-07:00
Follow up on duplicate key checking
diff --git a/YamlDotNet.Test/Serialization/DeserializerTest.cs b/YamlDotNet.Test/Serialization/DeserializerTest.cs
@@ -299,6 +299,13 @@ public void DeserializeWithDuplicateKeyChecking_YamlWithDuplicateKeys_ThrowsYaml
 
             Action act = () => sut.Deserialize<Person>(yaml);
             act.ShouldThrow<YamlException>("Because there are duplicate name keys");
+            act = () => sut.Deserialize<IDictionary<object, object>>(yaml);
+            act.ShouldThrow<YamlException>("Because there are duplicate name keys");
+
+            var stream = Yaml.ReaderFrom("backreference.yaml");
+            var parser = new MergingParser(new Parser(stream));
+            act = () => sut.Deserialize<Dictionary<string, Dictionary<string, string>>>(parser);
+            act.ShouldThrow<YamlException>("Because there are duplicate name keys");
         }
 
         [Fact]
@@ -316,6 +323,13 @@ public void DeserializeWithoutDuplicateKeyChecking_YamlWithDuplicateKeys_DoesNot
 
             Action act = () => sut.Deserialize<Person>(yaml);
             act.ShouldNotThrow<YamlException>("Because duplicate key checking is not enabled");
+            act = () => sut.Deserialize<IDictionary<object, object>>(yaml);
+            act.ShouldNotThrow<YamlException>("Because duplicate key checking is not enabled");
+
+            var stream = Yaml.ReaderFrom("backreference.yaml");
+            var parser = new MergingParser(new Parser(stream));
+            act = () => sut.Deserialize<Dictionary<string, Dictionary<string, string>>>(parser);
+            act.ShouldNotThrow<YamlException>("Because duplicate key checking is not enabled");
         }
 
         public class Test
diff --git a/YamlDotNet/Serialization/DeserializerBuilder.cs b/YamlDotNet/Serialization/DeserializerBuilder.cs
@@ -91,7 +91,7 @@ public DeserializerBuilder()
                 { typeof(NullNodeDeserializer), _ => new NullNodeDeserializer() },
                 { typeof(ScalarNodeDeserializer), _ => new ScalarNodeDeserializer(attemptUnknownTypeDeserialization) },
                 { typeof(ArrayNodeDeserializer), _ => new ArrayNodeDeserializer() },
-                { typeof(DictionaryNodeDeserializer), _ => new DictionaryNodeDeserializer(objectFactory.Value) },
+                { typeof(DictionaryNodeDeserializer), _ => new DictionaryNodeDeserializer(objectFactory.Value, duplicateKeyChecking) },
                 { typeof(CollectionNodeDeserializer), _ => new CollectionNodeDeserializer(objectFactory.Value) },
                 { typeof(EnumerableNodeDeserializer), _ => new EnumerableNodeDeserializer() },
                 { typeof(ObjectNodeDeserializer), _ => new ObjectNodeDeserializer(objectFactory.Value, BuildTypeInspector(), ignoreUnmatched, duplicateKeyChecking) }
@@ -151,7 +151,7 @@ public DeserializerBuilder WithStaticContext(StaticContext context)
             _baseTypeInspector = context.GetTypeInspector();
 
             WithoutNodeDeserializer<DictionaryNodeDeserializer>();
-            WithNodeDeserializer(new StaticDictionaryNodeDeserializer(context.GetFactory()));
+            WithNodeDeserializer(new StaticDictionaryNodeDeserializer(context.GetFactory(), duplicateKeyChecking));
 
             return Self;
         }
diff --git a/YamlDotNet/Serialization/NodeDeserializers/DictionaryNodeDeserializer.cs b/YamlDotNet/Serialization/NodeDeserializers/DictionaryNodeDeserializer.cs
@@ -32,10 +32,12 @@ namespace YamlDotNet.Serialization.NodeDeserializers
     public class DictionaryNodeDeserializer : INodeDeserializer
     {
         private readonly IObjectFactory objectFactory;
+        private readonly bool duplicateKeyChecking;
 
-        public DictionaryNodeDeserializer(IObjectFactory objectFactory)
+        public DictionaryNodeDeserializer(IObjectFactory objectFactory, bool duplicateKeyChecking)
         {
             this.objectFactory = objectFactory ?? throw new ArgumentNullException(nameof(objectFactory));
+            this.duplicateKeyChecking = duplicateKeyChecking;
         }
 
         public virtual bool Deserialize(IParser parser, Type expectedType, Func<IParser, Type, object?> nestedObjectDeserializer, out object? value)
@@ -77,9 +79,18 @@ public virtual bool Deserialize(IParser parser, Type expectedType, Func<IParser,
             return true;
         }
 
+        private void TryAssign(IDictionary result, object key, object value, MappingStart propertyName)
+        {
+            if (duplicateKeyChecking && result.Contains(key))
+            {
+                throw new YamlException(propertyName.Start, propertyName.End, $"Encountered duplicate key {key}");
+            }
+            result[key] = value!;
+        }
+
         protected void Deserialize(Type tKey, Type tValue, IParser parser, Func<IParser, Type, object?> nestedObjectDeserializer, IDictionary result)
         {
-            parser.Consume<MappingStart>();
+            var property = parser.Consume<MappingStart>();
             while (!parser.TryConsume<MappingEnd>(out var _))
             {
                 var key = nestedObjectDeserializer(parser, tKey);
@@ -91,7 +102,7 @@ protected void Deserialize(Type tKey, Type tValue, IParser parser, Func<IParser,
                     if (valuePromise == null)
                     {
                         // Key is pending, value is known
-                        keyPromise.ValueAvailable += v => result[v!] = value!;
+                        keyPromise.ValueAvailable += v => TryAssign(result, v!, value!, property);
                     }
                     else
                     {
@@ -102,7 +113,7 @@ protected void Deserialize(Type tKey, Type tValue, IParser parser, Func<IParser,
                         {
                             if (hasFirstPart)
                             {
-                                result[v!] = value!;
+                                TryAssign(result, v!, value!, property);
                             }
                             else
                             {
@@ -115,7 +126,7 @@ protected void Deserialize(Type tKey, Type tValue, IParser parser, Func<IParser,
                         {
                             if (hasFirstPart)
                             {
-                                result[key] = v!;
+                                TryAssign(result, key, v!, property);
                             }
                             else
                             {
@@ -135,12 +146,12 @@ protected void Deserialize(Type tKey, Type tValue, IParser parser, Func<IParser,
                     if (valuePromise == null)
                     {
                         // Happy path: both key and value are known
-                        result[key] = value!;
+                        TryAssign(result, key, value!, property);
                     }
                     else
                     {
                         // Key is known, value is pending
-                        valuePromise.ValueAvailable += v => result[key!] = v!;
+                        valuePromise.ValueAvailable += v => TryAssign(result, key!, v!, property);
                     }
                 }
             }
diff --git a/YamlDotNet/Serialization/NodeDeserializers/ObjectNodeDeserializer.cs b/YamlDotNet/Serialization/NodeDeserializers/ObjectNodeDeserializer.cs
@@ -55,17 +55,16 @@ public bool Deserialize(IParser parser, Type expectedType, Func<IParser, Type, o
             var implementationType = Nullable.GetUnderlyingType(expectedType) ?? expectedType;
 
             value = objectFactory.Create(implementationType);
-            var consumedProperties = new List<string>();
+            var consumedProperties = new HashSet<string>(StringComparer.Ordinal);
             while (!parser.TryConsume<MappingEnd>(out var _))
             {
                 var propertyName = parser.Consume<Scalar>();
-                if (duplicateKeyChecking && consumedProperties.Contains(propertyName.Value))
+                if (duplicateKeyChecking && !consumedProperties.Add(propertyName.Value))
                 {
                     throw new YamlException(propertyName.Start, propertyName.End, $"Encountered duplicate key {propertyName.Value}");
                 }
                 try
                 {
-                    consumedProperties.Add(propertyName.Value);
                     var property = typeDescriptor.GetProperty(implementationType, null, propertyName.Value, ignoreUnmatched);
                     if (property == null)
                     {
diff --git a/YamlDotNet/Serialization/NodeDeserializers/StaticDictionaryNodeDeserializer.cs b/YamlDotNet/Serialization/NodeDeserializers/StaticDictionaryNodeDeserializer.cs
@@ -29,8 +29,8 @@ public class StaticDictionaryNodeDeserializer : DictionaryNodeDeserializer
     {
         private readonly ObjectFactories.StaticObjectFactory _objectFactory;
 
-        public StaticDictionaryNodeDeserializer(ObjectFactories.StaticObjectFactory objectFactory)
-            : base(objectFactory)
+        public StaticDictionaryNodeDeserializer(ObjectFactories.StaticObjectFactory objectFactory, bool duplicateKeyChecking)
+            : base(objectFactory, duplicateKeyChecking)
         {
             _objectFactory = objectFactory ?? throw new ArgumentNullException(nameof(objectFactory));
         }

Original file line number	Diff line number	Diff line change
`@@ -32,10 +32,12 @@ namespace YamlDotNet.Serialization.NodeDeserializers`
`32`	`32`	`public class DictionaryNodeDeserializer : INodeDeserializer`
`33`	`33`	`{`
`34`	`34`	`private readonly IObjectFactory objectFactory;`
	`35`	`+ private readonly bool duplicateKeyChecking;`
`35`	`36`
`36`		`- public DictionaryNodeDeserializer(IObjectFactory objectFactory)`
	`37`	`+ public DictionaryNodeDeserializer(IObjectFactory objectFactory, bool duplicateKeyChecking)`
`37`	`38`	`{`
`38`	`39`	`this.objectFactory = objectFactory ?? throw new ArgumentNullException(nameof(objectFactory));`
	`40`	`+ this.duplicateKeyChecking = duplicateKeyChecking;`
`39`	`41`	`}`
`40`	`42`
`41`	`43`	`public virtual bool Deserialize(IParser parser, Type expectedType, Func<IParser, Type, object?> nestedObjectDeserializer, out object? value)`
`@@ -77,9 +79,18 @@ public virtual bool Deserialize(IParser parser, Type expectedType, Func<IParser,`
`77`	`79`	`return true;`
`78`	`80`	`}`
`79`	`81`
	`82`	`+ private void TryAssign(IDictionary result, object key, object value, MappingStart propertyName)`
	`83`	`+ {`
	`84`	`+ if (duplicateKeyChecking && result.Contains(key))`
	`85`	`+ {`
	`86`	`+ throw new YamlException(propertyName.Start, propertyName.End, $"Encountered duplicate key {key}");`
	`87`	`+ }`
	`88`	`+ result[key] = value!;`
	`89`	`+ }`
	`90`	`+`
`80`	`91`	`protected void Deserialize(Type tKey, Type tValue, IParser parser, Func<IParser, Type, object?> nestedObjectDeserializer, IDictionary result)`
`81`	`92`	`{`
`82`		`- parser.Consume<MappingStart>();`
	`93`	`+ var property = parser.Consume<MappingStart>();`
`83`	`94`	`while (!parser.TryConsume<MappingEnd>(out var _))`
`84`	`95`	`{`
`85`	`96`	`var key = nestedObjectDeserializer(parser, tKey);`
`@@ -91,7 +102,7 @@ protected void Deserialize(Type tKey, Type tValue, IParser parser, Func<IParser,`
`91`	`102`	`if (valuePromise == null)`
`92`	`103`	`{`
`93`	`104`	`// Key is pending, value is known`
`94`		`- keyPromise.ValueAvailable += v => result[v!] = value!;`
	`105`	`+ keyPromise.ValueAvailable += v => TryAssign(result, v!, value!, property);`
`95`	`106`	`}`
`96`	`107`	`else`
`97`	`108`	`{`
`@@ -102,7 +113,7 @@ protected void Deserialize(Type tKey, Type tValue, IParser parser, Func<IParser,`
`102`	`113`	`{`
`103`	`114`	`if (hasFirstPart)`
`104`	`115`	`{`
`105`		`- result[v!] = value!;`
	`116`	`+ TryAssign(result, v!, value!, property);`
`106`	`117`	`}`
`107`	`118`	`else`
`108`	`119`	`{`
`@@ -115,7 +126,7 @@ protected void Deserialize(Type tKey, Type tValue, IParser parser, Func<IParser,`
`115`	`126`	`{`
`116`	`127`	`if (hasFirstPart)`
`117`	`128`	`{`
`118`		`- result[key] = v!;`
	`129`	`+ TryAssign(result, key, v!, property);`
`119`	`130`	`}`
`120`	`131`	`else`
`121`	`132`	`{`
`@@ -135,12 +146,12 @@ protected void Deserialize(Type tKey, Type tValue, IParser parser, Func<IParser,`
`135`	`146`	`if (valuePromise == null)`
`136`	`147`	`{`
`137`	`148`	`// Happy path: both key and value are known`
`138`		`- result[key] = value!;`
	`149`	`+ TryAssign(result, key, value!, property);`
`139`	`150`	`}`
`140`	`151`	`else`
`141`	`152`	`{`
`142`	`153`	`// Key is known, value is pending`
`143`		`- valuePromise.ValueAvailable += v => result[key!] = v!;`
	`154`	`+ valuePromise.ValueAvailable += v => TryAssign(result, key!, v!, property);`
`144`	`155`	`}`
`145`	`156`	`}`
`146`	`157`	`}`
Original file line number	Diff line number	Diff line change
`@@ -55,17 +55,16 @@ public bool Deserialize(IParser parser, Type expectedType, Func<IParser, Type, o`
`55`	`55`	`var implementationType = Nullable.GetUnderlyingType(expectedType) ?? expectedType;`
`56`	`56`
`57`	`57`	`value = objectFactory.Create(implementationType);`
`58`		`- var consumedProperties = new List<string>();`
	`58`	`+ var consumedProperties = new HashSet<string>(StringComparer.Ordinal);`
`59`	`59`	`while (!parser.TryConsume<MappingEnd>(out var _))`
`60`	`60`	`{`
`61`	`61`	`var propertyName = parser.Consume<Scalar>();`
`62`		`- if (duplicateKeyChecking && consumedProperties.Contains(propertyName.Value))`
	`62`	`+ if (duplicateKeyChecking && !consumedProperties.Add(propertyName.Value))`
`63`	`63`	`{`
`64`	`64`	`throw new YamlException(propertyName.Start, propertyName.End, $"Encountered duplicate key {propertyName.Value}");`
`65`	`65`	`}`
`66`	`66`	`try`
`67`	`67`	`{`
`68`		`- consumedProperties.Add(propertyName.Value);`
`69`	`68`	`var property = typeDescriptor.GetProperty(implementationType, null, propertyName.Value, ignoreUnmatched);`
`70`	`69`	`if (property == null)`
`71`	`70`	`{`
Original file line number	Diff line number	Diff line change
`@@ -29,8 +29,8 @@ public class StaticDictionaryNodeDeserializer : DictionaryNodeDeserializer`
`29`	`29`	`{`
`30`	`30`	`private readonly ObjectFactories.StaticObjectFactory _objectFactory;`
`31`	`31`
`32`		`- public StaticDictionaryNodeDeserializer(ObjectFactories.StaticObjectFactory objectFactory)`
`33`		`- : base(objectFactory)`
	`32`	`+ public StaticDictionaryNodeDeserializer(ObjectFactories.StaticObjectFactory objectFactory, bool duplicateKeyChecking)`
	`33`	`+ : base(objectFactory, duplicateKeyChecking)`
`34`	`34`	`{`
`35`	`35`	`_objectFactory = objectFactory ?? throw new ArgumentNullException(nameof(objectFactory));`
`36`	`36`	`}`