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

Simplify the JsonTypeInfo.CreateObject implemenetation #2

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStac
internal override void ConfigureJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options)
{
// Deserialize as List<T> for interface types that support it.
if (jsonTypeInfo.UntypedCreateObject is null && TypeToConvert.IsAssignableFrom(typeof(List<TElement>)))
if (jsonTypeInfo.CreateObject is null && TypeToConvert.IsAssignableFrom(typeof(List<TElement>)))
{
Debug.Assert(TypeToConvert.IsInterface);
jsonTypeInfo.UntypedCreateObject = () => new List<TElement>();
jsonTypeInfo.CreateObject = () => new List<TElement>();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ protected internal override bool OnWriteResume(Utf8JsonWriter writer, TDictionar
internal override void ConfigureJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options)
{
// Deserialize as Dictionary<TKey,TValue> for interface types that support it.
if (jsonTypeInfo.UntypedCreateObject is null && TypeToConvert.IsAssignableFrom(typeof(Dictionary<string, object?>)))
if (jsonTypeInfo.CreateObject is null && TypeToConvert.IsAssignableFrom(typeof(Dictionary<string, object?>)))
{
Debug.Assert(TypeToConvert.IsInterface);
jsonTypeInfo.UntypedCreateObject = () => new Dictionary<string, object?>();
jsonTypeInfo.CreateObject = () => new Dictionary<string, object?>();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStac
internal override void ConfigureJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options)
{
// Deserialize as Dictionary<TKey,TValue> for interface types that support it.
if (jsonTypeInfo.UntypedCreateObject is null && TypeToConvert.IsAssignableFrom(typeof(Dictionary<TKey, TValue>)))
if (jsonTypeInfo.CreateObject is null && TypeToConvert.IsAssignableFrom(typeof(Dictionary<TKey, TValue>)))
{
Debug.Assert(TypeToConvert.IsInterface);
jsonTypeInfo.UntypedCreateObject = () => new Dictionary<TKey, TValue>();
jsonTypeInfo.CreateObject = () => new Dictionary<TKey, TValue>();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value,
internal override void ConfigureJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options)
{
// Deserialize as List<object?> for interface types that support it.
if (jsonTypeInfo.UntypedCreateObject is null && TypeToConvert.IsAssignableFrom(typeof(List<object?>)))
if (jsonTypeInfo.CreateObject is null && TypeToConvert.IsAssignableFrom(typeof(List<object?>)))
{
Debug.Assert(TypeToConvert.IsInterface);
jsonTypeInfo.UntypedCreateObject = () => new List<object?>();
jsonTypeInfo.CreateObject = () => new List<object?>();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStac
internal override void ConfigureJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options)
{
// Deserialize as List<T> for interface types that support it.
if (jsonTypeInfo.UntypedCreateObject is null && TypeToConvert.IsAssignableFrom(typeof(List<TElement>)))
if (jsonTypeInfo.CreateObject is null && TypeToConvert.IsAssignableFrom(typeof(List<TElement>)))
{
Debug.Assert(TypeToConvert.IsInterface);
jsonTypeInfo.UntypedCreateObject = () => new List<TElement>();
jsonTypeInfo.CreateObject = () => new List<TElement>();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStac
internal override void ConfigureJsonTypeInfo(JsonTypeInfo jsonTypeInfo, JsonSerializerOptions options)
{
// Deserialize as HashSet<TElement> for interface types that support it.
if (jsonTypeInfo.UntypedCreateObject is null && TypeToConvert.IsAssignableFrom(typeof(HashSet<TElement>)))
if (jsonTypeInfo.CreateObject is null && TypeToConvert.IsAssignableFrom(typeof(HashSet<TElement>)))
{
Debug.Assert(TypeToConvert.IsInterface);
jsonTypeInfo.UntypedCreateObject = () => new HashSet<TElement>();
jsonTypeInfo.CreateObject = () => new HashSet<TElement>();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ protected virtual void CreateCollection(ref Utf8JsonReader reader, ref ReadStack
{
JsonTypeInfo typeInfo = state.Current.JsonTypeInfo;

if (typeInfo.UntypedCreateObject is null)
if (typeInfo.CreateObject is null)
{
// The contract model was not able to produce a default constructor for two possible reasons:
// 1. Either the declared collection type is abstract and cannot be instantiated.
Expand All @@ -41,7 +41,7 @@ protected virtual void CreateCollection(ref Utf8JsonReader reader, ref ReadStack
}
}

state.Current.ReturnValue = typeInfo.UntypedCreateObject();
state.Current.ReturnValue = typeInfo.CreateObject();
Debug.Assert(state.Current.ReturnValue is TCollection);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ protected virtual void CreateCollection(ref Utf8JsonReader reader, ref ReadStack
{
JsonTypeInfo typeInfo = state.Current.JsonTypeInfo;

if (typeInfo.UntypedCreateObject is null)
if (typeInfo.CreateObject is null)
{
// The contract model was not able to produce a default constructor for two possible reasons:
// 1. Either the declared collection type is abstract and cannot be instantiated.
Expand All @@ -56,7 +56,7 @@ protected virtual void CreateCollection(ref Utf8JsonReader reader, ref ReadStack
}
}

state.Current.ReturnValue = typeInfo.UntypedCreateObject()!;
state.Current.ReturnValue = typeInfo.CreateObject()!;
Debug.Assert(state.Current.ReturnValue is TDictionary);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ protected override void Add(in TElement value, ref ReadStack state)

protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
{
if (state.Current.JsonTypeInfo.UntypedCreateObject == null)
if (state.Current.JsonTypeInfo.CreateObject == null)
{
ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(state.Current.JsonTypeInfo.Type);
}

state.Current.ReturnValue = state.Current.JsonTypeInfo.UntypedCreateObject();
state.Current.ReturnValue = state.Current.JsonTypeInfo.CreateObject();
}

protected override bool OnWriteResume(Utf8JsonWriter writer, TCollection value, JsonSerializerOptions options, ref WriteStack state)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ protected override void Add(in TElement value, ref ReadStack state)

protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
{
if (state.Current.JsonTypeInfo.UntypedCreateObject == null)
if (state.Current.JsonTypeInfo.CreateObject == null)
{
ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(state.Current.JsonTypeInfo.Type);
}

state.Current.ReturnValue = state.Current.JsonTypeInfo.UntypedCreateObject();
state.Current.ReturnValue = state.Current.JsonTypeInfo.CreateObject();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ protected override void Add(in TElement value, ref ReadStack state)

protected override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
{
if (state.Current.JsonTypeInfo.UntypedCreateObject == null)
if (state.Current.JsonTypeInfo.CreateObject == null)
{
ThrowHelper.ThrowNotSupportedException_SerializationNotSupported(state.Current.JsonTypeInfo.Type);
}

state.Current.ReturnValue = state.Current.JsonTypeInfo.UntypedCreateObject();
state.Current.ReturnValue = state.Current.JsonTypeInfo.CreateObject();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ protected sealed override void Add(in object? value, ref ReadStack state)
protected sealed override void CreateCollection(ref Utf8JsonReader reader, ref ReadStack state, JsonSerializerOptions options)
{
JsonTypeInfo typeInfo = state.Current.JsonTypeInfo;
Func<object>? constructorDelegate = typeInfo.UntypedCreateObject;
Func<object>? constructorDelegate = typeInfo.CreateObject;

if (constructorDelegate == null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
ThrowHelper.ThrowJsonException_DeserializeUnableToConvertValue(TypeToConvert);
}

if (jsonTypeInfo.UntypedCreateObject == null)
if (jsonTypeInfo.CreateObject == null)
{
ThrowHelper.ThrowNotSupportedException_DeserializeNoConstructor(jsonTypeInfo.Type, ref reader, ref state);
}

obj = jsonTypeInfo.UntypedCreateObject()!;
obj = jsonTypeInfo.CreateObject()!;

if (obj is IJsonOnDeserializing onDeserializing)
{
Expand Down Expand Up @@ -127,12 +127,12 @@ internal override bool OnTryRead(ref Utf8JsonReader reader, Type typeToConvert,
return true;
}

if (jsonTypeInfo.UntypedCreateObject == null)
if (jsonTypeInfo.CreateObject == null)
{
ThrowHelper.ThrowNotSupportedException_DeserializeNoConstructor(jsonTypeInfo.Type, ref reader, ref state);
}

obj = jsonTypeInfo.UntypedCreateObject()!;
obj = jsonTypeInfo.CreateObject()!;

if (state.Current.MetadataPropertyNames.HasFlag(MetadataPropertyName.Id))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ internal static void CreateDataExtensionProperty(
genericArgs[1].UnderlyingSystemType == typeof(JsonElement) ||
genericArgs[1].UnderlyingSystemType == typeof(Nodes.JsonNode));
#endif
if (jsonPropertyInfo.JsonTypeInfo.UntypedCreateObject == null)
if (jsonPropertyInfo.JsonTypeInfo.CreateObject == null)
{
// Avoid a reference to the JsonNode type for trimming
if (jsonPropertyInfo.PropertyType.FullName == JsonTypeInfo.JsonObjectTypeName)
Expand All @@ -135,7 +135,7 @@ internal static void CreateDataExtensionProperty(
}
else
{
extensionData = jsonPropertyInfo.JsonTypeInfo.UntypedCreateObject();
extensionData = jsonPropertyInfo.JsonTypeInfo.CreateObject();
}

jsonPropertyInfo.SetExtensionDictionaryAsObject(obj, extensionData);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,16 @@ public abstract partial class JsonTypeInfo
/// </summary>
public Func<object>? CreateObject
{
get => UntypedCreateObjectAbstract;
set => UntypedCreateObjectAbstract = value;
get => _createObject;
set
{
SetCreateObject(value);
}
}

private protected abstract void SetCreateObject(Delegate? createObject);
krwq marked this conversation as resolved.
Show resolved Hide resolved
private protected Func<object>? _createObject;

/// <summary>
/// Gets JsonPropertyInfo list. Only applicable when Kind is Object.
/// </summary>
Expand Down Expand Up @@ -69,13 +75,6 @@ public IList<JsonPropertyInfo> Properties
}
}

// Untyped CreateObject is non-virtual public API so we pretend it's virtual by using this indirection
// We need it to be abstract so that we can keep typed value in sync
internal abstract Func<object>? UntypedCreateObjectAbstract { get; set; }

// Actual value of UntypedCreateObject, for perf it's non-virtual
internal Func<object>? UntypedCreateObject { get; set; }

internal object? CreateObjectWithArgs { get; set; }

// Add method delegate for non-generic Stack and Queue; and types that derive from them.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,23 +21,36 @@ public abstract class JsonTypeInfo<T> : JsonTypeInfo
/// </summary>
public new Func<T>? CreateObject
{
get => _typedCreateObject ??= UntypedCreateObject == null ? null : () => (T)UntypedCreateObject();
get => _typedCreateObject;
set
{
_typedCreateObject = value;
UntypedCreateObject = value == null ? null : () => value()!;
SetCreateObject(value);
}
}

internal override Func<object>? UntypedCreateObjectAbstract
private protected override void SetCreateObject(Delegate? createObject)
{
get => UntypedCreateObject;
set
Debug.Assert(createObject is null or Func<object> or Func<T>);

if (createObject is null)
{
UntypedCreateObject = value;
// We invalidate the cached typed value
_createObject = null;
_typedCreateObject = null;
return;
}

if (createObject is Func<object> untypedDelegate)
{
_createObject = untypedDelegate;
_typedCreateObject = () => (T)untypedDelegate();
return;
}

Debug.Assert(createObject is Func<T>);

Func<T> typedDelegate = (Func<T>)createObject;
_createObject = () => typedDelegate()!;
_typedCreateObject = typedDelegate;
}

internal JsonTypeInfo(JsonConverter converter, JsonSerializerOptions options)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ internal ReflectionJsonTypeInfo(JsonConverter converter, JsonSerializerOptions o
AddPropertiesAndParametersUsingReflection();
}

UntypedCreateObject = Options.MemberAccessorStrategy.CreateConstructor(typeof(T));
((JsonTypeInfo)this).CreateObject = Options.MemberAccessorStrategy.CreateConstructor(typeof(T));
eiriktsarpalis marked this conversation as resolved.
Show resolved Hide resolved
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026:RequiresUnreferencedCode",
Expand Down