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 8b2ff5859b3..747e51bcc2f 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 @@ -1221,8 +1221,13 @@ internal static List GetMethodParameters( int optional = 400; var operation = serviceMethod.Operation; - // For convenience methods, use the service method parameters - var inputParameters = methodType is ScmMethodKind.Convenience ? serviceMethod.Parameters : operation.Parameters; + // Convenience methods use the service method parameters. The protocol method does too when + // @@override grouped the operation's parameters into an options bag, so both surfaces share + // the same shape (https://github.com/microsoft/typespec/issues/11214). + var inputParameters = methodType is ScmMethodKind.Convenience + || (methodType is ScmMethodKind.Protocol && ShouldGroupProtocolParameters(serviceMethod)) + ? serviceMethod.Parameters + : operation.Parameters; var pageSizeParameterName = GetPageSizeParameterName(serviceMethod as InputPagingServiceMethod); @@ -1307,7 +1312,7 @@ internal static List GetMethodParameters( if (methodType is ScmMethodKind.Protocol or ScmMethodKind.CreateRequest) { - if (inputParam is InputBodyParameter) + if (inputParam is InputBodyParameter || inputParam is InputMethodParameter { Location: InputRequestLocation.Body }) { if (methodType == ScmMethodKind.CreateRequest) { @@ -1393,6 +1398,89 @@ internal static List GetMethodParameters( return [.. sortedParams.Values]; } + /// + /// Determines whether the protocol method should adopt the grouped (options bag) parameter shape + /// produced by @@override. Grouping is skipped when the request body itself was folded into + /// the bag, because the protocol method must keep exposing the body as raw request content. + /// + internal static bool ShouldGroupProtocolParameters(InputServiceMethod serviceMethod) + { + bool hasGroupedParameter = false; + foreach (var parameter in serviceMethod.Operation.Parameters) + { + if (parameter.MethodParameterSegments is not { Count: > 1 } segments) + { + continue; + } + + // The bag is (or contains) the request body, so the protocol method has to stay flattened + // to keep accepting a raw payload. + if (parameter is InputBodyParameter + || segments[0] is InputMethodParameter { Location: InputRequestLocation.Body }) + { + return false; + } + + if (!SegmentsPreserveRequiredness(parameter, segments)) + { + return false; + } + + hasGroupedParameter = true; + } + + return hasGroupedParameter; + } + + /// + /// A required wire parameter must map to a required property so the bag's constructor forces callers + /// to supply it. TCGC does not validate this, and when it does not hold, grouping the protocol method + /// would silently drop the compile-time guarantee that the flattened signature provides. + /// + private static bool SegmentsPreserveRequiredness(InputParameter parameter, IReadOnlyList segments) + { + if (!parameter.IsRequired) + { + return true; + } + + var currentType = segments[0].Type; + for (int i = 1; i < segments.Count; i++) + { + if (currentType is not InputModelType model) + { + return false; + } + + var property = FindPropertyInHierarchy(model, segments[i].Name); + if (property is null || !property.IsRequired) + { + return false; + } + + currentType = property.Type; + } + + return true; + } + + private static InputModelProperty? FindPropertyInHierarchy(InputModelType model, string name) + { + for (var current = model; current != null; current = current.BaseModel) + { + foreach (var property in current.Properties) + { + if (property.SerializedName == name + || string.Equals(property.Name, name, StringComparison.OrdinalIgnoreCase)) + { + return property; + } + } + } + + return null; + } + private static bool HasLiteralContentTypeHeader(InputOperation operation) { foreach (var p in operation.Parameters) 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 d039522a128..2ccade53ab7 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 @@ -207,7 +207,16 @@ private ScmMethodProvider BuildConvenienceMethod(MethodProvider protocolMethod, if (_pagingServiceMethod != null) { collection = ScmCodeModelGenerator.Instance.TypeFactory.ClientResponseApi.CreateClientCollectionResultDefinition(Client, _pagingServiceMethod, responseBodyType, isAsync); - methodBody = [.. GetPagingMethodBody(collection, convenienceBodyParameters, true)]; + var createRequestSignature = Client.RestClient.GetCreateRequestMethod(ServiceMethod.Operation).Signature; + methodBody = + [ + .. GetPagingMethodBody( + collection, + createRequestSignature, + convenienceBodyParameters, + true, + [.. protocolMethod.Signature.Parameters]) + ]; } else if (responseBodyType is null) { @@ -1112,6 +1121,14 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod requestOptionsParameter = ScmKnownParameters.OptionalRequestOptions; } + // A grouped options bag adopted by the protocol method can carry the same name as the request + // options/context parameter, which would emit a duplicate parameter name. Rename the request + // options parameter when that happens. + requestOptionsParameter = ResolveRequestOptionsNameCollision( + requestOptionsParameter, + requiredParameters, + optionalParameters); + ParameterProvider[] parameters = [.. requiredParameters, .. optionalParameters, requestOptionsParameter]; var methodName = isAsync ? ServiceMethod.Name + "Async" : ServiceMethod.Name; @@ -1128,7 +1145,7 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod { // Partial methods cannot have optional parameters in the implementation. var requiredCustomParameters = PartialMethodCustomization.RenameAndCloneParameters( - customSignature.Parameters, + parameters, customSignature.Parameters, removeDefaults: true).ToArray(); @@ -1161,7 +1178,7 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod if (_pagingServiceMethod != null) { collection = ScmCodeModelGenerator.Instance.TypeFactory.ClientResponseApi.CreateClientCollectionResultDefinition(Client, _pagingServiceMethod, null, isAsync); - methodBody = [.. GetPagingMethodBody(collection, bodyParameters, false)]; + methodBody = [.. GetPagingMethodBody(collection, createRequestMethod.Signature, bodyParameters, false)]; } else { @@ -1196,6 +1213,38 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod return protocolMethod; } + private static ParameterProvider ResolveRequestOptionsNameCollision( + ParameterProvider requestOptionsParameter, + IReadOnlyList requiredParameters, + IReadOnlyList optionalParameters) + { + var otherParameters = requiredParameters.Concat(optionalParameters).ToList(); + if (!otherParameters.Any(p => string.Equals(p.Name, requestOptionsParameter.Name, StringComparison.OrdinalIgnoreCase))) + { + return requestOptionsParameter; + } + + var baseName = "request" + + char.ToUpperInvariant(requestOptionsParameter.Name[0]) + + requestOptionsParameter.Name.Substring(1); + var uniqueName = baseName; + var suffix = 1; + while (otherParameters.Any(p => string.Equals(p.Name, uniqueName, StringComparison.OrdinalIgnoreCase))) + { + uniqueName = baseName + suffix++; + } + + return new ParameterProvider( + uniqueName, + requestOptionsParameter.Description, + requestOptionsParameter.Type, + requestOptionsParameter.DefaultValue, + location: requestOptionsParameter.Location, + wireInfo: requestOptionsParameter.WireInfo, + validation: requestOptionsParameter.Validation, + inputParameter: requestOptionsParameter.InputParameter); + } + // The protocol method orders its parameters required-first (optional parameters and the // request options/context parameter are moved to the end so they can have default values). // This order can differ from the CreateRequest method's parameter order, which follows the @@ -1204,11 +1253,24 @@ private ScmMethodProvider BuildProtocolMethod(MethodProvider createRequestMethod // parameter with the request body). Reorder the arguments to match the CreateRequest // signature by mapping each CreateRequest parameter to the protocol parameter with the same // name. If the names cannot be reconciled, fall back to the original positional behavior. - private static ValueExpression[] BuildCreateRequestArguments( + private ValueExpression[] BuildCreateRequestArguments( MethodSignature createRequestSignature, IReadOnlyList bodyParameters) { var createRequestParameters = createRequestSignature.Parameters; + + // When the protocol method exposes an options bag, its parameters no longer line up with the + // CreateRequest method's flattened wire parameters. Expand each grouped parameter back out of + // the bag (e.g. `options.Top`) so CreateRequest still receives the individual values. + if (RestClientProvider.ShouldGroupProtocolParameters(ServiceMethod)) + { + var groupedArguments = BuildGroupedCreateRequestArguments(createRequestParameters, bodyParameters); + if (groupedArguments is not null) + { + return groupedArguments; + } + } + if (createRequestParameters.Count == bodyParameters.Count) { var arguments = new ValueExpression[createRequestParameters.Count]; @@ -1234,6 +1296,98 @@ private static ValueExpression[] BuildCreateRequestArguments( return [.. bodyParameters.Select(p => (ValueExpression)p)]; } + private static ValueExpression[]? BuildGroupedCreateRequestArguments( + IReadOnlyList createRequestParameters, + IReadOnlyList protocolParameters, + Func? getArgument = null) + { + getArgument ??= parameter => parameter; + var arguments = new ValueExpression[createRequestParameters.Count]; + + for (int i = 0; i < createRequestParameters.Count; i++) + { + var createRequestParameter = createRequestParameters[i]; + var segments = createRequestParameter.InputParameter?.MethodParameterSegments; + + if (segments is not { Count: > 1 }) + { + var match = FindUngroupedArgument(protocolParameters, createRequestParameter); + if (match is null) + { + return null; + } + + arguments[i] = getArgument(match); + continue; + } + + var groupParameter = protocolParameters.FirstOrDefault( + p => string.Equals(p.InputParameter?.Name, segments[0].Name, StringComparison.OrdinalIgnoreCase) + || string.Equals(p.Name, segments[0].Name, StringComparison.OrdinalIgnoreCase)); + if (groupParameter is null + || !ScmCodeModelGenerator.Instance.TypeFactory.CSharpTypeMap.TryGetValue(groupParameter.Type, out var typeProvider) + || typeProvider is not ModelProvider groupModel) + { + return null; + } + + var propertySegments = segments.Skip(1).Select(s => s.Name).ToList(); + var propertyExpression = groupModel.GetPropertyExpression(getArgument(groupParameter), propertySegments, out var leafProperty); + + // CreateRequest takes the serialized (string/number) form of an enum, so convert before forwarding. + if (leafProperty.Type.IsEnum && !createRequestParameter.Type.IsEnum) + { + if (leafProperty.Type.IsNullable) + { + propertyExpression = propertyExpression.NullConditional(); + } + propertyExpression = leafProperty.Type.ToSerial(propertyExpression); + } + + arguments[i] = propertyExpression; + } + + return arguments; + } + + /// + /// Locates the protocol parameter to forward for a CreateRequest parameter that was not + /// folded into the options bag. The name alone is not reliable: a bag named after the request + /// options parameter causes that parameter to be renamed, so the original name now resolves to + /// the bag. The type disambiguates in that case. + /// + private static ParameterProvider? FindUngroupedArgument( + IReadOnlyList protocolParameters, + ParameterProvider createRequestParameter) + { + foreach (var parameter in protocolParameters) + { + if (string.Equals(parameter.Name, createRequestParameter.Name, StringComparison.OrdinalIgnoreCase) + && parameter.Type.Equals(createRequestParameter.Type)) + { + return parameter; + } + } + + ParameterProvider? typeMatch = null; + foreach (var parameter in protocolParameters) + { + if (!parameter.Type.Equals(createRequestParameter.Type)) + { + continue; + } + + if (typeMatch is not null) + { + return null; + } + + typeMatch = parameter; + } + + return typeMatch; + } + private ParameterProvider ProcessOptionalParameters( List optionalParameters, List requiredParameters, @@ -1301,19 +1455,41 @@ private ParameterProvider ProcessOptionalParameters( private IEnumerable GetPagingMethodBody( TypeProvider collection, + MethodSignature createRequestSignature, IReadOnlyList parameters, - bool isConvenience) + bool isConvenience, + IReadOnlyList? pagingProtocolParameters = null) { if (isConvenience) { + var conversionStatements = GetStackVariablesForProtocolParamConversion(ConvenienceMethodParameters, out var declarations); + var protocolArguments = GetProtocolMethodArguments(declarations); + + IReadOnlyList constructorArguments = protocolArguments; + if (RestClientProvider.ShouldGroupProtocolParameters(ServiceMethod)) + { + var protocolParameters = pagingProtocolParameters + ?? throw new InvalidOperationException("Paging protocol parameters are required to expand grouped arguments."); + var argumentMap = new Dictionary(); + for (int i = 0; i < protocolParameters.Count; i++) + { + argumentMap[protocolParameters[i]] = protocolArguments[i]; + } + + constructorArguments = BuildGroupedCreateRequestArguments( + createRequestSignature.Parameters, + protocolParameters, + parameter => argumentMap[parameter]) ?? protocolArguments; + } + return [ - .. GetStackVariablesForProtocolParamConversion(ConvenienceMethodParameters, out var declarations), + .. conversionStatements, Return(New.Instance( collection.Type, [ This, - .. GetProtocolMethodArguments(declarations) + .. constructorArguments ])) ]; } @@ -1322,7 +1498,7 @@ .. GetProtocolMethodArguments(declarations) collection.Type, [ This, - .. parameters + .. BuildCreateRequestArguments(createRequestSignature, parameters) ])); } 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 c26f86c02c4..4ef1a0610f3 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 @@ -1729,6 +1729,14 @@ public async Task MethodParameterSegments_RenamedGroupedQueryParam_MapsByClientN // name ("bandIndex"). var methodBody = convenienceMethod!.BodyStatements!.ToDisplayString(); Assert.AreEqual(Helpers.GetExpectedFromFile(), methodBody); + + // The protocol method takes the same options bag, so the client-name lookup now happens + // when it expands the bag for the CreateRequest call. + var protocolMethod = methodCollection.FirstOrDefault(m => + m.Signature.Name == "GetPoint" && + m.Signature.Parameters.All(p => p.Type.Name != "CancellationToken")); + Assert.IsNotNull(protocolMethod); + Assert.That(protocolMethod!.BodyStatements!.ToDisplayString(), Does.Contain("options.BandIndex")); } [Test] @@ -1793,6 +1801,379 @@ public async Task MethodParameterSegments_EnumGroupedQueryParam_SerializesToProt var methodBody = convenienceMethod!.BodyStatements!.ToDisplayString(); Assert.AreEqual(Helpers.GetExpectedFromFile(), methodBody); + + // The protocol method takes the same options bag, so the enum serialization now happens + // when it expands the bag for the CreateRequest call, which still takes a string. + var protocolMethod = methodCollection.FirstOrDefault(m => + m.Signature.Name == "GetPoint" && + m.Signature.Parameters.All(p => p.Type.Name != "CancellationToken")); + Assert.IsNotNull(protocolMethod); + Assert.That(protocolMethod!.BodyStatements!.ToDisplayString(), Does.Contain("options.Resampling?.ToString()")); + } + + [Test] + public async Task OptionsBagOverride_AppliesToProtocolMethod() + { + // https://github.com/microsoft/typespec/issues/11214 + // When @@override groups an operation's parameters into an options bag, the protocol + // method should adopt the same grouped shape instead of listing every parameter. + var optionsModel = InputFactory.Model( + "GetWidgetOptions", + properties: + [ + InputFactory.Property("id", InputPrimitiveType.String, isRequired: true, isHttpMetadata: true, wireName: "id"), + InputFactory.Property("filter", InputPrimitiveType.String, isRequired: false, isHttpMetadata: true, wireName: "filter"), + InputFactory.Property("top", InputPrimitiveType.Int32, isRequired: false, isHttpMetadata: true, wireName: "top"), + ]); + + var optionsMethodParameter = InputFactory.MethodParameter( + "options", + optionsModel, + isRequired: true, + location: InputRequestLocation.Query); + + var idParam = InputFactory.PathParameter("id", InputPrimitiveType.String, isRequired: true); + idParam.Update(methodParameterSegments: + [ + optionsMethodParameter, + InputFactory.MethodParameter("id", InputPrimitiveType.String, isRequired: true), + ]); + var filterParam = InputFactory.QueryParameter("filter", InputPrimitiveType.String, isRequired: false, serializedName: "filter"); + filterParam.Update(methodParameterSegments: + [ + optionsMethodParameter, + InputFactory.MethodParameter("filter", InputPrimitiveType.String, isRequired: false), + ]); + var topParam = InputFactory.QueryParameter("top", InputPrimitiveType.Int32, isRequired: false, serializedName: "top"); + topParam.Update(methodParameterSegments: + [ + optionsMethodParameter, + InputFactory.MethodParameter("top", InputPrimitiveType.Int32, isRequired: false), + ]); + + var serviceMethod = InputFactory.BasicServiceMethod( + "GetWidget", + InputFactory.Operation( + "GetWidget", + parameters: [idParam, filterParam, topParam], + responses: [InputFactory.OperationResponse([200])]), + parameters: [optionsMethodParameter]); + + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + await MockHelpers.LoadMockGeneratorAsync(clients: () => [inputClient], inputModels: () => [optionsModel]); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + Assert.IsNotNull(client); + + var methodCollection = new ScmMethodProviderCollection(serviceMethod, client!); + var protocolMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Protocol && !m.Signature.Name.EndsWith("Async")); + Assert.IsNotNull(protocolMethod); + + var parameters = protocolMethod!.Signature.Parameters; + var actual = string.Join(", ", parameters.Select(p => $"{p.Type.Name} {p.Name}")); + + // Expected shape: (GetWidgetOptions options, RequestOptions ) + Assert.AreEqual(2, parameters.Count, $"protocol method should take the options bag, but was: ({actual})"); + Assert.AreEqual("GetWidgetOptions", parameters[0].Type.Name, $"actual: ({actual})"); + + // The trailing request options parameter must not collide with the options bag name. + Assert.AreNotEqual(parameters[0].Name, parameters[1].Name, $"actual: ({actual})"); + + // The protocol method expands the bag when calling CreateRequest, which still takes the + // individual wire parameters. + var protocolBody = protocolMethod.BodyStatements!.ToDisplayString(); + Assert.AreEqual(Helpers.GetExpectedFromFile("Protocol"), protocolBody); + + // The convenience method forwards the bag straight through rather than unpacking it. + var convenienceMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Convenience && !m.Signature.Name.EndsWith("Async")); + Assert.IsNotNull(convenienceMethod); + Assert.AreEqual( + Helpers.GetExpectedFromFile("Convenience"), + convenienceMethod!.BodyStatements!.ToDisplayString()); + + // The bag now carries parameters that used to be required method parameters, so its public + // constructor must still force callers to supply the required ones. + var optionsProvider = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(optionsModel); + Assert.IsNotNull(optionsProvider); + var publicCtor = optionsProvider!.Constructors.FirstOrDefault( + c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.IsNotNull(publicCtor, "options bag should expose a public constructor"); + var ctorParams = publicCtor!.Signature.Parameters.Select(p => p.Name).ToList(); + Assert.That(ctorParams, Does.Contain("id"), $"required property must be a required ctor arg, but ctor was: ({string.Join(", ", ctorParams)})"); + Assert.That(ctorParams, Does.Not.Contain("filter"), $"optional property must not be a ctor arg, but ctor was: ({string.Join(", ", ctorParams)})"); + Assert.That(ctorParams, Does.Not.Contain("top"), $"optional property must not be a ctor arg, but ctor was: ({string.Join(", ", ctorParams)})"); + } + + [Test] + public async Task OptionsBagOverride_PagingMethodExpandsBagForCollectionResult() + { + var optionsModel = InputFactory.Model( + "GetWidgetsOptions", + properties: + [ + InputFactory.Property("id", InputPrimitiveType.String, isRequired: true, isHttpMetadata: true, wireName: "id"), + InputFactory.Property("filter", InputPrimitiveType.String, isRequired: false, isHttpMetadata: true, wireName: "filter"), + ]); + var itemModel = InputFactory.Model( + "Widget", + properties: [InputFactory.Property("name", InputPrimitiveType.String, isRequired: true)]); + var optionsMethodParameter = InputFactory.MethodParameter( + "options", + optionsModel, + isRequired: true, + location: InputRequestLocation.Query); + + var idParam = InputFactory.PathParameter("id", InputPrimitiveType.String, isRequired: true); + idParam.Update(methodParameterSegments: + [ + optionsMethodParameter, + InputFactory.MethodParameter("id", InputPrimitiveType.String, isRequired: true), + ]); + var filterParam = InputFactory.QueryParameter("filter", InputPrimitiveType.String, isRequired: false, serializedName: "filter"); + filterParam.Update(methodParameterSegments: + [ + optionsMethodParameter, + InputFactory.MethodParameter("filter", InputPrimitiveType.String, isRequired: false), + ]); + + var response = InputFactory.OperationResponse( + [200], + InputFactory.Model( + "Page", + properties: [InputFactory.Property("items", InputFactory.Array(itemModel))])); + var serviceMethod = InputFactory.PagingServiceMethod( + "GetWidgets", + InputFactory.Operation("GetWidgets", parameters: [idParam, filterParam], responses: [response]), + parameters: [optionsMethodParameter], + pagingMetadata: InputFactory.PagingMetadata(["items"], null, null)); + + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + await MockHelpers.LoadMockGeneratorAsync( + clients: () => [inputClient], + inputModels: () => [optionsModel, itemModel]); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + Assert.IsNotNull(client); + + var methodCollection = new ScmMethodProviderCollection(serviceMethod, client!); + var protocolMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Protocol && !m.Signature.Name.EndsWith("Async")); + var convenienceMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Convenience && !m.Signature.Name.EndsWith("Async")); + Assert.IsNotNull(protocolMethod); + Assert.IsNotNull(convenienceMethod); + Assert.IsTrue(protocolMethod!.Signature.Parameters.Any(p => p.Type.Name == "GetWidgetsOptions")); + + Assert.AreEqual( + Helpers.GetExpectedFromFile("Protocol"), + protocolMethod.BodyStatements!.ToDisplayString()); + Assert.AreEqual( + Helpers.GetExpectedFromFile("Convenience"), + convenienceMethod!.BodyStatements!.ToDisplayString()); + } + + [Test] + public async Task OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping() + { + var optionsModel = InputFactory.Model( + "GetWidgetOptions", + properties: + [ + InputFactory.Property("id", InputPrimitiveType.String, isRequired: true, isHttpMetadata: true, wireName: "id"), + InputFactory.Property("filter", InputPrimitiveType.String, isRequired: false, isHttpMetadata: true, wireName: "filter"), + ]); + var optionsMethodParameter = InputFactory.MethodParameter( + "options", + optionsModel, + isRequired: true, + location: InputRequestLocation.Query); + + var idParam = InputFactory.PathParameter("id", InputPrimitiveType.String, isRequired: true); + idParam.Update(methodParameterSegments: + [ + optionsMethodParameter, + InputFactory.MethodParameter("id", InputPrimitiveType.String, isRequired: true), + ]); + var filterParam = InputFactory.QueryParameter("filter", InputPrimitiveType.String, isRequired: false, serializedName: "filter"); + filterParam.Update(methodParameterSegments: + [ + optionsMethodParameter, + InputFactory.MethodParameter("filter", InputPrimitiveType.String, isRequired: false), + ]); + + var serviceMethod = InputFactory.BasicServiceMethod( + "GetWidget", + InputFactory.Operation( + "GetWidget", + parameters: [idParam, filterParam], + responses: [InputFactory.OperationResponse([200])]), + parameters: [optionsMethodParameter]); + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + await MockHelpers.LoadMockGeneratorAsync( + clients: () => [inputClient], + inputModels: () => [optionsModel], + compilation: async () => await Helpers.GetCompilationFromDirectoryAsync()); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + Assert.IsNotNull(client); + + var methodCollection = new ScmMethodProviderCollection(serviceMethod, client!); + var protocolMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Protocol + && m.IsPartialMethod + && !m.Signature.Name.EndsWith("Async")); + Assert.IsNotNull(protocolMethod); + Assert.AreEqual("renamedOptions", protocolMethod!.Signature.Parameters[0].Name); + Assert.AreEqual("renamedRequestOptions", protocolMethod.Signature.Parameters[1].Name); + Assert.AreEqual( + Helpers.GetExpectedFromFile(), + protocolMethod.BodyStatements!.ToDisplayString()); + } + + [Test] + public async Task OptionsBagOverride_RequiredParamOptionalInBag_ProtocolStaysFlattened() + { + // https://github.com/microsoft/typespec/issues/11214 + // TCGC does not validate that a required wire parameter maps to a required bag property, so + // the bag's constructor would not force callers to supply it. Grouping the protocol method + // would drop the compile-time guarantee the flattened signature provides, so it stays flat. + var optionsModel = InputFactory.Model( + "GetWidgetOptions", + properties: + [ + InputFactory.Property("id", InputPrimitiveType.String, isRequired: false, isHttpMetadata: true, wireName: "id"), + ]); + + var optionsMethodParameter = InputFactory.MethodParameter("options", optionsModel, isRequired: true, location: InputRequestLocation.Query); + + var idParam = InputFactory.PathParameter("id", InputPrimitiveType.String, isRequired: true); + idParam.Update(methodParameterSegments: [optionsMethodParameter, InputFactory.MethodParameter("id", InputPrimitiveType.String, isRequired: false)]); + + var serviceMethod = InputFactory.BasicServiceMethod( + "GetWidget", + InputFactory.Operation("GetWidget", parameters: [idParam], responses: [InputFactory.OperationResponse([200])]), + parameters: [optionsMethodParameter]); + + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + await MockHelpers.LoadMockGeneratorAsync(clients: () => [inputClient], inputModels: () => [optionsModel]); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + var methodCollection = new ScmMethodProviderCollection(serviceMethod, client!); + var protocolMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Protocol && !m.Signature.Name.EndsWith("Async")); + Assert.IsNotNull(protocolMethod); + + var parameters = protocolMethod!.Signature.Parameters; + var actual = string.Join(", ", parameters.Select(p => $"{p.Type.Name} {p.Name}")); + + // The bag's constructor cannot force `id`, so the protocol method must keep requiring it directly. + var publicCtor = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(optionsModel)!.Constructors + .FirstOrDefault(c => c.Signature.Modifiers.HasFlag(MethodSignatureModifiers.Public)); + Assert.That(publicCtor!.Signature.Parameters.Select(p => p.Name), Does.Not.Contain("id")); + + Assert.IsFalse(parameters.Any(p => p.Type.Name == "GetWidgetOptions"), $"protocol must stay flattened, but was: ({actual})"); + Assert.IsTrue(parameters.Any(p => p.Name == "id"), $"required parameter must stay on the signature, but was: ({actual})"); + Assert.AreEqual( + Helpers.GetExpectedFromFile(), + protocolMethod.BodyStatements!.ToDisplayString()); + } + + [Test] + public async Task OptionsBagOverride_BodyOutsideBag_ProtocolKeepsRequestContent() + { + // https://github.com/microsoft/typespec/issues/11214 + // The bag only groups non-body parameters, so the protocol method can adopt it while still + // taking the raw request content. + var bodyModel = InputFactory.Model("Widget", properties: [InputFactory.Property("data", InputPrimitiveType.String, isRequired: true)]); + var optionsModel = InputFactory.Model( + "CreateWidgetOptions", + properties: + [ + InputFactory.Property("filter", InputPrimitiveType.String, isRequired: false, isHttpMetadata: true, wireName: "filter"), + InputFactory.Property("top", InputPrimitiveType.Int32, isRequired: false, isHttpMetadata: true, wireName: "top"), + ]); + + var optionsMethodParameter = InputFactory.MethodParameter("options", optionsModel, isRequired: true, location: InputRequestLocation.Query); + + var bodyParam = InputFactory.BodyParameter("body", bodyModel, isRequired: true); + var filterParam = InputFactory.QueryParameter("filter", InputPrimitiveType.String, isRequired: false, serializedName: "filter"); + filterParam.Update(methodParameterSegments: [optionsMethodParameter, InputFactory.MethodParameter("filter", InputPrimitiveType.String, isRequired: false)]); + var topParam = InputFactory.QueryParameter("top", InputPrimitiveType.Int32, isRequired: false, serializedName: "top"); + topParam.Update(methodParameterSegments: [optionsMethodParameter, InputFactory.MethodParameter("top", InputPrimitiveType.Int32, isRequired: false)]); + + var serviceMethod = InputFactory.BasicServiceMethod( + "CreateWidget", + InputFactory.Operation("CreateWidget", parameters: [bodyParam, filterParam, topParam], responses: [InputFactory.OperationResponse([200])]), + parameters: + [ + InputFactory.MethodParameter("body", bodyModel, isRequired: true, location: InputRequestLocation.Body), + optionsMethodParameter, + ]); + + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + await MockHelpers.LoadMockGeneratorAsync(clients: () => [inputClient], inputModels: () => [bodyModel, optionsModel]); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + var methodCollection = new ScmMethodProviderCollection(serviceMethod, client!); + var protocolMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Protocol && !m.Signature.Name.EndsWith("Async")); + Assert.IsNotNull(protocolMethod); + + var parameters = protocolMethod!.Signature.Parameters; + var actual = string.Join(", ", parameters.Select(p => $"{p.Type.Name} {p.Name}")); + + Assert.AreEqual(3, parameters.Count, $"actual: ({actual})"); + Assert.IsTrue(parameters.Any(p => p.IsContentParameter), $"raw body must be preserved, but was: ({actual})"); + Assert.IsTrue(parameters.Any(p => p.Type.Name == "CreateWidgetOptions"), $"actual: ({actual})"); + Assert.AreEqual( + Helpers.GetExpectedFromFile(), + protocolMethod.BodyStatements!.ToDisplayString()); + } + + [Test] + public async Task OptionsBagOverride_BodyInsideBag_ProtocolStaysFlattened() + { + // https://github.com/microsoft/typespec/issues/11214 + // When the request body itself was folded into the bag, grouping the protocol method would + // remove its only way to send a raw payload, so it stays flattened. + var requestModel = InputFactory.Model( + "RequestModel", + properties: + [ + InputFactory.Property("data", InputPrimitiveType.String, isRequired: true, isHttpMetadata: false), + InputFactory.Property("filter", InputPrimitiveType.String, isRequired: false, isHttpMetadata: true, wireName: "filter"), + ]); + + var requestMethodParameter = InputFactory.MethodParameter("request", requestModel, isRequired: true, location: InputRequestLocation.Body); + + var bodyParam = InputFactory.BodyParameter("body", requestModel, isRequired: true); + var filterParam = InputFactory.QueryParameter("filter", InputPrimitiveType.String, isRequired: false, serializedName: "filter"); + filterParam.Update(methodParameterSegments: [requestMethodParameter, InputFactory.MethodParameter("filter", InputPrimitiveType.String, isRequired: false)]); + + var serviceMethod = InputFactory.BasicServiceMethod( + "CreateWidget", + InputFactory.Operation("CreateWidget", parameters: [bodyParam, filterParam], responses: [InputFactory.OperationResponse([200])]), + parameters: [requestMethodParameter]); + + var inputClient = InputFactory.Client("TestClient", methods: [serviceMethod]); + await MockHelpers.LoadMockGeneratorAsync(clients: () => [inputClient], inputModels: () => [requestModel]); + + var client = ScmCodeModelGenerator.Instance.TypeFactory.CreateClient(inputClient); + var methodCollection = new ScmMethodProviderCollection(serviceMethod, client!); + var protocolMethod = methodCollection.FirstOrDefault( + m => m.Kind == ScmMethodKind.Protocol && !m.Signature.Name.EndsWith("Async")); + Assert.IsNotNull(protocolMethod); + + var parameters = protocolMethod!.Signature.Parameters; + var actual = string.Join(", ", parameters.Select(p => $"{p.Type.Name} {p.Name}")); + + Assert.IsTrue(parameters.Any(p => p.IsContentParameter), $"raw body must be preserved, but was: ({actual})"); + Assert.IsFalse(parameters.Any(p => p.Type.Name == "RequestModel"), $"protocol must stay flattened, but was: ({actual})"); + Assert.AreEqual( + Helpers.GetExpectedFromFile(), + protocolMethod.BodyStatements!.ToDisplayString()); } [Test] diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_EnumGroupedQueryParam_SerializesToProtocol.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_EnumGroupedQueryParam_SerializesToProtocol.cs index 72441024051..e536733cf7b 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_EnumGroupedQueryParam_SerializesToProtocol.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_EnumGroupedQueryParam_SerializesToProtocol.cs @@ -1,4 +1,4 @@ global::Sample.Argument.AssertNotNullOrEmpty(collectionId, nameof(collectionId)); global::Sample.Argument.AssertNotNull(options, nameof(options)); -return this.GetPoint(collectionId, options.Resampling?.ToString(), cancellationToken.ToRequestOptions()); +return this.GetPoint(collectionId, options, cancellationToken.ToRequestOptions()); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_RenamedGroupedQueryParam_MapsByClientName.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_RenamedGroupedQueryParam_MapsByClientName.cs index d6af6d4230b..e536733cf7b 100644 --- a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_RenamedGroupedQueryParam_MapsByClientName.cs +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/MethodParameterSegments_RenamedGroupedQueryParam_MapsByClientName.cs @@ -1,4 +1,4 @@ global::Sample.Argument.AssertNotNullOrEmpty(collectionId, nameof(collectionId)); global::Sample.Argument.AssertNotNull(options, nameof(options)); -return this.GetPoint(collectionId, options.BandIndex, cancellationToken.ToRequestOptions()); +return this.GetPoint(collectionId, options, cancellationToken.ToRequestOptions()); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_AppliesToProtocolMethod(Convenience).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_AppliesToProtocolMethod(Convenience).cs new file mode 100644 index 00000000000..d55cae5b177 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_AppliesToProtocolMethod(Convenience).cs @@ -0,0 +1,3 @@ +global::Sample.Argument.AssertNotNull(options, nameof(options)); + +return this.GetWidget(options, cancellationToken.ToRequestOptions()); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_AppliesToProtocolMethod(Protocol).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_AppliesToProtocolMethod(Protocol).cs new file mode 100644 index 00000000000..b3c32f359c2 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_AppliesToProtocolMethod(Protocol).cs @@ -0,0 +1,4 @@ +global::Sample.Argument.AssertNotNull(options, nameof(options)); + +using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetWidgetRequest(options.Id, options.Filter, options.Top, requestOptions); +return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, requestOptions)); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_BodyInsideBag_ProtocolStaysFlattened.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_BodyInsideBag_ProtocolStaysFlattened.cs new file mode 100644 index 00000000000..1d8e176a583 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_BodyInsideBag_ProtocolStaysFlattened.cs @@ -0,0 +1,4 @@ +global::Sample.Argument.AssertNotNull(content, nameof(content)); + +using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateCreateWidgetRequest(content, filter, options); +return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, options)); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_BodyOutsideBag_ProtocolKeepsRequestContent.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_BodyOutsideBag_ProtocolKeepsRequestContent.cs new file mode 100644 index 00000000000..972a20ff897 --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_BodyOutsideBag_ProtocolKeepsRequestContent.cs @@ -0,0 +1,5 @@ +global::Sample.Argument.AssertNotNull(options, nameof(options)); +global::Sample.Argument.AssertNotNull(content, nameof(content)); + +using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateCreateWidgetRequest(content, options.Filter, options.Top, requestOptions); +return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, requestOptions)); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping.cs new file mode 100644 index 00000000000..26a6d3b0b7c --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping.cs @@ -0,0 +1,4 @@ +global::Sample.Argument.AssertNotNull(renamedOptions, nameof(renamedOptions)); + +using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetWidgetRequest(renamedOptions.Id, renamedOptions.Filter, renamedRequestOptions); +return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, renamedRequestOptions)); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping/TestClient.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping/TestClient.cs new file mode 100644 index 00000000000..583443b391c --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_CustomProtocolParameterNamesPreserveGroupedMapping/TestClient.cs @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System.ClientModel; +using System.ClientModel.Primitives; +using Sample.Models; + +namespace Sample +{ + public partial class TestClient + { + public partial ClientResult GetWidget(GetWidgetOptions renamedOptions, RequestOptions renamedRequestOptions); + } +} diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_PagingMethodExpandsBagForCollectionResult(Convenience).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_PagingMethodExpandsBagForCollectionResult(Convenience).cs new file mode 100644 index 00000000000..ae6b47c9f0a --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_PagingMethodExpandsBagForCollectionResult(Convenience).cs @@ -0,0 +1,3 @@ +global::Sample.Argument.AssertNotNull(options, nameof(options)); + +return new global::Sample.TestClientGetWidgetsCollectionResultOfT(this, options.Id, options.Filter, cancellationToken.ToRequestOptions()); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_PagingMethodExpandsBagForCollectionResult(Protocol).cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_PagingMethodExpandsBagForCollectionResult(Protocol).cs new file mode 100644 index 00000000000..632c2a8b25f --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_PagingMethodExpandsBagForCollectionResult(Protocol).cs @@ -0,0 +1,3 @@ +global::Sample.Argument.AssertNotNull(options, nameof(options)); + +return new global::Sample.TestClientGetWidgetsCollectionResult(this, options.Id, options.Filter, requestOptions); diff --git a/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_RequiredParamOptionalInBag_ProtocolStaysFlattened.cs b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_RequiredParamOptionalInBag_ProtocolStaysFlattened.cs new file mode 100644 index 00000000000..1c76f895f6c --- /dev/null +++ b/packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/TestData/ScmMethodProviderCollectionTests/OptionsBagOverride_RequiredParamOptionalInBag_ProtocolStaysFlattened.cs @@ -0,0 +1,4 @@ +global::Sample.Argument.AssertNotNullOrEmpty(id, nameof(id)); + +using global::System.ClientModel.Primitives.PipelineMessage message = this.CreateGetWidgetRequest(id, options); +return global::System.ClientModel.ClientResult.FromResponse(Pipeline.ProcessMessage(message, options));