From 615c477f777d71742c5a741e3852b3f780bf44c0 Mon Sep 17 00:00:00 2001 From: Steve Hetzel Date: Tue, 5 Sep 2023 09:42:35 -0600 Subject: [PATCH] Revert "feat!: component set components are now DecodableMaps (#1080)" (#1101) This reverts commit 26aef11495835caa7735fb1b96c6ab591c95ea7e. --- src/collections/componentSet.ts | 23 ++-- src/collections/decodeableMap.ts | 78 ------------ test/collections/decodeableMap.test.ts | 168 ------------------------- 3 files changed, 12 insertions(+), 257 deletions(-) delete mode 100644 src/collections/decodeableMap.ts delete mode 100644 test/collections/decodeableMap.test.ts diff --git a/src/collections/componentSet.ts b/src/collections/componentSet.ts index bea09a5346..6bb3e28130 100644 --- a/src/collections/componentSet.ts +++ b/src/collections/componentSet.ts @@ -34,7 +34,6 @@ import { PackageTypeMembers, } from './types'; import { LazyCollection } from './lazyCollection'; -import { DecodeableMap } from './decodeableMap'; Messages.importMessagesDirectory(__dirname); const messages = Messages.loadMessages('@salesforce/source-deploy-retrieve', 'sdr'); @@ -74,14 +73,14 @@ export class ComponentSet extends LazyCollection { public forceIgnoredPaths?: Set; private logger: Logger; private registry: RegistryAccess; - private components = new DecodeableMap>(); + private components = new Map>(); // internal component maps used by this.getObject() when building manifests. private destructiveComponents = { - [DestructiveChangesType.PRE]: new DecodeableMap>(), - [DestructiveChangesType.POST]: new DecodeableMap>(), + [DestructiveChangesType.PRE]: new Map>(), + [DestructiveChangesType.POST]: new Map>(), }; - private manifestComponents = new DecodeableMap>(); + private manifestComponents = new Map>(); private destructiveChangesType = DestructiveChangesType.POST; @@ -113,11 +112,11 @@ export class ComponentSet extends LazyCollection { return size; } - public get destructiveChangesPre(): DecodeableMap> { + public get destructiveChangesPre(): Map> { return this.destructiveComponents[DestructiveChangesType.PRE]; } - public get destructiveChangesPost(): DecodeableMap> { + public get destructiveChangesPost(): Map> { return this.destructiveComponents[DestructiveChangesType.POST]; } @@ -486,7 +485,7 @@ export class ComponentSet extends LazyCollection { public add(component: ComponentLike, deletionType?: DestructiveChangesType): void { const key = simpleKey(component); if (!this.components.has(key)) { - this.components.set(key, new DecodeableMap()); + this.components.set(key, new Map()); } if (!(component instanceof SourceComponent)) { @@ -503,12 +502,12 @@ export class ComponentSet extends LazyCollection { this.logger.debug(`Marking component for delete: ${component.fullName}`); const deletions = this.destructiveComponents[deletionType]; if (!deletions.has(key)) { - deletions.set(key, new DecodeableMap()); + deletions.set(key, new Map()); } deletions.get(key)?.set(sourceKey(component), component); } else { if (!this.manifestComponents.has(key)) { - this.manifestComponents.set(key, new DecodeableMap()); + this.manifestComponents.set(key, new Map()); } this.manifestComponents.get(key)?.set(sourceKey(component), component); } @@ -543,8 +542,10 @@ export class ComponentSet extends LazyCollection { * @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)) { + if (this.components.has(key) || this.components.has(decodeURIComponent(key))) { return true; } diff --git a/src/collections/decodeableMap.ts b/src/collections/decodeableMap.ts deleted file mode 100644 index 86edacb720..0000000000 --- a/src/collections/decodeableMap.ts +++ /dev/null @@ -1,78 +0,0 @@ -/* - * 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 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` - */ -export class DecodeableMap extends Map { - /** - * 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; - } - } - } -} diff --git a/test/collections/decodeableMap.test.ts b/test/collections/decodeableMap.test.ts deleted file mode 100644 index 743f3c91b4..0000000000 --- a/test/collections/decodeableMap.test.ts +++ /dev/null @@ -1,168 +0,0 @@ -/* - * Copyright (c) 2020, 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 - */ -import { expect } from 'chai'; -import * as sinon from 'sinon'; -import { DecodeableMap } from '../../src/collections/decodeableMap'; - -describe('DecodeableMap', () => { - let dMap: DecodeableMap; - const ENCODED_KEY = 'encodedKey'; - const DECODED_KEY = 'decodedKey'; - - const sandbox = sinon.createSandbox(); - let hasDecodedSpy: sinon.SinonSpy; - let getDecodedSpy: sinon.SinonSpy; - let hasMapSpy: sinon.SinonSpy; - let getMapSpy: sinon.SinonSpy; - let setMapSpy: sinon.SinonSpy; - let deleteMapSpy: sinon.SinonSpy; - - beforeEach(() => { - dMap = new DecodeableMap([ - ['Layout-v1%2E1 Layout', ENCODED_KEY], - ['Layout-v9.2 Layout', DECODED_KEY], - ]); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - hasDecodedSpy = sandbox.spy(dMap, 'hasDecoded' as any); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getDecodedSpy = sandbox.spy(dMap, 'getDecoded' as any); - hasMapSpy = sandbox.spy(Map.prototype, 'has'); - getMapSpy = sandbox.spy(Map.prototype, 'get'); - setMapSpy = sandbox.spy(Map.prototype, 'set'); - deleteMapSpy = sandbox.spy(Map.prototype, 'delete'); - }); - - afterEach(() => { - sandbox.restore(); - }); - - describe('has()', () => { - it('should match on exact key without decoding', () => { - expect(dMap.has('Layout-v1%2E1 Layout')).to.be.true; - expect(hasMapSpy.called).to.be.true; - expect(hasDecodedSpy.called).to.be.false; - }); - - it('should match encoded key with decoded value', () => { - expect(dMap.has('Layout-v1.1 Layout')).to.be.true; - expect(hasMapSpy.called).to.be.true; - expect(hasDecodedSpy.called).to.be.true; - }); - - it('should match decoded key with encoded value', () => { - expect(dMap.has('Layout-v9%2E2 Layout')).to.be.true; - expect(hasMapSpy.called).to.be.true; - expect(hasDecodedSpy.called).to.be.true; - }); - - it('should not match on no existing key', () => { - expect(dMap.has('Layout-MISSING Layout')).to.be.false; - expect(hasMapSpy.called).to.be.true; - expect(hasDecodedSpy.called).to.be.true; - }); - }); - - describe('get()', () => { - it('should get value with exact key without decoding', () => { - expect(dMap.get('Layout-v1%2E1 Layout')).to.equal(ENCODED_KEY); - expect(getMapSpy.calledOnce).to.be.true; - expect(getDecodedSpy.called).to.be.false; - }); - - it('should get value of encoded key using decoded key', () => { - expect(dMap.get('Layout-v1.1 Layout')).to.equal(ENCODED_KEY); - expect(getMapSpy.calledTwice).to.be.true; - expect(getDecodedSpy.calledOnce).to.be.true; - }); - - it('should get value of decoded key using encoded key', () => { - expect(dMap.get('Layout-v9%2E2 Layout')).to.equal(DECODED_KEY); - expect(getMapSpy.calledTwice).to.be.true; - expect(getDecodedSpy.calledOnce).to.be.true; - }); - - it('should return undefined on no existing key', () => { - expect(dMap.get('Layout-MISSING Layout')).to.be.undefined; - expect(getMapSpy.calledOnce).to.be.true; - expect(getDecodedSpy.called).to.be.true; - }); - }); - - describe('set()', () => { - const NEW_VALUE = 'new value from set'; - - it('should set value with exact key', () => { - expect(dMap.set('Layout-v1%2E1 Layout', NEW_VALUE)).to.equal(dMap); - expect(setMapSpy.called).to.be.true; - expect(setMapSpy.lastCall.args[0]).to.equal('Layout-v1%2E1 Layout'); - expect(setMapSpy.lastCall.args[1]).to.equal(NEW_VALUE); - expect(dMap.size).to.equal(2); - expect(dMap.get('Layout-v1%2E1 Layout')).to.equal(NEW_VALUE); - }); - - it('should set value of encoded key using decoded key', () => { - expect(dMap.set('Layout-v1.1 Layout', NEW_VALUE)).to.equal(dMap); - expect(setMapSpy.called).to.be.true; - expect(setMapSpy.lastCall.args[0]).to.equal('Layout-v1%2E1 Layout'); - expect(setMapSpy.lastCall.args[1]).to.equal(NEW_VALUE); - expect(dMap.size).to.equal(2); - expect(dMap.get('Layout-v1%2E1 Layout')).to.equal(NEW_VALUE); - }); - - it('should set value of decoded key using encoded key', () => { - expect(dMap.set('Layout-v9%2E2 Layout', NEW_VALUE)).to.equal(dMap); - expect(setMapSpy.called).to.be.true; - expect(setMapSpy.lastCall.args[0]).to.equal('Layout-v9.2 Layout'); - expect(setMapSpy.lastCall.args[1]).to.equal(NEW_VALUE); - expect(dMap.size).to.equal(2); - expect(dMap.get('Layout-v9.2 Layout')).to.equal(NEW_VALUE); - }); - - it('should set new entry on no existing key', () => { - expect(dMap.set('Layout-MISSING Layout', NEW_VALUE)).to.equal(dMap); - expect(setMapSpy.called).to.be.true; - expect(setMapSpy.lastCall.args[0]).to.equal('Layout-MISSING Layout'); - expect(setMapSpy.lastCall.args[1]).to.equal(NEW_VALUE); - expect(dMap.size).to.equal(3); - expect(dMap.get('Layout-MISSING Layout')).to.equal(NEW_VALUE); - }); - }); - - describe('delete()', () => { - it('should delete using exact key', () => { - expect(dMap.delete('Layout-v1%2E1 Layout')).to.be.true; - expect(deleteMapSpy.calledOnce).to.be.true; - expect(deleteMapSpy.firstCall.args[0]).to.equal('Layout-v1%2E1 Layout'); - expect(dMap.size).to.equal(1); - expect(dMap.has('Layout-v1%2E1 Layout')).to.be.false; - }); - - it('should delete the encoded key using decoded value', () => { - expect(dMap.delete('Layout-v1.1 Layout')).to.be.true; - expect(deleteMapSpy.calledOnce).to.be.true; - expect(deleteMapSpy.firstCall.args[0]).to.equal('Layout-v1%2E1 Layout'); - expect(dMap.size).to.equal(1); - expect(dMap.has('Layout-v1.1 Layout')).to.be.false; - }); - - it('should delete the decoded key using encoded value', () => { - expect(dMap.delete('Layout-v9%2E2 Layout')).to.be.true; - expect(deleteMapSpy.calledOnce).to.be.true; - expect(deleteMapSpy.firstCall.args[0]).to.equal('Layout-v9.2 Layout'); - expect(dMap.size).to.equal(1); - expect(dMap.has('Layout-v9%2E2 Layout')).to.be.false; - }); - - it('should not delete on no existing key', () => { - expect(dMap.delete('Layout-MISSING Layout')).to.be.false; - expect(deleteMapSpy.calledOnce).to.be.true; - expect(deleteMapSpy.firstCall.args[0]).to.equal('Layout-MISSING Layout'); - expect(dMap.size).to.equal(2); - expect(dMap.has('Layout-MISSING Layout')).to.be.false; - }); - }); -});