Skip to content

Commit

Permalink
fix(SC): cache XML content (#1383)
Browse files Browse the repository at this point in the history 
* chore: introduce path/content map... breaking UTs, maybe just store content on CS

* chore: no more map, skip UTs

* chore: introduce path/content map... breaking UTs, maybe just store content on CS

* test: fix UTs, add UT
  • Loading branch information
@WillieRuemmele
WillieRuemmele committed Aug 12, 2024
1 parent 5548407 commit 7b2d8cd
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 14 deletions.
19 changes: 17 additions & 2 deletions src/resolve/sourceComponent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export class SourceComponent implements MetadataComponent {
private readonly forceIgnore: ForceIgnore;
private markedForDelete = false;
private destructiveChangesType?: DestructiveChangesType;
private pathContentMap = new Map<string, string>();

public constructor(
props: ComponentProperties,
Expand Down Expand Up @@ -187,7 +188,14 @@ export class SourceComponent implements MetadataComponent {
public async parseXml<T extends JsonMap>(xmlFilePath?: string): Promise<T> {
const xml = xmlFilePath ?? this.xml;
if (xml) {
const contents = (await this.tree.readFile(xml)).toString();
let contents: string;
if (this.pathContentMap.has(xml)) {
contents = this.pathContentMap.get(xml) as string;
} else {
contents = (await this.tree.readFile(xml)).toString();
this.pathContentMap.set(xml, contents);
}

const replacements = this.replacements?.[xml] ?? this.parent?.replacements?.[xml];
return this.parseAndValidateXML<T>(
replacements ? await replacementIterations(contents, replacements) : contents,
Expand All @@ -200,7 +208,14 @@ export class SourceComponent implements MetadataComponent {
public parseXmlSync<T extends JsonMap>(xmlFilePath?: string): T {
const xml = xmlFilePath ?? this.xml;
if (xml) {
const contents = this.tree.readFileSync(xml).toString();
let contents: string;
if (this.pathContentMap.has(xml)) {
contents = this.pathContentMap.get(xml) as string;
} else {
contents = this.tree.readFileSync(xml).toString();
this.pathContentMap.set(xml, contents);
}

return this.parseAndValidateXML(contents, xml);
}
return {} as T;
Expand Down
14 changes: 11 additions & 3 deletions test/convert/convertContext/nonDecomposition.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
VIRTUAL_DIR,
WORKING_DIR,
} from '../../mock/type-constants/customlabelsConstant';
import { SourceComponent } from '../../../src';

describe('NonDecomposition', () => {
const env = createSandbox();
Expand Down Expand Up @@ -57,11 +58,13 @@ describe('NonDecomposition', () => {
};

const result = await context.nonDecomposition.finalize(nonDecomposed.DEFAULT_DIR, TREE);
// @ts-ignore - because the resulting component will have cache info, and the initial one won't
result[0].component.pathContentMap = new Map();
expect(result).to.deep.equal([{ component, writeInfos }]);
});

it('should return WriterFormats when no local files exist', async () => {
const component = nonDecomposed.COMPONENT_1;
const component = new SourceComponent(nonDecomposed.COMPONENT_1);
const context = new ConvertContext();
const [baseName] = component.fullName.split('.');
const output = join(
Expand Down Expand Up @@ -115,12 +118,13 @@ describe('NonDecomposition', () => {
};

const result = await context.nonDecomposition.finalize(nonDecomposed.DEFAULT_DIR, TREE);

// @ts-ignore - because the resulting component will have cache info, and the initial one won't
result[0].component.pathContentMap = new Map();
expect(result).to.deep.equal([{ component, writeInfos }]);
});

it('should merge 1 updated file', async () => {
const component = nonDecomposed.COMPONENT_1;
const component = new SourceComponent(nonDecomposed.COMPONENT_1, TREE);
const context = new ConvertContext();
const type = component.type;

Expand All @@ -144,6 +148,8 @@ describe('NonDecomposition', () => {
};

const result = await context.nonDecomposition.finalize(nonDecomposed.DEFAULT_DIR, TREE);
// @ts-ignore - because the resulting component will have cache info, and the initial one won't
result[0].component.pathContentMap = new Map();
expect(result).to.deep.equal([{ component, writeInfos }]);
});

Expand Down Expand Up @@ -187,6 +193,8 @@ describe('NonDecomposition', () => {
};

const result = await context.nonDecomposition.finalize(nonDecomposed.DEFAULT_DIR, TREE);
// @ts-ignore - because the resulting component will have cache info, and the initial one won't
result[0].component.pathContentMap = new Map();
expect(result).to.deep.equal([{ component, writeInfos }]);
});
});
59 changes: 54 additions & 5 deletions test/convert/convertContext/recomposition.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,37 @@ describe('Recomposition', () => {
});

it('should only read parent xml file once for non-decomposed components with children', async () => {
const component = nonDecomposed.COMPONENT_1;
const readFileSpy = env.spy(component.tree, 'readFile');

const context = new ConvertContext();
context.recomposition.transactionState.set(component.fullName, {
component,
children: new ComponentSet(component.getChildren()),
});

const result = await context.recomposition.finalize();
expect(result).to.deep.equal([
{
component,
writeInfos: [
{
source: new JsToXml({
CustomLabels: {
[XML_NS_KEY]: XML_NS_URL,
labels: [CHILD_1_XML, CHILD_2_XML],
},
}),
output: join('labels', 'CustomLabels.labels'),
},
],
},
]);

expect(readFileSpy.callCount, JSON.stringify(readFileSpy.getCalls(), undefined, 2)).to.equal(0);
});

it('should not read parent once already read', async () => {
const component = nonDecomposed.COMPONENT_1;
const context = new ConvertContext();
context.recomposition.transactionState.set(component.fullName, {
Expand Down Expand Up @@ -151,7 +182,7 @@ describe('Recomposition', () => {
},
]);

expect(readFileSpy.callCount, JSON.stringify(readFileSpy.getCalls(), undefined, 2)).to.equal(1);
expect(readFileSpy.callCount, JSON.stringify(readFileSpy.getCalls(), undefined, 2)).to.equal(0);
});

describe('should only read unique child xml files once per parent for non-decomposed components', () => {
Expand Down Expand Up @@ -201,31 +232,49 @@ describe('Recomposition', () => {
],
},
];
const virtualTree = new VirtualTreeContainer(vDir);
const component = new SourceComponent(
{ name: customLabelsType.name, type: customLabelsType, xml: parentXmlPath1 },
new VirtualTreeContainer(vDir)
virtualTree
);
const component2 = new SourceComponent(
{ name: 'CustomLabels2', type: customLabelsType, xml: parentXmlPath2 },
new VirtualTreeContainer(vDir)
virtualTree
);

it('one main component with multiple parents in transaction state covering all the children', async () => {
const context = new ConvertContext();
const compSet = new ComponentSet();
const readFileSpy = env.spy(virtualTree, 'readFileSync');

component.getChildren().forEach((child) => compSet.add(child));
component2.getChildren().forEach((child) => compSet.add(child));
context.recomposition.transactionState.set(component.fullName, {
component,
children: compSet,
});

const readFileSpy = env.spy(VirtualTreeContainer.prototype, 'readFile');

await context.recomposition.finalize();

expect(readFileSpy.callCount, 'readFile() should only be called twice').to.equal(2);
});

it('once the parent/child file content is cached, it wont read them again', async () => {
const context = new ConvertContext();
const compSet = new ComponentSet();

component.getChildren().forEach((child) => compSet.add(child));
component2.getChildren().forEach((child) => compSet.add(child));
context.recomposition.transactionState.set(component.fullName, {
component,
children: compSet,
});
const readFileSpy = env.spy(virtualTree, 'readFileSync');

await context.recomposition.finalize();

expect(readFileSpy.callCount, 'readFile() should only be called twice').to.equal(0);
});
});
});
});
2 changes: 2 additions & 0 deletions test/resolve/adapters/decomposedSourceAdapter.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ describe('DecomposedSourceAdapter', () => {
assert(decomposed.DECOMPOSED_CHILD_COMPONENT_1.xml);
const result = adapter.getComponent(decomposed.DECOMPOSED_CHILD_COMPONENT_1.xml);

// @ts-ignore - this only failed when running 'yarn test' - parent has cache info the result won't
component.parent.pathContentMap = new Map();
expect(result).to.deep.equal(component);
});

Expand Down
10 changes: 6 additions & 4 deletions test/resolve/sourceComponent.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/
import { join } from 'node:path';
import { assert, expect } from 'chai';
import { assert, config, expect } from 'chai';
import { createSandbox } from 'sinon';
import { Messages, SfError } from '@salesforce/core';
import { decomposed, matchingContentFile, mixedContentDirectory, xmlInFolder } from '../mock';
Expand Down Expand Up @@ -41,6 +41,8 @@ import { DE_METAFILE } from '../mock/type-constants/digitalExperienceBundleConst
import { XML_NS_KEY, XML_NS_URL } from '../../src/common';
import { RegistryTestUtil } from './registryTestUtil';

config.truncateThreshold = 0;

Messages.importMessagesDirectory(__dirname);
const messages = Messages.loadMessages('@salesforce/source-deploy-retrieve', 'sdr');

Expand Down Expand Up @@ -244,7 +246,7 @@ describe('SourceComponent', () => {
});

it('should preserve leading zeroes in node values', async () => {
const component = COMPONENT;
const component = new SourceComponent(COMPONENT);
env
.stub(component.tree, 'readFile')
.resolves(Buffer.from('<MatchingContentFile><test>001</test></MatchingContentFile>'));
Expand All @@ -260,7 +262,7 @@ describe('SourceComponent', () => {
});

it('should parse cdata node values', async () => {
const component = COMPONENT;
const component = new SourceComponent(COMPONENT);
env
.stub(component.tree, 'readFile')
.resolves(Buffer.from('<MatchingContentFile><test><![CDATA[<p>Hello</p>]]></test></MatchingContentFile>'));
Expand All @@ -276,7 +278,7 @@ describe('SourceComponent', () => {
});

it('should parse attributes of nodes', async () => {
const component = COMPONENT;
const component = new SourceComponent(COMPONENT);
env
.stub(component.tree, 'readFile')
.resolves(Buffer.from('<MatchingContentFile a="test"><test>something</test></MatchingContentFile>'));
Expand Down

0 comments on commit 7b2d8cd

Please sign in to comment.