Sh/better mdapi tmp retrieve (#1331)

* fix:  better mdapi temp dir structure and more output

* fix:  update tests for code changes

src/client/metadataApiDeploy.ts

@@ -104,6 +104,9 @@ export class MetadataApiDeploy extends MetadataTransfer<
     this.options = Object.assign({}, options);
     this.isRestDeploy = !!options.apiOptions?.rest;
     this.registry = options.registry ?? new RegistryAccess();
+    if (this.mdapiTempDir) {
+      this.mdapiTempDir = join(this.mdapiTempDir, `${new Date().toISOString()}_deploy`);
+    }
   }
 
   /**

src/client/metadataApiRetrieve.ts

@@ -4,7 +4,7 @@
  * 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 * as path from 'node:path';
+import { join, parse } from 'node:path';
 import fs from 'graceful-fs';
 import JSZip from 'jszip';
 import { asBoolean, isString } from '@salesforce/ts-types';
@@ -125,6 +125,9 @@ export class MetadataApiRetrieve extends MetadataTransfer<
   public constructor(options: MetadataApiRetrieveOptions) {
     super(options);
     this.options = Object.assign({}, MetadataApiRetrieve.DEFAULT_OPTIONS, options);
+    if (this.mdapiTempDir) {
+      this.mdapiTempDir = join(this.mdapiTempDir, `${new Date().toISOString()}_retrieve`);
+    }
   }
 
   /**
@@ -172,6 +175,18 @@ export class MetadataApiRetrieve extends MetadataTransfer<
       if (isMdapiRetrieve) {
         await handleMdapiResponse(this.options, zipFileContents);
       } else {
+        // If mdapiTempDir is set, write the raw retrieve result to the temp dir
+        if (this.mdapiTempDir && zipFileContents) {
+          const outputDir = join(this.mdapiTempDir, 'metadata');
+          fs.mkdirSync(outputDir, { recursive: true });
+          const mdapiTempOptions = {
+            usernameOrConnection: this.options.usernameOrConnection,
+            output: outputDir,
+            unzip: true,
+          };
+          await handleMdapiResponse(mdapiTempOptions, zipFileContents);
+        }
+
         ({ componentSet, partialDeleteFileResponses } = await extract({
           zip: zipFileContents,
           options: this.options,
@@ -272,22 +287,22 @@ export type ScopedPostRetrieve = {
 
 const handleMdapiResponse = async (options: MetadataApiRetrieveOptions, zipFileContents: Buffer): Promise<void> => {
   const name = options.zipFileName ?? 'unpackaged.zip';
-  const zipFilePath = path.join(options.output, name);
+  const zipFilePath = join(options.output, name);
   fs.writeFileSync(zipFilePath, zipFileContents);
 
   if (options.unzip) {
     const zip = await JSZip.loadAsync(zipFileContents, { base64: true, createFolders: true });
-    const extractPath = path.join(options.output, path.parse(name).name);
+    const extractPath = join(options.output, parse(name).name);
     fs.mkdirSync(extractPath, { recursive: true });
     for (const filePath of Object.keys(zip.files)) {
       const zipObj = zip.file(filePath);
       if (!zipObj || zipObj?.dir) {
-        fs.mkdirSync(path.join(extractPath, filePath), { recursive: true });
+        fs.mkdirSync(join(extractPath, filePath), { recursive: true });
       } else {
         // eslint-disable-next-line no-await-in-loop
         const content = await zipObj?.async('nodebuffer');
         if (content) {
-          fs.writeFileSync(path.join(extractPath, filePath), content);
+          fs.writeFileSync(join(extractPath, filePath), content);
         }
       }
     }

src/client/metadataTransfer.ts

@@ -42,6 +42,7 @@ export abstract class MetadataTransfer<
   protected components?: ComponentSet;
   protected logger: Logger;
   protected canceled = false;
+  protected mdapiTempDir?: string;
   private transferId: Options['id'];
   private event = new EventEmitter();
   private usernameOrConnection: string | Connection;
@@ -53,6 +54,7 @@ export abstract class MetadataTransfer<
     this.apiVersion = apiVersion;
     this.transferId = id;
     this.logger = Logger.childFromRoot(this.constructor.name);
+    this.mdapiTempDir = process.env.SF_MDAPI_TEMP_DIR;
   }
 
   // if you passed in an id, you don't have to worry about whether there'll be one if you ask for it
@@ -160,24 +162,24 @@ export abstract class MetadataTransfer<
   }
 
   protected async maybeSaveTempDirectory(target: SfdxFileFormat, cs?: ComponentSet): Promise<void> {
-    const mdapiTempDir = process.env.SF_MDAPI_TEMP_DIR;
-    if (mdapiTempDir) {
+    if (this.mdapiTempDir) {
       await Lifecycle.getInstance().emitWarning(
         'The SF_MDAPI_TEMP_DIR environment variable is set, which may degrade performance'
       );
       this.logger.debug(
-        `Converting metadata to: ${mdapiTempDir} because the SF_MDAPI_TEMP_DIR environment variable is set`
+        `Converting metadata to: ${this.mdapiTempDir} because the SF_MDAPI_TEMP_DIR environment variable is set`
       );
       try {
         const source = cs ?? this.components ?? new ComponentSet();
-        const converter = new MetadataConverter();
-        await converter.convert(source, target, {
+        const outputDirectory = join(this.mdapiTempDir, target);
+        await new MetadataConverter().convert(source, target, {
           type: 'directory',
-          outputDirectory: mdapiTempDir,
+          outputDirectory,
+          genUniqueDir: false,
         });
         if (target === 'source') {
           // for source convert the package.xml isn't included so write it separately
-          await fs.promises.writeFile(join(mdapiTempDir, 'package.xml'), await source.getPackageXml());
+          await fs.promises.writeFile(join(outputDirectory, 'package.xml'), await source.getPackageXml());
         }
       } catch (e) {
         this.logger.debug(e);

test/client/metadataApiDeploy.test.ts

@@ -4,7 +4,7 @@
  * 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 { basename, join } from 'node:path';
+import { basename, join, sep } from 'node:path';
 import { MockTestOrgData, TestContext } from '@salesforce/core/testSetup';
 import chai, { assert, expect } from 'chai';
 import { AnyJson, getString } from '@salesforce/ts-types';
@@ -100,7 +100,10 @@ describe('MetadataApiDeploy', () => {
 
           expect(deployStub.calledOnce).to.be.true;
           expect(deployStub.firstCall.args[0]).to.equal(zipBuffer);
-          expect(getString(convertStub.secondCall.args[2], 'outputDirectory', '')).to.equal('test');
+          // @ts-expect-error protected property
+          const expectedDir = join(operation.mdapiTempDir, 'metadata');
+          expect(expectedDir.startsWith(`test${sep}`)).to.be.true;
+          expect(getString(convertStub.secondCall.args[2], 'outputDirectory', '')).to.equal(expectedDir);
         } finally {
           delete process.env.SF_MDAPI_TEMP_DIR;
         }

test/client/metadataApiRetrieve.test.ts

@@ -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 { fail } from 'node:assert';
-import { join } from 'node:path';
+import { join, sep } from 'node:path';
 import { Messages } from '@salesforce/core';
 import { assert, expect } from 'chai';
 import chai = require('chai');
@@ -311,11 +311,16 @@ describe('MetadataApiRetrieve', () => {
             successes: toRetrieve,
           });
           $$.SANDBOX.stub(fs.promises, 'writeFile');
+          $$.SANDBOX.stub(fs, 'mkdirSync');
+          $$.SANDBOX.stub(fs, 'writeFileSync');
 
           await operation.start();
           await operation.pollStatus();
 
-          expect(getString(convertStub.secondCall.args[2], 'outputDirectory', '')).to.equal('test');
+          // @ts-expect-error protected property
+          const expectedDir = join(operation.mdapiTempDir, 'source');
+          expect(expectedDir.startsWith(`test${sep}`)).to.be.true;
+          expect(getString(convertStub.secondCall.args[2], 'outputDirectory', '')).to.equal(expectedDir);
         } finally {
           delete process.env.SF_MDAPI_TEMP_DIR;
         }