fix(functions): store CLOUD_TASKS_EMULATOR_HOST at construction time (#3167)

* fix(functions): store CLOUD_TASKS_EMULATOR_HOST at construction time

* fix: address gemini review

Author: jonathanedey

src/functions/functions-api-client-internal.ts

@@ -46,6 +46,7 @@ const DEFAULT_LOCATION = 'us-central1';
  */
 export class FunctionsApiClient {
   private readonly httpClient: HttpClient;
+  private readonly emulatorHost?: string;
   private projectId?: string;
   private accountId?: string;
 
@@ -56,7 +57,9 @@ export class FunctionsApiClient {
         message: 'First argument passed to getFunctions() must be a valid Firebase app instance.'
       });
     }
-    this.httpClient = new FunctionsHttpClient(app as FirebaseApp);
+    const emulatorHost = process.env.CLOUD_TASKS_EMULATOR_HOST?.trim();
+    this.emulatorHost = emulatorHost || undefined;
+    this.httpClient = new FunctionsHttpClient(app as FirebaseApp, this.emulatorHost);
   }
   /**
    * Deletes a task from a queue.
@@ -103,7 +106,7 @@ export class FunctionsApiClient {
     }
 
     try {
-      const serviceUrl = tasksEmulatorUrl(resources)?.concat('/', id)
+      const serviceUrl = tasksEmulatorUrl(resources, this.emulatorHost)?.concat('/', id)
         ?? await this.getUrl(resources, CLOUD_TASKS_API_URL_FORMAT.concat('/', id));
       const request: HttpRequestConfig = {
         method: 'DELETE',
@@ -165,7 +168,7 @@ export class FunctionsApiClient {
     const task = this.validateTaskOptions(data, resources, opts);
     try {
       const serviceUrl =
-        tasksEmulatorUrl(resources) ??
+        tasksEmulatorUrl(resources, this.emulatorHost) ??
         await this.getUrl(resources, CLOUD_TASKS_API_URL_FORMAT);
 
       const taskPayload = await this.updateTaskPayload(task, resources, extensionId);
@@ -347,7 +350,7 @@ export class FunctionsApiClient {
   }
 
   private async updateTaskPayload(task: Task, resources: utils.ParsedResource, extensionId?: string): Promise<Task> {
-    const defaultUrl = process.env.CLOUD_TASKS_EMULATOR_HOST ?
+    const defaultUrl = this.emulatorHost ?
       ''
       : await this.getUrl(resources, FIREBASE_FUNCTION_URL_FORMAT);
 
@@ -368,7 +371,7 @@ export class FunctionsApiClient {
         const account = await this.getServiceAccount();
         task.httpRequest.oidcToken = { serviceAccountEmail: account };
       } catch (e) {
-        if (process.env.CLOUD_TASKS_EMULATOR_HOST) {
+        if (this.emulatorHost) {
           task.httpRequest.oidcToken = { serviceAccountEmail: EMULATED_SERVICE_ACCOUNT_DEFAULT };
         } else {
           throw e;
@@ -408,8 +411,12 @@ export class FunctionsApiClient {
  * when communicating with the Emulator.
  */
 class FunctionsHttpClient extends AuthorizedHttpClient {
+  constructor(app: FirebaseApp, private readonly emulatorHost?: string) {
+    super(app);
+  }
+
   protected getToken(): Promise<string> {
-    if (process.env.CLOUD_TASKS_EMULATOR_HOST) {
+    if (this.emulatorHost) {
       return Promise.resolve('owner');
     }
     return super.getToken();
@@ -448,9 +455,9 @@ export interface Task {
   };
 }
 
-function tasksEmulatorUrl(resources: utils.ParsedResource): string | undefined {
-  if (process.env.CLOUD_TASKS_EMULATOR_HOST) {
-    return `http://${process.env.CLOUD_TASKS_EMULATOR_HOST}/projects/${resources.projectId}/locations/${resources.locationId}/queues/${resources.resourceId}/tasks`;
+function tasksEmulatorUrl(resources: utils.ParsedResource, emulatorHost?: string): string | undefined {
+  if (emulatorHost) {
+    return `http://${emulatorHost}/projects/${resources.projectId}/locations/${resources.locationId}/queues/${resources.resourceId}/tasks`;
   }
   return undefined;
 }

test/unit/functions/functions-api-client-internal.spec.ts

@@ -130,6 +130,56 @@ describe('FunctionsApiClient', () => {
       expect(() => new FunctionsApiClient(null as unknown as FirebaseApp))
         .to.throw('First argument passed to getFunctions() must be a valid Firebase app instance.');
     });
+
+    it('should cache CLOUD_TASKS_EMULATOR_HOST at construction time', async () => {
+      delete process.env.CLOUD_TASKS_EMULATOR_HOST;
+      const prodClient = new FunctionsApiClient(app);
+
+      process.env.CLOUD_TASKS_EMULATOR_HOST = CLOUD_TASKS_EMULATOR_HOST;
+      const emulatorClient = new FunctionsApiClient(app);
+
+      delete process.env.CLOUD_TASKS_EMULATOR_HOST;
+
+      const sendStub = sinon
+        .stub(HttpClient.prototype, 'send')
+        .resolves(utils.responseFrom({}, 200));
+      stubs.push(sendStub);
+
+      await prodClient.delete('mock-task', FUNCTION_NAME);
+      await emulatorClient.delete('mock-task', FUNCTION_NAME);
+
+      expect(sendStub).to.have.been.calledTwice;
+      expect(sendStub.firstCall).to.have.been.calledWith({
+        method: 'DELETE',
+        url: CLOUD_TASKS_URL.concat('/', 'mock-task'),
+        headers: EXPECTED_HEADERS,
+      });
+      expect(sendStub.secondCall).to.have.been.calledWith({
+        method: 'DELETE',
+        url: CLOUD_TASKS_URL_EMULATOR.concat('/', 'mock-task'),
+        headers: EXPECTED_HEADERS_EMULATOR,
+      });
+    });
+
+    for (const hostVal of ['', '   ']) {
+      it(`should ignore CLOUD_TASKS_EMULATOR_HOST when set to "${hostVal}"`, async () => {
+        process.env.CLOUD_TASKS_EMULATOR_HOST = hostVal;
+        const emptyHostClient = new FunctionsApiClient(app);
+        delete process.env.CLOUD_TASKS_EMULATOR_HOST;
+
+        const stub = sinon
+          .stub(HttpClient.prototype, 'send')
+          .resolves(utils.responseFrom({}, 200));
+        stubs.push(stub);
+
+        await emptyHostClient.delete('mock-task', FUNCTION_NAME);
+        expect(stub).to.have.been.calledWith({
+          method: 'DELETE',
+          url: CLOUD_TASKS_URL.concat('/', 'mock-task'),
+          headers: EXPECTED_HEADERS,
+        });
+      });
+    }
   });
 
   describe('enqueue', () => {
@@ -529,6 +579,7 @@ describe('FunctionsApiClient', () => {
         .resolves(utils.responseFrom({}, 200));
       stubs.push(stub);
       process.env.CLOUD_TASKS_EMULATOR_HOST = CLOUD_TASKS_EMULATOR_HOST;
+      apiClient = new FunctionsApiClient(app);
       return apiClient.enqueue({}, FUNCTION_NAME, '', { uri: TEST_TASK_PAYLOAD.httpRequest.url })
         .then(() => {
           expect(stub).to.have.been.calledOnce.and.calledWith({
@@ -550,6 +601,7 @@ describe('FunctionsApiClient', () => {
         .resolves(utils.responseFrom({}, 200));
       stubs.push(stub);
       process.env.CLOUD_TASKS_EMULATOR_HOST = CLOUD_TASKS_EMULATOR_HOST;
+      apiClient = new FunctionsApiClient(app);
       return apiClient.enqueue({}, FUNCTION_NAME)
         .then(() => {
           expect(stub).to.have.been.calledOnce.and.calledWith({
@@ -569,7 +621,6 @@ describe('FunctionsApiClient', () => {
         projectId: 'test-project',
         serviceAccountId: ''
       });
-      apiClient = new FunctionsApiClient(app);
 
       const expectedPayload = deepCopy(TEST_TASK_PAYLOAD);
       expectedPayload.httpRequest.oidcToken = { serviceAccountEmail: EMULATED_SERVICE_ACCOUNT_DEFAULT };
@@ -578,6 +629,7 @@ describe('FunctionsApiClient', () => {
         .resolves(utils.responseFrom({}, 200));
       stubs.push(stub);
       process.env.CLOUD_TASKS_EMULATOR_HOST = CLOUD_TASKS_EMULATOR_HOST;
+      apiClient = new FunctionsApiClient(app);
       return apiClient.enqueue({}, FUNCTION_NAME, '', { uri: TEST_TASK_PAYLOAD.httpRequest.url })
         .then(() => {
           expect(stub).to.have.been.calledOnce.and.calledWith({
@@ -631,6 +683,7 @@ describe('FunctionsApiClient', () => {
 
     it('should redirect to the emulator when CLOUD_TASKS_EMULATOR_HOST is set', async () => {
       process.env.CLOUD_TASKS_EMULATOR_HOST = CLOUD_TASKS_EMULATOR_HOST;
+      apiClient = new FunctionsApiClient(app);
       const stub = sinon
         .stub(HttpClient.prototype, 'send')
         .resolves(utils.responseFrom({}, 200));