Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Replace collection instances on deserialize #39442

Merged
merged 1 commit into from
Jul 15, 2019
Merged

Replace collection instances on deserialize #39442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 52 additions & 64 deletions src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -24,7 +22,6 @@ internal abstract class JsonPropertyInfo
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;
Expand Down Expand Up @@ -141,82 +138,73 @@ private void DetermineSerializationCapabilities()
{
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;
}
}
}

Expand Down
6 changes: 0 additions & 6 deletions src/System.Text.Json/src/System/Text/Json/Serialization/JsonPropertyInfoCommon.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Diagnostics;
using System.Reflection;
using System.Text.Json.Serialization;
using System.Text.Json.Serialization.Converters;

namespace System.Text.Json
{
Expand Down Expand Up @@ -72,11 +71,6 @@ public override JsonConverter ConverterBase
}
}

public override void GetPolicies()
{
base.GetPolicies();
}

public override object GetValueAsObject(object obj)
{
if (IsPropertyPolicy)
Expand Down
37 changes: 16 additions & 21 deletions src/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleArray.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ private static void HandleStartArray(
ref Utf8JsonReader reader,
ref ReadStack state)
{
JsonPropertyInfo jsonPropertyInfo = state.Current.JsonPropertyInfo;

if (state.Current.SkipProperty)
{
// The array is not being applied to the object.
Expand All @@ -26,6 +24,7 @@ private static void HandleStartArray(
return;
}

JsonPropertyInfo jsonPropertyInfo = state.Current.JsonPropertyInfo;
if (jsonPropertyInfo == null)
{
jsonPropertyInfo = state.Current.JsonClassInfo.CreateRootObject(options);
Expand All @@ -35,9 +34,9 @@ private static void HandleStartArray(
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);
}
Expand All @@ -54,32 +53,28 @@ private static void HandleStartArray(
}

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);
}
}
}
Expand Down
9 changes: 7 additions & 2 deletions ...stem.Text.Json/src/System/Text/Json/Serialization/JsonSerializer.Read.HandleDictionary.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ private static void HandleStartDictionary(JsonSerializerOptions options, ref Utf
{
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;
Expand All @@ -96,6 +96,11 @@ private static void HandleStartDictionary(JsonSerializerOptions options, ref Utf

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.
Expand Down
9 changes: 7 additions & 2 deletions src/System.Text.Json/src/System/Text/Json/Serialization/ReadStackFrame.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public void EndProperty()
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;

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

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))
{
Expand Down
28 changes: 21 additions & 7 deletions src/System.Text.Json/tests/Serialization/Array.ReadTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ public static void ReadPrimitiveArray()
Assert.Equal(0, i.Length);
}

[ActiveIssue(38435)]
[Fact]
public static void ReadInitializedArrayTest()
{
Expand Down Expand Up @@ -198,7 +197,7 @@ public static void ReadPrimitiveArrayFail()
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
Expand Down Expand Up @@ -412,17 +411,32 @@ public static void ReadClassWithObjectImmutableTypes()
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be something like "we replace the contents of this collection"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I will change in future PR to avoid CI reset.

string json = @"{""MyList"":[2,3]}";
ClassWithPopulatedListAndSetter obj = JsonSerializer.Deserialize<ClassWithPopulatedListAndSetter>(json);
Assert.Equal(2, obj.MyList.Count);
}
}
}
Loading