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
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public static bool HasRequiredMemberAttribute(this ICustomAttributeProvider memb
#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");
return memberInfo.HasCustomAttributeWithName("System.Runtime.CompilerServices.RequiredMemberAttribute", inherit: true);
#endif
}

Expand All @@ -57,16 +57,16 @@ public static bool HasSetsRequiredMembersAttribute(this ICustomAttributeProvider
#if NET7_0_OR_GREATER
return memberInfo.IsDefined(typeof(SetsRequiredMembersAttribute), inherit: true);
#else
return memberInfo.HasCustomAttributeWithName("System.Diagnostics.CodeAnalysis.SetsRequiredMembersAttribute");
return memberInfo.HasCustomAttributeWithName("System.Diagnostics.CodeAnalysis.SetsRequiredMembersAttribute", inherit: true);
#endif
}

#if !NET7_0_OR_GREATER
private static bool HasCustomAttributeWithName(this ICustomAttributeProvider memberInfo, string name)
private static bool HasCustomAttributeWithName(this ICustomAttributeProvider memberInfo, string fullName, bool inherit)
{
foreach (object attribute in memberInfo.GetCustomAttributes(inherit: true))
foreach (object attribute in memberInfo.GetCustomAttributes(inherit))
{
if (attribute.GetType().FullName == name)
if (attribute.GetType().FullName == fullName)
{
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,14 @@ private bool NumberHandingIsApplicable()
potentialNumberType == JsonTypeInfo.ObjectType;
}

private void DetermineIsRequired(MemberInfo memberInfo, bool shouldCheckForRequiredKeyword)
{
if (shouldCheckForRequiredKeyword && memberInfo.HasRequiredMemberAttribute())
{
IsRequired = true;
}
}

internal abstract bool GetMemberAndWriteJson(object obj, ref WriteStack state, Utf8JsonWriter writer);
internal abstract bool GetMemberAndWriteJsonExtensionData(object obj, ref WriteStack state, Utf8JsonWriter writer);

Expand Down Expand Up @@ -531,7 +539,7 @@ internal string GetDebugInfo(int indent = 0)
internal bool HasGetter => _untypedGet is not null;
internal bool HasSetter => _untypedSet is not null;

internal void InitializeUsingMemberReflection(MemberInfo memberInfo, JsonConverter? customConverter, JsonIgnoreCondition? ignoreCondition)
internal void InitializeUsingMemberReflection(MemberInfo memberInfo, JsonConverter? customConverter, JsonIgnoreCondition? ignoreCondition, bool shouldCheckForRequiredKeyword)
{
Debug.Assert(AttributeProvider == null);

Expand All @@ -558,6 +566,7 @@ internal void InitializeUsingMemberReflection(MemberInfo memberInfo, JsonConvert
CustomConverter = customConverter;
DeterminePoliciesFromMember(memberInfo);
DeterminePropertyNameFromMember(memberInfo);
DetermineIsRequired(memberInfo, shouldCheckForRequiredKeyword);

if (ignoreCondition != JsonIgnoreCondition.Always)
{
Expand Down Expand Up @@ -849,15 +858,17 @@ public JsonNumberHandling? NumberHandling
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.
/// Required property index on the list of JsonTypeInfo properties.
/// It is used as a unique identifier for required properties.
/// It is set just before property is configured and does not change afterward.
/// It is not equivalent to index on the properties list
/// </summary>
internal int Index
internal int RequiredPropertyIndex
{
get
{
Debug.Assert(_isConfigured);
Debug.Assert(IsRequired);
return _index;
}
set
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public abstract partial class JsonTypeInfo
/// <summary>
/// Indices of required properties.
/// </summary>
internal int[]? RequiredPropertiesIndices { get; private set; }
internal int NumberOfRequiredProperties { get; private set; }

private Action<object>? _onSerializing;
private Action<object>? _onSerialized;
Expand Down Expand Up @@ -884,37 +884,20 @@ internal void InitializePropertyCache()
}

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

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

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

if (numberOfRequiredProperties > 0)
{
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;
}
NumberOfRequiredProperties = numberOfRequiredProperties;
}

internal void InitializeConstructorParameters(JsonParameterInfoValues[] jsonParameters, bool sourceGenMode = false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,37 +28,6 @@ internal ReflectionJsonTypeInfo(JsonConverter converter, JsonSerializerOptions o
if (PropertyInfoForTypeInfo.ConverterStrategy == ConverterStrategy.Object)
{
AddPropertiesAndParametersUsingReflection();

// 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 =
typeof(T).HasRequiredMemberAttribute()
&& !(converter.ConstructorInfo?.HasSetsRequiredMembersAttribute() ?? false);

if (shouldCheckMembersForRequiredMemberAttribute)
{
Debug.Assert(PropertyCache != null);
foreach (KeyValuePair<string, JsonPropertyInfo> kv in PropertyCache.List)
{
JsonPropertyInfo property = kv.Value;
Debug.Assert(property.AttributeProvider != null);
if (property.AttributeProvider.HasRequiredMemberAttribute())
{
property.IsRequired = true;
}
}

if (ExtensionDataProperty != null)
{
Debug.Assert(ExtensionDataProperty.AttributeProvider != null);
if (ExtensionDataProperty.AttributeProvider.HasRequiredMemberAttribute())
{
// This will end up in error in Configure
// but we need to give contract resolver a chance to fix this
ExtensionDataProperty.IsRequired = true;
}
}
}
}

Func<object>? createObject = Options.MemberAccessorStrategy.CreateConstructor(typeof(T));
Expand Down Expand Up @@ -88,6 +57,11 @@ private void AddPropertiesAndParametersUsingReflection()

Dictionary<string, JsonPropertyInfo>? ignoredMembers = null;
bool propertyOrderSpecified = false;
// 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 =
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
typeof(T).HasRequiredMemberAttribute()
&& !(Converter.ConstructorInfo?.HasSetsRequiredMembersAttribute() ?? false);

// Walk through the inheritance hierarchy, starting from the most derived type upward.
for (Type? currentType = Type; currentType != null; currentType = currentType.BaseType)
Expand Down Expand Up @@ -118,7 +92,8 @@ private void AddPropertiesAndParametersUsingReflection()
typeToConvert: propertyInfo.PropertyType,
memberInfo: propertyInfo,
ref propertyOrderSpecified,
ref ignoredMembers);
ref ignoredMembers,
shouldCheckMembersForRequiredMemberAttribute);
}
else
{
Expand Down Expand Up @@ -150,7 +125,8 @@ private void AddPropertiesAndParametersUsingReflection()
typeToConvert: fieldInfo.FieldType,
memberInfo: fieldInfo,
ref propertyOrderSpecified,
ref ignoredMembers);
ref ignoredMembers,
shouldCheckMembersForRequiredMemberAttribute);
}
}
else
Expand All @@ -177,9 +153,10 @@ private void CacheMember(
Type typeToConvert,
MemberInfo memberInfo,
ref bool propertyOrderSpecified,
ref Dictionary<string, JsonPropertyInfo>? ignoredMembers)
ref Dictionary<string, JsonPropertyInfo>? ignoredMembers,
bool shouldCheckForRequiredKeyword)
{
JsonPropertyInfo? jsonPropertyInfo = CreateProperty(typeToConvert, memberInfo, Options);
JsonPropertyInfo? jsonPropertyInfo = CreateProperty(typeToConvert, memberInfo, Options, shouldCheckForRequiredKeyword);
if (jsonPropertyInfo == null)
{
// ignored invalid property
Expand All @@ -199,7 +176,8 @@ private void CacheMember(
private JsonPropertyInfo? CreateProperty(
Type typeToConvert,
MemberInfo memberInfo,
JsonSerializerOptions options)
JsonSerializerOptions options,
bool shouldCheckForRequiredKeyword)
{
JsonIgnoreCondition? ignoreCondition = memberInfo.GetCustomAttribute<JsonIgnoreAttribute>(inherit: false)?.Condition;

Expand All @@ -224,7 +202,7 @@ private void CacheMember(
}

JsonPropertyInfo jsonPropertyInfo = CreatePropertyUsingReflection(typeToConvert);
jsonPropertyInfo.InitializeUsingMemberReflection(memberInfo, customConverter, ignoreCondition);
jsonPropertyInfo.InitializeUsingMemberReflection(memberInfo, customConverter, ignoreCondition, shouldCheckForRequiredKeyword);
return jsonPropertyInfo;
}

Expand Down
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.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.CompilerServices;
Expand Down Expand Up @@ -68,7 +69,7 @@ public JsonTypeInfo BaseJsonTypeInfo
public JsonNumberHandling? NumberHandling;

// Required properties left
public HashSet<int>? RequiredPropertiesLeft;
public BitArray? RequiredPropertiesLeft;
krwq marked this conversation as resolved.
Show resolved Hide resolved

public void EndConstructorParameter()
{
Expand Down Expand Up @@ -117,7 +118,7 @@ public void MarkRequiredPropertyAsRead(JsonPropertyInfo propertyInfo)
if (propertyInfo.IsRequired)
krwq marked this conversation as resolved.
Show resolved Hide resolved
{
Debug.Assert(RequiredPropertiesLeft != null);
RequiredPropertiesLeft.Remove(propertyInfo.Index);
RequiredPropertiesLeft.Set(propertyInfo.RequiredPropertyIndex, false);
krwq marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -126,22 +127,26 @@ internal void InitializeRequiredPropertiesValidationState(JsonTypeInfo typeInfo)
{
Debug.Assert(RequiredPropertiesLeft == null);

if (typeInfo.RequiredPropertiesIndices != null)
if (typeInfo.NumberOfRequiredProperties > 0)
{
RequiredPropertiesLeft = new HashSet<int>(typeInfo.RequiredPropertiesIndices);
RequiredPropertiesLeft = new BitArray(typeInfo.NumberOfRequiredProperties, defaultValue: true);
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void ValidateAllRequiredPropertiesAreRead(JsonTypeInfo typeInfo)
{
if (typeInfo.RequiredPropertiesIndices != null)
if (typeInfo.NumberOfRequiredProperties > 0)
{
Debug.Assert(RequiredPropertiesLeft != null);

if (RequiredPropertiesLeft.Count != 0)
// Optimize this when https://github.com/dotnet/runtime/issues/72999 is fixed
for (int i = 0; i < RequiredPropertiesLeft.Count; i++)
krwq marked this conversation as resolved.
Show resolved Hide resolved
{
ThrowHelper.ThrowJsonException_JsonRequiredPropertyMissing(typeInfo, RequiredPropertiesLeft);
if (RequiredPropertiesLeft[i])
{
ThrowHelper.ThrowJsonException_JsonRequiredPropertyMissing(typeInfo, RequiredPropertiesLeft);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Buffers;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
Expand Down Expand Up @@ -215,18 +216,37 @@ public static void ThrowInvalidOperationException_JsonPropertyRequiredAndExtensi
}

[DoesNotReturn]
public static void ThrowJsonException_JsonRequiredPropertyMissing(JsonTypeInfo parent, IEnumerable<int> missingJsonProperties)
public static void ThrowJsonException_JsonRequiredPropertyMissing(JsonTypeInfo parent, BitArray missingJsonProperties)
{
StringBuilder listOfMissingPropertiesBuilder = new();
bool first = true;

Debug.Assert(parent.PropertyCache != null);

foreach (int missingPropertyIndex in missingJsonProperties)
int propertyIdx = 0;

for (int requiredPropertyIdx = 0; requiredPropertyIdx < missingJsonProperties.Length; requiredPropertyIdx++)
krwq marked this conversation as resolved.
Show resolved Hide resolved
{
JsonPropertyInfo missingProperty = parent.PropertyCache.List[missingPropertyIndex].Value;
Debug.Assert(missingProperty.IsRequired);
Debug.Assert(missingProperty.Index == missingPropertyIndex);
if (!missingJsonProperties[requiredPropertyIdx])
{
continue;
}

JsonPropertyInfo? missingProperty = null;

// requiredPropertyIdx indices occur consecutively so we can resume iteration
for (; propertyIdx < parent.PropertyCache.List.Count; propertyIdx++)
{
JsonPropertyInfo maybeMissingProperty = parent.PropertyCache.List[propertyIdx].Value;
if (maybeMissingProperty.IsRequired && maybeMissingProperty.RequiredPropertyIndex == requiredPropertyIdx)
{
missingProperty = maybeMissingProperty;
break;
}
}

Debug.Assert(propertyIdx != parent.PropertyCache.List.Count);
Debug.Assert(missingProperty != null);

if (!first)
{
Expand Down