Skip to content

Commit fb6cecc

Browse files
baywetbaywet
authored
Merge pull request #2686 from microsoft/fix/discriminator-required-validation-any-allof
fix: discriminator property validation fails any/allOf cases when it shouldn't
2 parents 6fce1ee + a8fb81c commit fb6cecc

3 files changed

Lines changed: 126 additions & 18 deletions

File tree

‎src/Microsoft.OpenApi/Validations/Rules/OpenApiSchemaRules.cs‎

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
namespace Microsoft.OpenApi
77
{
8+
using System;
9+
using System.ComponentModel;
810
using System.Linq;
911

1012
/// <summary>
@@ -48,6 +50,7 @@ public static class OpenApiSchemaRules
4850
{
4951
var discriminatorName = schema.Discriminator?.PropertyName;
5052

53+
#pragma warning disable CS0618 // Type or member is obsolete
5154
if (!ValidateChildSchemaAgainstDiscriminator(schema, discriminatorName))
5255
{
5356
context.Enter("discriminator");
@@ -56,6 +59,7 @@ public static class OpenApiSchemaRules
5659
schema is OpenApiSchemaReference { Reference: not null} schemaReference ? schemaReference.Reference.Id : string.Empty, discriminatorName));
5760
context.Exit();
5861
}
62+
#pragma warning restore CS0618 // Type or member is obsolete
5963
}
6064
});
6165

