JSON support required properties (#73063)

* Support RequiredAttribute for reflection serialization * Re-use existing tests for source gen * Source-gen support and address feedback * polymorphic cases testing, address other feedback
dotnet · Aug 2, 2022 · 18d0367 · 18d0367
1 parent b705ed9
commit 18d0367
Show file tree

Hide file tree

Showing 12 changed files with 853 additions and 37 deletions.
diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Emitter.cs
@@ -29,6 +29,7 @@ private sealed partial class Emitter
             private const string DefaultContextBackingStaticVarName = "s_defaultContext";
             internal const string GetConverterFromFactoryMethodName = "GetConverterFromFactory";
             private const string InfoVarName = "info";
+            private const string PropertyInfoVarName = "propertyInfo";
             internal const string JsonContextVarName = "jsonContext";
             private const string NumberHandlingPropName = "NumberHandling";
             private const string ObjectCreatorPropName = "ObjectCreator";
@@ -709,6 +710,7 @@ private static string GeneratePropMetadataInitFunc(TypeGenerationSpec typeGenera
                     string memberTypeCompilableName = memberTypeMetadata.TypeRef;
 
                     string infoVarName = $"{InfoVarName}{i}";
+                    string propertyInfoVarName = $"{PropertyInfoVarName}{i}";
 
                     sb.Append($@"
     {JsonPropertyInfoValuesTypeRef}<{memberTypeCompilableName}> {infoVarName} = new {JsonPropertyInfoValuesTypeRef}<{memberTypeCompilableName}>()
@@ -728,7 +730,16 @@ private static string GeneratePropMetadataInitFunc(TypeGenerationSpec typeGenera
         JsonPropertyName = {jsonPropertyNameValue}
     }};
 
-    {PropVarName}[{i}] = {JsonMetadataServicesTypeRef}.CreatePropertyInfo<{memberTypeCompilableName}>({OptionsLocalVariableName}, {infoVarName});
+    {JsonPropertyInfoTypeRef} {propertyInfoVarName} = {JsonMetadataServicesTypeRef}.CreatePropertyInfo<{memberTypeCompilableName}>({OptionsLocalVariableName}, {infoVarName});");
+
+                    if (memberMetadata.IsRequired)
+                    {
+                        sb.Append($@"
+    {propertyInfoVarName}.IsRequired = true;");
+                    }
+
+                    sb.Append($@"
+    {PropVarName}[{i}] = {propertyInfoVarName};
 ");
                 }
 

diff --git a/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs b/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs
@@ -40,6 +40,7 @@ private sealed class Parser
             private const string JsonNumberHandlingAttributeFullName = "System.Text.Json.Serialization.JsonNumberHandlingAttribute";
             private const string JsonPropertyNameAttributeFullName = "System.Text.Json.Serialization.JsonPropertyNameAttribute";
             private const string JsonPropertyOrderAttributeFullName = "System.Text.Json.Serialization.JsonPropertyOrderAttribute";
+            private const string JsonRequiredAttributeFullName = "System.Text.Json.Serialization.JsonRequiredAttribute";
             private const string JsonSerializerContextFullName = "System.Text.Json.Serialization.JsonSerializerContext";
             private const string JsonSourceGenerationOptionsAttributeFullName = "System.Text.Json.Serialization.JsonSourceGenerationOptionsAttribute";
 
@@ -1197,7 +1198,8 @@ private PropertyGenerationSpec GetPropertyGenerationSpec(
                     out string? converterInstantiationLogic,
                     out int order,
                     out bool hasFactoryConverter,
-                    out bool isExtensionData);
+                    out bool isExtensionData,
+                    out bool isRequired);
 
                 ProcessMember(
                     memberInfo,
@@ -1229,6 +1231,7 @@ private PropertyGenerationSpec GetPropertyGenerationSpec(
                     IsProperty = memberInfo.MemberType == MemberTypes.Property,
                     IsPublic = isPublic,
                     IsVirtual = isVirtual,
+                    IsRequired = isRequired,
                     JsonPropertyName = jsonPropertyName,
                     RuntimePropertyName = runtimePropertyName,
                     PropertyNameVarName = propertyNameVarName,
@@ -1275,7 +1278,8 @@ private void ProcessMemberCustomAttributes(
                 out string? converterInstantiationLogic,
                 out int order,
                 out bool hasFactoryConverter,
-                out bool isExtensionData)
+                out bool isExtensionData,
+                out bool isRequired)
             {
                 hasJsonInclude = false;
                 jsonPropertyName = null;
@@ -1284,6 +1288,7 @@ private void ProcessMemberCustomAttributes(
                 converterInstantiationLogic = null;
                 order = 0;
                 isExtensionData = false;
+                isRequired = false;
 
                 bool foundDesignTimeCustomConverter = false;
                 hasFactoryConverter = false;
@@ -1350,6 +1355,11 @@ private void ProcessMemberCustomAttributes(
                                     isExtensionData = true;
                                 }
                                 break;
+                            case JsonRequiredAttributeFullName:
+                                {
+                                    isRequired = true;
+                                }
+                                break;
                             default:
                                 break;
                         }

diff --git a/src/libraries/System.Text.Json/gen/PropertyGenerationSpec.cs b/src/libraries/System.Text.Json/gen/PropertyGenerationSpec.cs
@@ -30,6 +30,11 @@ internal sealed class PropertyGenerationSpec
 
         public bool IsVirtual { get; init; }
 
+        /// <summary>
+        /// The property has JsonRequiredAttribute.
+        /// </summary>
+        public bool IsRequired { get; init; }
+
         /// <summary>
         /// The property name specified via JsonPropertyNameAttribute, if available.
         /// </summary>

diff --git a/src/libraries/System.Text.Json/ref/System.Text.Json.cs b/src/libraries/System.Text.Json/ref/System.Text.Json.cs
@@ -944,6 +944,11 @@ public sealed partial class JsonPropertyOrderAttribute : System.Text.Json.Serial
         public JsonPropertyOrderAttribute(int order) { }
         public int Order { get { throw null; } }
     }
+    [System.AttributeUsageAttribute(System.AttributeTargets.Field | System.AttributeTargets.Property, AllowMultiple = false)]
+    public sealed partial class JsonRequiredAttribute : System.Text.Json.Serialization.JsonAttribute
+    {
+        public JsonRequiredAttribute() { }
+    }
     [System.AttributeUsageAttribute(System.AttributeTargets.Class, AllowMultiple=true)]
     public sealed partial class JsonSerializableAttribute : System.Text.Json.Serialization.JsonAttribute
     {
@@ -1152,6 +1157,7 @@ internal JsonPropertyInfo() { }
         public System.Text.Json.Serialization.JsonConverter? CustomConverter { get { throw null; } set { } }
         public System.Func<object, object?>? Get { get { throw null; } set { } }
         public bool IsExtensionData { get { throw null; } set { } }
+        public bool IsRequired { get { throw null; } set { } }
         public string Name { get { throw null; } set { } }
         public System.Text.Json.Serialization.JsonNumberHandling? NumberHandling { get { throw null; } set { } }
         public System.Text.Json.JsonSerializerOptions Options { get { throw null; } }

diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj
@@ -100,6 +100,7 @@ The System.Text.Json library is built-in as part of the shared framework in .NET
     <Compile Include="System\Text\Json\Serialization\Attributes\JsonNumberHandlingAttribute.cs" />
     <Compile Include="System\Text\Json\Serialization\Attributes\JsonPolymorphicAttribute.cs" />
     <Compile Include="System\Text\Json\Serialization\Attributes\JsonPropertyNameAttribute.cs" />
+    <Compile Include="System\Text\Json\Serialization\Attributes\JsonRequiredAttribute.cs" />
     <Compile Include="System\Text\Json\Serialization\Attributes\JsonPropertyOrderAttribute.cs" />
     <Compile Include="System\Text\Json\Serialization\Converters\CastingConverter.cs" />
     <Compile Include="System\Text\Json\Serialization\Converters\Collection\ImmutableDictionaryOfTKeyTValueConverterWithReflection.cs" />

diff --git a/...s/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonRequiredAttribute.cs b/...s/System.Text.Json/src/System/Text/Json/Serialization/Attributes/JsonRequiredAttribute.cs
@@ -0,0 +1,24 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Text.Json.Serialization.Metadata;
+
+namespace System.Text.Json.Serialization
+{
+    /// <summary>
+    /// Indicates that the annotated member must bind to a JSON property on deserialization.
+    /// </summary>
+    /// <remarks>
+    /// <see langword="null"/> token in JSON will not trigger a validation error.
+    /// For contracts originating from <see cref="DefaultJsonTypeInfoResolver"/> or <see cref="JsonSerializerContext"/>,
+    /// this attribute will be mapped to <see cref="JsonPropertyInfo.IsRequired"/>.
+    /// </remarks>
+    [AttributeUsage(AttributeTargets.Property | AttributeTargets.Field, AllowMultiple = false)]
+    public sealed class JsonRequiredAttribute : JsonAttribute
+    {
+        /// <summary>
+        /// Initializes a new instance of <see cref="JsonRequiredAttribute"/>.
+        /// </summary>
+        public JsonRequiredAttribute() { }
+    }
+}
diff --git a/...ibraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs b/...ibraries/System.Text.Json/src/System/Text/Json/Serialization/Metadata/JsonPropertyInfo.cs
@@ -203,7 +203,21 @@ public bool IsExtensionData
 
         private bool _isExtensionDataProperty;
 
-        internal bool IsRequired
+        /// <summary>
+        /// Specifies whether the current property is required for deserialization to be successful.
+        /// </summary>
+        /// <exception cref="InvalidOperationException">
+        /// The <see cref="JsonPropertyInfo"/> instance has been locked for further modification.
+        /// </exception>
+        /// <remarks>
+        /// For contracts originating from <see cref="DefaultJsonTypeInfoResolver"/> or <see cref="JsonSerializerContext"/>,
+        /// the value of this property will be mapped from <see cref="JsonRequiredAttribute"/> annotations.
+        ///
+        /// For contracts using <see cref="DefaultJsonTypeInfoResolver"/>, properties using the <see langword="required"/> keyword
+        /// will also map to this setting, unless deserialization uses a SetsRequiredMembersAttribute on a constructor that populates all required properties.
+        /// <see langword="required"/> keyword is currently not supported in <see cref="JsonSerializerContext"/> contracts.
+        /// </remarks>
+        public bool IsRequired
         {
             get => _isRequired;
             set
@@ -506,10 +520,8 @@ private bool NumberHandingIsApplicable()
 
         private void DetermineIsRequired(MemberInfo memberInfo, bool shouldCheckForRequiredKeyword)
         {
-            if (shouldCheckForRequiredKeyword && memberInfo.HasRequiredMemberAttribute())
-            {
-                IsRequired = true;
-            }
+            IsRequired = memberInfo.GetCustomAttribute<JsonRequiredAttribute>(inherit: false) != null
+                || (shouldCheckForRequiredKeyword && memberInfo.HasRequiredMemberAttribute());
         }
 
         internal abstract bool GetMemberAndWriteJson(object obj, ref WriteStack state, Utf8JsonWriter writer);

diff --git a/....Text.Json/tests/System.Text.Json.Tests/Serialization/JsonSerializerWrapper.Reflection.cs b/....Text.Json/tests/System.Text.Json.Tests/Serialization/JsonSerializerWrapper.Reflection.cs
@@ -646,10 +646,9 @@ public static JsonSerializerOptions ResolveOptionsInstanceWithSmallBuffer(JsonSe
 
                 JsonSerializerOptions smallBufferCopy = new JsonSerializerOptions(options)
                 {
-                    // Copy the resolver explicitly until https://github.com/dotnet/aspnetcore/issues/38720 is resolved.
-                    TypeInfoResolver = options.TypeInfoResolver,
                     DefaultBufferSize = 1,
                 };
+
                 s_smallBufferMap.Add(options, smallBufferCopy);
                 return smallBufferCopy;
             }