Refactor JsonPropertyInfo initialization infrastructure and implement…

… JsonPropertyInfo.AttributeProvider (#71514) * Implement JsonPropertyInfo.AttributeProvider and refactor initialization infrastructure * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs * Update src/libraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs * _shouldSerializeIsExplicitlySet set to false when setting IgnoreCondition * Reinstante _converterIsExternalAndPolymorphic * Move unique attribute resolution logic to reflection extensions * address feedback
dotnet · Jul 4, 2022 · 32d0360 · 32d0360
1 parent 7496163
commit 32d0360
Show file tree

Hide file tree

Showing 16 changed files with 337 additions and 493 deletions.
diff --git a/src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs b/src/libraries/System.Text.Json/src/System/ReflectionExtensions.cs
@@ -43,5 +43,24 @@ public static bool IsAssignableFromInternal(this Type type, Type from)
 
         private static bool HasJsonConstructorAttribute(ConstructorInfo constructorInfo)
             => constructorInfo.GetCustomAttribute<JsonConstructorAttribute>() != null;
+
+        public static TAttribute? GetUniqueCustomAttribute<TAttribute>(this MemberInfo memberInfo, bool inherit)
+            where TAttribute : Attribute
+        {
+            object[] attributes = memberInfo.GetCustomAttributes(typeof(TAttribute), inherit);
+
+            if (attributes.Length == 0)
+            {
+                return null;
+            }
+
+            if (attributes.Length == 1)
+            {
+                return (TAttribute)attributes[0];
+            }
+
+            ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateAttribute(typeof(TAttribute), memberInfo);
+            return null;
+        }
     }
 }
diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverter.cs
@@ -67,7 +67,7 @@ internal virtual void ReadElementAndSetProperty(
             throw new InvalidOperationException(SR.NodeJsonObjectCustomConverterNotAllowedOnExtensionProperty);
         }
 
-        internal abstract JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo parentTypeInfo);
+        internal abstract JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo declaringTypeInfo, Type propertyType, JsonSerializerOptions options);
 
         internal abstract JsonParameterInfo CreateJsonParameterInfo();
 

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterFactory.cs
@@ -32,7 +32,7 @@ protected JsonConverterFactory() { }
         /// </returns>
         public abstract JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options);
 
-        internal override JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo parentTypeInfo)
+        internal override JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo declaringTypeInfo, Type propertyType, JsonSerializerOptions options)
         {
             Debug.Fail("We should never get here.");
 

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonConverterOfT.cs
@@ -69,9 +69,9 @@ public override bool CanConvert(Type typeToConvert)
 
         internal override ConverterStrategy ConverterStrategy => ConverterStrategy.Value;
 
-        internal sealed override JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo parentTypeInfo)
+        internal sealed override JsonPropertyInfo CreateJsonPropertyInfo(JsonTypeInfo declaringTypeInfo, Type propertyType, JsonSerializerOptions options)
         {
-            return new JsonPropertyInfo<T>(parentTypeInfo);
+            return new JsonPropertyInfo<T>(declaringTypeInfo.Type, propertyType, declaringTypeInfo, options);
         }
 
         internal sealed override JsonParameterInfo CreateJsonParameterInfo()

diff --git a/...s/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs b/...s/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs
@@ -27,21 +27,15 @@ public sealed partial class JsonSerializerOptions
         public IList<JsonConverter> Converters => _converters;
 
         // This may return factory converter
