Skip to content

Commit

Permalink
[http-client-csharp] Make RequestOptions parameter optional in protoc…
Browse files Browse the repository at this point in the history 
…ol methods (#5018)

fixes: #4990

contributes to #4903
  • Loading branch information
@jorgerangel-msft
jorgerangel-msft authored Nov 7, 2024
1 parent 050139d commit 910ca35
Show file tree
Hide file tree
Showing 164 changed files with 618 additions and 478 deletions.
5 changes: 5 additions & 0 deletions ...arp/generator/Microsoft.Generator.CSharp.ClientModel/src/Primitives/ScmKnownParameters.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ public static ParameterProvider ClientOptions(CSharpType clientOptionsType)
=> new("options", $"The options for configuring the client.", clientOptionsType.WithNullable(true), initializationValue: New.Instance(clientOptionsType.WithNullable(true)));
public static readonly ParameterProvider KeyAuth = new("keyCredential", $"The token credential to copy", ClientModelPlugin.Instance.TypeFactory.KeyCredentialType);
public static readonly ParameterProvider MatchConditionsParameter = new("matchConditions", $"The content to send as the request conditions of the request.", ClientModelPlugin.Instance.TypeFactory.MatchConditionsType, DefaultOf(ClientModelPlugin.Instance.TypeFactory.MatchConditionsType));
public static readonly ParameterProvider OptionalRequestOptions = new(
ClientModelPlugin.Instance.TypeFactory.HttpRequestOptionsApi.ParameterName,
$"The request options, which can override default behaviors of the client pipeline on a per-call basis.",
ClientModelPlugin.Instance.TypeFactory.HttpRequestOptionsApi.HttpRequestOptionsType.WithNullable(true),
defaultValue: Null);
public static readonly ParameterProvider RequestOptions = new(ClientModelPlugin.Instance.TypeFactory.HttpRequestOptionsApi.ParameterName, $"The request options, which can override default behaviors of the client pipeline on a per-call basis.", ClientModelPlugin.Instance.TypeFactory.HttpRequestOptionsApi.HttpRequestOptionsType);
public static readonly ParameterProvider RequestContent = new("content", $"The content to send as the body of the request.", ClientModelPlugin.Instance.TypeFactory.RequestContentApi.RequestContentType, location: ParameterLocation.Body) { Validation = ParameterValidationType.AssertNotNull };

Expand Down
64 changes: 55 additions & 9 deletions ...rator/Microsoft.Generator.CSharp.ClientModel/src/Providers/ScmMethodProviderCollection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ protected override IReadOnlyList<MethodProvider> BuildMethods()
];
}

