fix: update deploy and retrieve beta to use 1.1.19 #3048

brpowell · 2021-03-09T23:15:59Z

What does this PR do?

Updates the deploy and retrieve beta functionality to utilize the latest changes in the library.

Fix timeout issues. Occurs when deploy/retrieve operation runs longer than 10 seconds. Cancellation coming in a subsequent PR.
Fix result output when deploying/retrieving multi-file components
Fix an issue with unzipping some zip StaticResource components
Fix log name for manifest command
Updated command labels that use i18n messages.
More shared code paths between the commands. This ensures consistent outputting and logging.
Utilize tooling retrieve for retrieving source-backed single components through org browser and manifest. Grants a performance boost for tooling supported types.
Removes Tooling API deploy code path. Through A/B testing, benefits were not seen.

What issues does this PR fix or reference?

@W-8899035@

codecov · 2021-03-09T23:16:05Z

Codecov Report

Merging #3048 (c26f7e5) into develop (904b541) will decrease coverage by 7.87%.
The diff coverage is 90.15%.

@@             Coverage Diff             @@
##           develop    #3048      +/-   ##
===========================================
- Coverage    84.04%   76.17%   -7.88%     
===========================================
  Files           69      276     +207     
  Lines         3103    10502    +7399     
  Branches       295     1235     +940     
===========================================
+ Hits          2608     8000    +5392     
- Misses         437     2158    +1721     
- Partials        58      344     +286

Impacted Files	Coverage Δ
...alesforcedx-vscode-core/src/commands/util/index.ts	`100.00% <ø> (ø)`
...cedx-vscode-core/src/commands/baseDeployCommand.ts	`50.57% <50.00%> (ø)`
...forcedx-vscode-core/src/diagnostics/diagnostics.ts	`77.77% <70.58%> (ø)`
...ceSourceRetrieveMetadata/forceSourceRetrieveCmp.ts	`68.42% <80.00%> (ø)`
...edx-vscode-core/src/commands/baseDeployRetrieve.ts	`93.22% <93.22%> (ø)`
...ode-core/src/commands/forceSourceDeployManifest.ts	`61.53% <100.00%> (ø)`
...e-core/src/commands/forceSourceDeploySourcePath.ts	`54.71% <100.00%> (ø)`
...e-core/src/commands/forceSourceRetrieveManifest.ts	`58.33% <100.00%> (ø)`
...core/src/commands/forceSourceRetrieveSourcePath.ts	`65.90% <100.00%> (ø)`
.../salesforcedx-vscode-core/src/diagnostics/index.ts	`100.00% <100.00%> (ø)`
... and 208 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 904b541...c26f7e5. Read the comment docs.

brpowell · 2021-03-10T16:58:37Z

packages/salesforcedx-vscode-core/src/commands/baseDeployRetrieve.ts

+      const components = await this.getComponents(response);
+
+      this.telemetry.addProperty(
+        'metadataCount',
+        this.createComponentCount(components)
+      );
+
+      result = await this.doOperation(components);
+
+      const status = this.getStatus(result);
+
+      return (
+        status === RequestStatus.Succeeded ||
+        status === RequestStatus.SucceededPartial
+      );


All DeployRetrieve commands need to report metadata counts and report status to the base executor.

brpowell · 2021-03-10T16:59:32Z

packages/salesforcedx-vscode-core/src/commands/baseDeployRetrieve.ts

+  protected getRelativeProjectPath(fsPath: string = '', packageDirs: string[]) {
+    let packageDirIndex;
+    for (let packageDir of packageDirs) {
+      if (!packageDir.startsWith(sep)) {
+        packageDir = sep + packageDir;
+      }
+      if (!packageDir.endsWith(sep)) {
+        packageDir = packageDir + sep;
+      }
+      packageDirIndex = fsPath.indexOf(packageDir);
+      if (packageDirIndex !== -1) {
+        packageDirIndex += 1;
+        break;
+      }
+    }
+    return packageDirIndex !== -1 ? fsPath.slice(packageDirIndex) : fsPath;
+  }
+
+  private createComponentCount(
+    components: Iterable<MetadataComponent>
+  ): string {
+    const quantities: { [type: string]: number } = {};
+    for (const component of components) {
+      const { name: typeName } = component.type;
+      const typeCount = quantities[typeName];
+      quantities[typeName] = typeCount ? typeCount + 1 : 1;
+    }
+    return JSON.stringify(
+      Object.keys(quantities).map(type => ({
+        type,
+        quantity: quantities[type]
+      }))
+    );
+  }


Some of these scattered utilities have been made private methods here since they are internal implementation to these commands.

brpowell · 2021-03-10T16:59:57Z

packages/salesforcedx-vscode-core/src/commands/baseDeployRetrieve.ts

