Skip to content

Commit 8ebf4fd

Browse files
stevehartersteveharter
authored
Replace collection instances on deserialize (dotnet/corefx#39442)
Commit migrated from dotnet/corefx@ef1ac63
1 parent fdf096a commit 8ebf4fd

File tree

8 files changed

+161
-113
lines changed

8 files changed

+161
-113
lines changed

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs

+52-64
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,8 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System.Collections;
6-
using System.Collections.Generic;
76
using System.Diagnostics;
87
using System.Reflection;
9-
using System.Runtime.CompilerServices;
108
using System.Text.Json.Serialization;
119
using System.Text.Json.Serialization.Converters;
1210

@@ -24,7 +22,6 @@ internal abstract class JsonPropertyInfo
2422
private static readonly JsonDictionaryConverter s_jsonIDictionaryConverter = new DefaultIDictionaryConverter();
2523
private static readonly JsonDictionaryConverter s_jsonImmutableDictionaryConverter = new DefaultImmutableDictionaryConverter();
2624

27-
2825
public static readonly JsonPropertyInfo s_missingProperty = new JsonPropertyInfoNotNullable<object, object, object, object>();
2926

3027
private Type _elementType;
@@ -141,82 +138,73 @@ private void DetermineSerializationCapabilities()
141138
{
142139
if (HasGetter)
143140
{
141+
ShouldSerialize = true;
142+
144143
if (HasSetter)
145144
{
146145
ShouldDeserialize = true;
147-
}
148-
else if (!RuntimePropertyType.IsArray &&
149-
(typeof(IList).IsAssignableFrom(RuntimePropertyType) || typeof(IDictionary).IsAssignableFrom(RuntimePropertyType)))
150-
{
151-
ShouldDeserialize = true;
152-
}
153-
}
154-
//else if (HasSetter)
155-
//{
156-
// // todo: Special case where there is no getter but a setter (and an EnumerableConverter)
157-
//}
158-
159-
if (ShouldDeserialize)
160-
{
161-
ShouldSerialize = HasGetter;
162146

163-
if (RuntimePropertyType.IsArray)
164-
{
165-
EnumerableConverter = s_jsonArrayConverter;
166-
}
167-
else if (ClassType == ClassType.IDictionaryConstructible)
168-
{
169-
// Natively supported type.
170-
if (DeclaredPropertyType == ImplementedPropertyType)
147+
if (RuntimePropertyType.IsArray)
171148
{
172-
if (RuntimePropertyType.FullName.StartsWith(JsonClassInfo.ImmutableNamespaceName))
149+
// Verify that we don't have a multidimensional array.
150+
if (RuntimePropertyType.GetArrayRank() > 1)
173151
{
174-
DefaultImmutableDictionaryConverter.RegisterImmutableDictionary(
175-
RuntimePropertyType, JsonClassInfo.GetElementType(RuntimePropertyType, ParentClassType, PropertyInfo, Options), Options);
152+
throw ThrowHelper.GetNotSupportedException_SerializationNotSupportedCollection(RuntimePropertyType, ParentClassType, PropertyInfo);
153+
}
176154

177-
DictionaryConverter = s_jsonImmutableDictionaryConverter;
155+
EnumerableConverter = s_jsonArrayConverter;
156+
}
157+
else if (ClassType == ClassType.IDictionaryConstructible)
158+
{
159+
// Natively supported type.
160+
if (DeclaredPropertyType == ImplementedPropertyType)
161+
{
162+
if (RuntimePropertyType.FullName.StartsWith(JsonClassInfo.ImmutableNamespaceName))
163+
{
164+
DefaultImmutableDictionaryConverter.RegisterImmutableDictionary(
165+
RuntimePropertyType, JsonClassInfo.GetElementType(RuntimePropertyType, ParentClassType, PropertyInfo, Options), Options);
166+
167+
DictionaryConverter = s_jsonImmutableDictionaryConverter;
168+
}
169+
else if (JsonClassInfo.IsDeserializedByConstructingWithIDictionary(RuntimePropertyType))
170+
{
171+
DictionaryConverter = s_jsonIDictionaryConverter;
172+
}
178173
}
179-
else if (JsonClassInfo.IsDeserializedByConstructingWithIDictionary(RuntimePropertyType))
174+
// Type that implements a type with ClassType IDictionaryConstructible.
175+
else
180176
{
181-
DictionaryConverter = s_jsonIDictionaryConverter;
177+
DictionaryConverter = s_jsonDerivedDictionaryConverter;
182178
}
183179
}
184-
// Type that implements a type with ClassType IDictionaryConstructible.
185-
else
186-
{
187-
DictionaryConverter = s_jsonDerivedDictionaryConverter;
188-
}
189-
}
190-
else if (ClassType == ClassType.Enumerable)
191-
{
192-
// Else if it's an implementing type that is not assignable from IList.
193-
if (DeclaredPropertyType != ImplementedPropertyType &&
194-
(!typeof(IList).IsAssignableFrom(DeclaredPropertyType) ||
195-
ImplementedPropertyType == typeof(ArrayList) ||
196-
ImplementedPropertyType == typeof(IList)))
197-
{
198-
EnumerableConverter = s_jsonDerivedEnumerableConverter;
199-
}
200-
else if (JsonClassInfo.IsDeserializedByConstructingWithIList(RuntimePropertyType) ||
201-
(!typeof(IList).IsAssignableFrom(RuntimePropertyType) &&
202-
JsonClassInfo.HasConstructorThatTakesGenericIEnumerable(RuntimePropertyType, Options)))
180+
else if (ClassType == ClassType.Enumerable)
203181
{
204-
EnumerableConverter = s_jsonICollectionConverter;
205-
}
206-
else if (RuntimePropertyType.IsGenericType &&
207-
RuntimePropertyType.FullName.StartsWith(JsonClassInfo.ImmutableNamespaceName) &&
208-
RuntimePropertyType.GetGenericArguments().Length == 1)
209-
{
210-
DefaultImmutableEnumerableConverter.RegisterImmutableCollection(RuntimePropertyType,
211-
JsonClassInfo.GetElementType(RuntimePropertyType, ParentClassType, PropertyInfo, Options), Options);
212-
EnumerableConverter = s_jsonImmutableEnumerableConverter;
182+
// Else if it's an implementing type that is not assignable from IList.
183+
if (DeclaredPropertyType != ImplementedPropertyType &&
184+
(!typeof(IList).IsAssignableFrom(DeclaredPropertyType) ||
185+
ImplementedPropertyType == typeof(ArrayList) ||
186+
ImplementedPropertyType == typeof(IList)))
187+
{
188+
EnumerableConverter = s_jsonDerivedEnumerableConverter;
189+
}
190+
else if (JsonClassInfo.IsDeserializedByConstructingWithIList(RuntimePropertyType) ||
191+
(!typeof(IList).IsAssignableFrom(RuntimePropertyType) &&
192+
JsonClassInfo.HasConstructorThatTakesGenericIEnumerable(RuntimePropertyType, Options)))
193+
{
194+
EnumerableConverter = s_jsonICollectionConverter;
195+
}
196+
else if (RuntimePropertyType.IsGenericType &&
197+
RuntimePropertyType.FullName.StartsWith(JsonClassInfo.ImmutableNamespaceName) &&
198+
RuntimePropertyType.GetGenericArguments().Length == 1)
199+
{
200+
DefaultImmutableEnumerableConverter.RegisterImmutableCollection(RuntimePropertyType,
201+
JsonClassInfo.GetElementType(RuntimePropertyType, ParentClassType, PropertyInfo, Options), Options);
202+
EnumerableConverter = s_jsonImmutableEnumerableConverter;
203+
}
204+
213205
}
214206
}
215207
}
216-
else
217-
{
218-
ShouldSerialize = HasGetter && !Options.IgnoreReadOnlyProperties;
219-
}
220208
}
221209
}
222210

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoCommon.cs

