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

feat!: component set components are now DecodableMaps #1080

Merged
merged 3 commits into from
Aug 21, 2023
Merged
Changes from 1 commit
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
90 changes: 80 additions & 10 deletions src/collections/componentSet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,78 @@ export type RetrieveSetOptions = Omit<MetadataApiRetrieveOptions, 'components'>;

const KEY_DELIMITER = '#';

/**
* This is an extension of the Map class that treats keys as the same by matching first normally,
* then decoded. Decoding the key before comparing can solve some edge cases in component fullNames
* such as Layouts. See: https://github.com/forcedotcom/cli/issues/1683
*
* Examples:
*
* Given a map with entries:
* ```javascript
* 'layout#Layout__Broker__c-v1%2E1 Broker Layout' : {...}
* 'layout#Layout__Broker__c-v9.2 Broker Layout' : {...}
* ```
*
* `decodeableMap.has('layout#Layout__Broker__c-v1.1 Broker Layout')` --> returns `true`
* `decodeableMap.has('layout#Layout__Broker__c-v9%2E2 Broker Layout')` --> returns `true`
*/
class DecodeableMap<K extends string, V> extends Map<string, V> {
shetzel marked this conversation as resolved.
Show resolved Hide resolved
/**
* boolean indicating whether an element with the specified key (matching decoded) exists or not.
*/
public has(key: K): boolean {
return super.has(key) || this.hasDecoded(key);
}

/**
* Returns a specified element from the Map object. If the value that is associated to
* the provided key (matching decoded) is an object, then you will get a reference to
* that object and any change made to that object will effectively modify it inside the Map.
*/
public get(key: K): V | undefined {
return super.get(key) ?? this.getDecoded(key);
}

/**
* Adds a new element with a specified key and value to the Map. If an element with the
* same key (matching decoded) already exists, the element will be updated.
*/
public set(key: K, value: V): this {
const sKey = this.getExistingKey(key) ?? key;
return super.set(sKey, value);
}

/**
* true if an element in the Map existed (matching decoded) and has been removed, or false
* if the element does not exist.
*/
public delete(key: K): boolean {
const sKey = this.getExistingKey(key) ?? key;
return super.delete(sKey);
}

// Returns true if the passed `key` matches an existing key entry when both keys are decoded.
private hasDecoded(key: string): boolean {
return !!this.getExistingKey(key);
}

// Returns the value of an entry matching on decoded keys.
private getDecoded(key: string): V | undefined {
const existingKey = this.getExistingKey(key);
return existingKey ? super.get(existingKey) : undefined;
}

// Returns the key as it is in the map, matching on decoded keys.
private getExistingKey(key: string): string | undefined {
for (const compKey of this.keys()) {
if (decodeURIComponent(compKey) === decodeURIComponent(key)) {
return compKey;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (const compKey of this.keys()) {
if (decodeURIComponent(compKey) === decodeURIComponent(key)) {
return compKey;
}
}
return this.keys().find((compKey) => decodeURIComponent(compKey) === decodeURIComponent(key))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this.keys() returns an IterableIterator<string> so find() isn't available.

Copy link
Contributor

@mshanemc mshanemc Aug 17, 2023

Choose a reason for hiding this comment

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

this.keys() returns an IterableIterator so find() isn't available.

you can call Array.from() on those to get a real array.

return Array.from(this.keys()).find((compKey) => decodeURIComponent(compKey) === decodeURIComponent(key))

}
}

/**
* A collection containing no duplicate metadata members (`fullName` and `type` pairs). `ComponentSets`
* are a convenient way of constructing a unique collection of components to perform operations such as
Expand Down Expand Up @@ -73,14 +145,14 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
public forceIgnoredPaths?: Set<string>;
private logger: Logger;
private registry: RegistryAccess;
private components = new Map<string, Map<string, SourceComponent>>();
private components = new DecodeableMap<string, DecodeableMap<string, SourceComponent>>();

// internal component maps used by this.getObject() when building manifests.
private destructiveComponents = {
[DestructiveChangesType.PRE]: new Map<string, Map<string, SourceComponent>>(),
[DestructiveChangesType.POST]: new Map<string, Map<string, SourceComponent>>(),
[DestructiveChangesType.PRE]: new DecodeableMap<string, DecodeableMap<string, SourceComponent>>(),
[DestructiveChangesType.POST]: new DecodeableMap<string, DecodeableMap<string, SourceComponent>>(),
};
private manifestComponents = new Map<string, Map<string, SourceComponent>>();
private manifestComponents = new DecodeableMap<string, DecodeableMap<string, SourceComponent>>();

private destructiveChangesType = DestructiveChangesType.POST;

Expand Down Expand Up @@ -485,7 +557,7 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
public add(component: ComponentLike, deletionType?: DestructiveChangesType): void {
const key = simpleKey(component);
if (!this.components.has(key)) {
this.components.set(key, new Map<string, SourceComponent>());
this.components.set(key, new DecodeableMap<string, SourceComponent>());
}

if (!(component instanceof SourceComponent)) {
Expand All @@ -502,12 +574,12 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
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.set(key, new DecodeableMap<string, SourceComponent>());
}
deletions.get(key)?.set(sourceKey(component), component);
} else {
if (!this.manifestComponents.has(key)) {
this.manifestComponents.set(key, new Map<string, SourceComponent>());
this.manifestComponents.set(key, new DecodeableMap<string, SourceComponent>());
}
this.manifestComponents.get(key)?.set(sourceKey(component), component);
}
Expand Down Expand Up @@ -542,10 +614,8 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
* @returns `true` if the component is in the set
*/
public has(component: ComponentLike): boolean {
// Compare the component key as is and decoded. Decoding the key before comparing can solve some edge cases
// in component fullNames such as Layouts. See: https://github.com/forcedotcom/cli/issues/1683
const key = simpleKey(component);
if (this.components.has(key) || this.components.has(decodeURIComponent(key))) {
if (this.components.has(key)) {
return true;
}

Expand Down
Loading