diff --git a/CHANGELOG.md b/CHANGELOG.md index deae6284e5..c34afd5d9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Fixed a bug where the discriminator validation rule would report false positives on nullable union types. + ## [1.9.1] - 2023-12-13 ### Added diff --git a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs index c5756793a8..2a37aaa315 100644 --- a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs @@ -2,7 +2,7 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; - +using Microsoft.OpenApi.Any; using Microsoft.OpenApi.Models; namespace Kiota.Builder.Extensions; @@ -60,8 +60,8 @@ public static bool IsArray(this OpenApiSchema? schema) return "array".Equals(schema?.Type, StringComparison.OrdinalIgnoreCase) && schema.Items != null && (schema.Items.IsComposedEnum() || schema.Items.IsEnum() || - IsSemanticallyMeaningful(schema.Items) || - FlattenEmptyEntries(new OpenApiSchema[] { schema.Items }, static x => x.AnyOf.Union(x.AllOf).Union(x.OneOf).ToList(), 1).FirstOrDefault() is OpenApiSchema flat && IsSemanticallyMeaningful(flat)); + schema.Items.IsSemanticallyMeaningful() || + FlattenEmptyEntries(new OpenApiSchema[] { schema.Items }, static x => x.AnyOf.Union(x.AllOf).Union(x.OneOf).ToList(), 1).FirstOrDefault() is OpenApiSchema flat && flat.IsSemanticallyMeaningful()); } public static bool IsObject(this OpenApiSchema? schema) @@ -70,13 +70,14 @@ public static bool IsObject(this OpenApiSchema? schema) } public static bool IsInclusiveUnion(this OpenApiSchema? schema) { - return schema?.AnyOf?.Count(IsSemanticallyMeaningful) > 1; + return schema?.AnyOf?.Count(static x => IsSemanticallyMeaningful(x, true)) > 1; + // so we don't consider any of object/nullable as a union type } public static bool IsInherited(this OpenApiSchema? schema) { if (schema is null) return false; - var meaningfulSchemas = schema.AllOf.FlattenSchemaIfRequired(static x => x.AllOf).Where(IsSemanticallyMeaningful).ToArray(); + var meaningfulSchemas = schema.AllOf.FlattenSchemaIfRequired(static x => x.AllOf).Where(static x => x.IsSemanticallyMeaningful()).ToArray(); return meaningfulSchemas.Count(static x => !string.IsNullOrEmpty(x.Reference?.Id)) == 1 && meaningfulSchemas.Count(static x => string.IsNullOrEmpty(x.Reference?.Id)) == 1; } @@ -86,20 +87,21 @@ public static bool IsInherited(this OpenApiSchema? schema) if (!schema.IsIntersection()) return schema; var result = new OpenApiSchema(schema); result.AllOf.Clear(); - var meaningfulSchemas = schema.AllOf.Where(IsSemanticallyMeaningful).Select(MergeIntersectionSchemaEntries).Where(x => x is not null).OfType(); + var meaningfulSchemas = schema.AllOf.Where(static x => x.IsSemanticallyMeaningful()).Select(MergeIntersectionSchemaEntries).Where(x => x is not null).OfType(); meaningfulSchemas.SelectMany(static x => x.Properties).ToList().ForEach(x => result.Properties.TryAdd(x.Key, x.Value)); return result; } public static bool IsIntersection(this OpenApiSchema? schema) { - var meaningfulSchemas = schema?.AllOf?.Where(IsSemanticallyMeaningful); + var meaningfulSchemas = schema?.AllOf?.Where(static x => x.IsSemanticallyMeaningful()); return meaningfulSchemas?.Count() > 3 || meaningfulSchemas?.Count(static x => !string.IsNullOrEmpty(x.Reference?.Id)) > 1 || meaningfulSchemas?.Count(static x => string.IsNullOrEmpty(x.Reference?.Id)) > 1; } public static bool IsExclusiveUnion(this OpenApiSchema? schema) { - return schema?.OneOf?.Count(IsSemanticallyMeaningful) > 1; + 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 oDataTypes = new(StringComparer.OrdinalIgnoreCase) { "number", @@ -121,15 +123,26 @@ public static bool IsODataPrimitiveType(this OpenApiSchema schema) } public static bool IsEnum(this OpenApiSchema schema) { - return (schema?.Enum?.Any() ?? false) && (string.IsNullOrEmpty(schema.Type) || "string".Equals(schema.Type, StringComparison.OrdinalIgnoreCase)); // number and boolean enums are not supported + if (schema is null) return false; + return schema.Enum.OfType().Any(static x => !string.IsNullOrEmpty(x.Value)) && + (string.IsNullOrEmpty(schema.Type) || "string".Equals(schema.Type, StringComparison.OrdinalIgnoreCase)) || + schema.AnyOf.Where(static x => x.IsSemanticallyMeaningful(true)).Count(static x => x.IsEnum()) == 1 && !schema.AnyOf.Where(static x => x.IsSemanticallyMeaningful(true)).Any(static x => !x.IsEnum()) || + schema.OneOf.Where(static x => x.IsSemanticallyMeaningful(true)).Count(static x => x.IsEnum()) == 1 && !schema.OneOf.Where(static x => x.IsSemanticallyMeaningful(true)).Any(static x => !x.IsEnum()) + ; // number and boolean enums are not supported } public static bool IsComposedEnum(this OpenApiSchema schema) { return (schema.IsInclusiveUnion() && schema.AnyOf.Any(static x => x.IsEnum())) || (schema.IsExclusiveUnion() && schema.OneOf.Any(static x => x.IsEnum())); } - private static bool IsSemanticallyMeaningful(this OpenApiSchema schema) + private static bool IsSemanticallyMeaningful(this OpenApiSchema schema, bool ignoreNullableObjects = false) { - return schema.Properties.Any() || schema.Items != null || !string.IsNullOrEmpty(schema.Type) || !string.IsNullOrEmpty(schema.Format) || !string.IsNullOrEmpty(schema.Reference?.Id); + return schema.Properties.Any() || + schema.Items != null || + (!string.IsNullOrEmpty(schema.Type) && + ((ignoreNullableObjects && !"object".Equals(schema.Type, StringComparison.OrdinalIgnoreCase)) || + !ignoreNullableObjects)) || + !string.IsNullOrEmpty(schema.Format) || + !string.IsNullOrEmpty(schema.Reference?.Id); } public static IEnumerable GetSchemaReferenceIds(this OpenApiSchema schema, HashSet? visitedSchemas = null) { diff --git a/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs b/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs index c8bba1226c..1c1985b8a5 100644 --- a/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs @@ -2,7 +2,7 @@ using System.Collections.Generic; using Kiota.Builder.Extensions; - +using Microsoft.OpenApi.Any; using Microsoft.OpenApi.Models; using Xunit; @@ -515,4 +515,123 @@ public void IsArrayFalseOnNullItems() }; Assert.False(schema.IsArray()); } + [Fact] + public void IsEnumFailsOnEmptyMembers() + { + var schema = new OpenApiSchema + { + Type = "string", + Enum = new List(), + }; + Assert.False(schema.IsEnum()); + + schema.Enum.Add(new OpenApiString("")); + Assert.False(schema.IsEnum()); + } + private static readonly OpenApiSchema enumSchema = new OpenApiSchema + { + Title = "riskLevel", + Enum = new List + { + new OpenApiString("low"), + new OpenApiString("medium"), + new OpenApiString("high"), + new OpenApiString("hidden"), + new OpenApiString("none"), + new OpenApiString("unknownFutureValue") + }, + Type = "string" + }; + [Fact] + public void IsEnumIgnoresNullableUnions() + { + var schema = new OpenApiSchema + { + AnyOf = new List + { + enumSchema, + new OpenApiSchema + { + Type = "object", + Nullable = true + } + } + }; + Assert.True(schema.IsEnum()); + } + [Fact] + public void IsEnumFailsOnNullableInheritance() + { + var schema = new OpenApiSchema + { + AllOf = new List + { + enumSchema, + new OpenApiSchema + { + Type = "object", + Nullable = true + } + } + }; + Assert.False(schema.IsEnum()); + } + [Fact] + public void IsEnumIgnoresNullableExclusiveUnions() + { + var schema = new OpenApiSchema + { + OneOf = new List + { + enumSchema, + new OpenApiSchema + { + Type = "object", + Nullable = true + } + } + }; + Assert.True(schema.IsEnum()); + } + private static readonly OpenApiSchema numberSchema = new OpenApiSchema + { + Type = "number", + Format = "double", + }; + [Fact] + public void IsEnumDoesNotMaskExclusiveUnions() + { + var schema = new OpenApiSchema + { + OneOf = new List + { + enumSchema, + numberSchema, + new OpenApiSchema + { + Type = "object", + Nullable = true + } + } + }; + Assert.False(schema.IsEnum()); + } + [Fact] + public void IsEnumDoesNotMaskUnions() + { + var schema = new OpenApiSchema + { + AnyOf = new List + { + enumSchema, + numberSchema, + new OpenApiSchema + { + Type = "object", + Nullable = true + } + } + }; + Assert.False(schema.IsEnum()); + } }