private MethodProvider BuildConvenienceMethod(MethodProvider protocolMethod, bool isAsync)
private ScmMethodProvider BuildConvenienceMethod(MethodProvider protocolMethod, bool isAsync)
{
if (EnclosingType is not ClientProvider client)
if (EnclosingType is not ClientProvider)
{
throw new InvalidOperationException("Protocol methods can only be built for client types.");
}
Expand Down Expand Up @@ -370,10 +370,9 @@ private IReadOnlyList<ValueExpression> GetParamConversions(IReadOnlyList<Paramet
private IReadOnlyList<ParameterProvider>? _convenienceMethodParameters;
private IReadOnlyList<ParameterProvider> ConvenienceMethodParameters => _convenienceMethodParameters ??= RestClientProvider.GetMethodParameters(Operation);

private MethodProvider BuildProtocolMethod(MethodProvider createRequestMethod, bool isAsync)
private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod, bool isAsync)
{
ClientProvider? client = EnclosingType as ClientProvider;
if (client is null)
if (EnclosingType is not ClientProvider client)
{
throw new InvalidOperationException("Protocol methods can only be built for client types.");
}
Expand All @@ -383,19 +382,40 @@ private MethodProvider BuildProtocolMethod(MethodProvider createRequestMethod, b
{
methodModifier |= MethodSignatureModifiers.Async;
}

ParameterProvider requestOptionsParameter = ScmKnownParameters.RequestOptions;
bool addOptionalRequestOptionsParameter = ShouldAddOptionalRequestOptionsParameter();

// construct the protocol method parameters from the create request method parameters
ParameterProvider[] methodParameters = new ParameterProvider[MethodParameters.Count];

for (var i = 0; i < MethodParameters.Count; i++)
{
var parameter = MethodParameters[i];
if (parameter == ScmKnownParameters.RequestOptions && addOptionalRequestOptionsParameter)
{
requestOptionsParameter = ScmKnownParameters.OptionalRequestOptions;
methodParameters[i] = requestOptionsParameter;
}
else
{
methodParameters[i] = parameter;
}
}

var methodSignature = new MethodSignature(
isAsync ? _cleanOperationName + "Async" : _cleanOperationName,
FormattableStringHelpers.FromString(Operation.Description),
methodModifier,
GetResponseType(Operation.Responses, false, isAsync, out var responseBodyType),
GetResponseType(Operation.Responses, false, isAsync, out _),
$"The response returned from the service.",
MethodParameters);
methodParameters);
var processMessageName = isAsync ? "ProcessMessageAsync" : "ProcessMessage";

MethodBodyStatement[] methodBody =
[
UsingDeclare("message", ClientModelPlugin.Instance.TypeFactory.HttpMessageApi.HttpMessageType, This.Invoke(createRequestMethod.Signature, [.. MethodParameters]), out var message),
Return(ClientModelPlugin.Instance.TypeFactory.ClientResponseApi.ToExpression().FromResponse(client.PipelineProperty.Invoke(processMessageName, [message, ScmKnownParameters.RequestOptions], isAsync, true))),
UsingDeclare("message", ClientModelPlugin.Instance.TypeFactory.HttpMessageApi.HttpMessageType, This.Invoke(createRequestMethod.Signature, [.. methodParameters]), out var message),
Return(ClientModelPlugin.Instance.TypeFactory.ClientResponseApi.ToExpression().FromResponse(client.PipelineProperty.Invoke(processMessageName, [message, requestOptionsParameter], isAsync, true))),
];

var protocolMethod =
Expand Down Expand Up @@ -431,5 +451,31 @@ private static CSharpType GetConvenienceReturnType(IReadOnlyList<OperationRespon
? ClientModelPlugin.Instance.TypeFactory.ClientResponseApi.ClientResponseType
: new CSharpType(ClientModelPlugin.Instance.TypeFactory.ClientResponseApi.ClientResponseOfTType.FrameworkType, responseBodyType);
}

private bool ShouldAddOptionalRequestOptionsParameter()
{
var convenienceMethodParameterCount = ConvenienceMethodParameters.Count;
if (convenienceMethodParameterCount == 0)
{
return false;
}

// the request options parameter is optional if the methods have different parameters.
// We can exclude the request options parameter from the protocol method count.
if ((MethodParameters.Count - 1) != convenienceMethodParameterCount)
{
return true;
}

for (int i = 0; i < convenienceMethodParameterCount; i++)
{
if (!MethodParameters[i].Type.Equals(ConvenienceMethodParameters[i].Type))
{
return true;
}
}

return false;
}
}
}
95 changes: 92 additions & 3 deletions ...rosoft.Generator.CSharp.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ public void TestBuildConstructors_ForSubClient()
Assert.IsNotNull(mockingConstructor);
}

