From 02f0b2135c7c559b35c4086655aa4f7d47a9ca4c Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Thu, 7 Apr 2022 17:07:43 +0200 Subject: [PATCH] Cleanup JsonTypeInfo construction (#67700) --- .../JsonSerializerOptions.Converters.cs | 2 +- .../Metadata/JsonMetadataServices.cs | 12 -- .../Metadata/JsonPropertyInfo.cs | 52 ++----- .../Metadata/JsonPropertyInfoOfT.cs | 145 ++++++++---------- .../Metadata/JsonTypeInfo.Cache.cs | 18 +-- .../Serialization/Metadata/JsonTypeInfo.cs | 17 +- .../Serialization/Metadata/JsonTypeInfoOfT.cs | 9 -- .../Metadata/ReflectionJsonTypeInfoOfT.cs | 4 +- .../Metadata/SourceGenJsonTypeInfoOfT.cs | 50 +++--- 9 files changed, 123 insertions(+), 186 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs index 4c977084c51d1..2f12221b70bdb 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs @@ -59,7 +59,7 @@ static JsonTypeInfo CreateJsonTypeInfo(Type type, JsonSerializerOptions options) // Some of the validation is done during construction (i.e. validity of JsonConverter, inner types etc.) // therefore we need to unwrap TargetInvocationException for better user experience ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); - throw ex.InnerException; + throw null!; } #endif } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.cs index 1184d9c10a08f..c59a8498253ca 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.cs @@ -81,19 +81,7 @@ public static JsonPropertyInfo CreatePropertyInfo(JsonSerializerOptions optio public static JsonTypeInfo CreateValueInfo(JsonSerializerOptions options, JsonConverter converter) { JsonTypeInfo info = new SourceGenJsonTypeInfo(converter, options); - info.PropertyInfoForTypeInfo = CreateJsonPropertyInfoForClassInfo(typeof(T), info, converter, options); return info; } - - internal static JsonPropertyInfo CreateJsonPropertyInfoForClassInfo( - Type type, - JsonTypeInfo typeInfo, - JsonConverter converter, - JsonSerializerOptions options) - { - JsonPropertyInfo propertyInfo = converter.CreateJsonPropertyInfo(); - propertyInfo.InitializeForTypeInfo(type, typeInfo, converter, options); - return propertyInfo; - } } } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs index 812c500513536..0c6f5001bff7d 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs @@ -90,24 +90,21 @@ internal void Configure() _isConfigured = true; } - internal virtual void GetPolicies(JsonIgnoreCondition? ignoreCondition) + internal void GetPolicies(JsonIgnoreCondition? ignoreCondition) { - if (!IsForTypeInfo) - { - Debug.Assert(MemberInfo != null); - DetermineSerializationCapabilities(ignoreCondition); - DeterminePropertyName(); - DetermineIgnoreCondition(ignoreCondition); - - JsonPropertyOrderAttribute? orderAttr = GetAttribute(MemberInfo); - if (orderAttr != null) - { - Order = orderAttr.Order; - } + Debug.Assert(MemberInfo != null); + DetermineSerializationCapabilities(ignoreCondition); + DeterminePropertyName(); + DetermineIgnoreCondition(ignoreCondition); - JsonNumberHandlingAttribute? attribute = GetAttribute(MemberInfo); - NumberHandling = attribute?.Handling; + JsonPropertyOrderAttribute? orderAttr = GetAttribute(MemberInfo); + if (orderAttr != null) + { + Order = orderAttr.Order; } + + JsonNumberHandlingAttribute? attribute = GetAttribute(MemberInfo); + NumberHandling = attribute?.Handling; } private void DeterminePropertyName() @@ -330,7 +327,7 @@ private bool NumberHandingIsApplicable() internal bool HasGetter { get; set; } internal bool HasSetter { get; set; } - internal virtual void Initialize( + internal abstract void Initialize( Type parentClassType, Type declaredPropertyType, ConverterStrategy converterStrategy, @@ -338,25 +335,8 @@ private bool NumberHandingIsApplicable() bool isVirtual, JsonConverter converter, JsonIgnoreCondition? ignoreCondition, - JsonNumberHandling? parentTypeNumberHandling, - JsonSerializerOptions options) - { - Debug.Assert(converter != null); - - DeclaringType = parentClassType; - PropertyType = declaredPropertyType; - ConverterStrategy = converterStrategy; - MemberInfo = memberInfo; - IsVirtual = isVirtual; - ConverterBase = converter; - Options = options; - } - - internal abstract void InitializeForTypeInfo( - Type declaredType, - JsonTypeInfo runtimeTypeInfo, - JsonConverter converter, - JsonSerializerOptions options); + JsonSerializerOptions options, + JsonTypeInfo? jsonTypeInfo = null); internal bool IgnoreDefaultValuesOnRead { get; private set; } internal bool IgnoreDefaultValuesOnWrite { get; private set; } @@ -483,7 +463,7 @@ internal bool ReadJsonExtensionDataValue(ref ReadStack state, ref Utf8JsonReader internal Type DeclaringType { get; set; } = null!; - internal MemberInfo? MemberInfo { get; private set; } + internal MemberInfo? MemberInfo { get; set; } internal JsonTypeInfo JsonTypeInfo { diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs index 0f257bd460128..321c50f01a0ef 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfoOfT.cs @@ -43,78 +43,89 @@ internal sealed class JsonPropertyInfo : JsonPropertyInfo bool isVirtual, JsonConverter converter, JsonIgnoreCondition? ignoreCondition, - JsonNumberHandling? parentTypeNumberHandling, - JsonSerializerOptions options) + JsonSerializerOptions options, + JsonTypeInfo? jsonTypeInfo = null) { - base.Initialize( - parentClassType, - declaredPropertyType, - converterStrategy, - memberInfo, - isVirtual, - converter, - ignoreCondition, - parentTypeNumberHandling, - options); - - switch (memberInfo) + Debug.Assert(converter != null); + + PropertyType = declaredPropertyType; + ConverterStrategy = converterStrategy; + if (jsonTypeInfo != null) { - case PropertyInfo propertyInfo: - { - bool useNonPublicAccessors = GetAttribute(propertyInfo) != null; + JsonTypeInfo = jsonTypeInfo; + } - MethodInfo? getMethod = propertyInfo.GetMethod; - if (getMethod != null && (getMethod.IsPublic || useNonPublicAccessors)) - { - HasGetter = true; - Get = options.MemberAccessorStrategy.CreatePropertyGetter(propertyInfo); - } + ConverterBase = converter; + Options = options; + DeclaringType = parentClassType; + MemberInfo = memberInfo; + IsVirtual = isVirtual; - MethodInfo? setMethod = propertyInfo.SetMethod; - if (setMethod != null && (setMethod.IsPublic || useNonPublicAccessors)) + if (memberInfo != null) + { + switch (memberInfo) + { + case PropertyInfo propertyInfo: { - HasSetter = true; - Set = options.MemberAccessorStrategy.CreatePropertySetter(propertyInfo); - } + bool useNonPublicAccessors = GetAttribute(propertyInfo) != null; - MemberType = MemberTypes.Property; + MethodInfo? getMethod = propertyInfo.GetMethod; + if (getMethod != null && (getMethod.IsPublic || useNonPublicAccessors)) + { + HasGetter = true; + Get = options.MemberAccessorStrategy.CreatePropertyGetter(propertyInfo); + } - break; - } + MethodInfo? setMethod = propertyInfo.SetMethod; + if (setMethod != null && (setMethod.IsPublic || useNonPublicAccessors)) + { + HasSetter = true; + Set = options.MemberAccessorStrategy.CreatePropertySetter(propertyInfo); + } - case FieldInfo fieldInfo: - { - Debug.Assert(fieldInfo.IsPublic); + MemberType = MemberTypes.Property; - HasGetter = true; - Get = options.MemberAccessorStrategy.CreateFieldGetter(fieldInfo); + break; + } - if (!fieldInfo.IsInitOnly) + case FieldInfo fieldInfo: { - HasSetter = true; - Set = options.MemberAccessorStrategy.CreateFieldSetter(fieldInfo); - } + Debug.Assert(fieldInfo.IsPublic); - MemberType = MemberTypes.Field; + HasGetter = true; + Get = options.MemberAccessorStrategy.CreateFieldGetter(fieldInfo); - break; - } + if (!fieldInfo.IsInitOnly) + { + HasSetter = true; + Set = options.MemberAccessorStrategy.CreateFieldSetter(fieldInfo); + } - default: - { - IsForTypeInfo = true; - HasGetter = true; - HasSetter = true; + MemberType = MemberTypes.Field; - break; - } - } + break; + } - _converterIsExternalAndPolymorphic = !converter.IsInternalConverter && PropertyType != converter.TypeToConvert; - PropertyTypeCanBeNull = PropertyType.CanBeNull(); - _propertyTypeEqualsTypeToConvert = typeof(T) == PropertyType; + default: + { + Debug.Fail($"Invalid memberInfo type: {memberInfo.GetType().FullName}"); + break; + } + } + + // TODO (perf): can we pre-compute some of these values during source gen? + _converterIsExternalAndPolymorphic = !converter.IsInternalConverter && PropertyType != converter.TypeToConvert; + PropertyTypeCanBeNull = PropertyType.CanBeNull(); + _propertyTypeEqualsTypeToConvert = typeof(T) == PropertyType; - GetPolicies(ignoreCondition); + GetPolicies(ignoreCondition); + } + else + { + IsForTypeInfo = true; + HasGetter = true; + HasSetter = true; + } } internal void InitializeForSourceGen(JsonSerializerOptions options, JsonPropertyInfoValues propertyInfo) @@ -189,30 +200,6 @@ internal void InitializeForSourceGen(JsonSerializerOptions options, JsonProperty } } - /// - /// Create a for a given Type. - /// See . - /// - internal override void InitializeForTypeInfo( - Type declaredType, - JsonTypeInfo runtimeTypeInfo, - JsonConverter converter, - JsonSerializerOptions options) - { - PropertyType = declaredType; - ConverterStrategy = converter.ConverterStrategy; - JsonTypeInfo = runtimeTypeInfo; - ConverterBase = converter; - Options = options; - IsForTypeInfo = true; - HasGetter = true; - HasSetter = true; - // TODO (perf): can we pre-compute some of these values during source gen? - _converterIsExternalAndPolymorphic = !converter.IsInternalConverter && declaredType != converter.TypeToConvert; - PropertyTypeCanBeNull = declaredType.CanBeNull(); - _propertyTypeEqualsTypeToConvert = typeof(T) == declaredType; - } - internal override JsonConverter ConverterBase { get diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs index 73bcbeffbffc4..b5e56245aea54 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.Cache.cs @@ -58,8 +58,8 @@ public partial class JsonTypeInfo bool isVirtual, JsonConverter converter, JsonSerializerOptions options, - JsonNumberHandling? parentTypeNumberHandling = null, - JsonIgnoreCondition? ignoreCondition = null) + JsonIgnoreCondition? ignoreCondition = null, + JsonTypeInfo? jsonTypeInfo = null) { // Create the JsonPropertyInfo instance. JsonPropertyInfo jsonPropertyInfo = converter.CreateJsonPropertyInfo(); @@ -72,8 +72,8 @@ public partial class JsonTypeInfo isVirtual, converter, ignoreCondition, - parentTypeNumberHandling, - options); + options, + jsonTypeInfo); return jsonPropertyInfo; } @@ -85,17 +85,17 @@ public partial class JsonTypeInfo private static JsonPropertyInfo CreatePropertyInfoForTypeInfo( Type declaredPropertyType, JsonConverter converter, - JsonNumberHandling? numberHandling, - JsonSerializerOptions options) + JsonSerializerOptions options, + JsonTypeInfo? jsonTypeInfo = null) { JsonPropertyInfo jsonPropertyInfo = CreateProperty( declaredPropertyType: declaredPropertyType, memberInfo: null, // Not a real property so this is null. parentClassType: ObjectType, // a dummy value (not used) isVirtual: false, - converter, - options, - parentTypeNumberHandling: numberHandling); + converter: converter, + options: options, + jsonTypeInfo: jsonTypeInfo); Debug.Assert(jsonPropertyInfo.IsForTypeInfo); diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs index baf429066e87c..f00913781c1bb 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfo.cs @@ -152,7 +152,7 @@ internal void ValidateCanBeUsedForDeserialization() /// TypeInfo (for the cases mentioned above). In addition, methods that have a JsonPropertyInfo argument would also likely /// need to add an argument for JsonTypeInfo. /// - internal JsonPropertyInfo PropertyInfoForTypeInfo { get; set; } + internal JsonPropertyInfo PropertyInfoForTypeInfo { get; private set; } /// /// Returns a helper class used for computing the default value. @@ -162,24 +162,11 @@ internal void ValidateCanBeUsedForDeserialization() internal JsonNumberHandling? NumberHandling { get; set; } - internal JsonTypeInfo() - { - Debug.Assert(false, "This constructor should not be called."); - } - - internal JsonTypeInfo(Type type, JsonSerializerOptions options!!) - { - Type = type; - Options = options; - // Setting this option is deferred to the initialization methods of the various metadada info types. - PropertyInfoForTypeInfo = null!; - } - internal JsonTypeInfo(Type type, JsonConverter converter, JsonSerializerOptions options) { Type = type; Options = options; - PropertyInfoForTypeInfo = CreatePropertyInfoForTypeInfo(Type, converter, NumberHandling, Options); + PropertyInfoForTypeInfo = CreatePropertyInfoForTypeInfo(Type, converter, Options, this); ElementType = converter.ElementType; switch (PropertyInfoForTypeInfo.ConverterStrategy) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs index 6e4a5a40b4815..f5e3e341bbceb 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonTypeInfoOfT.cs @@ -15,19 +15,10 @@ public abstract class JsonTypeInfo : JsonTypeInfo { private Action? _serialize; - internal JsonTypeInfo(Type type, JsonSerializerOptions options) : - base(type, options) - { } - internal JsonTypeInfo(JsonConverter converter, JsonSerializerOptions options) : base(typeof(T), converter, options) { } - internal JsonTypeInfo() - { - Debug.Assert(false, "This constructor should not be called."); - } - /// /// Serializes an instance of using /// values specified at design time. diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs index ee1df1392d5b2..b69c63adb1188 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/ReflectionJsonTypeInfoOfT.cs @@ -176,7 +176,7 @@ private void AddPropertiesAndParametersUsingReflection() ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateTypeAttribute(Type, typeof(JsonExtensionDataAttribute)); } - JsonPropertyInfo jsonPropertyInfo = AddProperty(memberInfo, memberType, declaringType, isVirtual, typeNumberHandling, Options); + JsonPropertyInfo jsonPropertyInfo = AddProperty(memberInfo, memberType, declaringType, isVirtual, Options); Debug.Assert(jsonPropertyInfo.NameAsString != null); if (hasExtensionAttribute) @@ -197,7 +197,6 @@ private void AddPropertiesAndParametersUsingReflection() Type memberType, Type parentClassType, bool isVirtual, - JsonNumberHandling? parentTypeNumberHandling, JsonSerializerOptions options) { JsonIgnoreCondition? ignoreCondition = JsonPropertyInfo.GetAttribute(memberInfo)?.Condition; @@ -221,7 +220,6 @@ private void AddPropertiesAndParametersUsingReflection() isVirtual, converter, options, - parentTypeNumberHandling, ignoreCondition); } diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs index 6dc1a5b17d231..c9e5ea88df3d2 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/SourceGenJsonTypeInfoOfT.cs @@ -18,40 +18,28 @@ internal sealed class SourceGenJsonTypeInfo : JsonTypeInfo /// Creates serialization metadata for a type using a simple converter. /// public SourceGenJsonTypeInfo(JsonConverter converter, JsonSerializerOptions options) - : base(typeof(T), options) + : base(converter, options) { - ElementType = converter.ElementType; } /// /// Creates serialization metadata for an object. /// - public SourceGenJsonTypeInfo(JsonSerializerOptions options, JsonObjectInfoValues objectInfo) : base(typeof(T), options) + public SourceGenJsonTypeInfo(JsonSerializerOptions options, JsonObjectInfoValues objectInfo) : base(GetConverter(objectInfo), options) { -#pragma warning disable CS8714 - // The type cannot be used as type parameter in the generic type or method. - // Nullability of type argument doesn't match 'notnull' constraint. - JsonConverter converter; - if (objectInfo.ObjectWithParameterizedConstructorCreator != null) { - converter = new JsonMetadataServicesConverter( - () => new LargeObjectWithParameterizedConstructorConverter(), - ConverterStrategy.Object); CreateObjectWithArgs = objectInfo.ObjectWithParameterizedConstructorCreator; CtorParamInitFunc = objectInfo.ConstructorParameterMetadataInitializer; } else { - converter = new JsonMetadataServicesConverter(() => new ObjectDefaultConverter(), ConverterStrategy.Object); SetCreateObjectFunc(objectInfo.ObjectCreator); } -#pragma warning restore CS8714 + PropInitFunc = objectInfo.PropertyMetadataInitializer; - ElementType = converter.ElementType; SerializeHandler = objectInfo.SerializeHandler; - PropertyInfoForTypeInfo = JsonMetadataServices.CreateJsonPropertyInfoForClassInfo(typeof(T), this, converter, Options); NumberHandling = objectInfo.NumberHandling; } @@ -64,23 +52,41 @@ public SourceGenJsonTypeInfo(JsonSerializerOptions options, JsonObjectInfoValues Func> converterCreator, object? createObjectWithArgs = null, object? addFunc = null) - : base(typeof(T), options) + : base(GetConverter(collectionInfo, converterCreator), options) { - ConverterStrategy strategy = collectionInfo.KeyInfo == null ? ConverterStrategy.Enumerable : ConverterStrategy.Dictionary; - JsonConverter converter = new JsonMetadataServicesConverter(converterCreator, strategy); - - KeyType = converter.KeyType; - ElementType = converter.ElementType; KeyTypeInfo = collectionInfo.KeyInfo; ElementTypeInfo = collectionInfo.ElementInfo ?? throw new ArgumentNullException(nameof(collectionInfo.ElementInfo)); NumberHandling = collectionInfo.NumberHandling; - PropertyInfoForTypeInfo = JsonMetadataServices.CreateJsonPropertyInfoForClassInfo(typeof(T), this, converter, options); SerializeHandler = collectionInfo.SerializeHandler; CreateObjectWithArgs = createObjectWithArgs; AddMethodDelegate = addFunc; SetCreateObjectFunc(collectionInfo.ObjectCreator); } + private static JsonConverter GetConverter(JsonObjectInfoValues objectInfo) + { +#pragma warning disable CS8714 + // The type cannot be used as type parameter in the generic type or method. + // Nullability of type argument doesn't match 'notnull' constraint. + if (objectInfo.ObjectWithParameterizedConstructorCreator != null) + { + return new JsonMetadataServicesConverter( + () => new LargeObjectWithParameterizedConstructorConverter(), + ConverterStrategy.Object); + } + else + { + return new JsonMetadataServicesConverter(() => new ObjectDefaultConverter(), ConverterStrategy.Object); + } +#pragma warning restore CS8714 + } + + private static JsonConverter GetConverter(JsonCollectionInfoValues collectionInfo, Func> converterCreator) + { + ConverterStrategy strategy = collectionInfo.KeyInfo == null ? ConverterStrategy.Enumerable : ConverterStrategy.Dictionary; + return new JsonMetadataServicesConverter(converterCreator, strategy); + } + internal override void LateAddProperties() { AddPropertiesUsingSourceGenInfo();