Merge pull request #4459 from microsoft/andrueastman/testFix

Fixes template re-use.

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

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<OpenApiParameter>(),
             };
             var parameters = pathItem.Parameters

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

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, OpenApiOperation> {
+                { 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