From b0459ef1a6ab2b40394529bbcf4e2512c96b8590 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 15 May 2024 10:24:48 -0400 Subject: [PATCH 1/2] - fixes invalid inheritance cases --- CHANGELOG.md | 5 +-- .../Extensions/OpenApiSchemaExtensions.cs | 12 +++---- src/Kiota.Builder/KiotaBuilder.cs | 11 ++++--- .../Kiota.Builder.Tests/KiotaBuilderTests.cs | 32 +++++++++---------- 4 files changed, 31 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 801b383df0..85bb9e8dc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added missing nullable directives for CLI generation. - Fixed handling of nested arrays to be handled as `UntypedNode` instances [#4549](https://github.com/microsoft/kiota/issues/4549) - Fixed `InvalidOperationException` thrown when serializing IBacked models with no changes present in the additional data in dotnet [microsoftgraph/msgraph-sdk-dotnet#2471](https://github.com/microsoftgraph/msgraph-sdk-dotnet/issues/2471). +- Fixed a bug where enums could be considered inheritance parents. [#4640](https://github.com/microsoft/kiota/issues/4640) - Fixed `RequestConfiguration` Classes dropped in RequestBuilder methods in python [#4535](https://github.com/microsoft/kiota/issues/4535) - Fixed incorrect optional types in method parameters in Python [#4507](https://github.com/microsoft/kiota/issues/4507) - Changed enum parsing methods to return nil in the default case in Go [#4621](https://github.com/microsoft/kiota/issues/4621) @@ -31,9 +32,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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) - Fixed a bug where some allOf scenarios would be missing properties if type object wasn't set on the schema. [#4074](https://github.com/microsoft/kiota/issues/4074) -- Fixed a bug where schema with multiple allOf entries would incorrectly get merged to inherit from the first entry [#4428] (https://github.com/microsoft/kiota/issues/4428) +- Fixed a bug where schema with multiple allOf entries would incorrectly get merged to inherit from the first entry [#4428](https://github.com/microsoft/kiota/issues/4428) - Fixes constructor generation for nullable properties that are initialized as null in C#,Java and PHP -- Fixed a bug where the hash alias in typescript wasn't being generated uniformly for similar interfaces [#84](https://github.com/microsoftgraph/msgraph-beta-sdk-typescript/issues/84) +- Fixed a bug where the hash alias in typescript wasn't being generated uniformly for similar interfaces [microsoftgraph/msgraph-beta-sdk-typescript#84](https://github.com/microsoftgraph/msgraph-beta-sdk-typescript/issues/84) ## [1.14.0] - 2024-05-02 diff --git a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs index 17a67c842c..272c31e57d 100644 --- a/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiSchemaExtensions.cs @@ -77,8 +77,8 @@ public static bool IsInclusiveUnion(this OpenApiSchema? schema) public static bool IsInherited(this OpenApiSchema? schema) { if (schema is null) return false; - var meaningfulMemberSchemas = schema.AllOf.FlattenSchemaIfRequired(static x => x.AllOf).Where(static x => x.IsSemanticallyMeaningful()).ToArray(); - var isRootSchemaMeaningful = schema.IsSemanticallyMeaningful(); + var meaningfulMemberSchemas = schema.AllOf.FlattenSchemaIfRequired(static x => x.AllOf).Where(static x => x.IsSemanticallyMeaningful(ignoreEnums: true, ignoreArrays: true, ignoreType: true)).ToArray(); + var isRootSchemaMeaningful = schema.IsSemanticallyMeaningful(ignoreEnums: true, ignoreArrays: true, ignoreType: true); return meaningfulMemberSchemas.Count(static x => !string.IsNullOrEmpty(x.Reference?.Id)) == 1 && (meaningfulMemberSchemas.Count(static x => string.IsNullOrEmpty(x.Reference?.Id)) == 1 || isRootSchemaMeaningful); @@ -141,13 +141,13 @@ public static bool IsComposedEnum(this OpenApiSchema schema) 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) + public static bool IsSemanticallyMeaningful(this OpenApiSchema schema, bool ignoreNullableObjects = false, bool ignoreEnums = false, bool ignoreArrays = false, bool ignoreType = false) { if (schema is null) return false; return schema.HasAnyProperty() || - schema.Enum is { Count: > 0 } || - schema.Items != null || - (!string.IsNullOrEmpty(schema.Type) && + (!ignoreEnums && schema.Enum is { Count: > 0 }) || + (!ignoreArrays && schema.Items != null) || + (!ignoreType && !string.IsNullOrEmpty(schema.Type) && ((ignoreNullableObjects && !"object".Equals(schema.Type, StringComparison.OrdinalIgnoreCase)) || !ignoreNullableObjects)) || !string.IsNullOrEmpty(schema.Format) || diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index 559815502a..d193292dbf 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -1588,9 +1588,12 @@ private CodeClass CreateInheritedModelDeclaration(OpenApiUrlTreeNode currentNode (false, null, not null) => AddModelDeclarationIfDoesntExist(currentNode, referencedSchema, className, shortestNamespace), // empty schema + inline schema (false, not null, null) => AddModelDeclarationIfDoesntExist(currentNode, inlineSchema, className, shortestNamespace), - // meaningless scenarios + // too much information but we can make a choice -> maps to properties + inline schema + (true, not null, not null) when inlineSchema.HasAnyProperty() => AddModelDeclarationIfDoesntExist(currentNode, schema, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, inlineSchema, operation, classNameSuffix, codeNamespace, isRequestBody, typeNameForInlineSchema)), + // too much information but we can make a choice -> maps to properties + referenced schema + (true, not null, not null) when referencedSchema.HasAnyProperty() => AddModelDeclarationIfDoesntExist(currentNode, schema, className, shortestNamespace, CreateInheritedModelDeclaration(currentNode, referencedSchema, operation, classNameSuffix, codeNamespace, isRequestBody, string.Empty)), + // meaningless scenario (false, null, null) or (true, not null, not null) => throw new InvalidOperationException("invalid inheritance case"), - }; if (codeDeclaration is not CodeClass currentClass) throw new InvalidOperationException("Inheritance is only supported for classes"); if (!currentClass.Documentation.DescriptionAvailable && @@ -1894,9 +1897,7 @@ private CodeClass AddModelClass(OpenApiUrlTreeNode currentNode, OpenApiSchema sc } var mappings = GetDiscriminatorMappings(currentNode, schema, currentNamespace, newClass) - .Where(x => x.Value is CodeType type && - type.TypeDefinition != null && - type.TypeDefinition is CodeClass definition && + .Where(x => x.Value is { TypeDefinition: CodeClass definition } && definition.DerivesFrom(newClass)); // only the mappings that derive from the current class AddDiscriminatorMethod(newClass, schema.GetDiscriminatorPropertyName(), mappings, static s => s); diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index 66dd9e4f5f..4489dca7f8 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -3983,7 +3983,7 @@ public void IntersectionOfInlineSchemasWorks() [Fact] public void InheritedTypeWithInlineSchemaWorks() { - var baseObjet = new OpenApiSchema + var baseObject = new OpenApiSchema { Type = "object", Properties = new Dictionary { @@ -4014,11 +4014,11 @@ public void InheritedTypeWithInlineSchemaWorks() }, UnresolvedReference = false }; - var derivedObjet = new OpenApiSchema + var derivedObject = new OpenApiSchema { Type = "object", - AllOf = new List { - baseObjet, + AllOf = [ + baseObject, new OpenApiSchema { Type = "object", Properties = new Dictionary { @@ -4037,7 +4037,7 @@ public void InheritedTypeWithInlineSchemaWorks() } }, } - }, + ], Reference = new OpenApiReference { Id = "subNS.derivedObject", @@ -4048,8 +4048,8 @@ public void InheritedTypeWithInlineSchemaWorks() var secondLevelDerivedObject = new OpenApiSchema { Type = "object", - AllOf = new List { - derivedObjet, + AllOf = [ + derivedObject, new OpenApiSchema { Type = "object", Properties = new Dictionary { @@ -4060,7 +4060,7 @@ public void InheritedTypeWithInlineSchemaWorks() } } } - }, + ], Reference = new OpenApiReference { Id = "subNS.secondLevelDerivedObject", @@ -4082,7 +4082,7 @@ public void InheritedTypeWithInlineSchemaWorks() ["200"] = new OpenApiResponse { Content = { ["application/json"] = new OpenApiMediaType { - Schema = derivedObjet + Schema = derivedObject } } }, @@ -4095,10 +4095,10 @@ public void InheritedTypeWithInlineSchemaWorks() { Schemas = new Dictionary { { - "subNS.baseObject", baseObjet + "subNS.baseObject", baseObject }, { - "subNS.derivedObject", derivedObjet + "subNS.derivedObject", derivedObject }, { "subNS.secondLevelDerivedObject", secondLevelDerivedObject @@ -4119,12 +4119,12 @@ public void InheritedTypeWithInlineSchemaWorks() var executorReturnType = requestExecutorMethod.ReturnType as CodeType; Assert.NotNull(executorReturnType); Assert.Contains("derivedObject", requestExecutorMethod.ReturnType.Name); - var secondLevelDerivedClass = codeModel.FindChildByName("derivedObject"); - Assert.NotNull(secondLevelDerivedObject); - var factoryMethod = secondLevelDerivedClass.GetChildElements(true).OfType().FirstOrDefault(x => x.IsOfKind(CodeMethodKind.Factory)); + var derivedObjectClass = codeModel.FindChildByName("derivedObject"); + Assert.NotNull(derivedObjectClass); + var factoryMethod = derivedObjectClass.GetChildElements(true).OfType().FirstOrDefault(x => x.IsOfKind(CodeMethodKind.Factory)); Assert.NotNull(factoryMethod); - Assert.Equal("kind", secondLevelDerivedClass.DiscriminatorInformation.DiscriminatorPropertyName); - Assert.NotEmpty(secondLevelDerivedClass.DiscriminatorInformation.DiscriminatorMappings); + Assert.Equal("kind", derivedObjectClass.DiscriminatorInformation.DiscriminatorPropertyName); + Assert.NotEmpty(derivedObjectClass.DiscriminatorInformation.DiscriminatorMappings); } [InlineData("string", "", "string")]// https://spec.openapis.org/registry/format/ [InlineData("string", "commonmark", "string")] From 9d8a0a1eb8edb2f2d8d72a14363f99519e8fd32b Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 15 May 2024 10:31:05 -0400 Subject: [PATCH 2/2] - formatting fixes Signed-off-by: Vincent Biret --- tests/Kiota.Builder.Tests/KiotaBuilderTests.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index 4489dca7f8..b9297956ea 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -4019,7 +4019,8 @@ public void InheritedTypeWithInlineSchemaWorks() Type = "object", AllOf = [ baseObject, - new OpenApiSchema { + new OpenApiSchema + { Type = "object", Properties = new Dictionary { { @@ -4028,7 +4029,8 @@ public void InheritedTypeWithInlineSchemaWorks() } } }, - Discriminator = new OpenApiDiscriminator { + Discriminator = new OpenApiDiscriminator + { PropertyName = "kind", Mapping = new Dictionary { { @@ -4050,7 +4052,8 @@ public void InheritedTypeWithInlineSchemaWorks() Type = "object", AllOf = [ derivedObject, - new OpenApiSchema { + new OpenApiSchema + { Type = "object", Properties = new Dictionary { {