From 9159471974c6b3144eb7c0759d2b664ce0baeb30 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 2 Aug 2023 15:23:06 -0400 Subject: [PATCH 1/5] - initial refactoring of the indexers infrastructure to allow specialized types --- src/Kiota.Builder/CodeDOM/CodeClass.cs | 19 ++++++++++++------- src/Kiota.Builder/KiotaBuilder.cs | 2 +- .../CodeDOM/CodeClassTests.cs | 10 +++++----- .../Refiners/CSharpLanguageRefinerTests.cs | 8 ++++---- .../Refiners/GoLanguageRefinerTests.cs | 2 +- .../Refiners/JavaLanguageRefinerTests.cs | 4 ++-- .../Refiners/ShellRefinerTests.cs | 6 +++--- .../Writers/CSharp/CodeIndexerWriterTests.cs | 2 +- .../Writers/Go/CodeMethodWriterTests.cs | 4 ++-- 9 files changed, 31 insertions(+), 26 deletions(-) diff --git a/src/Kiota.Builder/CodeDOM/CodeClass.cs b/src/Kiota.Builder/CodeDOM/CodeClass.cs index f92963c7be..d14d859e86 100644 --- a/src/Kiota.Builder/CodeDOM/CodeClass.cs +++ b/src/Kiota.Builder/CodeDOM/CodeClass.cs @@ -45,14 +45,20 @@ public CodeComposedTypeBase? OriginalComposedType { get; set; } - public CodeIndexer? Indexer + public CodeIndexer? Indexer => InnerChildElements.Values.OfType().FirstOrDefault(static x => !x.Deprecation?.IsDeprecated ?? true); + public void AddIndexer(params CodeIndexer[] indexers) { - set + if (indexers == null || indexers.Any(static x => x == null)) + throw new ArgumentNullException(nameof(indexers)); + if (!indexers.Any()) + throw new ArgumentOutOfRangeException(nameof(indexers)); + + foreach (var value in indexers) { - ArgumentNullException.ThrowIfNull(value); - if (InnerChildElements.Values.OfType().Any() || InnerChildElements.Values.OfType().Any(static x => x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility))) - { - if (Indexer is CodeIndexer existingIndexer) + var existingIndexers = InnerChildElements.Values.OfType().ToArray(); + if (existingIndexers.Any() || InnerChildElements.Values.OfType().Any(static x => x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility))) + {//TODO defend against obsolete + other type parameter + foreach (var existingIndexer in existingIndexers) { RemoveChildElement(existingIndexer); AddRange(CodeMethod.FromIndexer(existingIndexer, static x => $"With{x.ToFirstCharacterUpperCase()}", static x => x.ToFirstCharacterUpperCase(), true)); @@ -62,7 +68,6 @@ public CodeIndexer? Indexer else AddRange(value); } - get => InnerChildElements.Values.OfType().FirstOrDefault(); } public override IEnumerable AddProperty(params CodeProperty[] properties) { diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index 4a4d4d8979..1d99957c1b 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -678,7 +678,7 @@ private void CreateRequestBuilderClass(CodeNamespace currentNamespace, OpenApiUr var propType = child.Value.GetNavigationPropertyName(config.StructuredMimeTypes, child.Value.DoesNodeBelongToItemSubnamespace() ? ItemRequestBuilderSuffix : RequestBuilderSuffix); if (child.Value.IsPathSegmentWithSingleSimpleParameter()) - codeClass.Indexer = CreateIndexer($"{propIdentifier}-indexer", propType, child.Value, currentNode); + codeClass.AddIndexer(CreateIndexer($"{propIdentifier}-indexer", propType, child.Value, currentNode)); else if (child.Value.IsComplexPathMultipleParameters()) CreateMethod(propIdentifier, propType, codeClass, child.Value); else diff --git a/tests/Kiota.Builder.Tests/CodeDOM/CodeClassTests.cs b/tests/Kiota.Builder.Tests/CodeDOM/CodeClassTests.cs index 7a9638117a..b1760535dd 100644 --- a/tests/Kiota.Builder.Tests/CodeDOM/CodeClassTests.cs +++ b/tests/Kiota.Builder.Tests/CodeDOM/CodeClassTests.cs @@ -50,7 +50,7 @@ public void SetsIndexer() { Name = "class1" }).First(); - codeClass.Indexer = new CodeIndexer + codeClass.AddIndexer(new CodeIndexer { Name = "idx", SerializationName = "idx_smth", @@ -67,13 +67,13 @@ public void SetsIndexer() IsNullable = true, }, IndexParameterName = "idxSmth", - }; + }); Assert.Single(codeClass.GetChildElements(true).OfType()); Assert.Throws(() => { - codeClass.Indexer = null; + codeClass.AddIndexer(null); }); - codeClass.Indexer = new CodeIndexer + codeClass.AddIndexer(new CodeIndexer { Name = "idx2", SerializationName = "idx-2", @@ -90,7 +90,7 @@ public void SetsIndexer() IsNullable = true, }, IndexParameterName = "idx2", - }; + }); Assert.Empty(codeClass.GetChildElements(true).OfType()); var methods = codeClass.GetChildElements(true).OfType().Where(x => x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility)).ToArray(); Assert.Equal(2, methods.Length); diff --git a/tests/Kiota.Builder.Tests/Refiners/CSharpLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/CSharpLanguageRefinerTests.cs index 0ce39bc136..37a83c9349 100644 --- a/tests/Kiota.Builder.Tests/Refiners/CSharpLanguageRefinerTests.cs +++ b/tests/Kiota.Builder.Tests/Refiners/CSharpLanguageRefinerTests.cs @@ -245,7 +245,7 @@ public async Task EscapesReservedKeywordsForReservedNamespaceNameSegments() ReturnType = indexerCodeType, IndexParameterName = "id", }; - requestBuilder.Indexer = indexer; + requestBuilder.AddIndexer(indexer); var itemSubNamespace = root.AddNamespace($"{subNS.Name}.item"); // otherwise the import gets trimmed @@ -255,7 +255,7 @@ public async Task EscapesReservedKeywordsForReservedNamespaceNameSegments() Kind = CodeClassKind.RequestBuilder, }).First(); - var requestExecutor = itemRequestBuilder.AddMethod(new CodeMethod + itemRequestBuilder.AddMethod(new CodeMethod { Name = "get", Kind = CodeMethodKind.IndexerBackwardCompatibility, @@ -263,7 +263,7 @@ public async Task EscapesReservedKeywordsForReservedNamespaceNameSegments() { Name = "String" }, - }).First(); + }); await ILanguageRefiner.Refine(new GenerationConfiguration { Language = GenerationLanguage.CSharp }, root); @@ -315,7 +315,7 @@ public async Task ConvertsUnionTypesToWrapper() }, IndexParameterName = "id", }; - model.Indexer = indexer; + model.AddIndexer(indexer); method.AddParameter(parameter); await ILanguageRefiner.Refine(new GenerationConfiguration { Language = GenerationLanguage.CSharp }, root); //using CSharp so the indexer doesn't get removed Assert.True(property.Type is CodeType); diff --git a/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs index db3e17797c..f1b2dc2a12 100644 --- a/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs +++ b/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs @@ -346,7 +346,7 @@ public async Task ConvertsUnionTypesToWrapper() }, IndexParameterName = "id", }; - model.Indexer = indexer; + model.AddIndexer(indexer); method.AddParameter(parameter); await ILanguageRefiner.Refine(new GenerationConfiguration { Language = GenerationLanguage.Go }, root); //using CSharp so the indexer doesn't get removed Assert.True(property.Type is CodeType); diff --git a/tests/Kiota.Builder.Tests/Refiners/JavaLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/JavaLanguageRefinerTests.cs index 9881ab624a..1f70409d70 100644 --- a/tests/Kiota.Builder.Tests/Refiners/JavaLanguageRefinerTests.cs +++ b/tests/Kiota.Builder.Tests/Refiners/JavaLanguageRefinerTests.cs @@ -334,7 +334,7 @@ public async Task ReplacesIndexersByMethodsWithParameter() Name = "string", } }); - requestBuilder.Indexer = new CodeIndexer + requestBuilder.AddIndexer(new CodeIndexer { Name = "idx", ReturnType = new CodeType @@ -347,7 +347,7 @@ public async Task ReplacesIndexersByMethodsWithParameter() Name = "string", }, IndexParameterName = "id", - }; + }); var collectionRequestBuilder = collectionNS.AddClass(new CodeClass { Name = "CollectionRequestBuilder", diff --git a/tests/Kiota.Builder.Tests/Refiners/ShellRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/ShellRefinerTests.cs index 6da8d8b332..cda049a0c1 100644 --- a/tests/Kiota.Builder.Tests/Refiners/ShellRefinerTests.cs +++ b/tests/Kiota.Builder.Tests/Refiners/ShellRefinerTests.cs @@ -100,7 +100,7 @@ public async Task CreatesCommandBuilders() }); // Add indexer - requestBuilder.Indexer = new CodeIndexer + requestBuilder.AddIndexer(new CodeIndexer { Name = "Users-idx", ReturnType = new CodeType @@ -112,7 +112,7 @@ public async Task CreatesCommandBuilders() Name = "string" }, IndexParameterName = "id", - }; + }); // Add request executor requestBuilder.AddMethod(new CodeMethod @@ -277,7 +277,7 @@ public async Task RenamesNavPropertiesInIndexersWithConflicts() TypeDefinition = indexerType, } }; - rootRequestBuilder.Indexer = indexer; + rootRequestBuilder.AddIndexer(indexer); await ILanguageRefiner.Refine(new GenerationConfiguration { Language = GenerationLanguage.Shell }, root); diff --git a/tests/Kiota.Builder.Tests/Writers/CSharp/CodeIndexerWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/CSharp/CodeIndexerWriterTests.cs index e21d7e6f47..a027b14b25 100644 --- a/tests/Kiota.Builder.Tests/Writers/CSharp/CodeIndexerWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/CSharp/CodeIndexerWriterTests.cs @@ -40,7 +40,7 @@ public CodeIndexerWriterTests() }, IndexParameterName = "position" }; - parentClass.Indexer = indexer; + parentClass.AddIndexer(indexer); parentClass.AddProperty(new() { Name = "pathParameters", diff --git a/tests/Kiota.Builder.Tests/Writers/Go/CodeMethodWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Go/CodeMethodWriterTests.cs index 71cd25b756..7dbb644e19 100644 --- a/tests/Kiota.Builder.Tests/Writers/Go/CodeMethodWriterTests.cs +++ b/tests/Kiota.Builder.Tests/Writers/Go/CodeMethodWriterTests.cs @@ -1574,7 +1574,7 @@ public void WritesIndexer() { setup(); AddRequestProperties(); - parentClass.Indexer = new() + parentClass.AddIndexer(new CodeIndexer { Name = "indx", SerializationName = "id", @@ -1588,7 +1588,7 @@ public void WritesIndexer() Name = "Somecustomtype", }, IndexParameterName = "id", - }; + }); if (parentClass.Indexer is null) throw new InvalidOperationException("Indexer is null"); var methodForTest = parentClass.AddMethod(CodeMethod.FromIndexer(parentClass.Indexer, static x => $"With{x.ToFirstCharacterUpperCase()}", static x => x.ToFirstCharacterLowerCase(), false)).First(); From 7c46aa28269d9a4be739efce7a8341d19d698923 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 3 Aug 2023 14:21:44 -0400 Subject: [PATCH 2/5] - adds support for type specific indexers --- src/Kiota.Builder/CodeDOM/CodeClass.cs | 5 +- src/Kiota.Builder/CodeDOM/CodeIndexer.cs | 17 ++++- .../CodeDOM/DeprecationInformation.cs | 2 +- src/Kiota.Builder/KiotaBuilder.cs | 44 ++++++++++-- .../Refiners/CommonLanguageRefiner.cs | 24 ++++++- src/Kiota.Builder/Refiners/GoRefiner.cs | 3 +- src/Kiota.Builder/Refiners/JavaRefiner.cs | 3 +- src/Kiota.Builder/Refiners/PhpRefiner.cs | 3 +- src/Kiota.Builder/Refiners/PythonRefiner.cs | 3 +- src/Kiota.Builder/Refiners/RubyRefiner.cs | 3 +- src/Kiota.Builder/Refiners/SwiftRefiner.cs | 3 +- .../Refiners/TypeScriptRefiner.cs | 3 +- .../CodeDOM/CodeClassTests.cs | 2 +- .../Kiota.Builder.Tests/KiotaBuilderTests.cs | 61 ++++++++++++++++ .../Refiners/GoLanguageRefinerTests.cs | 70 ++++++++++++++++++- 15 files changed, 225 insertions(+), 21 deletions(-) diff --git a/src/Kiota.Builder/CodeDOM/CodeClass.cs b/src/Kiota.Builder/CodeDOM/CodeClass.cs index d14d859e86..585d74ca55 100644 --- a/src/Kiota.Builder/CodeDOM/CodeClass.cs +++ b/src/Kiota.Builder/CodeDOM/CodeClass.cs @@ -56,8 +56,9 @@ public void AddIndexer(params CodeIndexer[] indexers) foreach (var value in indexers) { var existingIndexers = InnerChildElements.Values.OfType().ToArray(); - if (existingIndexers.Any() || InnerChildElements.Values.OfType().Any(static x => x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility))) - {//TODO defend against obsolete + other type parameter + if (existingIndexers.Any(x => !x.IndexParameterName.Equals(value.IndexParameterName, StringComparison.OrdinalIgnoreCase)) || + InnerChildElements.Values.OfType().Any(static x => x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility))) + { foreach (var existingIndexer in existingIndexers) { RemoveChildElement(existingIndexer); diff --git a/src/Kiota.Builder/CodeDOM/CodeIndexer.cs b/src/Kiota.Builder/CodeDOM/CodeIndexer.cs index fce56ece33..07131a0e1d 100644 --- a/src/Kiota.Builder/CodeDOM/CodeIndexer.cs +++ b/src/Kiota.Builder/CodeDOM/CodeIndexer.cs @@ -1,7 +1,7 @@ using System; namespace Kiota.Builder.CodeDOM; -public class CodeIndexer : CodeTerminal, IDocumentedElement, IDeprecableElement +public class CodeIndexer : CodeTerminal, IDocumentedElement, IDeprecableElement, ICloneable { #nullable disable // exposing property is required private CodeTypeBase indexType; @@ -44,4 +44,19 @@ public DeprecationInformation? Deprecation { get; set; } + public object Clone() + { + return new CodeIndexer + { + Name = Name, + Parent = Parent, + IndexType = IndexType.Clone() as CodeTypeBase ?? throw new InvalidOperationException($"Cloning failed. Cloned type is invalid."), + ReturnType = ReturnType.Clone() as CodeTypeBase ?? throw new InvalidOperationException($"Cloning failed. Cloned type is invalid."), + IndexParameterName = IndexParameterName, + SerializationName = SerializationName, + Documentation = Documentation.Clone() as CodeDocumentation ?? throw new InvalidOperationException($"Cloning failed. Cloned type is invalid."), + PathSegment = PathSegment, + Deprecation = Deprecation == null ? null : Deprecation with { } + }; + } } diff --git a/src/Kiota.Builder/CodeDOM/DeprecationInformation.cs b/src/Kiota.Builder/CodeDOM/DeprecationInformation.cs index 11f2c1029d..7a06eac09e 100644 --- a/src/Kiota.Builder/CodeDOM/DeprecationInformation.cs +++ b/src/Kiota.Builder/CodeDOM/DeprecationInformation.cs @@ -2,4 +2,4 @@ namespace Kiota.Builder.CodeDOM; -public record DeprecationInformation(string? Description, DateTimeOffset? Date, DateTimeOffset? RemovalDate, string? Version, bool IsDeprecated = true); +public record DeprecationInformation(string? Description, DateTimeOffset? Date = null, DateTimeOffset? RemovalDate = null, string? Version = "", bool IsDeprecated = true); diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index 1d99957c1b..2e2f211498 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -678,7 +678,10 @@ private void CreateRequestBuilderClass(CodeNamespace currentNamespace, OpenApiUr var propType = child.Value.GetNavigationPropertyName(config.StructuredMimeTypes, child.Value.DoesNodeBelongToItemSubnamespace() ? ItemRequestBuilderSuffix : RequestBuilderSuffix); if (child.Value.IsPathSegmentWithSingleSimpleParameter()) - codeClass.AddIndexer(CreateIndexer($"{propIdentifier}-indexer", propType, child.Value, currentNode)); + { + var indexerParameterType = GetIndexerParameterType(child.Value, currentNode); + codeClass.AddIndexer(CreateIndexer($"{propIdentifier}-indexer", propType, indexerParameterType, child.Value, currentNode)); + } else if (child.Value.IsComplexPathMultipleParameters()) CreateMethod(propIdentifier, propType, codeClass, child.Value); else @@ -967,23 +970,54 @@ private IEnumerable GetUnmappedTypeDefinitions(CodeElement codeElement _ => childElementsUnmappedTypes, }; } - private CodeIndexer CreateIndexer(string childIdentifier, string childType, OpenApiUrlTreeNode currentNode, OpenApiUrlTreeNode parentNode) + private static CodeType DefaultIndexerParameterType => new() { Name = "string", IsExternal = true }; + private CodeType GetIndexerParameterType(OpenApiUrlTreeNode currentNode, OpenApiUrlTreeNode parentNode) + { + var parameterName = currentNode.Path[parentNode.Path.Length..].Trim('\\', ForwardSlash, '{', '}'); + var parameter = currentNode.PathItems.TryGetValue(Constants.DefaultOpenApiLabel, out var pathItem) ? pathItem.Parameters + .Select(static x => new { Parameter = x, IsPathParameter = true }) + .Union(currentNode.PathItems[Constants.DefaultOpenApiLabel].Operations.SelectMany(static x => x.Value.Parameters).Select(static x => new { Parameter = x, IsPathParameter = false })) + .OrderBy(static x => x.IsPathParameter) + .Select(static x => x.Parameter) + .FirstOrDefault(x => x.Name.Equals(parameterName, StringComparison.OrdinalIgnoreCase) && x.In == ParameterLocation.Path) : + default; + var type = parameter switch + { + null => DefaultIndexerParameterType, + _ => GetPrimitiveType(parameter.Schema), + } ?? DefaultIndexerParameterType; + type.IsNullable = false; + return type; + } + private CodeIndexer[] CreateIndexer(string childIdentifier, string childType, CodeType parameterType, OpenApiUrlTreeNode currentNode, OpenApiUrlTreeNode parentNode) { logger.LogTrace("Creating indexer {Name}", childIdentifier); - return new CodeIndexer + var result = new List { new CodeIndexer { Name = childIdentifier, Documentation = new() { Description = currentNode.GetPathItemDescription(Constants.DefaultOpenApiLabel, $"Gets an item from the {currentNode.GetNodeNamespaceFromPath(config.ClientNamespaceName)} collection"), }, - IndexType = new CodeType { Name = "string", IsExternal = true, }, + IndexType = parameterType, ReturnType = new CodeType { Name = childType }, SerializationName = currentNode.Segment.SanitizeParameterNameForUrlTemplate(), PathSegment = parentNode.GetNodeNamespaceFromPath(string.Empty).Split('.').Last(), IndexParameterName = currentNode.Segment.CleanupSymbolName(), Deprecation = currentNode.GetDeprecationInformation(), - }; + }}; + + if (!"string".Equals(parameterType.Name, StringComparison.OrdinalIgnoreCase)) + { // adding a second indexer for the string version of the parameter so we keep backward compatibility + //TODO remove for v2 + var backCompatibleValue = (CodeIndexer)result[0].Clone(); + backCompatibleValue.Name += "-string"; + backCompatibleValue.IndexType = DefaultIndexerParameterType; + backCompatibleValue.Deprecation = new DeprecationInformation("This indexer is deprecated and will be removed in the next major version. Use the one with the typed parameter instead."); + result.Add(backCompatibleValue); + } + + return result.ToArray(); } private CodeProperty? CreateProperty(string childIdentifier, string childType, OpenApiSchema? propertySchema = null, CodeTypeBase? existingType = null, CodePropertyKind kind = CodePropertyKind.Custom) diff --git a/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs b/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs index 59ce751879..0f4eb7442d 100644 --- a/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs +++ b/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs @@ -688,15 +688,33 @@ protected static void MoveClassesWithNamespaceNamesUnderNamespace(CodeElement cu } CrawlTree(currentElement, MoveClassesWithNamespaceNamesUnderNamespace); } - protected static void ReplaceIndexersByMethodsWithParameter(CodeElement currentElement, bool parameterNullable, Func methodNameCallback, Func parameterNameCallback) + private static readonly Func IsIndexerTypeSpecificVersion = + (currentIndexerParameterName, existingIndexer) => currentIndexerParameterName.Equals(existingIndexer.IndexParameterName, StringComparison.OrdinalIgnoreCase) && !"string".Equals(existingIndexer.IndexType.Name, StringComparison.OrdinalIgnoreCase); + protected static void ReplaceIndexersByMethodsWithParameter(CodeElement currentElement, bool parameterNullable, Func methodNameCallback, Func parameterNameCallback, GenerationLanguage language) { if (currentElement is CodeIndexer currentIndexer && currentElement.Parent is CodeClass indexerParentClass) { indexerParentClass.RemoveChildElement(currentElement); - indexerParentClass.AddMethod(CodeMethod.FromIndexer(currentIndexer, methodNameCallback, parameterNameCallback, parameterNullable)); + //TODO remove for v2 + var isIndexerStringBackwardCompatible = "string".Equals(currentIndexer.IndexType.Name, StringComparison.OrdinalIgnoreCase) && + currentIndexer.Deprecation is not null && currentIndexer.Deprecation.IsDeprecated && + (indexerParentClass.Methods.Any(x => x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility) && x.OriginalIndexer is not null && IsIndexerTypeSpecificVersion(currentIndexer.IndexParameterName, x.OriginalIndexer)) || + (indexerParentClass.Indexer != null && indexerParentClass.Indexer != currentIndexer && IsIndexerTypeSpecificVersion(currentIndexer.IndexParameterName, indexerParentClass.Indexer))); + if (isIndexerStringBackwardCompatible && language == GenerationLanguage.Go) + { + if (indexerParentClass.Methods.FirstOrDefault(x => x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility) && x.OriginalIndexer is not null && IsIndexerTypeSpecificVersion(currentIndexer.IndexParameterName, x.OriginalIndexer)) is CodeMethod typeSpecificCompatibleMethod && + typeSpecificCompatibleMethod.OriginalIndexer is not null) + { + indexerParentClass.RenameChildElement(typeSpecificCompatibleMethod.Name, typeSpecificCompatibleMethod.Name + typeSpecificCompatibleMethod.OriginalIndexer.IndexType.Name.ToFirstCharacterUpperCase()); + } + indexerParentClass.AddMethod(CodeMethod.FromIndexer(currentIndexer, methodNameCallback, parameterNameCallback, parameterNullable)); + } + else if (!isIndexerStringBackwardCompatible) + indexerParentClass.AddMethod(CodeMethod.FromIndexer(currentIndexer, methodNameCallback, parameterNameCallback, parameterNullable)); + } - CrawlTree(currentElement, c => ReplaceIndexersByMethodsWithParameter(c, parameterNullable, methodNameCallback, parameterNameCallback)); + CrawlTree(currentElement, c => ReplaceIndexersByMethodsWithParameter(c, parameterNullable, methodNameCallback, parameterNameCallback, language)); } internal void DisableActionOf(CodeElement current, params CodeParameterKind[] kinds) { diff --git a/src/Kiota.Builder/Refiners/GoRefiner.cs b/src/Kiota.Builder/Refiners/GoRefiner.cs index c7a51d4dc8..34d37a3d53 100644 --- a/src/Kiota.Builder/Refiners/GoRefiner.cs +++ b/src/Kiota.Builder/Refiners/GoRefiner.cs @@ -33,7 +33,8 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance generatedCode, false, static x => $"By{x.ToFirstCharacterUpperCase()}", - static x => x.ToFirstCharacterLowerCase()); + static x => x.ToFirstCharacterLowerCase(), + GenerationLanguage.Go); FlattenNestedHierarchy(generatedCode); FlattenGoParamsFileNames(generatedCode); FlattenGoFileNames(generatedCode); diff --git a/src/Kiota.Builder/Refiners/JavaRefiner.cs b/src/Kiota.Builder/Refiners/JavaRefiner.cs index 5a244e667a..3b5bfb91b8 100644 --- a/src/Kiota.Builder/Refiners/JavaRefiner.cs +++ b/src/Kiota.Builder/Refiners/JavaRefiner.cs @@ -53,7 +53,8 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance ReplaceIndexersByMethodsWithParameter(generatedCode, true, static x => $"By{x.ToFirstCharacterUpperCase()}", - static x => x.ToFirstCharacterLowerCase()); + static x => x.ToFirstCharacterLowerCase(), + GenerationLanguage.Java); cancellationToken.ThrowIfCancellationRequested(); RemoveCancellationParameter(generatedCode); ConvertUnionTypesToWrapper(generatedCode, diff --git a/src/Kiota.Builder/Refiners/PhpRefiner.cs b/src/Kiota.Builder/Refiners/PhpRefiner.cs index ee9aedf8ba..e92eee5158 100644 --- a/src/Kiota.Builder/Refiners/PhpRefiner.cs +++ b/src/Kiota.Builder/Refiners/PhpRefiner.cs @@ -37,7 +37,8 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance ReplaceIndexersByMethodsWithParameter(generatedCode, false, static x => $"By{x.ToFirstCharacterUpperCase()}", - static x => x.ToFirstCharacterLowerCase()); + static x => x.ToFirstCharacterLowerCase(), + GenerationLanguage.PHP); RemoveCancellationParameter(generatedCode); ConvertUnionTypesToWrapper(generatedCode, _configuration.UsesBackingStore, diff --git a/src/Kiota.Builder/Refiners/PythonRefiner.cs b/src/Kiota.Builder/Refiners/PythonRefiner.cs index f0c9c32544..756ba934ec 100644 --- a/src/Kiota.Builder/Refiners/PythonRefiner.cs +++ b/src/Kiota.Builder/Refiners/PythonRefiner.cs @@ -33,7 +33,8 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance ReplaceIndexersByMethodsWithParameter(generatedCode, false, static x => $"by_{x.ToSnakeCase()}", - static x => x.ToSnakeCase()); + static x => x.ToSnakeCase(), + GenerationLanguage.Python); RemoveCancellationParameter(generatedCode); CorrectCoreType(generatedCode, CorrectMethodType, CorrectPropertyType, CorrectImplements); cancellationToken.ThrowIfCancellationRequested(); diff --git a/src/Kiota.Builder/Refiners/RubyRefiner.cs b/src/Kiota.Builder/Refiners/RubyRefiner.cs index 6f5a156c3b..790b586e9f 100644 --- a/src/Kiota.Builder/Refiners/RubyRefiner.cs +++ b/src/Kiota.Builder/Refiners/RubyRefiner.cs @@ -20,7 +20,8 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance ReplaceIndexersByMethodsWithParameter(generatedCode, false, static x => $"by_{x.ToSnakeCase()}", - static x => x.ToSnakeCase()); + static x => x.ToSnakeCase(), + GenerationLanguage.Ruby); MoveRequestBuilderPropertiesToBaseType(generatedCode, new CodeUsing { diff --git a/src/Kiota.Builder/Refiners/SwiftRefiner.cs b/src/Kiota.Builder/Refiners/SwiftRefiner.cs index 483fcad91a..2aff3183de 100644 --- a/src/Kiota.Builder/Refiners/SwiftRefiner.cs +++ b/src/Kiota.Builder/Refiners/SwiftRefiner.cs @@ -22,7 +22,8 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance generatedCode, false, static x => $"By{x.ToFirstCharacterUpperCase()}", - static x => x.ToFirstCharacterUpperCase()); + static x => x.ToFirstCharacterUpperCase(), + GenerationLanguage.Swift); cancellationToken.ThrowIfCancellationRequested(); ReplaceReservedNames( generatedCode, diff --git a/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs b/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs index ddd8fd1b09..2b54e97169 100644 --- a/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs +++ b/src/Kiota.Builder/Refiners/TypeScriptRefiner.cs @@ -32,7 +32,8 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance ReplaceIndexersByMethodsWithParameter(generatedCode, false, static x => $"by{x.ToFirstCharacterUpperCase()}", - static x => x.ToFirstCharacterLowerCase()); + static x => x.ToFirstCharacterLowerCase(), + GenerationLanguage.TypeScript); RemoveCancellationParameter(generatedCode); CorrectCoreType(generatedCode, CorrectMethodType, CorrectPropertyType, CorrectImplements); CorrectCoreTypesForBackingStore(generatedCode, "BackingStoreFactorySingleton.instance.createBackingStore()"); diff --git a/tests/Kiota.Builder.Tests/CodeDOM/CodeClassTests.cs b/tests/Kiota.Builder.Tests/CodeDOM/CodeClassTests.cs index b1760535dd..463485d360 100644 --- a/tests/Kiota.Builder.Tests/CodeDOM/CodeClassTests.cs +++ b/tests/Kiota.Builder.Tests/CodeDOM/CodeClassTests.cs @@ -92,7 +92,7 @@ public void SetsIndexer() IndexParameterName = "idx2", }); Assert.Empty(codeClass.GetChildElements(true).OfType()); - var methods = codeClass.GetChildElements(true).OfType().Where(x => x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility)).ToArray(); + var methods = codeClass.GetChildElements(true).OfType().Where(static x => x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility)).ToArray(); Assert.Equal(2, methods.Length); Assert.Equal("WithIdxSmth", methods.FirstOrDefault(static x => x.OriginalIndexer.Name.Equals("idx")).Name); Assert.Equal("WithIdx2", methods.FirstOrDefault(static x => x.OriginalIndexer.Name.Equals("idx2")).Name); diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index b4b8185ace..7b4afffaba 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -5711,6 +5711,67 @@ public async Task IndexerAndRequestBuilderNamesMatch() var collectionRequestBuilder = collectionRequestBuilderNamespace.FindChildByName("postsRequestBuilder"); var collectionIndexer = collectionRequestBuilder.Indexer; Assert.NotNull(collectionIndexer); + Assert.Equal("string", collectionIndexer.IndexType.Name); + Assert.False(collectionIndexer.Deprecation.IsDeprecated); + var itemRequestBuilderNamespace = codeModel.FindNamespaceByName("ApiSdk.me.posts.item"); + Assert.NotNull(itemRequestBuilderNamespace); + var itemRequestBuilder = itemRequestBuilderNamespace.FindChildByName("postItemRequestBuilder"); + Assert.Equal(collectionIndexer.ReturnType.Name, itemRequestBuilder.Name); + } + [Fact] + public async Task IndexerTypeIsAccurateAndBackwardCompatibleIndexersAreAdded() + { + var tempFilePath = Path.Combine(Path.GetTempPath(), Path.GetTempFileName()); + await using var fs = await GetDocumentStream(@"openapi: 3.0.0 +info: + title: Microsoft Graph get user API + version: 1.0.0 +servers: + - url: https://graph.microsoft.com/v1.0/ +paths: + /me/posts/{post-id}: + get: + parameters: + - name: post-id + in: path + required: true + schema: + type: integer + format: int32 + responses: + 200: + description: Success! + content: + application/json: + schema: + $ref: '#/components/schemas/microsoft.graph.post' +components: + schemas: + microsoft.graph.post: + type: object + properties: + id: + type: string + displayName: + 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 collectionRequestBuilderNamespace = codeModel.FindNamespaceByName("ApiSdk.me.posts"); + Assert.NotNull(collectionRequestBuilderNamespace); + var collectionRequestBuilder = collectionRequestBuilderNamespace.FindChildByName("postsRequestBuilder"); + var collectionIndexer = collectionRequestBuilder.Indexer; + Assert.NotNull(collectionIndexer); + Assert.Equal("integer", collectionIndexer.IndexType.Name); + Assert.False(collectionIndexer.IndexType.IsNullable); + Assert.False(collectionIndexer.Deprecation.IsDeprecated); + var collectionStringIndexer = collectionRequestBuilder.FindChildByName($"{collectionIndexer.Name}-string"); + Assert.NotNull(collectionStringIndexer); + Assert.Equal("string", collectionStringIndexer.IndexType.Name); + Assert.True(collectionStringIndexer.IndexType.IsNullable); + Assert.True(collectionStringIndexer.Deprecation.IsDeprecated); var itemRequestBuilderNamespace = codeModel.FindNamespaceByName("ApiSdk.me.posts.item"); Assert.NotNull(itemRequestBuilderNamespace); var itemRequestBuilder = itemRequestBuilderNamespace.FindChildByName("postItemRequestBuilder"); diff --git a/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs index f1b2dc2a12..63e3528c71 100644 --- a/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs +++ b/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs @@ -348,7 +348,7 @@ public async Task ConvertsUnionTypesToWrapper() }; model.AddIndexer(indexer); method.AddParameter(parameter); - await ILanguageRefiner.Refine(new GenerationConfiguration { Language = GenerationLanguage.Go }, root); //using CSharp so the indexer doesn't get removed + await ILanguageRefiner.Refine(new GenerationConfiguration { Language = GenerationLanguage.Go }, root); Assert.True(property.Type is CodeType); Assert.True(parameter.Type is CodeType); Assert.True(method.ReturnType is CodeType); @@ -359,6 +359,74 @@ public async Task ConvertsUnionTypesToWrapper() Assert.NotNull(resultingWrapper.Methods.Single(static x => x.IsOfKind(CodeMethodKind.ComposedTypeMarker))); } [Fact] + public async Task SupportsTypeSpecificOverrideIndexers() + { + var model = root.AddClass(new CodeClass + { + Name = "model", + Kind = CodeClassKind.Model + }).First(); + var union = new CodeUnionType + { + Name = "union", + }; + union.AddType(new() + { + Name = "type1", + }, new() + { + Name = "type2" + }); + var property = model.AddProperty(new CodeProperty + { + Name = "deserialize", + Kind = CodePropertyKind.Custom, + Type = union.Clone() as CodeTypeBase, + }).First(); + var method = model.AddMethod(new CodeMethod + { + Name = "method", + ReturnType = union.Clone() as CodeTypeBase + }).First(); + var parameter = new CodeParameter + { + Name = "param1", + Type = union.Clone() as CodeTypeBase + }; + var indexer = new CodeIndexer + { + Name = "idx-string", + ReturnType = union.Clone() as CodeTypeBase, + IndexType = new CodeType + { + Name = "string" + }, + IndexParameterName = "id", + Deprecation = new("foo") + }; + var typeSpecificIndexer = new CodeIndexer + { + Name = "idx", + ReturnType = new CodeType + { + Name = "type1", + TypeDefinition = union.Types.First(), + }, + IndexType = new CodeType + { + Name = "integer" + }, + IndexParameterName = "id", + }; + model.AddIndexer(indexer, typeSpecificIndexer); + Assert.NotNull(model.Indexer); + method.AddParameter(parameter); + await ILanguageRefiner.Refine(new GenerationConfiguration { Language = GenerationLanguage.Go }, root); + Assert.Null(model.Indexer); + Assert.NotNull(model.Methods.SingleOrDefault(static x => x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility) && x.Name.Equals("ByIdInteger") && x.OriginalIndexer != null && x.OriginalIndexer.IndexType.Name.Equals("Integer", StringComparison.OrdinalIgnoreCase))); + Assert.NotNull(model.Methods.SingleOrDefault(static x => x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility) && x.Name.Equals("ById") && x.OriginalIndexer != null && x.OriginalIndexer.IndexType.Name.Equals("string", StringComparison.OrdinalIgnoreCase))); + } + [Fact] public async Task AddsExceptionInheritanceOnErrorClasses() { var model = root.AddClass(new CodeClass From 28fa9b324fe5053e6388ee441dd0ce3cc18702b0 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 3 Aug 2023 14:22:44 -0400 Subject: [PATCH 3/5] - adds changelog entry for indexers type specific --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc372e5783..07767373ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added auto-generation header for class and enums in CSharp [#2886](https://github.com/microsoft/kiota/issues/2886) - Added support for multipart form data request body in CSharp, Go, Java, and TypeScript. [#220](https://github.com/microsoft/kiota/issues/220) - Added support for base64 encoded properties in TypeScript. +- Added support for type specific (non string) indexers parameters. [#2594](https://github.com/microsoft/kiota/issues/2594) ### Changed From afcb4ad18e7cd2dbdf1ccfa88d1b8813ba6a96e2 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 3 Aug 2023 14:49:52 -0400 Subject: [PATCH 4/5] - code linting Signed-off-by: Vincent Biret --- src/Kiota.Builder/CodeDOM/CodeClass.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Kiota.Builder/CodeDOM/CodeClass.cs b/src/Kiota.Builder/CodeDOM/CodeClass.cs index 585d74ca55..7b6112ca9c 100644 --- a/src/Kiota.Builder/CodeDOM/CodeClass.cs +++ b/src/Kiota.Builder/CodeDOM/CodeClass.cs @@ -48,7 +48,7 @@ public CodeComposedTypeBase? OriginalComposedType public CodeIndexer? Indexer => InnerChildElements.Values.OfType().FirstOrDefault(static x => !x.Deprecation?.IsDeprecated ?? true); public void AddIndexer(params CodeIndexer[] indexers) { - if (indexers == null || indexers.Any(static x => x == null)) + if (indexers == null || Array.Exists(indexers, static x => x == null)) throw new ArgumentNullException(nameof(indexers)); if (!indexers.Any()) throw new ArgumentOutOfRangeException(nameof(indexers)); @@ -56,7 +56,7 @@ public void AddIndexer(params CodeIndexer[] indexers) foreach (var value in indexers) { var existingIndexers = InnerChildElements.Values.OfType().ToArray(); - if (existingIndexers.Any(x => !x.IndexParameterName.Equals(value.IndexParameterName, StringComparison.OrdinalIgnoreCase)) || + if (Array.Exists(existingIndexers, x => !x.IndexParameterName.Equals(value.IndexParameterName, StringComparison.OrdinalIgnoreCase)) || InnerChildElements.Values.OfType().Any(static x => x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility))) { foreach (var existingIndexer in existingIndexers) From 1736364cd19766debeb129ff7f3b82d1cffc9e4a Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 3 Aug 2023 15:11:17 -0400 Subject: [PATCH 5/5] - fixes flaky behavior for indexer replacement --- src/Kiota.Builder/CodeDOM/CodeMethod.cs | 5 +++-- src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs | 11 +++++++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Kiota.Builder/CodeDOM/CodeMethod.cs b/src/Kiota.Builder/CodeDOM/CodeMethod.cs index 49b8fbc692..c34b4d1157 100644 --- a/src/Kiota.Builder/CodeDOM/CodeMethod.cs +++ b/src/Kiota.Builder/CodeDOM/CodeMethod.cs @@ -2,6 +2,7 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Linq; +using Kiota.Builder.Extensions; namespace Kiota.Builder.CodeDOM; @@ -78,7 +79,7 @@ public object Clone() public class CodeMethod : CodeTerminalWithKind, ICloneable, IDocumentedElement, IDeprecableElement { public static readonly CodeParameterKind ParameterKindForConvertedIndexers = CodeParameterKind.Custom; - public static CodeMethod FromIndexer(CodeIndexer originalIndexer, Func methodNameCallback, Func parameterNameCallback, bool parameterNullable) + public static CodeMethod FromIndexer(CodeIndexer originalIndexer, Func methodNameCallback, Func parameterNameCallback, bool parameterNullable, bool typeSpecificOverload = false) { ArgumentNullException.ThrowIfNull(originalIndexer); ArgumentNullException.ThrowIfNull(methodNameCallback); @@ -89,7 +90,7 @@ public static CodeMethod FromIndexer(CodeIndexer originalIndexer, Func x.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility) && x.OriginalIndexer is not null && IsIndexerTypeSpecificVersion(currentIndexer.IndexParameterName, x.OriginalIndexer)) || @@ -708,6 +709,12 @@ currentIndexer.Deprecation is not null && currentIndexer.Deprecation.IsDeprecate { indexerParentClass.RenameChildElement(typeSpecificCompatibleMethod.Name, typeSpecificCompatibleMethod.Name + typeSpecificCompatibleMethod.OriginalIndexer.IndexType.Name.ToFirstCharacterUpperCase()); } + else if (indexerParentClass.Indexer != null && indexerParentClass.Indexer != currentIndexer && IsIndexerTypeSpecificVersion(currentIndexer.IndexParameterName, indexerParentClass.Indexer)) + { + var specificIndexer = indexerParentClass.Indexer; + indexerParentClass.RemoveChildElement(specificIndexer); + indexerParentClass.AddMethod(CodeMethod.FromIndexer(specificIndexer, methodNameCallback, parameterNameCallback, parameterNullable, true)); + } indexerParentClass.AddMethod(CodeMethod.FromIndexer(currentIndexer, methodNameCallback, parameterNameCallback, parameterNullable)); } else if (!isIndexerStringBackwardCompatible)