Skip to content

Commit

Permalink
fix: use parent xml to parse child (#470)
Browse files Browse the repository at this point in the history 
* fix: use parent xml to parse child

* fix: cache parent parsed xml for children

* test: add a unit test
  • Loading branch information
@shetzel
shetzel authored Oct 11, 2021
1 parent e619d05 commit 440d2be
Show file tree
Hide file tree
Showing 4 changed files with 187 additions and 19 deletions.
44 changes: 36 additions & 8 deletions src/convert/convertContext.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { META_XML_SUFFIX, XML_NS_KEY, XML_NS_URL } from '../common';
import { getString, JsonArray, JsonMap } from '@salesforce/ts-types';
import { ComponentSet } from '../collections';
import { normalizeToArray } from '../utils/collections';
import { RecompositionStrategy } from '../registry/types';
import { RecompositionStrategy, TransformerStrategy } from '../registry/types';
import { isEmpty } from '@salesforce/kit';

abstract class ConvertTransactionFinalizer<T> {
Expand Down Expand Up @@ -49,9 +49,12 @@ export interface RecompositionState {
class RecompositionFinalizer extends ConvertTransactionFinalizer<RecompositionState> {
protected _state: RecompositionState = {};

// A cache of SourceComponent xml file paths to parsed contents so that identical child xml
// files are not read and parsed multiple times.
private parsedXmlCache = new Map<string, JsonMap>();

public async finalize(): Promise<WriterFormat[]> {
const writerData: WriterFormat[] = [];

for (const { component: parent, children } of Object.values(this.state)) {
const recomposedXmlObj = await this.recompose(children, parent);
writerData.push({
Expand All @@ -69,15 +72,40 @@ class RecompositionFinalizer extends ConvertTransactionFinalizer<RecompositionSt
}

private async recompose(children: ComponentSet, parent: SourceComponent): Promise<JsonMap> {
// When recomposing children that are non-decomposed, read and cache the parent XML to prevent
// reading the parent source file (referenced in all child SourceComponents) multiple times.
let parentXml: JsonMap;
if (parent.type.strategies.transformer === TransformerStrategy.NonDecomposed) {
parentXml = await parent.parseXml();
this.parsedXmlCache.set(parent.xml, parentXml);
}

const parentXmlObj =
parent.type.strategies.recomposition === RecompositionStrategy.StartEmpty
? {}
: await parent.parseXml();
: parentXml ?? (await parent.parseXml());

for (const child of children) {
const { directoryName: groupName } = child.type;
const { name: parentName } = child.parent.type;
const xmlObj = await (child as SourceComponent).parseXml();
const childSourceComponent = child as SourceComponent;

let xmlObj: JsonMap;
if (parentXml) {
// If the xml file for the child is in the cache, use it. Otherwise
// read and cache the xml file that contains this child and use it.
if (!this.parsedXmlCache.has(childSourceComponent.xml)) {
this.parsedXmlCache.set(
childSourceComponent.xml,
await parent.parseXml(childSourceComponent.xml)
);
}
xmlObj = childSourceComponent.parseFromParentXml(
this.parsedXmlCache.get(childSourceComponent.xml)
);
} else {
xmlObj = await childSourceComponent.parseXml();
}
const childContents = xmlObj[child.type.name] || xmlObj;

if (!parentXmlObj[parentName]) {
Expand All @@ -90,14 +118,14 @@ class RecompositionFinalizer extends ConvertTransactionFinalizer<RecompositionSt
delete (childContents as JsonMap)[XML_NS_KEY];
}

const parent = parentXmlObj[parentName] as JsonMap;
const parentObj = parentXmlObj[parentName] as JsonMap;

if (!parent[groupName]) {
parent[groupName] = [];
if (!parentObj[groupName]) {
parentObj[groupName] = [];
}

// it might be an object and not an array. Example: custom object with a Field property containing a single field
const group = normalizeToArray(parent[groupName]) as JsonArray;
const group = normalizeToArray(parentObj[groupName]) as JsonArray;

group.push(childContents);
}
Expand Down
38 changes: 28 additions & 10 deletions src/resolve/sourceComponent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,22 +102,43 @@ export class SourceComponent implements MetadataComponent {
return [];
}

public async parseXml<T = JsonMap>(): Promise<T> {
if (this.xml) {
const contents = await this.tree.readFile(this.xml);
public async parseXml<T = JsonMap>(xmlFilePath?: string): Promise<T> {
const xml = xmlFilePath ?? this.xml;
if (xml) {
const contents = await this.tree.readFile(xml);
return this.parse<T>(contents.toString());
}
return {} as T;
}

public parseXmlSync<T = JsonMap>(): T {
if (this.xml) {
const contents = this.tree.readFileSync(this.xml);
public parseXmlSync<T = JsonMap>(xmlFilePath?: string): T {
const xml = xmlFilePath ?? this.xml;
if (xml) {
const contents = this.tree.readFileSync(xml);
return this.parse<T>(contents.toString());
}
return {} as T;
}

/**
* As a performance enhancement, use the already parsed parent xml source
* to return the child section of xml source. This is useful for non-decomposed
* transformers where all child source components reference the parent's
* xml file to prevent re-reading the same file multiple times.
*
* @param parentXml parsed parent XMl source as an object
* @returns child section of the parent's xml
*/
public parseFromParentXml<T = JsonMap>(parentXml: T): T {
if (!this.parent) {
return parentXml;
}
const children = normalizeToArray(
get(parentXml, `${this.parent.type.name}.${this.type.directoryName}`)
) as T[];
return children.find((c) => getString(c, this.type.uniqueIdElement) === this.name);
}

public getPackageRelativePath(fsPath: string, format: SfdxFileFormat): string {
return format === 'source'
? join(DEFAULT_PACKAGE_ROOT_SFDX, this.calculateRelativePath(fsPath))
Expand Down Expand Up @@ -167,10 +188,7 @@ export class SourceComponent implements MetadataComponent {
if (firstElement === this.type.name) {
return parsed;
} else if (this.parent) {
const children = normalizeToArray(
get(parsed, `${this.parent.type.name}.${this.type.directoryName}`)
) as T[];
return children.find((c) => getString(c, this.type.uniqueIdElement) === this.name);
return this.parseFromParentXml(parsed);
} else {
return parsed;
}
Expand Down
116 changes: 115 additions & 1 deletion test/convert/convertContext.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,14 @@ const { expect } = chai;
import { join } from 'path';
import { createSandbox, match } from 'sinon';
import { Readable } from 'stream';
import { SourceComponent, ComponentSet, WriterFormat } from '../../src';
import {
SourceComponent,
ComponentSet,
WriterFormat,
RegistryAccess,
VirtualTreeContainer,
VirtualDirectory,
} from '../../src';
import {
DEFAULT_PACKAGE_ROOT_SFDX,
META_XML_SUFFIX,
Expand Down Expand Up @@ -65,6 +72,8 @@ describe('Convert Transaction Constructs', () => {
};
});

const readFileSpy = env.spy(component.tree, 'readFile');

const result = await context.recomposition.finalize();

expect(result).to.deep.equal([
Expand All @@ -85,6 +94,7 @@ describe('Convert Transaction Constructs', () => {
],
},
]);
expect(readFileSpy.callCount).to.equal(3);
});

it('should still recompose if parent xml is empty', async () => {
Expand Down Expand Up @@ -124,6 +134,110 @@ describe('Convert Transaction Constructs', () => {
},
]);
});

it('should only read parent xml file once for non-decomposed components with children', async () => {
const component = nonDecomposed.COMPONENT_1;
const context = new ConvertContext();
context.recomposition.setState((state) => {
state[component.type.name] = {
component,
children: new ComponentSet(component.getChildren(), mockRegistry),
};
});

const readFileSpy = env.spy(component.tree, 'readFile');

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

expect(readFileSpy.callCount).to.equal(1);
});

it('should only read unique child xml files once for non-decomposed components', async () => {
// This test sets up 2 CustomLabels files; 1 in each package directory. The CustomLabels files
// each have 2 labels within them. This should result in only 2 file reads.
const customLabelsType = new RegistryAccess().getTypeByName('CustomLabels');
const labelsFileName = 'CustomLabels.labels-meta.xml';
const projectDir = join(process.cwd(), 'my-project');
const packageDir1 = join(projectDir, 'pkgDir1');
const packageDir2 = join(projectDir, 'pkgDir2');
const dir1Labels = join(packageDir1, 'labels');
const dir2Labels = join(packageDir2, 'labels');
const parentXmlPath1 = join(dir1Labels, labelsFileName);
const parentXmlPath2 = join(dir2Labels, labelsFileName);
const labelXmls = [1, 2, 3, 4].map((i) => ({
fullName: `Child_${i}`,
description: `child ${i} desc`,
}));
const labelsXmls = [0, 2].map((i) => ({
[customLabelsType.name]: {
[XML_NS_KEY]: XML_NS_URL,
[customLabelsType.directoryName]: [labelXmls[i], labelXmls[i + 1]],
},
}));
const vDir: VirtualDirectory[] = [
{ dirPath: projectDir, children: ['pkgDir1', 'pkgDir2'] },
{ dirPath: packageDir1, children: ['labels'] },
{ dirPath: packageDir2, children: ['labels'] },
{
dirPath: dir1Labels,
children: [
{
name: labelsFileName,
data: Buffer.from(new JsToXml(labelsXmls[0]).read().toString()),
},
],
},
{
dirPath: dir2Labels,
children: [
{
name: labelsFileName,
data: Buffer.from(new JsToXml(labelsXmls[1]).read().toString()),
},
],
},
];
const component = new SourceComponent(
{ name: customLabelsType.name, type: customLabelsType, xml: parentXmlPath1 },
new VirtualTreeContainer(vDir)
);
const component2 = new SourceComponent(
{ name: customLabelsType.name, type: customLabelsType, xml: parentXmlPath2 },
new VirtualTreeContainer(vDir)
);
const context = new ConvertContext();
const compSet = new ComponentSet();
component.getChildren().forEach((child) => compSet.add(child));
component2.getChildren().forEach((child) => compSet.add(child));
context.recomposition.setState((state) => {
state[component.type.name] = {
component,
children: compSet,
};
});

const readFileSpy = env.spy(component.tree, 'readFile');

await context.recomposition.finalize();

expect(readFileSpy.callCount).to.equal(2);
});
});

describe('Decomposition', () => {
Expand Down
8 changes: 8 additions & 0 deletions test/resolve/sourceComponent.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import { COMPONENT } from '../mock/registry/type-constants/matchingContentFileCo
import {
COMPONENT_1,
CHILD_1_NAME,
CHILD_1_XML,
VIRTUAL_DIR,
COMPONENT_1_XML,
COMPONENT_1_XML_PATH,
CHILD_2_NAME,
MATCHING_RULES_TYPE,
Expand Down Expand Up @@ -334,6 +336,12 @@ describe('SourceComponent', () => {
expect(expectedChild.fullName).to.equal(expectedChild.name);
});

it('should parse child xml from parent xml', () => {
const childXml = expectedChild.parseFromParentXml(COMPONENT_1_XML);
expect(childXml).to.deep.equal(CHILD_1_XML);
expect(COMPONENT_1.parseFromParentXml(COMPONENT_1_XML)).to.deep.equal(COMPONENT_1_XML);
});

// https://github.com/forcedotcom/salesforcedx-vscode/issues/3210
it('should return empty children for types that do not have uniqueIdElement but xmlPathToChildren returns elements', () => {
const noUniqueIdElementType: MetadataType = JSON.parse(JSON.stringify(MATCHING_RULES_TYPE));
Expand Down

0 comments on commit 440d2be

Please sign in to comment.