Merge pull request #874 from forcedotcom/wr/unsetAliasConfig

Wr/unset alias config

.github/workflows/perf.yml

@@ -0,0 +1,43 @@
+# runs some very-large-repo tests (for windows filesystem limits) and commits perf results for comparison
+name: perf-tests
+on:
+  push:
+    branches-ignore: [main]
+  workflow_dispatch:
+
+# linux will finish ahead of windows, but prevent other branches/commits from hitting simultaneously
+# since we're pushing git commits and there would be conflicts
+concurrency: perf-test
+
+jobs:
+  perf-tests:
+    strategy:
+      matrix:
+        os: [ubuntu-latest, windows-latest]
+      fail-fast: false
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v3
+      - uses: actions/setup-node@v3
+        with:
+          cache: yarn
+      - run: yarn install
+      - run: yarn build
+      - run: npm run test:perf | tee test/perf/output.txt
+
+      # Run `github-action-benchmark` action
+      - name: Store benchmark result
+        uses: benchmark-action/github-action-benchmark@5bbce78ef18edf5b96cb2d23e8d240b485f9dc4a
+        with:
+          name: Logger Benchmarks - ${{ matrix.os }}
+          tool: 'benchmarkjs'
+          output-file-path: test/perf/output.txt
+          comment-on-alert: true
+          fail-on-alert: true
+          # Push and deploy GitHub pages branch automatically
+          # this has a bug where it creates duplicate commits when summary-always and aut-push are both true
+          # summary-always: true
+          comment-always: true
+          benchmark-data-dir-path: perf-${{ runner.os}}
+          auto-push: true
+          github-token: ${{ secrets.SVC_CLI_BOT_GITHUB_TOKEN }}

.mocharc.json

@@ -4,5 +4,5 @@
   "watch-files": ["src", "test"],
   "recursive": true,
   "reporter": "spec",
-  "timeout": 10000
+  "timeout": 20000
 }

README.md

