Replace collection instances on deserialize (dotnet/corefx#39442)
authorSteve Harter <steveharter@users.noreply.github.com>
Mon, 15 Jul 2019 14:07:30 +0000 (07:07 -0700)
committerGitHub <noreply@github.com>
Mon, 15 Jul 2019 14:07:30 +0000 (07:07 -0700)
Commit migrated from https://github.com/dotnet/corefx/commit/ef1ac63f264c6c1f170315c636da49b4bf421dd9

src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoCommon.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleArray.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleDictionary.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs
src/libraries/System.Text.Json/tests/Serialization/Array.ReadTests.cs
src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.cs
src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs

index 73bdfa6..c85f5bd 100644 (file)
@@ -3,10 +3,8 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Collections;
-using System.Collections.Generic;
 using System.Diagnostics;
 using System.Reflection;
-using System.Runtime.CompilerServices;
 using System.Text.Json.Serialization;
 using System.Text.Json.Serialization.Converters;
 
@@ -24,7 +22,6 @@ namespace System.Text.Json
         private static readonly JsonDictionaryConverter s_jsonIDictionaryConverter = new DefaultIDictionaryConverter();
         private static readonly JsonDictionaryConverter s_jsonImmutableDictionaryConverter = new DefaultImmutableDictionaryConverter();
         
-
         public static readonly JsonPropertyInfo s_missingProperty = new JsonPropertyInfoNotNullable<object, object, object, object>();
 
         private Type _elementType;
@@ -141,82 +138,73 @@ namespace System.Text.Json
             {
                 if (HasGetter)
                 {
+                    ShouldSerialize = true;
+
                     if (HasSetter)
                     {
                         ShouldDeserialize = true;
-                    }
-                    else if (!RuntimePropertyType.IsArray &&
-                        (typeof(IList).IsAssignableFrom(RuntimePropertyType) || typeof(IDictionary).IsAssignableFrom(RuntimePropertyType)))
-                    {
-                        ShouldDeserialize = true;
-                    }
-                }
-                //else if (HasSetter)
-                //{
-                //    // todo: Special case where there is no getter but a setter (and an EnumerableConverter)
-                //}
-
-                if (ShouldDeserialize)
-                {
-                    ShouldSerialize = HasGetter;
 
-                    if (RuntimePropertyType.IsArray)
-                    {
-                        EnumerableConverter = s_jsonArrayConverter;
-                    }
-                    else if (ClassType == ClassType.IDictionaryConstructible)
-                    {
-                        // Natively supported type.
-                        if (DeclaredPropertyType == ImplementedPropertyType)
+                        if (RuntimePropertyType.IsArray)
                         {
-                            if (RuntimePropertyType.FullName.StartsWith(JsonClassInfo.ImmutableNamespaceName))
+                            // Verify that we don't have a multidimensional array.
+                            if (RuntimePropertyType.GetArrayRank() > 1)
                             {
-                                DefaultImmutableDictionaryConverter.RegisterImmutableDictionary(
-                                    RuntimePropertyType, JsonClassInfo.GetElementType(RuntimePropertyType, ParentClassType, PropertyInfo, Options), Options);
+                                throw ThrowHelper.GetNotSupportedException_SerializationNotSupportedCollection(RuntimePropertyType, ParentClassType, PropertyInfo);
+                            }
 
-                                DictionaryConverter = s_jsonImmutableDictionaryConverter;
+                            EnumerableConverter = s_jsonArrayConverter;
+                        }
+                        else if (ClassType == ClassType.IDictionaryConstructible)
+                        {
+                            // Natively supported type.
+                            if (DeclaredPropertyType == ImplementedPropertyType)
+                            {
+                                if (RuntimePropertyType.FullName.StartsWith(JsonClassInfo.ImmutableNamespaceName))
+                                {
+                                    DefaultImmutableDictionaryConverter.RegisterImmutableDictionary(
+                                        RuntimePropertyType, JsonClassInfo.GetElementType(RuntimePropertyType, ParentClassType, PropertyInfo, Options), Options);
+
+                                    DictionaryConverter = s_jsonImmutableDictionaryConverter;
+                                }
+                                else if (JsonClassInfo.IsDeserializedByConstructingWithIDictionary(RuntimePropertyType))
+                                {
+                                    DictionaryConverter = s_jsonIDictionaryConverter;
+                                }
                             }
-                            else if (JsonClassInfo.IsDeserializedByConstructingWithIDictionary(RuntimePropertyType))
+                            // Type that implements a type with ClassType IDictionaryConstructible.
+                            else
                             {
-                                DictionaryConverter = s_jsonIDictionaryConverter;
+                                DictionaryConverter = s_jsonDerivedDictionaryConverter;
                             }
                         }
-                        // Type that implements a type with ClassType IDictionaryConstructible.
-                        else
-                        {
-                            DictionaryConverter = s_jsonDerivedDictionaryConverter;
-                        }
-                    }
-                    else if (ClassType == ClassType.Enumerable)
-                    {
-                        // Else if it's an implementing type that is not assignable from IList.
-                        if (DeclaredPropertyType != ImplementedPropertyType &&
-                            (!typeof(IList).IsAssignableFrom(DeclaredPropertyType) ||
-                            ImplementedPropertyType == typeof(ArrayList) ||
-                            ImplementedPropertyType == typeof(IList)))
-                        {
-                            EnumerableConverter = s_jsonDerivedEnumerableConverter;
-                        }
-                        else if (JsonClassInfo.IsDeserializedByConstructingWithIList(RuntimePropertyType) ||
-                            (!typeof(IList).IsAssignableFrom(RuntimePropertyType) && 
-                            JsonClassInfo.HasConstructorThatTakesGenericIEnumerable(RuntimePropertyType, Options)))
+                        else if (ClassType == ClassType.Enumerable)
                         {
-                            EnumerableConverter = s_jsonICollectionConverter;
-                        }
-                        else if (RuntimePropertyType.IsGenericType &&
-                            RuntimePropertyType.FullName.StartsWith(JsonClassInfo.ImmutableNamespaceName) &&
-                            RuntimePropertyType.GetGenericArguments().Length == 1)
-                        {
-                            DefaultImmutableEnumerableConverter.RegisterImmutableCollection(RuntimePropertyType,
-                                JsonClassInfo.GetElementType(RuntimePropertyType, ParentClassType, PropertyInfo, Options), Options);
-                            EnumerableConverter = s_jsonImmutableEnumerableConverter;
+                            // Else if it's an implementing type that is not assignable from IList.
+                            if (DeclaredPropertyType != ImplementedPropertyType &&
+                                (!typeof(IList).IsAssignableFrom(DeclaredPropertyType) ||
+                                ImplementedPropertyType == typeof(ArrayList) ||
+                                ImplementedPropertyType == typeof(IList)))
+                            {
+                                EnumerableConverter = s_jsonDerivedEnumerableConverter;
+                            }
+                            else if (JsonClassInfo.IsDeserializedByConstructingWithIList(RuntimePropertyType) ||
+                                (!typeof(IList).IsAssignableFrom(RuntimePropertyType) &&
+                                JsonClassInfo.HasConstructorThatTakesGenericIEnumerable(RuntimePropertyType, Options)))
+                            {
+                                EnumerableConverter = s_jsonICollectionConverter;
+                            }
+                            else if (RuntimePropertyType.IsGenericType &&
+                                RuntimePropertyType.FullName.StartsWith(JsonClassInfo.ImmutableNamespaceName) &&
+                                RuntimePropertyType.GetGenericArguments().Length == 1)
+                            {
+                                DefaultImmutableEnumerableConverter.RegisterImmutableCollection(RuntimePropertyType,
+                                    JsonClassInfo.GetElementType(RuntimePropertyType, ParentClassType, PropertyInfo, Options), Options);
+                                EnumerableConverter = s_jsonImmutableEnumerableConverter;
+                            }
+
                         }
                     }
                 }
-                else
-                {
-                    ShouldSerialize = HasGetter && !Options.IgnoreReadOnlyProperties;
-                }
             }
         }
 
