From 98000a4e4a160271416a143c79695236c92b30b3 Mon Sep 17 00:00:00 2001 From: Andrea Peruffo Date: Thu, 8 Feb 2024 10:54:45 +0000 Subject: [PATCH 1/7] Repro for 4151 --- .../Kiota.Builder.Tests/KiotaBuilderTests.cs | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index cc0f7850af..a14f358faa 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -6592,6 +6592,113 @@ public void SinglePathParametersAreDeduplicated() Assert.Equal("{+baseurl}/users/{users%2Did}/careerAdvisor/{id}", careerAdvisorItemUrlTemplate.DefaultValue.Trim('"')); } [Fact] + public void SinglePathParametersAreDeduplicatedAndOrderIsRespected() + { + var ownerSchema = new OpenApiSchema + { + Type = "object", + Properties = new Dictionary { + { + "id", new OpenApiSchema { + Type = "string" + } + } + }, + Reference = new OpenApiReference + { + Id = "#/components/schemas/owner" + }, + UnresolvedReference = false + }; + var repoSchema = new OpenApiSchema + { + Type = "object", + Properties = new Dictionary { + { + "id", new OpenApiSchema { + Type = "string" + } + } + }, + Reference = new OpenApiReference + { + Id = "#/components/schemas/repo" + }, + UnresolvedReference = false + }; + var document = new OpenApiDocument + { + Paths = new OpenApiPaths + { + ["/repos/{owner}/{repo}"] = new OpenApiPathItem + { + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Responses = new OpenApiResponses { + ["200"] = new OpenApiResponse + { + Content = { + ["application/json"] = new OpenApiMediaType + { + Schema = repoSchema + } + } + } + } + } + } + }, + ["/repos/{template_owner}/{template_repo}/generate"] = new OpenApiPathItem + { + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Responses = new OpenApiResponses { + ["200"] = new OpenApiResponse + { + Content = { + ["application/json"] = new OpenApiMediaType + { + Schema = repoSchema + } + } + } + } + } + } + } + }, + Components = new OpenApiComponents + { + Schemas = new Dictionary { + {"owner", ownerSchema}, + {"repo", repoSchema} + } + } + }; + var mockLogger = new CountLogger(); + var builder = new KiotaBuilder(mockLogger, new GenerationConfiguration { ClientClassName = "GitHub", ApiRootUrl = "https://localhost" }, _httpClient); + var node = builder.CreateUriSpace(document); + var codeModel = builder.CreateSourceModel(node); + + // Expected + var reposRB = codeModel.FindNamespaceByName("ApiSdk.repos.item.item").FindChildByName("ReposItemRequestBuilder", false); + Assert.NotNull(reposRB); + var repoUrlTemplate = reposRB.FindChildByName("UrlTemplate", false); + Assert.NotNull(repoUrlTemplate); + Assert.Equal("{+baseurl}/repos/{owner}/{repos%2Did}", repoUrlTemplate.DefaultValue.Trim('"')); + Console.WriteLine(repoUrlTemplate.DefaultValue.Trim('"')); + + // Current behavior + // var reposRB = codeModel.FindNamespaceByName("ApiSdk.repos.item.item").FindChildByName("OwnerItemRequestBuilder", false); + // Assert.NotNull(reposRB); + // var repoUrlTemplate = reposRB.FindChildByName("UrlTemplate", false); + // Assert.NotNull(repoUrlTemplate); + // Assert.Equal("{+baseurl}/repos/{repos%2Did}/{Owner%2Did}", repoUrlTemplate.DefaultValue.Trim('"')); + // Console.WriteLine(repoUrlTemplate.DefaultValue.Trim('"')); + } + [Fact] public void AddReservedPathParameterSymbol() { var userSchema = new OpenApiSchema From 1d920e5c8104a45413169cd2d74430d00ec5914d Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 15 Feb 2024 13:35:26 -0500 Subject: [PATCH 2/7] - moves node deduplication to extension methods --- .../OpenApiUrlTreeNodeExtensions.cs | 70 ++++++++++++++++++ src/Kiota.Builder/KiotaBuilder.cs | 71 +------------------ 2 files changed, 71 insertions(+), 70 deletions(-) diff --git a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs index b531c7f77b..8910eab4c2 100644 --- a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Text.RegularExpressions; using Kiota.Builder.Configuration; +using Microsoft.Extensions.Logging; using Microsoft.OpenApi.MicrosoftExtensions; using Microsoft.OpenApi.Models; using Microsoft.OpenApi.Services; @@ -268,4 +269,73 @@ public static void AddDeduplicatedSegment(this OpenApiUrlTreeNode openApiUrlTree ArgumentException.ThrowIfNullOrEmpty(newName); openApiUrlTreeNode.AdditionalData.Add(DeduplicatedSegmentKey, [newName]); } + internal static void MergeIndexNodesAtSameLevel(this OpenApiUrlTreeNode node, ILogger logger) + { + var indexNodes = node.Children + .Where(static x => x.Value.IsPathSegmentWithSingleSimpleParameter()) + .OrderBy(static x => x.Key, StringComparer.OrdinalIgnoreCase) + .ToArray(); + if (indexNodes.Length > 1) + { + var indexNode = indexNodes[0]; + 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, logger); + foreach (var child in indexNodes.Except([indexNode])) + { + node.Children.Remove(child.Key); + CopyNodeIntoOtherNode(child.Value, indexNode.Value, child.Key, newSegmentParameterName, logger); + } + } + + foreach (var child in node.Children.Values) + MergeIndexNodesAtSameLevel(child, logger); + } + private static void CopyNodeIntoOtherNode(OpenApiUrlTreeNode source, OpenApiUrlTreeNode destination, string pathParameterNameToReplace, string pathParameterNameReplacement, ILogger logger) + { + foreach (var child in source.Children) + { + child.Value.Path = child.Value.Path.Replace(pathParameterNameToReplace, pathParameterNameReplacement, StringComparison.OrdinalIgnoreCase); + if (!destination.Children.TryAdd(child.Key, child.Value)) + CopyNodeIntoOtherNode(child.Value, destination.Children[child.Key], pathParameterNameToReplace, pathParameterNameReplacement, logger); + } + pathParameterNameToReplace = pathParameterNameToReplace.Trim('{', '}'); + pathParameterNameReplacement = pathParameterNameReplacement.Trim('{', '}'); + foreach (var pathItem in source.PathItems) + { + foreach (var pathParameter in pathItem + .Value + .Parameters + .Where(x => x.In == ParameterLocation.Path && pathParameterNameToReplace.Equals(x.Name, StringComparison.Ordinal)) + .Union( + pathItem + .Value + .Operations + .SelectMany(static x => x.Value.Parameters) + .Where(x => x.In == ParameterLocation.Path && pathParameterNameToReplace.Equals(x.Name, StringComparison.Ordinal)) + )) + { + pathParameter.Name = pathParameterNameReplacement; + } + if (source != destination && !destination.PathItems.TryAdd(pathItem.Key, pathItem.Value)) + { + var destinationPathItem = destination.PathItems[pathItem.Key]; + foreach (var operation in pathItem.Value.Operations) + if (!destinationPathItem.Operations.TryAdd(operation.Key, operation.Value)) + { + logger.LogWarning("Duplicate operation {Operation} in path {Path}", operation.Key, pathItem.Key); + } + foreach (var pathParameter in pathItem.Value.Parameters) + destinationPathItem.Parameters.Add(pathParameter); + foreach (var extension in pathItem.Value.Extensions) + if (!destinationPathItem.Extensions.TryAdd(extension.Key, extension.Value)) + { + logger.LogWarning("Duplicate extension {Extension} in path {Path}", extension.Key, pathItem.Key); + } + } + } + } } diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index 0351366d1b..756afb3228 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -549,80 +549,11 @@ public OpenApiUrlTreeNode CreateUriSpace(OpenApiDocument doc) var stopwatch = new Stopwatch(); stopwatch.Start(); var node = OpenApiUrlTreeNode.Create(doc, Constants.DefaultOpenApiLabel); - MergeIndexNodesAtSameLevel(node); + node.MergeIndexNodesAtSameLevel(logger); stopwatch.Stop(); logger.LogTrace("{Timestamp}ms: Created UriSpace tree", stopwatch.ElapsedMilliseconds); return node; } - private void MergeIndexNodesAtSameLevel(OpenApiUrlTreeNode node) - { - var indexNodes = node.Children - .Where(static x => x.Value.IsPathSegmentWithSingleSimpleParameter()) - .OrderBy(static x => x.Key, StringComparer.OrdinalIgnoreCase) - .ToArray(); - if (indexNodes.Length > 1) - { - var indexNode = indexNodes[0]; - 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 })) - { - node.Children.Remove(child.Key); - CopyNodeIntoOtherNode(child.Value, indexNode.Value, child.Key, newSegmentParameterName); - } - } - - foreach (var child in node.Children.Values) - MergeIndexNodesAtSameLevel(child); - } - private void CopyNodeIntoOtherNode(OpenApiUrlTreeNode source, OpenApiUrlTreeNode destination, string pathParameterNameToReplace, string pathParameterNameReplacement) - { - foreach (var child in source.Children) - { - child.Value.Path = child.Value.Path.Replace(pathParameterNameToReplace, pathParameterNameReplacement, StringComparison.OrdinalIgnoreCase); - if (!destination.Children.TryAdd(child.Key, child.Value)) - CopyNodeIntoOtherNode(child.Value, destination.Children[child.Key], pathParameterNameToReplace, pathParameterNameReplacement); - } - pathParameterNameToReplace = pathParameterNameToReplace.Trim('{', '}'); - pathParameterNameReplacement = pathParameterNameReplacement.Trim('{', '}'); - foreach (var pathItem in source.PathItems) - { - foreach (var pathParameter in pathItem - .Value - .Parameters - .Where(x => x.In == ParameterLocation.Path && pathParameterNameToReplace.Equals(x.Name, StringComparison.Ordinal)) - .Union( - pathItem - .Value - .Operations - .SelectMany(static x => x.Value.Parameters) - .Where(x => x.In == ParameterLocation.Path && pathParameterNameToReplace.Equals(x.Name, StringComparison.Ordinal)) - )) - { - pathParameter.Name = pathParameterNameReplacement; - } - if (source != destination && !destination.PathItems.TryAdd(pathItem.Key, pathItem.Value)) - { - var destinationPathItem = destination.PathItems[pathItem.Key]; - foreach (var operation in pathItem.Value.Operations) - if (!destinationPathItem.Operations.TryAdd(operation.Key, operation.Value)) - { - logger.LogWarning("Duplicate operation {Operation} in path {Path}", operation.Key, pathItem.Key); - } - foreach (var pathParameter in pathItem.Value.Parameters) - destinationPathItem.Parameters.Add(pathParameter); - foreach (var extension in pathItem.Value.Extensions) - if (!destinationPathItem.Extensions.TryAdd(extension.Key, extension.Value)) - { - logger.LogWarning("Duplicate extension {Extension} in path {Path}", extension.Key, pathItem.Key); - } - } - } - } private CodeNamespace? rootNamespace; private CodeNamespace? modelsNamespace; private string? modelNamespacePrefixToTrim; From dc91c810b4f3ddc43ed031f4ddc7894e2b63fcd4 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 15 Feb 2024 14:25:06 -0500 Subject: [PATCH 3/7] - moves unit tests for path merge to url tree node extensions --- .../OpenApiUrlTreeNodeExtensions.cs | 2 +- .../OpenApiUrlTreeNodeExtensionsTests.cs | 270 +++++++++++++++++- .../Kiota.Builder.Tests/KiotaBuilderTests.cs | 227 --------------- 3 files changed, 268 insertions(+), 231 deletions(-) diff --git a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs index 8910eab4c2..280d8a2c69 100644 --- a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs @@ -80,7 +80,7 @@ public static IEnumerable GetPathParametersForCurrentSegment(t .Where(static x => x.Value.PathItems.ContainsKey(Constants.DefaultOpenApiLabel)) .SelectMany(x => GetParametersForPathItem(x.Value.PathItems[Constants.DefaultOpenApiLabel], node.DeduplicatedSegment())) .Distinct(); - return Enumerable.Empty(); + return []; } private const char PathNameSeparator = '\\'; [GeneratedRegex(@"-?id\d?}?$", RegexOptions.Singleline | RegexOptions.IgnoreCase, 500)] diff --git a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs index c97dd49ea1..53b9e3bd78 100644 --- a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs @@ -1,6 +1,8 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Linq; - +using System.Net.Http; +using Kiota.Builder.Configuration; using Kiota.Builder.Extensions; using Microsoft.OpenApi.Models; @@ -9,7 +11,7 @@ using Xunit; namespace Kiota.Builder.Tests.Extensions; -public class OpenApiUrlTreeNodeExtensionsTests +public sealed class OpenApiUrlTreeNodeExtensionsTests : IDisposable { [Fact] public void Defensive() @@ -673,4 +675,266 @@ public void GetsClassNameWithSegmentsToSkipForClassNames() // validate that we get a valid class name Assert.Equal("json", responseClassName); } + [Fact] + public void SinglePathParametersAreDeduplicated() + { + var userSchema = new OpenApiSchema + { + Type = "object", + Properties = new Dictionary { + { + "id", new OpenApiSchema { + Type = "string" + } + }, + { + "displayName", new OpenApiSchema { + Type = "string" + } + } + }, + Reference = new OpenApiReference + { + Id = "#/components/schemas/microsoft.graph.user" + }, + UnresolvedReference = false + }; + var document = new OpenApiDocument + { + Paths = new OpenApiPaths + { + ["users/{foo}/careerAdvisor/{id}"] = new OpenApiPathItem + { + Parameters = { + new OpenApiParameter { + Name = "foo", + In = ParameterLocation.Path, + Required = true, + Schema = new OpenApiSchema { + Type = "string" + } + }, + }, + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Responses = new OpenApiResponses { + ["200"] = new OpenApiResponse + { + Content = { + ["application/json"] = new OpenApiMediaType + { + Schema = userSchema + } + } + } + } + } + } + }, + ["users/{id}/careerAdvisor"] = new OpenApiPathItem + { + Parameters = { + new OpenApiParameter { + Name = "id", + In = ParameterLocation.Path, + Required = true, + Schema = new OpenApiSchema { + Type = "string" + } + }, + }, + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Responses = new OpenApiResponses { + ["200"] = new OpenApiResponse + { + Content = { + ["application/json"] = new OpenApiMediaType + { + Schema = userSchema + } + } + } + } + } + } + }, + ["users/{user-id}/manager"] = new OpenApiPathItem + { + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Parameters = { + new OpenApiParameter { + Name = "user-id", + In = ParameterLocation.Path, + Required = true, + Schema = new OpenApiSchema { + Type = "string" + } + }, + }, + Responses = new OpenApiResponses { + ["200"] = new OpenApiResponse + { + Content = { + ["application/json"] = new OpenApiMediaType + { + Schema = userSchema + } + } + } + } + } + } + }, + }, + Components = new OpenApiComponents + { + Schemas = new Dictionary { + { + "microsoft.graph.user", userSchema + } + } + } + }; + var mockLogger = new CountLogger(); + var builder = new KiotaBuilder(mockLogger, new GenerationConfiguration { ClientClassName = "Graph", ApiRootUrl = "https://localhost" }, _httpClient); + var node = builder.CreateUriSpace(document); + node.MergeIndexNodesAtSameLevel(mockLogger); + var usersCollectionIndexNode = GetChildNodeByPath(node, "users/{users-id}"); + Assert.NotNull(usersCollectionIndexNode); + Assert.Equal("{+baseurl}/users/{users%2Did}", usersCollectionIndexNode.GetUrlTemplate()); + + var managerNode = GetChildNodeByPath(node, "users/{users-id}/manager"); + Assert.NotNull(managerNode); + Assert.Equal("{+baseurl}/users/{users%2Did}/manager", managerNode.GetUrlTemplate()); + + var careerAdvisorNode = GetChildNodeByPath(node, "users/{users-id}/careerAdvisor"); + Assert.NotNull(careerAdvisorNode); + Assert.Equal("{+baseurl}/users/{users%2Did}/careerAdvisor", careerAdvisorNode.GetUrlTemplate()); + + var careerAdvisorIndexNode = GetChildNodeByPath(node, "users/{users-id}/careerAdvisor/{id}"); + Assert.NotNull(careerAdvisorIndexNode); + Assert.Equal("{+baseurl}/users/{users%2Did}/careerAdvisor/{id}", careerAdvisorIndexNode.GetUrlTemplate()); + var pathItem = careerAdvisorIndexNode.PathItems[Constants.DefaultOpenApiLabel]; + Assert.NotNull(pathItem); + var parameter = pathItem.Parameters.FirstOrDefault(static p => p.Name == "users-id"); + Assert.NotNull(parameter); + } + [Fact] + public void SinglePathParametersAreDeduplicatedAndOrderIsRespected() + { + var ownerSchema = new OpenApiSchema + { + Type = "object", + Properties = new Dictionary { + { + "id", new OpenApiSchema { + Type = "string" + } + } + }, + Reference = new OpenApiReference + { + Id = "#/components/schemas/owner" + }, + UnresolvedReference = false + }; + var repoSchema = new OpenApiSchema + { + Type = "object", + Properties = new Dictionary { + { + "id", new OpenApiSchema { + Type = "string" + } + } + }, + Reference = new OpenApiReference + { + Id = "#/components/schemas/repo" + }, + UnresolvedReference = false + }; + var document = new OpenApiDocument + { + Paths = new OpenApiPaths + { + ["/repos/{owner}/{repo}"] = new OpenApiPathItem + { + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Responses = new OpenApiResponses { + ["200"] = new OpenApiResponse + { + Content = { + ["application/json"] = new OpenApiMediaType + { + Schema = repoSchema + } + } + } + } + } + } + }, + ["/repos/{template_owner}/{template_repo}/generate"] = new OpenApiPathItem + { + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Responses = new OpenApiResponses { + ["200"] = new OpenApiResponse + { + Content = { + ["application/json"] = new OpenApiMediaType + { + Schema = repoSchema + } + } + } + } + } + } + } + }, + Components = new OpenApiComponents + { + Schemas = new Dictionary { + {"owner", ownerSchema}, + {"repo", repoSchema} + } + } + }; + var mockLogger = new CountLogger(); + var builder = new KiotaBuilder(mockLogger, new GenerationConfiguration { ClientClassName = "GitHub", ApiRootUrl = "https://localhost" }, _httpClient); + var node = builder.CreateUriSpace(document); + node.MergeIndexNodesAtSameLevel(mockLogger); + + // Expected + var resultNode = GetChildNodeByPath(node, "repos/{owner}/{repos%2Did}/generate"); + Assert.NotNull(resultNode); + Assert.Equal("{+baseurl}/repos/{owner}/{repos%2Did}/generate", resultNode.GetUrlTemplate()); + } + private static OpenApiUrlTreeNode GetChildNodeByPath(OpenApiUrlTreeNode node, string path) + { + var pathSegments = path.Split('/'); + if (pathSegments.Length == 0) + return null; + if (pathSegments.Length == 1 && node.Children.TryGetValue(pathSegments[0], out var result)) + return result; + if (node.Children.TryGetValue(pathSegments[0], out var currentNode)) + return GetChildNodeByPath(currentNode, string.Join('/', pathSegments.Skip(1))); + return null; + } + public void Dispose() + { + _httpClient.Dispose(); + GC.SuppressFinalize(this); + } + private static readonly HttpClient _httpClient = new(); } diff --git a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs index a14f358faa..473b2775b4 100644 --- a/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs +++ b/tests/Kiota.Builder.Tests/KiotaBuilderTests.cs @@ -6472,233 +6472,6 @@ await File.WriteAllTextAsync(tempFilePath, @$"openapi: 3.0.1 Assert.NotNull(inlineType); } [Fact] - public void SinglePathParametersAreDeduplicated() - { - var userSchema = new OpenApiSchema - { - Type = "object", - Properties = new Dictionary { - { - "id", new OpenApiSchema { - Type = "string" - } - }, - { - "displayName", new OpenApiSchema { - Type = "string" - } - } - }, - Reference = new OpenApiReference - { - Id = "#/components/schemas/microsoft.graph.user" - }, - UnresolvedReference = false - }; - var document = new OpenApiDocument - { - Paths = new OpenApiPaths - { - ["users/{foo}/careerAdvisor/{id}"] = new OpenApiPathItem - { - Operations = { - [OperationType.Get] = new OpenApiOperation - { - Responses = new OpenApiResponses { - ["200"] = new OpenApiResponse - { - Content = { - ["application/json"] = new OpenApiMediaType - { - Schema = userSchema - } - } - } - } - } - } - }, - ["users/{id}/careerAdvisor"] = new OpenApiPathItem - { - Operations = { - [OperationType.Get] = new OpenApiOperation - { - Responses = new OpenApiResponses { - ["200"] = new OpenApiResponse - { - Content = { - ["application/json"] = new OpenApiMediaType - { - Schema = userSchema - } - } - } - } - } - } - }, - ["users/{user-id}/manager"] = new OpenApiPathItem - { - Operations = { - [OperationType.Get] = new OpenApiOperation - { - Responses = new OpenApiResponses { - ["200"] = new OpenApiResponse - { - Content = { - ["application/json"] = new OpenApiMediaType - { - Schema = userSchema - } - } - } - } - } - } - }, - }, - Components = new OpenApiComponents - { - Schemas = new Dictionary { - { - "microsoft.graph.user", userSchema - } - } - } - }; - var mockLogger = new CountLogger(); - 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); - Assert.NotNull(managerUrlTemplate); - Assert.Equal("{+baseurl}/users/{users%2Did}/manager", managerUrlTemplate.DefaultValue.Trim('"')); - var careerAdvisorRB = codeModel.FindNamespaceByName("ApiSdk.users.item.careerAdvisor").FindChildByName("CareerAdvisorRequestBuilder", false); - Assert.NotNull(careerAdvisorRB); - var careerAdvisorUrlTemplate = careerAdvisorRB.FindChildByName("UrlTemplate", false); - Assert.NotNull(careerAdvisorUrlTemplate); - Assert.Equal("{+baseurl}/users/{users%2Did}/careerAdvisor", careerAdvisorUrlTemplate.DefaultValue.Trim('"')); - var careerAdvisorItemRB = codeModel.FindNamespaceByName("ApiSdk.users.item.careerAdvisor.item").FindChildByName("CareerAdvisorItemRequestBuilder", false); - Assert.NotNull(careerAdvisorItemRB); - var careerAdvisorItemUrlTemplate = careerAdvisorItemRB.FindChildByName("UrlTemplate", false); - Assert.NotNull(careerAdvisorItemUrlTemplate); - Assert.Equal("{+baseurl}/users/{users%2Did}/careerAdvisor/{id}", careerAdvisorItemUrlTemplate.DefaultValue.Trim('"')); - } - [Fact] - public void SinglePathParametersAreDeduplicatedAndOrderIsRespected() - { - var ownerSchema = new OpenApiSchema - { - Type = "object", - Properties = new Dictionary { - { - "id", new OpenApiSchema { - Type = "string" - } - } - }, - Reference = new OpenApiReference - { - Id = "#/components/schemas/owner" - }, - UnresolvedReference = false - }; - var repoSchema = new OpenApiSchema - { - Type = "object", - Properties = new Dictionary { - { - "id", new OpenApiSchema { - Type = "string" - } - } - }, - Reference = new OpenApiReference - { - Id = "#/components/schemas/repo" - }, - UnresolvedReference = false - }; - var document = new OpenApiDocument - { - Paths = new OpenApiPaths - { - ["/repos/{owner}/{repo}"] = new OpenApiPathItem - { - Operations = { - [OperationType.Get] = new OpenApiOperation - { - Responses = new OpenApiResponses { - ["200"] = new OpenApiResponse - { - Content = { - ["application/json"] = new OpenApiMediaType - { - Schema = repoSchema - } - } - } - } - } - } - }, - ["/repos/{template_owner}/{template_repo}/generate"] = new OpenApiPathItem - { - Operations = { - [OperationType.Get] = new OpenApiOperation - { - Responses = new OpenApiResponses { - ["200"] = new OpenApiResponse - { - Content = { - ["application/json"] = new OpenApiMediaType - { - Schema = repoSchema - } - } - } - } - } - } - } - }, - Components = new OpenApiComponents - { - Schemas = new Dictionary { - {"owner", ownerSchema}, - {"repo", repoSchema} - } - } - }; - var mockLogger = new CountLogger(); - var builder = new KiotaBuilder(mockLogger, new GenerationConfiguration { ClientClassName = "GitHub", ApiRootUrl = "https://localhost" }, _httpClient); - var node = builder.CreateUriSpace(document); - var codeModel = builder.CreateSourceModel(node); - - // Expected - var reposRB = codeModel.FindNamespaceByName("ApiSdk.repos.item.item").FindChildByName("ReposItemRequestBuilder", false); - Assert.NotNull(reposRB); - var repoUrlTemplate = reposRB.FindChildByName("UrlTemplate", false); - Assert.NotNull(repoUrlTemplate); - Assert.Equal("{+baseurl}/repos/{owner}/{repos%2Did}", repoUrlTemplate.DefaultValue.Trim('"')); - Console.WriteLine(repoUrlTemplate.DefaultValue.Trim('"')); - - // Current behavior - // var reposRB = codeModel.FindNamespaceByName("ApiSdk.repos.item.item").FindChildByName("OwnerItemRequestBuilder", false); - // Assert.NotNull(reposRB); - // var repoUrlTemplate = reposRB.FindChildByName("UrlTemplate", false); - // Assert.NotNull(repoUrlTemplate); - // Assert.Equal("{+baseurl}/repos/{repos%2Did}/{Owner%2Did}", repoUrlTemplate.DefaultValue.Trim('"')); - // Console.WriteLine(repoUrlTemplate.DefaultValue.Trim('"')); - } - [Fact] public void AddReservedPathParameterSymbol() { var userSchema = new OpenApiSchema From 1e39f50cc22760f13474456a0068fbc80909488d Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Thu, 15 Feb 2024 15:07:00 -0500 Subject: [PATCH 4/7] - fixes deduplication algorythm taking parent segment name - fixes deduplication algo not renaming child paths and parameter names --- .../OpenApiUrlTreeNodeExtensions.cs | 30 ++++++++++++++++++- .../OpenApiUrlTreeNodeExtensionsTests.cs | 23 +++++++------- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs index 280d8a2c69..c88bce001d 100644 --- a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs @@ -279,7 +279,9 @@ internal static void MergeIndexNodesAtSameLevel(this OpenApiUrlTreeNode node, IL { var indexNode = indexNodes[0]; node.Children.Remove(indexNode.Key); - var newSegmentParameterName = $"{{{node.Segment.CleanupSymbolName()}-id}}"; + var oldSegmentName = indexNode.Value.Segment.Trim('{', '}').CleanupSymbolName(); + var segmentIndex = indexNode.Value.Path.Split('\\', StringSplitOptions.RemoveEmptyEntries).ToList().IndexOf(indexNode.Value.Segment); + var newSegmentParameterName = oldSegmentName.EndsWith("-id", StringComparison.OrdinalIgnoreCase) ? oldSegmentName : $"{{{oldSegmentName}-id}}"; indexNode.Value.Path = indexNode.Value.Path.Replace(indexNode.Key, newSegmentParameterName, StringComparison.OrdinalIgnoreCase); indexNode.Value.AddDeduplicatedSegment(newSegmentParameterName); node.Children.Add(newSegmentParameterName, indexNode.Value); @@ -289,11 +291,37 @@ internal static void MergeIndexNodesAtSameLevel(this OpenApiUrlTreeNode node, IL node.Children.Remove(child.Key); CopyNodeIntoOtherNode(child.Value, indexNode.Value, child.Key, newSegmentParameterName, logger); } + ReplaceParameterInPathForAllChildNodes(indexNode.Value, segmentIndex, newSegmentParameterName); } foreach (var child in node.Children.Values) MergeIndexNodesAtSameLevel(child, logger); } + private static void ReplaceParameterInPathForAllChildNodes(OpenApiUrlTreeNode node, int parameterIndex, string newParameterName) + { + if (parameterIndex < 0) + return; + foreach (var child in node.Children.Values) + { + var splatPath = child.Path.Split('\\', StringSplitOptions.RemoveEmptyEntries); + if (splatPath.Length > parameterIndex) + { + var oldName = splatPath[parameterIndex]; + splatPath[parameterIndex] = newParameterName; + child.Path = "\\" + string.Join('\\', splatPath); + if (node.PathItems.TryGetValue(Constants.DefaultOpenApiLabel, out var pathItem)) + { + foreach (var pathParameter in pathItem.Parameters + .Union(pathItem.Operations.SelectMany(static x => x.Value.Parameters)) + .Where(x => x.In == ParameterLocation.Path && oldName.Equals(x.Name, StringComparison.Ordinal))) + { + pathParameter.Name = newParameterName; + } + } + } + ReplaceParameterInPathForAllChildNodes(child, parameterIndex, newParameterName); + } + } private static void CopyNodeIntoOtherNode(OpenApiUrlTreeNode source, OpenApiUrlTreeNode destination, string pathParameterNameToReplace, string pathParameterNameReplacement, ILogger logger) { foreach (var child in source.Children) diff --git a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs index 53b9e3bd78..396eb1fdf5 100644 --- a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs @@ -804,24 +804,24 @@ public void SinglePathParametersAreDeduplicated() var builder = new KiotaBuilder(mockLogger, new GenerationConfiguration { ClientClassName = "Graph", ApiRootUrl = "https://localhost" }, _httpClient); var node = builder.CreateUriSpace(document); node.MergeIndexNodesAtSameLevel(mockLogger); - var usersCollectionIndexNode = GetChildNodeByPath(node, "users/{users-id}"); + var usersCollectionIndexNode = GetChildNodeByPath(node, "users/{foo-id}"); Assert.NotNull(usersCollectionIndexNode); - Assert.Equal("{+baseurl}/users/{users%2Did}", usersCollectionIndexNode.GetUrlTemplate()); + Assert.Equal("{+baseurl}/users/{foo%2Did}", usersCollectionIndexNode.GetUrlTemplate()); - var managerNode = GetChildNodeByPath(node, "users/{users-id}/manager"); + var managerNode = GetChildNodeByPath(node, "users/{foo-id}/manager"); Assert.NotNull(managerNode); - Assert.Equal("{+baseurl}/users/{users%2Did}/manager", managerNode.GetUrlTemplate()); + Assert.Equal("{+baseurl}/users/{foo%2Did}/manager", managerNode.GetUrlTemplate()); - var careerAdvisorNode = GetChildNodeByPath(node, "users/{users-id}/careerAdvisor"); + var careerAdvisorNode = GetChildNodeByPath(node, "users/{foo-id}/careerAdvisor"); Assert.NotNull(careerAdvisorNode); - Assert.Equal("{+baseurl}/users/{users%2Did}/careerAdvisor", careerAdvisorNode.GetUrlTemplate()); + Assert.Equal("{+baseurl}/users/{foo%2Did}/careerAdvisor", careerAdvisorNode.GetUrlTemplate()); - var careerAdvisorIndexNode = GetChildNodeByPath(node, "users/{users-id}/careerAdvisor/{id}"); + var careerAdvisorIndexNode = GetChildNodeByPath(node, "users/{foo-id}/careerAdvisor/{id}"); Assert.NotNull(careerAdvisorIndexNode); - Assert.Equal("{+baseurl}/users/{users%2Did}/careerAdvisor/{id}", careerAdvisorIndexNode.GetUrlTemplate()); + Assert.Equal("{+baseurl}/users/{foo%2Did}/careerAdvisor/{id}", careerAdvisorIndexNode.GetUrlTemplate()); var pathItem = careerAdvisorIndexNode.PathItems[Constants.DefaultOpenApiLabel]; Assert.NotNull(pathItem); - var parameter = pathItem.Parameters.FirstOrDefault(static p => p.Name == "users-id"); + var parameter = pathItem.Parameters.FirstOrDefault(static p => p.Name == "foo-id"); Assert.NotNull(parameter); } [Fact] @@ -916,9 +916,10 @@ public void SinglePathParametersAreDeduplicatedAndOrderIsRespected() node.MergeIndexNodesAtSameLevel(mockLogger); // Expected - var resultNode = GetChildNodeByPath(node, "repos/{owner}/{repos%2Did}/generate"); + var resultNode = GetChildNodeByPath(node, "repos/{owner-id}/{repo-id}/generate"); Assert.NotNull(resultNode); - Assert.Equal("{+baseurl}/repos/{owner}/{repos%2Did}/generate", resultNode.GetUrlTemplate()); + Assert.Equal("\\repos\\{owner-id}\\{repo-id}\\generate", resultNode.Path); + Assert.Equal("{+baseurl}/repos/{owner%2Did}/{repo%2Did}/generate", resultNode.GetUrlTemplate()); } private static OpenApiUrlTreeNode GetChildNodeByPath(OpenApiUrlTreeNode node, string path) { From 8759fb17ed46b6d44fd16dadb975fd4056b4db29 Mon Sep 17 00:00:00 2001 From: Andrea Peruffo Date: Mon, 19 Feb 2024 11:50:35 +0000 Subject: [PATCH 5/7] tests --- .github/workflows/integration-tests.yml | 1 + it/config.json | 5 +- it/java/gh/pom.xml | 74 +++++++++++++++++++ it/java/gh/src/test/java/GHAPITest.java | 23 ++++++ .../OpenApiUrlTreeNodeExtensionsTests.cs | 69 +++++++++++++++++ 5 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 it/java/gh/pom.xml create mode 100644 it/java/gh/src/test/java/GHAPITest.java diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 53739c92f5..0d367a37e3 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -57,6 +57,7 @@ jobs: - "https://developers.pipedrive.com/docs/api/v1/openapi.yaml" - "apisguru::twilio.com:api" - "apisguru::docusign.net" + - "apisguru::github.com:api.github.com" steps: - uses: actions/checkout@v4 - uses: actions/download-artifact@v4 diff --git a/it/config.json b/it/config.json index 39ef0c14d5..e153f64c5a 100644 --- a/it/config.json +++ b/it/config.json @@ -73,6 +73,9 @@ "./tests/Kiota.Builder.IntegrationTests/GeneratesUritemplateHints.yaml": { "MockServerITFolder": "query-params" }, + "apisguru::github.com:api.github.com": { + "MockServerITFolder": "gh" + }, "apisguru::notion.com": { "ExcludePatterns": [ { @@ -325,4 +328,4 @@ } ] } -} +} \ No newline at end of file diff --git a/it/java/gh/pom.xml b/it/java/gh/pom.xml new file mode 100644 index 0000000000..641d5bdc48 --- /dev/null +++ b/it/java/gh/pom.xml @@ -0,0 +1,74 @@ + + + 4.0.0 + io.kiota + kiota-gh-api + 0.1.0-SNAPSHOT + + + 11 + 11 + 11 + UTF-8 + UTF-8 + + 0.12.1 + + + + + com.microsoft.kiota + microsoft-kiota-abstractions + ${kiota-java.version} + + + com.microsoft.kiota + microsoft-kiota-serialization-json + ${kiota-java.version} + + + com.microsoft.kiota + microsoft-kiota-serialization-text + ${kiota-java.version} + + + com.microsoft.kiota + microsoft-kiota-serialization-form + ${kiota-java.version} + + + com.microsoft.kiota + microsoft-kiota-serialization-multipart + ${kiota-java.version} + + + com.microsoft.kiota + microsoft-kiota-http-okHttp + ${kiota-java.version} + + + jakarta.annotation + jakarta.annotation-api + 2.1.1 + + + org.junit.jupiter + junit-jupiter-engine + 5.9.2 + test + + + + + + + org.apache.maven.plugins + maven-surefire-plugin + 3.0.0-M9 + + + + \ No newline at end of file diff --git a/it/java/gh/src/test/java/GHAPITest.java b/it/java/gh/src/test/java/GHAPITest.java new file mode 100644 index 0000000000..12135b3dd8 --- /dev/null +++ b/it/java/gh/src/test/java/GHAPITest.java @@ -0,0 +1,23 @@ +import apisdk.ApiClient; +import com.microsoft.kiota.ApiException; +import com.microsoft.kiota.authentication.AnonymousAuthenticationProvider; +import com.microsoft.kiota.http.OkHttpRequestAdapter; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import java.util.concurrent.TimeUnit; + +public class BasicAPITest { + + @Test + void basicTest() throws Exception { + var adapter = new OkHttpRequestAdapter(new AnonymousAuthenticationProvider()); + adapter.setBaseUrl("http://127.0.0.1:1080"); + var client = new ApiClient(adapter); + + client.repos().byOwnerId("my-owner").byReposId("my-repo").get(); + client.repos().byTemplateOwner("my-template-owner").byTemplateRepo("my-repo").post(null); + } + +} diff --git a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs index 396eb1fdf5..e0a96d9ff4 100644 --- a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs @@ -921,6 +921,75 @@ public void SinglePathParametersAreDeduplicatedAndOrderIsRespected() Assert.Equal("\\repos\\{owner-id}\\{repo-id}\\generate", resultNode.Path); Assert.Equal("{+baseurl}/repos/{owner%2Did}/{repo%2Did}/generate", resultNode.GetUrlTemplate()); } + [Fact] + public void repro4085() + { + var document = new OpenApiDocument + { + Paths = new OpenApiPaths + { + ["/path/{thingId}/abc/{second}"] = new OpenApiPathItem + { + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Responses = new OpenApiResponses { + ["200"] = new OpenApiResponse + { + Content = { + ["application/json"] = new OpenApiMediaType + { + Schema = new OpenApiSchema { + Type = "string" + } + } + } + } + } + } + } + }, + ["/path/{differentThingId}/def/{second}"] = new OpenApiPathItem + { + Operations = { + [OperationType.Get] = new OpenApiOperation + { + Responses = new OpenApiResponses { + ["200"] = new OpenApiResponse + { + Content = { + ["application/json"] = new OpenApiMediaType + { + Schema = new OpenApiSchema { + Type = "string" + } + } + } + } + } + } + } + } + } + }; + var mockLogger = new CountLogger(); + var builder = new KiotaBuilder(mockLogger, new GenerationConfiguration { ClientClassName = "GitHub", ApiRootUrl = "https://localhost" }, _httpClient); + var node = builder.CreateUriSpace(document); + node.MergeIndexNodesAtSameLevel(mockLogger); + + // Expected + var resultNode = GetChildNodeByPath(node, "path"); + Assert.NotNull(resultNode); + Assert.Equal("\\path", resultNode.Path); + + var thingId = GetChildNodeByPath(resultNode, "{thingId}"); + Assert.Equal("\\path\\{thingId}", thingId.Path); + Assert.Equal("{+baseurl}/path/{thingId}", thingId.GetUrlTemplate()); + + var differentThingId = GetChildNodeByPath(resultNode, "{differentThingId}"); + Assert.Equal("\\path\\{differentThingId}", differentThingId.Path); + Assert.Equal("{+baseurl}/path/{differentThingId}", differentThingId.GetUrlTemplate()); + } private static OpenApiUrlTreeNode GetChildNodeByPath(OpenApiUrlTreeNode node, string path) { var pathSegments = path.Split('/'); From d027b51f5357703011157657b0b7fa02c0d70d8e Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 21 Feb 2024 14:15:59 -0500 Subject: [PATCH 6/7] - updates unit test to match implementation Signed-off-by: Vincent Biret --- .../Extensions/OpenApiUrlTreeNodeExtensionsTests.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs index e0a96d9ff4..0a7030661a 100644 --- a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs @@ -982,13 +982,12 @@ public void repro4085() Assert.NotNull(resultNode); Assert.Equal("\\path", resultNode.Path); - var thingId = GetChildNodeByPath(resultNode, "{thingId}"); - Assert.Equal("\\path\\{thingId}", thingId.Path); - Assert.Equal("{+baseurl}/path/{thingId}", thingId.GetUrlTemplate()); + Assert.Null(GetChildNodeByPath(resultNode, "{thingId}")); + Assert.Null(GetChildNodeByPath(resultNode, "{differentThingId}")); - var differentThingId = GetChildNodeByPath(resultNode, "{differentThingId}"); - Assert.Equal("\\path\\{differentThingId}", differentThingId.Path); - Assert.Equal("{+baseurl}/path/{differentThingId}", differentThingId.GetUrlTemplate()); + var differentThingId = GetChildNodeByPath(resultNode, "{differentThingId-id}"); + Assert.Equal("\\path\\{differentThingId-id}", differentThingId.Path); + Assert.Equal("{+baseurl}/path/{differentThingId%2Did}", differentThingId.GetUrlTemplate()); } private static OpenApiUrlTreeNode GetChildNodeByPath(OpenApiUrlTreeNode node, string path) { From 34a698db67242646d9685057fcc0b08d98d17f98 Mon Sep 17 00:00:00 2001 From: Vincent Biret Date: Wed, 21 Feb 2024 14:44:23 -0500 Subject: [PATCH 7/7] - fixes fluent API test for GH --- it/java/gh/src/test/java/GHAPITest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/it/java/gh/src/test/java/GHAPITest.java b/it/java/gh/src/test/java/GHAPITest.java index 12135b3dd8..cc9f50ec48 100644 --- a/it/java/gh/src/test/java/GHAPITest.java +++ b/it/java/gh/src/test/java/GHAPITest.java @@ -16,8 +16,8 @@ void basicTest() throws Exception { adapter.setBaseUrl("http://127.0.0.1:1080"); var client = new ApiClient(adapter); - client.repos().byOwnerId("my-owner").byReposId("my-repo").get(); - client.repos().byTemplateOwner("my-template-owner").byTemplateRepo("my-repo").post(null); + client.repos().byOrgId("my-owner").byRepoId("my-repo").get(); + client.repos().byOrgId("my-owner").byRepoId("my-repo").generate().post(null); } }