fix: use parent xml to parse child

src/convert/convertContext.ts

@@ -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> {
@@ -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({
@@ -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]) {
@@ -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);
     }

src/resolve/sourceComponent.ts

@@ -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))
@@ -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;
     }

test/convert/convertContext.test.ts

@@ -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,
@@ -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([
@@ -85,6 +94,7 @@ describe('Convert Transaction Constructs', () => {
             ],
           },
         ]);
+        expect(readFileSpy.callCount).to.equal(3);
       });
 
       it('should still recompose if parent xml is empty', async () => {
@@ -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', () => {

test/resolve/sourceComponent.test.ts

@@ -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,
@@ -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));