From 4e6fdc055a03597591dec18b33da5091c3c5e019 Mon Sep 17 00:00:00 2001 From: jolov Date: Fri, 13 Feb 2026 19:02:52 -0800 Subject: [PATCH] wip --- .../src/Providers/ClientProvider.cs | 20 +++- .../src/Providers/RestClientProvider.cs | 65 +++++++------ .../Providers/ScmMethodProviderCollection.cs | 2 +- .../ListPageableTests.cs | 92 +++++++++++++++++++ .../MockableTestResource.cs | 18 ++++ 5 files changed, 167 insertions(+), 30 deletions(-) create mode 100644 packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/CollectionResultDefinitions/TestData/ListPageableTests/TopParameterPreservedViaBackCompatProvider/MockableTestResource.cs diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs index a87e1b5936a..0a9a9e2d5a2 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/ClientProvider.cs @@ -58,6 +58,13 @@ private record ApiVersionFields(FieldProvider Field, PropertyProvider? Correspon private Dictionary? _methodCache; private Dictionary MethodCache => _methodCache ??= []; + private TypeProvider? _backCompatProvider; + + /// + /// When set, this provider's is used for backward + /// compatibility checks on paging parameter names instead of this client's own LastContractView. + /// + internal TypeProvider? BackCompatProvider => _backCompatProvider; public ParameterProvider? ClientOptionsParameter { get; } @@ -835,8 +842,19 @@ [.. primaryCtorOrderedParams.Select(p => secondaryParamNames.Contains(p.Name) ? this); } - public ScmMethodProviderCollection GetMethodCollectionByOperation(InputOperation operation) + public ScmMethodProviderCollection GetMethodCollectionByOperation(InputOperation operation, TypeProvider? backCompatProvider = null) { + // Normalize: passing `this` is the same as no override. + var effective = backCompatProvider != null && backCompatProvider != this ? backCompatProvider : null; + if (_backCompatProvider != effective) + { + _backCompatProvider = effective; + // Clear caches so methods are rebuilt with the new backcompat provider. + // Since InputParameter objects are not mutated, Reset() is safe to call + // repeatedly — the rebuild will produce correct results each time. + Reset(); + _methodCache = null; + } _ = Methods; // Ensure methods are built return MethodCache[operation]; } diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs index b7d15ea32e8..c02e25a76e2 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/RestClientProvider.cs @@ -875,24 +875,30 @@ private static bool TryGetSpecialHeaderParam(InputParameter inputParameter, [Not : null; } - private static void UpdateParameterNameWithBackCompat(InputParameter inputParameter, string proposedName, ClientProvider client) + private static void UpdateParameterNameWithBackCompat(ParameterProvider parameter, string proposedName, TypeProvider backCompatProvider) { - // Check if the original parameter name exists in LastContractView for backward compatibility - var existingParam = client.LastContractView?.Methods - ?.SelectMany(method => method.Signature.Parameters) - .FirstOrDefault(p => string.Equals(p.Name, inputParameter.Name, StringComparison.OrdinalIgnoreCase)) - ?.Name; - - if (existingParam != null) + // Check if the original wire name exists in LastContractView for backward compatibility. + // We use WireInfo.SerializedName (the original wire name) to look up the parameter in the + // previous contract, since this is stable and never mutated. + var serializedName = parameter.WireInfo?.SerializedName; + if (serializedName != null) { - // Preserve the exact name (including casing) from the previous contract for backward compatibility - proposedName = existingParam; + var existingParamName = backCompatProvider.LastContractView?.Methods + ?.SelectMany(method => method.Signature.Parameters) + .FirstOrDefault(p => string.Equals(p.Name, serializedName, StringComparison.OrdinalIgnoreCase)) + ?.Name; + + if (existingParamName != null) + { + // Preserve the exact name (including casing) from the previous contract for backward compatibility + proposedName = existingParamName; + } } - // Use the updated name - if (!string.Equals(inputParameter.Name, proposedName, StringComparison.Ordinal)) + // Apply the updated name to the ParameterProvider (not the InputParameter) + if (!string.Equals(parameter.Name, proposedName, StringComparison.Ordinal)) { - inputParameter.Update(name: proposedName); + parameter.Update(name: proposedName); } } @@ -949,7 +955,8 @@ public MethodProvider GetCreateNextLinkRequestMethod(InputOperation operation) internal static List GetMethodParameters( InputServiceMethod serviceMethod, ScmMethodKind methodType, - ClientProvider client) + ClientProvider client, + TypeProvider? backCompatProvider = null) { SortedList sortedParams = []; int path = 0; @@ -1009,33 +1016,35 @@ internal static List GetMethodParameters( continue; } - // For paging operations, handle parameter name corrections with backward compatibility + ParameterProvider? parameter = ScmCodeModelGenerator.Instance.TypeFactory.CreateParameter(inputParam)?.ToPublicInputParameter(); + if (parameter is null) + { + continue; + } + + // For paging operations, handle parameter name corrections with backward compatibility. + // This is done after ParameterProvider creation so we don't mutate shared InputParameter objects. if (serviceMethod is InputPagingServiceMethod) { - // Rename "top" parameter to "maxCount" (with backward compatibility) - if (string.Equals(inputParam.Name, TopParameterName, StringComparison.OrdinalIgnoreCase)) + var backCompatTarget = backCompatProvider ?? client; + + // Rename "top" parameter to "maxCount" (with backward compatibility). + // Use SerializedName (the original wire name) which is stable and never mutated. + if (string.Equals(inputParam.SerializedName, TopParameterName, StringComparison.OrdinalIgnoreCase)) { - UpdateParameterNameWithBackCompat(inputParam, MaxCountParameterName, client); + UpdateParameterNameWithBackCompat(parameter, MaxCountParameterName, backCompatTarget); } // Ensure page size parameter uses the correct casing (with backward compatibility) - if (pageSizeParameterName != null && string.Equals(inputParam.Name, pageSizeParameterName, StringComparison.OrdinalIgnoreCase)) + if (pageSizeParameterName != null && string.Equals(inputParam.SerializedName, pageSizeParameterName, StringComparison.OrdinalIgnoreCase)) { var updatedPageSizeParameterName = pageSizeParameterName.Equals(MaxPageSizeParameterName, StringComparison.OrdinalIgnoreCase) ? MaxPageSizeParameterName : pageSizeParameterName; - // For page size parameters, normalize badly-cased "maxpagesize" variants to proper camelCase, but always - // respect backcompat. - UpdateParameterNameWithBackCompat(inputParam, updatedPageSizeParameterName, client); + UpdateParameterNameWithBackCompat(parameter, updatedPageSizeParameterName, backCompatTarget); } } - ParameterProvider? parameter = ScmCodeModelGenerator.Instance.TypeFactory.CreateParameter(inputParam)?.ToPublicInputParameter(); - if (parameter is null) - { - continue; - } - if (methodType is ScmMethodKind.Protocol or ScmMethodKind.CreateRequest) { if (inputParam is InputBodyParameter) 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 2d931e09f99..205c1eb2f4a 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 @@ -31,7 +31,7 @@ public class ScmMethodProviderCollection : IReadOnlyList private IList ProtocolMethodParameters => _protocolMethodParameters ??= RestClientProvider.GetMethodParameters(ServiceMethod, ScmMethodKind.Protocol, Client); private IList? _protocolMethodParameters; - private IReadOnlyList ConvenienceMethodParameters => _convenienceMethodParameters ??= RestClientProvider.GetMethodParameters(ServiceMethod, ScmMethodKind.Convenience, Client); + private IReadOnlyList ConvenienceMethodParameters => _convenienceMethodParameters ??= RestClientProvider.GetMethodParameters(ServiceMethod, ScmMethodKind.Convenience, Client, Client.BackCompatProvider); private IReadOnlyList? _convenienceMethodParameters; private readonly InputPagingServiceMethod? _pagingServiceMethod; private IReadOnlyList? _methods; diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/CollectionResultDefinitions/ListPageableTests.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/CollectionResultDefinitions/ListPageableTests.cs index b4d3827a2f9..bca60f9ffd9 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/CollectionResultDefinitions/ListPageableTests.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/CollectionResultDefinitions/ListPageableTests.cs @@ -163,6 +163,98 @@ public void NoNextLinkOrContinuationTokenOfTAsync() Assert.AreEqual(Helpers.GetExpectedFromFile(), file.Content); } + [Test] + public async Task TopParameterPreservedViaBackCompatProvider() + { + // This test verifies that when a different TypeProvider (e.g., MockableResourceProvider in mgmt) + // is passed as a backCompatProvider and has "top" in its LastContractView, the parameter name + // is preserved as "top" instead of being renamed to "maxCount". + var topParameter = InputFactory.QueryParameter("top", InputPrimitiveType.Int32, isRequired: false, serializedName: "top"); + + List parameters = [topParameter]; + List methodParameters = + [ + InputFactory.MethodParameter("top", InputPrimitiveType.Int32, isRequired: false, + location: InputRequestLocation.Query, serializedName: "top"), + ]; + + var inputModel = InputFactory.Model("Item", properties: + [ + InputFactory.Property("id", InputPrimitiveType.String, isRequired: true), + ]); + + var pagingMetadata = new InputPagingServiceMetadata( + ["items"], + new InputNextLink(null, ["nextLink"], InputResponseLocation.Body, []), + null, + null); + + var response = InputFactory.OperationResponse( + [200], + InputFactory.Model( + "PagedItems", + properties: [ + InputFactory.Property("items", InputFactory.Array(inputModel)), + InputFactory.Property("nextLink", InputPrimitiveType.Url) + ])); + + var operation = InputFactory.Operation("getItems", responses: [response], parameters: parameters); + var inputServiceMethod = InputFactory.PagingServiceMethod( + "getItems", + operation, + pagingMetadata: pagingMetadata, + parameters: methodParameters); + + var client = InputFactory.Client("testClient", methods: [inputServiceMethod]); + + var generator = await MockHelpers.LoadMockGeneratorAsync( + clients: () => [client], + lastContractCompilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var clientProvider = generator.Object.OutputLibrary.TypeProviders.OfType().FirstOrDefault(); + Assert.IsNotNull(clientProvider); + + // Without a backcompat provider, the parameter should be renamed to "maxCount" + var methodsWithoutBackCompat = clientProvider!.GetMethodCollectionByOperation(operation); + var convenienceMethod = methodsWithoutBackCompat[^2]; // sync convenience method + var maxCountParam = convenienceMethod.Signature.Parameters.FirstOrDefault(p => + string.Equals(p.Name, "maxCount", StringComparison.Ordinal)); + Assert.IsNotNull(maxCountParam, "Without backcompat provider, parameter should be 'maxCount'"); + + // With a backcompat provider whose LastContractView has "top", the name should be preserved + var backCompatProvider = new BackCompatTypeProvider("MockableTestResource", "Sample"); + Assert.IsNotNull(backCompatProvider.LastContractView, "BackCompat provider should have a LastContractView"); + + var methodsWithBackCompat = clientProvider.GetMethodCollectionByOperation(operation, backCompatProvider); + convenienceMethod = methodsWithBackCompat[^2]; + var topParam = convenienceMethod.Signature.Parameters.FirstOrDefault(p => + string.Equals(p.Name, "top", StringComparison.Ordinal)); + + Assert.IsNotNull(topParam, "With backcompat provider, parameter should be 'top' (preserved from LastContractView)"); + Assert.AreEqual("top", topParam!.Name, + "Parameter name should be 'top' (from backcompat provider's LastContractView), not 'maxCount'"); + } + + /// + /// A simple TypeProvider used to simulate a backcompat provider (e.g., MockableResourceProvider) + /// whose LastContractView contains previously published parameter names. + /// + private class BackCompatTypeProvider : TypeProvider + { + private readonly string _name; + private readonly string _namespace; + + public BackCompatTypeProvider(string name, string ns) + { + _name = name; + _namespace = ns; + } + + protected override string BuildRelativeFilePath() => $"{_name}.cs"; + protected override string BuildName() => _name; + protected override string BuildNamespace() => _namespace; + } + private static void CreatePagingOperation() { var inputModel = InputFactory.Model("cat", properties: diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/CollectionResultDefinitions/TestData/ListPageableTests/TopParameterPreservedViaBackCompatProvider/MockableTestResource.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/CollectionResultDefinitions/TestData/ListPageableTests/TopParameterPreservedViaBackCompatProvider/MockableTestResource.cs new file mode 100644 index 00000000000..a16b13865c6 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/CollectionResultDefinitions/TestData/ListPageableTests/TopParameterPreservedViaBackCompatProvider/MockableTestResource.cs @@ -0,0 +1,18 @@ +#nullable disable + +using System.ClientModel; +using System.ClientModel.Primitives; +using System.Threading.Tasks; + +namespace Sample +{ + /// + /// Represents the previous contract for the enclosing type (e.g., MockableResourceProvider) + /// that has the "top" parameter in its public API methods. + /// + public partial class MockableTestResource + { + public virtual Task GetItemsAsync(int? top, CancellationToken cancellationToken = default) { return null; } + public virtual ClientResult GetItems(int? top, CancellationToken cancellationToken = default) { return null; } + } +}