private void ValidatePrimaryConstructor(
private static void ValidatePrimaryConstructor(
ConstructorProvider? primaryPublicConstructor,
List<InputParameter> inputParameters)
{
Expand Down Expand Up @@ -415,9 +415,9 @@ public void ValidateClientWithSpread(InputClient inputClient)
Assert.AreEqual(2, protocolMethods[1].Signature.Parameters.Count);

Assert.AreEqual(new CSharpType(typeof(BinaryContent)), protocolMethods[0].Signature.Parameters[0].Type);
Assert.AreEqual(new CSharpType(typeof(RequestOptions)), protocolMethods[0].Signature.Parameters[1].Type);
Assert.AreEqual(new CSharpType(typeof(RequestOptions), true), protocolMethods[0].Signature.Parameters[1].Type);
Assert.AreEqual(new CSharpType(typeof(BinaryContent)), protocolMethods[1].Signature.Parameters[0].Type);
Assert.AreEqual(new CSharpType(typeof(RequestOptions)), protocolMethods[1].Signature.Parameters[1].Type);
Assert.AreEqual(new CSharpType(typeof(RequestOptions), true), protocolMethods[1].Signature.Parameters[1].Type);

var convenienceMethods = methods.Where(m => m.Signature.Parameters.Any(p => p.Type.Equals(typeof(string)))).ToList();
Assert.AreEqual(2, convenienceMethods.Count);
Expand All @@ -428,6 +428,33 @@ public void ValidateClientWithSpread(InputClient inputClient)

}

[TestCaseSource(nameof(RequestOptionsParameterInSignatureTestCases))]
public void TestRequestOptionsParameterInSignature(InputOperation inputOperation, bool shouldBeOptional)
{
var client = InputFactory.Client(TestClientName, operations: [inputOperation]);
var clientProvider = new ClientProvider(client);
var protocolMethods = clientProvider.Methods.Where(m => m.Signature.Parameters.Any(p => p.Type.Name == "RequestOptions")).ToList();
var syncMethod = protocolMethods.FirstOrDefault(m => !m.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Async));
Assert.IsNotNull(syncMethod);

var requestOptionsParameterInSyncMethod = syncMethod!.Signature.Parameters.FirstOrDefault(p => p.Type.Name == "RequestOptions");
Assert.IsNotNull(requestOptionsParameterInSyncMethod);
Assert.AreEqual(shouldBeOptional, requestOptionsParameterInSyncMethod!.Type.IsNullable);

var asyncMethod = protocolMethods.FirstOrDefault(m => m.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Async));
Assert.IsNotNull(asyncMethod);

var requestOptionsParameterInAsyncMethod = asyncMethod!.Signature.Parameters.FirstOrDefault(p => p.Type.Name == "RequestOptions");
Assert.IsNotNull(requestOptionsParameterInAsyncMethod);
Assert.AreEqual(shouldBeOptional, requestOptionsParameterInAsyncMethod!.Type.IsNullable);

if (shouldBeOptional)
{
Assert.IsNotNull(requestOptionsParameterInSyncMethod!.DefaultValue);
Assert.IsNotNull(requestOptionsParameterInAsyncMethod!.DefaultValue);
}
}

[Test]
public void TestApiVersionOfClient()
{
Expand Down Expand Up @@ -673,6 +700,68 @@ public static IEnumerable<TestCaseData> BuildConstructorsTestCases
}
}

public static IEnumerable<TestCaseData> RequestOptionsParameterInSignatureTestCases
{
get
{
// Protocol & convenience methods will have the same parameters, so RequestOptions should be required.
yield return new TestCaseData(
InputFactory.Operation(
"TestOperation",
parameters:
[
InputFactory.Parameter(
"p1",
InputPrimitiveType.String,
location: RequestLocation.None,
isRequired: true),
InputFactory.Parameter(
"p2",
InputPrimitiveType.Int64,
location: RequestLocation.None,
isRequired: true),
]), false);

// convenience method only has a body param, so RequestOptions should be optional in protocol method.
yield return new TestCaseData(
InputFactory.Operation(
"TestOperation",
parameters:
[
InputFactory.Parameter(
"p1",
InputPrimitiveType.String,
location: RequestLocation.Body),
]), true);

// Protocol & convenience methods will have different parameters since there is a model body param, so RequestOptions should be optional.
yield return new TestCaseData(
InputFactory.Operation(
"TestOperation",
parameters:
[
InputFactory.Parameter(
"p1",
InputPrimitiveType.String,
location: RequestLocation.None,
isRequired: true),
InputFactory.Parameter(
"p2",
InputFactory.Model("SampleModel"),
location: RequestLocation.Body,
isRequired: true),
]), true);

// Convenience method has no parameters, RequestOptions should be required in protocol method.
yield return new TestCaseData(
InputFactory.Operation(
"TestOperation",
responses: [InputFactory.OperationResponse([201], InputFactory.Model("testModel"))],
parameters: []),
false);
}
}

