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
10 changes: 5 additions & 5 deletions src/libraries/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -665,10 +665,10 @@
<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 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="JsonRequiredPropertiesMissingItem" xml:space="preserve">
<value>- {0}</value>
<data name="JsonPropertyRequiredAndIgnoreNullValues" xml:space="preserve">
<value>Required JSON properties cannot be used in conjunction with JsonSerializerOptions.IgnoreNullValues.</value>
</data>
</root>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
obj = jsonTypeInfo.CreateObject()!;

jsonTypeInfo.OnDeserializing?.Invoke(obj);
jsonTypeInfo.InitializeRequiredProperties(ref state);
state.Current.InitializeRequiredProperties(jsonTypeInfo);
krwq marked this conversation as resolved.
Show resolved Hide resolved

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

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

jsonTypeInfo.InitializeRequiredProperties(ref state);
state.Current.InitializeRequiredProperties(jsonTypeInfo);
}
else
{
Expand Down Expand Up @@ -253,7 +252,7 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
}

jsonTypeInfo.OnDeserialized?.Invoke(obj);
jsonTypeInfo.CheckRequiredProperties(ref state);
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,8 @@ 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,13 +67,12 @@ private static bool TryRead<TArg>(

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

if (value == null && jsonParameterInfo.IgnoreNullTokensOnRead)
{
arg = (TArg?)info.DefaultValue!; // Use default value specified on parameter, if any.
}
else
arg = value == null && jsonParameterInfo.IgnoreNullTokensOnRead
? (TArg?)info.DefaultValue! // Use default value specified on parameter, if any.
: value!;

if (success)
{
arg = value!;
state.Current.MarkRequiredPropertyAsRead(jsonParameterInfo.MatchingProperty);
}

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.SetValueAsObject(obj, propValue, ref state);
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,7 +219,7 @@ internal sealed override bool OnTryRead(ref Utf8JsonReader reader, Type typeToCo
}

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

// Unbox
Debug.Assert(obj != null);
Expand Down Expand Up @@ -534,7 +542,7 @@ private void BeginRead(ref ReadStack state, ref Utf8JsonReader reader, JsonSeria
ThrowHelper.ThrowInvalidOperationException_ConstructorParameterIncompleteBinding(TypeToConvert);
}

jsonTypeInfo.InitializeRequiredProperties(ref state);
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 @@ -303,6 +303,11 @@ internal void Configure()
{
ThrowHelper.ThrowInvalidOperationException_JsonPropertyRequiredAndExtensionData(this);
}

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

Expand Down Expand Up @@ -785,10 +790,6 @@ 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 Expand Up @@ -850,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 @@ -339,10 +339,10 @@ internal override bool ReadJsonAndSetMember(object obj, ref ReadStack state, ref
{
T? value = default;
Set!(obj, value!);
state.Current.MarkRequiredPropertyAsRead(this);
}

success = true;
state.Current.MarkRequiredPropertyAsRead(this);
}
else if (TypedEffectiveConverter.CanUseDirectReadOrWrite && state.Current.NumberHandling == null)
{
Expand All @@ -354,10 +354,10 @@ 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;
state.Current.MarkRequiredPropertyAsRead(this);
}
else
{
Expand Down Expand Up @@ -411,25 +411,6 @@ internal override bool ReadJsonAsObject(ref ReadStack state, ref Utf8JsonReader
return success;
}

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

internal sealed override void SetValueAsObject(object obj, object? value, ref ReadStack state)
{
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,7 +25,11 @@ 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; }

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

private Action<object>? _onSerializing;
private Action<object>? _onSerialized;
Expand Down Expand Up @@ -270,17 +274,16 @@ 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; }

// 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
// Configure would normally have thrown why 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 Exception? DeserializationException { get; private protected set; }
internal bool MetadataSerializationNotSupported { get; private protected set; }

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void ValidateCanBeUsedForMetadataSerialization()
{
if (DeserializationException != null)
if (MetadataSerializationNotSupported)
{
throw DeserializationException;
ThrowHelper.ThrowInvalidOperationException_NoMetadataForTypeProperties(Options.TypeInfoResolver, Type);
}
}

Expand Down Expand Up @@ -881,6 +884,7 @@ internal void InitializePropertyCache()
}

int numberOfRequiredProperties = 0;
int propertyIndex = 0;
foreach (KeyValuePair<string, JsonPropertyInfo> jsonPropertyInfoKv in PropertyCache.List)
{
JsonPropertyInfo jsonPropertyInfo = jsonPropertyInfoKv.Value;
Expand All @@ -890,25 +894,26 @@ internal void InitializePropertyCache()
numberOfRequiredProperties++;
}

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

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

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

RequiredProperties = requiredProperties;
RequiredPropertiesIndices = requiredPropertiesIndices;
}
}

Expand Down Expand Up @@ -1027,30 +1032,6 @@ 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);
}
}

[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 @@ -113,7 +113,7 @@ internal override JsonParameterInfoValues[] GetParameterInfoValues()
}

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

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Text.Json.Serialization;
using System.Text.Json.Serialization.Metadata;
Expand Down Expand Up @@ -67,7 +68,7 @@ public JsonTypeInfo BaseJsonTypeInfo
public JsonNumberHandling? NumberHandling;

// Required properties left
public HashSet<JsonPropertyInfo>? RequiredPropertiesLeft;
public HashSet<int>? RequiredPropertiesLeft;

public void EndConstructorParameter()
{
Expand Down Expand Up @@ -110,12 +111,37 @@ public bool IsProcessingEnumerable()
return (JsonTypeInfo.PropertyInfoForTypeInfo.ConverterStrategy & ConverterStrategy.Enumerable) != 0;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
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);
RequiredPropertiesLeft.Remove(propertyInfo.Index);
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal void InitializeRequiredProperties(JsonTypeInfo typeInfo)
{
if (typeInfo.RequiredPropertiesIndices != null)
{
Debug.Assert(RequiredPropertiesLeft == null);
krwq marked this conversation as resolved.
Show resolved Hide resolved
RequiredPropertiesLeft = new HashSet<int>(typeInfo.RequiredPropertiesIndices);
krwq marked this conversation as resolved.
Show resolved Hide resolved
}
}

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

if (RequiredPropertiesLeft.Count != 0)
{
ThrowHelper.ThrowJsonException_JsonRequiredPropertyMissing(typeInfo, RequiredPropertiesLeft);
}
}
}

Expand Down
Loading