Merge pull request #3340 from microsoft/bugfix/composed-type-member-n…

…aming bugfix/composed type member naming
microsoft · Sep 22, 2023 · 9b7fc49 · 9b7fc49
2 parents 387bfd0 + 0c7bcc0
commit 9b7fc49
Show file tree

Hide file tree

Showing 20 changed files with 139 additions and 102 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,7 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Localhost based descriptions are not cached anymore to facilitate development workflows. [#3316](https://github.com/microsoft/kiota/issues/3316)
-- Changed parameter order in with_url method body to match the signature of RequestBuilder constructor in Python. [#3328](https://github.com/microsoft/kiota/issues/3328
+- Fixed a bug where inline composed types for components schemas would have the wrong name. [#3067](https://github.com/microsoft/kiota/issues/3067)
+- Changed parameter order in with_url method body to match the signature of RequestBuilder constructor in Python. [#3328](https://github.com/microsoft/kiota/issues/3328)
 - Removed redundant undefined qualifier in TypeScript for properties. [#3244](https://github.com/microsoft/kiota/issues/3244)
 - The default status code response is now used as 4XX and 5XX when those class responses are not provided in the description. [#3245](https://github.com/microsoft/kiota/issues/3245)
 - Adds codes files in typescript to reduce number of generated files. [#2116](https://github.com/microsoft/kiota/issues/2116)

diff --git a/it/config.json b/it/config.json
@@ -259,32 +259,32 @@
     ],
     "IdempotencySuppressions": [
       {
-        "Language": "typescript",
-        "Rationale": "https://github.com/microsoft/kiota/issues/1812"
+        "Language": "csharp",
+        "Rationale": "https://github.com/microsoft/kiota/issues/2952"
       },
       {
         "Language": "go",
-        "Rationale": "https://github.com/microsoft/kiota/issues/3067"
+        "Rationale": "https://github.com/microsoft/kiota/issues/2834"
       },
       {
         "Language": "php",
-        "Rationale": "https://github.com/microsoft/kiota/issues/3067"
+        "Rationale": "https://github.com/microsoft/kiota/issues/2964"
       },
       {
         "Language": "java",
-        "Rationale": "https://github.com/microsoft/kiota/issues/3067"
+        "Rationale": "https://github.com/microsoft/kiota/issues/2842"
       },
       {
-        "Language": "csharp",
-        "Rationale": "https://github.com/microsoft/kiota/issues/3067"
+        "Language": "python",
+        "Rationale": "https://github.com/microsoft/kiota/issues/2842"
       },
       {
-        "Language": "ruby",
-        "Rationale": "https://github.com/microsoft/kiota/issues/1816"
+        "Language": "typescript",
+        "Rationale": "https://github.com/microsoft/kiota/issues/1812"
       },
       {
-        "Language": "python",
-        "Rationale": "https://github.com/microsoft/kiota/issues/3067"
+        "Language": "ruby",
+        "Rationale": "https://github.com/microsoft/kiota/issues/1816"
       }
     ]
   },
@@ -324,14 +324,10 @@
       }
     ],
     "IdempotencySuppressions": [
-      {
-        "Language": "php",
-        "Rationale": "https://github.com/microsoft/kiota/issues/3067"
-      },
       {
         "Language": "typescript",
         "Rationale": "https://github.com/microsoft/kiota/issues/1812"
       }
     ]
   }
-}
+}
diff --git a/src/Kiota.Builder/BaseCodeParameterOrderComparer.cs b/src/Kiota.Builder/BaseCodeParameterOrderComparer.cs
@@ -28,15 +28,12 @@ protected virtual int GetKindOrderHint(CodeParameterKind kind)
             CodeParameterKind.Path => 4,
             CodeParameterKind.RequestConfiguration => 5,
             CodeParameterKind.RequestBody => 6,
-#pragma warning disable CS0618
-            CodeParameterKind.ResponseHandler => 7,
-#pragma warning restore CS0618
-            CodeParameterKind.Serializer => 8,
-            CodeParameterKind.BackingStore => 9,
-            CodeParameterKind.SetterValue => 10,
-            CodeParameterKind.ParseNode => 11,
-            CodeParameterKind.Custom => 12,
-            _ => 13,
+            CodeParameterKind.Serializer => 7,
+            CodeParameterKind.BackingStore => 8,
+            CodeParameterKind.SetterValue => 9,
+            CodeParameterKind.ParseNode => 10,
+            CodeParameterKind.Custom => 11,
+            _ => 12,
         };
     }
     private const int OptionalWeight = 10000;