index 5c0e2ed..56d7bd7 100644 (file)
@@ -7,7 +7,6 @@ using System.Collections.Generic;
 using System.Diagnostics;
 using System.Reflection;
 using System.Text.Json.Serialization;
-using System.Text.Json.Serialization.Converters;
 
 namespace System.Text.Json
 {
@@ -72,11 +71,6 @@ namespace System.Text.Json
             }
         }
 
-        public override void GetPolicies()
-        {
-            base.GetPolicies();
-        }
-
         public override object GetValueAsObject(object obj)
         {
             if (IsPropertyPolicy)
index 52aa468..7c5d335 100644 (file)
@@ -16,8 +16,6 @@ namespace System.Text.Json
             ref Utf8JsonReader reader,
             ref ReadStack state)
         {
-            JsonPropertyInfo jsonPropertyInfo = state.Current.JsonPropertyInfo;
-
             if (state.Current.SkipProperty)
             {
                 // The array is not being applied to the object.
@@ -26,6 +24,7 @@ namespace System.Text.Json
                 return;
             }
 
+            JsonPropertyInfo jsonPropertyInfo = state.Current.JsonPropertyInfo;
             if (jsonPropertyInfo == null)
             {
                 jsonPropertyInfo = state.Current.JsonClassInfo.CreateRootObject(options);
@@ -35,9 +34,9 @@ namespace System.Text.Json
                 jsonPropertyInfo = state.Current.JsonClassInfo.CreatePolymorphicProperty(jsonPropertyInfo, typeof(object), options);
             }
 
-            // Verify that we don't have a multidimensional array.
+            // Verify that we have a valid enumerable.
             Type arrayType = jsonPropertyInfo.RuntimePropertyType;
-            if (!typeof(IEnumerable).IsAssignableFrom(arrayType) || (arrayType.IsArray && arrayType.GetArrayRank() > 1))
+            if (!typeof(IEnumerable).IsAssignableFrom(arrayType))
             {
                 ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(arrayType, reader, state.JsonPath);
             }
@@ -54,32 +53,28 @@ namespace System.Text.Json
             }
 
             state.Current.CollectionPropertyInitialized = true;
