BREAKING CHANGE Wr/destructive changes (#450)

* fix: inital POC for destructiveChangesPre and Post

* chore: pre/post destructive changes working, UT failing

* chore: pre/post destructive changes working, UT passing

* refactor: remove asDelete param from methods, using deletion type

* chore: code review I - clean up ComponentSet

* chore: most of Steve's changes made

* chore: steve's second change

* chore: actually resolve conflicts

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@salesforce/source-deploy-retrieve",
-  "version": "4.5.12",
+  "version": "5.0.0",
   "description": "JavaScript library to run Salesforce metadata deploys and retrieves",
   "main": "lib/src/index.js",
   "author": "Salesforce",

src/collections/componentSet.ts

@@ -64,7 +64,10 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
   private components = new Map<string, Map<string, SourceComponent>>();
 
   // internal component maps used by this.getObject() when building manifests.
-  private destructiveComponents = new Map<string, Map<string, SourceComponent>>();
+  private destructiveComponents = {
+    [DestructiveChangesType.PRE]: new Map<string, Map<string, SourceComponent>>(),
+    [DestructiveChangesType.POST]: new Map<string, Map<string, SourceComponent>>(),
+  };
   private manifestComponents = new Map<string, Map<string, SourceComponent>>();
 
   private destructiveChangesType = DestructiveChangesType.POST;
@@ -76,11 +79,12 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
     this.logger = Logger.childFromRoot(this.constructor.name);
 
     for (const component of components) {
-      let asDeletion = false;
-      if (component instanceof SourceComponent) {
-        asDeletion = component.isMarkedForDelete();
-      }
-      this.add(component, asDeletion);
+      const destructiveType =
+        component instanceof SourceComponent
+          ? component.getDestructiveChangesType()
+          : this.destructiveChangesType;
+
+      this.add(component, destructiveType);
     }
   }
 
@@ -128,15 +132,15 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
 
     const resolver = new MetadataResolver(registry, tree);
     const set = new ComponentSet([], registry);
-    const buildComponents = (paths: string[], asDeletes: boolean): void => {
+    const buildComponents = (paths: string[], destructiveType?: DestructiveChangesType): void => {
       for (const path of paths) {
         for (const component of resolver.getComponentsFromPath(path, inclusiveFilter)) {
-          set.add(component, asDeletes);
+          set.add(component, destructiveType);
         }
       }
     };
-    buildComponents(fsPaths, false);
-    buildComponents(fsDeletePaths, true);
+    buildComponents(fsPaths);
+    buildComponents(fsDeletePaths, DestructiveChangesType.POST);
 
     set.forceIgnoredPaths = resolver.forceIgnoredPaths;
 
@@ -171,21 +175,49 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
 
     const manifestResolver = new ManifestResolver(options.tree, options.registry);
     const manifest = await manifestResolver.resolve(manifestPath);
+
     const resolveIncludeSet = options.resolveSourcePaths
       ? new ComponentSet([], options.registry)
       : undefined;
     const result = new ComponentSet([], options.registry);
     result.apiVersion = manifest.apiVersion;
     result.fullName = manifest.fullName;
 
-    for (const component of manifest.components) {
+    const addComponent = (
+      component: MetadataComponent,
+      deletionType?: DestructiveChangesType
+    ): void => {
       if (resolveIncludeSet) {
-        resolveIncludeSet.add(component);
+        resolveIncludeSet.add(component, deletionType);
       }
       const memberIsWildcard = component.fullName === ComponentSet.WILDCARD;
       if (!memberIsWildcard || options.forceAddWildcards || !options.resolveSourcePaths) {
-        result.add(component);
+        result.add(component, deletionType);
       }
+    };
+
+    const resolveDestructiveChanges = async (
+      path: string,
+      destructiveChangeType: DestructiveChangesType
+    ) => {
+      const manifest = await manifestResolver.resolve(path);
+      for (const comp of manifest.components) {
+        addComponent(
+          new SourceComponent({ type: comp.type, name: comp.fullName }),
+          destructiveChangeType
+        );
+      }
+    };
+
+    if (options.destructivePre) {
+      await resolveDestructiveChanges(options.destructivePre, DestructiveChangesType.PRE);
+    }
+    if (options.destructivePost) {
+      await resolveDestructiveChanges(options.destructivePost, DestructiveChangesType.POST);
+    }
+
+    for (const component of manifest.components) {
+      addComponent(component);
     }
 
     if (options.resolveSourcePaths) {
@@ -251,20 +283,18 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
 
   /**
    * Get an object representation of a package manifest based on the set components.
-   *
+   * @param destructiveType Optional value for generating objects representing destructive change manifests
    * @returns Object representation of a package manifest
    */
-  public getObject(forDestructiveChanges = false): PackageManifestObject {
+  public getObject(destructiveType?: DestructiveChangesType): PackageManifestObject {
     // If this ComponentSet has components marked for delete, we need to
     // only include those components in a destructiveChanges.xml and
     // all other components in the regular manifest.
     let components = this.components;
-    if (this.hasDeletes) {
-      if (forDestructiveChanges) {
-        components = this.destructiveComponents;
-      } else {
-        components = this.manifestComponents;
-      }
+    if (this.getTypesOfDestructiveChanges().length) {
+      components = destructiveType
+        ? this.destructiveComponents[destructiveType]
+        : this.manifestComponents;
     }
 
     const typeMap = new Map<string, string[]>();
@@ -276,7 +306,7 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
           typeMap.set(typeName, []);
         }
         const typeEntry = typeMap.get(typeName);
-        if (fullName === ComponentSet.WILDCARD && !type.supportsWildcardAndName) {
+        if (fullName === ComponentSet.WILDCARD && !type.supportsWildcardAndName && !destructiveType) {
           // if the type doesn't support mixed wildcards and specific names, overwrite the names to be a wildcard
           typeMap.set(typeName, [fullName]);
         } else if (
@@ -330,13 +360,13 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
    * @param indentation Number of spaces to indent lines by.
    * @param forDestructiveChanges Whether to build a manifest for destructive changes.
    */
-  public getPackageXml(indentation = 4, forDestructiveChanges = false): string {
+  public getPackageXml(indentation = 4, destructiveType?: DestructiveChangesType): string {
     const j2x = new j2xParser({
       format: true,
       indentBy: new Array(indentation + 1).join(' '),
       ignoreAttributes: false,
     });
-    const toParse = this.getObject(forDestructiveChanges);
+    const toParse = this.getObject(destructiveType);
     toParse.Package[XML_NS_KEY] = XML_NS_URL;
     return XML_DECL.concat(j2x.parse(toParse));
   }
@@ -363,29 +393,49 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
     >;
   }
 
-  public add(component: ComponentLike, asDeletion?: boolean): void {
+  public add(component: ComponentLike, deletionType?: DestructiveChangesType): void {
     const key = this.simpleKey(component);
     if (!this.components.has(key)) {
       this.components.set(key, new Map<string, SourceComponent>());
     }
-    if (component instanceof SourceComponent) {
-      this.components.get(key).set(this.sourceKey(component), component);
-
-      // Build maps of destructive components and regular components as they are added
-      // as an optimization when building manifests.
-      if (asDeletion) {
-        component.setMarkedForDelete(true);
-        this.logger.debug(`Marking component for delete: ${component.fullName}`);
-        if (!this.destructiveComponents.has(key)) {
-          this.destructiveComponents.set(key, new Map<string, SourceComponent>());
-        }
-        this.destructiveComponents.get(key).set(this.sourceKey(component), component);
-      } else {
-        if (!this.manifestComponents.has(key)) {
-          this.manifestComponents.set(key, new Map<string, SourceComponent>());
-        }
-        this.manifestComponents.get(key).set(this.sourceKey(component), component);
+
+    if (!(component instanceof SourceComponent)) {
+      return;
+    }
+
+    // we're working with SourceComponents now
+    this.components.get(key).set(this.sourceKey(component), component);
+
+    // Build maps of destructive components and regular components as they are added
+    // as an optimization when building manifests.
+    if (deletionType) {
+      component.setMarkedForDelete(deletionType);
+      this.logger.debug(`Marking component for delete: ${component.fullName}`);
+      const deletions = this.destructiveComponents[deletionType];
+      if (!deletions.has(key)) {
+        deletions.set(key, new Map<string, SourceComponent>());
       }
+      deletions.get(key).set(this.sourceKey(component), component);
+    } else {
+      if (!this.manifestComponents.has(key)) {
+        this.manifestComponents.set(key, new Map<string, SourceComponent>());
+      }
+      this.manifestComponents.get(key).set(this.sourceKey(component), component);
+    }
+
+    // something could try adding a component meant for deletion improperly, which would be marked as an addition
+    // specifically the ComponentSet.fromManifest with the `resolveSourcePaths` options which calls
+    // ComponentSet.fromSource, and adds everything as an addition
+    if (
+      this.manifestComponents.has(key) &&
+      (this.destructiveChangesPre.has(key) || this.destructiveChangesPost.has(key))
+    ) {
+      // if a component is in the manifestComponents, as well as being part of a destructive manifest, keep in the destructive manifest
+      component.setMarkedForDelete(deletionType);
+      this.manifestComponents.delete(key);
+      this.logger.debug(
+        `Component: ${key} was found in both destructive and constructive manifests - keeping as a destructive change`
+      );
     }
   }
 
@@ -477,6 +527,22 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
     return this.destructiveChangesType;
   }
 
+  /**
+   * Will return the types of destructive changes in the component set
+   * or an empty array if there aren't destructive components present
+   * @return DestructiveChangesType[]
+   */
+  public getTypesOfDestructiveChanges(): DestructiveChangesType[] {
+    const destructiveChangesTypes: DestructiveChangesType[] = [];
+    if (this.destructiveChangesPre.size) {
+      destructiveChangesTypes.push(DestructiveChangesType.PRE);
+    }
+    if (this.destructiveChangesPost.size) {
+      destructiveChangesTypes.push(DestructiveChangesType.POST);
+    }
+    return destructiveChangesTypes;
+  }
+
   /**
    * Each {@link SourceComponent} counts as an element in the set, even if multiple
    * ones map to the same `fullName` and `type` pair.
@@ -492,11 +558,12 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
     return size;
   }
 
-  /**
-   * Returns `true` if this `ComponentSet` contains components marked for deletion.
-   */
-  get hasDeletes(): boolean {
-    return this.destructiveComponents.size > 0;
+  get destructiveChangesPre(): Map<string, Map<string, SourceComponent>> {
+    return this.destructiveComponents[DestructiveChangesType.PRE];
+  }
+
+  get destructiveChangesPost(): Map<string, Map<string, SourceComponent>> {
+    return this.destructiveComponents[DestructiveChangesType.POST];
   }
 
   private sourceKey(component: SourceComponent): string {

src/collections/types.ts

@@ -62,4 +62,13 @@ export interface FromManifestOptions extends OptionalTreeRegistryOptions {
    * conditions.
    */
   forceAddWildcards?: boolean;
+
+  /**
+   * path to a `destructiveChangesPre.xml` file in XML format
+   */
+  destructivePre?: string;
+  /**
+   * path to a `destructiveChangesPost.xml` file in XML format
+   */
+  destructivePost?: string;
 }

src/convert/metadataConverter.ts

@@ -5,28 +5,27 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 import {
-  SfdxFileFormat,
   ConvertOutputConfig,
   ConvertResult,
   DirectoryConfig,
+  SfdxFileFormat,
   ZipConfig,
 } from './types';
-import { DestructiveChangesType } from '../collections/types';
 import { SourceComponent } from '../resolve';
 import { promises } from 'graceful-fs';
 import { dirname, join, normalize } from 'path';
 import { ensureDirectoryExists } from '../utils/fileSystemHandler';
 import {
-  ComponentReader,
   ComponentConverter,
-  StandardWriter,
+  ComponentReader,
+  ComponentWriter,
   pipeline,
+  StandardWriter,
   ZipWriter,
-  ComponentWriter,
 } from './streams';
 import { ConversionError, LibraryError } from '../errors';
 import { SourcePath } from '../common';
-import { ComponentSet } from '../collections';
+import { ComponentSet, DestructiveChangesType } from '../collections';
 import { RegistryAccess } from '../registry';
 
 export class MetadataConverter {
@@ -103,13 +102,19 @@ export class MetadataConverter {
           if (!isSource) {
             const manifestPath = join(packagePath, MetadataConverter.PACKAGE_XML_FILE);
             tasks.push(promises.writeFile(manifestPath, manifestContents));
-
             // For deploying destructive changes
-            if (cs.hasDeletes) {
-              const manifestFileName = this.getDestructiveManifestFileName(cs);
-              const destructiveManifestContents = cs.getPackageXml(undefined, true);
-              const destructiveManifestPath = join(packagePath, manifestFileName);
-              tasks.push(promises.writeFile(destructiveManifestPath, destructiveManifestContents));
+            const destructiveChangesTypes = cs.getTypesOfDestructiveChanges();
+            if (destructiveChangesTypes.length) {
+              // for each of the destructive changes in the component set, convert and write the correct metadata
+              // to each manifest
+              destructiveChangesTypes.map((destructiveChangesType) => {
+                const file = this.getDestructiveManifest(destructiveChangesType);
+                const destructiveManifestContents = cs.getPackageXml(4, destructiveChangesType);
+                const destructiveManifestPath = join(packagePath, file);
+                tasks.push(
+                  promises.writeFile(destructiveManifestPath, destructiveManifestContents)
+                );
+              });
             }
           }
           break;
@@ -123,12 +128,16 @@ export class MetadataConverter {
           writer = new ZipWriter(packagePath);
           if (!isSource) {
             (writer as ZipWriter).addToZip(manifestContents, MetadataConverter.PACKAGE_XML_FILE);
-
             // For deploying destructive changes
-            if (cs.hasDeletes) {
-              const manifestFileName = this.getDestructiveManifestFileName(cs);
-              const destructiveManifestContents = cs.getPackageXml(undefined, true);
-              (writer as ZipWriter).addToZip(destructiveManifestContents, manifestFileName);
+            const destructiveChangesTypes = cs.getTypesOfDestructiveChanges();
+            if (destructiveChangesTypes.length) {
+              // for each of the destructive changes in the component set, convert and write the correct metadata
+              // to each manifest
+              destructiveChangesTypes.map((destructiveChangeType) => {
+                const file = this.getDestructiveManifest(destructiveChangeType);
+                const destructiveManifestContents = cs.getPackageXml(4, destructiveChangeType);
+                (writer as ZipWriter).addToZip(destructiveManifestContents, file);
+              });
             }
           }
           break;
@@ -167,16 +176,6 @@ export class MetadataConverter {
     }
   }
 
-  private getDestructiveManifestFileName(cs: ComponentSet): string {
-    let manifestFileName: string;
-    if (cs.getDestructiveChangesType() === DestructiveChangesType.POST) {
-      manifestFileName = MetadataConverter.DESTRUCTIVE_CHANGES_POST_XML_FILE;
-    } else {
-      manifestFileName = MetadataConverter.DESTRUCTIVE_CHANGES_PRE_XML_FILE;
-    }
-    return manifestFileName;
-  }
-
   private getPackagePath(outputConfig: DirectoryConfig | ZipConfig): SourcePath | undefined {
     let packagePath: SourcePath;
     const { genUniqueDir = true, outputDirectory, packageName, type } = outputConfig;
@@ -203,4 +202,12 @@ export class MetadataConverter {
     }
     return packagePath;
   }
+
+  private getDestructiveManifest(destructiveChangesType: DestructiveChangesType): string {
+    if (destructiveChangesType === DestructiveChangesType.POST) {
+      return MetadataConverter.DESTRUCTIVE_CHANGES_POST_XML_FILE;
+    } else if (destructiveChangesType === DestructiveChangesType.PRE) {
+      return MetadataConverter.DESTRUCTIVE_CHANGES_PRE_XML_FILE;
+    }
+  }
 }