Support [DataMember] IsRequired in NewtonsoftDataContractResolver (#2644

)

* ✅ Add non-regression tests for DataMember required

As DataMember implementation comes after the implementation for [JsonProperty] and [JsonObject], we want to be sure that the current behavior is not bypassed.

* ✨Add support for [DataMember(IsRequired)]

Add test cases for DataMember (IsRequired and Name).
It seems that NewtonSoft's ResolveContract correctly set the Required, using [DataMember], [JsonProperpty] and [JsonRequired] so we can directly use it instead of the IsRequiredSpecifed extension method.

* ✨ Keep IsRequiredSpecified() but use JsonProperty.Required internally

Newtonsoft do all the work for us, handling [JsonProperty(Required = Required.Always)] or [JsonRequired] or [DataMember(IsRequired = true)] and filling the JsonProperty.Required field.

src/Swashbuckle.AspNetCore.Newtonsoft/SchemaGenerator/JsonPropertyExtensions.cs

@@ -17,16 +17,7 @@ public static bool TryGetMemberInfo(this JsonProperty jsonProperty, out MemberIn
 
         public static bool IsRequiredSpecified(this JsonProperty jsonProperty)
         {
-            if (!jsonProperty.TryGetMemberInfo(out MemberInfo memberInfo))
-                return false;
-
-            if (memberInfo.GetCustomAttribute<JsonRequiredAttribute>() != null)
-                return true;
-
-            var jsonPropertyAttributeData = memberInfo.GetCustomAttributesData()
-                .FirstOrDefault(attrData => attrData.AttributeType == typeof(JsonPropertyAttribute));
-
-            return (jsonPropertyAttributeData != null) && jsonPropertyAttributeData.NamedArguments.Any(arg => arg.MemberName == "Required");
+            return jsonProperty.Required != Required.Default;
         }
     }
 }

test/Swashbuckle.AspNetCore.Newtonsoft.Test/Fixtures/DataMemberAnnotatedType.cs

@@ -0,0 +1,20 @@
+using System.Runtime.Serialization;
+
+namespace Swashbuckle.AspNetCore.Newtonsoft.Test
+{
+    [DataContract(Name = "CustomNameFromDataContract")]
+    public class DataMemberAnnotatedType
+    {
+        [DataMember(IsRequired = true)]
+        public string StringWithDataMemberRequired { get; set; }
+
+        [DataMember(IsRequired = false)]
+        public string StringWithDataMemberNonRequired { get; set; }
+
+        [DataMember(IsRequired = true, Name = "RequiredWithCustomNameFromDataMember")]
+        public string StringWithDataMemberRequiredWithCustomName { get; set; }
+
+        [DataMember(IsRequired = false, Name = "NonRequiredWithCustomNameFromDataMember")]
+        public string StringWithDataMemberNonRequiredWithCustomName { get; set; }
+    }
+}

test/Swashbuckle.AspNetCore.Newtonsoft.Test/Fixtures/JsonObjectAnnotatedType.cs

@@ -1,8 +1,10 @@
-using Newtonsoft.Json;
+using System.Runtime.Serialization;
+using Newtonsoft.Json;
 
 namespace Swashbuckle.AspNetCore.Newtonsoft.Test
 {
     [JsonObject(ItemRequired = Required.Always)]
+    [DataContract]
     public class JsonObjectAnnotatedType
     {
         public string StringWithNoAnnotation { get; set; }
@@ -12,5 +14,8 @@ public class JsonObjectAnnotatedType
 
         [JsonProperty(Required = Required.AllowNull)]
         public string StringWithRequiredAllowNull { get; set; }
+
+        [DataMember(IsRequired = false)]
+        public string StringWithDataMemberRequiredFalse { get; set; }
     }
 }

test/Swashbuckle.AspNetCore.Newtonsoft.Test/Fixtures/JsonPropertyAnnotatedType.cs

@@ -1,7 +1,9 @@
-using Newtonsoft.Json;
+using System.Runtime.Serialization;
+using Newtonsoft.Json;
 
 namespace Swashbuckle.AspNetCore.Newtonsoft.Test
 {
+    [DataContract]
     public class JsonPropertyAnnotatedType
     {
         [JsonProperty("string-with-json-property-name")]
@@ -21,5 +23,13 @@ public class JsonPropertyAnnotatedType
 
         [JsonProperty(Required = Required.AllowNull)]
         public string StringWithRequiredAllowNull { get; set; }
+
+        [DataMember(IsRequired = false)] //As the support for DataMember has been implemented later, JsonProperty.Required should take precedence
+        [JsonProperty(Required = Required.Always)]
+        public string StringWithRequiredAlwaysButConflictingDataMember { get; set; }
+
+        [DataMember(IsRequired = true)] //As the support for DataMember has been implemented later, JsonProperty.Required should take precedence
+        [JsonProperty(Required = Required.Default)]
+        public string StringWithRequiredDefaultButConflictingDataMember { get; set; }
     }
-}
+}

test/Swashbuckle.AspNetCore.Newtonsoft.Test/Fixtures/JsonRequiredAnnotatedType.cs

