From 30438c1d8fef14fc52da01dfffb73456fd6b6afa Mon Sep 17 00:00:00 2001 From: iscai-msft <43154838+iscai-msft@users.noreply.github.com> Date: Wed, 17 Apr 2024 12:56:32 -0400 Subject: [PATCH] [tcgc] add subscriptionId to client parameters (#673) Co-authored-by: iscai-msft --- .../subscription_id-2024-3-15-14-48-17.md | 7 +++ .../src/http.ts | 30 +++++++++- .../src/internal-utils.ts | 11 +++- .../src/package.ts | 19 ++----- .../test/package.test.ts | 57 ++++++++++++------- 5 files changed, 85 insertions(+), 39 deletions(-) create mode 100644 .chronus/changes/subscription_id-2024-3-15-14-48-17.md diff --git a/.chronus/changes/subscription_id-2024-3-15-14-48-17.md b/.chronus/changes/subscription_id-2024-3-15-14-48-17.md new file mode 100644 index 0000000000..f9f65e9956 --- /dev/null +++ b/.chronus/changes/subscription_id-2024-3-15-14-48-17.md @@ -0,0 +1,7 @@ +--- +changeKind: feature +packages: + - "@azure-tools/typespec-client-generator-core" +--- + +add subscriptionId to client parameters \ No newline at end of file diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts index c9fac9e91d..fef2e7be41 100644 --- a/packages/typespec-client-generator-core/src/http.ts +++ b/packages/typespec-client-generator-core/src/http.ts @@ -43,6 +43,7 @@ import { getDocHelper, isAcceptHeader, isNullable, + isSubscriptionId, } from "./internal-utils.js"; import { createDiagnostic } from "./lib.js"; import { @@ -453,10 +454,37 @@ export function getCorrespondingMethodParams( ); return diagnostics.wrap([]); } - context.__api_version_parameter = apiVersionParam; + context.__api_version_parameter = { + ...apiVersionParam, + name: "apiVersion", + nameInClient: "apiVersion", + isGeneratedName: apiVersionParam.name !== "apiVersion", + optional: false, + clientDefaultValue: context.__api_version_client_default_value, + }; } return diagnostics.wrap([context.__api_version_parameter]); } + if (isSubscriptionId(context, serviceParam)) { + if (!context.__subscriptionIdParameter) { + const subscriptionIdParam = methodParameters.find((x) => isSubscriptionId(context, x)); + if (!subscriptionIdParam) { + diagnostics.add( + createDiagnostic({ + code: "no-corresponding-method-param", + target: serviceParam.__raw!, + format: { + paramName: "subscriptionId", + methodName: methodName, + }, + }) + ); + return diagnostics.wrap([]); + } + context.__subscriptionIdParameter = subscriptionIdParam; + } + return diagnostics.wrap([context.__subscriptionIdParameter]); + } const correspondingMethodParameter = methodParameters.find((x) => x.name === serviceParam.name); if (correspondingMethodParameter) { return diagnostics.wrap([correspondingMethodParameter]); diff --git a/packages/typespec-client-generator-core/src/internal-utils.ts b/packages/typespec-client-generator-core/src/internal-utils.ts index 38c7578254..2c0cd5ee17 100644 --- a/packages/typespec-client-generator-core/src/internal-utils.ts +++ b/packages/typespec-client-generator-core/src/internal-utils.ts @@ -105,7 +105,7 @@ export function updateWithApiVersionInformation( return { isApiVersionParam, clientDefaultValue: isApiVersionParam ? context.__api_version_client_default_value : undefined, - onClient: isApiVersionParam, + onClient: onClient(context, type), }; } @@ -259,6 +259,7 @@ export interface TCGCContext { __api_versions?: string[]; knownScalars?: Record; diagnostics: readonly Diagnostic[]; + __subscriptionIdParameter?: SdkParameter; } export function createTCGCContext(program: Program): TCGCContext { @@ -345,3 +346,11 @@ export function isMultipartFormData( ): boolean { return isMultipartOperation(context, operation) && isOperationBodyType(context, type, operation); } + +export function isSubscriptionId(context: TCGCContext, parameter: { name: string }): boolean { + return Boolean(context.arm) && parameter.name === "subscriptionId"; +} + +export function onClient(context: TCGCContext, parameter: { name: string }): boolean { + return isSubscriptionId(context, parameter) || isApiVersion(context, parameter); +} diff --git a/packages/typespec-client-generator-core/src/package.ts b/packages/typespec-client-generator-core/src/package.ts index 3e5768da38..c2023a10b3 100644 --- a/packages/typespec-client-generator-core/src/package.ts +++ b/packages/typespec-client-generator-core/src/package.ts @@ -259,20 +259,6 @@ function getSdkBasicServiceMethod< } } - // if there's an api version parameter, we want to bubble it up to the client - // we don't want it on the method level, but we will keep it on the service operation level - const apiVersionParam = methodParameters.find((x) => x.isApiVersionParam); - if (apiVersionParam && context.__api_version_parameter === undefined) { - context.__api_version_parameter = { - ...apiVersionParam, - name: "apiVersion", - nameInClient: "apiVersion", - isGeneratedName: apiVersionParam.name !== "apiVersion", - onClient: true, - optional: false, - clientDefaultValue: context.__api_version_client_default_value, - }; - } const serviceOperation = diagnostics.pipe( getSdkServiceOperation(context, operation, methodParameters) ); @@ -283,7 +269,7 @@ function getSdkBasicServiceMethod< kind: "basic", name, access: getAccess(context, operation), - parameters: methodParameters.filter((x) => !x.isApiVersionParam), + parameters: methodParameters, description: getDocHelper(context, operation).description, details: getDocHelper(context, operation).details, operation: serviceOperation, @@ -354,6 +340,9 @@ function getSdkInitializationType< if (context.__api_version_parameter) { properties.push(context.__api_version_parameter); } + if (context.__subscriptionIdParameter) { + properties.push(context.__subscriptionIdParameter); + } const namePrefix = client.kind === "SdkClient" ? client.name : client.groupPath; const name = `${namePrefix.split(".").at(-1)}Options`; return diagnostics.wrap({ diff --git a/packages/typespec-client-generator-core/test/package.test.ts b/packages/typespec-client-generator-core/test/package.test.ts index a4205bdebb..40f1a92ef3 100644 --- a/packages/typespec-client-generator-core/test/package.test.ts +++ b/packages/typespec-client-generator-core/test/package.test.ts @@ -733,7 +733,9 @@ describe("typespec-client-generator-core: package", () => { strictEqual(sdkPackage.clients[0].methods.length, 1); const withApiVersion = sdkPackage.clients[0].methods[0]; strictEqual(withApiVersion.kind, "basic"); - strictEqual(withApiVersion.parameters.length, 0); + strictEqual(withApiVersion.parameters.length, 1); + strictEqual(withApiVersion.operation.parameters[0].name, "apiVersion"); + strictEqual(withApiVersion.operation.parameters[0].isApiVersionParam, true); strictEqual(withApiVersion.operation.parameters.length, 1); const apiVersionParam = withApiVersion.operation.parameters[0]; @@ -841,8 +843,10 @@ describe("typespec-client-generator-core: package", () => { const withApiVersion = client.methods[0]; strictEqual(withApiVersion.name, "withQueryApiVersion"); strictEqual(withApiVersion.kind, "basic"); - strictEqual(withApiVersion.parameters.length, 0); + strictEqual(withApiVersion.parameters.length, 1); strictEqual(withApiVersion.operation.parameters.length, 1); + strictEqual(withApiVersion.parameters[0].isApiVersionParam, true); + strictEqual(withApiVersion.parameters[0].name, "apiVersion"); const apiVersionParam = withApiVersion.operation.parameters[0]; strictEqual(apiVersionParam.kind, "query"); @@ -890,7 +894,9 @@ describe("typespec-client-generator-core: package", () => { const withApiVersion = client.methods[0]; strictEqual(withApiVersion.name, "withPathApiVersion"); strictEqual(withApiVersion.kind, "basic"); - strictEqual(withApiVersion.parameters.length, 0); + strictEqual(withApiVersion.parameters.length, 1); + strictEqual(withApiVersion.parameters[0].isApiVersionParam, true); + strictEqual(withApiVersion.parameters[0].name, "apiVersion"); strictEqual(withApiVersion.operation.parameters.length, 1); const apiVersionParam = withApiVersion.operation.parameters[0]; @@ -2321,7 +2327,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(parentClient.name, "WidgetManagerClient"); strictEqual(method.name, "getWidget"); strictEqual(method.kind, "basic"); - strictEqual(method.parameters.length, 7); + strictEqual(method.parameters.length, 8); // TODO: what should we do with eTags and client request id? const methodWidgetName = method.parameters.find((p) => p.name === "widgetName"); @@ -2418,7 +2424,7 @@ describe("typespec-client-generator-core: package", () => { ok(getStatus); strictEqual(getStatus.name, "getWidgetOperationStatus"); strictEqual(getStatus.kind, "basic"); - strictEqual(getStatus.parameters.length, 3); + strictEqual(getStatus.parameters.length, 4); const methodWidgetName = getStatus.parameters.find((p) => p.name === "widgetName"); ok(methodWidgetName); @@ -2498,13 +2504,10 @@ describe("typespec-client-generator-core: package", () => { const createOrUpdate = client.methods.find((x) => x.name === "createOrUpdateWidget"); ok(createOrUpdate); strictEqual(createOrUpdate.kind, "lro"); - strictEqual( - createOrUpdate.parameters.find((x) => x.name === "apiVersion"), - undefined - ); deepStrictEqual( createOrUpdate.parameters.map((x) => x.name), [ + "apiVersion", "widgetName", "contentType", "repeatabilityRequestId", @@ -2619,10 +2622,10 @@ describe("typespec-client-generator-core: package", () => { strictEqual(listManufacturers.name, "listManufacturers"); strictEqual(listManufacturers.kind, "paging"); - strictEqual(listManufacturers.parameters.length, 2); + strictEqual(listManufacturers.parameters.length, 3); deepStrictEqual( listManufacturers.parameters.map((x) => x.name), - ["clientRequestId", "accept"] + ["apiVersion", "clientRequestId", "accept"] ); const methodResponse = listManufacturers.response.type; ok(methodResponse); @@ -2648,13 +2651,13 @@ describe("typespec-client-generator-core: package", () => { strictEqual(clientRequestId.kind, "header"); deepStrictEqual( clientRequestId.correspondingMethodParams[0], - listManufacturers.parameters[0] + listManufacturers.parameters[1] ); const accept = operation.parameters.find((x) => x.name === "accept"); ok(accept); strictEqual(accept.kind, "header"); - deepStrictEqual(accept.correspondingMethodParams[0], listManufacturers.parameters[1]); + deepStrictEqual(accept.correspondingMethodParams[0], listManufacturers.parameters[2]); strictEqual(operation.responses.size, 1); const response200 = operation.responses.get(200); @@ -2987,17 +2990,27 @@ describe("typespec-client-generator-core: package", () => { const createOrReplace = sdkPackage.clients[0].methods[1]; strictEqual(createOrReplace.kind, "basic"); strictEqual(createOrReplace.name, "createOrReplaceDataConnection"); - strictEqual(createOrReplace.parameters.length, 4); - strictEqual(createOrReplace.parameters[0].name, "dataConnectionName"); - strictEqual(createOrReplace.parameters[0].type.kind, "string"); - strictEqual(createOrReplace.parameters[1].name, "dataConnectionData"); - strictEqual(createOrReplace.parameters[1].type.kind, "model"); - strictEqual(createOrReplace.parameters[1].type.name, "DataConnectionData"); - strictEqual(createOrReplace.parameters[2].name, "contentType"); - strictEqual(createOrReplace.parameters[3].name, "accept"); + strictEqual(createOrReplace.parameters.length, 5); + ok( + createOrReplace.parameters.find( + (x) => x.name === "dataConnectionName" && x.type.kind === "string" + ) + ); + ok( + createOrReplace.parameters.find( + (x) => + x.name === "dataConnectionData" && + x.type.kind === "model" && + x.type.name === "DataConnectionData" + ) + ); + ok(createOrReplace.parameters.find((x) => x.name === "contentType")); + ok(createOrReplace.parameters.find((x) => x.name === "accept")); + ok(createOrReplace.parameters.find((x) => x.isApiVersionParam && x.onClient)); const opParams = createOrReplace.operation.parameters; strictEqual(opParams.length, 4); + ok(opParams.find((x) => x.isApiVersionParam === true && x.kind === "query")); ok(opParams.find((x) => x.kind === "path" && x.serializedName === "dataConnectionName")); ok(opParams.find((x) => x.kind === "header" && x.serializedName === "Content-Type")); ok(opParams.find((x) => x.kind === "header" && x.serializedName === "Accept")); @@ -3005,7 +3018,7 @@ describe("typespec-client-generator-core: package", () => { strictEqual(createOrReplace.operation.bodyParam?.type.name, "DataConnectionData"); deepStrictEqual( createOrReplace.operation.bodyParam.correspondingMethodParams[0], - createOrReplace.parameters[1] + createOrReplace.parameters[2] ); strictEqual(createOrReplace.operation.responses.size, 1); const response200 = createOrReplace.operation.responses.get(200);