private static IEnumerable<TestCaseData> EndpointParamInitializationValueTestCases()
{
// string primitive type
Expand Down
8 changes: 4 additions & 4 deletions ...t-csharp/generator/TestProjects/CadlRanch/http/client/naming/src/Generated/ClientModel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@ public partial class ClientModel

public ClientPipeline Pipeline => throw null;

public virtual ClientResult Client(BinaryContent content, RequestOptions options) => throw null;
public virtual ClientResult Client(BinaryContent content, RequestOptions options = null) => throw null;

public virtual Task<ClientResult> ClientAsync(BinaryContent content, RequestOptions options) => throw null;
public virtual Task<ClientResult> ClientAsync(BinaryContent content, RequestOptions options = null) => throw null;

public virtual ClientResult Client(Models.ClientModel body) => throw null;

public virtual Task<ClientResult> ClientAsync(Models.ClientModel body) => throw null;

public virtual ClientResult Language(BinaryContent content, RequestOptions options) => throw null;
public virtual ClientResult Language(BinaryContent content, RequestOptions options = null) => throw null;

public virtual Task<ClientResult> LanguageAsync(BinaryContent content, RequestOptions options) => throw null;
public virtual Task<ClientResult> LanguageAsync(BinaryContent content, RequestOptions options = null) => throw null;

public virtual ClientResult Language(CSModel body) => throw null;

Expand Down
12 changes: 6 additions & 6 deletions ...-csharp/generator/TestProjects/CadlRanch/http/client/naming/src/Generated/NamingClient.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,25 +34,25 @@ public partial class NamingClient

public virtual Task<ClientResult> ParameterAsync(string clientName) => throw null;

public virtual ClientResult Client(BinaryContent content, RequestOptions options) => throw null;
public virtual ClientResult Client(BinaryContent content, RequestOptions options = null) => throw null;

public virtual Task<ClientResult> ClientAsync(BinaryContent content, RequestOptions options) => throw null;
public virtual Task<ClientResult> ClientAsync(BinaryContent content, RequestOptions options = null) => throw null;

public virtual ClientResult Client(ClientNameModel body) => throw null;

public virtual Task<ClientResult> ClientAsync(ClientNameModel body) => throw null;

public virtual ClientResult Language(BinaryContent content, RequestOptions options) => throw null;
public virtual ClientResult Language(BinaryContent content, RequestOptions options = null) => throw null;

public virtual Task<ClientResult> LanguageAsync(BinaryContent content, RequestOptions options) => throw null;
public virtual Task<ClientResult> LanguageAsync(BinaryContent content, RequestOptions options = null) => throw null;

public virtual ClientResult Language(LanguageClientNameModel body) => throw null;

public virtual Task<ClientResult> LanguageAsync(LanguageClientNameModel body) => throw null;

public virtual ClientResult CompatibleWithEncodedName(BinaryContent content, RequestOptions options) => throw null;
public virtual ClientResult CompatibleWithEncodedName(BinaryContent content, RequestOptions options = null) => throw null;

public virtual Task<ClientResult> CompatibleWithEncodedNameAsync(BinaryContent content, RequestOptions options) => throw null;
public virtual Task<ClientResult> CompatibleWithEncodedNameAsync(BinaryContent content, RequestOptions options = null) => throw null;

public virtual ClientResult CompatibleWithEncodedName(ClientNameAndJsonEncodedNameModel body) => throw null;

Expand Down
8 changes: 4 additions & 4 deletions ...ent-csharp/generator/TestProjects/CadlRanch/http/client/naming/src/Generated/UnionEnum.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,17 @@ public partial class UnionEnum

public ClientPipeline Pipeline => throw null;

public virtual ClientResult UnionEnumName(BinaryContent content, RequestOptions options) => throw null;
public virtual ClientResult UnionEnumName(BinaryContent content, RequestOptions options = null) => throw null;

public virtual Task<ClientResult> UnionEnumNameAsync(BinaryContent content, RequestOptions options) => throw null;
public virtual Task<ClientResult> UnionEnumNameAsync(BinaryContent content, RequestOptions options = null) => throw null;

public virtual ClientResult UnionEnumName(ClientExtensibleEnum body) => throw null;

public virtual Task<ClientResult> UnionEnumNameAsync(ClientExtensibleEnum body) => throw null;

public virtual ClientResult UnionEnumMemberName(BinaryContent content, RequestOptions options) => throw null;
public virtual ClientResult UnionEnumMemberName(BinaryContent content, RequestOptions options = null) => throw null;

public virtual Task<ClientResult> UnionEnumMemberNameAsync(BinaryContent content, RequestOptions options) => throw null;
public virtual Task<ClientResult> UnionEnumMemberNameAsync(BinaryContent content, RequestOptions options = null) => throw null;

public virtual ClientResult UnionEnumMemberName(ExtensibleEnum body) => throw null;

Expand Down
Loading

0 comments on commit 910ca35

Please sign in to comment.