Fix wrong client cache for package clients calculation (#434)

Azure · Mar 19, 2024 · 42f95a3 · 42f95a3
1 parent 5104e22
commit 42f95a3
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 44 deletions.
diff --git a/.chronus/changes/fix_client-2024-2-18-17-59-28.md b/.chronus/changes/fix_client-2024-2-18-17-59-28.md
@@ -0,0 +1,7 @@
+---
+changeKind: fix
+packages:
+  - "@azure-tools/typespec-client-generator-core"
+---
+
+Fix wrong client cache for package clients calculation
diff --git a/packages/typespec-client-generator-core/src/interfaces.ts b/packages/typespec-client-generator-core/src/interfaces.ts
@@ -25,7 +25,7 @@ export interface SdkContext<
 > extends TCGCContext {
   emitContext: EmitContext<TOptions>;
   experimental_sdkPackage: SdkPackage<TServiceOperation>;
-  __clients?: Map<string, SdkClientType<TServiceOperation>>;
+  __clients?: SdkClientType<TServiceOperation>[];
 }
 
 export interface SdkEmitterOptions {

diff --git a/packages/typespec-client-generator-core/src/package.ts b/packages/typespec-client-generator-core/src/package.ts
@@ -790,9 +790,6 @@ function createSdkClientType<
   operationGroup?: SdkOperationGroup
 ): [SdkClientType<TServiceOperation>, readonly Diagnostic[]] {
   const diagnostics = createDiagnosticCollector();
-  if (!context.__clients) {
-    context.__clients = new Map<string, SdkClientType<TServiceOperation>>();
-  }
   const baseClientType = operationGroup ?? client;
   const isClient = baseClientType.kind === "SdkClient";
   const clientName = isClient ? baseClientType.name : baseClientType.type.name;
@@ -804,34 +801,26 @@ function createSdkClientType<
   // NOTE: getSdkMethods recursively calls createSdkClientType
   const methods = diagnostics.pipe(getSdkMethods(context, client, baseClientType));
 
-  // check to see if this client was previously created via a recursive call.
-  // if it was, then fetch it and append any methods that were just created.
-  let sdkClientType: SdkClientType<TServiceOperation>;
-  if (context.__clients.has(clientName)) {
-    sdkClientType = context.__clients.get(clientName)!;
-    sdkClientType.methods = sdkClientType.methods.concat(methods);
-  } else {
-    const endpointInfo = diagnostics.pipe(
-      getEndpointAndEndpointParameters<TOptions, TServiceOperation>(context, client)
-    );
-    const docWrapper = getDocHelper(context, baseClientType.type);
-    sdkClientType = {
-      kind: "client",
-      name: clientName,
-      description: docWrapper.description,
-      details: docWrapper.details,
-      methods: methods,
-      apiVersions: getAvailableApiVersions(context, client.type),
-      nameSpace: getClientNamespaceStringHelper(context, client.service)!,
-      initialization: isClient
-        ? diagnostics.pipe(getSdkInitializationType<TOptions, TServiceOperation>(context, client)) // MUST call this after getSdkMethods has been called
-        : undefined,
-      endpoint: endpointInfo.endpoint,
-      hasParameterizedEndpoint: endpointInfo.hasParameterizedEndpoint,
-      arm: client.arm,
-    };
-    context.__clients.set(clientName, sdkClientType);
-  }
+  const endpointInfo = diagnostics.pipe(
+    getEndpointAndEndpointParameters<TOptions, TServiceOperation>(context, client)
+  );
+  const docWrapper = getDocHelper(context, baseClientType.type);
+  const sdkClientType: SdkClientType<TServiceOperation> = {
+    kind: "client",
+    name: clientName,
+    description: docWrapper.description,
+    details: docWrapper.details,
+    methods: methods,
+    apiVersions: getAvailableApiVersions(context, client.type),
+    nameSpace: getClientNamespaceStringHelper(context, client.service)!,
+    initialization: isClient
+      ? diagnostics.pipe(getSdkInitializationType<TOptions, TServiceOperation>(context, client)) // MUST call this after getSdkMethods has been called
+      : undefined,
+    endpoint: endpointInfo.endpoint,
+    hasParameterizedEndpoint: endpointInfo.hasParameterizedEndpoint,
+    arm: client.arm,
+  };
+  context.__clients!.push(sdkClientType);
   return diagnostics.wrap(sdkClientType);
 }
 
@@ -841,7 +830,7 @@ export function experimental_getSdkPackage<
 >(context: SdkContext<TOptions, TServiceOperation>): SdkPackage<TServiceOperation> {
   const diagnostics = createDiagnosticCollector();
   const modelsAndEnums = diagnostics.pipe(getAllModelsWithDiagnostics(context));
-  context.__clients = new Map<string, SdkClientType<TServiceOperation>>();
+  context.__clients = new Array<SdkClientType<TServiceOperation>>();
   for (const client of listClients(context)) {
     createSdkClientType(context, client);
   }

diff --git a/packages/typespec-client-generator-core/test/package.test.ts b/packages/typespec-client-generator-core/test/package.test.ts
@@ -539,11 +539,12 @@ describe("typespec-client-generator-core: package", () => {
         }
       `);
       const sdkPackage = runner.context.experimental_sdkPackage;
-      strictEqual(sdkPackage.clients.length, 3);
+      strictEqual(sdkPackage.clients.length, 4);
 
       const mainClient = sdkPackage.clients.find((c) => c.name === "TestServiceClient")!;
       const fooClient = sdkPackage.clients.find((c) => c.name === "Foo")!;
-      const barClient = sdkPackage.clients.find((c) => c.name === "Bar")!;
+      const fooBarClient = sdkPackage.clients.filter((c) => c.name === "Bar")![0];
+      const barClient = sdkPackage.clients.filter((c) => c.name === "Bar")![1];
 
       strictEqual(mainClient.methods.length, 2);
       strictEqual(mainClient.initialization!.properties.length, 1);
@@ -565,18 +566,23 @@ describe("typespec-client-generator-core: package", () => {
 
       strictEqual(fooClient.initialization, undefined);
       strictEqual(fooClient.methods.length, 1);
-      strictEqual(fooClient.methods[0].kind, "clientaccessor");
-      strictEqual(fooClient.methods[0].access, "internal");
-      strictEqual(fooClient.methods[0].name, "getBar");
-      strictEqual(fooClient.methods[0].parameters.length, 0);
-      strictEqual(fooClient.methods[0].response, barClient);
+
+      const fooBarAccessor = fooClient.methods[0];
+      strictEqual(fooBarAccessor.kind, "clientaccessor");
+      strictEqual(fooBarAccessor.access, "internal");
+      strictEqual(fooBarAccessor.name, "getBar");
+      strictEqual(fooBarAccessor.parameters.length, 0);
+      strictEqual(fooBarAccessor.response, fooBarClient);
+
+      strictEqual(fooBarClient.initialization, undefined);
+      strictEqual(fooBarClient.methods.length, 1);
+      strictEqual(fooBarClient.methods[0].kind, "basic");
+      strictEqual(fooBarClient.methods[0].name, "one");
 
       strictEqual(barClient.initialization, undefined);
-      strictEqual(barClient.methods.length, 2);
+      strictEqual(barClient.methods.length, 1);
       strictEqual(barClient.methods[0].kind, "basic");
-      strictEqual(barClient.methods[0].name, "one");
-      strictEqual(barClient.methods[1].kind, "basic");
-      strictEqual(barClient.methods[1].name, "two");
+      strictEqual(barClient.methods[0].name, "two");
     });
 
     function getServiceNoDefaultApiVersion(op: string) {