Skip to content

Fix duplicate API version field names for multi-service clients with colliding namespaces #10061

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JoshLove-msft merged 7 commits into from
Mar 17, 2026
Merged

Fix duplicate API version field names for multi-service clients with colliding namespaces #10061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b566c8b
Initial plan
Copilot Mar 17, 2026
f402ec5
fix: deduplicate API version field names for multi-service clients wi…
Copilot Mar 17, 2026
543e34e
test: improve assertions for multi-service same-namespace regression …
Copilot Mar 17, 2026
d1fe573
refactor: use full namespace for multi-service API version property n…
Copilot Mar 17, 2026
86495a8
fix: use full namespace for service version enum names in ApiVersionE…
Copilot Mar 17, 2026
46a2f0e
test: remove redundant assertion in service version enum collision test
Copilot Mar 17, 2026
b8b550e
chore: remove unused BuildNameForService helper and its tests
Copilot Mar 17, 2026
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
10 changes: 4 additions & 6 deletions ...generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientOptionsProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using Microsoft.TypeSpec.Generator.Input.Extensions;
using Microsoft.TypeSpec.Generator.Primitives;
using Microsoft.TypeSpec.Generator.Providers;
using Microsoft.TypeSpec.Generator.Shared;
using Microsoft.TypeSpec.Generator.Snippets;
using Microsoft.TypeSpec.Generator.Statements;
using Microsoft.TypeSpec.Generator.Utilities;
Expand Down Expand Up @@ -134,8 +133,10 @@ private static bool UseSingletonInstance(InputClient inputClient)
var properties = new Dictionary<EnumProvider, PropertyProvider>(_serviceVersionsEnums.Count);
foreach (var (inputEnum, enumProvider) in _serviceVersionsEnums)
{
// For multi-service clients, use the full namespace to guarantee uniqueness
// (the last segment alone can collide when services share a namespace).
var versionPropertyName = _inputClient.IsMultiServiceClient
? ClientHelper.BuildNameForService(inputEnum.Namespace, ServicePrefix, ApiVersionSuffix)
? $"{inputEnum.Namespace.ToIdentifierName()}{ApiVersionSuffix}"
: VersionSuffix;

var versionProperty = new PropertyProvider(
Expand Down Expand Up @@ -256,10 +257,7 @@ protected override ConstructorProvider[] BuildConstructors()
FormattableString versionParamDescription = $"The service version";
if (_inputClient.IsMultiServiceClient)
{
versionParameterName = ClientHelper.BuildNameForService(
serviceVersionEnum.Name,
ServicePrefix,
VersionSuffix).ToVariableName();
versionParameterName = serviceVersionEnum.Name.ToVariableName();
versionParamDescription = $"The {serviceVersionEnum.Name} service version";
}

Expand Down
41 changes: 41 additions & 0 deletions ...tor/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/ClientOptionsProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,47 @@ public void MultiServiceCombinedClient_WithThreeServices_GeneratesExpectedClient
Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content);
}

[Test]
public void MultiServiceClient_SameLastSegment_ProducesUniqueVersionEnums()
{
// Regression test: when two services have different full namespaces but the same last
// segment, the generated service version enums should still have distinct names.
List<string> serviceOneVersions = ["1.0", "2.0"];
List<string> serviceTwoVersions = ["3.0", "4.0"];

var serviceOneEnumValues = serviceOneVersions.Select(a => (a, a));
var serviceTwoEnumValues = serviceTwoVersions.Select(a => (a, a));

// Different full namespaces, same last segment ("Tests")
var serviceOneEnum = InputFactory.StringEnum(
"ServiceOneVersions",
serviceOneEnumValues,
usage: InputModelTypeUsage.ApiVersionEnum,
clientNamespace: "Azure.ServiceOne.Tests");
var serviceTwoEnum = InputFactory.StringEnum(
"ServiceTwoVersions",
serviceTwoEnumValues,
usage: InputModelTypeUsage.ApiVersionEnum,
clientNamespace: "Azure.ServiceTwo.Tests");

var client = InputFactory.Client("TestClient", isMultiServiceClient: true);

MockHelpers.LoadMockGenerator(
apiVersions: () => [.. serviceOneVersions, .. serviceTwoVersions],
clients: () => [client],
inputEnums: () => [serviceOneEnum, serviceTwoEnum]);

var clientProvider = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(client);
var clientOptionsProvider = clientProvider?.ClientOptions;

Assert.IsNotNull(clientOptionsProvider);

// Validate nested service version enums have unique names
var nestedTypes = clientOptionsProvider!.NestedTypes;
Assert.AreEqual(2, nestedTypes.Count);
CollectionAssert.AllItemsAreUnique(nestedTypes.Select(t => t.Name).ToList());
}

