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
11 changes: 10 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,13 @@
<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>
</root>
34 changes: 34 additions & 0 deletions src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;
krwq marked this conversation as resolved.
Show resolved Hide resolved
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.ExceptionServices;
Expand Down Expand Up @@ -42,6 +43,39 @@ public static bool IsInSubtypeRelationshipWith(this Type type, Type other) =>
private static bool HasJsonConstructorAttribute(ConstructorInfo constructorInfo)
=> constructorInfo.GetCustomAttribute<JsonConstructorAttribute>() != null;

public static bool HasRequiredMemberAttribute(this ICustomAttributeProvider memberInfo)
{
#if NET7_0_OR_GREATER
return memberInfo.IsDefined(typeof(RequiredMemberAttribute), inherit: true);
krwq marked this conversation as resolved.
Show resolved Hide resolved
#else
return memberInfo.HasCustomAttributeWithName("System.Runtime.CompilerServices.RequiredMemberAttribute");
#endif
}

public static bool HasSetsRequiredMembersAttribute(this ICustomAttributeProvider memberInfo)
{
#if NET7_0_OR_GREATER
return memberInfo.IsDefined(typeof(SetsRequiredMembersAttribute), inherit: true);
#else
return memberInfo.HasCustomAttributeWithName("System.Diagnostics.CodeAnalysis.SetsRequiredMembersAttribute");
#endif
}

#if !NET7_0_OR_GREATER
private static bool HasCustomAttributeWithName(this ICustomAttributeProvider memberInfo, string name)
krwq marked this conversation as resolved.
Show resolved Hide resolved
{
foreach (object attribute in memberInfo.GetCustomAttributes(inherit: true))
krwq marked this conversation as resolved.
Show resolved Hide resolved
{
if (attribute.GetType().FullName == name)
{
return true;
}
}

return false;
}
#endif

public static TAttribute? GetUniqueCustomAttribute<TAttribute>(this MemberInfo memberInfo, bool inherit)
where TAttribute : Attribute
{
Expand Down
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.InitializeRequiredPropertiesValidationState(jsonTypeInfo);

// 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.InitializeRequiredPropertiesValidationState(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.InitializeRequiredPropertiesValidationState(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,21 @@ internal void Configure()
DetermineIgnoreCondition();
DetermineSerializationCapabilities();
}

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

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

Debug.Assert(!IgnoreNullTokensOnRead);
}
}

private protected abstract void DetermineEffectiveConverter(JsonTypeInfo jsonTypeInfo);
Expand Down Expand Up @@ -341,7 +368,7 @@ private void DetermineIgnoreCondition()
Debug.Assert(Options.DefaultIgnoreCondition == JsonIgnoreCondition.Never);
if (PropertyTypeCanBeNull)
{
IgnoreNullTokensOnRead = !_isUserSpecifiedSetter;
IgnoreNullTokensOnRead = !_isUserSpecifiedSetter && !IsRequired;
IgnoreDefaultValuesOnWrite = ShouldSerialize is null;
}
}
Expand Down Expand Up @@ -760,8 +787,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 +848,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
Loading