+  protected async doOperation(
+    components: ComponentSet
+  ): Promise<DeployResult | undefined> {
+    return components
+      .deploy({
+        usernameOrConnection: await workspaceContext.getConnection()
+      })
+      .start();
+  }


All deploy commands follow the same pattern for kicking off

brpowell · 2021-03-10T17:00:44Z

packages/salesforcedx-vscode-core/src/commands/baseDeployRetrieve.ts

+    try {
+      if (result) {
+        BaseDeployExecutor.errorCollection.clear();
+
+        const relativePackageDirs = await SfdxPackageDirectories.getPackageDirectoryPaths();
+        const output = this.createOutput(result, relativePackageDirs);
+        channelService.appendLine(output);
+
+        const success = result.response.status === RequestStatus.Succeeded;
+
+        if (!success) {
+          handleDeployDiagnostics(result, BaseDeployExecutor.errorCollection);
+        }
+      }
+    } finally {
+      await DeployQueue.get().unlock();
+    }


All deploy commands need to:

output results the same way

handle diagnostics the same way

unlock the deploy queue when they are finished

brpowell · 2021-03-10T17:01:39Z

packages/salesforcedx-vscode-core/src/commands/baseDeployRetrieve.ts

+    }
+  }
+
+  private createOutput(


Again, consolidating scattered output utilities as internal implementation to the deploy command. Did the same for retrieve below.

brpowell · 2021-03-10T17:02:25Z

packages/salesforcedx-vscode-core/src/commands/baseDeployRetrieve.ts

+    // utilize the tooling API for single component retrieves for improved performance
+    const oneComponent = components.getSourceComponents().first();
+
+    if (components.size === 1 && this.isToolingSupported(oneComponent)) {


All retrieve commands will utilize the tooling api when it can, since single component retrieves are quite fast.

Why are we not adding this into SDR ?

This is how it has always been in the extensions up until this point. It should be added to the library though.

brpowell · 2021-03-10T17:03:38Z

packages/salesforcedx-vscode-core/src/commands/baseDeployRetrieve.ts

+   * This exists because the Tooling API result currently doesn't conform to the
+   * same interface as the Metadata API deploy and retrieve result objects.
+   */
+  private createToolingOutput(


Eventually, consumers shouldn't really have to think about using the tooling api or not. The library should handle using the best api when it can. Also tooling results currently still have their own result format, so we have to do some gymnastics to handle that.

brpowell · 2021-03-10T17:04:27Z

packages/salesforcedx-vscode-core/src/commands/forceSourceDeployManifest.ts

+  protected async getComponents(
+    response: ContinueResponse<string>
+  ): Promise<ComponentSet> {
    const packageDirs = await SfdxPackageDirectories.getPackageDirectoryPaths();
-    try {
-      const components = await ComponentSet.fromManifestFile(response.data, {
-        resolve: packageDirs.map(dir => join(getRootWorkspacePath(), dir))
-      });
-      const deployPromise = components.deploy(
-        await workspaceContext.getConnection()
-      );
-      this.telemetry.addProperty(
-        'metadataCount',
-        JSON.stringify(createComponentCount(components))
-      );
-      const result = await deployPromise;
-
-      const outputResult = createDeployOutput(result, packageDirs);
-      channelService.appendLine(outputResult);
-      BaseDeployExecutor.errorCollection.clear();
-
-      if (result.status === DeployStatus.Succeeded) {
-        return true;
-      }
-
-      handleDeployRetrieveLibraryDiagnostics(
-        result,
-        BaseDeployExecutor.errorCollection
-      );
-
-      return false;
-    } finally {
-      await DeployQueue.get().unlock();
-    }
+    return ComponentSet.fromManifestFile(response.data, {
+      resolve: packageDirs.map(dir => join(getRootWorkspacePath(), dir))
+    });


Now individual commands for the most part just have to extend the relevant base class (Deploy or Retrieve), and then describe how to fetch components.

.gitignore

sfsholden · 2021-03-10T20:56:07Z

packages/salesforcedx-vscode-core/src/commands/baseDeployRetrieve.ts

+      const components = await this.getComponents(response);
+
+      this.telemetry.addProperty(
+        'metadataCount',


Does this make more sense to have as a constant?

sfsholden · 2021-03-11T15:29:12Z

packages/salesforcedx-vscode-core/src/commands/baseDeployRetrieve.ts

+            filePath: this.getRelativeProjectPath(xml, relativePackageDirs)
+          });
+        }
+      } else if (properties) {


I'm curious about testing this piece. Is this the type that would trigger this path? https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_classes_properties.htm

This is sort of dead code...it's a relic from a previous implementation to provide a fallback if component isn't set. I can probably get rid of it

sfsholden · 2021-03-11T16:37:55Z

packages/salesforcedx-vscode-core/src/commands/forceSourceDeployManifest.ts

-import { ContinueResponse } from '@salesforce/salesforcedx-utils-vscode/src/types';
-import { ComponentSet, DeployStatus } from '@salesforce/source-deploy-retrieve';
+import { ContinueResponse } from '@salesforce/salesforcedx-utils-vscode/out/src/types';
+import { ComponentSet, DeployResult } from '@salesforce/source-deploy-retrieve';


Small nit - some unused imports here

sfsholden · 2021-03-11T17:00:42Z

.../salesforcedx-vscode-core/src/commands/forceSourceRetrieveMetadata/forceSourceRetrieveCmp.ts

-    const components = new ComponentSet(
-      comps.map(lc => ({ fullName: lc.fileName, type: lc.type }))
+  ): Promise<ComponentSet> {
+    const filter = new ComponentSet(


I'm not able to walkthrough this portion of the code - I can't find a way to fire it. Is that because this class LibraryRetrieveSourcePathExecutor is also defined in forceSourceRetrieveSourcePath? Let me know if this is just simple user error on my end.

It looked like the index was only referring to the one defined within forceSourceRetrieveSourcePath.

This command is only used by the org browser (right now), so there isn't a command palette option for it. Try retrieving components through that tree.

Bryan Powell added 17 commits March 3, 2021 14:26

refactor: update deploy w/ manifest

c06eff5

refactor: update deploy w/ sourcepath

3493bcd

refactor: update retrieve w/ manifest

c65fe22

refactor: updated retrieve w/ source path

4ae806e

refactor: update retrieve component

44b2079

refactor: add diagnostics back in

3ecca56

fix: command labels and log names

1982b77

test: update source path tests

801554b

test: update retrieve component tests

0f521c4

test: update deploy manifest tests

3730564

test: update retrieve with manifest test

59b599f

test: update retrieve path tests

718cc8b

refactor: deploy base classes for shared func

6cba0a0

refactor: base class for retrieves

fafeb23

refactor: hoisted deploy operation

c2bd52f

test: skeleton tests

292ba16

refactor: handle promises

d9b80e9

brpowell requested a review from a team March 9, 2021 23:22

Bryan Powell added 2 commits March 9, 2021 16:00

test: tests for deployretrieveexecutor

2747ece

test: deploy tests

f666868

brpowell commented Mar 10, 2021

View reviewed changes

refactor: remove some dead code

0bb7ee7

sfsholden reviewed Mar 10, 2021

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

Bryan Powell added 3 commits March 10, 2021 11:06

test: retrieve tests

75914da

test: fix retrieve component test

02207e3

refactor: rename executors

5b6f3b3

brpowell marked this pull request as ready for review March 10, 2021 20:15

test: add tooling namespace test

92547e8

sfsholden reviewed Mar 10, 2021

View reviewed changes

Bryan Powell added 2 commits March 10, 2021 16:14

refactor: constant for telemetry key

6d49558

fix: remove org browser commands from palette

357bf65

sfsholden reviewed Mar 11, 2021

View reviewed changes

refactor: remove dead code

3e3230c

sfsholden approved these changes Mar 12, 2021

View reviewed changes

Merge branch 'develop' into bp/sdrUpdate-1.1.19

c26f7e5

brpowell merged commit d628c7d into develop Mar 12, 2021

brpowell deleted the bp/sdrUpdate-1.1.19 branch March 12, 2021 16:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update deploy and retrieve beta to use 1.1.19 #3048

fix: update deploy and retrieve beta to use 1.1.19 #3048

brpowell commented Mar 9, 2021 •

edited

codecov bot commented Mar 9, 2021 •

edited

brpowell Mar 10, 2021

brpowell Mar 10, 2021

brpowell Mar 10, 2021

brpowell Mar 10, 2021

brpowell Mar 10, 2021

brpowell Mar 10, 2021

lcampos Mar 11, 2021

brpowell Mar 11, 2021

brpowell Mar 10, 2021

brpowell Mar 10, 2021

sfsholden Mar 10, 2021 •

edited

sfsholden Mar 11, 2021

brpowell Mar 11, 2021

sfsholden Mar 11, 2021

sfsholden Mar 11, 2021

sfsholden Mar 11, 2021

brpowell Mar 11, 2021

fix: update deploy and retrieve beta to use 1.1.19 #3048

fix: update deploy and retrieve beta to use 1.1.19 #3048

Conversation

brpowell commented Mar 9, 2021 • edited

What does this PR do?

What issues does this PR fix or reference?

codecov bot commented Mar 9, 2021 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfsholden Mar 10, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brpowell commented Mar 9, 2021 •

edited

codecov bot commented Mar 9, 2021 •

edited

sfsholden Mar 10, 2021 •

edited