Fix client version params (#778)

Right now, we have all api version information tied to the `sdkContext` as a whole. As an example, this means that when we add a client in a later version, we're not able to have granular handling of api version for the second client. In this PR, I've made all api version information a mapping from namespace to apiVersion information. I also added a test that reproduces one of the previous issues. The PR diff is large bc I've had to add parameters everywhere so I'm still passing --------- Co-authored-by: iscai-msft <[email protected]>
Azure · May 6, 2024 · 574936e · 574936e
1 parent 199748d
commit 574936e
Show file tree

Hide file tree

Showing 7 changed files with 248 additions and 70 deletions.
diff --git a/.chronus/changes/fix_client_version_params-2024-4-3-14-57-7.md b/.chronus/changes/fix_client_version_params-2024-4-3-14-57-7.md
@@ -0,0 +1,7 @@
+---
+changeKind: fix
+packages:
+  - "@azure-tools/typespec-client-generator-core"
+---
+
+tie api version information to clients so we can have diff api version information per client
diff --git a/packages/typespec-client-generator-core/src/decorators.ts b/packages/typespec-client-generator-core/src/decorators.ts
@@ -604,6 +604,9 @@ export function createSdkContext<
     diagnostics: diagnostics.diagnostics,
     apiVersion: options?.versionStrategy === "ignore" ? "all" : context.options["api-version"],
     originalProgram: context.program,
+    __namespaceToApiVersionParameter: new Map(),
+    __namespaceToApiVersions: new Map(),
+    __namespaceToApiVersionClientDefaultValue: new Map(),
   };
   sdkContext.experimental_sdkPackage = getSdkPackage(sdkContext);
   if (sdkContext.diagnostics) {

diff --git a/packages/typespec-client-generator-core/src/http.ts b/packages/typespec-client-generator-core/src/http.ts
@@ -1,6 +1,7 @@
 import {
   Diagnostic,
   ModelProperty,
+  Operation,
   Type,
   createDiagnosticCollector,
   ignoreDiagnostics,
@@ -40,6 +41,7 @@ import {
   TCGCContext,
   getAvailableApiVersions,
   getDocHelper,
+  getLocationOfOperation,
   isAcceptHeader,
   isContentTypeHeader,
   isNullable,
@@ -106,7 +108,9 @@ function getSdkHttpParameters(
     bodyParam: undefined,
   };
   retval.parameters = httpOperation.parameters.parameters
-    .map((x) => diagnostics.pipe(getSdkHttpParameter(context, x.param, x.type)))
+    .map((x) =>
+      diagnostics.pipe(getSdkHttpParameter(context, x.param, httpOperation.operation, x.type))
+    )
     .filter(
       (x): x is SdkHeaderParameter | SdkQueryParameter | SdkPathParameter =>
         x.kind === "header" || x.kind === "query" || x.kind === "path"
@@ -122,7 +126,7 @@ function getSdkHttpParameters(
     // if there's a param on the body, we can just rely on getSdkHttpParameter
     if (tspBody.parameter) {
       const getParamResponse = diagnostics.pipe(
-        getSdkHttpParameter(context, tspBody.parameter, "body")
+        getSdkHttpParameter(context, tspBody.parameter, httpOperation.operation, "body")
       );
       if (getParamResponse.kind !== "body") {
         diagnostics.add(
@@ -155,7 +159,11 @@ function getSdkHttpParameters(
         contentTypes: [],
         defaultContentType: "application/json", // actual content type info is added later
         isApiVersionParam: false,
-        apiVersions: getAvailableApiVersions(context, tspBody.type),
+        apiVersions: getAvailableApiVersions(
+          context,
+          tspBody.type,
+          httpOperation.operation.namespace
+        ),
         type,
         optional: false,
         nullable: isNullable(tspBody.type),
@@ -166,7 +174,7 @@ function getSdkHttpParameters(
     retval.bodyParam.correspondingMethodParams = diagnostics.pipe(
       getCorrespondingMethodParams(
         context,
-        httpOperation.operation.name,
+        httpOperation.operation,
         methodParameters,
         retval.bodyParam
       )
@@ -211,7 +219,7 @@ function getSdkHttpParameters(
   }
   for (const param of retval.parameters) {
     param.correspondingMethodParams = diagnostics.pipe(
-      getCorrespondingMethodParams(context, httpOperation.operation.name, methodParameters, param)
+      getCorrespondingMethodParams(context, httpOperation.operation, methodParameters, param)
     );
   }
   return diagnostics.wrap(retval);
@@ -284,10 +292,11 @@ function addContentTypeInfoToBodyParam(
 export function getSdkHttpParameter(
   context: TCGCContext,
   type: ModelProperty,
+  operation?: Operation,
   location?: "path" | "query" | "header" | "body"
 ): [SdkHttpParameter, readonly Diagnostic[]] {
   const diagnostics = createDiagnosticCollector();
-  const base = diagnostics.pipe(getSdkModelPropertyTypeBase(context, type));
+  const base = diagnostics.pipe(getSdkModelPropertyTypeBase(context, type, operation));
   const program = context.program;
   const correspondingMethodParams: SdkParameter[] = []; // we set it later in the operation
   if (isPathParam(context.program, type) || location === "path") {
@@ -416,7 +425,11 @@ function getSdkHttpResponseAndExceptions(
       defaultContentType: contentTypes.includes("application/json")
         ? "application/json"
         : contentTypes[0],
-      apiVersions: getAvailableApiVersions(context, httpOperation.operation),
+      apiVersions: getAvailableApiVersions(
+        context,
+        httpOperation.operation,
+        httpOperation.operation.namespace
+      ),
       nullable: body ? isNullable(body) : true,
     };
     if (response.statusCodes === "*" || (body && isErrorModel(context.program, body))) {
@@ -430,13 +443,16 @@ function getSdkHttpResponseAndExceptions(
 
 export function getCorrespondingMethodParams(
   context: TCGCContext,
-  methodName: string,
+  operation: Operation,
   methodParameters: SdkParameter[],
   serviceParam: SdkHttpParameter
 ): [SdkModelPropertyType[], readonly Diagnostic[]] {
   const diagnostics = createDiagnosticCollector();
+
+  const operationLocation = getLocationOfOperation(operation);
   if (serviceParam.isApiVersionParam) {
-    if (!context.__api_version_parameter) {
+    const existingApiVersion = context.__namespaceToApiVersionParameter.get(operationLocation);
+    if (!existingApiVersion) {
       const apiVersionParam = methodParameters.find((x) => x.name.includes("apiVersion"));
       if (!apiVersionParam) {
         diagnostics.add(
@@ -445,22 +461,24 @@ export function getCorrespondingMethodParams(
             target: serviceParam.__raw!,
             format: {
               paramName: "apiVersion",
-              methodName: methodName,
+              methodName: operation.name,
             },
           })
         );
         return diagnostics.wrap([]);
       }
-      context.__api_version_parameter = {
+      const apiVersionParamUpdated: SdkParameter = {
         ...apiVersionParam,
         name: "apiVersion",
         nameInClient: "apiVersion",
         isGeneratedName: apiVersionParam.name !== "apiVersion",
         optional: false,
-        clientDefaultValue: context.__api_version_client_default_value,
+        clientDefaultValue:
+          context.__namespaceToApiVersionClientDefaultValue.get(operationLocation),
       };
+      context.__namespaceToApiVersionParameter.set(operationLocation, apiVersionParamUpdated);
     }
-    return diagnostics.wrap([context.__api_version_parameter]);
+    return diagnostics.wrap([context.__namespaceToApiVersionParameter.get(operationLocation)!]);
   }
   if (isSubscriptionId(context, serviceParam)) {
     if (!context.__subscriptionIdParameter) {
@@ -472,7 +490,7 @@ export function getCorrespondingMethodParams(
             target: serviceParam.__raw!,
             format: {
               paramName: "subscriptionId",
-              methodName: methodName,
+              methodName: operation.name,
             },
           })
         );
@@ -523,7 +541,7 @@ export function getCorrespondingMethodParams(
         target: serviceParam.__raw!,
         format: {
           paramName: serviceParam.name,
-          methodName: methodName,
+          methodName: operation.name,
         },
       })
     );
@@ -544,7 +562,7 @@ export function getCorrespondingMethodParams(
       target: serviceParam.__raw!,
       format: {
         paramName: serviceParam.name,
-        methodName: methodName,
+        methodName: operation.name,
       },
     })
   );

diff --git a/packages/typespec-client-generator-core/src/internal-utils.ts b/packages/typespec-client-generator-core/src/internal-utils.ts
@@ -1,6 +1,7 @@
 import { getUnionAsEnum } from "@azure-tools/typespec-azure-core";
 import {
   Diagnostic,
+  Interface,
   Model,
   Namespace,
   Operation,
@@ -97,7 +98,8 @@ export function getClientNamespaceStringHelper(
  */
 export function updateWithApiVersionInformation(
   context: TCGCContext,
-  type: { name: string }
+  type: { name: string },
+  namespace?: Namespace | Interface
 ): {
   isApiVersionParam: boolean;
   clientDefaultValue?: unknown;
@@ -106,24 +108,19 @@ export function updateWithApiVersionInformation(
   const isApiVersionParam = isApiVersion(context, type);
   return {
     isApiVersionParam,
-    clientDefaultValue: isApiVersionParam ? context.__api_version_client_default_value : undefined,
+    clientDefaultValue:
+      isApiVersionParam && namespace
+        ? context.__namespaceToApiVersionClientDefaultValue.get(namespace)
+        : undefined,
     onClient: onClient(context, type),
   };
 }
 
-/**
- *
- * @param context
- * @param type
- * @returns All api versions the type is available on
- */
-export function getAvailableApiVersions(context: TCGCContext, type: Type): string[] {
-  const apiVersions =
-    context.__api_versions ||
-    getVersions(context.program, type)[1]
-      ?.getVersions()
-      .map((x) => x.value);
-  if (!apiVersions) return [];
+export function filterApiVersionsWithDecorators(
+  context: TCGCContext,
+  type: Type,
+  apiVersions: string[]
+): string[] {
   const addedOnVersions = getAddedOnVersions(context.program, type)?.map((x) => x.value) ?? [];
   const removedOnVersions = getRemovedOnVersions(context.program, type)?.map((x) => x.value) ?? [];
   let added: boolean = addedOnVersions.length ? false : true;
@@ -155,6 +152,32 @@ export function getAvailableApiVersions(context: TCGCContext, type: Type): strin
   return retval;
 }
 
+/**
+ *
+ * @param context
+ * @param type
+ * @param client If it's associated with a client, meaning it's a param etc, we can see if it's available on that client
+ * @returns All api versions the type is available on
+ */
+export function getAvailableApiVersions(
+  context: TCGCContext,
+  type: Type,
+  namespace?: Namespace | Interface
+): string[] {
+  let cachedApiVersions: string[] = [];
+  if (namespace) {
+    cachedApiVersions = context.__namespaceToApiVersions.get(namespace) || [];
+  }
+
+  const apiVersions =
+    cachedApiVersions ||
+    getVersions(context.program, type)[1]
+      ?.getVersions()
+      .map((x) => x.value);
+  if (!apiVersions) return [];
+  return filterApiVersionsWithDecorators(context, type, apiVersions);
+}
+
 interface DocWrapper {
   description?: string;
   details?: string;
@@ -271,9 +294,9 @@ export interface TCGCContext {
   generatedNames?: Map<Union | Model, string>;
   httpOperationCache?: Map<Operation, HttpOperation>;
   unionsMap?: Map<Union, SdkUnionType>;
-  __api_version_parameter?: SdkParameter;
-  __api_version_client_default_value?: string;
-  __api_versions?: string[];
+  __namespaceToApiVersionParameter: Map<Interface | Namespace, SdkParameter>;
+  __namespaceToApiVersions: Map<Interface | Namespace, string[]>;
+  __namespaceToApiVersionClientDefaultValue: Map<Interface | Namespace, string | undefined>;
   knownScalars?: Record<string, SdkBuiltInKinds>;
   diagnostics: readonly Diagnostic[];
   __subscriptionIdParameter?: SdkParameter;
@@ -289,6 +312,9 @@ export function createTCGCContext(program: Program): TCGCContext {
     emitterName: "__TCGC_INTERNAL__",
     diagnostics: [],
     originalProgram: program,
+    __namespaceToApiVersionParameter: new Map(),
+    __namespaceToApiVersions: new Map(),
+    __namespaceToApiVersionClientDefaultValue: new Map(),
   };
 }
 
@@ -376,3 +402,8 @@ export function isSubscriptionId(context: TCGCContext, parameter: { name: string
 export function onClient(context: TCGCContext, parameter: { name: string }): boolean {
   return isSubscriptionId(context, parameter) || isApiVersion(context, parameter);
 }
+
+export function getLocationOfOperation(operation: Operation): Namespace | Interface {
+  // have to check interface first, because interfaces are more granular than namespaces
+  return (operation.interface || operation.namespace)!;
+}