-6
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
using System.Diagnostics;
88
using System.Reflection;
99
using System.Text.Json.Serialization;
10-
using System.Text.Json.Serialization.Converters;
1110

1211
namespace System.Text.Json
1312
{
@@ -72,11 +71,6 @@ public override JsonConverter ConverterBase
7271
}
7372
}
7473

75-
public override void GetPolicies()
76-
{
77-
base.GetPolicies();
78-
}
79-
8074
public override object GetValueAsObject(object obj)
8175
{
8276
if (IsPropertyPolicy)

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleArray.cs

+16-21
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ private static void HandleStartArray(
1616
ref Utf8JsonReader reader,
1717
ref ReadStack state)
1818
{
19-
JsonPropertyInfo jsonPropertyInfo = state.Current.JsonPropertyInfo;
20-
2119
if (state.Current.SkipProperty)
2220
{
2321
// The array is not being applied to the object.
@@ -26,6 +24,7 @@ private static void HandleStartArray(
2624
return;
2725
}
2826

27+
JsonPropertyInfo jsonPropertyInfo = state.Current.JsonPropertyInfo;
2928
if (jsonPropertyInfo == null)
3029
{
3130
jsonPropertyInfo = state.Current.JsonClassInfo.CreateRootObject(options);
@@ -35,9 +34,9 @@ private static void HandleStartArray(
3534
jsonPropertyInfo = state.Current.JsonClassInfo.CreatePolymorphicProperty(jsonPropertyInfo, typeof(object), options);
3635
}
3736

38-
// Verify that we don't have a multidimensional array.
37+
// Verify that we have a valid enumerable.
3938
Type arrayType = jsonPropertyInfo.RuntimePropertyType;
40-
if (!typeof(IEnumerable).IsAssignableFrom(arrayType) || (arrayType.IsArray && arrayType.GetArrayRank() > 1))
39+
if (!typeof(IEnumerable).IsAssignableFrom(arrayType))
4140
{
4241
ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(arrayType, reader, state.JsonPath);
4342
}
@@ -54,32 +53,28 @@ private static void HandleStartArray(
5453
}
5554

5655
state.Current.CollectionPropertyInitialized = true;
57-
jsonPropertyInfo = state.Current.JsonPropertyInfo;
5856

5957
if (state.Current.JsonClassInfo.ClassType == ClassType.Value)
6058
{
59+
// Custom converter code path.
6160
state.Current.JsonPropertyInfo.Read(JsonTokenType.StartObject, ref state, ref reader);
6261
}
6362
else
6463
{
65-
// If current property is already set (from a constructor, for example) leave as-is.
66-
if (jsonPropertyInfo.GetValueAsObject(state.Current.ReturnValue) == null)
67-
{
68-
// Create the enumerable.
69-
object value = ReadStackFrame.CreateEnumerableValue(ref reader, ref state, options);
64+
// Set or replace the existing enumerable value.
65+
object value = ReadStackFrame.CreateEnumerableValue(ref reader, ref state);
7066

71-
// If value is not null, then we don't have a converter so apply the value.
72-
if (value != null)
67+
// If value is not null, then we don't have a converter so apply the value.
68+
if (value != null)
69+
{
70+
if (state.Current.ReturnValue != null)
71+
{
72+
state.Current.JsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, value);
73+
}
74+
else
7375
{
74-
if (state.Current.ReturnValue != null)
75-
{
76-
state.Current.JsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, value);
77-
}
78-
else
79-
{
80-
// Primitive arrays being returned without object
81-
state.Current.SetReturnValue(value);
82-
}
76+
// Primitive arrays being returned without object
77+
state.Current.SetReturnValue(value);
8378
}
8479
}
8580
}

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleDictionary.cs

+7-2
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,10 @@ private static void HandleStartDictionary(JsonSerializerOptions options, ref Utf
7070
{
7171
dictionaryClassInfo = options.GetOrAddClass(jsonPropertyInfo.DeclaredPropertyType);
7272
}
73+
7374
state.Current.TempDictionaryValues = (IDictionary)dictionaryClassInfo.CreateConcreteDictionary();
7475
}
75-
// Else if current property is already set (from a constructor, for example), leave as-is.
76-
else if (jsonPropertyInfo.GetValueAsObject(state.Current.ReturnValue) == null)
76+
else
7777
{
7878
// Create the dictionary.
7979
JsonClassInfo dictionaryClassInfo = jsonPropertyInfo.RuntimeClassInfo;
@@ -96,6 +96,11 @@ private static void HandleStartDictionary(JsonSerializerOptions options, ref Utf
9696

9797
private static void HandleEndDictionary(JsonSerializerOptions options, ref Utf8JsonReader reader, ref ReadStack state)
9898
{
99+
if (state.Current.SkipProperty)
100+
{
101+
return;
102+
}
103+
99104
if (state.Current.IsDictionaryProperty)
100105
{
101106
// We added the items to the dictionary already.

‎src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs

+7-2
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ public void EndProperty()
135135
KeyName = null;
136136
}
137137

138-
public static object CreateEnumerableValue(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
138+
public static object CreateEnumerableValue(ref Utf8JsonReader reader, ref ReadStack state)
139139
{
140140
JsonPropertyInfo jsonPropertyInfo = state.Current.JsonPropertyInfo;
141141

@@ -154,10 +154,15 @@ public static object CreateEnumerableValue(ref Utf8JsonReader reader, ref ReadSt
154154

155155
state.Current.TempEnumerableValues = converterList;
156156

157+
// Clear the value if present to ensure we don't confuse tempEnumerableValues with the collection.
158+
if (!jsonPropertyInfo.IsPropertyPolicy)
159+
{
160+
jsonPropertyInfo.SetValueAsObject(state.Current.ReturnValue, null);
161+
}
162+
157163
return null;
158164
}
159165

160-
161166
Type propertyType = jsonPropertyInfo.RuntimePropertyType;
162167
if (typeof(IList).IsAssignableFrom(propertyType))
163168
{

‎src/libraries/System.Text.Json/tests/Serialization/Array.ReadTests.cs

+21-7
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,6 @@ public static void ReadPrimitiveArray()
168168
Assert.Equal(0, i.Length);
169169
}
170170

171-
[ActiveIssue(38435)]
172171
[Fact]
173172
public static void ReadInitializedArrayTest()
174173
{
@@ -198,7 +197,7 @@ public static void ReadPrimitiveArrayFail()
198197
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<List<int?>>(Encoding.UTF8.GetBytes(@"[1,""a""]")));
199198

200199
// Multidimensional arrays currently not supported
201-
Assert.Throws<JsonException>(() => JsonSerializer.Deserialize<int[,]>(Encoding.UTF8.GetBytes(@"[[1,2],[3,4]]")));
200+
Assert.Throws<NotSupportedException>(() => JsonSerializer.Deserialize<int[,]>(Encoding.UTF8.GetBytes(@"[[1,2],[3,4]]")));
202201
}
203202

204203
public static IEnumerable<object[]> ReadNullJson
@@ -412,17 +411,32 @@ public static void ReadClassWithObjectImmutableTypes()
412411
obj.Verify();
413412
}
414413

414+
public class ClassWithPopulatedListAndNoSetter
415+
{
416+
public List<int> MyList { get; } = new List<int>() { 1 };
417+
}
418+
415419
[Fact]
416420
public static void ClassWithNoSetter()
417421
{
418-
string json = @"{""MyList"":[1]}";
419-
ClassWithListButNoSetter obj = JsonSerializer.Deserialize<ClassWithListButNoSetter>(json);
420-
Assert.Equal(1, obj.MyList[0]);
422+
// We don't attempt to deserialize into collections without a setter.
423+
string json = @"{""MyList"":[1,2]}";
424+
ClassWithPopulatedListAndNoSetter obj = JsonSerializer.Deserialize<ClassWithPopulatedListAndNoSetter>(json);
425+
Assert.Equal(1, obj.MyList.Count);
421426
}
422427

423-
public class ClassWithListButNoSetter
428+
public class ClassWithPopulatedListAndSetter
429+
{
430+
public List<int> MyList { get; set; } = new List<int>() { 1 };
431+
}
432+
433+
[Fact]
434+
public static void ClassWithPopulatedList()
424435
{
425-
public List<int> MyList { get; } = new List<int>();
436+
// We don't attempt to deserialize into collections without a setter.
437+
string json = @"{""MyList"":[2,3]}";
438+
ClassWithPopulatedListAndSetter obj = JsonSerializer.Deserialize<ClassWithPopulatedListAndSetter>(json);
439+
Assert.Equal(2, obj.MyList.Count);
426440
}
427441
}
428442
}

0 commit comments

Comments
 (0)