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

Make JSON support required properties #72937

Merged
merged 11 commits into from
Jul 29, 2022
12 changes: 12 additions & 0 deletions src/libraries/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -659,4 +659,16 @@
<data name="JsonPolymorphismOptionsAssociatedWithDifferentJsonTypeInfo" xml:space="preserve">
<value>Parameter already associated with a different JsonTypeInfo instance.</value>
</data>
<data name="JsonPropertyRequiredAndNotDeserializable" xml:space="preserve">
<value>JsonPropertyInfo with name '{0}' for type '{1}' is required but is not deserializable.</value>
krwq marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="JsonPropertyRequiredAndExtensionData" xml:space="preserve">
<value>JsonPropertyInfo with name '{0}' for type '{1}' cannot be required and be an extension data property.</value>
krwq marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="JsonRequiredPropertiesMissingHeader" xml:space="preserve">
<value>Type '{0}' contains required properties which were not set during deserialization:</value>
</data>
<data name="JsonRequiredPropertiesMissingItem" xml:space="preserve">
<value>- {0}</value>
krwq marked this conversation as resolved.
Show resolved Hide resolved
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
obj = jsonTypeInfo.CreateObject()!;

jsonTypeInfo.OnDeserializing?.Invoke(obj);
jsonTypeInfo.InitializeRequiredProperties(ref state);

// Process all properties.
while (true)
Expand Down Expand Up @@ -143,6 +144,8 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,

state.Current.ReturnValue = obj;
state.Current.ObjectState = StackFrameObjectState.CreatedObject;

jsonTypeInfo.InitializeRequiredProperties(ref state);
krwq marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
Expand Down Expand Up @@ -250,6 +253,7 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
}

jsonTypeInfo.OnDeserialized?.Invoke(obj);
jsonTypeInfo.CheckRequiredProperties(ref state);
krwq marked this conversation as resolved.
Show resolved Hide resolved

