-
Notifications
You must be signed in to change notification settings - Fork 103
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
Changes from 13 commits
4c62a7e
81d9eb2
9625604
18c5b5f
9eea66f
fc53ee3
200508c
3fa4cdc
238c041
9661bed
ef1e6a0
953b9b5
ea00ffa
29519ea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 { | ||
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[]>(); | ||
|
@@ -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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. replacing the |
||
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` | ||
); | ||
} | ||
} | ||
|
||
|
@@ -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[] { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 suggestion: make an array, and conditionally push to it (there might be 0,1, or 2 values). then we can eliminate the
|
||
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. | ||
|
@@ -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 { | ||
|
There was a problem hiding this comment.
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 whenset.add(component, null)
is called and then later used?There was a problem hiding this comment.
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 adestructiveChangesPost.xml