From 8d91e34cc36e63e416c911e91b55a48ff45e73ad Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Mon, 15 Apr 2024 14:48:03 -0700 Subject: [PATCH 1/4] add subscriptionid to client params --- .../src/http.ts | 20 +++++++ .../src/internal-utils.ts | 11 +++- .../src/package.ts | 3 +- .../test/package.test.ts | 57 ++++++++++++------- 4 files changed, 66 insertions(+), 25 deletions(-) diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts index c9fac9e91d..464cd69dc6 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 { @@ -457,6 +458,25 @@ export function getCorrespondingMethodParams( } 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; + } + } 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..f270a80a61 100644 --- a/packages/typespec-client-generator-core/src/package.ts +++ b/packages/typespec-client-generator-core/src/package.ts @@ -268,7 +268,6 @@ function getSdkBasicServiceMethod< name: "apiVersion", nameInClient: "apiVersion", isGeneratedName: apiVersionParam.name !== "apiVersion", - onClient: true, optional: false, clientDefaultValue: context.__api_version_client_default_value, }; @@ -283,7 +282,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, 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); From 88f5e7937d910bdf6fa7f844230fb100a57687cb Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Mon, 15 Apr 2024 16:00:22 -0700 Subject: [PATCH 2/4] add changeset --- .chronus/changes/subscription_id-2024-3-15-14-48-17.md | 7 +++++++ 1 file changed, 7 insertions(+) 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 From d5be5d9a84e81a4a56eadd76df6c6162f500c673 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Mon, 15 Apr 2024 16:34:02 -0700 Subject: [PATCH 3/4] only set api version param in one place --- .../typespec-client-generator-core/src/http.ts | 9 ++++++++- .../src/package.ts | 16 +++------------- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts index 464cd69dc6..8b7cd4aadc 100644 --- a/packages/typespec-client-generator-core/src/http.ts +++ b/packages/typespec-client-generator-core/src/http.ts @@ -454,7 +454,14 @@ 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]); } diff --git a/packages/typespec-client-generator-core/src/package.ts b/packages/typespec-client-generator-core/src/package.ts index f270a80a61..c2023a10b3 100644 --- a/packages/typespec-client-generator-core/src/package.ts +++ b/packages/typespec-client-generator-core/src/package.ts @@ -259,19 +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", - optional: false, - clientDefaultValue: context.__api_version_client_default_value, - }; - } const serviceOperation = diagnostics.pipe( getSdkServiceOperation(context, operation, methodParameters) ); @@ -353,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({ From a149d2c3d62bd08da2af17134b9a9a66d62f1b17 Mon Sep 17 00:00:00 2001 From: iscai-msft Date: Mon, 15 Apr 2024 16:36:46 -0700 Subject: [PATCH 4/4] return subscription id param --- packages/typespec-client-generator-core/src/http.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts index 8b7cd4aadc..fef2e7be41 100644 --- a/packages/typespec-client-generator-core/src/http.ts +++ b/packages/typespec-client-generator-core/src/http.ts @@ -483,6 +483,7 @@ export function getCorrespondingMethodParams( } context.__subscriptionIdParameter = subscriptionIdParam; } + return diagnostics.wrap([context.__subscriptionIdParameter]); } const correspondingMethodParameter = methodParameters.find((x) => x.name === serviceParam.name); if (correspondingMethodParameter) {