[Test]
public void TestConfigurationSectionConstructorBody_WithBoolProperty()
{
Expand Down
83 changes: 79 additions & 4 deletions ...soft.TypeSpec.Generator.ClientModel/test/Providers/ClientProviders/ClientProviderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3678,12 +3678,12 @@ public void GetApiVersionFieldForService_MultiService_ReturnsMatchingField()
// Should return the matching field for ServiceA
var fieldA = clientProvider!.GetApiVersionFieldForService("Sample.ServiceA");
Assert.IsNotNull(fieldA);
Assert.AreEqual("_serviceAApiVersion", fieldA!.Name);
Assert.AreEqual("_sampleServiceAApiVersion", fieldA!.Name);

// Should return the matching field for ServiceB
var fieldB = clientProvider.GetApiVersionFieldForService("Sample.ServiceB");
Assert.IsNotNull(fieldB);
Assert.AreEqual("_serviceBApiVersion", fieldB!.Name);
Assert.AreEqual("_sampleServiceBApiVersion", fieldB!.Name);
}

[Test]
Expand Down Expand Up @@ -3814,11 +3814,86 @@ public void GetApiVersionFieldForService_MultiService_CaseInsensitiveMatch()
// Should match case-insensitively
var fieldLowerCase = clientProvider!.GetApiVersionFieldForService("sample.serviceA");
Assert.IsNotNull(fieldLowerCase);
Assert.AreEqual("_serviceAApiVersion", fieldLowerCase!.Name);
Assert.AreEqual("_sampleServiceAApiVersion", fieldLowerCase!.Name);

var fieldUpperCase = clientProvider.GetApiVersionFieldForService("SAMPLE.SERVICEa");
Assert.IsNotNull(fieldUpperCase);
Assert.AreEqual("_serviceAApiVersion", fieldUpperCase!.Name);
Assert.AreEqual("_sampleServiceAApiVersion", fieldUpperCase!.Name);
}

