diff --git a/CHANGELOG.md b/CHANGELOG.md index e17050735e..7327f1b128 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 - Fixed a bug where composed types wrappers would not build in CSharp. - Fixed a bug where the type name for inherited inline models would be incorrect. [#5610](https://github.com/microsoft/kiota/issues/5610) - Fixes typing inconsistencies in generated code and libraries in Python [kiota-python#333](https://github.com/microsoft/kiota-python/issues/333) +- Fixes generation of superfluous fields for Models with discriminator due to refiners adding the same properties to the same model [#4178](https://github.com/microsoft/kiota/issues/4178). ## [1.19.1] - 2024-10-11 diff --git a/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs b/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs index 5289f6546f..6cdaebe093 100644 --- a/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs +++ b/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs @@ -485,8 +485,10 @@ private static CodeType ConvertComposedTypeToWrapper(CodeClass codeClass, CodeCo if (!supportsInnerClasses) { var @namespace = codeClass.GetImmediateParentOfType(); - if (@namespace.FindChildByName(codeComposedType.Name, false) is CodeClass { OriginalComposedType: null }) + if (@namespace.FindChildByName(codeComposedType.Name, false) is { OriginalComposedType: null }) codeComposedType.Name = $"{codeComposedType.Name}Wrapper"; + if (GetAlreadyExistingComposedCodeType(@namespace, codeComposedType) is { } existingCodeType) + return existingCodeType; newClass = @namespace.AddClass(new CodeClass { Name = codeComposedType.Name, @@ -499,8 +501,10 @@ private static CodeType ConvertComposedTypeToWrapper(CodeClass codeClass, CodeCo Deprecation = codeComposedType.Deprecation, }).Last(); } - else if (codeComposedType.TargetNamespace is CodeNamespace targetNamespace) + else if (codeComposedType.TargetNamespace is { } targetNamespace) { + if (GetAlreadyExistingComposedCodeType(targetNamespace, codeComposedType) is { } existingCodeType) + return existingCodeType; newClass = targetNamespace.AddClass(new CodeClass { Name = codeComposedType.Name, @@ -523,6 +527,8 @@ private static CodeType ConvertComposedTypeToWrapper(CodeClass codeClass, CodeCo { if (codeComposedType.Name.Equals(codeClass.Name, StringComparison.OrdinalIgnoreCase) || codeClass.FindChildByName(codeComposedType.Name, false) is not null) codeComposedType.Name = $"{codeComposedType.Name}Wrapper"; + if (GetAlreadyExistingComposedCodeType(codeClass, codeComposedType) is { } existingCodeType) + return existingCodeType; newClass = codeClass.AddInnerClass(new CodeClass { Name = codeComposedType.Name, @@ -602,6 +608,24 @@ private static CodeType ConvertComposedTypeToWrapper(CodeClass codeClass, CodeCo ActionOf = codeComposedType.ActionOf, }; } + + private static CodeType? GetAlreadyExistingComposedCodeType(IBlock targetNamespace, CodeComposedTypeBase codeComposedType) + { + if (targetNamespace.FindChildByName(codeComposedType.Name, false) is { OriginalComposedType: not null } existingClass && existingClass.OriginalComposedType.Name.Equals(codeComposedType.Name, StringComparison.OrdinalIgnoreCase)) + { // the composed type was already added/created and the typeDefinition(codeclass) is already present in the namespace/class. + return new CodeType + { + Name = codeComposedType.Name, + TypeDefinition = existingClass, + CollectionKind = codeComposedType.CollectionKind, + IsNullable = codeComposedType.IsNullable, + ActionOf = codeComposedType.ActionOf, + }; + } + + return null; + } + protected static void MoveClassesWithNamespaceNamesUnderNamespace(CodeElement currentElement) { if (currentElement is CodeClass currentClass && diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index b1c6dcc045..606bbe5c62 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -581,6 +581,103 @@ public async Task DoesntConflictOnModelsNamespaceAsync() Assert.NotNull(innerRequestBuilderNS.FindChildByName("InnerRequestBuilder", false)); } + [Theory] + [InlineData(GenerationLanguage.CSharp)] + [InlineData(GenerationLanguage.Java)] + [InlineData(GenerationLanguage.TypeScript)] + [InlineData(GenerationLanguage.Python)] + [InlineData(GenerationLanguage.Go)] + [InlineData(GenerationLanguage.PHP)] + [InlineData(GenerationLanguage.Ruby)] + public async Task DoesNotAddSuperflousFieldsToModelsAsync(GenerationLanguage language) + { + var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName()); + await using var fs = await GetDocumentStreamAsync(@"openapi: 3.0.3 +info: + title: Example API + version: 1.0.0 +servers: + - url: ""https://localhost:8080"" +paths: + ""/api/all"": + post: + requestBody: + content: + application/json: + schema: + $ref: ""#/components/schemas/AorB"" + required: true + responses: + ""200"": + $ref: ""#/components/responses/AorBResponse"" +components: + schemas: + A: + type: object + required: + - type + properties: + type: + type: string + default: ""a"" + B: + type: object + required: + - type + properties: + type: + type: string + default: ""b"" + AorB: + oneOf: + - $ref: ""#/components/schemas/A"" + - $ref: ""#/components/schemas/B"" + discriminator: + propertyName: type + mapping: + a: ""#/components/schemas/A"" + b: ""#/components/schemas/B"" + responses: + AorBResponse: + description: mandatory + content: + application/json: + schema: + $ref: ""#/components/schemas/AorB"""); + var mockLogger = new Mock>(); + var generationConfiguration = new GenerationConfiguration { ClientClassName = "Graph", OpenAPIFilePath = tempFilePath, Language = language }; // we can use any language that creates wrapper types for composed types in different ways + var builder = new KiotaBuilder(mockLogger.Object, generationConfiguration, _httpClient); + var document = await builder.CreateOpenApiDocumentAsync(fs); + var node = builder.CreateUriSpace(document); + var codeModel = builder.CreateSourceModel(node); + await builder.ApplyLanguageRefinementAsync(generationConfiguration, codeModel, CancellationToken.None); + var requestBuilderNamespace = codeModel.FindNamespaceByName("ApiSdk.api.all"); + Assert.NotNull(requestBuilderNamespace); + if (language == GenerationLanguage.TypeScript || language == GenerationLanguage.Go) + {// these languages use CodeFiles + var requestExecutorMethod = codeModel.FindChildByName("post"); + Assert.NotNull(requestExecutorMethod); + var returnType = requestExecutorMethod.ReturnType as CodeType; + var returnTypeDefinition = returnType.TypeDefinition as CodeInterface; + if (language == GenerationLanguage.TypeScript) + { + Assert.Equal(2, returnTypeDefinition.Properties.Count()); + } + if (language == GenerationLanguage.Go) + { + Assert.Equal(4, returnTypeDefinition.Methods.Count());// a getter and a setter for each property in Go. + } + } + else + { + var allRequestBuilderClass = requestBuilderNamespace.FindChildByName("allRequestBuilder", false); + var executor = allRequestBuilderClass.Methods.FirstOrDefault(m => m.IsOfKind(CodeMethodKind.RequestExecutor)); + Assert.NotNull(executor); + var returnType = executor.ReturnType as CodeType; + var returnTypeDefinition = returnType.TypeDefinition as CodeClass; + Assert.Equal(2, returnTypeDefinition.Properties.Count()); + } + } [Fact] public async Task NamesComponentsInlineSchemasProperlyAsync() {