Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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
9 changes: 7 additions & 2 deletions docs/samples/client/csharp/SampleService/main.tsp
Original file line number Diff line number Diff line change
Expand Up @@ -833,14 +833,19 @@ interface PlantOperations {
};
}

model MetricsClientParams {
metricsNamespace: string;
}

@clientInitialization({
Comment thread
JoshLove-msft marked this conversation as resolved.
initializedBy: InitializedBy.individually | InitializedBy.parent,
parameters: MetricsClientParams,
})
interface Metrics {
@doc("Get Widget metrics for given day of week")
@get
@route("/metrics/widgets/daysOfWeek")
getWidgetMetrics(@path day: DaysOfWeekExtensibleEnum): {
@route("/metrics/{metricsNamespace}/widgets/daysOfWeek")
getWidgetMetrics(@path metricsNamespace: string, @path day: DaysOfWeekExtensibleEnum): {
numSold: int32;
averagePrice: float32;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,24 @@ private IReadOnlyList<ParameterProvider> GetSubClientInternalConstructorParamete
return subClientParameters;
}

/// <summary>
/// Determines whether this subclient has non-infrastructure parameters (i.e. parameters that
/// are not API versions and not the endpoint parameter) that are absent from the given parent
/// client and therefore need to be included as parameters in the parent's accessor method.
/// Uses the raw <see cref="InputClient.Parameters"/> to avoid circular lazy-initialization dependencies.
/// </summary>
internal bool HasAccessorOnlyParameters(InputClient parentInputClient)
Comment thread
JoshLove-msft marked this conversation as resolved.
{
var parentParamNames = parentInputClient.Parameters
.Select(p => p.Name)
.ToHashSet(StringComparer.OrdinalIgnoreCase);

return _inputClient.Parameters
.Any(p => !p.IsApiVersion &&
!(p is InputEndpointParameter ep && ep.IsEndpoint) &&
!parentParamNames.Contains(p.Name));
}

private Lazy<IReadOnlyList<ParameterProvider>> _clientParameters;
internal IReadOnlyList<ParameterProvider> ClientParameters => _clientParameters.Value;
private IReadOnlyList<ParameterProvider> GetClientParameters()
Expand Down Expand Up @@ -397,7 +415,10 @@ protected override FieldProvider[] BuildFields()
// add sub-client caching fields
foreach (var subClient in _subClients.Value)
{
if (subClient._clientCachingField != null)
// Only add caching field when the accessor does not require additional parameters.
// If the subclient has parameters that are not on the parent, each accessor call may
// produce a different client instance, so caching is not appropriate.
if (subClient._clientCachingField != null && !subClient.HasAccessorOnlyParameters(_inputClient))
{
fields.Add(subClient._clientCachingField);
}
Expand Down Expand Up @@ -899,8 +920,21 @@ protected override ScmMethodProvider[] BuildMethods()

var cachedClientFieldVar = new VariableExpression(subClient.Type, subClient._clientCachingField.Declaration, IsRef: true);
List<ValueExpression> subClientConstructorArgs = new(3);

// Populate constructor arguments
List<ParameterProvider> accessorMethodParams = [];

// Determine which subclient parameters are "extra" — present in the subclient's
// InputClient.Parameters but not in the parent's InputClient.Parameters.
// Only these should be exposed as accessor method parameters.
var parentInputParamNames = _inputClient.Parameters
.Select(p => p.Name)
.ToHashSet(StringComparer.OrdinalIgnoreCase);
var subClientExtraInputParamNames = subClient._inputClient.Parameters
Comment thread
JoshLove-msft marked this conversation as resolved.
Outdated
.Where(p => !p.IsApiVersion && !(p is InputEndpointParameter ep && ep.IsEndpoint))
.Where(p => !parentInputParamNames.Contains(p.Name))
.Select(p => p.Name)
.ToHashSet(StringComparer.OrdinalIgnoreCase);

// Populate constructor arguments, collecting extra params for the accessor method signature
foreach (var param in subClient._subClientInternalConstructorParams.Value)
{
if (parentClientProperties.TryGetValue(param.Name, out var parentProperty))
Expand All @@ -920,30 +954,59 @@ protected override ScmMethodProvider[] BuildMethods()
subClientConstructorArgs.Add(correspondingApiVersionField.Field);
}
}
else if (subClientExtraInputParamNames.Contains(param.Name))
{
// This parameter is subclient-specific and not available on the parent --
// expose it as an accessor method parameter.
accessorMethodParams.Add(param);
subClientConstructorArgs.Add(param);
}
// else: infra param (pipeline, auth, endpoint) not found in parent mock — silently skip
}

// Create the interlocked compare exchange expression for the body
var interlockedCompareExchange = Static(typeof(Interlocked)).Invoke(
nameof(Interlocked.CompareExchange),
[cachedClientFieldVar, New.Instance(subClient.Type, subClientConstructorArgs), Null]);
var factoryMethodName = subClient.Name.EndsWith(ClientSuffix, StringComparison.OrdinalIgnoreCase)
? $"Get{subClient.Name}"
: $"Get{subClient.Name}{ClientSuffix}";

var factoryMethod = new ScmMethodProvider(
new(
factoryMethodName,
$"Initializes a new instance of {subClient.Type.Name}",
MethodSignatureModifiers.Public | MethodSignatureModifiers.Virtual,
subClient.Type,
null,
[]),
// return Volatile.Read(ref _cachedClient) ?? Interlocked.CompareExchange(ref _cachedClient, new Client(_pipeline, _keyCredential, _endpoint), null) ?? _cachedClient;
Return(
Static(typeof(Volatile)).Invoke(nameof(Volatile.Read), cachedClientFieldVar)
.NullCoalesce(interlockedCompareExchange.NullCoalesce(subClient._clientCachingField))),
this,
ScmMethodKind.Convenience);
ScmMethodProvider factoryMethod;
if (accessorMethodParams.Count > 0)
{
// When the accessor requires extra parameters, caching is not appropriate
// (different parameter values may produce different client instances).
// Return a new instance directly.
factoryMethod = new ScmMethodProvider(
new(
factoryMethodName,
$"Initializes a new instance of {subClient.Type.Name}",
MethodSignatureModifiers.Public | MethodSignatureModifiers.Virtual,
subClient.Type,
null,
[.. accessorMethodParams]),
Return(New.Instance(subClient.Type, subClientConstructorArgs)),
this,
ScmMethodKind.Convenience);
}
else
{
// No extra params - use the existing caching pattern
var interlockedCompareExchange = Static(typeof(Interlocked)).Invoke(
nameof(Interlocked.CompareExchange),
[cachedClientFieldVar, New.Instance(subClient.Type, subClientConstructorArgs), Null]);
factoryMethod = new ScmMethodProvider(
new(
factoryMethodName,
$"Initializes a new instance of {subClient.Type.Name}",
MethodSignatureModifiers.Public | MethodSignatureModifiers.Virtual,
subClient.Type,
null,
[]),
// return Volatile.Read(ref _cachedClient) ?? Interlocked.CompareExchange(ref _cachedClient, new Client(_pipeline, _keyCredential, _endpoint), null) ?? _cachedClient;
Return(
Static(typeof(Volatile)).Invoke(nameof(Volatile.Read), cachedClientFieldVar)
.NullCoalesce(interlockedCompareExchange.NullCoalesce(subClient._clientCachingField))),
this,
ScmMethodKind.Convenience);
}
methods.Add(factoryMethod);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,116 @@ public void TestBuildMethods_ForParent_InitializedByBoth_HasSubClientAccessor()
Assert.IsNotNull(cachingField, "Parent should have caching field for subclient with InitializedBy.Individually | Parent");
}

