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/Extensions/OpenApiUrlTreeNodeExtensions.cs b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs index ea6116804f..ba925d728b 100644 --- a/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs +++ b/src/Kiota.Builder/Extensions/OpenApiUrlTreeNodeExtensions.cs @@ -182,6 +182,18 @@ 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); + 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); + } + public static string GetUrlTemplate(this OpenApiUrlTreeNode currentNode, OperationType? operationType = null, bool includeQueryParameters = true, bool includeBaseUrl = true) { ArgumentNullException.ThrowIfNull(currentNode); @@ -192,7 +204,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..1f8499d3d6 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())// 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..dba74a113f 100644 --- a/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs +++ b/tests/Kiota.Builder.Tests/Extensions/OpenApiUrlTreeNodeExtensionsTests.cs @@ -232,12 +232,95 @@ 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)); // 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.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)); + 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