Fix Nullable<T> deserialization/serialization and add support for read-only collection and dictionary interface types

Author: xPaw

ValveKeyValue/ValveKeyValue.Test/TestFixtureSources.cs

@@ -15,6 +15,9 @@ public static IEnumerable SupportedEnumerableTypesForDeserialization
                 yield return CreateTestFixtureDataForGenericTest(typeof(Collection<string>));
                 yield return CreateTestFixtureDataForGenericTest(typeof(IList<string>));
                 yield return CreateTestFixtureDataForGenericTest(typeof(ICollection<string>));
+                yield return CreateTestFixtureDataForGenericTest(typeof(IReadOnlyList<string>));
+                yield return CreateTestFixtureDataForGenericTest(typeof(IReadOnlyCollection<string>));
+                yield return CreateTestFixtureDataForGenericTest(typeof(IEnumerable<string>));
                 yield return CreateTestFixtureDataForGenericTest(typeof(ObservableCollection<string>));
             }
         }

ValveKeyValue/ValveKeyValue.Test/Text/NullablePropertyDeserializationTestCase.cs

@@ -85,6 +85,34 @@ public void RequiredStringPropertyIsDefaultWhenMissingFromData()
 
         #endregion
 
+        #region Deserialization - non-nullable properties missing from data
+
+        [Test]
+        public void NonNullableStringPropertyIsNullWhenMissingFromData()
+        {
+            // Only "Name" is present, "Title" and "Count" are missing
+            var vdf = "\"object\"\n{\n\t\"Name\"\t\"hello\"\n}";
+            var obj = KVSerializer.Create(KVSerializationFormat.KeyValues1Text).Deserialize<NonNullableObject>(vdf);
+
+            Assert.That(obj.Name, Is.EqualTo("hello"));
+
+            // GetUninitializedObject zeroes all memory, so non-nullable string is null
+            // This violates the non-nullable contract silently
+            Assert.That(obj.Title, Is.Null);
+        }
+
+        [Test]
+        public void NonNullableIntPropertyIsZeroWhenMissingFromData()
+        {
+            var vdf = "\"object\"\n{\n\t\"Name\"\t\"hello\"\n}";
+            var obj = KVSerializer.Create(KVSerializationFormat.KeyValues1Text).Deserialize<NonNullableObject>(vdf);
+
+            // Non-nullable int defaults to 0 when missing
+            Assert.That(obj.Count, Is.EqualTo(0));
+        }
+
+        #endregion
+
         #region Serialization - nullable properties
 
         [Test]
@@ -212,6 +240,13 @@ class RequiredObject
             public string? Name { get; set; }
         }
 
+        class NonNullableObject
+        {
+            public string Name { get; set; } = null!;
+            public string Title { get; set; } = null!;
+            public int Count { get; set; }
+        }
+
         #endregion
     }
 }

ValveKeyValue/ValveKeyValue.Test/Text/StronglyTypedCollectionDeserializationTestCase.cs

@@ -20,6 +20,7 @@ internal class SimpleObject
     }
 
     [TestFixture(typeof(Dictionary<string, SimpleObject>))]
+    [TestFixture(typeof(IDictionary<string, SimpleObject>))]
     class StronglyTypedDictionaryDeserializationTestCase<TDictionary>
         where TDictionary : IDictionary<string, SimpleObject>
     {
@@ -52,6 +53,39 @@ public void CanDeserializeToObject()
         }
     }
 
