From 96da3af028a6193174f0a76276e8a2a8dbdbb4a0 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Mon, 8 Apr 2024 12:42:43 +0300 Subject: [PATCH 1/3] Test generation --- .../Extensions/OpenApiUrlTreeNodeExtensions.cs | 12 +++++++++++- src/Kiota.Builder/KiotaBuilder.cs | 3 ++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs index ea6116804f..046dd3dfc4 100644 --- a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs @@ -182,6 +182,16 @@ private static bool IsPathSegmentWithNumberOfParameters(this string currentSegme private static partial Regex stripExtensionForIndexersTestRegex(); // so {param-name}.json is considered as indexer public static bool IsComplexPathMultipleParameters(this OpenApiUrlTreeNode currentNode) => (currentNode?.DeduplicatedSegment()?.IsPathSegmentWithNumberOfParameters(static x => x.Any()) ?? false) && !currentNode.IsPathSegmentWithSingleSimpleParameter(); + + public static bool HasRequiredQueryParametersAcrossOperations(this OpenApiUrlTreeNode currentNode) + { + ArgumentNullException.ThrowIfNull(currentNode); + var pathItem = currentNode.PathItems[Constants.DefaultOpenApiLabel]; + var operationQueryParameters = pathItem.Operations.SelectMany(static x => x.Value.Parameters); + return operationQueryParameters.Union(pathItem.Parameters).Where(static x => x.In == ParameterLocation.Query) + .Any(static x => x.Required); + } + public static string GetUrlTemplate(this OpenApiUrlTreeNode currentNode, OperationType? operationType = null, bool includeQueryParameters = true, bool includeBaseUrl = true) { ArgumentNullException.ThrowIfNull(currentNode); @@ -192,7 +202,7 @@ public static string GetUrlTemplate(this OpenApiUrlTreeNode currentNode, Operati var operationQueryParameters = (operationType, pathItem.Operations.Any()) switch { (OperationType ot, _) when pathItem.Operations.TryGetValue(ot, out var operation) => operation.Parameters, - (null, true) => pathItem.Operations.OrderBy(static x => x.Key).FirstOrDefault().Value.Parameters, + (null, true) => pathItem.Operations.SelectMany(static x => x.Value.Parameters).Where(static x => x.In == ParameterLocation.Query), _ => Enumerable.Empty(), }; var parameters = pathItem.Parameters diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index 097d68ce33..ce8a3426ae 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -1289,7 +1289,8 @@ private void CreateOperationMethods(OpenApiUrlTreeNode currentNode, OperationTyp Deprecation = deprecationInformation, }; var operationUrlTemplate = currentNode.GetUrlTemplate(operationType); - if (!operationUrlTemplate.Equals(parentClass.Properties.FirstOrDefault(static x => x.Kind is CodePropertyKind.UrlTemplate)?.DefaultValue?.Trim('"'), StringComparison.Ordinal)) + if (!operationUrlTemplate.Equals(parentClass.Properties.FirstOrDefault(static x => x.Kind is CodePropertyKind.UrlTemplate)?.DefaultValue?.Trim('"'), StringComparison.Ordinal) + && currentNode.HasRequiredQueryParametersAcrossOperations()) generatorMethod.UrlTemplateOverride = operationUrlTemplate; var mediaTypes = schema switch From 039a84c5fffa2205dc515fe95c29f3404eeb2dd3 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Mon, 8 Apr 2024 13:46:11 +0300 Subject: [PATCH 2/3] Fixes --- CHANGELOG.md | 6 ++ src/Kiota.Builder/KiotaBuilder.cs | 2 +- .../OpenApiUrlTreeNodeExtensionsTests.cs | 81 +++++++++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 252974848e..184394a048 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +### Changed + +- Changed URI template generation to reuse templates when required templates are absent across operations. + ## [1.13.0] - 2024-04-04 ### Added diff --git a/src/Kiota.Builder/KiotaBuilder.cs b/src/Kiota.Builder/KiotaBuilder.cs index ce8a3426ae..1f8499d3d6 100644 --- a/src/Kiota.Builder/KiotaBuilder.cs +++ b/src/Kiota.Builder/KiotaBuilder.cs @@ -1290,7 +1290,7 @@ private void CreateOperationMethods(OpenApiUrlTreeNode currentNode, OperationTyp }; var operationUrlTemplate = currentNode.GetUrlTemplate(operationType); if (!operationUrlTemplate.Equals(parentClass.Properties.FirstOrDefault(static x => x.Kind is CodePropertyKind.UrlTemplate)?.DefaultValue?.Trim('"'), StringComparison.Ordinal) - && currentNode.HasRequiredQueryParametersAcrossOperations()) + && currentNode.HasRequiredQueryParametersAcrossOperations())// no need to generate extra strings/templates as optional parameters will have no effect on resolved url. generatorMethod.UrlTemplateOverride = operationUrlTemplate; var mediaTypes = schema switch diff --git a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs index 0a7030661a..7a0c41f151 100644 --- a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs @@ -232,12 +232,93 @@ public void DifferentUrlTemplatesPerOperation() } }); var node = OpenApiUrlTreeNode.Create(doc, Label); + Assert.False(node.HasRequiredQueryParametersAcrossOperations()); Assert.Equal("{+baseurl}/{param%2Dwith%2Ddashes}/existing-segment{?%24select}", node.Children.First().Value.GetUrlTemplate()); Assert.Equal("{+baseurl}/{param%2Dwith%2Ddashes}/existing-segment{?%24select}", node.Children.First().Value.GetUrlTemplate(OperationType.Get)); Assert.Equal("{+baseurl}/{param%2Dwith%2Ddashes}/existing-segment", node.Children.First().Value.GetUrlTemplate(OperationType.Put)); // the query parameters will be decoded by a middleware at runtime before the request is executed } [Fact] + public void DifferentUrlTemplatesPerOperationWithRequiredParameter() + { + var doc = new OpenApiDocument + { + Paths = [], + }; + doc.Paths.Add("{param-with-dashes}\\existing-segment", new() + { + Parameters = [ + new() + { + Name = "param-with-dashes", + In = ParameterLocation.Path, + Required = true, + Schema = new() + { + Type = "string" + }, + Style = ParameterStyle.Simple, + }, + ], + Operations = new Dictionary { + { OperationType.Get, new() { + Parameters = [ + + new (){ + Name = "$select", + In = ParameterLocation.Query, + Schema = new () { + Type = "string" + }, + Style = ParameterStyle.Simple, + } + ] + } + }, + { OperationType.Post, new() { + Parameters = [ + + new (){ + Name = "$expand", + In = ParameterLocation.Query, + Schema = new () { + Type = "string" + }, + Style = ParameterStyle.Simple, + } + ] + } + }, + { + OperationType.Put, new() {} + }, + { OperationType.Delete, new() { + Parameters = [ + + new (){ + Name = "id", + In = ParameterLocation.Query, + Schema = new () { + Type = "string" + }, + Style = ParameterStyle.Simple, + Required = true + } + ] + } + }, + } + }); + var node = OpenApiUrlTreeNode.Create(doc, Label); + Assert.True(node.HasRequiredQueryParametersAcrossOperations()); + Assert.Equal("{+baseurl}/{param%2Dwith%2Ddashes}/existing-segment?id={id}{&%24expand,%24select}", node.Children.First().Value.GetUrlTemplate());//the default contains a combination of everything. + Assert.Equal("{+baseurl}/{param%2Dwith%2Ddashes}/existing-segment{?%24select}", node.Children.First().Value.GetUrlTemplate(OperationType.Get)); + Assert.Equal("{+baseurl}/{param%2Dwith%2Ddashes}/existing-segment{?%24expand}", node.Children.First().Value.GetUrlTemplate(OperationType.Post)); + Assert.Equal("{+baseurl}/{param%2Dwith%2Ddashes}/existing-segment", node.Children.First().Value.GetUrlTemplate(OperationType.Put)); + Assert.Equal("{+baseurl}/{param%2Dwith%2Ddashes}/existing-segment?id={id}", node.Children.First().Value.GetUrlTemplate(OperationType.Delete)); + // the query parameters will be decoded by a middleware at runtime before the request is executed + } + [Fact] public void GeneratesRequiredQueryParametersAndOptionalMixInPathItem() { var doc = new OpenApiDocument From 3ce38134f0db89da200f7ea8b12042b0c0005a04 Mon Sep 17 00:00:00 2001 From: Andrew Omondi Date: Mon, 8 Apr 2024 14:11:09 +0300 Subject: [PATCH 3/3] Fix tests --- src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs | 4 +++- .../Extensions/OpenApiUrlTreeNodeExtensionsTests.cs | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs index 046dd3dfc4..ba925d728b 100644 --- a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs @@ -186,7 +186,9 @@ public static bool IsComplexPathMultipleParameters(this OpenApiUrlTreeNode curre public static bool HasRequiredQueryParametersAcrossOperations(this OpenApiUrlTreeNode currentNode) { ArgumentNullException.ThrowIfNull(currentNode); - var pathItem = currentNode.PathItems[Constants.DefaultOpenApiLabel]; + if (!currentNode.PathItems.TryGetValue(Constants.DefaultOpenApiLabel, out var pathItem)) + return false; + var operationQueryParameters = pathItem.Operations.SelectMany(static x => x.Value.Parameters); return operationQueryParameters.Union(pathItem.Parameters).Where(static x => x.In == ParameterLocation.Query) .Any(static x => x.Required); diff --git a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs index 7a0c41f151..dba74a113f 100644 --- a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs @@ -233,6 +233,7 @@ public void DifferentUrlTemplatesPerOperation() }); var node = OpenApiUrlTreeNode.Create(doc, Label); Assert.False(node.HasRequiredQueryParametersAcrossOperations()); + Assert.False(node.Children.First().Value.HasRequiredQueryParametersAcrossOperations()); Assert.Equal("{+baseurl}/{param%2Dwith%2Ddashes}/existing-segment{?%24select}", node.Children.First().Value.GetUrlTemplate()); Assert.Equal("{+baseurl}/{param%2Dwith%2Ddashes}/existing-segment{?%24select}", node.Children.First().Value.GetUrlTemplate(OperationType.Get)); Assert.Equal("{+baseurl}/{param%2Dwith%2Ddashes}/existing-segment", node.Children.First().Value.GetUrlTemplate(OperationType.Put)); @@ -310,7 +311,8 @@ public void DifferentUrlTemplatesPerOperationWithRequiredParameter() } }); var node = OpenApiUrlTreeNode.Create(doc, Label); - Assert.True(node.HasRequiredQueryParametersAcrossOperations()); + Assert.False(node.HasRequiredQueryParametersAcrossOperations()); + Assert.True(node.Children.First().Value.HasRequiredQueryParametersAcrossOperations()); Assert.Equal("{+baseurl}/{param%2Dwith%2Ddashes}/existing-segment?id={id}{&%24expand,%24select}", node.Children.First().Value.GetUrlTemplate());//the default contains a combination of everything. Assert.Equal("{+baseurl}/{param%2Dwith%2Ddashes}/existing-segment{?%24select}", node.Children.First().Value.GetUrlTemplate(OperationType.Get)); Assert.Equal("{+baseurl}/{param%2Dwith%2Ddashes}/existing-segment{?%24expand}", node.Children.First().Value.GetUrlTemplate(OperationType.Post));