feat: sourcemember polling like toolbelt

Author: mshanemc

.gitignore

@@ -40,4 +40,4 @@ node_modules
 .DS_Store
 .idea
 
-testProj
+testProj*

README.md

@@ -15,7 +15,7 @@ You should use the class named sourceTracking.
 
 ## TODO
 
-Push can have partial successes and needs a proper status code ex:
+Push can have partial successes but rolls back. Filter those out, except if they deployed, then we need to commit only the successes. It's...complicated
 
 ```json
 {
@@ -95,11 +95,9 @@ Push can have partial successes and needs a proper status code ex:
 }
 ```
 
-- push: ignoreWarnings logic? What is this actually doing originally?
 - push/pull throw proper error for conflicts (with label!)
-- polling for source tracking to complete (use it, w/ rewrite)
-- RSTS: don't use trackSourceMembers with empty sourceMembers array (equivalent of sync all)
-- push/pull proper table output
+- push/pull proper table output (exists over in plugin-source so why redo it here?)
+- don't use fs.promises (won't support node12)
 
 - SDR sets all retrieve FileResponse as `Changed` even if it didn't exist locally. That's going to yield slightly different json output on a `pull` than toolbelt did. See `remoteChanges.nut.ts > remote changes:add > can pull the add`. Fixing in pull is less optimal than fixing in SDR (because source:retrieve is also currently reporting those as `Changed` instead of `Created`)
 

package.json

@@ -66,7 +66,7 @@
   },
   "devDependencies": {
     "@oclif/plugin-command-snapshot": "^2.2.2",
-    "@salesforce/cli-plugins-testkit": "^1.2.11",
+    "@salesforce/cli-plugins-testkit": "^1.3.7",
     "@salesforce/dev-config": "^2.1.2",
     "@salesforce/dev-scripts": "^0.9.18",
     "@salesforce/plugin-command-reference": "^1.3.4",

src/shared/remoteSourceTrackingService.ts

@@ -9,10 +9,11 @@
 
 import * as path from 'path';
 import * as fs from 'fs';
+import { retryDecorator } from 'ts-retry-promise';
 import { ConfigFile, Logger, Org, SfdxError, Messages } from '@salesforce/core';
+import { ComponentStatus } from '@salesforce/source-deploy-retrieve';
 import { Dictionary, Optional } from '@salesforce/ts-types';
-import { Duration, env, toNumber } from '@salesforce/kit';
-import { retryDecorator } from 'ts-retry-promise';
+import { env, toNumber } from '@salesforce/kit';
 import { RemoteSyncInput } from '../shared/types';
 import { getMetadataKeyFromFileResponse } from './metadataKeys';
 
@@ -312,27 +313,15 @@ export class RemoteSourceTrackingService extends ConfigFile<RemoteSourceTracking
   }
   // Adds the given SourceMembers to the list of tracked MemberRevisions, optionally updating
   // the lastRetrievedFromServer field (sync), and persists the changes to maxRevision.json.