[Test]
public void GetApiVersionFieldForService_MultiService_SameLastSegment_ProducesUniqueFields()
{
// Regression test: when two services have different full namespaces but the same last
// segment, using only the last segment would produce duplicate field names. The fix
// uses the full namespace to guarantee uniqueness.
List<string> serviceOneVersions = ["1.0", "2.0"];
List<string> serviceTwoVersions = ["3.0", "4.0"];

var serviceOneEnumValues = serviceOneVersions.Select(a => (a, a));
var serviceTwoEnumValues = serviceTwoVersions.Select(a => (a, a));

// Different full namespaces, same last segment ("Tests") — would collide with last-segment-only naming
var serviceOneEnum = InputFactory.StringEnum(
"ServiceOneVersions",
serviceOneEnumValues,
usage: InputModelTypeUsage.ApiVersionEnum,
clientNamespace: "Azure.ServiceOne.Tests");
var serviceTwoEnum = InputFactory.StringEnum(
"ServiceTwoVersions",
serviceTwoEnumValues,
usage: InputModelTypeUsage.ApiVersionEnum,
clientNamespace: "Azure.ServiceTwo.Tests");

InputParameter apiVersionParameter = InputFactory.QueryParameter(
"apiVersion",
InputPrimitiveType.String,
isRequired: true,
scope: InputParameterScope.Client,
isApiVersion: true);

var serviceOneOperation = InputFactory.Operation(
"ServiceOneOperation",
parameters: [apiVersionParameter],
ns: "Azure.ServiceOne.Tests");

var serviceTwoOperation = InputFactory.Operation(
"ServiceTwoOperation",
parameters: [apiVersionParameter],
ns: "Azure.ServiceTwo.Tests");

var client = InputFactory.Client(
TestClientName,
methods:
[
InputFactory.BasicServiceMethod("ServiceOneMethod", serviceOneOperation),
InputFactory.BasicServiceMethod("ServiceTwoMethod", serviceTwoOperation)
],
parameters: [apiVersionParameter],
isMultiServiceClient: true);

MockHelpers.LoadMockGenerator(
apiVersions: () => [.. serviceOneVersions, .. serviceTwoVersions],
clients: () => [client],
inputEnums: () => [serviceOneEnum, serviceTwoEnum]);

var clientProvider = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(client);
Assert.IsNotNull(clientProvider);

// This should not crash — previously it threw due to duplicate field names
Assert.DoesNotThrow(() => _ = clientProvider!.Fields);

// Verify we have two distinct api version fields using the full namespace
var apiVersionFields = clientProvider!.Fields
.Where(f => f.Name.Contains("ApiVersion", StringComparison.OrdinalIgnoreCase))
.OrderBy(f => f.Name)
.ToList();
Assert.AreEqual(2, apiVersionFields.Count);
Assert.AreNotEqual(apiVersionFields[0].Name, apiVersionFields[1].Name);

// Full namespace produces unique names: "Azure.ServiceOne.Tests" → "AzureServiceOneTests"
Assert.AreEqual("_azureServiceOneTestsApiVersion", apiVersionFields[0].Name);
Assert.AreEqual("_azureServiceTwoTestsApiVersion", apiVersionFields[1].Name);
}

[TestCase("{endpoint}")]
Expand Down
12 changes: 6 additions & 6 deletions ...lientProviders/TestData/ClientProviderTests/MultiServiceClient_GeneratesExpectedClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ public partial class TestClient
{
private readonly global::System.Uri _endpoint;
private readonly string _subscriptionId;
private readonly string _serviceAApiVersion;
private readonly string _serviceBApiVersion;
private readonly string _sampleServiceAApiVersion;
private readonly string _sampleServiceBApiVersion;
private global::Sample.ServiceA.ServiceA _cachedServiceA;
private global::Sample.ServiceB.ServiceB _cachedServiceB;

Expand Down Expand Up @@ -44,8 +44,8 @@ internal TestClient(global::System.ClientModel.Primitives.AuthenticationPolicy a
{
Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty<global::System.ClientModel.Primitives.PipelinePolicy>(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly) }, Array.Empty<global::System.ClientModel.Primitives.PipelinePolicy>());
}
_serviceAApiVersion = options.ServiceAApiVersion;
_serviceBApiVersion = options.ServiceBApiVersion;
_sampleServiceAApiVersion = options.SampleServiceAApiVersion;
_sampleServiceBApiVersion = options.SampleServiceBApiVersion;
}