// Unbox
Debug.Assert(obj != null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ protected sealed override bool ReadAndCacheConstructorArgument(ref ReadStack sta
if (success && !(arg == null && jsonParameterInfo.IgnoreNullTokensOnRead))
{
((object[])state.Current.CtorArgumentState!.Arguments)[jsonParameterInfo.ClrInfo.Position] = arg!;
state.Current.MarkRequiredPropertyAsRead(jsonParameterInfo.MatchingProperty);
}

return success;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,15 @@ private static bool TryRead<TArg>(

bool success = converter.TryRead(ref reader, info.PropertyType, info.Options!, ref state, out TArg? value);

arg = value == null && jsonParameterInfo.IgnoreNullTokensOnRead
? (TArg?)info.DefaultValue! // Use default value specified on parameter, if any.
: value!;
if (value == null && jsonParameterInfo.IgnoreNullTokensOnRead)
krwq marked this conversation as resolved.
Show resolved Hide resolved
{
arg = (TArg?)info.DefaultValue!; // Use default value specified on parameter, if any.
}
else
{
arg = value!;
state.Current.MarkRequiredPropertyAsRead(jsonParameterInfo.MatchingProperty);
}

return success;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ internal sealed override bool OnTryRead(ref Utf8JsonReader reader, Type typeToCo

if (dataExtKey == null)
{
jsonPropertyInfo.SetExtensionDictionaryAsObject(obj, propValue);
jsonPropertyInfo.SetValueAsObject(obj, propValue, ref state);
krwq marked this conversation as resolved.
Show resolved Hide resolved
}
else
{
Expand All @@ -211,6 +211,7 @@ internal sealed override bool OnTryRead(ref Utf8JsonReader reader, Type typeToCo
}

jsonTypeInfo.OnDeserialized?.Invoke(obj);
jsonTypeInfo.CheckRequiredProperties(ref state);

// Unbox
Debug.Assert(obj != null);
Expand Down Expand Up @@ -272,6 +273,7 @@ private void ReadConstructorArguments(ref ReadStack state, ref Utf8JsonReader re
continue;
}

Debug.Assert(jsonParameterInfo.MatchingProperty != null);
ReadAndCacheConstructorArgument(ref state, ref reader, jsonParameterInfo);

state.Current.EndConstructorParameter();
Expand Down Expand Up @@ -532,6 +534,8 @@ private void BeginRead(ref ReadStack state, ref Utf8JsonReader reader, JsonSeria
ThrowHelper.ThrowInvalidOperationException_ConstructorParameterIncompleteBinding(TypeToConvert);
}

jsonTypeInfo.InitializeRequiredProperties(ref state);

// Set current JsonPropertyInfo to null to avoid conflicts on push.
state.Current.JsonPropertyInfo = null;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,11 @@ public JsonTypeInfo JsonTypeInfo

public bool ShouldDeserialize { get; private set; }

public JsonPropertyInfo MatchingProperty { get; private set; } = null!;

public virtual void Initialize(JsonParameterInfoValues parameterInfo, JsonPropertyInfo matchingProperty, JsonSerializerOptions options)
{
MatchingProperty = matchingProperty;
ClrInfo = parameterInfo;
Options = options;
ShouldDeserialize = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,18 @@ public bool IsExtensionData

private bool _isExtensionDataProperty;

internal bool IsRequired
{
get => _isRequired;
set
{
VerifyMutable();
_isRequired = value;
}
}

private bool _isRequired;

internal JsonPropertyInfo(Type declaringType, Type propertyType, JsonTypeInfo? declaringTypeInfo, JsonSerializerOptions options)
{
Debug.Assert(declaringTypeInfo is null || declaringTypeInfo.Type == declaringType);
Expand Down Expand Up @@ -279,6 +291,19 @@ internal void Configure()
DetermineIgnoreCondition();
DetermineSerializationCapabilities();
}

if (IsRequired)
{
if (!CanDeserialize)
{
ThrowHelper.ThrowInvalidOperationException_JsonPropertyRequiredAndNotDeserializable(this);
}

if (IsExtensionData)
{
ThrowHelper.ThrowInvalidOperationException_JsonPropertyRequiredAndExtensionData(this);
}
}
}

private protected abstract void DetermineEffectiveConverter(JsonTypeInfo jsonTypeInfo);
Expand Down Expand Up @@ -762,6 +787,8 @@ internal JsonTypeInfo JsonTypeInfo

internal abstract void SetExtensionDictionaryAsObject(object obj, object? extensionDict);

internal abstract void SetValueAsObject(object obj, object? value, ref ReadStack state);

internal bool IsIgnored => _ignoreCondition == JsonIgnoreCondition.Always;

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref
{
T? value = default;
Set!(obj, value!);
state.Current.MarkRequiredPropertyAsRead(this);
}

success = true;
Expand All @@ -353,6 +354,7 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref
// Optimize for internal converters by avoiding the extra call to TryRead.
T? fastValue = TypedEffectiveConverter.Read(ref reader, PropertyType, Options);
Set!(obj, fastValue!);
state.Current.MarkRequiredPropertyAsRead(this);
}

success = true;
Expand All @@ -366,6 +368,7 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref
if (success)
{
Set!(obj, value!);
state.Current.MarkRequiredPropertyAsRead(this);
}
}
}
Expand Down Expand Up @@ -408,13 +411,25 @@ internal override bool ReadJsonAsObject(ref ReadStack state, ref Utf8JsonReader
return success;
}

internal override void SetExtensionDictionaryAsObject(object obj, object? extensionDict)
internal sealed override void SetExtensionDictionaryAsObject(object obj, object? extensionDict)
krwq marked this conversation as resolved.
Show resolved Hide resolved
{
Debug.Assert(HasSetter);
T typedValue = (T)extensionDict!;
Set!(obj, typedValue);
}

internal sealed override void SetValueAsObject(object obj, object? value, ref ReadStack state)
krwq marked this conversation as resolved.
Show resolved Hide resolved
{
Debug.Assert(HasSetter);

if (value is not null || !IgnoreNullTokensOnRead || default(T) is not null)
{
T typedValue = (T)value!;
Set!(obj, typedValue);
state.Current.MarkRequiredPropertyAsRead(this);
}
}

private protected override void ConfigureIgnoreCondition(JsonIgnoreCondition? ignoreCondition)
{
switch (ignoreCondition)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public abstract partial class JsonTypeInfo
internal delegate T ParameterizedConstructorDelegate<T, TArg0, TArg1, TArg2, TArg3>(TArg0 arg0, TArg1 arg1, TArg2 arg2, TArg3 arg3);

private JsonPropertyInfoList? _properties;
internal JsonPropertyInfo[]? RequiredProperties { get; private set; }

private Action<object>? _onSerializing;
private Action<object>? _onSerialized;
Expand Down Expand Up @@ -269,16 +270,17 @@ public JsonPolymorphismOptions? PolymorphismOptions
// Avoids having to perform an expensive cast to JsonTypeInfo<T> to check if there is a Serialize method.
internal bool HasSerializeHandler { get; private protected set; }

// Configure would normally have thrown why initializing properties for source gen but type had SerializeHandler
// Exception used to throw on deserialization. In some scenarios i.e.:
// configure would have thrown while initializing properties for source gen but type had SerializeHandler
// so it is allowed to be used for fast-path serialization but it will throw if used for metadata-based serialization
internal bool MetadataSerializationNotSupported { get; private protected set; }
internal Exception? DeserializationException { get; private protected set; }
krwq marked this conversation as resolved.
Show resolved Hide resolved

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void ValidateCanBeUsedForMetadataSerialization()
{
if (MetadataSerializationNotSupported)
if (DeserializationException != null)
{
ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeProperties(Options.TypeInfoResolver, Type);
throw DeserializationException;
krwq marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -878,13 +880,36 @@ internal void InitializePropertyCache()
ExtensionDataProperty.EnsureConfigured();
}

int numberOfRequiredProperties = 0;
foreach (KeyValuePair<string, JsonPropertyInfo> jsonPropertyInfoKv in PropertyCache.List)
{
JsonPropertyInfo jsonPropertyInfo = jsonPropertyInfoKv.Value;

if (jsonPropertyInfo.IsRequired)
{
numberOfRequiredProperties++;
}

jsonPropertyInfo.EnsureChildOf(this);
jsonPropertyInfo.EnsureConfigured();
}

if (numberOfRequiredProperties > 0)
krwq marked this conversation as resolved.
Show resolved Hide resolved
{
JsonPropertyInfo[] requiredProperties = new JsonPropertyInfo[numberOfRequiredProperties];
int idx = 0;
foreach (KeyValuePair<string, JsonPropertyInfo> jsonPropertyInfoKv in PropertyCache.List)
{
JsonPropertyInfo jsonPropertyInfo = jsonPropertyInfoKv.Value;

if (jsonPropertyInfo.IsRequired)
{
requiredProperties[idx++] = jsonPropertyInfo;
}
}

RequiredProperties = requiredProperties;
}
}

internal void InitializeConstructorParameters(JsonParameterInfoValues[] jsonParameters, bool sourceGenMode = false)
Expand Down Expand Up @@ -1002,6 +1027,30 @@ internal JsonPropertyDictionary<JsonPropertyInfo> CreatePropertyCache(int capaci
return new JsonPropertyDictionary<JsonPropertyInfo>(Options.PropertyNameCaseInsensitive, capacity);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void InitializeRequiredProperties(ref ReadStack state)
{
if (RequiredProperties != null)
{
Debug.Assert(state.Current.RequiredPropertiesLeft == null);
state.Current.RequiredPropertiesLeft = new HashSet<JsonPropertyInfo>(RequiredProperties);
krwq marked this conversation as resolved.
Show resolved Hide resolved
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void CheckRequiredProperties(ref ReadStack state)
{
if (RequiredProperties != null)
{
Debug.Assert(state.Current.RequiredPropertiesLeft != null);

if (state.Current.RequiredPropertiesLeft.Count != 0)
{
ThrowHelper.ThrowJsonException_JsonRequiredPropertyMissing(this, state.Current.RequiredPropertiesLeft);
}
}
}

private static JsonParameterInfo CreateConstructorParameter(
JsonParameterInfoValues parameterInfo,
JsonPropertyInfo jsonPropertyInfo,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Text.Json.Reflection;

namespace System.Text.Json.Serialization.Metadata
Expand All @@ -26,6 +27,38 @@ internal ReflectionJsonTypeInfo(JsonConverter converter, JsonSerializerOptions o
if (PropertyInfoForTypeInfo.ConverterStrategy == ConverterStrategy.Object)
{
AddPropertiesAndParametersUsingReflection();

#if NET7_0_OR_GREATER
// Compiler adds RequiredMemberAttribute to type if any of the members is marked with 'required' keyword.
// SetsRequiredMembersAttribute means that all required members are assigned by constructor and therefore there is no enforcement
bool shouldCheckMembersForRequiredMemberAttribute =
krwq marked this conversation as resolved.
Show resolved Hide resolved
typeof(T).IsDefined(typeof(RequiredMemberAttribute), inherit: true)
&& !(converter.ConstructorInfo?.IsDefined(typeof(SetsRequiredMembersAttribute), inherit: true) ?? false);

if (shouldCheckMembersForRequiredMemberAttribute)
{
Debug.Assert(PropertyCache != null);
foreach (var (_, property) in PropertyCache.List)
{
Debug.Assert(property.AttributeProvider != null);
if (property.AttributeProvider.IsDefined(typeof(RequiredMemberAttribute), inherit: true))
{
property.IsRequired = true;
}
}

if (ExtensionDataProperty != null)
{
Debug.Assert(ExtensionDataProperty.AttributeProvider != null);
if (ExtensionDataProperty.AttributeProvider.IsDefined(typeof(RequiredMemberAttribute), inherit: true))
{
// This will end up in error in Configure
// but we need to give contract resolver a chance to fix this
ExtensionDataProperty.IsRequired = true;
}
}
}
#endif
}

Func<object>? createObject = Options.MemberAccessorStrategy.CreateConstructor(typeof(T));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ internal override JsonParameterInfoValues[] GetParameterInfoValues()
}

array = Array.Empty<JsonParameterInfoValues>();
MetadataSerializationNotSupported = true;
DeserializationException = ThrowHelper.GetInvalidOperationException_NoMetadataForTypeProperties(Options.TypeInfoResolver, Type);
}

return array;
Expand Down Expand Up @@ -149,7 +149,7 @@ internal override void LateAddProperties()
ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeProperties(Options.TypeInfoResolver, Type);
}

MetadataSerializationNotSupported = true;
DeserializationException = ThrowHelper.GetInvalidOperationException_NoMetadataForTypeProperties(Options.TypeInfoResolver, Type);
return;
}

Expand Down
Loading