-            jsonPropertyInfo = state.Current.JsonPropertyInfo;
 
             if (state.Current.JsonClassInfo.ClassType == ClassType.Value)
             {
+                // Custom converter code path.
                 state.Current.JsonPropertyInfo.Read(JsonTokenType.StartObject, ref state, ref reader);
             }
             else
             {
-                // If current property is already set (from a constructor, for example) leave as-is.
-                if (jsonPropertyInfo.GetValueAsObject(state.Current.ReturnValue) == null)
-                {
-                    // Create the enumerable.
-                    object value = ReadStackFrame.CreateEnumerableValue(ref reader, ref state, options);
+                // Set or replace the existing enumerable value.
+                object value = ReadStackFrame.CreateEnumerableValue(ref reader, ref state);
 
-                    // If value is not null, then we don't have a converter so apply the value.
-                    if (value != null)
+                // If value is not null, then we don't have a converter so apply the value.
+                if (value != null)
+                {
+                    if (state.Current.ReturnValue != null)
+                    {
+                        state.Current.JsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, value);
+                    }
+                    else
                     {
-                        if (state.Current.ReturnValue != null)
-                        {
-                            state.Current.JsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, value);
-                        }
-                        else
-                        {
-                            // Primitive arrays being returned without object
-                            state.Current.SetReturnValue(value);
-                        }
+                        // Primitive arrays being returned without object
+                        state.Current.SetReturnValue(value);
                     }
                 }
             }
index f4ac055..02300e0 100644 (file)
@@ -70,10 +70,10 @@ namespace System.Text.Json
                 {
                     dictionaryClassInfo = options.GetOrAddClass(jsonPropertyInfo.DeclaredPropertyType);
                 }
+
                 state.Current.TempDictionaryValues = (IDictionary)dictionaryClassInfo.CreateConcreteDictionary();
             }
