Skip to content

Commit

Permalink
Throw NotSupportedException when applying JsonObjectHandling.Populate…
Browse files Browse the repository at this point in the history
… on types with parameterized constructors. (#92947)

Co-authored-by: Eirik Tsarpalis <[email protected]>
  • Loading branch information
github-actions[bot] and eiriktsarpalis authored Oct 4, 2023
1 parent a84f8ff commit 1954c37
Show file tree
Hide file tree
Showing 11 changed files with 125 additions and 33 deletions.
3 changes: 3 additions & 0 deletions src/libraries/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,9 @@
<data name="ObjectCreationHandlingPropertyCannotAllowReferenceHandling" xml:space="preserve">
<value>JsonObjectCreationHandling.Populate is incompatible with reference handling.</value>
</data>
<data name="ObjectCreationHandlingPropertyDoesNotSupportParameterizedConstructors" xml:space="preserve">
<value>JsonObjectCreationHandling.Populate is currently not supported in types with parameterized constructors.</value>
</data>
<data name="FormatInt128" xml:space="preserve">
<value>Either the JSON value is not in a supported format, or is out of bounds for an Int128.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ internal sealed override bool OnTryRead(ref Utf8JsonReader reader, Type typeToCo
{
JsonTypeInfo jsonTypeInfo = state.Current.JsonTypeInfo;

if (jsonTypeInfo.CreateObject != null || state.Current.IsPopulating)
if (!jsonTypeInfo.UsesParameterizedConstructor || state.Current.IsPopulating)
{
// Fall back to default object converter in following cases:
// - if user has set a default constructor delegate with contract customization
// - if user configuration has invalidated the parameterized constructor
// - we're continuing populating an object.
return base.OnTryRead(ref reader, typeToConvert, options, ref state, out value);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,20 @@ internal static MemberAccessor MemberAccessor
private static JsonTypeInfo CreateTypeInfoCore(Type type, JsonConverter converter, JsonSerializerOptions options)
{
JsonTypeInfo typeInfo = JsonTypeInfo.CreateJsonTypeInfo(type, converter, options);
typeInfo.NumberHandling = GetNumberHandlingForType(typeInfo.Type);
typeInfo.PreferredPropertyObjectCreationHandling = GetObjectCreationHandlingForType(typeInfo.Type);

if (typeInfo.Kind == JsonTypeInfoKind.Object)
if (GetNumberHandlingForType(typeInfo.Type) is { } numberHandling)
{
typeInfo.UnmappedMemberHandling = GetUnmappedMemberHandling(typeInfo.Type);
typeInfo.NumberHandling = numberHandling;
}

if (GetObjectCreationHandlingForType(typeInfo.Type) is { } creationHandling)
{
typeInfo.PreferredPropertyObjectCreationHandling = creationHandling;
}

if (GetUnmappedMemberHandling(typeInfo.Type) is { } unmappedMemberHandling)
{
typeInfo.UnmappedMemberHandling = unmappedMemberHandling;
}

typeInfo.PopulatePolymorphismMetadata();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -495,9 +495,17 @@ private void DetermineEffectiveObjectCreationHandlingForProperty()
Debug.Assert(ParentTypeInfo != null, "We should have ensured parent is assigned in JsonTypeInfo");
Debug.Assert(!IsConfigured, "Should not be called post-configuration.");

JsonObjectCreationHandling effectiveObjectCreationHandling = JsonObjectCreationHandling.Replace;
if (ObjectCreationHandling == null)
{
JsonObjectCreationHandling preferredCreationHandling = ParentTypeInfo.PreferredPropertyObjectCreationHandling ?? Options.PreferredObjectCreationHandling;
// Consult type-level configuration, then global configuration.
// Ignore global configuration if we're using a parameterized constructor.
JsonObjectCreationHandling preferredCreationHandling =
ParentTypeInfo.PreferredPropertyObjectCreationHandling
?? (ParentTypeInfo.DetermineUsesParameterizedConstructor()
? JsonObjectCreationHandling.Replace
: Options.PreferredObjectCreationHandling);

bool canPopulate =
preferredCreationHandling == JsonObjectCreationHandling.Populate &&
EffectiveConverter.CanPopulate &&
Expand All @@ -506,7 +514,7 @@ private void DetermineEffectiveObjectCreationHandlingForProperty()
!ParentTypeInfo.SupportsPolymorphicDeserialization &&
!(Set == null && IgnoreReadOnlyMember);

EffectiveObjectCreationHandling = canPopulate ? JsonObjectCreationHandling.Populate : JsonObjectCreationHandling.Replace;
effectiveObjectCreationHandling = canPopulate ? JsonObjectCreationHandling.Populate : JsonObjectCreationHandling.Replace;
}
else if (ObjectCreationHandling == JsonObjectCreationHandling.Populate)
{
Expand Down Expand Up @@ -537,18 +545,24 @@ private void DetermineEffectiveObjectCreationHandlingForProperty()
ThrowHelper.ThrowInvalidOperationException_ObjectCreationHandlingPropertyCannotAllowReadOnlyMember(this);
}

EffectiveObjectCreationHandling = JsonObjectCreationHandling.Populate;
}
else
{
Debug.Assert(EffectiveObjectCreationHandling == JsonObjectCreationHandling.Replace);
effectiveObjectCreationHandling = JsonObjectCreationHandling.Populate;
}

if (EffectiveObjectCreationHandling == JsonObjectCreationHandling.Populate &&
Options.ReferenceHandlingStrategy != ReferenceHandlingStrategy.None)
if (effectiveObjectCreationHandling is JsonObjectCreationHandling.Populate)
{
ThrowHelper.ThrowInvalidOperationException_ObjectCreationHandlingPropertyCannotAllowReferenceHandling();
if (ParentTypeInfo.DetermineUsesParameterizedConstructor())
{
ThrowHelper.ThrowNotSupportedException_ObjectCreationHandlingPropertyDoesNotSupportParameterizedConstructors();
}

if (Options.ReferenceHandlingStrategy != ReferenceHandlingStrategy.None)
{
ThrowHelper.ThrowInvalidOperationException_ObjectCreationHandlingPropertyCannotAllowReferenceHandling();
}
}

// Validation complete, commit configuration.
EffectiveObjectCreationHandling = effectiveObjectCreationHandling;
}

private bool NumberHandingIsApplicable()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ public abstract partial class JsonTypeInfo
// All of the serializable parameters on a POCO constructor keyed on parameter name.
// Only parameters which bind to properties are cached.
internal JsonPropertyDictionary<JsonParameterInfo>? ParameterCache { get; private set; }
internal bool UsesParameterizedConstructor
{
get
{
Debug.Assert(IsConfigured);
return ParameterCache != null;
}
}

// All of the serializable properties on a POCO (except the optional extension property) keyed on property name.
internal JsonPropertyDictionary<JsonPropertyInfo>? PropertyCache { get; private set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -552,17 +552,14 @@ public JsonObjectCreationHandling? PreferredPropertyObjectCreationHandling
{
VerifyMutable();

if (value is not null)
if (Kind != JsonTypeInfoKind.Object)
{
if (Kind != JsonTypeInfoKind.Object)
{
ThrowHelper.ThrowInvalidOperationException_JsonTypeInfoOperationNotPossibleForKind(Kind);
}
ThrowHelper.ThrowInvalidOperationException_JsonTypeInfoOperationNotPossibleForKind(Kind);
}

if (!JsonSerializer.IsValidCreationHandlingValue(value.Value))
{
throw new ArgumentOutOfRangeException(nameof(value));
}
if (value is not null && !JsonSerializer.IsValidCreationHandlingValue(value.Value))
{
throw new ArgumentOutOfRangeException(nameof(value));
}

_preferredPropertyObjectCreationHandling = value;
Expand Down Expand Up @@ -684,7 +681,7 @@ private void Configure()
{
ConfigureProperties();

if (Converter.ConstructorIsParameterized)
if (DetermineUsesParameterizedConstructor())
{
ConfigureConstructorParameters();
}
Expand Down Expand Up @@ -808,6 +805,12 @@ bool IsCurrentNodeCompatible()
/// </summary>
private bool IsCompatibleWithCurrentOptions { get; set; } = true;

/// <summary>
/// Determine if the current configuration is compatible with using a parameterized constructor.
/// </summary>
internal bool DetermineUsesParameterizedConstructor()
=> Converter.ConstructorIsParameterized && CreateObject is null;

#if DEBUG
internal string GetPropertyDebugInfo(ReadOnlySpan<byte> unescapedPropertyName)
{
Expand Down Expand Up @@ -1107,7 +1110,7 @@ internal void ConfigureProperties()
internal void ConfigureConstructorParameters()
{
Debug.Assert(Kind == JsonTypeInfoKind.Object);
Debug.Assert(Converter.ConstructorIsParameterized);
Debug.Assert(DetermineUsesParameterizedConstructor());
Debug.Assert(PropertyCache is not null);
Debug.Assert(ParameterCache is null);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,20 +386,20 @@ public JsonTypeInfo GetTopJsonTypeInfoWithParameterizedConstructor()

for (int i = 0; i < _count - 1; i++)
{
if (_stack[i].JsonTypeInfo.Converter.ConstructorIsParameterized)
if (_stack[i].JsonTypeInfo.UsesParameterizedConstructor)
{
return _stack[i].JsonTypeInfo;
}
}

Debug.Assert(Current.JsonTypeInfo.Converter.ConstructorIsParameterized);
Debug.Assert(Current.JsonTypeInfo.UsesParameterizedConstructor);
return Current.JsonTypeInfo;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void SetConstructorArgumentState()
{
if (Current.JsonTypeInfo.Converter.ConstructorIsParameterized)
if (Current.JsonTypeInfo.UsesParameterizedConstructor)
{
Current.CtorArgumentState ??= new();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,12 @@ public static void ThrowInvalidOperationException_ObjectCreationHandlingProperty
throw new InvalidOperationException(SR.ObjectCreationHandlingPropertyCannotAllowReferenceHandling);
}

[DoesNotReturn]
public static void ThrowNotSupportedException_ObjectCreationHandlingPropertyDoesNotSupportParameterizedConstructors()
{
throw new NotSupportedException(SR.ObjectCreationHandlingPropertyDoesNotSupportParameterizedConstructors);
}

[DoesNotReturn]
public static void ThrowJsonException_SerializationConverterRead(JsonConverter? converter)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1165,4 +1165,52 @@ public class ClassWithInvalidPropertyAnnotation
[JsonObjectCreationHandling((JsonObjectCreationHandling)(-1))]
public List<int> Property { get; }
}

[Theory]
[InlineData(typeof(ClassWithParameterizedConstructorWithPopulateProperty))]
[InlineData(typeof(ClassWithParameterizedConstructorWithPopulateType))]
public async Task ClassWithParameterizedCtor_UsingPopulateConfiguration_ThrowsNotSupportedException(Type type)
{
object instance = Activator.CreateInstance(type, "Jim");
string json = """{"Username":"Jim","PhoneNumbers":["123456"]}""";

await Assert.ThrowsAsync<NotSupportedException>(() => Serializer.SerializeWrapper(instance, type));
await Assert.ThrowsAsync<NotSupportedException>(() => Serializer.DeserializeWrapper(json, type));
Assert.Throws<NotSupportedException>(() => Serializer.GetTypeInfo(type));
}

public class ClassWithParameterizedConstructorWithPopulateProperty(string name)
{
public string Name { get; } = name;

[JsonObjectCreationHandling(JsonObjectCreationHandling.Populate)]
public List<string> PhoneNumbers { get; } = new();
}

[JsonObjectCreationHandling(JsonObjectCreationHandling.Populate)]
public class ClassWithParameterizedConstructorWithPopulateType(string name)
{
public string Name { get; } = name;

public List<string> PhoneNumbers { get; } = new();
}

[Fact]
public async Task ClassWithParameterizedCtor_NoPopulateConfiguration_WorksWithGlobalPopulateConfiguration()
{
string json = """{"Username":"Jim","PhoneNumbers":["123456"]}""";

JsonSerializerOptions options = Serializer.CreateOptions(makeReadOnly: false);
options.PreferredObjectCreationHandling = JsonObjectCreationHandling.Populate;

ClassWithParameterizedConstructorNoPopulate result = await Serializer.DeserializeWrapper<ClassWithParameterizedConstructorNoPopulate>(json, options);
Assert.Empty(result.PhoneNumbers);
}

public class ClassWithParameterizedConstructorNoPopulate(string name)
{
public string Name { get; } = name;

public List<string> PhoneNumbers { get; } = new();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,9 @@ public sealed class JsonCreationHandlingTests_AsyncStreamWithSmallBuffer()
[JsonSerializable(typeof(SimpleClassWitNonPopulatableProperty))]
[JsonSerializable(typeof(ClassWithInvalidTypeAnnotation))]
[JsonSerializable(typeof(ClassWithInvalidPropertyAnnotation))]
[JsonSerializable(typeof(ClassWithParameterizedConstructorWithPopulateProperty))]
[JsonSerializable(typeof(ClassWithParameterizedConstructorWithPopulateType))]
[JsonSerializable(typeof(ClassWithParameterizedConstructorNoPopulate))]
internal partial class CreationHandlingTestContext : JsonSerializerContext
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1440,11 +1440,10 @@ public static void PreferredPropertyObjectCreationHandling_NonObjectKind_ThrowsI
{
JsonTypeInfo jsonTypeInfo = JsonTypeInfo.CreateJsonTypeInfo(type, new());

// Invalid kinds default to null and can be set to null.
Assert.Null(jsonTypeInfo.PreferredPropertyObjectCreationHandling);
jsonTypeInfo.PreferredPropertyObjectCreationHandling = null;
// Invalid kinds default to null.
Assert.Null(jsonTypeInfo.PreferredPropertyObjectCreationHandling);

Assert.Throws<InvalidOperationException>(() => jsonTypeInfo.PreferredPropertyObjectCreationHandling = null);
Assert.Throws<InvalidOperationException>(() => jsonTypeInfo.PreferredPropertyObjectCreationHandling = JsonObjectCreationHandling.Populate);
Assert.Throws<InvalidOperationException>(() => jsonTypeInfo.PreferredPropertyObjectCreationHandling = JsonObjectCreationHandling.Replace);
Assert.Null(jsonTypeInfo.PreferredPropertyObjectCreationHandling);
Expand Down

0 comments on commit 1954c37

Please sign in to comment.