Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tcgc] add subscriptionId to client parameters #673

Merged
merged 4 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .chronus/changes/subscription_id-2024-3-15-14-48-17.md
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
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
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
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
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
Loading