-        internal JsonConverter? GetCustomConverterFromMember(Type? parentClassType, Type typeToConvert, MemberInfo? memberInfo)
+        internal JsonConverter? GetCustomConverterFromMember(Type typeToConvert, MemberInfo memberInfo)
         {
+            Debug.Assert(memberInfo.DeclaringType != null, "Properties and fields always have a declaring type.");
             JsonConverter? converter = null;
 
-            if (memberInfo != null)
+            JsonConverterAttribute? converterAttribute = memberInfo.GetUniqueCustomAttribute<JsonConverterAttribute>(inherit: false);
+            if (converterAttribute != null)
             {
-                Debug.Assert(parentClassType != null);
-
-                JsonConverterAttribute? converterAttribute = (JsonConverterAttribute?)
-                    GetAttributeThatCanHaveMultiple(parentClassType!, typeof(JsonConverterAttribute), memberInfo);
-
-                if (converterAttribute != null)
-                {
-                    converter = GetConverterFromAttribute(converterAttribute, typeToConvert, classTypeAttributeIsOn: parentClassType!, memberInfo);
-                }
+                converter = GetConverterFromAttribute(converterAttribute, typeToConvert, memberInfo);
             }
 
             return converter;
@@ -180,12 +174,10 @@ private JsonConverter GetConverterFromOptionsOrReflectionConverter(Type typeToCo
             // Priority 2: Attempt to get converter from [JsonConverter] on the type being converted.
             if (converter == null)
             {
-                JsonConverterAttribute? converterAttribute = (JsonConverterAttribute?)
-                    GetAttributeThatCanHaveMultiple(typeToConvert, typeof(JsonConverterAttribute));
-
+                JsonConverterAttribute? converterAttribute = typeToConvert.GetUniqueCustomAttribute<JsonConverterAttribute>(inherit: false);
                 if (converterAttribute != null)
                 {
-                    converter = GetConverterFromAttribute(converterAttribute, typeToConvert: typeToConvert, classTypeAttributeIsOn: typeToConvert, memberInfo: null);
+                    converter = GetConverterFromAttribute(converterAttribute, typeToConvert: typeToConvert, memberInfo: null);
                 }
             }
 
@@ -215,29 +207,30 @@ private JsonConverter GetConverterFromOptionsOrReflectionConverter(Type typeToCo
         // This suppression needs to be removed. https://github.com/dotnet/runtime/issues/68878
         [UnconditionalSuppressMessage("AotAnalysis", "IL3050:RequiresDynamicCode", Justification = "The factory constructors are only invoked in the context of reflection serialization code paths " +
             "and are marked RequiresDynamicCode")]
-        private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, Type classTypeAttributeIsOn, MemberInfo? memberInfo)
+        private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converterAttribute, Type typeToConvert, MemberInfo? memberInfo)
         {
             JsonConverter? converter;
 
-            Type? type = converterAttribute.ConverterType;
-            if (type == null)
+            Type declaringType = memberInfo?.DeclaringType ?? typeToConvert;
+            Type? converterType = converterAttribute.ConverterType;
+            if (converterType == null)
             {
                 // Allow the attribute to create the converter.
                 converter = converterAttribute.CreateConverter(typeToConvert);
                 if (converter == null)
                 {
-                    ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classTypeAttributeIsOn, memberInfo, typeToConvert);
+                    ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(declaringType, memberInfo, typeToConvert);
                 }
             }
             else
             {
-                ConstructorInfo? ctor = type.GetConstructor(Type.EmptyTypes);
-                if (!typeof(JsonConverter).IsAssignableFrom(type) || ctor == null || !ctor.IsPublic)
+                ConstructorInfo? ctor = converterType.GetConstructor(Type.EmptyTypes);
+                if (!typeof(JsonConverter).IsAssignableFrom(converterType) || ctor == null || !ctor.IsPublic)
                 {
-                    ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeInvalid(classTypeAttributeIsOn, memberInfo);
+                    ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeInvalid(declaringType, memberInfo);
                 }
 
-                converter = (JsonConverter)Activator.CreateInstance(type)!;
+                converter = (JsonConverter)Activator.CreateInstance(converterType)!;
             }
 
             Debug.Assert(converter != null);
@@ -255,38 +248,10 @@ private JsonConverter GetConverterFromAttribute(JsonConverterAttribute converter
                     return NullableConverterFactory.CreateValueConverter(underlyingType, converter);
                 }
 
-                ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classTypeAttributeIsOn, memberInfo, typeToConvert);
+                ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(declaringType, memberInfo, typeToConvert);
             }
 
             return converter;
         }
-
-        private static Attribute? GetAttributeThatCanHaveMultiple(Type classType, Type attributeType, MemberInfo memberInfo)
-        {
-            object[] attributes = memberInfo.GetCustomAttributes(attributeType, inherit: false);
-            return GetAttributeThatCanHaveMultiple(attributeType, classType, memberInfo, attributes);
-        }
-
-        internal static Attribute? GetAttributeThatCanHaveMultiple(Type classType, Type attributeType)
-        {
-            object[] attributes = classType.GetCustomAttributes(attributeType, inherit: false);
-            return GetAttributeThatCanHaveMultiple(attributeType, classType, null, attributes);
-        }
-
-        private static Attribute? GetAttributeThatCanHaveMultiple(Type attributeType, Type classType, MemberInfo? memberInfo, object[] attributes)
-        {
-            if (attributes.Length == 0)
-            {
-                return null;
-            }
-
-            if (attributes.Length == 1)
-            {
-                return (Attribute)attributes[0];
-            }
-
-            ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateAttribute(attributeType, classType, memberInfo);
-            return default;
-        }
     }
 }
diff --git a/...ies/System.Text.Json/src/System/Text/Json/Serialization/Metadata/CustomJsonTypeInfoOfT.cs b/...ies/System.Text.Json/src/System/Text/Json/Serialization/Metadata/CustomJsonTypeInfoOfT.cs
@@ -31,11 +31,7 @@ internal CustomJsonTypeInfo(JsonSerializerOptions options)
         private static JsonConverter GetConverter(JsonSerializerOptions options)
         {
             DefaultJsonTypeInfoResolver.RootDefaultInstance();
-            return GetEffectiveConverter(
-                    typeof(T),
-                    parentClassType: null, // A TypeInfo never has a "parent" class.
-                    memberInfo: null, // A TypeInfo never has a "parent" property.
-                    options);
+            return options.GetConverterForType(typeof(T));
         }
 
         internal override JsonParameterInfoValues[] GetParameterInfoValues()

diff --git a/...stem.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs b/...stem.Text.Json/src/System/Text/Json/Serialization/Metadata/DefaultJsonTypeInfoResolver.cs
@@ -50,7 +50,7 @@ public virtual JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options
 
             _mutable = false;
 
-            JsonTypeInfo.ValidateType(type, null, null, options);
+            JsonTypeInfo.ValidateType(type);
             JsonTypeInfo typeInfo = CreateJsonTypeInfo(type, options);
 
             if (_modifiers != null)

diff --git a/...ries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.cs b/...ries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonMetadataServices.cs
@@ -48,9 +48,7 @@ public static JsonPropertyInfo CreatePropertyInfo<T>(JsonSerializerOptions optio
                 throw new InvalidOperationException(SR.Format(SR.FieldCannotBeVirtual, nameof(propertyInfo.IsProperty), nameof(propertyInfo.IsVirtual)));
             }
 
-            JsonPropertyInfo<T> jsonPropertyInfo = new JsonPropertyInfo<T>(parentTypeInfo: null);
-            jsonPropertyInfo.InitializeForSourceGen(options, propertyInfo);
-            return jsonPropertyInfo;
+            return new JsonPropertyInfo<T>(options, propertyInfo);
         }
 
         /// <summary>