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
14 changes: 13 additions & 1 deletion 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>
</root>
<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="JsonRequiredPropertiesMissing" xml:space="preserve">
<value>Type '{0}' contains required properties which were not set during deserialization: {1}</value>
krwq marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="JsonPropertyRequiredAndIgnoreNullValues" xml:space="preserve">
<value>Required JSON properties cannot be used in conjunction with JsonSerializerOptions.IgnoreNullValues.</value>
</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);
state.Current.InitializeRequiredProperties(jsonTypeInfo);
krwq marked this conversation as resolved.
Show resolved Hide resolved

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

state.Current.ReturnValue = obj;
state.Current.ObjectState = StackFrameObjectState.CreatedObject;
state.Current.InitializeRequiredProperties(jsonTypeInfo);
}
else
{
Expand Down Expand Up @@ -250,6 +252,7 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
}

jsonTypeInfo.OnDeserialized?.Invoke(obj);
state.Current.ValidateAllRequiredPropertiesAreRead(jsonTypeInfo);

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

// if this is required property IgnoreNullTokensOnRead will always be false because we don't allow for both to be true
state.Current.MarkRequiredPropertyAsRead(jsonParameterInfo.MatchingProperty);
}

return success;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ private static bool TryRead<TArg>(
? (TArg?)info.DefaultValue! // Use default value specified on parameter, if any.
: value!;

if (success)
{
state.Current.MarkRequiredPropertyAsRead(jsonParameterInfo.MatchingProperty);
}

return success;
}

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

if (dataExtKey == null)
{
jsonPropertyInfo.SetExtensionDictionaryAsObject(obj, propValue);
Debug.Assert(jsonPropertyInfo.Set != null);

if (propValue is not null || !jsonPropertyInfo.IgnoreNullTokensOnRead || default(T) is not null)
{
jsonPropertyInfo.Set(obj, propValue);

// if this is required property IgnoreNullTokensOnRead will always be false because we don't allow for both to be true
state.Current.MarkRequiredPropertyAsRead(jsonPropertyInfo);
}
}
else
{
Expand All @@ -211,6 +219,7 @@ internal sealed override bool OnTryRead(ref Utf8JsonReader reader, Type typeToCo
}

jsonTypeInfo.OnDeserialized?.Invoke(obj);
state.Current.ValidateAllRequiredPropertiesAreRead(jsonTypeInfo);

// Unbox
Debug.Assert(obj != null);
Expand Down Expand Up @@ -272,6 +281,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 +542,8 @@ private void BeginRead(ref ReadStack state, ref Utf8JsonReader reader, JsonSeria
ThrowHelper.ThrowInvalidOperationException_ConstructorParameterIncompleteBinding(TypeToConvert);
}

state.Current.InitializeRequiredProperties(jsonTypeInfo);

// 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 @@ -139,7 +139,8 @@ internal static void CreateExtensionDataProperty(
}

extensionData = createObjectForExtensionDataProp();
jsonPropertyInfo.SetExtensionDictionaryAsObject(obj, extensionData);
Debug.Assert(jsonPropertyInfo.Set != null);
jsonPropertyInfo.Set(obj, extensionData);
}

// We don't add the value to the dictionary here because we need to support the read-ahead functionality for Streams.
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,24 @@ internal void Configure()
DetermineIgnoreCondition();
DetermineSerializationCapabilities();
}

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

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

if (IgnoreNullTokensOnRead)
{
ThrowHelper.ThrowInvalidOperationException_JsonPropertyRequiredAndIgnoreNullValues();
}
}
}

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

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

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

/// <summary>
Expand Down Expand Up @@ -823,6 +851,27 @@ public JsonNumberHandling? NumberHandling
/// </summary>
internal abstract object? DefaultValue { get; }

/// <summary>
/// Property index on the list of JsonTypeInfo properties.
/// Can be used as a unique identifier for i.e. required properties.
/// It is set just before property is configured and does not change afterward.
/// </summary>
internal int Index
{
get
{
Debug.Assert(_isConfigured);
return _index;
}
set
{
Debug.Assert(!_isConfigured);
_index = value;
}
}

private int _index;

[DebuggerBrowsable(DebuggerBrowsableState.Never)]
private string DebuggerDisplay => $"PropertyType = {PropertyType}, Name = {Name}, DeclaringType = {DeclaringType}";
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref
}

success = true;
state.Current.MarkRequiredPropertyAsRead(this);
}
else if (TypedEffectiveConverter.CanUseDirectReadOrWrite && state.Current.NumberHandling == null)
{
Expand All @@ -356,6 +357,7 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref
}

success = true;
state.Current.MarkRequiredPropertyAsRead(this);
}
else
{
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,6 @@ internal override bool ReadJsonAsObject(ref ReadStack state, ref Utf8JsonReader
return success;
}

internal override void SetExtensionDictionaryAsObject(object obj, object? extensionDict)
{
Debug.Assert(HasSetter);
T typedValue = (T)extensionDict!;
Set!(obj, typedValue);
}

private protected override void ConfigureIgnoreCondition(JsonIgnoreCondition? ignoreCondition)
{
switch (ignoreCondition)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ public abstract partial class JsonTypeInfo

private JsonPropertyInfoList? _properties;

/// <summary>
/// Indices of required properties.
/// </summary>
internal int[]? RequiredPropertiesIndices { get; private set; }

private Action<object>? _onSerializing;
private Action<object>? _onSerialized;
private Action<object>? _onDeserializing;
Expand Down Expand Up @@ -878,13 +883,38 @@ internal void InitializePropertyCache()
ExtensionDataProperty.EnsureConfigured();
}

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

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

jsonPropertyInfo.Index = propertyIndex++;
jsonPropertyInfo.EnsureChildOf(this);
jsonPropertyInfo.EnsureConfigured();
}

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

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

RequiredPropertiesIndices = requiredPropertiesIndices;
}
}

internal void InitializeConstructorParameters(JsonParameterInfoValues[] jsonParameters, bool sourceGenMode = false)
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
Loading