Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: fix
packages:
- "@typespec/http-client-csharp"
---

Fix protocol method call site passing arguments to the `CreateRequest` method in the wrong order. When an optional parameter (such as an optional path parameter) appeared before a required body parameter, the protocol method reordered its parameters required-first, which differs from the `CreateRequest` method's declaration order. The generated call site now maps each argument to the matching `CreateRequest` parameter by name so the values are passed in the order the request builder expects.
Original file line number Diff line number Diff line change
Expand Up @@ -1117,7 +1117,7 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod
[
UsingDeclare("message", ScmCodeModelGenerator.Instance.TypeFactory.HttpMessageApi.HttpMessageType,
This.Invoke(createRequestMethod.Signature,
[.. bodyParameters.Select(p => (ValueExpression)p)]), out var message),
BuildCreateRequestArguments(createRequestMethod.Signature, bodyParameters)), out var message),
Return(ScmCodeModelGenerator.Instance.TypeFactory.ClientResponseApi.ToExpression().FromResponse(client
.PipelineProperty.Invoke(processMessageName, [message, requestOptionsParameter], isAsync, true, extensionType: _clientPipelineExtensionsDefinition.Type)))
];
Expand All @@ -1143,6 +1143,44 @@ [.. bodyParameters.Select(p => (ValueExpression)p)]), out var message),
return protocolMethod;
}

// The protocol method orders its parameters required-first (optional parameters and the
// request options/context parameter are moved to the end so they can have default values).
// This order can differ from the CreateRequest method's parameter order, which follows the
// operation/declaration order. Passing the protocol parameters positionally would therefore
// bind them to the wrong CreateRequest parameters (for example, swapping an optional path
// parameter with the request body). Reorder the arguments to match the CreateRequest
// signature by mapping each CreateRequest parameter to the protocol parameter with the same
// name. If the names cannot be reconciled, fall back to the original positional behavior.
private static ValueExpression[] BuildCreateRequestArguments(
MethodSignature createRequestSignature,
IReadOnlyList<ParameterProvider> bodyParameters)
{
var createRequestParameters = createRequestSignature.Parameters;
if (createRequestParameters.Count == bodyParameters.Count)
{
var arguments = new ValueExpression[createRequestParameters.Count];
bool allMatched = true;
for (int i = 0; i < createRequestParameters.Count; i++)
{
var match = bodyParameters.FirstOrDefault(p => p.Name == createRequestParameters[i].Name);
if (match is null)
{
allMatched = false;
break;
}

arguments[i] = match;
}

if (allMatched)
{
return arguments;
}
}

return [.. bodyParameters.Select(p => (ValueExpression)p)];
}

private ParameterProvider ProcessOptionalParameters(
List<ParameterProvider> optionalParameters,
List<ParameterProvider> requiredParameters,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public partial class MultipartClient
{
public virtual global::System.ClientModel.ClientResult Upload(string contentType, global::System.ClientModel.BinaryContent content, global::System.ClientModel.Primitives.RequestOptions options = null)
{
using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateUploadRequest(contentType, content, options);
using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateUploadRequest(content, contentType, options);
return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, options));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,76 @@ public void ProtocolMethodWithOptionalBodyParameter()
}
}

// Regression test for https://github.com/Azure/azure-sdk-for-net/issues/60160.
// When an optional parameter appears before a required body parameter in the operation
// (e.g. an optional path version followed by the required body), the protocol method
// reorders its parameters required-first, which differs from the CreateRequest method's
// declaration order. The call site must still pass the arguments in the CreateRequest
// method's parameter order, not the protocol method's order.
[Test]
public void ProtocolMethodCallsCreateRequestWithArgumentsInBuilderOrder()
{
MockHelpers.LoadMockGenerator();
List<InputParameter> parameters =
[
InputFactory.PathParameter(
"name",
InputPrimitiveType.String,
isRequired: true),
InputFactory.PathParameter(
"version",
InputPrimitiveType.String,
isRequired: false),
InputFactory.BodyParameter(
"body",
InputPrimitiveType.String,
isRequired: true),
];
List<InputMethodParameter> methodParameters =
[
InputFactory.MethodParameter(
"name",
InputPrimitiveType.String,
isRequired: true,
location: InputRequestLocation.Path),
InputFactory.MethodParameter(
"version",
InputPrimitiveType.String,
isRequired: false,
location: InputRequestLocation.Path),
InputFactory.MethodParameter(
"body",
InputPrimitiveType.String,
isRequired: true,
location: InputRequestLocation.Body),
];
var inputOperation = InputFactory.Operation(
"TestOperation",
parameters: parameters);
var inputServiceMethod = InputFactory.BasicServiceMethod("Test", inputOperation, parameters: methodParameters);
var inputClient = InputFactory.Client("TestClient", methods: [inputServiceMethod]);
var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient);
var methodCollection = new ScmMethodProviderCollection(inputServiceMethod, client!);

var protocolMethod = methodCollection.Single(m =>
m.Signature.Parameters.Any(p => p.Name == "options")
&& m.Signature.Name == "TestOperation");

// The protocol method orders parameters required-first: (name, content, version, options).
var protocolParameterOrder = string.Join(", ", protocolMethod.Signature.Parameters.Select(p => p.Name));
Assert.AreEqual("name, content, version, options", protocolParameterOrder);

// The CreateRequest builder keeps declaration order: (name, version, content, options).
var createRequestMethod = client!.RestClient.GetCreateRequestMethod(inputOperation);
var expectedArguments = string.Join(", ", createRequestMethod.Signature.Parameters.Select(p => p.Name));
Assert.AreEqual("name, version, content, options", expectedArguments);

// The call site must use the CreateRequest builder's order, not the protocol method's order.
var body = protocolMethod.BodyStatements!.ToDisplayString();
StringAssert.Contains($"{createRequestMethod.Signature.Name}({expectedArguments})", body);
StringAssert.DoesNotContain($"{createRequestMethod.Signature.Name}(name, content, version, options)", body);
}

[Test]
public void OperationWithOptionalEnum()
{
Expand Down
Loading