[Test]
public void TestBuildMethods_ForParent_InitializedByBoth_WithSubClientParams_HasParameterizedAccessor()
{
var parentClient = InputFactory.Client("ParentClient");
var subClientParam = InputFactory.PathParameter("resourceId", InputPrimitiveType.String, scope: InputParameterScope.Client);
var subClient = InputFactory.Client(
"SubClient",
parent: parentClient,
parameters: [subClientParam],
initializedBy: InputClientInitializedBy.Individually | InputClientInitializedBy.Parent);

MockHelpers.LoadMockGenerator(
clients: () => [parentClient]);

var parentProvider = new ClientProvider(parentClient);

Assert.IsNotNull(parentProvider);

// The parent should have a factory method for the subclient
var factoryMethod = parentProvider.Methods.FirstOrDefault(
m => m.Signature?.Name == "GetSubClient" || m.Signature?.Name == "GetSubClientClient");
Assert.IsNotNull(factoryMethod, "Parent should have factory method for subclient with parameters");

// The accessor method should include the subclient's extra parameters
Assert.IsNotNull(factoryMethod!.Signature, "Factory method should have a signature");
Assert.AreEqual(1, factoryMethod.Signature!.Parameters.Count,
"Accessor method should include subclient parameters not present on parent");
Assert.AreEqual("resourceId", factoryMethod.Signature.Parameters[0].Name,
"Accessor method parameter should be the subclient's extra parameter");
}

