Skip to content
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
8717fbc
Put all optional client parameters into client options
pshao25 Nov 7, 2023
71287d1
Merge branch 'feature/v3' of https://github.com/Azure/autorest.csharp…
pshao25 Nov 7, 2023
4472b2f
Merge branch 'feature/v3' of https://github.com/Azure/autorest.csharp…
pshao25 Nov 8, 2023
c26032e
Merge branch 'feature/v3' of https://github.com/Azure/autorest.csharp…
pshao25 Nov 10, 2023
ab7d989
update
pshao25 Nov 13, 2023
4abe948
Regenerate all code (any successful run even without change will have…
actions-user Nov 13, 2023
15b9299
update
pshao25 Nov 14, 2023
f750175
Merge branch 'constructor3706' of https://github.com/pshao25/autorest…
pshao25 Nov 14, 2023
4997efc
Merge branch 'feature/v3' of https://github.com/Azure/autorest.csharp…
pshao25 Nov 14, 2023
3d9d590
update
pshao25 Nov 14, 2023
e2d833e
Merge branch 'feature/v3' of https://github.com/Azure/autorest.csharp…
pshao25 Nov 15, 2023
7aa23a5
update
pshao25 Nov 15, 2023
862d4eb
Merge branch 'feature/v3' of https://github.com/Azure/autorest.csharp…
pshao25 Nov 16, 2023
66beca9
update
pshao25 Nov 16, 2023
b303284
update
pshao25 Nov 16, 2023
37f52ba
update
pshao25 Nov 16, 2023
d9f06da
update
pshao25 Nov 16, 2023
87d5950
Merge branch 'feature/v3' of https://github.com/Azure/autorest.csharp…
pshao25 Nov 27, 2023
1506b29
update
pshao25 Nov 27, 2023
2c96f11
update
pshao25 Nov 27, 2023
c9f5a2c
Merge branch 'feature/v3' of https://github.com/Azure/autorest.csharp…
pshao25 Nov 27, 2023
b428cee
update
pshao25 Nov 27, 2023
9fabdbe
update
pshao25 Nov 28, 2023
e4d0ab7
Merge branch 'feature/v3' of https://github.com/Azure/autorest.csharp…
pshao25 Nov 28, 2023
e20e3b7
update
pshao25 Nov 28, 2023
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
56 changes: 56 additions & 0 deletions docs/features/service_client_constructors.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
## Service Client Constrctors

Scope: This document only applies to DPG services.
Comment thread
pshao25 marked this conversation as resolved.
Outdated

There will be two **public** client constructors in a generated service client. One primary public constructor with all the user defined client parameters plus `ClientOptions`, and one secondary public constructor with all the user defined **required** client parameters. The secondary public constructor calls the primary public constructor underneath.
Comment thread
pshao25 marked this conversation as resolved.
Outdated

For example, user defines required parameters `Uri endpoint` and `AzureKeyCredential credential`, then the generated public constructors are:
Comment thread
pshao25 marked this conversation as resolved.
Outdated

```C#
public ServiceClient(Uri endpoint, AzureKeyCredential credential) : this(endpoint, credential, new ServiceClientOptions()) {}

public ServiceClient(Uri endpoint, AzureKeyCredential credential, ServiceClientOptions options) {}
```

Normally, all the client parameters are required. But if user defines some additional optional client parameters, all the optional client parameters will go to `ClientOptions`, except `endpoint`.
Comment thread
pshao25 marked this conversation as resolved.
Outdated

For example, user defines an optional parameter `string optional`. The `ClientOptions` will have a new property as
Comment thread
pshao25 marked this conversation as resolved.
Outdated

```C#
public partial class ServiceClientOptions : ClientOptions
{
...

public string Optional { get; set; }
}
```

The signature of the public constructors will keep the same, but its implementation will become to
Comment thread
pshao25 marked this conversation as resolved.
Outdated
```C#
public ServiceClient(Uri endpoint, AzureKeyCredential credential) : this(endpoint, credential, new ServiceClientOptions()) {}

public ServiceClient(Uri endpoint, AzureKeyCredential credential, ServiceClientOptions options)
{
...
Comment thread
pshao25 marked this conversation as resolved.
Outdated
_optional = options.Optional;
}
```

If the optional client parameters user defines contains `endpoint`, it will still be in the client constructor, not going to `ClientOptions`. For example, user defines an optional parameter `string optional`, an optional `endpoint` with default value `"http://localhost:3000"`, and a required parameter `AzureKeyCredential credential`, the generated public constructors are:
Comment thread
pshao25 marked this conversation as resolved.
Outdated

```C#
public ServiceClient(AzureKeyCredential credential) : this(new Uri("http://localhost:3000"), credential, new ServiceClientOptions()) {}

public ServiceClient(Uri endpoint, AzureKeyCredential credential, ServiceClientOptions options)
{
...
Comment thread
pshao25 marked this conversation as resolved.
Outdated
_optional = options.Optional;
}

public partial class ServiceClientOptions : ClientOptions
{
...

public string Optional { get; set; }
}
```
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Linq;
using AutoRest.CSharp.Common.Input;
using AutoRest.CSharp.Output.Models.Types;
using AutoRest.CSharp.Utilities;

namespace AutoRest.CSharp.Generation.Writers
{
Expand Down Expand Up @@ -49,6 +50,15 @@ public static void WriteClientOptions(CodeWriter writer, ClientOptionsTypeProvid
writer.Line($"_ => throw new {typeof(NotSupportedException)}()");
}
}

writer.Line();
}

