Merge pull request #766 from forcedotcom/sm/cache-limits

@W-18349042 fix: keep caches from growing unbounded in multitenant environments

Author: shetzel

package.json

@@ -1,7 +1,7 @@
 {
   "name": "@salesforce/source-tracking",
   "description": "API for tracking local and remote Salesforce metadata changes",
-  "version": "7.3.22",
+  "version": "7.3.23-qa.1",
   "author": "Salesforce",
   "license": "Apache-2.0",
   "main": "lib/index.js",
@@ -51,9 +51,9 @@
   },
   "dependencies": {
     "@oclif/core": "^4.2.10",
-    "@salesforce/core": "^8.8.7",
+    "@salesforce/core": "^8.9.1",
     "@salesforce/kit": "^3.2.3",
-    "@salesforce/source-deploy-retrieve": "^12.19.0",
+    "@salesforce/source-deploy-retrieve": "^12.19.3",
     "@salesforce/ts-types": "^2.0.12",
     "fast-xml-parser": "^4.5.3",
     "graceful-fs": "^4.2.11",

src/shared/remote/remoteSourceTrackingService.ts

@@ -37,6 +37,9 @@ const POLLING_DELAY_MS = 1000;
 const CONSECUTIVE_EMPTY_POLLING_RESULT_LIMIT =
   (env.getNumber('SF_SOURCE_MEMBER_POLLING_TIMEOUT') ?? 120) / Duration.milliseconds(POLLING_DELAY_MS).seconds;
 
+/** if a cached instance is older than this, it will be purged */
+const MAX_INSTANCE_CACHE_TTL = 1000 * 60 * 60 * 1; // 1 hour
+
 /** Options for RemoteSourceTrackingService.getInstance */
 export type RemoteSourceTrackingServiceOptions = {
   org: Org;
@@ -73,15 +76,20 @@ export type RemoteSourceTrackingServiceOptions = {
  * In this example, `ApexClass###MyClass` has been changed in the org because the `serverRevisionCounter` is different
  * from the `lastRetrievedFromServer`. When a pull is performed, all of the pulled members will have their counters set
  * to the corresponding `RevisionCounter` from the `SourceMember` of the org.
- * 
- * Tracking files are written to the older format described in `MemberRevisionLegacy` 
- * if the environment variable CURRENT_FILE_VERSION_ENV is not set to 1 
- * 
+ *
+ * Tracking files are written to the older format described in `MemberRevisionLegacy`
+ * if the environment variable CURRENT_FILE_VERSION_ENV is not set to 1
+ *
  * The "in memorgy" storage is in MemberRevision format.
  */
+type CachedInstance = {
+  service: RemoteSourceTrackingService;
+  lastUsed: number;
+};
+
 export class RemoteSourceTrackingService {
   /** map of constructed, init'ed instances; key is orgId.  It's like a singleton at the org level */
-  private static instanceMap = new Map<string, RemoteSourceTrackingService>();
+  private static instanceMap = new Map<string, CachedInstance>();
   public readonly filePath: string;
 
   private logger!: PinoLogger;
@@ -112,13 +120,14 @@ export class RemoteSourceTrackingService {
    */
   public static async getInstance(options: RemoteSourceTrackingServiceOptions): Promise<RemoteSourceTrackingService> {
     const orgId = options.org.getOrgId();
-    let service = this.instanceMap.get(orgId);
-    if (!service) {
-      service = await new RemoteSourceTrackingService(options).init();
-      this.instanceMap.set(orgId, service);
-    }
+    const service = this.instanceMap.get(orgId)?.service ?? (await new RemoteSourceTrackingService(options).init());
+    this.instanceMap.set(orgId, { service, lastUsed: Date.now() });
+    // when we get an instance, we make sure old ones are not accumulating.  Important in multitenant environments
+    purgeOldInstances(this.instanceMap);
+
     // even if there was already an instance around, its queries might no longer be accurate (ex: missing new changes but queryFrom would return stale results)
     service.queryCache.clear();
+    service.userQueryCache.clear();
     service.org = options.org;
     return service;
   }
@@ -537,3 +546,12 @@ const formatSourceMemberWarnings = (outstandingSourceMembers: Map<string, Remote
 
 const doesNotMatchServer = (member: MemberRevision): boolean =>
   member.RevisionCounter !== member.lastRetrievedFromServer;
+
+const purgeOldInstances = (instances: Map<string, CachedInstance>): void => {
+  const now = Date.now();
+  Array.from(instances.entries())
+    .filter(([, { lastUsed }]) => now - lastUsed > MAX_INSTANCE_CACHE_TTL)
+    .map(([orgId]) => {
+      instances.delete(orgId);
+    });
+};

test/unit/remote/remoteSourceTracking.test.ts

@@ -8,8 +8,8 @@
 /* eslint-disable camelcase */
 
 import { writeFile, mkdir, readFile } from 'node:fs/promises';
-import { existsSync } from 'node:fs';
-import { sep, dirname } from 'node:path';
+import { existsSync, rmSync } from 'node:fs';
+import { sep, dirname, resolve } from 'node:path';
 import { MockTestOrgData, instantiateContext, stubContext, restoreContext } from '@salesforce/core/testSetup';
 import { EnvVars, envVars, Messages, Org } from '@salesforce/core';
 import { expect, config } from 'chai';
@@ -74,7 +74,6 @@ type SetContentsInput = {
 };
 describe('remoteSourceTrackingService', () => {
   const username = 'foo@bar.com';
-  let orgId: string;
   const $$ = instantiateContext();
   let remoteSourceTrackingService: RemoteSourceTrackingService;
 
@@ -97,15 +96,14 @@ describe('remoteSourceTrackingService', () => {
     sourceMembers: Object.fromEntries(remoteSourceTrackingService.sourceMembers),
   });
 
-  afterEach(async () => {
-    await RemoteSourceTrackingService.delete(orgId);
+  afterEach(() => {
+    RemoteSourceTrackingService['instanceMap'].clear();
     restoreContext($$);
   });
 
   beforeEach(async () => {
     stubContext($$);
     const orgData = new MockTestOrgData();
-    orgId = orgData.orgId;
     orgData.username = username;
     orgData.tracksSource = true;
     await $$.stubAuths(orgData);
@@ -115,57 +113,57 @@ describe('remoteSourceTrackingService', () => {
       org,
       projectPath: await $$.localPathRetriever($$.id),
     });
+  });
 
-    describe('remoteChangeElementToChangeResult()', () => {
-      const memberIdOrName = '00eO4000003cP5J';
-      it('should return correct ChangeResult for EmailTemplateFolder', () => {
-        const rce: RemoteChangeElement = {
-          name: 'level1/level2/level3',
-          type: 'EmailTemplateFolder',
-          deleted: false,
-          modified: true,
-          changedBy: 'Shelby McLaughlin',
-          revisionCounter: 1,
-          memberIdOrName,
-          lastModifiedDate: defaultSourceMemberValues.LastModifiedDate,
-        };
-        const changeResult = remoteChangeElementToChangeResult(rce);
-        expect(changeResult).to.deep.equal({
-          origin: 'remote',
-          name: 'level1/level2/level3',
-          type: 'EmailFolder',
-          deleted: false,
-          modified: true,
-          changedBy: 'Shelby McLaughlin',
-          revisionCounter: 1,
-          memberIdOrName,
-          lastModifiedDate: defaultSourceMemberValues.LastModifiedDate,
-        });
+  describe('remoteChangeElementToChangeResult()', () => {
+    const memberIdOrName = '00eO4000003cP5J';
+    it('should return correct ChangeResult for EmailTemplateFolder', () => {
+      const rce: RemoteChangeElement = {
+        name: 'level1/level2/level3',
+        type: 'EmailTemplateFolder',
+        deleted: false,
+        modified: true,
+        changedBy: 'Shelby McLaughlin',
+        revisionCounter: 1,
+        memberIdOrName,
+        lastModifiedDate: defaultSourceMemberValues.LastModifiedDate,
+      };
+      const changeResult = remoteChangeElementToChangeResult(rce);
+      expect(changeResult).to.deep.equal({
+        origin: 'remote',
+        name: 'level1/level2/level3',
+        type: 'EmailFolder',
+        deleted: false,
+        modified: true,
+        changedBy: 'Shelby McLaughlin',
+        revisionCounter: 1,
+        memberIdOrName,
+        lastModifiedDate: defaultSourceMemberValues.LastModifiedDate,
       });
+    });
 
-      it('should return correct ChangeResult for LightningComponentResource', () => {
-        const rce: RemoteChangeElement = {
-          name: 'fooLWC/bar',
-          type: 'LightningComponentResource',
-          deleted: false,
-          modified: true,
-          changedBy: 'Shelby McLaughlin',
-          revisionCounter: 1,
-          memberIdOrName,
-          lastModifiedDate: defaultSourceMemberValues.LastModifiedDate,
-        };
-        const changeResult = remoteChangeElementToChangeResult(rce);
-        expect(changeResult).to.deep.equal({
-          origin: 'remote',
-          name: 'fooLWC',
-          type: 'LightningComponentBundle',
-          deleted: false,
-          modified: true,
-          changedBy: 'Shelby McLaughlin',
-          revisionCounter: 1,
-          memberIdOrName,
-          lastModifiedDate: defaultSourceMemberValues.LastModifiedDate,
-        });
+    it('should return correct ChangeResult for LightningComponentResource', () => {
+      const rce: RemoteChangeElement = {
+        name: 'fooLWC/bar',
+        type: 'LightningComponentResource',
+        deleted: false,
+        modified: true,
+        changedBy: 'Shelby McLaughlin',
+        revisionCounter: 1,
+        memberIdOrName,
+        lastModifiedDate: defaultSourceMemberValues.LastModifiedDate,
+      };
+      const changeResult = remoteChangeElementToChangeResult(rce);
+      expect(changeResult).to.deep.equal({
+        origin: 'remote',
+        name: 'fooLWC',
+        type: 'LightningComponentBundle',
+        deleted: false,
+        modified: true,
+        changedBy: 'Shelby McLaughlin',
+        revisionCounter: 1,
+        memberIdOrName,
+        lastModifiedDate: defaultSourceMemberValues.LastModifiedDate,
       });
     });
   });
@@ -178,6 +176,50 @@ describe('remoteSourceTrackingService', () => {
     });
   });
 
+  describe('getInstance', () => {
+    const org2Dir = resolve('temp2');
+    const org3Dir = resolve('temp3');
+
+    it('should purge old instances when they exceed MAX_INSTANCE_CACHE_TTL', async () => {
+      // Create second instance
+      const org2 = await Org.create({ aliasOrUsername: 'org2@test.com' });
+      $$.SANDBOX.stub(org2, 'getOrgId').returns('00D999999999999999');
+      await RemoteSourceTrackingService.getInstance({
+        org: org2,
+        projectPath: org2Dir,
+      });
+
+      // Verify both instances exist
+      expect(RemoteSourceTrackingService['instanceMap'].size).to.equal(2);
+
+      // Get a date that is over 1 hour from now
+      const MAX_INSTANCE_CACHE_TTL = 1000 * 60 * 60 * 1; // 1 hour
+      const futureDateNow = Date.now() + MAX_INSTANCE_CACHE_TTL + 1000;
+
+      // Stub Date.now to simulate time passing
+      const dateNowStub = $$.SANDBOX.stub(Date, 'now').returns(futureDateNow);
+
+      // Create a new instance which should trigger purge
+      const org3 = await Org.create({ aliasOrUsername: 'org3@test.com' });
+      $$.SANDBOX.stub(org3, 'getOrgId').returns('00D777777777777777');
+      await RemoteSourceTrackingService.getInstance({
+        org: org3,
+        projectPath: org3Dir,
+      });
+
+      // Verify old instances were purged
+      expect(RemoteSourceTrackingService['instanceMap'].size).to.equal(1);
+      expect(RemoteSourceTrackingService['instanceMap'].get('00D777777777777777')).to.be.an('object');
+      expect(dateNowStub.called).to.equal(true);
+    });
+
+    afterEach(() => {
+      // clean up the tracking files created by the tests
+      rmSync(org2Dir, { recursive: true });
+      rmSync(org3Dir, { recursive: true });
+    });
+  });
+
   describe('init', () => {
     it('should set initial state of contents', async () => {
       const queryMembersFromSpy = $$.SANDBOX.spy(orgQueryMocks, 'querySourceMembersFrom');