Merge pull request #5657 from microsoft/andrueastman/fixSuperflousFields

Fix superfluous fields when converting composed types to wrapper types.
microsoft · Oct 22, 2024 · cb6f67c · cb6f67c
2 parents 7c3a878 + dda0486
commit cb6f67c
Show file tree

Hide file tree

Showing 3 changed files with 124 additions and 2 deletions.
diff --git 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
@@ -485,8 +485,10 @@ private static CodeType ConvertComposedTypeToWrapper(CodeClass codeClass, CodeCo
         if (!supportsInnerClasses)
         {
             var @namespace = codeClass.GetImmediateParentOfType<CodeNamespace>();
-            if (@namespace.FindChildByName<CodeClass>(codeComposedType.Name, false) is CodeClass { OriginalComposedType: null })
+            if (@namespace.FindChildByName<CodeClass>(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<CodeProperty>(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<CodeClass>(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
@@ -581,6 +581,103 @@ public async Task DoesntConflictOnModelsNamespaceAsync()
         Assert.NotNull(innerRequestBuilderNS.FindChildByName<CodeClass>("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<ILogger<KiotaBuilder>>();
+        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<CodeMethod>("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<CodeClass>("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()
     {