diff --git a/src/Kiota.Builder/CodeDOM/CodeBlock.cs b/src/Kiota.Builder/CodeDOM/CodeBlock.cs
@@ -75,6 +75,7 @@ private T HandleDuplicatedExceptions<T>(T element, CodeElement returnedValue) wh
         if (returnedValue == element)
             return element;
         if (element is CodeMethod currentMethod)
+        {
             if (currentMethod.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility) &&
                 returnedValue is CodeProperty cProp &&
                 cProp.IsOfKind(CodePropertyKind.RequestBuilder) &&
@@ -99,6 +100,17 @@ returnedValue is CodeProperty cProp &&
                         return result2;
                 }
             }
+        }
+        else if (element is CodeProperty currentProperty &&
+                currentProperty.Kind is CodePropertyKind.Custom &&
+                returnedValue is CodeClass returnedClass && returnedClass.Kind is CodeClassKind.Model &&
+                InnerChildElements.TryAdd($"{element.Name}-property", currentProperty))
+            return element; // inline type property: transforming union type to wrapper class
+        else if (element is CodeClass currentClass &&
+                currentClass.Kind is CodeClassKind.Model &&
+                returnedValue is CodeProperty returnedProperty && returnedProperty.Kind is CodePropertyKind.Custom &&
+                InnerChildElements.TryAdd($"{element.Name}-model", currentClass))
+            return element; // inline type property: transforming wrapper class to union type
 
         if (element.GetType() == returnedValue.GetType())
             return (T)returnedValue;

