Skip to content

Commit 475df3c

Browse files
gadcamgadcam
committed
Discriminator - Ensure warning shows up in recursive case
1 parent e7e5c9b commit 475df3c

File tree

3 files changed

+99
-4
lines changed

3 files changed

+99
-4
lines changed

‎src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs

+21-1
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@
66
using Microsoft.OpenApi.Models;
77

88
namespace Kiota.Builder.Extensions;
9+
910
public static class OpenApiSchemaExtensions
1011
{
1112
private static readonly Func<OpenApiSchema, IList<OpenApiSchema>> classNamesFlattener = x =>
1213
(x.AnyOf ?? Enumerable.Empty<OpenApiSchema>()).Union(x.AllOf).Union(x.OneOf).ToList();
14+
1315
public static IEnumerable<string> GetSchemaNames(this OpenApiSchema schema)
1416
{
1517
if (schema == null)
@@ -30,13 +32,15 @@ public static IEnumerable<string> GetSchemaNames(this OpenApiSchema schema)
3032
return new[] { schema.Xml.Name };
3133
return Enumerable.Empty<string>();
3234
}
35+
3336
internal static IEnumerable<OpenApiSchema> FlattenSchemaIfRequired(this IList<OpenApiSchema> schemas, Func<OpenApiSchema, IList<OpenApiSchema>> subsequentGetter)
3437
{
3538
if (schemas is null) return Enumerable.Empty<OpenApiSchema>();
3639
return schemas.Count == 1 ?
3740
schemas.FlattenEmptyEntries(subsequentGetter, 1) :
3841
schemas;
3942
}
43+
4044
private static IEnumerable<string> FlattenIfRequired(this IList<OpenApiSchema> schemas, Func<OpenApiSchema, IList<OpenApiSchema>> subsequentGetter)
4145
{
4246
return schemas.FlattenSchemaIfRequired(subsequentGetter).Where(static x => !string.IsNullOrEmpty(x.Title)).Select(static x => x.Title);
@@ -68,6 +72,12 @@ public static bool IsObject(this OpenApiSchema? schema)
6872
{
6973
return "object".Equals(schema?.Type, StringComparison.OrdinalIgnoreCase);
7074
}
75+
76+
public static bool IsScalar(this OpenApiSchema? schema)
77+
{
78+
return schema?.IsObject() == false && schema?.Reference?.Id == null;
79+
}
80+
7181
public static bool IsInclusiveUnion(this OpenApiSchema? schema)
7282
{
7383
return schema?.AnyOf?.Count(static x => IsSemanticallyMeaningful(x, true)) > 1;
@@ -103,10 +113,12 @@ public static bool IsExclusiveUnion(this OpenApiSchema? schema)
103113
return schema?.OneOf?.Count(static x => IsSemanticallyMeaningful(x, true)) > 1;
104114
// so we don't consider one of object/nullable as a union type
105115
}
116+
106117
private static readonly HashSet<string> oDataTypes = new(StringComparer.OrdinalIgnoreCase) {
107118
"number",
108119
"integer",
109120
};
121+
110122
public static bool IsODataPrimitiveType(this OpenApiSchema schema)
111123
{
112124
return schema.IsExclusiveUnion() &&
@@ -121,18 +133,21 @@ public static bool IsODataPrimitiveType(this OpenApiSchema schema)
121133
schema.AnyOf.Count(static x => oDataTypes.Contains(x.Type)) == 1 &&
122134
schema.AnyOf.Count(static x => "string".Equals(x.Type, StringComparison.OrdinalIgnoreCase)) == 1;
123135
}
136+
124137
public static bool IsEnum(this OpenApiSchema schema)
125138
{
126139
if (schema is null) return false;
127140
return schema.Enum.OfType<OpenApiString>().Any(static x => !string.IsNullOrEmpty(x.Value)) &&
128141
(string.IsNullOrEmpty(schema.Type) || "string".Equals(schema.Type, StringComparison.OrdinalIgnoreCase)); // number and boolean enums are not supported
129142
}
143+
130144
public static bool IsComposedEnum(this OpenApiSchema schema)
131145
{
132146
if (schema is null) return false;
133147
return schema.AnyOf.Count(static x => !x.IsSemanticallyMeaningful(true)) == 1 && schema.AnyOf.Count(static x => x.IsEnum()) == 1 ||
134148
schema.OneOf.Count(static x => !x.IsSemanticallyMeaningful(true)) == 1 && schema.OneOf.Count(static x => x.IsEnum()) == 1;
135149
}
150+
136151
public static bool IsSemanticallyMeaningful(this OpenApiSchema schema, bool ignoreNullableObjects = false)
137152
{
138153
if (schema is null) return false;
@@ -145,6 +160,7 @@ public static bool IsSemanticallyMeaningful(this OpenApiSchema schema, bool igno
145160
!string.IsNullOrEmpty(schema.Format) ||
146161
!string.IsNullOrEmpty(schema.Reference?.Id);
147162
}
163+
148164
public static IEnumerable<string> GetSchemaReferenceIds(this OpenApiSchema schema, HashSet<OpenApiSchema>? visitedSchemas = null)
149165
{
150166
visitedSchemas ??= new();
@@ -173,6 +189,7 @@ public static IEnumerable<string> GetSchemaReferenceIds(this OpenApiSchema schem
173189

174190
return Enumerable.Empty<string>();
175191
}
192+
176193
private static IEnumerable<OpenApiSchema> FlattenEmptyEntries(this IEnumerable<OpenApiSchema> schemas, Func<OpenApiSchema, IList<OpenApiSchema>> subsequentGetter, int? maxDepth = default)
177194
{
178195
if (schemas == null) return Enumerable.Empty<OpenApiSchema>();
@@ -205,6 +222,7 @@ private static IEnumerable<OpenApiSchema> FlattenEmptyEntries(this IEnumerable<O
205222
}
206223
return result;
207224
}
225+
208226
internal static string GetDiscriminatorPropertyName(this OpenApiSchema schema)
209227
{
210228
if (schema == null)
@@ -222,6 +240,7 @@ internal static string GetDiscriminatorPropertyName(this OpenApiSchema schema)
222240

223241
return string.Empty;
224242
}
243+
225244
internal static IEnumerable<KeyValuePair<string, string>> GetDiscriminatorMappings(this OpenApiSchema schema, ConcurrentDictionary<string, ConcurrentDictionary<string, bool>> inheritanceIndex)
226245
{
227246
if (schema == null)
@@ -245,7 +264,9 @@ internal static IEnumerable<KeyValuePair<string, string>> GetDiscriminatorMappin
245264
return schema.Discriminator
246265
.Mapping;
247266
}
267+
248268
private static readonly Func<OpenApiSchema, bool> allOfEvaluatorForMappings = static x => x.Discriminator?.Mapping.Any() ?? false;
269+
249270
private static IEnumerable<string> GetAllInheritanceSchemaReferences(string currentReferenceId, ConcurrentDictionary<string, ConcurrentDictionary<string, bool>> inheritanceIndex)
250271
{
251272
ArgumentException.ThrowIfNullOrEmpty(currentReferenceId);
@@ -255,4 +276,3 @@ private static IEnumerable<string> GetAllInheritanceSchemaReferences(string curr
255276
return Enumerable.Empty<string>();
256277
}
257278
}
258-

‎src/Kiota.Builder/Validation/MissingDiscriminator.cs

+36-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Collections.Concurrent;
3+
using System.Collections.Generic;
34
using System.Linq;
45
using System.Threading.Tasks;
56
using Kiota.Builder.Configuration;
@@ -8,6 +9,7 @@
89
using Microsoft.OpenApi.Validations;
910

1011
namespace Kiota.Builder.Validation;
12+
1113
public class MissingDiscriminator : ValidationRule<OpenApiDocument>
1214
{
1315
public MissingDiscriminator(GenerationConfiguration configuration) : base(nameof(MissingDiscriminator), (context, document) =>
@@ -17,7 +19,7 @@ public MissingDiscriminator(GenerationConfiguration configuration) : base(nameof
1719
if (document.Components != null)
1820
Parallel.ForEach(document.Components.Schemas, entry =>
1921
{
20-
ValidateSchema(entry.Value, context, idx, entry.Key);
22+
ValidateSchemaRecursively(entry.Value, context, idx, entry.Key);
2123
});
2224
var inlineSchemasToValidate = document.Paths
2325
?.SelectMany(static x => x.Value.Operations.Values.Select(y => (x.Key, Operation: y)))
@@ -26,16 +28,47 @@ public MissingDiscriminator(GenerationConfiguration configuration) : base(nameof
2628
.ToArray() ?? Array.Empty<(string, OpenApiSchema)>();
2729
Parallel.ForEach(inlineSchemasToValidate, entry =>
2830
{
29-
ValidateSchema(entry.Schema, context, idx, entry.Key);
31+
ValidateSchemaRecursively(entry.Schema, context, idx, entry.Key);
3032
});
3133
})
3234
{
3335
}
36+
37+
private static void ValidateSchemaRecursively(OpenApiSchema schema, IValidationContext context, ConcurrentDictionary<string, ConcurrentDictionary<string, bool>> idx, string address)
38+
{
39+
foreach (var property in schema.Properties ?? Enumerable.Empty<KeyValuePair<string, OpenApiSchema>>())
40+
{
41+
ValidateSchemaRecursively(property.Value, context, idx, $"{address}.Properties.{property.Key}");
42+
}
43+
44+
foreach (var allOfSchema in schema.AllOf)
45+
{
46+
ValidateSchemaRecursively(allOfSchema, context, idx, $"{address}.AllOf");
47+
}
48+
49+
foreach (var oneOfSchema in schema.OneOf)
50+
{
51+
ValidateSchemaRecursively(oneOfSchema, context, idx, $"{address}.OneOf");
52+
}
53+
54+
foreach (var anyOfSchema in schema.AnyOf)
55+
{
56+
ValidateSchemaRecursively(anyOfSchema, context, idx, $"{address}.AnyOf");
57+
}
58+
59+
if (schema.Items != null)
60+
{
61+
ValidateSchemaRecursively(schema.Items, context, idx, $"{address}.Items");
62+
}
63+
64+
ValidateSchema(schema, context, idx, address);
65+
}
66+
3467
private static void ValidateSchema(OpenApiSchema schema, IValidationContext context, ConcurrentDictionary<string, ConcurrentDictionary<string, bool>> idx, string address)
3568
{
3669
if (!schema.IsInclusiveUnion() && !schema.IsExclusiveUnion())
3770
return;
38-
if (schema.AnyOf.All(static x => !x.IsObject()) && schema.OneOf.All(static x => !x.IsObject()))
71+
if (schema.AnyOf.All(static x => x.IsScalar()) && schema.OneOf.All(static x => x.IsScalar()))
3972
return;
4073
if (string.IsNullOrEmpty(schema.GetDiscriminatorPropertyName()) || !schema.GetDiscriminatorMappings(idx).Any())
4174
context.CreateWarning(nameof(MissingDiscriminator), $"The schema {address} is a polymorphic type but does not define a discriminator. This will result in a serialization errors.");

‎tests/Kiota.Builder.Tests/Validation/MissingDiscriminatorTests.cs

+42
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using Xunit;
77

88
namespace Kiota.Builder.Tests.Validation;
9+
910
public class MissingDiscriminatorTests
1011
{
1112
[Fact]
@@ -35,6 +36,7 @@ public void DoesntAddAWarningWhenBodyIsSimple()
3536
var doc = reader.Read(stream, out var diag);
3637
Assert.Empty(diag.Warnings);
3738
}
39+
3840
[Fact]
3941
public void AddsWarningOnInlineSchemas()
4042
{
@@ -70,6 +72,44 @@ public void AddsWarningOnInlineSchemas()
7072
var doc = reader.Read(stream, out var diag);
7173
Assert.Single(diag.Warnings);
7274
}
75+
76+
[Fact]
77+
public void AddsWarningOnNestedInlineSchemas()
78+
{
79+
var rule = new MissingDiscriminator(new());
80+
var documentTxt = @"openapi: 3.0.1
81+
info:
82+
title: OData Service for namespace microsoft.graph
83+
description: This OData service is located at https://graph.microsoft.com/v1.0
84+
version: 1.0.1
85+
paths:
86+
/enumeration:
87+
get:
88+
responses:
89+
'200':
90+
content:
91+
application/json:
92+
schema:
93+
type: array
94+
items:
95+
oneOf:
96+
- type: object
97+
properties:
98+
type:
99+
type: string
100+
- type: object
101+
properties:
102+
type2:
103+
type: string";
104+
using var stream = new MemoryStream(Encoding.UTF8.GetBytes(documentTxt));
105+
var reader = new OpenApiStreamReader(new OpenApiReaderSettings
106+
{
107+
RuleSet = new(new ValidationRule[] { rule }),
108+
});
109+
var doc = reader.Read(stream, out var diag);
110+
Assert.Single(diag.Warnings);
111+
}
112+
73113
[Fact]
74114
public void AddsWarningOnComponentSchemas()
75115
{
@@ -113,6 +153,7 @@ public void AddsWarningOnComponentSchemas()
113153
var doc = reader.Read(stream, out var diag);
114154
Assert.Single(diag.Warnings);
115155
}
156+
116157
[Fact]
117158
public void DoesntAddsWarningOnComponentSchemasWithDiscriminatorInformation()
118159
{
@@ -158,6 +199,7 @@ public void DoesntAddsWarningOnComponentSchemasWithDiscriminatorInformation()
158199
var doc = reader.Read(stream, out var diag);
159200
Assert.Empty(diag.Warnings);
160201
}
202+
161203
[Fact]
162204
public void DoesntAddsWarningOnComponentSchemasScalars()
163205
{

0 commit comments

Comments
 (0)