Skip to content

Commit 5fb3b7a

Browse files
William-FroelichWilliam-Froelich
authored
Fix handling of 'Required' fields for Newtonsoft (Azure#506)
1 parent 5a64b15 commit 5fb3b7a

File tree

9 files changed

+100
-8
lines changed

9 files changed

+100
-8
lines changed

‎docs/openapi-core.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ public class MyOpenApiConfigurationOptions : IOpenApiConfigurationOptions
117117
```
118118

119119
> **NOTE**:
120-
>
120+
>
121121
> * If no base URL is declared, the Azure Functions app's URL will be added as a default.
122122
> * The OpenAPI v2 (Swagger) document only shows the the first server name on both UI and document, while the OpenAPI v3 document shows the first server name on the UI and all server names on the document.
123123
@@ -869,7 +869,7 @@ Properties decorated with the `JsonIgnoreAttribute` attribute class will not be
869869

870870
### `JsonPropertyAttribute` ###
871871

872-
Properties decorated with `JsonPropertyAttribute` attribute class will use `JsonProperty.Name` value instead of their property names. In addition to this, if `JsonProperty.Required` property has `Required.Always` or `Required.DisallowNull`, the property will be recognised as the `required` field.
872+
Properties decorated with `JsonPropertyAttribute` attribute class will use `JsonProperty.Name` value instead of their property names. In addition to this, if `JsonProperty.Required` property has `Required.Always` or `Required.AllowNull`, the property will be recognised as the `required` field.
873873

874874

875875
### `JsonRequiredAttribute` ###

‎src/Microsoft.Azure.WebJobs.Extensions.OpenApi.Core/Visitors/ObjectTypeVisitor.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ private void ProcessProperties(IOpenApiSchemaAcceptor instance, string schemaNam
189189
// Add required properties to schema.
190190
var jsonPropertyAttributes = properties.Where(p => !p.Value.GetCustomAttribute<JsonPropertyAttribute>(inherit: false).IsNullOrDefault())
191191
.Select(p => new KeyValuePair<string, JsonPropertyAttribute>(p.Key, p.Value.GetCustomAttribute<JsonPropertyAttribute>(inherit: false)))
192-
.Where(p => p.Value.Required == Required.Always || p.Value.Required == Required.DisallowNull);
192+
.Where(p => p.Value.Required == Required.Always || p.Value.Required == Required.AllowNull);
193193
foreach (var attribute in jsonPropertyAttributes)
194194
{
195195
instance.Schemas[schemaName].Required.Add(attribute.Key);

‎src/Microsoft.Azure.WebJobs.Extensions.OpenApi.Core/Visitors/OpenApiSchemaAcceptor.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using Microsoft.Azure.WebJobs.Extensions.OpenApi.Core.Extensions;
99
using Microsoft.OpenApi.Models;
1010

11+
using Newtonsoft.Json;
1112
using Newtonsoft.Json.Serialization;
1213

1314
namespace Microsoft.Azure.WebJobs.Extensions.OpenApi.Core.Visitors
@@ -63,6 +64,7 @@ public void Accept(VisitorCollection collection, NamingStrategy namingStrategy)
6364
property.Value.GetCustomAttribute<OpenApiPropertyAttribute>(inherit: false),
6465
};
6566
attributes.AddRange(property.Value.GetCustomAttributes<ValidationAttribute>(inherit: false));
67+
attributes.AddRange(property.Value.GetCustomAttributes<JsonPropertyAttribute>(inherit: false));
6668

6769
foreach (var visitor in collection.Visitors)
6870
{

‎src/Microsoft.Azure.WebJobs.Extensions.OpenApi.Core/Visitors/RecursiveObjectTypeVisitor.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ private void ProcessProperties(IOpenApiSchemaAcceptor instance, string schemaNam
195195
// Add required properties to schema.
196196
var jsonPropertyAttributes = properties.Where(p => !p.Value.GetCustomAttribute<JsonPropertyAttribute>(inherit: false).IsNullOrDefault())
197197
.Select(p => new KeyValuePair<string, JsonPropertyAttribute>(p.Key, p.Value.GetCustomAttribute<JsonPropertyAttribute>(inherit: false)))
198-
.Where(p => p.Value.Required == Required.Always || p.Value.Required == Required.DisallowNull);
198+
.Where(p => p.Value.Required == Required.Always || p.Value.Required == Required.AllowNull);
199199
foreach (var attribute in jsonPropertyAttributes)
200200
{
201201
instance.Schemas[schemaName].Required.Add(attribute.Key);

‎src/Microsoft.Azure.WebJobs.Extensions.OpenApi.Core/Visitors/TypeVisitor.cs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using Microsoft.OpenApi.Any;
1010
using Microsoft.OpenApi.Models;
1111

12+
using Newtonsoft.Json;
1213
using Newtonsoft.Json.Serialization;
1314

1415
namespace Microsoft.Azure.WebJobs.Extensions.OpenApi.Core.Visitors
@@ -163,6 +164,12 @@ protected string Visit(IAcceptor acceptor, string name, string title, string dat
163164
schema.Extensions.Add("x-ms-visibility", extension);
164165
}
165166

167+
attr = attributes.OfType<JsonPropertyAttribute>().SingleOrDefault();
168+
if (!attr.IsNullOrDefault())
169+
{
170+
schema.Nullable = this.GetNewtonsoftPropertNullable(attr as JsonPropertyAttribute);
171+
}
172+
166173
schema.ApplyValidationAttributes(attributes.OfType<ValidationAttribute>());
167174
}
168175

@@ -375,7 +382,6 @@ protected string GetOpenApiPropertyDescription(OpenApiPropertyAttribute attr)
375382
return attr.Description;
376383
}
377384

378-
379385
/// <summary>
380386
/// Gets the property deprecated.
381387
/// </summary>
@@ -385,5 +391,15 @@ protected bool GetOpenApiPropertyDeprecated(OpenApiPropertyAttribute attr)
385391
{
386392
return attr.Deprecated;
387393
}
394+
395+
/// <summary>
396+
/// Gets whether Newtonsoft Serialization allows nullability based on the JsonPropertyAttribute
397+
/// </summary>
398+
/// <param name="attr"><see cref="JsonPropertyAttribute"/> instance.</param>
399+
/// <returns>Returns the property deprecated.</returns>
400+
private bool GetNewtonsoftPropertNullable(JsonPropertyAttribute attr)
401+
{
402+
return attr.Required == Required.AllowNull || attr.Required == Required.Default;
403+
}
388404
}
389405
}

‎test/Microsoft.Azure.WebJobs.Extensions.OpenApi.Core.Tests.Fakes/FakeModel.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ public class FakeModel
2424
[JsonProperty(Required = Required.Default)]
2525
public string FakePropertyNoPropertyValue { get; set; }
2626

27+
[JsonProperty(Required = Required.AllowNull)]
28+
public string FakePropertyRequiredAllowNullPropertyValue { get; set; }
29+
30+
[JsonProperty(Required = Required.DisallowNull)]
31+
public string FakePropertyRequiredDisallowAllowNullPropertyValue { get; set; }
32+
2733
[JsonProperty]
2834
public string FakePropertyNoAnnotation { get; set; }
2935

‎test/Microsoft.Azure.WebJobs.Extensions.OpenApi.Core.Tests.Fakes/FakeRecursiveModel.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,17 @@ public class FakeRecursiveModel
1515
public FakeOtherClassModel FirstValue { get; set; }
1616

1717
public FakeOtherClassModel SecondValue { get; set; }
18+
19+
[JsonProperty(Required = Required.Always)]
20+
public string FakeAlwaysRequiredProperty { get; set; }
21+
22+
[JsonProperty(Required = Required.Default)]
23+
public string FakePropertyNoPropertyValue { get; set; }
24+
25+
[JsonProperty(Required = Required.AllowNull)]
26+
public string FakePropertyRequiredAllowNullPropertyValue { get; set; }
27+
28+
[JsonProperty(Required = Required.DisallowNull)]
29+
public string FakePropertyRequiredDisallowAllowNullPropertyValue { get; set; }
1830
}
1931
}

‎test/Microsoft.Azure.WebJobs.Extensions.OpenApi.Core.Tests/Visitors/ObjectTypeVisitorTests.cs

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
using Microsoft.OpenApi.Models;
1515
using Microsoft.VisualStudio.TestTools.UnitTesting;
1616

17+
using Newtonsoft.Json;
1718
using Newtonsoft.Json.Serialization;
1819

1920
namespace Microsoft.Azure.WebJobs.Extensions.OpenApi.Core.Tests.Visitors
@@ -84,9 +85,9 @@ public void Given_Type_When_IsPayloadVisitable_Invoked_Then_It_Should_Return_Res
8485
}
8586

8687
[DataTestMethod]
87-
[DataRow(typeof(FakeModel), "object", null, 2, 3, "fakeModel")]
88+
[DataRow(typeof(FakeModel), "object", null, 3, 3, "fakeModel")]
8889
[DataRow(typeof(FakeRequiredModel), "object", null, 1, 0, "fakeRequiredModel")]
89-
[DataRow(typeof(FakeRecursiveModel), "object", null, 1, 2, "fakeRecursiveModel")]
90+
[DataRow(typeof(FakeRecursiveModel), "object", null, 3, 2, "fakeRecursiveModel")]
9091
public void Given_Type_When_Visit_Invoked_Then_It_Should_Return_Result(Type objectType, string dataType, string dataFormat, int requiredCount, int rootSchemaCount, string referenceId)
9192
{
9293
var name = "hello";
@@ -125,6 +126,46 @@ public void Given_OpenApiPropertyAttribute_When_Visit_Invoked_Then_It_Should_Ret
125126
acceptor.Schemas[name].Description.Should().Be(description);
126127
}
127128

129+
[DataTestMethod]
130+
[DataRow("hello", 3)]
131+
public void Given_ObjectModel_With_NewtonsoftJsonPropertyAttribute_When_Visit_Invoked_Then_It_Should_Set_Required_And_Nullability(string name, int requiredCount)
132+
{
133+
var acceptor = new OpenApiSchemaAcceptor();
134+
var type = new KeyValuePair<string, Type>(name, typeof(FakeModel));
135+
136+
this._visitor.Visit(acceptor, type, this._strategy);
137+
138+
acceptor.Schemas[name].Required.Count.Should().Be(requiredCount);
139+
acceptor.Schemas[name].Required.Should().Contain("fakeProperty");
140+
acceptor.Schemas[name].Required.Should().Contain("anotherJsonFakeProperty");
141+
acceptor.Schemas[name].Required.Should().Contain("fakePropertyRequiredAllowNullPropertyValue");
142+
143+
acceptor.Schemas[name].Properties["anotherJsonFakeProperty"].Nullable.Should().BeFalse();
144+
acceptor.Schemas[name].Properties["fakePropertyNoPropertyValue"].Nullable.Should().BeTrue();
145+
acceptor.Schemas[name].Properties["fakePropertyRequiredAllowNullPropertyValue"].Nullable.Should().BeTrue();
146+
acceptor.Schemas[name].Properties["fakePropertyRequiredDisallowAllowNullPropertyValue"].Nullable.Should().BeFalse();
147+
}
148+
149+
[DataTestMethod]
150+
[DataRow("hello", 3)]
151+
public void Given_RecursiveModel_With_NewtonsoftJsonPropertyAttribute_When_Visit_Invoked_Then_It_Should_Set_Required_And_Nullability(string name, int requiredCount)
152+
{
153+
var acceptor = new OpenApiSchemaAcceptor();
154+
var type = new KeyValuePair<string, Type>(name, typeof(FakeRecursiveModel));
155+
156+
this._visitor.Visit(acceptor, type, this._strategy);
157+
158+
acceptor.Schemas[name].Required.Count.Should().Be(requiredCount);
159+
acceptor.Schemas[name].Required.Should().Contain("stringValue");
160+
acceptor.Schemas[name].Required.Should().Contain("fakeAlwaysRequiredProperty");
161+
acceptor.Schemas[name].Required.Should().Contain("fakePropertyRequiredAllowNullPropertyValue");
162+
163+
acceptor.Schemas[name].Properties["fakeAlwaysRequiredProperty"].Nullable.Should().BeFalse();
164+
acceptor.Schemas[name].Properties["fakePropertyNoPropertyValue"].Nullable.Should().BeTrue();
165+
acceptor.Schemas[name].Properties["fakePropertyRequiredAllowNullPropertyValue"].Nullable.Should().BeTrue();
166+
acceptor.Schemas[name].Properties["fakePropertyRequiredDisallowAllowNullPropertyValue"].Nullable.Should().BeFalse();
167+
}
168+
128169
[DataTestMethod]
129170
[DataRow("hello", OpenApiVisibilityType.Advanced)]
130171
[DataRow("hello", OpenApiVisibilityType.Important)]

‎test/Microsoft.Azure.WebJobs.Extensions.OpenApi.Core.Tests/Visitors/RecursiveObjectTypeVisitorTests.cs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public void Given_Type_When_IsPayloadVisitable_Invoked_Then_It_Should_Return_Res
7272
}
7373

7474
[DataTestMethod]
75-
[DataRow(typeof(FakeRecursiveModel), "object", null, 1, 0, "fakeRecursiveModel")]
75+
[DataRow(typeof(FakeRecursiveModel), "object", null, 3, 0, "fakeRecursiveModel")]
7676
public void Given_Type_When_Visit_Invoked_Then_It_Should_Return_Result(Type objectType, string dataType, string dataFormat, int requiredCount, int rootSchemaCount, string referenceId)
7777
{
7878
var name = "hello";
@@ -93,6 +93,21 @@ public void Given_Type_When_Visit_Invoked_Then_It_Should_Return_Result(Type obje
9393
acceptor.Schemas[name].Reference.Id.Should().Be(referenceId);
9494
}
9595

96+
[DataTestMethod]
97+
[DataRow("hello", 3)]
98+
public void Given_NewtonsoftJsonPropertyAttribute_When_Visit_Invoked_Then_It_Should_Set_Required(string name, int requiredCount)
99+
{
100+
var acceptor = new OpenApiSchemaAcceptor();
101+
var type = new KeyValuePair<string, Type>(name, typeof(FakeRecursiveModel));
102+
103+
this._visitor.Visit(acceptor, type, this._strategy);
104+
105+
acceptor.Schemas[name].Required.Count.Should().Be(requiredCount);
106+
acceptor.Schemas[name].Required.Should().Contain("stringValue");
107+
acceptor.Schemas[name].Required.Should().Contain("fakeAlwaysRequiredProperty");
108+
acceptor.Schemas[name].Required.Should().Contain("fakePropertyRequiredAllowNullPropertyValue");
109+
}
110+
96111
[DataTestMethod]
97112
[DataRow("hello", "lorem ipsum")]
98113
public void Given_OpenApiPropertyAttribute_When_Visit_Invoked_Then_It_Should_Return_Result(string name, string description)

0 commit comments

Comments
 (0)