diff --git a/src/Kiota.Builder/CodeDOM/CodeClass.cs b/src/Kiota.Builder/CodeDOM/CodeClass.cs
@@ -183,7 +183,7 @@ public bool ContainsPropertyWithWireName(string wireName)
     }
     public IEnumerable<CodeClass> AddInnerClass(params CodeClass[] codeClasses)
     {
-        if (codeClasses == null || codeClasses.Any(x => x == null))
+        if (codeClasses == null || codeClasses.Any(static x => x == null))
             throw new ArgumentNullException(nameof(codeClasses));
         if (!codeClasses.Any())
             throw new ArgumentOutOfRangeException(nameof(codeClasses));

diff --git a/src/Kiota.Builder/CodeDOM/CodeComposedTypeBase.cs b/src/Kiota.Builder/CodeDOM/CodeComposedTypeBase.cs
@@ -1,23 +1,34 @@
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
 
 namespace Kiota.Builder.CodeDOM;
 /// <summary>
 /// The base class for composed types like union or exclusion.
 /// </summary>
-public abstract class CodeComposedTypeBase : CodeTypeBase, IDiscriminatorInformationHolder
+public abstract class CodeComposedTypeBase : CodeTypeBase, IDiscriminatorInformationHolder, IDeprecableElement
 {
+    private static string NormalizeKey(CodeType codeType) => $"{codeType.Name}_{codeType.CollectionKind}";
     public void AddType(params CodeType[] codeTypes)
     {
+        ArgumentNullException.ThrowIfNull(codeTypes);
+        if (Array.Exists(codeTypes, static x => x == null))
+            throw new ArgumentNullException(nameof(codeTypes), "One of the provided types was null");
         EnsureElementsAreChildren(codeTypes);
-        foreach (var codeType in codeTypes.Where(x => x != null && !Types.Contains(x)))
-            types.Add(codeType);
+        foreach (var codeType in codeTypes)
+            if (!types.TryAdd(NormalizeKey(codeType), codeType))
+                throw new InvalidOperationException($"The type {codeType.Name} was already added");
     }
-    private readonly List<CodeType> types = new();
+    public bool ContainsType(CodeType codeType)
+    {
+        ArgumentNullException.ThrowIfNull(codeType);
+        return types.ContainsKey(NormalizeKey(codeType));
+    }
+    private readonly ConcurrentDictionary<string, CodeType> types = new(StringComparer.OrdinalIgnoreCase);
     public IEnumerable<CodeType> Types
     {
-        get => types;
+        get => types.Values.OrderBy(NormalizeKey, StringComparer.OrdinalIgnoreCase);
     }
     private DiscriminatorInformation? _discriminatorInformation;
     /// <inheritdoc />
@@ -45,6 +56,7 @@ protected override TChildType BaseClone<TChildType>(CodeTypeBase source, bool cl
         if (sourceComposed.Types?.Any() ?? false)
             AddType(sourceComposed.Types.ToArray());
         DiscriminatorInformation = (DiscriminatorInformation)sourceComposed.DiscriminatorInformation.Clone();
+        Deprecation = sourceComposed.Deprecation;
         return this is TChildType casted ? casted : throw new InvalidCastException($"Cannot cast {GetType().Name} to {typeof(TChildType).Name}");
     }
     /// <summary>
@@ -54,4 +66,9 @@ public CodeNamespace? TargetNamespace
     {
         get; set;
     }
+    public DeprecationInformation? Deprecation
+    {
+        get;
+        set;
+    }
 }
diff --git a/src/Kiota.Builder/CodeDOM/CodeIntersectionType.cs b/src/Kiota.Builder/CodeDOM/CodeIntersectionType.cs
@@ -5,17 +5,10 @@ namespace Kiota.Builder.CodeDOM;
 /// <summary>
 /// The base class for exclusion types. (one of the properties at a time)
 /// </summary>
-public class CodeIntersectionType : CodeComposedTypeBase, ICloneable, IDeprecableElement
+public class CodeIntersectionType : CodeComposedTypeBase, ICloneable
 {
-    public DeprecationInformation? Deprecation
-    {
-        get; set;
-    }
-
     public override object Clone()
     {
-        var value = new CodeIntersectionType().BaseClone<CodeIntersectionType>(this);
-        value.Deprecation = Deprecation;
-        return value;
+        return new CodeIntersectionType().BaseClone<CodeIntersectionType>(this);
     }
 }
diff --git a/src/Kiota.Builder/CodeDOM/CodeParameter.cs b/src/Kiota.Builder/CodeDOM/CodeParameter.cs
@@ -12,8 +12,6 @@ public enum CodeParameterKind
     /// The request headers when used as a executor/generator parameter. Most languages use the intermediate RequestConfiguration wrapper class.
     ///</summary>
     Headers,
-    [Obsolete("The parameter kind is replaced by a request option instead")]
-    ResponseHandler,
     RequestBody,
     SetterValue,
     RequestAdapter,

diff --git a/src/Kiota.Builder/CodeDOM/CodeUnionType.cs b/src/Kiota.Builder/CodeDOM/CodeUnionType.cs
@@ -5,16 +5,10 @@ namespace Kiota.Builder.CodeDOM;
 /// <summary>
 /// The base class for union types. (anyOf multiple properties at a time)
 /// </summary>
-public class CodeUnionType : CodeComposedTypeBase, ICloneable, IDeprecableElement
+public class CodeUnionType : CodeComposedTypeBase, ICloneable
 {
-    public DeprecationInformation? Deprecation
-    {
-        get; set;
-    }
     public override object Clone()
     {
-        var value = new CodeUnionType().BaseClone<CodeUnionType>(this);
-        value.Deprecation = Deprecation;
-        return value;
+        return new CodeUnionType().BaseClone<CodeUnionType>(this);
     }
 }
diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs
@@ -1334,21 +1334,6 @@ private void CreateOperationMethods(OpenApiUrlTreeNode currentNode, OperationTyp
             AddRequestBuilderMethodParameters(currentNode, operationType, operation, requestConfigClass, executorMethod);
             parentClass.AddMethod(executorMethod);
 
-#pragma warning disable CS0618
-            var handlerParam = new CodeParameter
-            {
-                Name = "responseHandler",
-                Optional = true,
-                Kind = CodeParameterKind.ResponseHandler,
-                Documentation = new()
-                {
-                    Description = "Response handler to use in place of the default response handling provided by the core service",
-                },
-                Type = new CodeType { Name = "IResponseHandler", IsExternal = true },
-            };
-            executorMethod.AddParameter(handlerParam);// Add response handler parameter
-#pragma warning restore CS0618
-
             var cancellationParam = new CodeParameter
             {
                 Name = "cancellationToken",
@@ -1602,9 +1587,9 @@ private CodeTypeBase CreateInheritedModelDeclaration(OpenApiUrlTreeNode currentN
         if (parentSchema.Items?.Reference?.Id?.EndsWith(title, StringComparison.OrdinalIgnoreCase) ?? false) return parentSchema.Items.Reference.Id;
         return parentSchema.GetSchemaReferenceIds().FirstOrDefault(refId => refId.EndsWith(title, StringComparison.OrdinalIgnoreCase));
     }
-    private CodeTypeBase CreateComposedModelDeclaration(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, string suffixForInlineSchema, CodeNamespace codeNamespace, bool isRequestBody)
+    private CodeTypeBase CreateComposedModelDeclaration(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, string suffixForInlineSchema, CodeNamespace codeNamespace, bool isRequestBody, string typeNameForInlineSchema)
     {
-        var typeName = currentNode.GetClassName(config.StructuredMimeTypes, operation: operation, suffix: suffixForInlineSchema, schema: schema, requestBody: isRequestBody).CleanupSymbolName();
+        var typeName = string.IsNullOrEmpty(typeNameForInlineSchema) ? currentNode.GetClassName(config.StructuredMimeTypes, operation: operation, suffix: suffixForInlineSchema, schema: schema, requestBody: isRequestBody).CleanupSymbolName() : typeNameForInlineSchema;
         var typesCount = schema.AnyOf?.Count ?? schema.OneOf?.Count ?? 0;
         if (typesCount == 1 && schema.Nullable && schema.IsInclusiveUnion() || // nullable on the root schema outside of anyOf
             typesCount == 2 && (schema.AnyOf?.Any(static x => // nullable on a schema in the anyOf
@@ -1654,16 +1639,18 @@ private CodeTypeBase CreateComposedModelDeclaration(OpenApiUrlTreeNode currentNo
             if (string.IsNullOrEmpty(className))
                 if (GetPrimitiveType(currentSchema) is CodeType primitiveType && !string.IsNullOrEmpty(primitiveType.Name))
                 {
-                    unionType.AddType(primitiveType);
+                    if (!unionType.ContainsType(primitiveType))
+                        unionType.AddType(primitiveType);
                     continue;
                 }
                 else
                     className = $"{unionType.Name}Member{++membersWithNoName}";
-            var codeDeclaration = AddModelDeclarationIfDoesntExist(currentNode, currentSchema, className, shortestNamespace);
-            unionType.AddType(new CodeType
+            var declarationType = new CodeType
             {
-                TypeDefinition = codeDeclaration,
-            });
+                TypeDefinition = AddModelDeclarationIfDoesntExist(currentNode, currentSchema, className, shortestNamespace),
+            };
+            if (!unionType.ContainsType(declarationType))
+                unionType.AddType(declarationType);
         }
         return unionType;
     }
@@ -1696,7 +1683,7 @@ private CodeTypeBase CreateComposedModelDeclaration(OpenApiUrlTreeNode currentNo
         if ((schema.IsInclusiveUnion() || schema.IsExclusiveUnion()) && string.IsNullOrEmpty(schema.Format)
             && !schema.IsODataPrimitiveType())
         { // OData types are oneOf string, type + format, enum
-            return CreateComposedModelDeclaration(currentNode, schema, operation, suffix, codeNamespace, isRequestBody);
+            return CreateComposedModelDeclaration(currentNode, schema, operation, suffix, codeNamespace, isRequestBody, typeNameForInlineSchema);
         }
 
         if (schema.IsObject() || schema.Properties.Any() || schema.IsEnum() || !string.IsNullOrEmpty(schema.AdditionalProperties?.Type))
@@ -2067,9 +2054,7 @@ private void CreatePropertiesForModelClass(OpenApiUrlTreeNode currentNode, OpenA
                     .Select(x =>
                     {
                         var propertySchema = x.Value;
-                        var className = propertySchema.GetSchemaName().CleanupSymbolName();
-                        if (string.IsNullOrEmpty(className))
-                            className = $"{model.Name}_{x.Key.CleanupSymbolName()}";
+                        var className = $"{model.Name}_{x.Key.CleanupSymbolName()}";
                         var shortestNamespaceName = GetModelsNamespaceNameFromReferenceId(propertySchema.Reference?.Id);
                         var targetNamespace = string.IsNullOrEmpty(shortestNamespaceName) ? ns :
                                             rootNamespace?.FindOrAddNamespace(shortestNamespaceName) ?? ns;

diff --git a/src/Kiota.Builder/Refiners/CSharpRefiner.cs b/src/Kiota.Builder/Refiners/CSharpRefiner.cs
@@ -107,7 +107,6 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
                 generatedCode,
                 "IParseNode"
             );
-            RemoveHandlerFromRequestBuilder(generatedCode);
         }, cancellationToken);
     }
     protected static void DisambiguatePropertiesWithClassNames(CodeElement currentElement)

diff --git a/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs b/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs
@@ -486,6 +486,7 @@ private static CodeTypeBase ConvertComposedTypeToWrapper(CodeClass codeClass, Co
                 {
                     Description = description,
                 },
+                Deprecation = codeComposedType.Deprecation,
             }).Last();
         }
         else if (codeComposedType.TargetNamespace is CodeNamespace targetNamespace)
@@ -1377,21 +1378,6 @@ mappingClass.Parent is CodeNamespace mappingNamespace &&
         }
         CrawlTree(currentElement, RemoveDiscriminatorMappingsTargetingSubNamespaces);
     }
-    protected void RemoveHandlerFromRequestBuilder(CodeElement currentElement)
-    {
-        if (currentElement is CodeClass currentClass && currentClass.IsOfKind(CodeClassKind.RequestBuilder))
-        {
-            var codeMethods = currentClass.Methods.Where(x => x.Kind == CodeMethodKind.RequestExecutor);
-            foreach (var codeMethod in codeMethods)
-            {
-#pragma warning disable CS0618
-                codeMethod.RemoveParametersByKind(CodeParameterKind.ResponseHandler);
-#pragma warning restore CS0618
-            }
-        }
-
-        CrawlTree(currentElement, RemoveHandlerFromRequestBuilder);
-    }
     protected static void MoveRequestBuilderPropertiesToBaseType(CodeElement currentElement, CodeUsing baseTypeUsing, AccessModifier? accessModifier = null)
     {
         ArgumentNullException.ThrowIfNull(baseTypeUsing);

diff --git a/src/Kiota.Builder/Refiners/GoRefiner.cs b/src/Kiota.Builder/Refiners/GoRefiner.cs
@@ -164,7 +164,6 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
                 generatedCode,
                 x => $"{x.Name}able"
             );
-            RemoveHandlerFromRequestBuilder(generatedCode);
             AddContextParameterToGeneratorMethods(generatedCode);
             CorrectTypes(generatedCode);
             CorrectCoreTypesForBackingStore(generatedCode, $"{conventions.StoreHash}.BackingStoreFactoryInstance()", false);

diff --git a/src/Kiota.Builder/Refiners/JavaRefiner.cs b/src/Kiota.Builder/Refiners/JavaRefiner.cs
@@ -137,7 +137,6 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
                 "ParseNode",
                 addUsings: true
             );
