Skip to content
New issue

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

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

Already on GitHub? # to your account

Make JSON support required properties #72937

Merged
merged 11 commits into from
Jul 29, 2022
9 changes: 9 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,13 @@
<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 '{0}' defined in type '{1}' is marked required but does not specify a setter.</value>
</data>
<data name="JsonPropertyRequiredAndExtensionData" xml:space="preserve">
<value>JsonPropertyInfo '{0}' defined in type '{1}' is marked both as required and as an extension data property. This combination is not supported.</value>
</data>
<data name="JsonRequiredPropertiesMissing" xml:space="preserve">
<value>JSON deserialization for type '{0}' was missing required properties, including the following: {1}</value>
</data>
</root>
27 changes: 27 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;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.ExceptionServices;
Expand Down Expand Up @@ -42,6 +43,32 @@ 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)
{
// For compiler related attributes we should only look at full type name rather than trying to do something different for version when attribute was introduced.
// I.e. library is targetting netstandard2.0 with polyfilled attributes and is being consumed by app targetting net7.0.
return memberInfo.HasCustomAttributeWithName("System.Runtime.CompilerServices.RequiredMemberAttribute", inherit: true);
}

public static bool HasSetsRequiredMembersAttribute(this ICustomAttributeProvider memberInfo)
{
// See comment for HasRequiredMemberAttribute for why we need to always only look at full name
return memberInfo.HasCustomAttributeWithName("System.Diagnostics.CodeAnalysis.SetsRequiredMembersAttribute", inherit: true);
}

private static bool HasCustomAttributeWithName(this ICustomAttributeProvider memberInfo, string fullName, bool inherit)
{
foreach (object attribute in memberInfo.GetCustomAttributes(inherit))
{
if (attribute.GetType().FullName == fullName)
{
return true;
}
}

return false;
}

public static TAttribute? GetUniqueCustomAttribute<TAttribute>(this MemberInfo memberInfo, bool inherit)
where TAttribute : Attribute
{
Expand Down
15 changes: 15 additions & 0 deletions src/libraries/System.Text.Json/src/System/Text/Json/JsonHelpers.cs
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.Runtime.CompilerServices;
Expand Down Expand Up @@ -147,5 +148,19 @@ public static void ValidateInt32MaxArrayLength(uint length)
ThrowHelper.ThrowOutOfMemoryException(length);
}
}

public static bool AllBitsEqual(this BitArray bitArray, bool value)
{
// Optimize this when https://github.com/dotnet/runtime/issues/72999 is fixed
for (int i = 0; i < bitArray.Count; i++)
{
if (bitArray[i] != value)
{
return false;
}
}

return true;
}
}
}
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 @@ -477,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 @@ -504,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 @@ -531,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 @@ -760,8 +796,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 +857,29 @@ public JsonNumberHandling? NumberHandling
/// </summary>
internal abstract object? DefaultValue { get; }

/// <summary>
/// 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 RequiredPropertyIndex
{
get
{
Debug.Assert(_isConfigured);
Debug.Assert(IsRequired);
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 NumberOfRequiredProperties { get; private set; }

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

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

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

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

NumberOfRequiredProperties = numberOfRequiredProperties;
}

internal void InitializeConstructorParameters(JsonParameterInfoValues[] jsonParameters, bool sourceGenMode = false)
Expand Down
Loading