-            // Else if current property is already set (from a constructor, for example), leave as-is.
-            else if (jsonPropertyInfo.GetValueAsObject(state.Current.ReturnValue) == null)
+            else
             {
                 // Create the dictionary.
                 JsonClassInfo dictionaryClassInfo = jsonPropertyInfo.RuntimeClassInfo;
@@ -96,6 +96,11 @@ namespace System.Text.Json
 
         private static void HandleEndDictionary(JsonSerializerOptions options, ref Utf8JsonReader reader, ref ReadStack state)
         {
+            if (state.Current.SkipProperty)
+            {
+                return;
+            }
+
             if (state.Current.IsDictionaryProperty)
             {
                 // We added the items to the dictionary already.
index 54a18c6..f795e62 100644 (file)
@@ -135,7 +135,7 @@ namespace System.Text.Json
             KeyName = null;
         }
 
-        public static object CreateEnumerableValue(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
+        public static object CreateEnumerableValue(ref Utf8JsonReader reader, ref ReadStack state)
         {
             JsonPropertyInfo jsonPropertyInfo = state.Current.JsonPropertyInfo;
 
@@ -154,10 +154,15 @@ namespace System.Text.Json
 
                 state.Current.TempEnumerableValues = converterList;
 
+                // Clear the value if present to ensure we don't confuse tempEnumerableValues with the collection. 
+                if (!jsonPropertyInfo.IsPropertyPolicy)
+                {
+                    jsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, null);
+                }
+
                 return null;
             }
 
-
             Type propertyType = jsonPropertyInfo.RuntimePropertyType;
             if (typeof(IList).IsAssignableFrom(propertyType))
             {
index a1648b8..d208f3a 100644 (file)
@@ -168,7 +168,6 @@ namespace System.Text.Json.Serialization.Tests
             Assert.Equal(0, i.Length);
         }
 
-        [ActiveIssue(38435)]
         [Fact]
         public static void ReadInitializedArrayTest()
         {
@@ -198,7 +197,7 @@ namespace System.Text.Json.Serialization.Tests
             Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<List<int?>>(Encoding.UTF8.GetBytes(@"[1,""a""]")));
 
             // Multidimensional arrays currently not supported
-            Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<int[,]>(Encoding.UTF8.GetBytes(@"[[1,2],[3,4]]")));
+            Assert.Throws<NotSupportedException>(() => JsonSerializer.Deserialize<int[,]>(Encoding.UTF8.GetBytes(@"[[1,2],[3,4]]")));
         }
 
         public static IEnumerable<object[]> ReadNullJson
@@ -412,17 +411,32 @@ namespace System.Text.Json.Serialization.Tests
             obj.Verify();
         }
 
+        public class ClassWithPopulatedListAndNoSetter
+        {
+            public List<int> MyList { get; } = new List<int>() { 1 };
+        }
+
         [Fact]
         public static void ClassWithNoSetter()
         {
-            string json = @"{""MyList"":[1]}";
-            ClassWithListButNoSetter obj = JsonSerializer.Deserialize<ClassWithListButNoSetter>(json);
-            Assert.Equal(1, obj.MyList[0]);
+            // We don't attempt to deserialize into collections without a setter.
+            string json = @"{""MyList"":[1,2]}";
+            ClassWithPopulatedListAndNoSetter obj = JsonSerializer.Deserialize<ClassWithPopulatedListAndNoSetter>(json);
+            Assert.Equal(1, obj.MyList.Count);
         }
 
-        public class ClassWithListButNoSetter
+        public class ClassWithPopulatedListAndSetter
+        {
+            public List<int> MyList { get; set;  } = new List<int>() { 1 };
+        }
+
+        [Fact]
+        public static void ClassWithPopulatedList()
         {
-            public List<int> MyList { get; } = new List<int>();
+            // We don't attempt to deserialize into collections without a setter.
+            string json = @"{""MyList"":[2,3]}";
+            ClassWithPopulatedListAndSetter obj = JsonSerializer.Deserialize<ClassWithPopulatedListAndSetter>(json);
+            Assert.Equal(2, obj.MyList.Count);
         }
     }
 }
index 285021b..a81a68a 100644 (file)
@@ -897,7 +897,7 @@ namespace System.Text.Json.Serialization.Tests
             Assert.Equal(2, foo.IntProperty);
 
             foo = JsonSerializer.Deserialize<PocoDuplicate>(@"{""DictProperty"" : {""a"" : ""b"", ""c"" : ""d""},""DictProperty"" : {""b"" : ""b"", ""c"" : ""e""}}");
-            Assert.Equal(3, foo.DictProperty.Count);
+            Assert.Equal(2, foo.DictProperty.Count); // We don't concat.
             Assert.Equal("e", foo.DictProperty["c"]);
         }
 
@@ -908,12 +908,62 @@ namespace System.Text.Json.Serialization.Tests
             public Dictionary<string, string> DictProperty { get; set; }
         }
 