[Test]
public void TestBuildFields_ForParent_InitializedByBoth_WithSubClientParams_NoCachingField()
{
var parentClient = InputFactory.Client("ParentClient");
var subClientParam = InputFactory.PathParameter("resourceId", InputPrimitiveType.String, scope: InputParameterScope.Client);
var subClient = InputFactory.Client(
"SubClient",
parent: parentClient,
parameters: [subClientParam],
initializedBy: InputClientInitializedBy.Individually | InputClientInitializedBy.Parent);

MockHelpers.LoadMockGenerator(
clients: () => [parentClient]);

var parentProvider = new ClientProvider(parentClient);

Assert.IsNotNull(parentProvider);

// The parent should NOT have a caching field for the subclient when the accessor requires parameters,
// since caching is not appropriate when different parameter values produce different client instances.
var cachingField = parentProvider.Fields.FirstOrDefault(f => f.Name == "_cachedSubClient");
Assert.IsNull(cachingField, "Parent should not have caching field for subclient that has subclient-specific parameters in its accessor");
}

[Test]
public void TestBuildMethods_ForParent_InitializedByParentOnly_WithSubClientParams_HasParameterizedAccessor()
{
var parentClient = InputFactory.Client("ParentClient");
var subClientParam = InputFactory.PathParameter("resourceId", InputPrimitiveType.String, scope: InputParameterScope.Client);
var subClient = InputFactory.Client(
"SubClient",
parent: parentClient,
parameters: [subClientParam],
initializedBy: InputClientInitializedBy.Parent);

MockHelpers.LoadMockGenerator(
clients: () => [parentClient]);

var parentProvider = new ClientProvider(parentClient);

Assert.IsNotNull(parentProvider);

// The parent should have a factory method for the subclient
var factoryMethod = parentProvider.Methods.FirstOrDefault(
m => m.Signature?.Name == "GetSubClient" || m.Signature?.Name == "GetSubClientClient");
Assert.IsNotNull(factoryMethod, "Parent should have factory method for subclient with parameters");

// The accessor method should include the subclient's extra parameters
Assert.IsNotNull(factoryMethod!.Signature, "Factory method should have a signature");
Assert.AreEqual(1, factoryMethod.Signature!.Parameters.Count,
"Accessor method should include subclient parameters not present on parent");
Assert.AreEqual("resourceId", factoryMethod.Signature.Parameters[0].Name,
"Accessor method parameter should be the subclient's extra parameter");
}

[Test]
public void TestBuildFields_ForParent_InitializedByParentOnly_WithSubClientParams_NoCachingField()
{
var parentClient = InputFactory.Client("ParentClient");
var subClientParam = InputFactory.PathParameter("resourceId", InputPrimitiveType.String, scope: InputParameterScope.Client);
var subClient = InputFactory.Client(
"SubClient",
parent: parentClient,
parameters: [subClientParam],
initializedBy: InputClientInitializedBy.Parent);

MockHelpers.LoadMockGenerator(
clients: () => [parentClient]);

var parentProvider = new ClientProvider(parentClient);

Assert.IsNotNull(parentProvider);

// The parent should NOT have a caching field for the subclient when the accessor requires parameters,
// since caching is not appropriate when different parameter values produce different client instances.
var cachingField = parentProvider.Fields.FirstOrDefault(f => f.Name == "_cachedSubClient");
Assert.IsNull(cachingField, "Parent should not have caching field for subclient that has subclient-specific parameters in its accessor");
}