-  public async trackSourceMembers(sourceMembers: SourceMember[] = [], sync = false): Promise<void> {
+  public async trackSourceMembers(sourceMembers: SourceMember[], sync = false): Promise<void> {
+    if (sourceMembers.length === 0) {
+      return;
+    }
     const quiet = sourceMembers.length > 100;
     if (quiet) {
       this.logger.debug(`Upserting ${sourceMembers.length} SourceMembers to maxRevision.json`);
     }
 
-    // A sync with empty sourceMembers means "update all currently tracked elements".
-    // This is what happens during a source:pull
-    if (!sourceMembers.length && sync) {
-      const trackedRevisions = this.getSourceMembers();
-      for (const key of Object.keys(trackedRevisions)) {
-        const member = trackedRevisions[key];
-        if (member) {
-          member.lastRetrievedFromServer = member.serverRevisionCounter;
-          trackedRevisions[key] = member;
-        }
-      }
-      await this.write();
-      return;
-    }
-
     let serverMaxRevisionCounter = this.getServerMaxRevision();
     sourceMembers.forEach((change) => {
       // try accessing the sourceMembers object at the index of the change's name
@@ -432,67 +421,77 @@ export class RemoteSourceTrackingService extends ConfigFile<RemoteSourceTracking
    * @param expectedMemberNames Array of metadata names to poll
    * @param pollingTimeout maximum amount of time in seconds to poll for SourceMembers
    */
-  public async pollForSourceTracking(
-    expectedMemberNames: string[],
-    pollingTimeout?: Duration.Unit.SECONDS
-  ): Promise<void> {
+  public async pollForSourceTracking(expectedMembers: RemoteSyncInput[]): Promise<void> {
     if (env.getBoolean('SFDX_DISABLE_SOURCE_MEMBER_POLLING', false)) {
       this.logger.warn('Not polling for SourceMembers since SFDX_DISABLE_SOURCE_MEMBER_POLLING = true.');
       return;
     }
 
-    if (expectedMemberNames.length === 0) {
+    if (expectedMembers.length === 0) {
       // Don't bother polling if we're not matching SourceMembers
       return;
     }
-
-    const overriddenTimeout = toNumber(env.getString('SFDX_SOURCE_MEMBER_POLLING_TIMEOUT', '0'));
-    if (overriddenTimeout > 0) {
-      this.logger.debug(`Overriding SourceMember polling timeout to ${overriddenTimeout}`);
-      pollingTimeout = overriddenTimeout;
-    }
-
-    // Calculate a polling timeout for SourceMembers based on the number of
-    // member names being polled plus a buffer of 5 seconds.  This will
-    // wait 50s for each 1000 components, plus 5s.
-    if (!pollingTimeout) {
-      pollingTimeout = Math.ceil(expectedMemberNames.length * 0.05) + 5;
-      this.logger.debug(`Computed SourceMember polling timeout of ${pollingTimeout}s`);
-    }
-
+    const adjustedExpectedMembers = expectedMembers.filter(
+      (fileResponse) =>
+        // unchanged files will never be in the sourceMembers.  Not really sure why SDR returns them.
+        fileResponse.state !== ComponentStatus.Unchanged &&
+        // aura meta.xml aren't tracked as SourceMembers
+        !(
+          fileResponse.filePath?.startsWith('AuraDefinition') &&
+          fileResponse.filePath?.endsWith('.cmp-meta.xml') &&
+          // if a listView is the only change inside an object, the object won't have a sourceMember change.  We won't wait for those to be found
+          fileResponse.type !== 'CustomObject'
+        )
+    );
     const fromRevision = this.getServerMaxRevision();
+    const pollingTimeout = this.calculateTimeout(adjustedExpectedMembers.length);
     this.logger.debug(
-      `Polling for ${expectedMemberNames.length} SourceMembers from revision ${fromRevision} with timeout of ${pollingTimeout}s`
+      `Polling for ${adjustedExpectedMembers.length} SourceMembers from revision ${fromRevision} with timeout of ${pollingTimeout}s`
     );
 
-    let pollAttempts = 0;
+    const outstandingSourceMembers = new Map();
 
-    // we need to keep asking the server for sourceMembers until time runs out OR we find everything in matches
-    const expectedSourceMembers = new Set(expectedMemberNames);
-    let sourceMembers;
-    const poll = async (): Promise<SourceMember[]> => {
+    // filter known Source tracking issues
+    adjustedExpectedMembers.map((member) => {
+      getMetadataKeyFromFileResponse(member).map((key) => outstandingSourceMembers.set(key, member));
+    });
+
+    let pollAttempts = 0;
+    const poll = async (): Promise<void> => {
       pollAttempts += 1; // not used to stop polling, but for debug logging
-      sourceMembers = await this.querySourceMembersFrom({
+
+      // get sourceMembers since maxRevision
+      const queriedMembers = await this.querySourceMembersFrom({
         fromRevision,
         quiet: pollAttempts !== 1,
         useCache: false,
       });
 
-      for (const member of sourceMembers) {
-        expectedSourceMembers.delete(member.MemberName);
-      }
+      // remove anything returned from the query list
+      queriedMembers.map((member) => {
+        outstandingSourceMembers.delete(getMetadataKey(member.MemberType, member.MemberName));
+      });
 
       this.logger.debug(
-        `[${pollAttempts}] Found ${expectedMemberNames.length - expectedSourceMembers.size} of ${
-          expectedMemberNames.length
+        `[${pollAttempts}] Found ${adjustedExpectedMembers.length - outstandingSourceMembers.size} of ${
+          adjustedExpectedMembers.length
         } SourceMembers`
       );
-      if (expectedSourceMembers.size === 0) {
-        return sourceMembers;
+
+      // update but don't sync
+      await this.trackSourceMembers(queriedMembers, false);
+
+      // exit if all have returned
+      if (outstandingSourceMembers.size === 0) {
+        return;
       }
 
-      if (expectedSourceMembers.size < 20) {
-        this.logger.debug(`Still looking for SourceMembers: ${Array.from(expectedSourceMembers).join(',')}`);
+      if (outstandingSourceMembers.size < 20) {
+        this.logger.debug(
+          outstandingSourceMembers.size < 20
+            ? `Still looking for SourceMembers: ${Array.from(outstandingSourceMembers.keys()).join(',')}`
+            : `Still looking for ${outstandingSourceMembers.size} Source Members`
+        );
       }
       throw new Error();
     };
@@ -506,21 +505,31 @@ export class RemoteSourceTrackingService extends ConfigFile<RemoteSourceTracking
       this.logger.debug(`Retrieved all SourceMember data after ${pollAttempts} attempts`);
     } catch {
       this.logger.warn(`Polling for SourceMembers timed out after ${pollAttempts} attempts`);
-      if (expectedSourceMembers.size < 51) {
+      if (outstandingSourceMembers.size < 51) {
         this.logger.debug(
-          `Could not find ${expectedSourceMembers.size} SourceMembers: ${Array.from(expectedSourceMembers).join(',')}`
+          `Could not find ${outstandingSourceMembers.size} SourceMembers: ${Array.from(outstandingSourceMembers).join(
+            ','
+          )}`
         );
       } else {
-        this.logger.debug(`Could not find SourceMembers for ${expectedSourceMembers.size} components`);
+        this.logger.debug(`Could not find SourceMembers for ${outstandingSourceMembers.size} components`);
       }
     }
+  }
+
+  private calculateTimeout(memberCount: number): number {
+    const overriddenTimeout = toNumber(env.getString('SFDX_SOURCE_MEMBER_POLLING_TIMEOUT', '0'));
+    if (overriddenTimeout > 0) {
+      this.logger.debug(`Overriding SourceMember polling timeout to ${overriddenTimeout}`);
+      return overriddenTimeout;
+    }
 
-    // NOTE: we are updating tracking for every SourceMember returned by the query once we match all memberNames
-    //       passed OR polling times out.  This does not update SourceMembers of *only* the memberNames passed.
-    //       This means if we ever want to support tracking on source:deploy or source:retrieve we would need
-    //       to update tracking for only the matched SourceMembers.  I.e., call trackSourceMembers() passing
-    //       only the SourceMembers that match the memberNames.
-    await this.trackSourceMembers(sourceMembers, true);
+    // Calculate a polling timeout for SourceMembers based on the number of
+    // member names being polled plus a buffer of 5 seconds.  This will
+    // wait 50s for each 1000 components, plus 5s.
+    const pollingTimeout = Math.ceil(memberCount * 0.05) + 5;
+    this.logger.debug(`Computed SourceMember polling timeout of ${pollingTimeout}s`);
+    return pollingTimeout;
   }
 
   private async querySourceMembersFrom({
@@ -562,8 +571,11 @@ export class RemoteSourceTrackingService extends ConfigFile<RemoteSourceTracking
       this.logger.debug(query);
     }
 
-    const results = await this.org.getConnection().tooling.autoFetchQuery<T>(query);
-
-    return results.records;
+    try {
+      const results = await this.org.getConnection().tooling.autoFetchQuery<T>(query);
+      return results.records;
+    } catch (error) {
+      throw SfdxError.wrap(error as Error);
+    }
   }
 }

src/shared/types.ts

@@ -7,6 +7,6 @@
 
 import { FileResponse } from '@salesforce/source-deploy-retrieve';
 
-export type RemoteSyncInput = Pick<FileResponse, 'fullName' | 'filePath' | 'type'>;
+export type RemoteSyncInput = Pick<FileResponse, 'fullName' | 'filePath' | 'type' | 'state'>;
 
 export type PushPullResponse = Pick<FileResponse, 'filePath' | 'fullName' | 'state' | 'type'>;

src/sourceTracking.ts

@@ -157,7 +157,7 @@ export class SourceTracking extends AsyncCreatable {
         files: successNonDeletes.map((fileResponse) => fileResponse.filePath) as string[],
         deletedFiles: successDeletes.map((fileResponse) => fileResponse.filePath) as string[],
       }),
-      this.updateRemoteTracking(successes), // this includes polling for sourceMembers
+      this.updateRemoteTracking(successes), // this should include polling for sourceMembers
     ]);
 
     return result.getFileResponses();
@@ -251,7 +251,12 @@ export class SourceTracking extends AsyncCreatable {
         files: successes.map((fileResponse) => fileResponse.filePath as string).filter(Boolean),
       }),
       this.updateRemoteTracking(
-        successes.map((success) => ({ filePath: success.filePath, type: success.type, fullName: success.fullName }))
+        successes.map((success) => ({
+          filePath: success.filePath,
+          type: success.type,
+          fullName: success.fullName,
+          state: success.state,
+        }))
       ),
     ]);
     return [...deletesAsFileResponse, ...retrieveResult.getFileResponses()];
@@ -349,13 +354,16 @@ export class SourceTracking extends AsyncCreatable {
   }
 
   /**
-   * Mark remote source tracking files that we have received to the latest version
+   * Mark remote source tracking files so say that we have received the latest version from the server
+   * Optionall skip polling for the SourceMembers to exist on the server and be updated in local files
    */
-  public async updateRemoteTracking(fileResponses: RemoteSyncInput[]): Promise<void> {
-    await this.ensureRemoteTracking();
-    // TODO: poll for source tracking to be complete
-    // to make sure we have the updates before syncing the ones from metadataKeys
-    await this.remoteSourceTrackingService.retrieveUpdates({ cache: false });
+  public async updateRemoteTracking(fileResponses: RemoteSyncInput[], skipPolling = false): Promise<void> {
+    // false to explicitly NOT query until we do the polling
+    await this.ensureRemoteTracking(false);
+    if (!skipPolling) {
+      // poll to make sure we have the updates before syncing the ones from metadataKeys
+      await this.remoteSourceTrackingService.pollForSourceTracking(fileResponses);
+    }
     await this.remoteSourceTrackingService.syncSpecifiedElements(fileResponses);
   }
 

test/nuts/commands/basics.nut.ts

@@ -11,20 +11,25 @@ import * as path from 'path';
 import * as fs from 'fs';
 
 import { TestSession, execCmd } from '@salesforce/cli-plugins-testkit';
+import { Env } from '@salesforce/kit';
+import { ensureString } from '@salesforce/ts-types';
 import { FileResponse } from '@salesforce/source-deploy-retrieve';
 import { expect } from 'chai';
 import { StatusResult } from '../../../src/commands/force/source/status';
 
 let session: TestSession;
-
+let hubUsername: string;
 describe('end-to-end-test for tracking with an org (single packageDir)', () => {
+  const env = new Env();
+
   before(async () => {
     session = await TestSession.create({
       project: {
         sourceDir: path.join('test', 'nuts', 'ebikes-lwc'),
       },
       setupCommands: [`sfdx force:org:create -d 1 -s -f ${path.join('config', 'project-scratch-def.json')}`],
     });
+    hubUsername = ensureString(env.getString('TESTKIT_HUB_USERNAME'));
   });
 
   after(async () => {
@@ -105,7 +110,13 @@ describe('end-to-end-test for tracking with an org (single packageDir)', () => {
   });
 
   describe('non-successes', () => {
-    it('should throw an err when attempting to pull from a non scratch-org');
+    it('should throw an err when attempting to pull from a non scratch-org', () => {
+      // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
+      const failure = execCmd(`force:source:status -u ${hubUsername} --remote --json`, {
+        ensureExitCode: 1,
+      }).jsonOutput;
+      expect(failure.name).to.equal('NonSourceTrackedOrgError');
+    });
     it('should not poll for SourceMembers when SFDX_DISABLE_SOURCE_MEMBER_POLLING=true');
 
     describe('push partial success', () => {