fix: add variable polling times based on comp. set (#520)

* fix: add variable polling times based on comp. set

* chore: review changes

Co-authored-by: Shane McLaughlin <[email protected]>

Author: WillieRuemmele

src/client/metadataTransfer.ts

@@ -14,6 +14,7 @@ import { MetadataConverter, SfdxFileFormat } from '../convert';
 import { MetadataTransferError } from '../errors';
 import { ComponentSet } from '../collections';
 import { AsyncResult, MetadataRequestStatus, MetadataTransferResult, RequestStatus } from './types';
+
 export interface MetadataTransferOptions {
   usernameOrConnection: string | Connection;
   components?: ComponentSet;
@@ -66,7 +67,7 @@ export abstract class MetadataTransfer<Status extends MetadataRequestStatus, Res
   public async pollStatus(options?: Partial<PollingClient.Options>): Promise<Result>;
   /**
    * Poll for the status of the metadata transfer request.
-   * Default frequency is 100 ms.
+   * Default frequency is based on the number of SourceComponents, n, in the transfer, it ranges from 100ms -> n
    * Default timeout is 60 minutes.
    *
    * @param frequency Polling frequency in milliseconds.
@@ -79,7 +80,7 @@ export abstract class MetadataTransfer<Status extends MetadataRequestStatus, Res
     timeout?: number
   ): Promise<Result> {
     let pollingOptions: PollingClient.Options = {
-      frequency: Duration.milliseconds(100),
+      frequency: Duration.milliseconds(this.calculatePollingFrequency()),
       timeout: Duration.minutes(60),
       // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
       poll: this.poll.bind(this),
@@ -92,6 +93,10 @@ export abstract class MetadataTransfer<Status extends MetadataRequestStatus, Res
     if (isNumber(timeout)) {
       pollingOptions.timeout = Duration.seconds(timeout);
     }
+    // from the overloaded methods, there's a possibility frequency/timeout isn't set
+    // guarantee frequency and timeout are set
+    pollingOptions.frequency ??= Duration.milliseconds(this.calculatePollingFrequency());
+    pollingOptions.timeout ??= Duration.minutes(60);
 
     const pollingClient = await PollingClient.create(pollingOptions);
 
@@ -216,6 +221,28 @@ export abstract class MetadataTransfer<Status extends MetadataRequestStatus, Res
     return { completed, payload: mdapiStatus as unknown as AnyJson };
   }
 
+  /**
+   * Based on the source components in the component set, it will return a polling frequency in milliseconds
+   */
+  private calculatePollingFrequency(): number {
+    const size = this.components?.getSourceComponents().toArray().length || 0;
+    // take a piece-wise approach to encapsulate discrete deployment sizes in polling frequencies that "feel" good when deployed
+    if (size === 0) {
+      // no component set size is possible for retrieve
+      return 1000;
+    } else if (size <= 10) {
+      return 100;
+    } else if (size <= 50) {
+      return 250;
+    } else if (size <= 100) {
+      return 500;
+    } else if (size <= 1000) {
+      return 1000;
+    } else {
+      return size;
+    }
+  }
+
   public abstract checkStatus(): Promise<Status>;
   public abstract cancel(): Promise<void>;
   protected abstract pre(): Promise<AsyncResult>;

test/client/metadataTransfer.test.ts

@@ -7,9 +7,9 @@
 import { fail } from 'assert';
 import { createSandbox, SinonStub } from 'sinon';
 import { MockTestOrgData, testSetup } from '@salesforce/core/lib/testSetup';
-import { AuthInfo, Connection } from '@salesforce/core';
+import { AuthInfo, Connection, PollingClient } from '@salesforce/core';
 import { expect } from 'chai';
-import { sleep } from '@salesforce/kit';
+import { Duration, sleep } from '@salesforce/kit';
 import { ComponentSet } from '../../src';
 import { MetadataTransfer } from '../../src/client/metadataTransfer';
 import { MetadataRequestStatus, MetadataTransferResult, RequestStatus } from '../../src/client/types';
@@ -163,7 +163,68 @@ describe('MetadataTransfer', () => {
       expect(listenerStub.callCount).to.equal(1);
     });
 
-    it('should wait for polling function to return before queing another', async () => {
+    it('should calculate polling frequency based on source components, 0 -> 1000', async () => {
+      const pollingClientStub = env.stub(PollingClient, 'create').resolves(PollingClient.prototype);
+      env.stub(PollingClient.prototype, 'subscribe').resolves({ status: RequestStatus.Canceled, done: true });
+      const { checkStatus } = operation.lifecycle;
+      checkStatus.resolves({ status: RequestStatus.Canceled, done: true });
+
+      operation.onCancel(() => listenerStub());
+      await operation.pollStatus();
+
+      expect(pollingClientStub.calledOnce).to.be.true;
+      expect((pollingClientStub.firstCall.args[0] as { frequency: number }).frequency).to.deep.equal(
+        Duration.milliseconds(1000)
+      );
+    });
+
+    it('should calculate polling frequency based on source components, 10 -> 100', async () => {
+      // @ts-ignore protected member access
+      env.stub(operation.components, 'getSourceComponents').returns({
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore only override length attribute
+        toArray: () => {
+          return { length: 10 };
+        },
+      });
+      const pollingClientStub = env.stub(PollingClient, 'create').resolves(PollingClient.prototype);
+      env.stub(PollingClient.prototype, 'subscribe').resolves({ status: RequestStatus.Canceled, done: true });
+      const { checkStatus } = operation.lifecycle;
+      checkStatus.resolves({ status: RequestStatus.Canceled, done: true });
+
+      operation.onCancel(() => listenerStub());
+      await operation.pollStatus({ frequency: undefined, timeout: Duration.minutes(60) });
+
+      expect(pollingClientStub.calledOnce).to.be.true;
+      expect((pollingClientStub.firstCall.args[0] as { frequency: number }).frequency).to.deep.equal(
+        Duration.milliseconds(100)
+      );
+    });
+
+    it('should calculate polling frequency based on source components, 2520 -> 2520', async () => {
+      // @ts-ignore protected member access
+      env.stub(operation.components, 'getSourceComponents').returns({
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore only override length attribute
+        toArray: () => {
+          return { length: 2520 };
+        },
+      });
+      const pollingClientStub = env.stub(PollingClient, 'create').resolves(PollingClient.prototype);
+      env.stub(PollingClient.prototype, 'subscribe').resolves({ status: RequestStatus.Canceled, done: true });
+      const { checkStatus } = operation.lifecycle;
+      checkStatus.resolves({ status: RequestStatus.Canceled, done: true });
+
+      operation.onCancel(() => listenerStub());
+      await operation.pollStatus(undefined, 60);
+
+      expect(pollingClientStub.calledOnce).to.be.true;
+      expect((pollingClientStub.firstCall.args[0] as { frequency: number }).frequency).to.deep.equal(
+        Duration.milliseconds(2520)
+      );
+    });
+
+    it('should wait for polling function to return before queuing another', async () => {
       const { checkStatus } = operation.lifecycle;
       const checkStatusRuntime = 50;
       const callOrder: string[] = [];