Skip to content

Commit

Permalink
[tcgc] add subscriptionId to client parameters (#673)
Browse files Browse the repository at this point in the history 
Co-authored-by: iscai-msft <[email protected]>
  • Loading branch information
@iscai-msft
iscai-msft and iscai-msft committed Apr 17, 2024
1 parent 4e522ae commit 30438c1
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 39 deletions.
7 changes: 7 additions & 0 deletions .chronus/changes/subscription_id-2024-3-15-14-48-17.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
changeKind: feature
packages:
- "@azure-tools/typespec-client-generator-core"
---

add subscriptionId to client parameters
30 changes: 29 additions & 1 deletion packages/typespec-client-generator-core/src/http.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
getDocHelper,
isAcceptHeader,
isNullable,
isSubscriptionId,
} from "./internal-utils.js";
import { createDiagnostic } from "./lib.js";
import {
Expand Down Expand Up @@ -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]);
Expand Down
11 changes: 10 additions & 1 deletion packages/typespec-client-generator-core/src/internal-utils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export function updateWithApiVersionInformation(
return {
isApiVersionParam,
clientDefaultValue: isApiVersionParam ? context.__api_version_client_default_value : undefined,
onClient: isApiVersionParam,
onClient: onClient(context, type),
};
}

Expand Down Expand Up @@ -259,6 +259,7 @@ export interface TCGCContext {
__api_versions?: string[];
knownScalars?: Record<string, SdkBuiltInKinds>;
diagnostics: readonly Diagnostic[];
__subscriptionIdParameter?: SdkParameter;
}

export function createTCGCContext(program: Program): TCGCContext {
Expand Down Expand Up @@ -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);
}
19 changes: 4 additions & 15 deletions packages/typespec-client-generator-core/src/package.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<TOptions, TServiceOperation>(context, operation, methodParameters)
);
Expand All @@ -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,
Expand Down Expand Up @@ -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({
Expand Down
57 changes: 35 additions & 22 deletions packages/typespec-client-generator-core/test/package.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -2987,25 +2990,35 @@ 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"));
strictEqual(createOrReplace.operation.bodyParam?.type.kind, "model");
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);
Expand Down

0 comments on commit 30438c1

Please sign in to comment.