From 4feb08b1e08d521cb7ce7b19d16fde32c8c29917 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 13 Dec 2023 11:16:43 -0500 Subject: [PATCH 1/2] - fixes a regression with path deduplication Signed-off-by: Vincent Biret --- CHANGELOG.md | 1 + .../OpenApiUrlTreeNodeExtensions.cs | 24 +++++++++++++++---- src/Kiota.Builder/KiotaBuilder.cs | 15 ++++++------ src/kiota/Handlers/KiotaShowCommandHandler.cs | 3 ++- src/kiota/Rpc/Server.cs | 3 ++- .../Kiota.Builder.Tests/KiotaBuilderTests.cs | 9 +++++-- 6 files changed, 39 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bae412d5d0..ba302dfa44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Fixed a regression introduced by [#3760](https://github.com/microsoft/kiota/issues/3760) in 1.9.0 and its previews where indexer parameter name would be wrong leading to invalid URLs. [#3901](https://github.com/microsoft/kiota/issues/3901) - Fixed a bug in the vscode extension where the "Paste API Manifest" button would not be able to parse the manifest. - Enhance the way Enums are expressed in Typescript. [#2105](https://github.com/microsoft/kiota/issues/2105) diff --git a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs index 22f30c8dbc..9e45a8ba83 100644 --- a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs @@ -73,11 +73,11 @@ public static IEnumerable GetPathParametersForCurrentSegment(t if (node != null && node.IsComplexPathMultipleParameters()) if (node.PathItems.TryGetValue(Constants.DefaultOpenApiLabel, out var pathItem)) - return GetParametersForPathItem(pathItem, node.Segment); + return GetParametersForPathItem(pathItem, node.DeduplicatedSegment()); else if (node.Children.Any()) return node.Children .Where(static x => x.Value.PathItems.ContainsKey(Constants.DefaultOpenApiLabel)) - .SelectMany(x => GetParametersForPathItem(x.Value.PathItems[Constants.DefaultOpenApiLabel], node.Segment)) + .SelectMany(x => GetParametersForPathItem(x.Value.PathItems[Constants.DefaultOpenApiLabel], node.DeduplicatedSegment())) .Distinct(); return Enumerable.Empty(); } @@ -110,7 +110,7 @@ private static string GetSegmentName(this OpenApiUrlTreeNode currentNode, Struct responseClassName : ((requestBody ? operation?.GetRequestSchema(structuredMimeTypes) : operation?.GetResponseSchema(structuredMimeTypes))?.Reference?.GetClassName() is string requestClassName && !string.IsNullOrEmpty(requestClassName) ? requestClassName : - CleanupParametersFromPath(currentNode.Segment)?.ReplaceValueIdentifier())); + CleanupParametersFromPath(currentNode.DeduplicatedSegment())?.ReplaceValueIdentifier())); if (!string.IsNullOrEmpty(rawClassName) && string.IsNullOrEmpty(referenceName)) { if (stripExtensionForIndexersTestRegex().IsMatch(rawClassName)) // {id}.json is considered as indexer @@ -159,7 +159,7 @@ public static string GetPathItemDescription(this OpenApiUrlTreeNode currentNode, } public static bool DoesNodeBelongToItemSubnamespace(this OpenApiUrlTreeNode currentNode) => currentNode.IsPathSegmentWithSingleSimpleParameter(); public static bool IsPathSegmentWithSingleSimpleParameter(this OpenApiUrlTreeNode currentNode) => - currentNode?.Segment.IsPathSegmentWithSingleSimpleParameter() ?? false; + currentNode?.DeduplicatedSegment().IsPathSegmentWithSingleSimpleParameter() ?? false; private static bool IsPathSegmentWithSingleSimpleParameter(this string currentSegment) { if (string.IsNullOrEmpty(currentSegment)) return false; @@ -180,7 +180,7 @@ private static bool IsPathSegmentWithNumberOfParameters(this string currentSegme [GeneratedRegex(@"\{\w+\}\.(?:json|yaml|yml|csv|txt)$", RegexOptions.Singleline, 500)] private static partial Regex stripExtensionForIndexersTestRegex(); // so {param-name}.json is considered as indexer public static bool IsComplexPathMultipleParameters(this OpenApiUrlTreeNode currentNode) => - (currentNode?.Segment?.IsPathSegmentWithNumberOfParameters(static x => x.Any()) ?? false) && !currentNode.IsPathSegmentWithSingleSimpleParameter(); + (currentNode?.DeduplicatedSegment()?.IsPathSegmentWithNumberOfParameters(static x => x.Any()) ?? false) && !currentNode.IsPathSegmentWithSingleSimpleParameter(); public static string GetUrlTemplate(this OpenApiUrlTreeNode currentNode) { ArgumentNullException.ThrowIfNull(currentNode); @@ -244,4 +244,18 @@ public static string SanitizeParameterNameForCodeSymbols(this string original, s if (string.IsNullOrEmpty(original)) return original; return removePctEncodedCharacters().Replace(original.ToCamelCase('-', '.', '~').SanitizeParameterNameForUrlTemplate(), replaceEncodedCharactersWith); } + private const string DeduplicatedSegmentKey = "x-ms-kiota-deduplicatedSegment"; + public static string DeduplicatedSegment(this OpenApiUrlTreeNode currentNode) + { + if (currentNode is null) return string.Empty; + if (currentNode.AdditionalData.TryGetValue(DeduplicatedSegmentKey, out var deduplicatedSegment) && deduplicatedSegment.FirstOrDefault() is string deduplicatedSegmentString) + return deduplicatedSegmentString; + return currentNode.Segment; + } + public static void AddDeduplicatedSegment(this OpenApiUrlTreeNode openApiUrlTreeNode, string newName) + { + ArgumentNullException.ThrowIfNull(openApiUrlTreeNode); + ArgumentException.ThrowIfNullOrEmpty(newName); + openApiUrlTreeNode.AdditionalData.Add(DeduplicatedSegmentKey, [newName]); + } } diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index 74a75272d5..c876f9d66d 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -552,6 +552,7 @@ private void MergeIndexNodesAtSameLevel(OpenApiUrlTreeNode node) node.Children.Remove(indexNode.Key); var newSegmentParameterName = $"{{{node.Segment.CleanupSymbolName()}-id}}"; indexNode.Value.Path = indexNode.Value.Path.Replace(indexNode.Key, newSegmentParameterName, StringComparison.OrdinalIgnoreCase); + indexNode.Value.AddDeduplicatedSegment(newSegmentParameterName); node.Children.Add(newSegmentParameterName, indexNode.Value); CopyNodeIntoOtherNode(indexNode.Value, indexNode.Value, indexNode.Key, newSegmentParameterName); foreach (var child in indexNodes.Except(new[] { indexNode })) @@ -727,7 +728,7 @@ private void CreateRequestBuilderClass(CodeNamespace currentNamespace, OpenApiUr if (child.Value.IsPathSegmentWithSingleSimpleParameter()) { - var indexerParameterType = GetIndexerParameterType(child.Value, currentNode); + var indexerParameterType = GetIndexerParameter(child.Value, currentNode); codeClass.AddIndexer(CreateIndexer($"{propIdentifier}-indexer", propType, indexerParameterType, child.Value, currentNode)); } else if (child.Value.IsComplexPathMultipleParameters()) @@ -1076,7 +1077,7 @@ private IEnumerable GetUnmappedTypeDefinitions(CodeElement codeElement } private static CodeType DefaultIndexerParameterType => new() { Name = "string", IsExternal = true }; private const char OpenAPIUrlTreeNodePathSeparator = '\\'; - private CodeParameter GetIndexerParameterType(OpenApiUrlTreeNode currentNode, OpenApiUrlTreeNode parentNode) + private CodeParameter GetIndexerParameter(OpenApiUrlTreeNode currentNode, OpenApiUrlTreeNode parentNode) { var parameterName = string.Join(OpenAPIUrlTreeNodePathSeparator, currentNode.Path.Split(OpenAPIUrlTreeNodePathSeparator, StringSplitOptions.RemoveEmptyEntries) .Skip(parentNode.Path.Count(static x => x == OpenAPIUrlTreeNodePathSeparator))) @@ -1095,11 +1096,12 @@ private CodeParameter GetIndexerParameterType(OpenApiUrlTreeNode currentNode, Op _ => GetPrimitiveType(parameter.Schema), } ?? DefaultIndexerParameterType; type.IsNullable = false; + var segment = currentNode.DeduplicatedSegment(); var result = new CodeParameter { Type = type, - SerializationName = currentNode.Segment.SanitizeParameterNameForUrlTemplate(), - Name = currentNode.Segment.CleanupSymbolName(), + SerializationName = segment.SanitizeParameterNameForUrlTemplate(), + Name = segment.CleanupSymbolName(), Documentation = new() { Description = parameter?.Description.CleanupDescription() is string description && !string.IsNullOrEmpty(description) ? description : "Unique identifier of the item", @@ -1127,15 +1129,14 @@ private static IDictionary GetPathItems(OpenApiUrlTreeN private CodeIndexer[] CreateIndexer(string childIdentifier, string childType, CodeParameter parameter, OpenApiUrlTreeNode currentNode, OpenApiUrlTreeNode parentNode) { logger.LogTrace("Creating indexer {Name}", childIdentifier); - var result = new List { new CodeIndexer - { + var result = new List { new() { Name = childIdentifier, Documentation = new() { Description = currentNode.GetPathItemDescription(Constants.DefaultOpenApiLabel, $"Gets an item from the {currentNode.GetNodeNamespaceFromPath(config.ClientNamespaceName)} collection"), }, ReturnType = new CodeType { Name = childType }, - PathSegment = parentNode.GetNodeNamespaceFromPath(string.Empty).Split('.').Last(), + PathSegment = parentNode.GetNodeNamespaceFromPath(string.Empty).Split('.')[^1], Deprecation = currentNode.GetDeprecationInformation(), IndexParameter = parameter, }}; diff --git a/src/kiota/Handlers/KiotaShowCommandHandler.cs b/src/kiota/Handlers/KiotaShowCommandHandler.cs index 33b51b2376..4c6b8febba 100644 --- a/src/kiota/Handlers/KiotaShowCommandHandler.cs +++ b/src/kiota/Handlers/KiotaShowCommandHandler.cs @@ -7,6 +7,7 @@ using System.Threading; using System.Threading.Tasks; using Kiota.Builder; +using Kiota.Builder.Extensions; using Microsoft.Extensions.Logging; using Microsoft.OpenApi.Services; @@ -119,7 +120,7 @@ public override async Task InvokeAsync(InvocationContext context) private const string Space = " "; private static void RenderNode(OpenApiUrlTreeNode node, uint maxDepth, StringBuilder builder, string indent = "", int nodeDepth = 0) { - builder.AppendLine(node.Segment); + builder.AppendLine(node.DeduplicatedSegment()); var children = node.Children; var numberOfChildren = children.Count; diff --git a/src/kiota/Rpc/Server.cs b/src/kiota/Rpc/Server.cs index 1db32d1ecc..b067faed4c 100644 --- a/src/kiota/Rpc/Server.cs +++ b/src/kiota/Rpc/Server.cs @@ -11,6 +11,7 @@ using Kiota.Builder.Lock; using Kiota.Builder.Logging; using Kiota.Generated; +using Kiota.Builder.Extensions; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging; using Microsoft.OpenApi.Models; @@ -218,7 +219,7 @@ private static PathItem ConvertOpenApiUrlTreeNodeToPathItem(OpenApiUrlTreeNode n .OrderByDescending(static x => x.isOperation) .ThenBy(static x => x.segment, StringComparer.OrdinalIgnoreCase) .ToArray(); - return new PathItem(node.Path, node.Segment, children, filteredPaths.Count == 0 || Array.Exists(children, static x => x.isOperation) && children.Where(static x => x.isOperation).All(static x => x.selected)); + return new PathItem(node.Path, node.DeduplicatedSegment(), children, filteredPaths.Count == 0 || Array.Exists(children, static x => x.isOperation) && children.Where(static x => x.isOperation).All(static x => x.selected)); } protected static string GetAbsolutePath(string source) { diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index 102da7644b..5d7da8d05d 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -932,8 +932,8 @@ await File.WriteAllTextAsync(tempFilePath, @"openapi: 3.0.1 var builder = new KiotaBuilder(mockLogger.Object, new GenerationConfiguration { ClientClassName = "Graph", OpenAPIFilePath = tempFilePath }, _httpClient); var treeNode = await builder.GetUrlTreeNodeAsync(new CancellationToken()); Assert.NotNull(treeNode); - Assert.Equal("/", treeNode.Segment); - Assert.Equal("enumeration", treeNode.Children.First().Value.Segment); + Assert.Equal("/", treeNode.DeduplicatedSegment()); + Assert.Equal("enumeration", treeNode.Children.First().Value.DeduplicatedSegment()); _tempFiles.Add(tempFilePath); } @@ -6520,6 +6520,11 @@ public void SinglePathParametersAreDeduplicated() var builder = new KiotaBuilder(mockLogger, new GenerationConfiguration { ClientClassName = "Graph", ApiRootUrl = "https://localhost" }, _httpClient); var node = builder.CreateUriSpace(document); var codeModel = builder.CreateSourceModel(node); + var usersRB = codeModel.FindNamespaceByName("ApiSdk.users").FindChildByName("UsersRequestBuilder", false); + Assert.NotNull(usersRB); + var usersIndexer = usersRB.Indexer; + Assert.NotNull(usersIndexer); + Assert.Equal("users%2Did", usersIndexer.IndexParameter.SerializationName); var managerRB = codeModel.FindNamespaceByName("ApiSdk.users.item.manager").FindChildByName("ManagerRequestBuilder", false); Assert.NotNull(managerRB); var managerUrlTemplate = managerRB.FindChildByName("UrlTemplate", false); From 92758dd46306e9cdbc4eac08bb1e799a19f596e1 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 13 Dec 2023 12:27:18 -0500 Subject: [PATCH 2/2] - fixes formatting --- src/kiota/Rpc/Server.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kiota/Rpc/Server.cs b/src/kiota/Rpc/Server.cs index b067faed4c..3972129f0b 100644 --- a/src/kiota/Rpc/Server.cs +++ b/src/kiota/Rpc/Server.cs @@ -8,10 +8,10 @@ using System.Threading.Tasks; using Kiota.Builder; using Kiota.Builder.Configuration; +using Kiota.Builder.Extensions; using Kiota.Builder.Lock; using Kiota.Builder.Logging; using Kiota.Generated; -using Kiota.Builder.Extensions; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging; using Microsoft.OpenApi.Models;