private void ValidatePrimaryConstructor(
ConstructorProvider primaryPublicConstructor,
List<InputParameter> inputParameters,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ internal PipelineMessage CreateGetWidgetMetricsRequest(string day, RequestOption
{
ClientUriBuilder uri = new ClientUriBuilder();
uri.Reset(_endpoint);
uri.AppendPath("/metrics/widgets/daysOfWeek/", false);
uri.AppendPath("/metrics/", false);
uri.AppendPath(_metricsNamespace, true);
uri.AppendPath("/widgets/daysOfWeek/", false);
uri.AppendPath(day, true);
PipelineMessage message = Pipeline.CreateMessage(uri.ToUri(), "GET", PipelineMessageClassifier200);
PipelineRequest request = message.Request;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace SampleTypeSpec
public partial class Metrics
{
private readonly Uri _endpoint;
private readonly string _metricsNamespace;

/// <summary> Initializes a new instance of Metrics for mocking. </summary>
protected Metrics()
Expand All @@ -26,30 +27,38 @@ protected Metrics()
/// <summary> Initializes a new instance of Metrics. </summary>
/// <param name="pipeline"> The HTTP pipeline for sending and receiving REST requests and responses. </param>
/// <param name="endpoint"> Service endpoint. </param>
internal Metrics(ClientPipeline pipeline, Uri endpoint)
/// <param name="metricsNamespace"></param>
internal Metrics(ClientPipeline pipeline, Uri endpoint, string metricsNamespace)
{
_endpoint = endpoint;
Pipeline = pipeline;
_metricsNamespace = metricsNamespace;
}

/// <summary> Initializes a new instance of Metrics. </summary>
/// <param name="endpoint"> Service endpoint. </param>
/// <exception cref="ArgumentNullException"> <paramref name="endpoint"/> is null. </exception>
public Metrics(Uri endpoint) : this(endpoint, new SampleTypeSpecClientOptions())
/// <param name="metricsNamespace"></param>
/// <exception cref="ArgumentNullException"> <paramref name="endpoint"/> or <paramref name="metricsNamespace"/> is null. </exception>
/// <exception cref="ArgumentException"> <paramref name="metricsNamespace"/> is an empty string, and was expected to be non-empty. </exception>
public Metrics(Uri endpoint, string metricsNamespace) : this(endpoint, metricsNamespace, new SampleTypeSpecClientOptions())
{
}

/// <summary> Initializes a new instance of Metrics. </summary>
/// <param name="endpoint"> Service endpoint. </param>
/// <param name="metricsNamespace"></param>
/// <param name="options"> The options for configuring the client. </param>
/// <exception cref="ArgumentNullException"> <paramref name="endpoint"/> is null. </exception>
public Metrics(Uri endpoint, SampleTypeSpecClientOptions options)
/// <exception cref="ArgumentNullException"> <paramref name="endpoint"/> or <paramref name="metricsNamespace"/> is null. </exception>
/// <exception cref="ArgumentException"> <paramref name="metricsNamespace"/> is an empty string, and was expected to be non-empty. </exception>
public Metrics(Uri endpoint, string metricsNamespace, SampleTypeSpecClientOptions options)
{
Argument.AssertNotNull(endpoint, nameof(endpoint));
Argument.AssertNotNullOrEmpty(metricsNamespace, nameof(metricsNamespace));

options ??= new SampleTypeSpecClientOptions();

_endpoint = endpoint;
_metricsNamespace = metricsNamespace;
Pipeline = ClientPipeline.Create(options, Array.Empty<PipelinePolicy>(), new PipelinePolicy[] { new UserAgentPolicy(typeof(Metrics).Assembly) }, Array.Empty<PipelinePolicy>());
}

Expand Down
Loading
Loading