diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Primitives/ScmKnownParameters.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Primitives/ScmKnownParameters.cs index e8d995dbdb6..b75ce2e99f3 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Primitives/ScmKnownParameters.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Primitives/ScmKnownParameters.cs @@ -30,23 +30,40 @@ internal static class ScmKnownParameters public static readonly ParameterProvider JsonElement = new("element", FormattableStringHelpers.Empty, typeof(JsonElement)); public static readonly ParameterProvider Data = new("data", FormattableStringHelpers.Empty, typeof(BinaryData)); public static ParameterProvider ClientOptions(CSharpType clientOptionsType) - => new("options", $"The options for configuring the client.", clientOptionsType.WithNullable(true), initializationValue: New.Instance(clientOptionsType.WithNullable(true))); + => new("options", $"The options for configuring the client.", clientOptionsType, initializationValue: New.Instance(clientOptionsType)); public static readonly ParameterProvider OptionalRequestOptions = new( ScmCodeModelGenerator.Instance.TypeFactory.HttpRequestOptionsApi.ParameterName, $"The request options, which can override default behaviors of the client pipeline on a per-call basis.", - ScmCodeModelGenerator.Instance.TypeFactory.HttpRequestOptionsApi.HttpRequestOptionsType.WithNullable(true), + ScmCodeModelGenerator.Instance.TypeFactory.HttpRequestOptionsApi.HttpRequestOptionsType, defaultValue: Null); public static readonly ParameterProvider RequestOptions = new(ScmCodeModelGenerator.Instance.TypeFactory.HttpRequestOptionsApi.ParameterName, $"The request options, which can override default behaviors of the client pipeline on a per-call basis.", ScmCodeModelGenerator.Instance.TypeFactory.HttpRequestOptionsApi.HttpRequestOptionsType); - public static readonly ParameterProvider RequestContent = new("content", $"The content to send as the body of the request.", ScmCodeModelGenerator.Instance.TypeFactory.RequestContentApi.RequestContentType, location: ParameterLocation.Body) { Validation = ParameterValidationType.AssertNotNull }; public static readonly ParameterProvider CancellationToken = new("cancellationToken", $"The cancellation token that can be used to cancel the operation.", new CSharpType(typeof(CancellationToken)), defaultValue: Default); - // There is intentionally no default value here to avoid ambiguous calls between convenience and protocol methods. - public static readonly ParameterProvider OptionalRequestContent = new( - "content", - $"The content to send as the body of the request.", - ScmCodeModelGenerator.Instance.TypeFactory.RequestContentApi.RequestContentType.WithNullable(true), + private static readonly FormattableString RequestContentDescription = $"The content to send as the body of the request."; + private const string RequestContentParameterName = "content"; + + public static readonly ParameterProvider RequestContent = new( + RequestContentParameterName, + RequestContentDescription, + ScmCodeModelGenerator.Instance.TypeFactory.RequestContentApi.RequestContentType, + location: ParameterLocation.Body) + { + Validation = ParameterValidationType.AssertNotNull + }; + + public static readonly ParameterProvider NullableRequiredRequestContent = new( + RequestContentParameterName, + RequestContentDescription, + ScmCodeModelGenerator.Instance.TypeFactory.RequestContentApi.RequestContentType, location: ParameterLocation.Body); + public static readonly ParameterProvider OptionalRequestContent = new( + RequestContentParameterName, + RequestContentDescription, + ScmCodeModelGenerator.Instance.TypeFactory.RequestContentApi.RequestContentType, + location: ParameterLocation.Body, + defaultValue: Null); + // Known header parameters public static readonly ParameterProvider RepeatabilityRequestId = new("repeatabilityRequestId", FormattableStringHelpers.Empty, typeof(Guid)) { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmMethodProviderCollection.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmMethodProviderCollection.cs index 8e0e060e9f7..c6f9fecc0e9 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmMethodProviderCollection.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ScmMethodProviderCollection.cs @@ -32,6 +32,7 @@ public class ScmMethodProviderCollection : IReadOnlyList private IReadOnlyList? _convenienceMethodParameters; private readonly InputPagingServiceMethod? _pagingServiceMethod; private IReadOnlyList? _methods; + private readonly bool _generateConvenienceMethod; private ClientProvider Client { get; } protected InputServiceMethod ServiceMethod { get; } @@ -69,6 +70,8 @@ public ScmMethodProviderCollection(InputServiceMethod serviceMethod, TypeProvide Client = enclosingType as ClientProvider ?? throw new InvalidOperationException("Scm methods can only be built for client types."); _createRequestMethod = Client.RestClient.GetCreateRequestMethod(ServiceMethod.Operation); + _generateConvenienceMethod = ServiceMethod.Operation is + { GenerateConvenienceMethod: true, IsMultipartFormData: false }; if (serviceMethod is InputPagingServiceMethod pagingServiceMethod) { @@ -92,7 +95,7 @@ protected virtual IReadOnlyList BuildMethods() var syncProtocol = BuildProtocolMethod(_createRequestMethod, false); var asyncProtocol = BuildProtocolMethod(_createRequestMethod, true); - if (ServiceMethod.Operation.GenerateConvenienceMethod && !ServiceMethod.Operation.IsMultipartFormData) + if (_generateConvenienceMethod) { return [ @@ -539,13 +542,21 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod { // If there are optional parameters, but the request options parameter is not optional, make the optional parameters nullable required. // This is to ensure that the request options parameter is always the last parameter. + var newlyRequiredParameters = new List(optionalParameters.Count); foreach (var parameter in optionalParameters) { + // don't mutate the static request content parameter + if (parameter.Equals(ScmKnownParameters.OptionalRequestContent)) + { + newlyRequiredParameters.Add(ScmKnownParameters.NullableRequiredRequestContent); + continue; + } parameter.DefaultValue = null; parameter.Type = parameter.Type.WithNullable(true); + newlyRequiredParameters.Add(parameter); } - requiredParameters.AddRange(optionalParameters); + requiredParameters.AddRange(newlyRequiredParameters); optionalParameters.Clear(); } @@ -694,19 +705,23 @@ private CSharpType GetConvenienceReturnType(IReadOnlyList m.Signature.Parameters.Any(p => p.Type.Equals(typeof(string)))).ToList(); Assert.AreEqual(2, convenienceMethods.Count); @@ -733,14 +733,14 @@ public void TestRequestOptionsParameterInSignature(InputServiceMethod inputServi var requestOptionsParameterInSyncMethod = syncMethod!.Signature.Parameters.FirstOrDefault(p => p.Type.Name == "RequestOptions"); Assert.IsNotNull(requestOptionsParameterInSyncMethod); - Assert.AreEqual(shouldBeOptional, requestOptionsParameterInSyncMethod!.Type.IsNullable); + Assert.AreEqual(shouldBeOptional, requestOptionsParameterInSyncMethod!.DefaultValue != null); 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); + Assert.AreEqual(shouldBeOptional, requestOptionsParameterInAsyncMethod!.DefaultValue != null); // request options should always be last parameter Assert.AreEqual("RequestOptions", syncMethod.Signature.Parameters[^1].Type.Name); @@ -1605,7 +1605,7 @@ public static IEnumerable RequestOptionsParameterInSignatureTestCa isRequired: true), ]), false, true); - // convenience method only has a body param, so RequestOptions should be optional in protocol method. + // convenience method only has a body param, but it is optional, so RequestOptions should be optional in protocol method. yield return new TestCaseData( InputFactory.BasicServiceMethod( "TestServiceMethod", @@ -1624,7 +1624,7 @@ public static IEnumerable RequestOptionsParameterInSignatureTestCa "p1", InputPrimitiveType.String, location: InputRequestLocation.Body), - ]), true, false); + ]), false, false); // Protocol & convenience methods will have different parameters since there is a model body param, so RequestOptions should be optional. yield return new TestCaseData( @@ -1659,8 +1659,7 @@ public static IEnumerable RequestOptionsParameterInSignatureTestCa isRequired: true), ]), true, false); - // Protocol & convenience methods will have different parameters since there is a model body param, so RequestOptions should be optional. - // One parameter is optional + // Protocol & convenience methods will have different parameters but since the body parameter is optional, the request options should be required in protocol method. yield return new TestCaseData( InputFactory.BasicServiceMethod( "TestServiceMethod", @@ -1691,7 +1690,7 @@ public static IEnumerable RequestOptionsParameterInSignatureTestCa InputFactory.Model("SampleModel"), location: InputRequestLocation.Body, isRequired: false), - ]), true, true); + ]), false, true); // Convenience method has no parameters, RequestOptions should be required in protocol method. diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmMethodProviderCollectionTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmMethodProviderCollectionTests.cs index a0d4eee7ecb..ae9ec698175 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmMethodProviderCollectionTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ScmMethodProviderCollectionTests.cs @@ -276,9 +276,11 @@ public void ListMethodWithEnumParameter(bool isExtensible, InputRequestLocation } } - [TestCase(true)] - [TestCase(false)] - public void RequestOptionsOptionality(bool inBody) + [TestCase(true, false, true)] + [TestCase(true, true, false)] + [TestCase(false, false, false)] + [TestCase(false, true, false)] + public void RequestOptionsOptionality(bool inBody, bool hasOptionalParameter, bool shouldBeOptional) { MockHelpers.LoadMockGenerator(); List parameters = @@ -286,7 +288,7 @@ public void RequestOptionsOptionality(bool inBody) InputFactory.Parameter( "message", InputPrimitiveType.Boolean, - isRequired: true, + isRequired: !hasOptionalParameter, location: inBody ? InputRequestLocation.Body : InputRequestLocation.Query) ]; var inputOperation = InputFactory.Operation( @@ -302,19 +304,34 @@ public void RequestOptionsOptionality(bool inBody) Assert.AreEqual(inputServiceMethod, protocolMethod!.ServiceMethod); var optionsParameter = protocolMethod!.Signature.Parameters.Single(p => p.Name == "options"); - if (inBody) - { - // When the parameter is in the body, the signatures of the protocol and convenience methods - // will differ due to the presence of the BinaryContent parameter, which means the options parameter - // can remain optional. - Assert.IsNotNull(optionsParameter.DefaultValue); - } - else + Assert.AreEqual(shouldBeOptional, optionsParameter.DefaultValue != null); + + if (!shouldBeOptional) { - Assert.IsNull(optionsParameter.DefaultValue); + Assert.IsTrue(protocolMethod.Signature.Parameters.All(p => p.DefaultValue == null)); } } + [Test] + public void RequestOptionsIsOptionalWhenNoConvenience() + { + MockHelpers.LoadMockGenerator(); + var inputOperation = InputFactory.Operation( + "TestOperation", + generateConvenienceMethod: false); + var inputServiceMethod = InputFactory.BasicServiceMethod("Test", inputOperation); + var inputClient = InputFactory.Client("TestClient", methods: [inputServiceMethod]); + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + var methodCollection = new ScmMethodProviderCollection(inputServiceMethod, client!); + var protocolMethod = methodCollection.FirstOrDefault( + m => m.Signature.Parameters.Any(p => p.Name == "options") && m.Signature.Name == "TestOperation"); + Assert.IsNotNull(protocolMethod); + Assert.AreEqual(inputServiceMethod, protocolMethod!.ServiceMethod); + + var optionsParameter = protocolMethod!.Signature.Parameters.Single(p => p.Name == "options"); + Assert.IsNotNull(optionsParameter.DefaultValue); + } + [Test] public void OperationWithOptionalEnum() { diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/InputFactory.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/InputFactory.cs index 107d10bdb0a..7693653fc08 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/InputFactory.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator/test/common/InputFactory.cs @@ -429,7 +429,8 @@ public static InputOperation Operation( IEnumerable? requestMediaTypes = null, string uri = "", string path = "", - string httpMethod = "GET") + string httpMethod = "GET", + bool generateConvenienceMethod = true) { return new InputOperation( name, @@ -447,7 +448,7 @@ public static InputOperation Operation( requestMediaTypes is null ? null : [.. requestMediaTypes], false, true, - true, + generateConvenienceMethod, name); } diff --git a/packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/SampleTypeSpecClient.cs b/packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/SampleTypeSpecClient.cs index 819da232494..9744859ef8b 100644 --- a/packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/SampleTypeSpecClient.cs +++ b/packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/SampleTypeSpecClient.cs @@ -522,7 +522,7 @@ public virtual async Task> TopActionAsync(DateTimeOffset act /// The request options, which can override default behaviors of the client pipeline on a per-call basis. /// Service returned a non-success status code. /// The response returned from the service. - public virtual ClientResult TopAction2(RequestOptions options) + public virtual ClientResult TopAction2(RequestOptions options = null) { using PipelineMessage message = CreateTopAction2Request(options); return ClientResult.FromResponse(Pipeline.ProcessMessage(message, options)); @@ -539,7 +539,7 @@ public virtual ClientResult TopAction2(RequestOptions options) /// The request options, which can override default behaviors of the client pipeline on a per-call basis. /// Service returned a non-success status code. /// The response returned from the service. - public virtual async Task TopAction2Async(RequestOptions options) + public virtual async Task TopAction2Async(RequestOptions options = null) { using PipelineMessage message = CreateTopAction2Request(options); return ClientResult.FromResponse(await Pipeline.ProcessMessageAsync(message, options).ConfigureAwait(false)); diff --git a/packages/http-client-csharp/generator/TestProjects/Spector/http/parameters/body-optionality/src/Generated/OptionalExplicit.cs b/packages/http-client-csharp/generator/TestProjects/Spector/http/parameters/body-optionality/src/Generated/OptionalExplicit.cs index ffa4809571b..a05734714ab 100644 --- a/packages/http-client-csharp/generator/TestProjects/Spector/http/parameters/body-optionality/src/Generated/OptionalExplicit.cs +++ b/packages/http-client-csharp/generator/TestProjects/Spector/http/parameters/body-optionality/src/Generated/OptionalExplicit.cs @@ -16,17 +16,17 @@ public partial class OptionalExplicit public ClientPipeline Pipeline => throw null; - public virtual ClientResult Set(BinaryContent content, RequestOptions options = null) => throw null; + public virtual ClientResult Set(BinaryContent content, RequestOptions options) => throw null; - public virtual Task SetAsync(BinaryContent content, RequestOptions options = null) => throw null; + public virtual Task SetAsync(BinaryContent content, RequestOptions options) => throw null; public virtual ClientResult Set(BodyModel body = default, CancellationToken cancellationToken = default) => throw null; public virtual Task SetAsync(BodyModel body = default, CancellationToken cancellationToken = default) => throw null; - public virtual ClientResult Omit(BinaryContent content, RequestOptions options = null) => throw null; + public virtual ClientResult Omit(BinaryContent content, RequestOptions options) => throw null; - public virtual Task OmitAsync(BinaryContent content, RequestOptions options = null) => throw null; + public virtual Task OmitAsync(BinaryContent content, RequestOptions options) => throw null; public virtual ClientResult Omit(BodyModel body = default, CancellationToken cancellationToken = default) => throw null; diff --git a/packages/http-client-csharp/generator/TestProjects/Spector/http/payload/json-merge-patch/src/Generated/JsonMergePatchClient.cs b/packages/http-client-csharp/generator/TestProjects/Spector/http/payload/json-merge-patch/src/Generated/JsonMergePatchClient.cs index bd7a96d4b2f..06f7a37dced 100644 --- a/packages/http-client-csharp/generator/TestProjects/Spector/http/payload/json-merge-patch/src/Generated/JsonMergePatchClient.cs +++ b/packages/http-client-csharp/generator/TestProjects/Spector/http/payload/json-merge-patch/src/Generated/JsonMergePatchClient.cs @@ -30,8 +30,8 @@ public partial class JsonMergePatchClient public virtual Task UpdateResourceAsync(BinaryContent content, RequestOptions options = null) => throw null; - public virtual ClientResult UpdateOptionalResource(BinaryContent content, RequestOptions options = null) => throw null; + public virtual ClientResult UpdateOptionalResource(BinaryContent content = null, RequestOptions options = null) => throw null; - public virtual Task UpdateOptionalResourceAsync(BinaryContent content, RequestOptions options = null) => throw null; + public virtual Task UpdateOptionalResourceAsync(BinaryContent content = null, RequestOptions options = null) => throw null; } }