From: Steve Harter Date: Mon, 15 Jul 2019 14:07:30 +0000 (-0700) Subject: Replace collection instances on deserialize (dotnet/corefx#39442) X-Git-Tag: submit/tizen/20210909.063632~11031^2~947 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=8ebf4fdde72c1b38d7e426131c5d394ed11b9179;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Replace collection instances on deserialize (dotnet/corefx#39442) Commit migrated from https://github.com/dotnet/corefx/commit/ef1ac63f264c6c1f170315c636da49b4bf421dd9 --- diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs index 73bdfa6..c85f5bd 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs @@ -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(); 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; - } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoCommon.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoCommon.cs index 5c0e2ed..56d7bd7 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoCommon.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoCommon.cs @@ -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) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleArray.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleArray.cs index 52aa468..7c5d335 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleArray.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleArray.cs @@ -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); } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleDictionary.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleDictionary.cs index f4ac055..02300e0 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleDictionary.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleDictionary.cs @@ -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. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs index 54a18c6..f795e62 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs @@ -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)) { diff --git a/src/libraries/System.Text.Json/tests/Serialization/Array.ReadTests.cs b/src/libraries/System.Text.Json/tests/Serialization/Array.ReadTests.cs index a1648b8..d208f3a 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/Array.ReadTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/Array.ReadTests.cs @@ -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(() => JsonSerializer.Deserialize>(Encoding.UTF8.GetBytes(@"[1,""a""]"))); // Multidimensional arrays currently not supported - Assert.Throws(() => JsonSerializer.Deserialize(Encoding.UTF8.GetBytes(@"[[1,2],[3,4]]"))); + Assert.Throws(() => JsonSerializer.Deserialize(Encoding.UTF8.GetBytes(@"[[1,2],[3,4]]"))); } public static IEnumerable ReadNullJson @@ -412,17 +411,32 @@ namespace System.Text.Json.Serialization.Tests obj.Verify(); } + public class ClassWithPopulatedListAndNoSetter + { + public List MyList { get; } = new List() { 1 }; + } + [Fact] public static void ClassWithNoSetter() { - string json = @"{""MyList"":[1]}"; - ClassWithListButNoSetter obj = JsonSerializer.Deserialize(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(json); + Assert.Equal(1, obj.MyList.Count); } - public class ClassWithListButNoSetter + public class ClassWithPopulatedListAndSetter + { + public List MyList { get; set; } = new List() { 1 }; + } + + [Fact] + public static void ClassWithPopulatedList() { - public List MyList { get; } = new List(); + // We don't attempt to deserialize into collections without a setter. + string json = @"{""MyList"":[2,3]}"; + ClassWithPopulatedListAndSetter obj = JsonSerializer.Deserialize(json); + Assert.Equal(2, obj.MyList.Count); } } } diff --git a/src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.cs b/src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.cs index 285021b..a81a68a 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/DictionaryTests.cs @@ -897,7 +897,7 @@ namespace System.Text.Json.Serialization.Tests Assert.Equal(2, foo.IntProperty); foo = JsonSerializer.Deserialize(@"{""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 DictProperty { get; set; } } + public class ClassWithPopulatedDictionaryAndNoSetter + { + public ClassWithPopulatedDictionaryAndNoSetter() + { + MyImmutableDictionary = MyImmutableDictionary.Add("Key", "Value"); + } + + public Dictionary MyDictionary { get; } = new Dictionary() { { "Key", "Value" } }; + public ImmutableDictionary MyImmutableDictionary { get; } = ImmutableDictionary.Create(); + } + [Fact] - public static void ClassWithNoSetter() + public static void ClassWithNoSetterAndDictionary() { - string json = @"{""MyDictionary"":{""Key"":""Value""}}"; - ClassWithDictionaryButNoSetter obj = JsonSerializer.Deserialize(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(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(json); + Assert.Equal(1, obj.MyImmutableDictionary.Count); + } + + public class ClassWithPopulatedDictionaryAndSetter + { + public ClassWithPopulatedDictionaryAndSetter() + { + MyImmutableDictionary = MyImmutableDictionary.Add("Key", "Value"); + } + + public Dictionary MyDictionary { get; set; } = new Dictionary() { { "Key", "Value" } }; + public ImmutableDictionary MyImmutableDictionary { get; set; } = ImmutableDictionary.Create(); + } + + [Fact] + public static void ClassWithPopulatedDictionary() + { + // We replace the contents. + string json = @"{""MyDictionary"":{""Key1"":""Value1"", ""Key2"":""Value2""}}"; + ClassWithPopulatedDictionaryAndSetter obj = JsonSerializer.Deserialize(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(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 MyDictionary { get; } = new Dictionary(); - } - public class ClassWithNotSupportedDictionary { public Dictionary MyDictionary { get; set; } diff --git a/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs b/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs index 676b215..2955981 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/PropertyVisibilityTests.cs @@ -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]