foreach (var parameter in clientOptions.AdditionalParameters)
{
writer.WriteXmlDocumentationSummary(parameter.Description);
writer.Line($"public {parameter.Type} {parameter.Name.ToCleanName()} {{ get; set; }}");
writer.Line();
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal static class KnownParameters
public static readonly Parameter Pipeline = new("pipeline", $"The HTTP pipeline for sending and receiving REST requests and responses", new CSharpType(Configuration.ApiTypes.HttpPipelineType), null, ValidationType.AssertNotNull, null);
public static readonly Parameter KeyAuth = new("keyCredential", $"The key credential to copy", new CSharpType(Configuration.ApiTypes.KeyCredentialType), null, ValidationType.None, null);
public static readonly Parameter TokenAuth = new("tokenCredential", $"The token credential to copy", new CSharpType(typeof(TokenCredential)), null, ValidationType.None, null);
public static readonly Parameter Endpoint = new("endpoint", $"Service endpoint", new CSharpType(typeof(Uri)), null, ValidationType.None, null, RequestLocation: RequestLocation.Uri);
public static readonly Parameter Endpoint = new("endpoint", $"Service endpoint", new CSharpType(typeof(Uri)), null, ValidationType.None, null, RequestLocation: RequestLocation.Uri, IsEndpoint: true);

public static readonly Parameter PageSizeHint = new("pageSizeHint", $"The number of items per {typeof(Page<>):C} that should be requested (from service operations that support it). It's not guaranteed that the value will be respected.", new CSharpType(typeof(int), true), null, ValidationType.None, null);
public static readonly Parameter NextLink = new("nextLink", $"Continuation token", typeof(string), null, ValidationType.None, null);
Expand Down
5 changes: 4 additions & 1 deletion src/AutoRest.CSharp/Common/Output/Models/Shared/Parameter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

namespace AutoRest.CSharp.Output.Models.Shared
{
internal record Parameter(string Name, FormattableString? Description, CSharpType Type, Constant? DefaultValue, ValidationType Validation, FormattableString? Initializer, bool IsApiVersionParameter = false, bool IsResourceIdentifier = false, bool SkipUrlEncoding = false, RequestLocation RequestLocation = RequestLocation.None, SerializationFormat SerializationFormat = SerializationFormat.Default, bool IsPropertyBag = false)
internal record Parameter(string Name, FormattableString? Description, CSharpType Type, Constant? DefaultValue, ValidationType Validation, FormattableString? Initializer, bool IsApiVersionParameter = false, bool IsEndpoint = false, bool IsResourceIdentifier = false, bool SkipUrlEncoding = false, RequestLocation RequestLocation = RequestLocation.None, SerializationFormat SerializationFormat = SerializationFormat.Default, bool IsPropertyBag = false)
Comment thread
archerzz marked this conversation as resolved.
{
public CSharpAttribute[] Attributes { get; init; } = Array.Empty<CSharpAttribute>();
public bool IsOptionalInSignature => DefaultValue != null;
Expand Down Expand Up @@ -74,6 +74,7 @@ public static Parameter FromInputParameter(in InputParameter operationParameter,
validation,
initializer,
IsApiVersionParameter: operationParameter.IsApiVersion,
IsEndpoint: operationParameter.IsEndpoint,
IsResourceIdentifier: operationParameter.IsResourceParameter,
SkipUrlEncoding: skipUrlEncoding,
RequestLocation: requestLocation,
Expand All @@ -82,6 +83,7 @@ public static Parameter FromInputParameter(in InputParameter operationParameter,

private static Constant? GetDefaultValue(InputParameter operationParameter, TypeFactory typeFactory) => operationParameter switch
{
{ Name: "$host", DefaultValue.Value: ""} => (Constant?)null, // In M4, when endpoint is not set, its default value is an empty string instead of null
Comment thread
pshao25 marked this conversation as resolved.
Outdated
{ NameInRequest: var nameInRequest } when RequestHeader.ClientRequestIdHeaders.Contains(nameInRequest) => Constant.FromExpression($"message.{Configuration.ApiTypes.HttpMessageRequestName}.ClientRequestId", new CSharpType(typeof(string))),
{ NameInRequest: var nameInRequest } when RequestHeader.ReturnClientRequestIdResponseHeaders.Contains(nameInRequest) => new Constant("true", new CSharpType(typeof(string))),
{ DefaultValue: not null } => BuilderHelpers.ParseConstant(operationParameter.DefaultValue.Value, typeFactory.CreateType(operationParameter.DefaultValue.Type)),
Expand Down Expand Up @@ -167,6 +169,7 @@ public static Parameter FromRequestParameter(in RequestParameter requestParamete
validation,
initializer,
IsApiVersionParameter: requestParameter.Origin == "modelerfour:synthesized/api-version",
IsEndpoint: IsEndpointParameter(requestParameter),
IsResourceIdentifier: requestParameter.IsResourceParameter,
SkipUrlEncoding: skipUrlEncoding,
RequestLocation: requestLocation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Linq;
using System.Text;
using AutoRest.CSharp.Input.Source;
using AutoRest.CSharp.Output.Models.Shared;

namespace AutoRest.CSharp.Output.Models.Types
{
Expand All @@ -16,6 +17,7 @@ internal sealed class ClientOptionsTypeProvider : TypeProvider

public FormattableString Description { get; }
public IReadOnlyList<ApiVersion>? ApiVersions { get; }
public IReadOnlyList<Parameter> AdditionalParameters { get; init; }
protected override string DefaultName { get; }
protected override string DefaultAccessibility { get; }

Expand All @@ -27,6 +29,8 @@ public ClientOptionsTypeProvider(IReadOnlyList<string>? versions, string name, s

if (versions is not null)
ApiVersions = ConvertApiVersions(versions);

AdditionalParameters = Array.Empty<Parameter>();
}

private static ApiVersion[] ConvertApiVersions(IReadOnlyList<string> versions) =>
Expand Down
4 changes: 4 additions & 0 deletions src/AutoRest.CSharp/LowLevel/Generation/DpgClientWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,10 @@ private void WritePrimaryPublicConstructor(ConstructorSignature signature)
{
_writer.Line($"{field.Name:I} = {clientOptionsParameter.Name:I}.Version;");
}
else if (_client.ClientOptions.AdditionalParameters.Contains(parameter))
{
_writer.Line($"{field.Name:I} = {clientOptionsParameter.Name:I}.{parameter.Name.ToCleanName()};");
}
else
{
_writer.Line($"{field.Name:I} = {parameter.Name:I};");
Expand Down
23 changes: 15 additions & 8 deletions src/AutoRest.CSharp/LowLevel/Output/DpgOutputLibraryBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using AutoRest.CSharp.Generation.Types;
using AutoRest.CSharp.Input.Source;
using AutoRest.CSharp.Output.Builders;
using AutoRest.CSharp.Output.Models.Shared;
using AutoRest.CSharp.Output.Models.Types;
using AutoRest.CSharp.Utilities;
using Microsoft.CodeAnalysis;
Expand Down Expand Up @@ -44,7 +45,8 @@ public DpgOutputLibrary Build(bool isTspInput)
.ToDictionary(ci => ci.Name);
AssignParentClients(inputClients, clientInfosByName);
var topLevelClientInfos = SetHierarchy(clientInfosByName);
var clientOptions = CreateClientOptions(topLevelClientInfos);
var parametersInClientOptions = new List<Parameter>();
var clientOptions = CreateClientOptions(topLevelClientInfos, parametersInClientOptions);

SetRequestsToClients(clientInfosByName.Values);

Expand All @@ -59,7 +61,7 @@ public DpgOutputLibrary Build(bool isTspInput)
CreateModels(models, library.TypeFactory);
CreateEnums(enums, models, library.TypeFactory);
}
CreateClients(clients, topLevelClientInfos, library.TypeFactory, clientOptions);
CreateClients(clients, topLevelClientInfos, library.TypeFactory, clientOptions, parametersInClientOptions);

return library;
}
Expand Down Expand Up @@ -459,7 +461,7 @@ private static IReadOnlyList<InputParameter> UpdateOperationParameters(IReadOnly
return parameters;
}

private ClientOptionsTypeProvider CreateClientOptions(IReadOnlyList<ClientInfo> topLevelClientInfos)
private ClientOptionsTypeProvider CreateClientOptions(IReadOnlyList<ClientInfo> topLevelClientInfos, List<Parameter> parametersInClientOptions)
{
var clientName = topLevelClientInfos.Count == 1
? topLevelClientInfos[0].Name
Expand All @@ -477,7 +479,10 @@ private ClientOptionsTypeProvider CreateClientOptions(IReadOnlyList<ClientInfo>
throw new InvalidOperationException("Multiple API versions are not supported in the unbranded path.");
apiVersions = null;
}
return new ClientOptionsTypeProvider(apiVersions, clientOptionsName, _defaultNamespace, description, _sourceInputModel);
return new ClientOptionsTypeProvider(apiVersions, clientOptionsName, _defaultNamespace, description, _sourceInputModel)
{
AdditionalParameters = parametersInClientOptions
};
}

private static ClientInfo CreateClientInfo(InputClient ns, SourceInputModel? sourceInputModel, string rootNamespaceName)
Expand Down Expand Up @@ -626,9 +631,9 @@ private static void SetRequestToClient(ClientInfo clientInfo, InputOperation ope
clientInfo.Requests.Add(operation);
}

private void CreateClients(List<LowLevelClient> allClients, IEnumerable<ClientInfo> topLevelClientInfos, TypeFactory typeFactory, ClientOptionsTypeProvider clientOptions)
private void CreateClients(List<LowLevelClient> allClients, IEnumerable<ClientInfo> topLevelClientInfos, TypeFactory typeFactory, ClientOptionsTypeProvider clientOptions, List<Parameter> parametersInClientOptions)
{
var topLevelClients = CreateClients(topLevelClientInfos, typeFactory, clientOptions, null);
var topLevelClients = CreateClients(topLevelClientInfos, typeFactory, clientOptions, null, parametersInClientOptions);

// Simple implementation of breadth first traversal
allClients.AddRange(topLevelClients);
Expand All @@ -638,7 +643,7 @@ private void CreateClients(List<LowLevelClient> allClients, IEnumerable<ClientIn
}
}

private IEnumerable<LowLevelClient> CreateClients(IEnumerable<ClientInfo> clientInfos, TypeFactory typeFactory, ClientOptionsTypeProvider clientOptions, LowLevelClient? parentClient)
private IEnumerable<LowLevelClient> CreateClients(IEnumerable<ClientInfo> clientInfos, TypeFactory typeFactory, ClientOptionsTypeProvider clientOptions, LowLevelClient? parentClient, List<Parameter> parametersInClientOptions)
{
foreach (var clientInfo in clientInfos)
{
Expand Down Expand Up @@ -673,7 +678,9 @@ private IEnumerable<LowLevelClient> CreateClients(IEnumerable<ClientInfo> client
SubClients = subClients
};

subClients.AddRange(CreateClients(clientInfo.Children, typeFactory, clientOptions, client));
subClients.AddRange(CreateClients(clientInfo.Children, typeFactory, clientOptions, client, parametersInClientOptions));
// parametersInClientOptions is assigned to ClientOptionsTypeProvider.AdditionalProperties before, which makes sure AdditionalProperties is readonly and won't change after ClientOptionsTypeProvider is built.
parametersInClientOptions.AddRange(client.GetOptionalParametersInOptions());
Comment thread
pshao25 marked this conversation as resolved.

yield return client;
}
Expand Down
13 changes: 12 additions & 1 deletion src/AutoRest.CSharp/LowLevel/Output/LowLevelClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public static IEnumerable<LowLevelClientMethod> BuildMethods(LowLevelClient? cli
if (!IsSubClient)
{
var requiredParameters = RestClientBuilder.GetRequiredParameters(orderedParameters).ToArray();
var optionalParameters = RestClientBuilder.GetOptionalParameters(orderedParameters).Append(CreateOptionsParameter()).ToArray();
var optionalParameters = GetOptionalParametersInConstructor(RestClientBuilder.GetOptionalParameters(orderedParameters).Append(CreateOptionsParameter())).ToArray();

return (
BuildPrimaryConstructors(requiredParameters, optionalParameters).ToArray(),
Expand All @@ -153,6 +153,17 @@ public static IEnumerable<LowLevelClientMethod> BuildMethods(LowLevelClient? cli
}
}

private IEnumerable<Parameter> GetOptionalParametersInConstructor(IEnumerable<Parameter> optionalParameters)
{
return optionalParameters.Where(
p => ClientOptions.Type.EqualsIgnoreNullable(p.Type) || p.IsEndpoint); // Endpoint is an exception, even it is optional, still need to be the parameter of constructor
}

public IEnumerable<Parameter> GetOptionalParametersInOptions()
{
return RestClientBuilder.GetOptionalParameters(Parameters).Where(p => !p.IsEndpoint);
}

private IEnumerable<ConstructorSignature> BuildPrimaryConstructors(IReadOnlyList<Parameter> requiredParameters, IReadOnlyList<Parameter> optionalParameters)
{
var optionalToRequired = optionalParameters
Expand Down
Loading