Skip to content

Commit 1e977cc

Browse files
[http-client-csharp] Fix trailing path separator for optional path parameter (#11096)
## Problem When a route ends with an **optional** path parameter, the `/` separator preceding that segment was emitted unconditionally, leaving a dangling trailing slash when the value is null. For example, `/certificates/{certificate-name}/{certificate-version}` (with `certificate-version` optional) generated: ```csharp uri.AppendPath(certificateName, true); uri.AppendPath("/", false); // emitted unconditionally if ((certificateVersion != null)) { uri.AppendPath(certificateVersion, true); } ``` So when `certificateVersion` is null the wire URL becomes `/certificates/{name}/` instead of `/certificates/{name}`. Services (e.g. Key Vault) and recorded test cassettes expect no trailing slash. The existing `shouldPrependWithPathSeparator` logic only handled the case where the preceding literal does *not* end in `/`; it missed the case where the separator had already been written unconditionally. ## Fix In `RestClientProvider.AddUriSegments`, when the upcoming path parameter is optional and the preceding literal ends with `/`, defer that trailing `/` so it is written together with the parameter value inside the null check: ```csharp uri.AppendPath(certificateName, true); if ((certificateVersion != null)) { uri.AppendPath("/", false); uri.AppendPath(certificateVersion, true); } ``` Required path parameters are unaffected. ## Tests - Added `TestBuildCreateRequestMethodWithOptionalPathParameter` (+ golden file) covering the trailing optional-segment case. - Updated two `ServerTemplate*` tests that relied on the test factory's default `isRequired: false` for what were intended to be normal required path parameters. - Full `Microsoft.TypeSpec.Generator.ClientModel` suite passes (1440 tests). Reported in Azure/azure-sdk-for-net#60162 (Bug A). --generated by Copilot --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 924435b commit 1e977cc

4 files changed

Lines changed: 78 additions & 5 deletions

File tree

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -717,10 +717,27 @@ private void AddUriSegments(
717717
}
718718

719719
var path = pathSpan.Slice(0, paramIndex);
720-
AppendLiteralSegment(uri, path.ToString(), statements);
721720
pathSpan = pathSpan.Slice(paramIndex + 1);
722721
var paramEndIndex = pathSpan.IndexOf('}');
723722
var paramName = pathSpan.Slice(0, paramEndIndex).ToString();
723+
724+
/* An optional path parameter that is null must not leave a dangling
725+
* path separator behind. For example "/foo/{bar}/{baz}" with an absent
726+
* optional "baz" should produce "/foo/{bar}", not "/foo/{bar}/". When the
727+
* upcoming parameter is optional, defer the trailing '/' of the preceding
728+
* literal so it is only written together with the parameter value inside
729+
* the null check below.
730+
*/
731+
var pathLiteral = path.ToString();
732+
bool separatorDeferred = false;
733+
if (pathLiteral.EndsWith('/')
734+
&& inputParamMap.TryGetValue(paramName, out var optionalCheckParam)
735+
&& optionalCheckParam is InputPathParameter { IsRequired: false })
736+
{
737+
pathLiteral = pathLiteral.Substring(0, pathLiteral.Length - 1);
738+
separatorDeferred = true;
739+
}
740+
AppendLiteralSegment(uri, pathLiteral, statements);
724741
/* when the parameter is in operation.uri, it is client parameter
725742
* It is not operation parameter and not in inputParamHash list.
726743
*/
@@ -766,7 +783,7 @@ private void AddUriSegments(
766783
MethodBodyStatement statement;
767784
if (inputParam?.IsRequired == false)
768785
{
769-
bool shouldPrependWithPathSeparator = path.Length > 0 && path[^1] != '/';
786+
bool shouldPrependWithPathSeparator = separatorDeferred || (path.Length > 0 && path[^1] != '/');
770787
List<MethodBodyStatement> appendPathStatements = shouldPrependWithPathSeparator
771788
? [uri.AppendPath(Literal("/"), false).Terminate(), uri.AppendPath(valueExpression, escape).Terminate()]
772789
: [uri.AppendPath(valueExpression, escape).Terminate()];

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3589,8 +3589,8 @@ public void ServerTemplateWithPathParameter_OnlyAppendsSegmentsAfterEndpoint()
35893589
MockHelpers.LoadMockGenerator();
35903590

35913591
var serverTemplate = "{endpoint}/{apiVersion}";
3592-
var apiVersionParam = InputFactory.PathParameter("apiVersion", InputPrimitiveType.String, serverUrlTemplate: serverTemplate);
3593-
var userIdParam = InputFactory.PathParameter("userId", InputPrimitiveType.String);
3592+
var apiVersionParam = InputFactory.PathParameter("apiVersion", InputPrimitiveType.String, isRequired: true, serverUrlTemplate: serverTemplate);
3593+
var userIdParam = InputFactory.PathParameter("userId", InputPrimitiveType.String, isRequired: true);
35943594

35953595
var operation = InputFactory.Operation(
35963596
name: "GetUser",
@@ -3633,7 +3633,7 @@ public void ServerTemplateWithMultipleSegments_HandlesCorrectly()
36333633
MockHelpers.LoadMockGenerator();
36343634

36353635
var serverTemplate = "{endpoint}/v1/services";
3636-
var operationIdParam = InputFactory.PathParameter("operationId", InputPrimitiveType.String);
3636+
var operationIdParam = InputFactory.PathParameter("operationId", InputPrimitiveType.String, isRequired: true);
36373637

36383638
var operation = InputFactory.Operation(
36393639
name: "GetOperation",

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/RestClientProviders/RestClientProviderTests.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -819,6 +819,34 @@ public void TestBuildCreateRequestMethodWithPathParameters()
819819
Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
820820
}
821821

822+
// An optional trailing path parameter must not emit a dangling separator when null.
823+
// e.g. "/certificates/{certificateName}/{certificateVersion}" with a null version
824+
// should produce "/certificates/{name}", not "/certificates/{name}/".
825+
[Test]
826+
public void TestBuildCreateRequestMethodWithOptionalPathParameter()
827+
{
828+
List<InputParameter> parameters =
829+
[
830+
InputFactory.PathParameter("certificateName", InputPrimitiveType.String, isRequired: true),
831+
InputFactory.PathParameter("certificateVersion", InputPrimitiveType.String, isRequired: false),
832+
];
833+
var operation = InputFactory.Operation(
834+
"getCertificate",
835+
parameters: parameters,
836+
uri: "/certificates/{certificateName}/{certificateVersion}");
837+
838+
var client = InputFactory.Client(
839+
"TestClient",
840+
methods: [InputFactory.BasicServiceMethod("Test", operation)]);
841+
842+
var clientProvider = new ClientProvider(client);
843+
var restClientProvider = new MockClientProvider(client, clientProvider);
844+
845+
var writer = new TypeProviderWriter(restClientProvider);
846+
var file = writer.Write();
847+
Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
848+
}
849+
822850
[Test]
823851
public void TestBuildCreateRequestMethodWithQueryInPath()
824852
{
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// <auto-generated/>
2+
3+
#nullable disable
4+
5+
using System.ClientModel.Primitives;
6+
7+
namespace Sample
8+
{
9+
public partial class TestClient
10+
{
11+
internal global::System.ClientModel.Primitives.PipelineMessage CreateGetCertificateRequest(string certificateName, string certificateVersion, global::System.ClientModel.Primitives.RequestOptions options)
12+
{
13+
global::Sample.ClientUriBuilder uri = new global::Sample.ClientUriBuilder();
14+
uri.Reset(_endpoint);
15+
uri.AppendPath("/certificates/", false);
16+
uri.AppendPath(certificateName, true);
17+
if ((certificateVersion != null))
18+
{
19+
uri.AppendPath("/", false);
20+
uri.AppendPath(certificateVersion, true);
21+
}
22+
global::System.ClientModel.Primitives.PipelineMessage message = Pipeline.CreateMessage(uri.ToUri(), "GET", PipelineMessageClassifier200);
23+
global::System.ClientModel.Primitives.PipelineRequest request = message.Request;
24+
message.Apply(options);
25+
return message;
26+
}
27+
}
28+
}

0 commit comments

Comments
 (0)