-            RemoveHandlerFromRequestBuilder(generatedCode);
             SplitLongDiscriminatorMethods(generatedCode);
             AddPrimaryErrorMessage(generatedCode,
                 "getMessage",

diff --git a/src/Kiota.Builder/Refiners/PhpRefiner.cs b/src/Kiota.Builder/Refiners/PhpRefiner.cs
@@ -102,7 +102,6 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
             AddPropertiesAndMethodTypesImports(generatedCode, true, false, true);
             CorrectBackingStoreSetterParam(generatedCode);
             CorrectCoreTypesForBackingStore(generatedCode, "BackingStoreFactorySingleton::getInstance()->createBackingStore()");
-            RemoveHandlerFromRequestBuilder(generatedCode);
             cancellationToken.ThrowIfCancellationRequested();
             AliasUsingWithSameSymbol(generatedCode);
             RemoveRequestConfigurationClassesCommonProperties(generatedCode,

diff --git a/src/Kiota.Builder/Refiners/PythonRefiner.cs b/src/Kiota.Builder/Refiners/PythonRefiner.cs
@@ -122,7 +122,6 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
                 addUsings: true,
                 includeParentNamespace: true
             );
-            RemoveHandlerFromRequestBuilder(generatedCode);
         }, cancellationToken);
     }
 

diff --git a/src/Kiota.Builder/Refiners/RubyRefiner.cs b/src/Kiota.Builder/Refiners/RubyRefiner.cs
@@ -122,7 +122,6 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
                 "ParseNode",
                 addUsings: true
             );
-            RemoveHandlerFromRequestBuilder(generatedCode);
         }, cancellationToken);
     }
     private static void ShortenLongNamespaceNames(CodeElement currentElement)

diff --git a/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs b/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs
@@ -17,7 +17,6 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
         {
             cancellationToken.ThrowIfCancellationRequested();
             RemoveMethodByKind(generatedCode, CodeMethodKind.RawUrlConstructor);
-            RemoveHandlerFromRequestBuilder(generatedCode);
             ReplaceReservedNames(generatedCode, new TypeScriptReservedNamesProvider(), static x => $"{x}Escaped");
             ReplaceReservedExceptionPropertyNames(generatedCode, new TypeScriptExceptionsReservedNamesProvider(), static x => $"{x}Escaped");
             MoveRequestBuilderPropertiesToBaseType(generatedCode,