public TestClient(global::System.Uri endpoint, string subscriptionId, global::Sample.TestClientOptions options) : this(null, endpoint, subscriptionId, options)
Expand All @@ -56,12 +56,12 @@ public TestClient(global::System.Uri endpoint, string subscriptionId, global::Sa

public virtual global::Sample.ServiceA.ServiceA GetServiceAClient()
{
return (global::System.Threading.Volatile.Read(ref _cachedServiceA) ?? (global::System.Threading.Interlocked.CompareExchange(ref _cachedServiceA, new global::Sample.ServiceA.ServiceA(Pipeline, _endpoint, _serviceAApiVersion, _subscriptionId), null) ?? _cachedServiceA));
return (global::System.Threading.Volatile.Read(ref _cachedServiceA) ?? (global::System.Threading.Interlocked.CompareExchange(ref _cachedServiceA, new global::Sample.ServiceA.ServiceA(Pipeline, _endpoint, _sampleServiceAApiVersion, _subscriptionId), null) ?? _cachedServiceA));
}

public virtual global::Sample.ServiceB.ServiceB GetServiceBClient()
{
return (global::System.Threading.Volatile.Read(ref _cachedServiceB) ?? (global::System.Threading.Interlocked.CompareExchange(ref _cachedServiceB, new global::Sample.ServiceB.ServiceB(Pipeline, _endpoint, _serviceBApiVersion, _subscriptionId), null) ?? _cachedServiceB));
return (global::System.Threading.Volatile.Read(ref _cachedServiceB) ?? (global::System.Threading.Interlocked.CompareExchange(ref _cachedServiceB, new global::Sample.ServiceB.ServiceB(Pipeline, _endpoint, _sampleServiceBApiVersion, _subscriptionId), null) ?? _cachedServiceB));
}
}
}
18 changes: 9 additions & 9 deletions ...tData/ClientProviderTests/MultiServiceClient_WithThreeServices_GeneratesExpectedClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ public partial class TestClient
{
private readonly global::System.Uri _endpoint;
private readonly string _subscriptionId;
private readonly string _serviceComputeApiVersion;
private readonly string _serviceKeyVaultApiVersion;
private readonly string _serviceStorageApiVersion;
private readonly string _sampleComputeApiVersion;
private readonly string _sampleKeyVaultApiVersion;
private readonly string _sampleStorageApiVersion;
private global::Sample.KeyVault.KeyVault _cachedKeyVault;
private global::Sample.Storage.Storage _cachedStorage;
private global::Sample.Compute.Compute _cachedCompute;
Expand Down Expand Up @@ -47,9 +47,9 @@ internal TestClient(global::System.ClientModel.Primitives.AuthenticationPolicy a
{
Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty<global::System.ClientModel.Primitives.PipelinePolicy>(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly) }, Array.Empty<global::System.ClientModel.Primitives.PipelinePolicy>());
}
_serviceComputeApiVersion = options.ServiceComputeApiVersion;
_serviceKeyVaultApiVersion = options.ServiceKeyVaultApiVersion;
_serviceStorageApiVersion = options.ServiceStorageApiVersion;
_sampleComputeApiVersion = options.SampleComputeApiVersion;
_sampleKeyVaultApiVersion = options.SampleKeyVaultApiVersion;
_sampleStorageApiVersion = options.SampleStorageApiVersion;
}

