Discriminator - Ensure warning shows up in recursive case

microsoft · Mar 18, 2024 · 475df3c · 475df3c
1 parent e7e5c9b
commit 475df3c
Show file tree

Hide file tree

Showing 3 changed files with 99 additions and 4 deletions.
diff --git a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs
@@ -6,10 +6,12 @@
 using Microsoft.OpenApi.Models;
 
 namespace Kiota.Builder.Extensions;
+
 public static class OpenApiSchemaExtensions
 {
     private static readonly Func<OpenApiSchema, IList<OpenApiSchema>> classNamesFlattener = x =>
     (x.AnyOf ?? Enumerable.Empty<OpenApiSchema>()).Union(x.AllOf).Union(x.OneOf).ToList();
+
     public static IEnumerable<string> GetSchemaNames(this OpenApiSchema schema)
     {
         if (schema == null)
@@ -30,13 +32,15 @@ public static IEnumerable<string> GetSchemaNames(this OpenApiSchema schema)
             return new[] { schema.Xml.Name };
         return Enumerable.Empty<string>();
     }
+
     internal static IEnumerable<OpenApiSchema> FlattenSchemaIfRequired(this IList<OpenApiSchema> schemas, Func<OpenApiSchema, IList<OpenApiSchema>> subsequentGetter)
     {
         if (schemas is null) return Enumerable.Empty<OpenApiSchema>();
         return schemas.Count == 1 ?
                     schemas.FlattenEmptyEntries(subsequentGetter, 1) :
                     schemas;
     }
+
     private static IEnumerable<string> FlattenIfRequired(this IList<OpenApiSchema> schemas, Func<OpenApiSchema, IList<OpenApiSchema>> subsequentGetter)
     {
         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)
     {
         return "object".Equals(schema?.Type, StringComparison.OrdinalIgnoreCase);
     }
+
+    public static bool IsScalar(this OpenApiSchema? schema)
+    {
+        return schema?.IsObject() == false && schema?.Reference?.Id == null;
+    }
+
     public static bool IsInclusiveUnion(this OpenApiSchema? schema)
     {
         return schema?.AnyOf?.Count(static x => IsSemanticallyMeaningful(x, true)) > 1;
@@ -103,10 +113,12 @@ public static bool IsExclusiveUnion(this OpenApiSchema? schema)
         return schema?.OneOf?.Count(static x => IsSemanticallyMeaningful(x, true)) > 1;
         // so we don't consider one of object/nullable as a union type
     }
+
     private static readonly HashSet<string> oDataTypes = new(StringComparer.OrdinalIgnoreCase) {
         "number",
         "integer",
     };
+
     public static bool IsODataPrimitiveType(this OpenApiSchema schema)
     {
         return schema.IsExclusiveUnion() &&
@@ -121,18 +133,21 @@ public static bool IsODataPrimitiveType(this OpenApiSchema schema)
                 schema.AnyOf.Count(static x => oDataTypes.Contains(x.Type)) == 1 &&
                 schema.AnyOf.Count(static x => "string".Equals(x.Type, StringComparison.OrdinalIgnoreCase)) == 1;
     }
+
     public static bool IsEnum(this OpenApiSchema schema)
     {
         if (schema is null) return false;
         return schema.Enum.OfType<OpenApiString>().Any(static x => !string.IsNullOrEmpty(x.Value)) &&
                 (string.IsNullOrEmpty(schema.Type) || "string".Equals(schema.Type, StringComparison.OrdinalIgnoreCase)); // number and boolean enums are not supported
     }
+
     public static bool IsComposedEnum(this OpenApiSchema schema)
     {
         if (schema is null) return false;
         return schema.AnyOf.Count(static x => !x.IsSemanticallyMeaningful(true)) == 1 && schema.AnyOf.Count(static x => x.IsEnum()) == 1 ||
                 schema.OneOf.Count(static x => !x.IsSemanticallyMeaningful(true)) == 1 && schema.OneOf.Count(static x => x.IsEnum()) == 1;
     }
+
     public static bool IsSemanticallyMeaningful(this OpenApiSchema schema, bool ignoreNullableObjects = false)
     {
         if (schema is null) return false;
@@ -145,6 +160,7 @@ public static bool IsSemanticallyMeaningful(this OpenApiSchema schema, bool igno
                 !string.IsNullOrEmpty(schema.Format) ||
                 !string.IsNullOrEmpty(schema.Reference?.Id);
     }
+
     public static IEnumerable<string> GetSchemaReferenceIds(this OpenApiSchema schema, HashSet<OpenApiSchema>? visitedSchemas = null)
     {
         visitedSchemas ??= new();
@@ -173,6 +189,7 @@ public static IEnumerable<string> GetSchemaReferenceIds(this OpenApiSchema schem
 
         return Enumerable.Empty<string>();
     }
+
     private static IEnumerable<OpenApiSchema> FlattenEmptyEntries(this IEnumerable<OpenApiSchema> schemas, Func<OpenApiSchema, IList<OpenApiSchema>> subsequentGetter, int? maxDepth = default)
     {
         if (schemas == null) return Enumerable.Empty<OpenApiSchema>();
@@ -205,6 +222,7 @@ private static IEnumerable<OpenApiSchema> FlattenEmptyEntries(this IEnumerable<O
         }
         return result;
     }
+
     internal static string GetDiscriminatorPropertyName(this OpenApiSchema schema)
     {
         if (schema == null)
@@ -222,6 +240,7 @@ internal static string GetDiscriminatorPropertyName(this OpenApiSchema schema)
 
         return string.Empty;
     }
+
     internal static IEnumerable<KeyValuePair<string, string>> GetDiscriminatorMappings(this OpenApiSchema schema, ConcurrentDictionary<string, ConcurrentDictionary<string, bool>> inheritanceIndex)
     {
         if (schema == null)
@@ -245,7 +264,9 @@ internal static IEnumerable<KeyValuePair<string, string>> GetDiscriminatorMappin
         return schema.Discriminator
                 .Mapping;
     }
+
     private static readonly Func<OpenApiSchema, bool> allOfEvaluatorForMappings = static x => x.Discriminator?.Mapping.Any() ?? false;
+
     private static IEnumerable<string> GetAllInheritanceSchemaReferences(string currentReferenceId, ConcurrentDictionary<string, ConcurrentDictionary<string, bool>> inheritanceIndex)
     {
         ArgumentException.ThrowIfNullOrEmpty(currentReferenceId);
@@ -255,4 +276,3 @@ private static IEnumerable<string> GetAllInheritanceSchemaReferences(string curr
         return Enumerable.Empty<string>();
     }
 }
-
diff --git a/src/Kiota.Builder/Validation/MissingDiscriminator.cs b/src/Kiota.Builder/Validation/MissingDiscriminator.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Concurrent;
+using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
 using Kiota.Builder.Configuration;
@@ -8,6 +9,7 @@
 using Microsoft.OpenApi.Validations;
 
 namespace Kiota.Builder.Validation;
+
 public class MissingDiscriminator : ValidationRule<OpenApiDocument>
 {
     public MissingDiscriminator(GenerationConfiguration configuration) : base(nameof(MissingDiscriminator), (context, document) =>
@@ -17,7 +19,7 @@ public MissingDiscriminator(GenerationConfiguration configuration) : base(nameof
         if (document.Components != null)
             Parallel.ForEach(document.Components.Schemas, entry =>
             {
-                ValidateSchema(entry.Value, context, idx, entry.Key);
+                ValidateSchemaRecursively(entry.Value, context, idx, entry.Key);
             });
         var inlineSchemasToValidate = document.Paths
                                         ?.SelectMany(static x => x.Value.Operations.Values.Select(y => (x.Key, Operation: y)))
@@ -26,16 +28,47 @@ public MissingDiscriminator(GenerationConfiguration configuration) : base(nameof
                                         .ToArray() ?? Array.Empty<(string, OpenApiSchema)>();
         Parallel.ForEach(inlineSchemasToValidate, entry =>
         {
-            ValidateSchema(entry.Schema, context, idx, entry.Key);
+            ValidateSchemaRecursively(entry.Schema, context, idx, entry.Key);
         });
     })
     {
     }
+
+    private static void ValidateSchemaRecursively(OpenApiSchema schema, IValidationContext context, ConcurrentDictionary<string, ConcurrentDictionary<string, bool>> idx, string address)
+    {
+        foreach (var property in schema.Properties ?? Enumerable.Empty<KeyValuePair<string, OpenApiSchema>>())
+        {
+            ValidateSchemaRecursively(property.Value, context, idx, $"{address}.Properties.{property.Key}");
+        }
+
+        foreach (var allOfSchema in schema.AllOf)
+        {
+            ValidateSchemaRecursively(allOfSchema, context, idx, $"{address}.AllOf");
+        }
+
+        foreach (var oneOfSchema in schema.OneOf)
+        {
+            ValidateSchemaRecursively(oneOfSchema, context, idx, $"{address}.OneOf");
+        }
+
+        foreach (var anyOfSchema in schema.AnyOf)
+        {
+            ValidateSchemaRecursively(anyOfSchema, context, idx, $"{address}.AnyOf");
+        }
+
+        if (schema.Items != null)
+        {
+            ValidateSchemaRecursively(schema.Items, context, idx, $"{address}.Items");
+        }
+
+        ValidateSchema(schema, context, idx, address);
+    }
+
     private static void ValidateSchema(OpenApiSchema schema, IValidationContext context, ConcurrentDictionary<string, ConcurrentDictionary<string, bool>> idx, string address)
     {
         if (!schema.IsInclusiveUnion() && !schema.IsExclusiveUnion())
             return;
-        if (schema.AnyOf.All(static x => !x.IsObject()) && schema.OneOf.All(static x => !x.IsObject()))
+        if (schema.AnyOf.All(static x => x.IsScalar()) && schema.OneOf.All(static x => x.IsScalar()))
             return;
         if (string.IsNullOrEmpty(schema.GetDiscriminatorPropertyName()) || !schema.GetDiscriminatorMappings(idx).Any())
             context.CreateWarning(nameof(MissingDiscriminator), $"The schema {address} is a polymorphic type but does not define a discriminator. This will result in a serialization errors.");

diff --git a/tests/Kiota.Builder.Tests/Validation/MissingDiscriminatorTests.cs b/tests/Kiota.Builder.Tests/Validation/MissingDiscriminatorTests.cs
@@ -6,6 +6,7 @@
 using Xunit;
 
 namespace Kiota.Builder.Tests.Validation;
+
 public class MissingDiscriminatorTests
 {
     [Fact]
@@ -35,6 +36,7 @@ public void DoesntAddAWarningWhenBodyIsSimple()
         var doc = reader.Read(stream, out var diag);
         Assert.Empty(diag.Warnings);
     }
+
     [Fact]
     public void AddsWarningOnInlineSchemas()
     {
@@ -70,6 +72,44 @@ public void AddsWarningOnInlineSchemas()
         var doc = reader.Read(stream, out var diag);
         Assert.Single(diag.Warnings);
     }
+
+    [Fact]
+    public void AddsWarningOnNestedInlineSchemas()
+    {
+        var rule = new MissingDiscriminator(new());
+        var documentTxt = @"openapi: 3.0.1
+info:
+  title: OData Service for namespace microsoft.graph
+  description: This OData service is located at https://graph.microsoft.com/v1.0
+  version: 1.0.1
+paths:
+  /enumeration:
+    get:
+      responses:
+        '200':
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  oneOf:
+                    - type: object
+                      properties:
+                        type:
+                          type: string
+                    - type: object
+                      properties:
+                        type2:
+                          type: string";
+        using var stream = new MemoryStream(Encoding.UTF8.GetBytes(documentTxt));
+        var reader = new OpenApiStreamReader(new OpenApiReaderSettings
+        {
+            RuleSet = new(new ValidationRule[] { rule }),
+        });
+        var doc = reader.Read(stream, out var diag);
+        Assert.Single(diag.Warnings);
+    }
+
     [Fact]
     public void AddsWarningOnComponentSchemas()
     {
@@ -113,6 +153,7 @@ public void AddsWarningOnComponentSchemas()
         var doc = reader.Read(stream, out var diag);
         Assert.Single(diag.Warnings);
     }
+
     [Fact]
     public void DoesntAddsWarningOnComponentSchemasWithDiscriminatorInformation()
     {
@@ -158,6 +199,7 @@ public void DoesntAddsWarningOnComponentSchemasWithDiscriminatorInformation()
         var doc = reader.Read(stream, out var diag);
         Assert.Empty(diag.Warnings);
     }
+
     [Fact]
     public void DoesntAddsWarningOnComponentSchemasScalars()
     {