From: Layomi Akinrinade Date: Wed, 13 Nov 2019 07:08:32 +0000 (-0800) Subject: Set values of array and immutable collection properties directly (dotnet/corefx#42420) X-Git-Tag: submit/tizen/20210909.063632~11031^2~16 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=184355fd76518c3e348252c090d5c89a4bc3c510;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Set values of array and immutable collection properties directly (dotnet/corefx#42420) * Set values of array and immutable collection properties directly * Address review feedback Commit migrated from https://github.com/dotnet/corefx/commit/a528006b4536e5b73227285b57d2a56116532506 --- diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/DefaultImmutableEnumerableConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/DefaultImmutableEnumerableConverter.cs index 309757d..2ae8b2e 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/DefaultImmutableEnumerableConverter.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Converters/DefaultImmutableEnumerableConverter.cs @@ -102,19 +102,16 @@ namespace System.Text.Json.Serialization.Converters options.TryAddCreateRangeDelegate(delegateKey, createRangeDelegate); } - public static bool IsImmutableEnumerable(Type type, out bool IsImmutableArray) + public static bool IsImmutableEnumerable(Type type) { if (!type.IsGenericType) { - IsImmutableArray = false; return false; } switch (type.GetGenericTypeDefinition().FullName) { case ImmutableArrayGenericTypeName: - IsImmutableArray = true; - return true; case ImmutableListGenericTypeName: case ImmutableListGenericInterfaceTypeName: case ImmutableStackGenericTypeName: @@ -124,10 +121,8 @@ namespace System.Text.Json.Serialization.Converters case ImmutableSortedSetGenericTypeName: case ImmutableHashSetGenericTypeName: case ImmutableSetGenericInterfaceTypeName: - IsImmutableArray = false; return true; default: - IsImmutableArray = false; return false; } } 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 e8913eb..d9c378e 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 @@ -27,7 +27,6 @@ namespace System.Text.Json private JsonPropertyInfo _dictionaryValuePropertyPolicy; public bool CanBeNull { get; private set; } - public bool IsImmutableArray { get; private set; } public ClassType ClassType; @@ -156,12 +155,10 @@ namespace System.Text.Json DefaultImmutableDictionaryConverter.RegisterImmutableDictionary(RuntimePropertyType, ElementType, Options); DictionaryConverter = s_jsonImmutableDictionaryConverter; } - else if (ClassType == ClassType.Enumerable && DefaultImmutableEnumerableConverter.IsImmutableEnumerable(RuntimePropertyType, out bool isImmutableArray)) + else if (ClassType == ClassType.Enumerable && DefaultImmutableEnumerableConverter.IsImmutableEnumerable(RuntimePropertyType)) { DefaultImmutableEnumerableConverter.RegisterImmutableCollection(RuntimePropertyType, ElementType, Options); EnumerableConverter = s_jsonImmutableEnumerableConverter; - - IsImmutableArray = isImmutableArray; } } } 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 2a89f05..47e092d 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 @@ -89,6 +89,7 @@ namespace System.Text.Json } IEnumerable value = ReadStackFrame.GetEnumerableValue(ref state.Current); + bool setPropertyDirectly = false; if (state.Current.TempEnumerableValues != null) { @@ -103,6 +104,13 @@ namespace System.Text.Json value = converter.CreateFromList(ref state, (IList)value, options); state.Current.TempEnumerableValues = null; + + // Since we used a converter, we just processed an array or an immutable collection. This means we created a new enumerable object. + // If we are processing an enumerable property, replace the current value of the property with the new instance. + if (state.Current.IsProcessingProperty(ClassType.Enumerable)) + { + setPropertyDirectly = true; + } } else if (state.Current.IsProcessingProperty(ClassType.Enumerable)) { @@ -132,7 +140,7 @@ namespace System.Text.Json state.Pop(); } - ApplyObjectToEnumerable(value, ref state); + ApplyObjectToEnumerable(value, ref state, setPropertyDirectly); return false; } @@ -184,9 +192,7 @@ namespace System.Text.Json JsonPropertyInfo jsonPropertyInfo = state.Current.JsonPropertyInfo; object currentEnumerable = jsonPropertyInfo.GetValueAsObject(state.Current.ReturnValue); - if (currentEnumerable == null || - // ImmutableArray is a struct, so default value won't be null. - jsonPropertyInfo.IsImmutableArray) + if (currentEnumerable == null) { jsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, value); } @@ -266,9 +272,7 @@ namespace System.Text.Json JsonPropertyInfo jsonPropertyInfo = state.Current.JsonPropertyInfo; object currentEnumerable = jsonPropertyInfo.GetValueAsObject(state.Current.ReturnValue); - if (currentEnumerable == null || - // ImmutableArray is a struct, so default value won't be null. - jsonPropertyInfo.IsImmutableArray) + if (currentEnumerable == null) { jsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, value); } 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 6418552..f9d064c 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 @@ -202,9 +202,8 @@ 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 && - !state.Current.JsonPropertyInfo.IsImmutableArray) + // Clear the value if present to ensure we don't confuse TempEnumerableValues with the collection. + if (!jsonPropertyInfo.IsPropertyPolicy && jsonPropertyInfo.CanBeNull) { jsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, null); } @@ -242,8 +241,8 @@ namespace System.Text.Json state.Current.TempDictionaryValues = converterDictionary; - // Clear the value if present to ensure we don't confuse tempEnumerableValues with the collection. - if (!jsonPropertyInfo.IsPropertyPolicy) + // Clear the value if present to ensure we don't confuse TempDictionaryValues with the collection. + if (!jsonPropertyInfo.IsPropertyPolicy && jsonPropertyInfo.CanBeNull) { jsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, null); } 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 7481b63..4032c28 100644 --- a/src/libraries/System.Text.Json/tests/Serialization/Array.ReadTests.cs +++ b/src/libraries/System.Text.Json/tests/Serialization/Array.ReadTests.cs @@ -3,6 +3,7 @@ // See the LICENSE file in the project root for more information. using System.Collections.Generic; +using System.Collections.Immutable; using System.Linq; using Xunit; @@ -487,5 +488,146 @@ namespace System.Text.Json.Serialization.Tests Assert.NotNull(parsedObject.ParsedChild3); Assert.True(parsedObject.ParsedChild3.SequenceEqual(new int[] { 3, 3 })); } + + public class ClassWithNonNullEnumerableGetters + { + private string[] _array = null; + private List _list = null; + private StringListWrapper _listWrapper = null; + // Immutable array is a struct. + private ImmutableArray _immutableArray = default; + private ImmutableList _immutableList = null; + + public string[] Array + { + get => _array ?? new string[] { "-1" }; + set { _array = value; } + } + + public List List + { + get => _list ?? new List { "-1" }; + set { _list = value; } + } + + public StringListWrapper ListWrapper + { + get => _listWrapper ?? new StringListWrapper { "-1" }; + set { _listWrapper = value; } + } + + public ImmutableArray MyImmutableArray + { + get => _immutableArray.IsDefault ? ImmutableArray.CreateRange(new List { "-1" }) : _immutableArray; + set { _immutableArray = value; } + } + + public ImmutableList MyImmutableList + { + get => _immutableList ?? ImmutableList.CreateRange(new List { "-1" }); + set { _immutableList = value; } + } + + internal object GetRawArray => _array; + internal object GetRawList => _list; + internal object GetRawListWrapper => _listWrapper; + internal object GetRawImmutableArray => _immutableArray; + internal object GetRawImmutableList => _immutableList; + } + + [Fact] + public static void ClassWithNonNullEnumerableGettersIsParsed() + { + static void TestRoundTrip(ClassWithNonNullEnumerableGetters obj) + { + ClassWithNonNullEnumerableGetters roundtrip = JsonSerializer.Deserialize(JsonSerializer.Serialize(obj)); + + if (obj.Array != null) + { + Assert.Equal(obj.Array.Length, roundtrip.Array.Length); + Assert.Equal(obj.List.Count, roundtrip.List.Count); + Assert.Equal(obj.ListWrapper.Count, roundtrip.ListWrapper.Count); + Assert.Equal(obj.MyImmutableArray.Length, roundtrip.MyImmutableArray.Length); + Assert.Equal(obj.MyImmutableList.Count, roundtrip.MyImmutableList.Count); + + if (obj.Array.Length > 0) + { + Assert.Equal(obj.Array[0], roundtrip.Array[0]); + Assert.Equal(obj.List[0], roundtrip.List[0]); + Assert.Equal(obj.ListWrapper[0], roundtrip.ListWrapper[0]); + Assert.Equal(obj.MyImmutableArray[0], roundtrip.MyImmutableArray[0]); + Assert.Equal(obj.MyImmutableList[0], roundtrip.MyImmutableList[0]); + } + } + else + { + Assert.Null(obj.GetRawArray); + Assert.Null(obj.GetRawList); + Assert.Null(obj.GetRawListWrapper); + Assert.Null(obj.GetRawImmutableList); + Assert.Null(roundtrip.GetRawArray); + Assert.Null(roundtrip.GetRawList); + Assert.Null(roundtrip.GetRawListWrapper); + Assert.Null(roundtrip.GetRawImmutableList); + Assert.Equal(obj.GetRawImmutableArray, roundtrip.GetRawImmutableArray); + } + } + + const string inputJsonWithCollectionElements = + @"{ + ""Array"":[""1""], + ""List"":[""2""], + ""ListWrapper"":[""3""], + ""MyImmutableArray"":[""4""], + ""MyImmutableList"":[""5""] + }"; + + ClassWithNonNullEnumerableGetters obj = JsonSerializer.Deserialize(inputJsonWithCollectionElements); + Assert.Equal(1, obj.Array.Length); + Assert.Equal("1", obj.Array[0]); + + Assert.Equal(1, obj.List.Count); + Assert.Equal("2", obj.List[0]); + + Assert.Equal(1, obj.ListWrapper.Count); + Assert.Equal("3", obj.ListWrapper[0]); + + Assert.Equal(1, obj.MyImmutableArray.Length); + Assert.Equal("4", obj.MyImmutableArray[0]); + + Assert.Equal(1, obj.MyImmutableList.Count); + Assert.Equal("5", obj.MyImmutableList[0]); + + TestRoundTrip(obj); + + const string inputJsonWithoutCollectionElements = + @"{ + ""Array"":[], + ""List"":[], + ""ListWrapper"":[], + ""MyImmutableArray"":[], + ""MyImmutableList"":[] + }"; + + obj = JsonSerializer.Deserialize(inputJsonWithoutCollectionElements); + Assert.Equal(0, obj.Array.Length); + Assert.Equal(0, obj.List.Count); + Assert.Equal(0, obj.ListWrapper.Count); + Assert.Equal(0, obj.MyImmutableArray.Length); + Assert.Equal(0, obj.MyImmutableList.Count); + TestRoundTrip(obj); + + // Skip ImmutableArray due to https://github.com/dotnet/corefx/issues/42399. + const string inputJsonWithNullCollections = + @"{ + ""Array"":null, + ""List"":null, + ""ListWrapper"":null, + ""MyImmutableList"":null + }"; + + obj = JsonSerializer.Deserialize(inputJsonWithNullCollections); + TestRoundTrip(obj); + } } }