@@ -1,10 +1,16 @@
-using Newtonsoft.Json;
+using System.Runtime.Serialization;
+using Newtonsoft.Json;
 
 namespace Swashbuckle.AspNetCore.Newtonsoft.Test
 {
+    [DataContract]
     public class JsonRequiredAnnotatedType
     {
         [JsonRequired]
         public string StringWithJsonRequired { get; set; }
+
+        [DataMember(IsRequired = false)] //As the support for DataMember has been implemented later, JsonRequired should take precedence
+        [JsonRequired]
+        public string StringWithConflictingRequired { get; set; }
     }
 }

test/Swashbuckle.AspNetCore.Newtonsoft.Test/SchemaGenerator/NewtonsoftSchemaGeneratorTests.cs

@@ -755,14 +755,17 @@ public void GenerateSchema_HonorsSerializerAttribute_JsonProperty()
                     "StringWithRequiredDisallowNull",
                     "StringWithRequiredAlways",
                     "StringWithRequiredAllowNull",
+                    "StringWithRequiredAlwaysButConflictingDataMember",
+                    "StringWithRequiredDefaultButConflictingDataMember"
                 },
                 schema.Properties.Keys.ToArray()
             );
             Assert.Equal(
                 new[]
                 {
                     "StringWithRequiredAllowNull",
-                    "StringWithRequiredAlways"
+                    "StringWithRequiredAlways",
+                    "StringWithRequiredAlwaysButConflictingDataMember"
                 },
                 schema.Required.ToArray()
             );
@@ -772,6 +775,8 @@ public void GenerateSchema_HonorsSerializerAttribute_JsonProperty()
             Assert.False(schema.Properties["StringWithRequiredDisallowNull"].Nullable);
             Assert.False(schema.Properties["StringWithRequiredAlways"].Nullable);
             Assert.True(schema.Properties["StringWithRequiredAllowNull"].Nullable);
+            Assert.False(schema.Properties["StringWithRequiredAlwaysButConflictingDataMember"].Nullable);
+            Assert.True(schema.Properties["StringWithRequiredDefaultButConflictingDataMember"].Nullable);
         }
 
         [Fact]
@@ -782,7 +787,7 @@ public void GenerateSchema_HonorsSerializerAttribute_JsonRequired()
             var referenceSchema = Subject().GenerateSchema(typeof(JsonRequiredAnnotatedType), schemaRepository);
 
             var schema = schemaRepository.Schemas[referenceSchema.Reference.Id];
-            Assert.Equal(new[] { "StringWithJsonRequired" }, schema.Required.ToArray());
+            Assert.Equal(new[] { "StringWithConflictingRequired", "StringWithJsonRequired"}, schema.Required.ToArray());
             Assert.False(schema.Properties["StringWithJsonRequired"].Nullable);
         }
 
@@ -797,6 +802,7 @@ public void GenerateSchema_HonorsSerializerAttribute_JsonObject()
             Assert.Equal(
                 new[]
                 {
+                    "StringWithDataMemberRequiredFalse",
                     "StringWithNoAnnotation",
                     "StringWithRequiredAllowNull",
                     "StringWithRequiredUnspecified"
@@ -806,6 +812,7 @@ public void GenerateSchema_HonorsSerializerAttribute_JsonObject()
             Assert.False(schema.Properties["StringWithNoAnnotation"].Nullable);
             Assert.False(schema.Properties["StringWithRequiredUnspecified"].Nullable);
             Assert.True(schema.Properties["StringWithRequiredAllowNull"].Nullable);
+            Assert.False(schema.Properties["StringWithDataMemberRequiredFalse"].Nullable);
         }
 
         [Fact]
@@ -821,6 +828,43 @@ public void GenerateSchema_HonorsSerializerAttribute_JsonExtensionData()
             Assert.Null(schema.AdditionalProperties.Type);
         }
 
+        [Fact]
+        public void GenerateSchema_HonorsDataMemberAttribute()
+        {
+            var schemaRepository = new SchemaRepository();
+
+            var referenceSchema = Subject().GenerateSchema(typeof(DataMemberAnnotatedType), schemaRepository);
+
+            var schema = schemaRepository.Schemas[referenceSchema.Reference.Id];
+
+
+            Assert.True(schema.Properties["StringWithDataMemberRequired"].Nullable);
+            Assert.True(schema.Properties["StringWithDataMemberNonRequired"].Nullable);
+            Assert.True(schema.Properties["RequiredWithCustomNameFromDataMember"].Nullable);
+            Assert.True(schema.Properties["NonRequiredWithCustomNameFromDataMember"].Nullable);
+
+            Assert.Equal(
+                new[]
+                {
+
+                    "StringWithDataMemberRequired",
+                    "StringWithDataMemberNonRequired",
+                    "RequiredWithCustomNameFromDataMember",
+                    "NonRequiredWithCustomNameFromDataMember"
+                },
+                schema.Properties.Keys.ToArray()
+            );
+
+            Assert.Equal(
+                new[]
+                {
+                    "RequiredWithCustomNameFromDataMember",
+                    "StringWithDataMemberRequired"
+                },
+                schema.Required.ToArray()
+            );
+        }
+
         [Theory]
         [InlineData(typeof(ProblemDetails))]
         [InlineData(typeof(ValidationProblemDetails))]