+        public class ClassWithPopulatedDictionaryAndNoSetter
+        {
+            public ClassWithPopulatedDictionaryAndNoSetter()
+            {
+                MyImmutableDictionary = MyImmutableDictionary.Add("Key", "Value");
+            }
+
+            public Dictionary<string, string> MyDictionary { get; } = new Dictionary<string, string>() { { "Key", "Value" } };
+            public ImmutableDictionary<string, string> MyImmutableDictionary { get; } = ImmutableDictionary.Create<string, string>();
+        }
+
         [Fact]
-        public static void ClassWithNoSetter()
+        public static void ClassWithNoSetterAndDictionary()
         {
-            string json = @"{""MyDictionary"":{""Key"":""Value""}}";
-            ClassWithDictionaryButNoSetter obj = JsonSerializer.Deserialize<ClassWithDictionaryButNoSetter>(json);
-            Assert.Equal("Value", obj.MyDictionary["Key"]);
+            // We don't attempt to deserialize into dictionaries without a setter.
+            string json = @"{""MyDictionary"":{""Key1"":""Value1"", ""Key2"":""Value2""}}";
+            ClassWithPopulatedDictionaryAndNoSetter obj = JsonSerializer.Deserialize<ClassWithPopulatedDictionaryAndNoSetter>(json);
+            Assert.Equal(1, obj.MyDictionary.Count);
+        }
+
+        [Fact]
+        public static void ClassWithNoSetterAndImmutableDictionary()
+        {
+            // We don't attempt to deserialize into dictionaries without a setter.
+            string json = @"{""MyImmutableDictionary"":{""Key1"":""Value1"", ""Key2"":""Value2""}}";
+            ClassWithPopulatedDictionaryAndNoSetter obj = JsonSerializer.Deserialize<ClassWithPopulatedDictionaryAndNoSetter>(json);
+            Assert.Equal(1, obj.MyImmutableDictionary.Count);
+        }
+
+        public class ClassWithPopulatedDictionaryAndSetter
+        {
+            public ClassWithPopulatedDictionaryAndSetter()
+            {
+                MyImmutableDictionary = MyImmutableDictionary.Add("Key", "Value");
+            }
+
+            public Dictionary<string, string> MyDictionary { get; set; } = new Dictionary<string, string>() { { "Key", "Value" } };
+            public ImmutableDictionary<string, string> MyImmutableDictionary { get; set; } = ImmutableDictionary.Create<string, string>();
+        }
+
+        [Fact]
+        public static void ClassWithPopulatedDictionary()
+        {
+            // We replace the contents.
+            string json = @"{""MyDictionary"":{""Key1"":""Value1"", ""Key2"":""Value2""}}";
+            ClassWithPopulatedDictionaryAndSetter obj = JsonSerializer.Deserialize<ClassWithPopulatedDictionaryAndSetter>(json);
+            Assert.Equal(2, obj.MyDictionary.Count);
+        }
+
+        [Fact]
+        public static void ClassWithPopulatedImmutableDictionary()
+        {
+            // We replace the contents.
+            string json = @"{""MyImmutableDictionary"":{""Key1"":""Value1"", ""Key2"":""Value2""}}";
+            ClassWithPopulatedDictionaryAndSetter obj = JsonSerializer.Deserialize<ClassWithPopulatedDictionaryAndSetter>(json);
+            Assert.Equal(2, obj.MyImmutableDictionary.Count);
         }
 
         [Fact]
@@ -1109,11 +1159,6 @@ namespace System.Text.Json.Serialization.Tests
             Assert.Null(dictionaryLast.Dict);
         }
 
-        public class ClassWithDictionaryButNoSetter
-        {
-            public Dictionary<string, string> MyDictionary { get; } = new Dictionary<string, string>();
-        }
-
         public class ClassWithNotSupportedDictionary
         {
             public Dictionary<int, int> MyDictionary { get; set; }
index 676b215..2955981 100644 (file)
@@ -32,7 +32,9 @@ namespace System.Text.Json.Serialization.Tests
             var obj = new ClassWithNoSetter();
 
             string json = JsonSerializer.Serialize(obj, options);
-            Assert.Equal(@"{}", json);
+
+            // Collections are always serialized unless they have [JsonIgnore].
+            Assert.Equal(@"{""MyInts"":[1,2]}", json);
         }
 
         [Fact]