+    [TestFixture(typeof(IReadOnlyDictionary<string, SimpleObject>))]
+    class StronglyTypedReadOnlyDictionaryDeserializationTestCase<TDictionary>
+        where TDictionary : IReadOnlyDictionary<string, SimpleObject>
+    {
+        [Test]
+        public void CanDeserializeToObject()
+        {
+            StronglyTypedCollectionDeserializationTestCase.RootObject<TDictionary> rootObject;
+
+            using (var resourceStream = TestDataHelper.OpenResource("Text.list_of_objects.vdf"))
+            {
+                rootObject = KVSerializer.Create(KVSerializationFormat.KeyValues1Text)
+                    .Deserialize<StronglyTypedCollectionDeserializationTestCase.RootObject<TDictionary>>(resourceStream);
+            }
+
+            Assert.That(rootObject, Is.Not.Null);
+            Assert.That(rootObject.Numbers, Is.Not.Null);
+            Assert.That(rootObject.Numbers, Is.InstanceOf<TDictionary>());
+            Assert.That(rootObject.Numbers, Has.Count.EqualTo(3));
+            Assert.That(rootObject.Numbers["0"], Is.Not.Null);
+            Assert.That(rootObject.Numbers["0"].Name, Is.EqualTo("zero"));
+            Assert.That(rootObject.Numbers["0"].Value, Is.EqualTo("nothing"));
+
+            Assert.That(rootObject.Numbers["1"], Is.Not.Null);
+            Assert.That(rootObject.Numbers["1"].Name, Is.EqualTo("one"));
+            Assert.That(rootObject.Numbers["1"].Value, Is.EqualTo("a bit"));
+
+            Assert.That(rootObject.Numbers["2"], Is.Not.Null);
+            Assert.That(rootObject.Numbers["2"].Name, Is.EqualTo("two"));
+            Assert.That(rootObject.Numbers["2"].Value, Is.EqualTo("a bit more"));
+        }
+    }
+
     [TestFixture(typeof(SimpleObject[]))]
     [TestFixture(typeof(List<SimpleObject>))]
     class StronglyTypedCollectionDeserializationTestCase<TCollection>

ValveKeyValue/ValveKeyValue/ObjectCopier.cs

@@ -231,6 +231,9 @@ static object MakeLookup(
         {
             [typeof(List<>)] = (type, values, reflector) => InvokeGeneric(nameof(MakeList), type.GetGenericArguments()[0], new object[] { values, reflector }),
             [typeof(IList<>)] = (type, values, reflector) => InvokeGeneric(nameof(MakeList), type.GetGenericArguments()[0], new object[] { values, reflector }),
+            [typeof(IReadOnlyList<>)] = (type, values, reflector) => InvokeGeneric(nameof(MakeList), type.GetGenericArguments()[0], new object[] { values, reflector }),
+            [typeof(IReadOnlyCollection<>)] = (type, values, reflector) => InvokeGeneric(nameof(MakeList), type.GetGenericArguments()[0], new object[] { values, reflector }),
+            [typeof(IEnumerable<>)] = (type, values, reflector) => InvokeGeneric(nameof(MakeList), type.GetGenericArguments()[0], new object[] { values, reflector }),
             [typeof(Collection<>)] = (type, values, reflector) => InvokeGeneric(nameof(MakeCollection), type.GetGenericArguments()[0], new object[] { values, reflector }),
             [typeof(ICollection<>)] = (type, values, reflector) => InvokeGeneric(nameof(MakeCollection), type.GetGenericArguments()[0], new object[] { values, reflector }),
             [typeof(ObservableCollection<>)] = (type, values, reflector) => InvokeGeneric(nameof(MakeObservableCollection), type.GetGenericArguments()[0], new object[] { values, reflector }),
@@ -352,12 +355,9 @@ static bool IsDictionary(Type type)
             }
 
             var genericType = type.GetGenericTypeDefinition();
-            if (genericType != typeof(Dictionary<,>))
-            {
-                return false;
-            }
-
-            return true;
+            return genericType == typeof(Dictionary<,>)
+                || genericType == typeof(IDictionary<,>)
+                || genericType == typeof(IReadOnlyDictionary<,>);
         }
 
         [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2060", Justification = "Analysis cannot follow MakeGenericMethod but we should be clear by here anyway.")]
@@ -367,9 +367,15 @@ static object MakeDictionary(
             KVObject kv,
             IObjectReflector reflector)
         {
-            var dictionary = Activator.CreateInstance(type);
             var genericArguments = type.GetGenericArguments();
 
+            // For interface types (IDictionary<,>, IReadOnlyDictionary<,>), use concrete Dictionary<,>
+            var concreteType = type.GetGenericTypeDefinition() == typeof(Dictionary<,>)
+                ? type
+                : typeof(Dictionary<,>).MakeGenericType(genericArguments);
+
+            var dictionary = Activator.CreateInstance(concreteType);
+
             var method = typeof(ObjectCopier)
                 .GetMethod(nameof(FillDictionary), BindingFlags.Static | BindingFlags.NonPublic)!;
             method.MakeGenericMethod(genericArguments)