@@ -65,22 +69,24 @@ public static class OpenApiSchemaRules
6569
/// <param name="schema">The parent schema.</param>
6670
/// <param name="discriminatorName">Adds support for polymorphism. The discriminator is an object name that is used to differentiate
6771
/// between other schemas which may satisfy the payload description.</param>
72+
[Obsolete("This method will be made private in future versions.")]
73+
[Browsable(false)]
6874
public static bool ValidateChildSchemaAgainstDiscriminator(IOpenApiSchema schema, string? discriminatorName)
6975
{
7076
if (discriminatorName is not null)
7177
{
7278
if (schema.Required is null || !schema.Required.Contains(discriminatorName))
7379
{
7480
// recursively check nested schema.OneOf, schema.AnyOf or schema.AllOf and their required fields for the discriminator
75-
if (schema.OneOf?.Count != 0)
81+
if (schema.OneOf is { Count: > 0})
7682
{
7783
return TraverseSchemaElements(discriminatorName, schema.OneOf);
7884
}
79-
if (schema.AnyOf?.Count != 0)
85+
if (schema.AnyOf is { Count: > 0})
8086
{
8187
return TraverseSchemaElements(discriminatorName, schema.AnyOf);
8288
}
83-
if (schema.AllOf?.Count != 0)
89+
if (schema.AllOf is { Count: > 0})
8490
{
8591
return TraverseSchemaElements(discriminatorName, schema.AllOf);
8692
}
@@ -102,25 +108,26 @@ public static bool ValidateChildSchemaAgainstDiscriminator(IOpenApiSchema schema
102108
/// between other schemas which may satisfy the payload description.</param>
103109
/// <param name="childSchema">The child schema.</param>
104110
/// <returns></returns>
111+
[Obsolete("This method will be made private in future versions.")]
112+
[Browsable(false)]
105113
public static bool TraverseSchemaElements(string discriminatorName, IList<IOpenApiSchema>? childSchema)
106114
{
107-
if (childSchema is not null)
115+
if (childSchema is null)
108116
{
109-
foreach (var childItem in childSchema)
117+
return false;
118+
}
119+
foreach (var childItem in childSchema)
120+
{
121+
if ((!childItem.Properties?.ContainsKey(discriminatorName) ?? false) &&
122+
(!childItem.Required?.Contains(discriminatorName) ?? false))
110123
{
111-
if ((!childItem.Properties?.ContainsKey(discriminatorName) ?? false) &&
112-
(!childItem.Required?.Contains(discriminatorName) ?? false))
113-
{
114-
return ValidateChildSchemaAgainstDiscriminator(childItem, discriminatorName);
115-
}
116-
else
117-
{
118-
return true;
119-
}
124+
return ValidateChildSchemaAgainstDiscriminator(childItem, discriminatorName);
120125
}
121-
return false;
122-
}
123-
126+
else
127+
{
128+
return true;
129+
}
130+
}
124131
return false;
125132
}
126133
}

‎test/Microsoft.OpenApi.Tests/PublicApi/PublicApi.approved.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1352,7 +1352,11 @@ namespace Microsoft.OpenApi
13521352
{
13531353
public static Microsoft.OpenApi.ValidationRule<Microsoft.OpenApi.IOpenApiSchema> ValidateSchemaDiscriminator { get; }
13541354
public static Microsoft.OpenApi.ValidationRule<Microsoft.OpenApi.IOpenApiSchema> ValidateSchemaPropertyHasValue { get; }
1355+
[System.ComponentModel.Browsable(false)]
1356+
[System.Obsolete("This method will be made private in future versions.")]
13551357
public static bool TraverseSchemaElements(string discriminatorName, System.Collections.Generic.IList<Microsoft.OpenApi.IOpenApiSchema>? childSchema) { }
1358+
[System.ComponentModel.Browsable(false)]
1359+
[System.Obsolete("This method will be made private in future versions.")]
13561360
public static bool ValidateChildSchemaAgainstDiscriminator(Microsoft.OpenApi.IOpenApiSchema schema, string? discriminatorName) { }
13571361
}
13581362
public class OpenApiSecurityRequirement : System.Collections.Generic.Dictionary<Microsoft.OpenApi.OpenApiSecuritySchemeReference, System.Collections.Generic.List<string>>, Microsoft.OpenApi.IOpenApiElement, Microsoft.OpenApi.IOpenApiSerializable

‎test/Microsoft.OpenApi.Tests/Validations/OpenApiSchemaValidationTests.cs‎

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ public void ValidateSchemaRequiredFieldListMustContainThePropertySpecifiedInTheD
246246
}
247247

248248
[Fact]
249-
public void ValidateOneOfSchemaPropertyNameContainsPropertySpecifiedInTheDiscriminator()
249+
public void ValidateOneOfSchemaPropertyNameContainsPropertySpecifiedInTheDiscriminatorOneOf()
250250
{
251251
// Arrange
252252
var components = new OpenApiComponents
@@ -293,5 +293,102 @@ public void ValidateOneOfSchemaPropertyNameContainsPropertySpecifiedInTheDiscrim
293293
//Assert
294294
Assert.Empty(errors);
295295
}
296+
297+
[Fact]
298+
public void ValidateOneOfSchemaPropertyNameContainsPropertySpecifiedInTheDiscriminatorAnyOf()
299+
{
300+
// Arrange
301+
var components = new OpenApiComponents
302+
{
303+
Schemas = new Dictionary<string, IOpenApiSchema>
304+
{
305+
{
306+
"Person",
307+
new OpenApiSchema
308+
{
309+
Type = JsonSchemaType.Array,
310+
Discriminator = new()
311+
{
312+
PropertyName = "type"
313+
},
314+
AnyOf =
315+
[
316+
new OpenApiSchema()
317+
{
318+
Properties = new Dictionary<string, IOpenApiSchema>
319+
{
320+
{
321+
"type",
322+
new OpenApiSchema
323+
{
324+
Type = JsonSchemaType.Array
325+
}
326+
}
327+
},
328+
}
329+
],
330+
}
331+
}
332+
}
333+
};
334+
335+
// Act
336+
var validator = new OpenApiValidator(ValidationRuleSet.GetDefaultRuleSet());
337+
var walker = new OpenApiWalker(validator);
338+
walker.Walk(components);
339+
340+
var errors = validator.Errors;
341+
342+
//Assert
343+
Assert.Empty(errors);
344+
}
345+
[Fact]
346+
public void ValidateOneOfSchemaPropertyNameContainsPropertySpecifiedInTheDiscriminatorAllOf()
347+
{
348+
// Arrange
349+
var components = new OpenApiComponents
350+
{
351+
Schemas = new Dictionary<string, IOpenApiSchema>
352+
{
353+
{
354+
"Person",
355+
new OpenApiSchema
356+
{
357+
Type = JsonSchemaType.Array,
358+
Discriminator = new()
359+
{
360+
PropertyName = "type"
361+
},
362+
AllOf =
363+
[
364+
new OpenApiSchema()
365+
{
366+
Properties = new Dictionary<string, IOpenApiSchema>
367+
{
368+
{
369+
"type",
370+
new OpenApiSchema
371+
{
372+
Type = JsonSchemaType.Array
373+
}
374+
}
375+
},
376+
}
377+
],
378+
}
379+
}
380+
}
381+
};
382+
383+
// Act
384+
var validator = new OpenApiValidator(ValidationRuleSet.GetDefaultRuleSet());
385+
var walker = new OpenApiWalker(validator);
386+
walker.Walk(components);
387+
388+
var errors = validator.Errors;
389+
390+
//Assert
391+
Assert.Empty(errors);
392+
}
296393
}
297394
}

0 commit comments

Comments
 (0)