@@ -69,3 +69,13 @@ In your plugin or library,
   }
 
 ```
+
+## Performance Testing
+
+There are some benchmark.js checks to get a baseline for Logger performance.
+https://forcedotcom.github.io/sfdx-core/perf-Linux
+https://forcedotcom.github.io/sfdx-core/perf-Windows
+
+You can add more test cases in test/perf/logger/main.js
+
+If you add tests for new parts of sfdx-core outside of Logger, add new test Suites and create new jobs in the GHA `perf.yml`

package.json

@@ -22,7 +22,8 @@
     "prepack": "sf-prepack",
     "prepare": "sf-install",
     "test": "wireit",
-    "test:only": "wireit"
+    "test:only": "wireit",
+    "test:perf": "ts-node test/perf/logger/main.test.ts"
   },
   "keywords": [
     "force",
@@ -60,6 +61,7 @@
     "@salesforce/dev-scripts": "^5.4.2",
     "@salesforce/prettier-config": "^0.0.3",
     "@salesforce/ts-sinon": "^1.4.8",
+    "@types/benchmark": "^2.1.2",
     "@types/chai-string": "^1.4.2",
     "@types/debug": "0.0.31",
     "@types/jsonwebtoken": "9.0.2",
@@ -68,6 +70,7 @@
     "@types/shelljs": "0.8.12",
     "@typescript-eslint/eslint-plugin": "^5.59.9",
     "@typescript-eslint/parser": "^5.59.11",
+    "benchmark": "^2.1.4",
     "chai": "^4.3.7",
     "chai-string": "^1.5.0",
     "eslint": "^8.43.0",
@@ -171,4 +174,4 @@
       ]
     }
   }
-}
+}

src/config/configAggregator.ts

@@ -80,8 +80,8 @@ export class ConfigAggregator extends AsyncOptionalCreatable<ConfigAggregator.Op
 
   // Initialized in loadProperties
   private allowedProperties!: ConfigPropertyMeta[];
-  private localConfig?: Config;
-  private globalConfig: Config;
+  private readonly localConfig?: Config;
+  private readonly globalConfig: Config;
   private envVars: Dictionary<string> = {};
 
   /**
@@ -334,6 +334,19 @@ export class ConfigAggregator extends AsyncOptionalCreatable<ConfigAggregator.Op
     return this.config;
   }
 
+  public async unsetByValue(key: string): Promise<void> {
+    const hasLocalWrites = this.localConfig
+      ?.getKeysByValue(key)
+      .map((k) => this.localConfig?.unset(k))
+      .some(Boolean);
+    if (hasLocalWrites) await this.localConfig?.write();
+    const hasGlobalWrites = this.globalConfig
+      ?.getKeysByValue(key)
+      .map((k) => this.globalConfig?.unset(k))
+      .some(Boolean);
+    if (hasGlobalWrites) await this.globalConfig?.write();
+  }
+
   /**
    * Get the config properties that are environment variables.
    */
@@ -412,8 +425,7 @@ export class ConfigAggregator extends AsyncOptionalCreatable<ConfigAggregator.Op
     configs.push(this.envVars);
 
     const json: JsonMap = {};
-    const reduced = configs.filter(isJsonMap).reduce((acc: JsonMap, el: AnyJson) => merge(acc, el), json);
-    return reduced;
+    return configs.filter(isJsonMap).reduce((acc: JsonMap, el: AnyJson) => merge(acc, el), json);
   }
 }
 

src/org/org.ts

@@ -507,6 +507,16 @@ export class Org extends AsyncOptionalCreatable<Org.Options> {
    * Will delete 'this' instance remotely and any files locally
    */
   public async delete(): Promise<void> {
+    const username = ensureString(this.getUsername());
+
+    // unset any aliases referencing this org
+    const stateAgg = await StateAggregator.getInstance();
+    const existingAliases = stateAgg.aliases.getAll(username);
+    await stateAgg.aliases.unsetValuesAndSave(existingAliases);
+
+    // unset any configs referencing this org
+    [...existingAliases, username].flatMap((name) => this.configAggregator.unsetByValue(name));
+
     if (await this.isSandbox()) {
       await this.deleteSandbox();
     } else {
@@ -1132,6 +1142,7 @@ export class Org extends AsyncOptionalCreatable<Org.Options> {
     try {
       const devHubConn = devHub.getConnection();
       const username = this.getUsername();
+
       const activeScratchOrgRecordId = (
         await devHubConn.singleRecordQuery<{ Id: string }>(
           `SELECT Id FROM ActiveScratchOrg WHERE SignupUsername='${username}'`

test/perf/logger/main.test.ts

@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2023, salesforce.com, inc.
+ * All rights reserved.
+ * Licensed under the BSD 3-Clause license.
+ * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+import { Suite } from 'benchmark';
+import { Logger } from '../../../src/exported';
+
+const suite = new Suite();
+
+const logger = new Logger('Benchmarks');
+
+// add tests
+suite
+  .add('Child logger creation', () => {
+    // eslint-disable-next-line @typescript-eslint/prefer-includes
+    Logger.childFromRoot('benchmarkChild');
+  })
+  .add('Logging a string on root logger', () => {
+    logger.warn('this is a string');
+  })
+  .add('Logging an object on root logger', () => {
+    logger.warn({ foo: 1, bar: 2, baz: 3 });
+  })
+  .add('Logging an object with a message on root logger', () => {
+    logger.warn({ foo: 1, bar: 2, baz: 3 }, 'this is a message');
+  })
+  .add('Logging an object with a redacted prop on root logger', () => {
+    logger.warn({ foo: 1, bar: 2, accessToken: '00D' });
+  })
+  .add('Logging a nested 3-level object on root logger', () => {
+    logger.warn({ foo: 1, bar: 2, baz: { foo: 1, bar: 2, baz: { foo: 1, bar: 2, baz: 3 } } });
+  })
+  // add listeners
+  .on('cycle', (event: any) => {
+    // eslint-disable-next-line no-console, @typescript-eslint/no-unsafe-member-access
+    console.log(String(event.target));
+  })
+  .run({ async: true });

test/tsconfig.json

@@ -1,6 +1,6 @@
 {
   "extends": "@salesforce/dev-config/tsconfig-test-strict",
-  "include": ["unit/**/*.ts"],
+  "include": ["unit/**/*.ts", "perf/**/*.ts"],
   "compilerOptions": {
     "noEmit": true,
     "skipLibCheck": true,

test/unit/org/orgTest.ts

@@ -329,6 +329,42 @@ describe('Org Tests', () => {
           }
         });
 
+        it('should unset the alias and any configs', async () => {
+          const dev = await createOrgViaAuthInfo(testData.username);
+          const configAgg = await ConfigAggregator.create();
+
+          const orgTestData = new MockTestOrgData();
+          const org = await createOrgViaAuthInfo(orgTestData.username, orgTestData);
+          const username = ensureString(org.getUsername());
+          $$.setConfigStubContents('Config', { contents: { [OrgConfigProperties.TARGET_ORG]: username } });
+
+          await configAgg.reload();
+          const stateAggregator = await StateAggregator.getInstance();
+
+          expect(configAgg.getConfig()['target-org']).to.equal(username);
+
+          await stateAggregator.aliases.setAndSave('deleteThisAlias', username);
+          expect(stateAggregator.aliases.getUsername('deleteThisAlias')).to.equal(username);
+
+          const devHubQuery = stubMethod($$.SANDBOX, Connection.prototype, 'singleRecordQuery').resolves({
+            Id: orgTestData.orgId,
+          });
+          const devHubDelete = stubMethod($$.SANDBOX, Org.prototype, 'destroyScratchOrg').resolves();
+          $$.SANDBOX.stub(org, 'getDevHubOrg').resolves(dev);
+
+          await org.delete();
+          expect(configAgg.getConfig()['target-org']).to.equal(undefined);
+
+          expect(stateAggregator.aliases.get(username)).to.be.null;
+          expect(devHubQuery.calledOnce).to.be.true;
+          expect(devHubQuery.firstCall.args[0]).to.equal(
+            `SELECT Id FROM ActiveScratchOrg WHERE SignupUsername='${orgTestData.username}'`
+          );
+
+          expect(devHubDelete.calledOnce).to.be.true;
+          expect(devHubDelete.firstCall.args[1]).to.equal(orgTestData.orgId);
+        });
+
         it('should handle SingleRecordQueryErrors.NoRecords errors', async () => {
           const dev = await createOrgViaAuthInfo();
 

yarn.lock

@@ -703,6 +703,11 @@
   resolved "https://registry.yarnpkg.com/@tsconfig/node16/-/node16-1.0.3.tgz#472eaab5f15c1ffdd7f8628bd4c4f753995ec79e"
   integrity sha512-yOlFc+7UtL/89t2ZhjPvvB/DeAr3r+Dq58IgzsFkOAvVC6NMJXmCGjbptdXdR9qsX7pKcTL+s87FtYREi2dEEQ==
 
+"@types/benchmark@^2.1.2":
+  version "2.1.2"
+  resolved "https://registry.yarnpkg.com/@types/benchmark/-/benchmark-2.1.2.tgz#b7838408c93dc08ceb4e6e13147dbfbe6a151f82"
+  integrity sha512-EDKtLYNMKrig22jEvhXq8TBFyFgVNSPmDF2b9UzJ7+eylPqdZVo17PCUMkn1jP6/1A/0u78VqYC6VrX6b8pDWA==
+
 "@types/chai-string@^1.4.2":
   version "1.4.2"
   resolved "https://registry.yarnpkg.com/@types/chai-string/-/chai-string-1.4.2.tgz#0f116504a666b6c6a3c42becf86634316c9a19ac"
@@ -1179,6 +1184,14 @@ base64url@^3.0.1:
   resolved "https://registry.yarnpkg.com/base64url/-/base64url-3.0.1.tgz#6399d572e2bc3f90a9a8b22d5dbb0a32d33f788d"
   integrity sha512-ir1UPr3dkwexU7FdV8qBBbNDRUhMmIekYMFZfi+C/sLNnRESKPl23nB9b2pltqfOQNnGzsDdId90AEtG5tCx4A==
 
+benchmark@^2.1.4:
+  version "2.1.4"
+  resolved "https://registry.yarnpkg.com/benchmark/-/benchmark-2.1.4.tgz#09f3de31c916425d498cc2ee565a0ebf3c2a5629"
+  integrity sha512-l9MlfN4M1K/H2fbhfMy3B7vJd6AGKJVQn2h6Sg/Yx+KckoUA7ewS5Vv6TjSq18ooE1kS9hhAlQRH3AkXIh/aOQ==
+  dependencies:
+    lodash "^4.17.4"
+    platform "^1.3.3"
+
 binary-extensions@^2.0.0:
   version "2.2.0"
   resolved "https://registry.yarnpkg.com/binary-extensions/-/binary-extensions-2.2.0.tgz#75f502eeaf9ffde42fc98829645be4ea76bd9e2d"
@@ -3201,7 +3214,7 @@ lodash.upperfirst@^4.3.1:
   resolved "https://registry.yarnpkg.com/lodash.upperfirst/-/lodash.upperfirst-4.3.1.tgz#1365edf431480481ef0d1c68957a5ed99d49f7ce"
   integrity sha512-sReKOYJIJf74dhJONhU4e0/shzi1trVbSWDOhKYE5XV2O+H7Sb2Dihwuc7xWxVl+DgFPyTqIN3zMfT9cq5iWDg==
 
-lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.21:
+lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.21, lodash@^4.17.4:
   version "4.17.21"
   resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c"
   integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==
@@ -3824,6 +3837,11 @@ pkg-dir@^4.1.0:
   dependencies:
     find-up "^4.0.0"
 
+platform@^1.3.3:
+  version "1.3.6"
+  resolved "https://registry.yarnpkg.com/platform/-/platform-1.3.6.tgz#48b4ce983164b209c2d45a107adb31f473a6e7a7"
+  integrity sha512-fnWVljUchTro6RiCFvCXBbNhJc2NijN7oIQxbwsyL0buWJPG85v81ehlHI9fXrJsMNgTofEoWIQeClKpgxFLrg==
+
 prelude-ls@^1.2.1:
   version "1.2.1"
   resolved "https://registry.yarnpkg.com/prelude-ls/-/prelude-ls-1.2.1.tgz#debc6489d7a6e6b0e7611888cec880337d316396"