Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/8.0] Throw when applying JsonObjectHandling.Populate to types with parameterized constructors. #92947

Merged
merged 3 commits into from
Oct 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading