Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BREAKING CHANGE Wr/destructive changes #450

Merged
merged 14 commits into from
Oct 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
161 changes: 114 additions & 47 deletions src/collections/componentSet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}
}

Expand Down Expand Up @@ -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);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just dbl-checking here, as I'm not clear on the code paths - since destructiveType is nullable, what happens when set.add(component, null) is called and then later used?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should default to a POST i.e. generating a destructiveChangesPost.xml

}
}
};
buildComponents(fsPaths, false);
buildComponents(fsDeletePaths, true);
buildComponents(fsPaths);
buildComponents(fsDeletePaths, DestructiveChangesType.POST);

set.forceIgnoredPaths = resolver.forceIgnoredPaths;

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
shetzel marked this conversation as resolved.
Show resolved Hide resolved
// 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[]>();
Expand All @@ -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 (
Expand Down Expand Up @@ -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));
}
Expand All @@ -363,29 +393,49 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
>;
}

public add(component: ComponentLike, asDeletion?: boolean): void {
public add(component: ComponentLike, deletionType?: DestructiveChangesType): void {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replacing the asDeletion param is the breaking change, which can also be seen on other methods

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);
shetzel marked this conversation as resolved.
Show resolved Hide resolved
this.logger.debug(
`Component: ${key} was found in both destructive and constructive manifests - keeping as a destructive change`
);
}
}

Expand Down Expand Up @@ -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[] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasDeletes is doing pretty much the same logic, and is checked in both the Converter and CS's getObject.

This could also return an empty array for "there are none" so there is only one.

As written, if you were to NOT check hasDeletes before asking getTypesOfDestructiveChanges, your CS would tell that both types are present (the else condition here).

suggestion: make an array, and conditionally push to it (there might be 0,1, or 2 values).

then we can eliminate the hasDeletes since this code would have no effect for an empty array.

const destructiveChangesTypes = cs.getTypesOfDestructiveChanges();
              destructiveChangesTypes.map((destructiveChangesType) => {

const destructiveChangesTypes: DestructiveChangesType[] = [];
if (this.destructiveChangesPre.size) {
destructiveChangesTypes.push(DestructiveChangesType.PRE);
}
if (this.destructiveChangesPost.size) {
destructiveChangesTypes.push(DestructiveChangesType.POST);
}
return destructiveChangesTypes;
}
shetzel marked this conversation as resolved.
Show resolved Hide resolved

/**
* Each {@link SourceComponent} counts as an element in the set, even if multiple
* ones map to the same `fullName` and `type` pair.
Expand All @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions src/collections/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
61 changes: 34 additions & 27 deletions src/convert/metadataConverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
}
shetzel marked this conversation as resolved.
Show resolved Hide resolved
}
Loading