From 8eeb7ca2a10a26fa04a28af6fac7d7e10d3029c2 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 21 Mar 2024 17:10:01 -0400 Subject: [PATCH] - fixes a bug where allOf structure with one entry and properties would not be considered as inheritance case Signed-off-by: Vincent Biret --- CHANGELOG.md | 1 + .../Extensions/OpenApiSchemaExtensions.cs | 32 ++- src/Kiota.Builder/KiotaBuilder.cs | 9 +- .../OpenApiSchemaExtensionsTests.cs | 187 +++++++++++++++--- .../Kiota.Builder.Tests/KiotaBuilderTests.cs | 98 +++++++++ 5 files changed, 273 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85ae37d9c9..291681bc87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed a bug where multiple Visual Studio Code instances would make the extension install/update fail. [#3686](https://github.com/microsoft/kiota/issues/3686) - Fixed a bug where models properties named "additionalData" or "backingstore" would be ignored. [#4224](https://github.com/microsoft/kiota/issues/4224) - Fixed a bug where multiple allOf entries type would not get merged if they were part of a discriminator. [#4325](https://github.com/microsoft/kiota/issues/4325) +- Fixed a bug where allOf structure with one entry and properties would not be considered as inheritance case. [#4346](https://github.com/microsoft/kiota/issues/4346) - PREVIEW: Renamed the config commands to workspace. [#4310](https://github.com/microsoft/kiota/issues/4310) - PREVIEW: Moved preview configuration files to the .kiota directory. [#4310](https://github.com/microsoft/kiota/issues/4310) - PREVIEW: Moved the copy descriptions to dedicated folders. [#4310](https://github.com/microsoft/kiota/issues/4310) diff --git a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs index b0bba5e344..d1e647a223 100644 --- a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs @@ -10,41 +10,37 @@ public static class OpenApiSchemaExtensions { private static readonly Func> classNamesFlattener = x => (x.AnyOf ?? Enumerable.Empty()).Union(x.AllOf).Union(x.OneOf).ToList(); - public static IEnumerable GetSchemaNames(this OpenApiSchema schema) + public static IEnumerable GetSchemaNames(this OpenApiSchema schema, bool directOnly = false) { if (schema == null) - return Enumerable.Empty(); - if (schema.Items != null) + return []; + if (!directOnly && schema.Items != null) return schema.Items.GetSchemaNames(); if (!string.IsNullOrEmpty(schema.Reference?.Id)) - return new[] { schema.Reference.Id.Split('/')[^1].Split('.')[^1] }; - if (schema.AnyOf.Any()) + return [schema.Reference.Id.Split('/')[^1].Split('.')[^1]]; + if (!directOnly && schema.AnyOf.Any()) return schema.AnyOf.FlattenIfRequired(classNamesFlattener); - if (schema.AllOf.Any()) + if (!directOnly && schema.AllOf.Any()) return schema.AllOf.FlattenIfRequired(classNamesFlattener); - if (schema.OneOf.Any()) + if (!directOnly && schema.OneOf.Any()) return schema.OneOf.FlattenIfRequired(classNamesFlattener); - if (!string.IsNullOrEmpty(schema.Title)) - return new[] { schema.Title }; - if (!string.IsNullOrEmpty(schema.Xml?.Name)) - return new[] { schema.Xml.Name }; - return Enumerable.Empty(); + return []; } internal static IEnumerable FlattenSchemaIfRequired(this IList schemas, Func> subsequentGetter) { - if (schemas is null) return Enumerable.Empty(); + if (schemas is null) return []; return schemas.Count == 1 ? schemas.FlattenEmptyEntries(subsequentGetter, 1) : schemas; } private static IEnumerable FlattenIfRequired(this IList schemas, Func> subsequentGetter) { - return schemas.FlattenSchemaIfRequired(subsequentGetter).Where(static x => !string.IsNullOrEmpty(x.Title)).Select(static x => x.Title); + return schemas.FlattenSchemaIfRequired(subsequentGetter).SelectMany(static x => x.GetSchemaNames()); } - public static string GetSchemaName(this OpenApiSchema schema) + public static string GetSchemaName(this OpenApiSchema schema, bool directOnly = false) { - return schema.GetSchemaNames().LastOrDefault()?.TrimStart('$') ?? string.Empty;// OData $ref + return schema.GetSchemaNames(directOnly).LastOrDefault()?.TrimStart('$') ?? string.Empty;// OData $ref } public static bool IsReferencedSchema(this OpenApiSchema schema) @@ -173,7 +169,7 @@ public static IEnumerable GetSchemaReferenceIds(this OpenApiSchema schem return result.Distinct(); } - return Enumerable.Empty(); + return []; } private static IEnumerable FlattenEmptyEntries(this IEnumerable schemas, Func> subsequentGetter, int? maxDepth = default) { @@ -188,7 +184,7 @@ private static IEnumerable FlattenEmptyEntries(this IEnumerable 0) diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index 24f8aa9c88..cdd8849783 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -1550,18 +1550,18 @@ private CodeType CreateModelDeclarationAndType(OpenApiUrlTreeNode currentNode, O } private CodeType CreateInheritedModelDeclaration(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, string classNameSuffix, CodeNamespace codeNamespace, bool isRequestBody, string typeNameForInlineSchema) { - var allOfs = (schema.IsSemanticallyMeaningful() ? new OpenApiSchema[] { schema } : []).Union(schema.AllOf.FlattenSchemaIfRequired(static x => x.AllOf)); + var rootSchemaIsMeaningful = schema.IsSemanticallyMeaningful(); + var allOfs = (rootSchemaIsMeaningful ? new OpenApiSchema[] { schema } : []).Union(schema.AllOf.FlattenSchemaIfRequired(static x => x.AllOf)); CodeElement? codeDeclaration = null; - var className = string.Empty; var codeNamespaceFromParent = GetShortestNamespace(codeNamespace, schema); foreach (var currentSchema in allOfs) { var referenceId = GetReferenceIdFromOriginalSchema(currentSchema, schema); var shortestNamespaceName = GetModelsNamespaceNameFromReferenceId(referenceId); var shortestNamespace = string.IsNullOrEmpty(referenceId) ? codeNamespaceFromParent : rootNamespace?.FindOrAddNamespace(shortestNamespaceName); - className = (currentSchema.GetSchemaName() is string cName && !string.IsNullOrEmpty(cName) ? + var className = (currentSchema.GetSchemaName(rootSchemaIsMeaningful && currentSchema == schema) is string cName && !string.IsNullOrEmpty(cName) ? cName : - (string.IsNullOrEmpty(className) && !string.IsNullOrEmpty(typeNameForInlineSchema) ? + (!string.IsNullOrEmpty(typeNameForInlineSchema) ? typeNameForInlineSchema : currentNode.GetClassName(config.StructuredMimeTypes, operation: operation, suffix: classNameSuffix, schema: currentSchema, requestBody: isRequestBody))) .CleanupSymbolName(); @@ -1919,6 +1919,7 @@ private void TrimInheritedModels() if (relatedModels.Contains(x) || classesInUse.Contains(x)) return; if (x is CodeClass currentClass) { + //TODO this is trimming mailboxsettingsbase when it shouldn't. Most likely because one of the indices is now broken var parents = currentClass.GetInheritanceTree(false, false); if (parents.Any(y => classesDirectlyInUse.Contains(y))) return; // to support the inheritance recursive downcast foreach (var baseClass in parents) // discriminator might also be in grand parent types diff --git a/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs b/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs index 19accd89df..41732417ba 100644 --- a/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiSchemaExtensionsTests.cs @@ -84,87 +84,179 @@ public void LocalReferencesAreSupported() Assert.True(mockSchema.IsReferencedSchema()); } [Fact] - public void GetSchemaNameAllOf() + public void GetSchemaNameAllOfTitleEmpty() { var schema = new OpenApiSchema { - AllOf = new List { + AllOf = [ new() { Title = "microsoft.graph.entity" }, new() { Title = "microsoft.graph.user" } - } + ] }; var names = schema.GetSchemaNames(); - Assert.Contains("microsoft.graph.entity", names); - Assert.Contains("microsoft.graph.user", names); - Assert.Equal("microsoft.graph.user", schema.GetSchemaName()); + Assert.Empty(names); + Assert.Empty(schema.GetSchemaName()); } [Fact] - public void GetSchemaNameAllOfNested() + public void GetSchemaNameAllOfReference() { var schema = new OpenApiSchema { - AllOf = new List { + AllOf = [ new() { - AllOf = new List { + Reference = new() { + Id = "microsoft.graph.entity" + } + }, + new() { + Reference = new() { + Id = "microsoft.graph.user" + } + } + ] + }; + var names = schema.GetSchemaNames(); + Assert.Contains("entity", names); + Assert.Contains("user", names); + Assert.Equal("user", schema.GetSchemaName()); + } + [Fact] + public void GetSchemaNameAllOfNestedTitleEmpty() + { + var schema = new OpenApiSchema + { + AllOf = [ + new() { + AllOf = [ new() { Title = "microsoft.graph.entity" }, new() { Title = "microsoft.graph.user" } - } + ] } - } + ] }; var names = schema.GetSchemaNames(); - Assert.Contains("microsoft.graph.entity", names); - Assert.Contains("microsoft.graph.user", names); - Assert.Equal("microsoft.graph.user", schema.GetSchemaName()); + Assert.Empty(names); + Assert.Empty(schema.GetSchemaName()); } [Fact] - public void GetSchemaNameAnyOf() + public void GetSchemaNameAllOfNestedReference() { var schema = new OpenApiSchema { - AnyOf = new List { + AllOf = [ + new() { + AllOf = [ + new() { + Reference = new() { + Id = "microsoft.graph.entity" + } + }, + new() { + Reference = new() { + Id = "microsoft.graph.user" + } + } + ] + } + ] + }; + var names = schema.GetSchemaNames(); + Assert.Contains("entity", names); + Assert.Contains("user", names); + Assert.Equal("user", schema.GetSchemaName()); + } + [Fact] + public void GetSchemaNameAnyOfTitleEmpty() + { + var schema = new OpenApiSchema + { + AnyOf = [ new() { Title = "microsoft.graph.entity" }, new() { Title = "microsoft.graph.user" } - } + ] }; var names = schema.GetSchemaNames(); - Assert.Contains("microsoft.graph.entity", names); - Assert.Contains("microsoft.graph.user", names); - Assert.Equal("microsoft.graph.user", schema.GetSchemaName()); + Assert.Empty(names); + Assert.Empty(schema.GetSchemaName()); } [Fact] - public void GetSchemaNameOneOf() + public void GetSchemaNameAnyOfReference() { var schema = new OpenApiSchema { - OneOf = new List { + AnyOf = [ + new() { + Reference = new() { + Id = "microsoft.graph.entity" + } + }, + new() { + Reference = new() { + Id = "microsoft.graph.user" + } + } + ] + }; + var names = schema.GetSchemaNames(); + Assert.Contains("entity", names); + Assert.Contains("user", names); + Assert.Equal("user", schema.GetSchemaName()); + } + [Fact] + public void GetSchemaNameOneOfTitleEmpty() + { + var schema = new OpenApiSchema + { + OneOf = [ new() { Title = "microsoft.graph.entity" }, new() { Title = "microsoft.graph.user" } - } + ] }; var names = schema.GetSchemaNames(); - Assert.Contains("microsoft.graph.entity", names); - Assert.Contains("microsoft.graph.user", names); - Assert.Equal("microsoft.graph.user", schema.GetSchemaName()); + Assert.Empty(names); + Assert.Empty(schema.GetSchemaName()); + } + [Fact] + public void GetSchemaNameOneOfReference() + { + var schema = new OpenApiSchema + { + OneOf = [ + new() { + Reference = new() { + Id = "microsoft.graph.entity" + } + }, + new() { + Reference = new() { + Id = "microsoft.graph.user" + } + } + ] + }; + var names = schema.GetSchemaNames(); + Assert.Contains("entity", names); + Assert.Contains("user", names); + Assert.Equal("user", schema.GetSchemaName()); } [Fact] - public void GetSchemaNameItems() + public void GetSchemaNameItemsTitleEmpty() { var schema = new OpenApiSchema { @@ -174,20 +266,51 @@ public void GetSchemaNameItems() }, }; var names = schema.GetSchemaNames(); - Assert.Contains("microsoft.graph.entity", names); - Assert.Equal("microsoft.graph.entity", schema.GetSchemaName()); + Assert.Empty(names); + Assert.Empty(schema.GetSchemaName()); + } + [Fact] + public void GetSchemaNameItemsReference() + { + var schema = new OpenApiSchema + { + Items = new() + { + Reference = new() + { + Id = "microsoft.graph.entity" + } + }, + }; + var names = schema.GetSchemaNames(); + Assert.Contains("entity", names); + Assert.Equal("entity", schema.GetSchemaName()); Assert.Single(names); } [Fact] - public void GetSchemaNameTitle() + public void GetSchemaNameTitleEmpty() { var schema = new OpenApiSchema { Title = "microsoft.graph.entity" }; var names = schema.GetSchemaNames(); - Assert.Contains("microsoft.graph.entity", names); - Assert.Equal("microsoft.graph.entity", schema.GetSchemaName()); + Assert.Empty(names); + Assert.Empty(schema.GetSchemaName()); + } + [Fact] + public void GetSchemaNameReference() + { + var schema = new OpenApiSchema + { + Reference = new() + { + Id = "microsoft.graph.entity" + } + }; + var names = schema.GetSchemaNames(); + Assert.Contains("entity", names); + Assert.Equal("entity", schema.GetSchemaName()); Assert.Single(names); } [Fact] diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index ca1a16ec12..6dec359fd0 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -7472,6 +7472,104 @@ public async Task InheritanceWithAllOfWith3Parts3Schema() Assert.Single(resultClass.Properties.Where(static x => x.Name.Equals("groupprop2", StringComparison.OrdinalIgnoreCase))); } [Fact] + public async Task InheritanceWithAllOfWith2Parts1Schema1InlineNoDiscriminator() + { + var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName()); + await using var fs = await GetDocumentStream(@"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 +servers: + - url: https://graph.microsoft.com/v1.0 +paths: + /group: + get: + responses: + '200': + content: + application/json: + schema: + $ref: '#/components/schemas/microsoft.graph.group' +components: + schemas: + microsoft.graph.directoryObject: + title: 'directoryObject' + required: ['@odata.type'] + type: 'object' + properties: + '@odata.type': + type: 'string' + default: '#microsoft.graph.directoryObject' + microsoft.graph.group: + type: 'object' + allOf: + - '$ref': '#/components/schemas/microsoft.graph.directoryObject' + - title: 'group part 1' + type: 'object' + properties: + groupprop1: + type: 'string'"); + var mockLogger = new Mock>(); + var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", OpenAPIFilePath = tempFilePath }, _httpClient); + var document = await builder.CreateOpenApiDocumentAsync(fs); + var node = builder.CreateUriSpace(document); + var codeModel = builder.CreateSourceModel(node); + var resultClass = codeModel.FindChildByName("Group"); + Assert.NotNull(resultClass); + Assert.Equal("directoryObject", resultClass.StartBlock.Inherits?.Name, StringComparer.OrdinalIgnoreCase); + Assert.Single(resultClass.Properties); + Assert.Single(resultClass.Properties.Where(static x => x.Name.Equals("groupprop1", StringComparison.OrdinalIgnoreCase))); + } + [Fact] + public async Task InheritanceWithAllOfWith1Part1SchemaAndPropertiesNoDiscriminator() + { + var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName()); + await using var fs = await GetDocumentStream(@"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 +servers: + - url: https://graph.microsoft.com/v1.0 +paths: + /group: + get: + responses: + '200': + content: + application/json: + schema: + $ref: '#/components/schemas/microsoft.graph.group' +components: + schemas: + microsoft.graph.directoryObject: + title: 'directoryObject' + required: ['@odata.type'] + type: 'object' + properties: + '@odata.type': + type: 'string' + default: '#microsoft.graph.directoryObject' + microsoft.graph.group: + allOf: + - '$ref': '#/components/schemas/microsoft.graph.directoryObject' + type: 'object' + properties: + groupprop1: + type: 'string'"); + var mockLogger = new Mock>(); + var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", OpenAPIFilePath = tempFilePath }, _httpClient); + var document = await builder.CreateOpenApiDocumentAsync(fs); + var node = builder.CreateUriSpace(document); + var codeModel = builder.CreateSourceModel(node); + var resultClass = codeModel.FindChildByName("Group"); + Assert.NotNull(resultClass); + Assert.Equal("directoryObject", resultClass.StartBlock.Inherits?.Name, StringComparer.OrdinalIgnoreCase); + Assert.Single(resultClass.Properties); + Assert.Single(resultClass.Properties.Where(static x => x.Name.Equals("groupprop1", StringComparison.OrdinalIgnoreCase))); + } + [Fact] public async Task InheritanceWithAllOfWith3Parts1Schema2Inline() { var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName());