Skip to content

Commit

Permalink
fix: component set maps treat encoded and decoded keys as the same (#…
Browse files Browse the repository at this point in the history
…1138)

* feat: component set components are now DecodableMaps

* fix: move DecodeableMap to a new file

* chore: add tests for DecodeableMap

* fix: use internal map of decoded to encoded keys

* chore: update comment for internal map initialization
  • Loading branch information
shetzel authored Oct 16, 2023
1 parent d9e0c05 commit 7fe0bab
Show file tree
Hide file tree
Showing 3 changed files with 350 additions and 12 deletions.
23 changes: 11 additions & 12 deletions src/collections/componentSet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
PackageTypeMembers,
} from './types';
import { LazyCollection } from './lazyCollection';
import { DecodeableMap } from './decodeableMap';

Messages.importMessagesDirectory(__dirname);
const messages = Messages.loadMessages('@salesforce/source-deploy-retrieve', 'sdr');
Expand Down Expand Up @@ -73,14 +74,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 @@ -112,11 +113,11 @@ export class ComponentSet extends LazyCollection<MetadataComponent> {
return size;
}

public get destructiveChangesPre(): Map<string, Map<string, SourceComponent>> {
public get destructiveChangesPre(): DecodeableMap<string, DecodeableMap<string, SourceComponent>> {
return this.destructiveComponents[DestructiveChangesType.PRE];
}

public get destructiveChangesPost(): Map<string, Map<string, SourceComponent>> {
public get destructiveChangesPost(): DecodeableMap<string, DecodeableMap<string, SourceComponent>> {
return this.destructiveComponents[DestructiveChangesType.POST];
}

Expand Down Expand Up @@ -485,7 +486,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 +503,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 +543,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
98 changes: 98 additions & 0 deletions src/collections/decodeableMap.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
* Copyright (c) 2023, salesforce.com, inc.
* All rights reserved.
* Licensed under the BSD 3-Clause license.
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

/**
* This is an extension of the Map class that can match keys whether they are encoded or decoded.
* Decoding the key can solve some edge cases in component fullNames such as Layouts and Profiles.
* See: https://github.com/forcedotcom/cli/issues/1683
*
* Examples:
*
* Given a map with entries:
* ```javascript
* 'layout#Layout__Broker__c-v1.1 Broker Layout' : {...}
* 'layout#Layout__Broker__c-v9%2E2 Broker Layout' : {...}
* ```
*
* `decodeableMap.has('layout#Layout__Broker__c-v1%2E1 Broker Layout')` --> returns `true`
* `decodeableMap.has('layout#Layout__Broker__c-v9.2 Broker Layout')` --> returns `true`
*/
export class DecodeableMap<K extends string, V> extends Map<string, V> {
// Internal map of decoded keys to encoded keys.
// E.g., { 'foo-v1.3 Layout': 'foo-v1%2E3 Layout' }
// This is initialized in the `keysMap` getter due to the constructor calling
// `super` before the initialization happens, and it needs to be initialized
// before `this.set()` is called or `TypeErrors` will be thrown.
private internalkeysMap!: Map<string, string>;

private get keysMap(): Map<string, string> {
if (!this.internalkeysMap) {
this.internalkeysMap = new Map();
}
return this.internalkeysMap;
}

/**
* boolean indicating whether an element with the specified key (matching decoded) exists or not.
*/
public has(key: K): boolean {
return !!this.getExistingKey(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 {
const existingKey = this.getExistingKey(key);
return existingKey ? super.get(existingKey) : undefined;
}

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

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

// Optimistically looks for an existing key. If the key is not found, detect if the
// key is encoded. If encoded, try using the decoded key. If decoded, look for an
// encoded entry in the internal map to use for the lookup.
private getExistingKey(key: K, setInKeysMap = false): string | undefined {
if (super.has(key)) {
return key;
} else {
const decodedKey = decodeURIComponent(key);
if (key !== decodedKey) {
// The key is encoded; If this is part of a set operation,
// set the { decodedKey : encodedKey } in the internal map.
if (setInKeysMap) {
this.keysMap.set(decodedKey, key);
}
if (super.has(decodedKey)) {
return decodedKey;
}
} else {
const encodedKey = this.keysMap.get(decodedKey);
if (encodedKey && super.has(encodedKey)) {
return encodedKey;
}
}
}
}
}
Loading

0 comments on commit 7fe0bab

Please sign in to comment.