public TestClient(global::System.Uri endpoint, string subscriptionId, global::Sample.TestClientOptions options) : this(null, endpoint, subscriptionId, options)
Expand All @@ -60,17 +60,17 @@ public TestClient(global::System.Uri endpoint, string subscriptionId, global::Sa

public virtual global::Sample.KeyVault.KeyVault GetKeyVaultClient()
{
return (global::System.Threading.Volatile.Read(ref _cachedKeyVault) ?? (global::System.Threading.Interlocked.CompareExchange(ref _cachedKeyVault, new global::Sample.KeyVault.KeyVault(Pipeline, _endpoint, _serviceKeyVaultApiVersion, _subscriptionId), null) ?? _cachedKeyVault));
return (global::System.Threading.Volatile.Read(ref _cachedKeyVault) ?? (global::System.Threading.Interlocked.CompareExchange(ref _cachedKeyVault, new global::Sample.KeyVault.KeyVault(Pipeline, _endpoint, _sampleKeyVaultApiVersion, _subscriptionId), null) ?? _cachedKeyVault));
}

public virtual global::Sample.Storage.Storage GetStorageClient()
{
return (global::System.Threading.Volatile.Read(ref _cachedStorage) ?? (global::System.Threading.Interlocked.CompareExchange(ref _cachedStorage, new global::Sample.Storage.Storage(Pipeline, _endpoint, _serviceStorageApiVersion, _subscriptionId), null) ?? _cachedStorage));
return (global::System.Threading.Volatile.Read(ref _cachedStorage) ?? (global::System.Threading.Interlocked.CompareExchange(ref _cachedStorage, new global::Sample.Storage.Storage(Pipeline, _endpoint, _sampleStorageApiVersion, _subscriptionId), null) ?? _cachedStorage));
}

public virtual global::Sample.Compute.Compute GetComputeClient()
{
return (global::System.Threading.Volatile.Read(ref _cachedCompute) ?? (global::System.Threading.Interlocked.CompareExchange(ref _cachedCompute, new global::Sample.Compute.Compute(Pipeline, _endpoint, _serviceComputeApiVersion, _subscriptionId), null) ?? _cachedCompute));
return (global::System.Threading.Volatile.Read(ref _cachedCompute) ?? (global::System.Threading.Interlocked.CompareExchange(ref _cachedCompute, new global::Sample.Compute.Compute(Pipeline, _endpoint, _sampleComputeApiVersion, _subscriptionId), null) ?? _cachedCompute));
}
}
}
8 changes: 4 additions & 4 deletions ...viders/TestData/ClientProviderTests/MultiServiceCombinedClient_GeneratesExpectedClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ namespace Sample
public partial class TestClient
{
private readonly global::System.Uri _endpoint;
private readonly string _serviceAApiVersion;
private readonly string _serviceBApiVersion;
private readonly string _sampleServiceAApiVersion;
private readonly string _sampleServiceBApiVersion;

protected TestClient()
{
Expand All @@ -39,8 +39,8 @@ internal TestClient(global::System.ClientModel.Primitives.AuthenticationPolicy a
{
Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty<global::System.ClientModel.Primitives.PipelinePolicy>(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly) }, Array.Empty<global::System.ClientModel.Primitives.PipelinePolicy>());
}
_serviceAApiVersion = options.ServiceAApiVersion;
_serviceBApiVersion = options.ServiceBApiVersion;
_sampleServiceAApiVersion = options.SampleServiceAApiVersion;
_sampleServiceBApiVersion = options.SampleServiceBApiVersion;
}

public TestClient(global::System.Uri endpoint, global::Sample.TestClientOptions options) : this(null, endpoint, options)
Expand Down
12 changes: 6 additions & 6 deletions ...ientProviderTests/MultiServiceCombinedClient_WithThreeServices_GeneratesExpectedClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ namespace Sample
public partial class TestClient
{
private readonly global::System.Uri _endpoint;
private readonly string _serviceComputeApiVersion;
private readonly string _serviceKeyVaultApiVersion;
private readonly string _serviceStorageApiVersion;
private readonly string _sampleComputeApiVersion;
private readonly string _sampleKeyVaultApiVersion;
private readonly string _sampleStorageApiVersion;

protected TestClient()
{
Expand All @@ -40,9 +40,9 @@ internal TestClient(global::System.ClientModel.Primitives.AuthenticationPolicy a
{
Pipeline = global::System.ClientModel.Primitives.ClientPipeline.Create(options, Array.Empty<global::System.ClientModel.Primitives.PipelinePolicy>(), new global::System.ClientModel.Primitives.PipelinePolicy[] { new global::System.ClientModel.Primitives.UserAgentPolicy(typeof(global::Sample.TestClient).Assembly) }, Array.Empty<global::System.ClientModel.Primitives.PipelinePolicy>());
}
_serviceComputeApiVersion = options.ServiceComputeApiVersion;
_serviceKeyVaultApiVersion = options.ServiceKeyVaultApiVersion;
_serviceStorageApiVersion = options.ServiceStorageApiVersion;
_sampleComputeApiVersion = options.SampleComputeApiVersion;
_sampleKeyVaultApiVersion = options.SampleKeyVaultApiVersion;
_sampleStorageApiVersion = options.SampleStorageApiVersion;
}

public TestClient(global::System.Uri endpoint, global::Sample.TestClientOptions options) : this(null, endpoint, options)
Expand Down
Loading
Loading