Skip to content

Commit

Permalink
feat: replaces unzipper library with jszip (#1086)
Browse files Browse the repository at this point in the history 
* major: initial changes for swapping unzipper with jszip

* feat: replace unzipper with jszip

* chore: bump deps for xnuts

* fix: create folders on load

* fix: normalize zip file paths before comparison

* fix: read directory with resolved path

* fix: only posix paths added via createMockZip

---------

Co-authored-by: mshanemc <[email protected]>
  • Loading branch information
@shetzel @mshanemc
shetzel and mshanemc authored Oct 30, 2023
1 parent de781d7 commit 6bc0c12
Show file tree
Hide file tree
Showing 11 changed files with 805 additions and 844 deletions.
1,146 changes: 572 additions & 574 deletions METADATA_SUPPORT.md
View file

Large diffs are not rendered by default.

4 changes: 1 addition & 3 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@
"jszip": "^3.10.1",
"mime": "2.6.0",
"minimatch": "^5.1.6",
"proxy-agent": "^6.3.1",
"unzipper": "0.10.14"
"proxy-agent": "^6.3.1"
},
"devDependencies": {
"@salesforce/cli-plugins-testkit": "^4.4.10",
Expand All @@ -52,7 +51,6 @@
"@types/minimatch": "^5.1.2",
"@types/proxy-from-env": "^1.0.3",
"@types/shelljs": "^0.8.13",
"@types/unzipper": "^0.10.7",
"@typescript-eslint/eslint-plugin": "^5.62.0",
"@typescript-eslint/parser": "^5.62.0",
"chai": "^4.3.8",
Expand Down
18 changes: 15 additions & 3 deletions src/client/metadataApiRetrieve.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
import * as path from 'path';
import * as fs from 'graceful-fs';
import * as unzipper from 'unzipper';
import * as JSZip from 'jszip';
import { asBoolean, isString } from '@salesforce/ts-types';
import { Messages, SfError, Lifecycle } from '@salesforce/core';
import { ensureArray } from '@salesforce/kit';
Expand Down Expand Up @@ -185,9 +185,21 @@ export class MetadataApiRetrieve extends MetadataTransfer<
fs.writeFileSync(zipFilePath, zipFileContents);

if (this.options.unzip) {
const dir = await unzipper.Open.buffer(zipFileContents);
const zip = await JSZip.loadAsync(zipFileContents, { base64: true, createFolders: true });
const extractPath = path.join(this.options.output, path.parse(name).name);
await dir.extract({ path: extractPath });
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 });
} else {
// eslint-disable-next-line no-await-in-loop
const content = await zipObj?.async('nodebuffer');
if (content) {
fs.writeFileSync(path.join(extractPath, filePath), content);
}
}
}
}
} else {
components = await this.extract(zipFileContents);
Expand Down
36 changes: 18 additions & 18 deletions src/convert/transformers/staticResourceMetadataTransformer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import { basename, dirname, isAbsolute, join, relative } from 'path';
import { Readable } from 'stream';
import * as JSZip from 'jszip';
import { getExtension } from 'mime';
import { CentralDirectory, Open } from 'unzipper';
import { JsonMap } from '@salesforce/ts-types';
import { createWriteStream } from 'graceful-fs';
import { Logger, Messages, SfError } from '@salesforce/core';
Expand Down Expand Up @@ -116,20 +115,21 @@ export class StaticResourceMetadataTransformer extends BaseMetadataTransformer {
if (shouldUnzipArchive) {
// for the bulk of static resource writing we'll start writing ASAP
// we'll still defer writing the resource-meta.xml file by pushing it onto the writeInfos
await Promise.all(
(
await openZipFile(component, content)
).files
.filter((f) => f.type === 'File')
.map(async (f) => {
const path = join(baseContentPath, f.path);
const fullDest = isAbsolute(path)
? path
: join(this.defaultDirectory ?? component.getPackageRelativePath('', 'source'), path);
// push onto the pipeline and start writing now
return this.pipeline(f.stream(), fullDest);
})
);

const srZip = await getStaticResourceZip(component, content);
const pipelinePromises: Array<Promise<void>> = [];
for (const filePath of Object.keys(srZip.files)) {
const zipObj = srZip.file(filePath);
if (zipObj && !zipObj.dir) {
const path = join(baseContentPath, filePath);
const fullDest = isAbsolute(path)
? path
: join(this.defaultDirectory ?? component.getPackageRelativePath('', 'source'), path);
pipelinePromises.push(this.pipeline(new Readable().wrap(zipObj.nodeStream()), fullDest));
}
}

await Promise.all(pipelinePromises);
}
if (!xml) {
throw messages.createError('error_parsing_xml', [component.fullName, component.type.name]);
Expand Down Expand Up @@ -233,10 +233,10 @@ const componentIsExpandedArchive = async (component: SourceComponent): Promise<b
return false;
};

/** wrapper around the Open command so we can emit a nicer error for bad zip files */
async function openZipFile(component: SourceComponent, content: string): Promise<CentralDirectory> {
async function getStaticResourceZip(component: SourceComponent, content: string): Promise<JSZip> {
try {
return await Open.buffer(await component.tree.readFile(content));
const staticResourceZip = await component.tree.readFile(content);
return await JSZip.loadAsync(staticResourceZip, { createFolders: true });
} catch (e) {
throw new SfError(`Unable to open zip file ${content} for ${component.name} (${component.xml})`, 'BadZipFile', [
'Check that your file really is a valid zip archive',
Expand Down
121 changes: 55 additions & 66 deletions src/resolve/treeContainers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { join, dirname, basename, normalize, sep } from 'path';
import { Readable } from 'stream';
import { statSync, existsSync, readdirSync, createReadStream, readFileSync } from 'graceful-fs';
import * as unzipper from 'unzipper';
import * as JSZip from 'jszip';
import { Messages, SfError } from '@salesforce/core';
import { baseName, parseMetadataXml } from '../utils';
import { SourcePath } from '../common';
Expand Down Expand Up @@ -120,66 +120,55 @@ export class NodeFSTreeContainer extends TreeContainer {
}
}

interface ZipEntry {
path: string;
stream?: () => unzipper.Entry;
buffer?: () => Promise<Buffer>;
}

/**
* A {@link TreeContainer} that utilizes the central directory of a zip file
* to perform I/O without unzipping it to the disk first.
* A {@link TreeContainer} that performs I/O without unzipping it to the disk first.
*/
export class ZipTreeContainer extends TreeContainer {
private tree = new Map<SourcePath, ZipEntry[] | ZipEntry>();
private zip: JSZip;

private constructor(directory: unzipper.CentralDirectory) {
private constructor(zip: JSZip) {
super();
this.populate(directory);
this.zip = zip;
}

/**
* Creates a `ZipTreeContainer` from a Buffer of a zip file.
*
* @param buffer - Buffer of the zip file
* @returns A Promise of a `ZipTreeContainer`
*/
public static async create(buffer: Buffer): Promise<ZipTreeContainer> {
const directory = await unzipper.Open.buffer(buffer);
return new ZipTreeContainer(directory);
const zip = await JSZip.loadAsync(buffer, { createFolders: true });
return new ZipTreeContainer(zip);
}

public exists(fsPath: string): boolean {
return this.tree.has(fsPath);
return !!this.match(fsPath);
}

public isDirectory(fsPath: string): boolean {
if (this.exists(fsPath)) {
return Array.isArray(this.tree.get(fsPath));
const resolvedPath = this.match(fsPath);
if (resolvedPath) {
return this.ensureDirectory(resolvedPath);
}
throw new SfError(messages.getMessage('error_path_not_found', [fsPath]), 'LibraryError');
}

public readDirectory(fsPath: string): string[] {
if (this.isDirectory(fsPath)) {
return (this.tree.get(fsPath) as ZipEntry[]).map((entry) => basename(entry.path));
const resolvedPath = this.match(fsPath);
if (resolvedPath && this.ensureDirectory(resolvedPath)) {
// Remove trailing path sep if it exists. JSZip always adds them for directories but
// when comparing we call `dirname()` which does not include them.
const dirPath = resolvedPath.endsWith('/') ? resolvedPath.slice(0, -1) : resolvedPath;
return Object.keys(this.zip.files)
.filter((filePath) => dirname(filePath) === dirPath)
.map((filePath) => basename(filePath));
}
throw new SfError(messages.getMessage('error_expected_directory_path', [fsPath]), 'LibraryError');
}

public readFile(fsPath: string): Promise<Buffer> {
if (!this.isDirectory(fsPath)) {
const matchingFile = this.tree.get(fsPath);
if (!matchingFile) {
throw new SfError(messages.getMessage('error_path_not_found', [matchingFile]), 'LibraryError');
}
if (Array.isArray(matchingFile)) {
throw messages.createError('tooManyFiles', [fsPath]);
public async readFile(fsPath: string): Promise<Buffer> {
const resolvedPath = this.match(fsPath);
if (resolvedPath) {
const jsZipObj = this.zip.file(resolvedPath);
if (jsZipObj?.dir === false) {
return jsZipObj.async('nodebuffer');
}
if (matchingFile.buffer) {
return matchingFile.buffer();
}
throw new SfError(`The file at path ${fsPath} does not have a buffer method.`);
throw new SfError(`Expected a file at path ${fsPath} but found a directory.`);
}
throw new SfError(messages.getMessage('error_expected_file_path', [fsPath]), 'LibraryError');
}
Expand All @@ -190,43 +179,43 @@ export class ZipTreeContainer extends TreeContainer {
}

public stream(fsPath: string): Readable {
if (!this.isDirectory(fsPath)) {
const matchingFile = this.tree.get(fsPath);
if (!matchingFile) {
throw new SfError(messages.getMessage('error_path_not_found', [fsPath]), 'LibraryError');
}
if (Array.isArray(matchingFile)) {
throw messages.createError('tooManyFiles', [fsPath]);
const resolvedPath = this.match(fsPath);
if (resolvedPath) {
const jsZipObj = this.zip.file(resolvedPath);
if (jsZipObj && !jsZipObj.dir) {
return new Readable().wrap(jsZipObj.nodeStream());
}
if (matchingFile.stream) {
return matchingFile.stream();
}
throw new SfError(`The file at path ${fsPath} does not have a stream method.`);
throw new SfError(messages.getMessage('error_no_directory_stream', [this.constructor.name]), 'LibraryError');
}
throw new SfError(messages.getMessage('error_no_directory_stream', [this.constructor.name]), 'LibraryError');
throw new SfError(messages.getMessage('error_expected_file_path', [fsPath]), 'LibraryError');
}

private populate(directory: unzipper.CentralDirectory): void {
for (const { path, type, stream, buffer } of directory.files) {
if (type === 'File') {
// normalize path to use OS separator since zip entries always use forward slash
const entry = { path: normalize(path), stream, buffer };
this.tree.set(entry.path, entry);
this.ensureDirPathExists(entry);
}
// Finds a matching entry in the zip by first comparing basenames, then dirnames.
// Note that zip files always use forward slash separators, so paths within the
// zip files are normalized for the OS file system before comparing.
private match(fsPath: string): string | undefined {
// "dot" has a special meaning as a directory name and always matches. Just return it.
if (fsPath === '.') {
return fsPath;
}

const fsPathBasename = basename(fsPath);
const fsPathDirname = dirname(fsPath);
return Object.keys(this.zip.files).find((filePath) => {
const normFilePath = normalize(filePath);
if (basename(normFilePath) === fsPathBasename) {
return dirname(normFilePath) === fsPathDirname;
}
});
}

private ensureDirPathExists(entry: ZipEntry): void {
const dirPath = dirname(entry.path);
if (dirPath === entry.path) {
return;
} else if (!this.exists(dirPath)) {
this.tree.set(dirPath, [entry]);
this.ensureDirPathExists({ path: dirPath });
} else {
(this.tree.get(dirPath) as ZipEntry[]).push(entry);
private ensureDirectory(dirPath: string): boolean {
if (dirPath) {
// JSZip can have directory entries or only file entries (with virtual directory entries)
const zipObj = this.zip.file(dirPath);
return zipObj?.dir === true || !zipObj;
}
throw new SfError(messages.getMessage('error_path_not_found', [dirPath]), 'LibraryError');
}
}

Expand Down
41 changes: 29 additions & 12 deletions test/client/metadataApiRetrieve.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { Messages } from '@salesforce/core';
import { assert, expect } from 'chai';
import chai = require('chai');
import deepEqualInAnyOrder = require('deep-equal-in-any-order');
import * as unzipper from 'unzipper';
import { SinonStub } from 'sinon';
import { getString } from '@salesforce/ts-types';
import * as fs from 'graceful-fs';
Expand Down Expand Up @@ -441,20 +440,30 @@ describe('MetadataApiRetrieve', () => {
describe('post', () => {
const output = join('mdapi', 'retrieve', 'dir');
const format = 'metadata';
const zipFile = 'abcd1234';
// This is what real zip file contents look like from the server
const zipFile = `UEsDBBQACAgIACKPFlcAAAAAAAAAAAAAAAAiAAAAdW5wYWNrYWdlZC9jbGFzc2VzL1BhZ2VkUmVzdWx
0LmNsc53MsQoCMQwG4P2eIu/hoojDLSo6ikPahl6l15Ym4cDj3t2qm5Oa4efnhy9FTQwWpiAD8IA1JA82IjMc0ZM7EWsUmDtot95
oxV1CE8m9hvLGfRLyVKE0cQ53ghk8yQr4GUv3td3raFr9Q0sWjL3QuM2a5JcPB3MjK5crVLK5Ov6wywNQSwcIZ6ozvYIAAAAgAQA
AUEsDBBQACAgIACKPFlcAAAAAAAAAAAAAAAArAAAAdW5wYWNrYWdlZC9jbGFzc2VzL1BhZ2VkUmVzdWx0LmNscy1tZXRhLnhtbE2
NQQrCMBBF9zlFyN5MFJUiaUoRPIG6H9KogTYJnbH0+BYq4t+9z4Nnm3no5RRGijnVaquNkiH53MX0rNXtetlUqnHCtiXM5x6J5OI
nqtWLuZwAKGPR9MijD9rnAXbGHMHsYQiMHTIqJ+QyiyXe14g7VNpY+DtWgxj5Ta71HKdg4YvCwi/txAdQSwcIwX3rpIgAAACuAAA
AUEsDBBQACAgIACKPFlcAAAAAAAAAAAAAAAAWAAAAdW5wYWNrYWdlZC9wYWNrYWdlLnhtbE2OywrCMBBF9/2KkL2ZKCpF0hQRXBf
RD4jpWIvNgyZK/XtDa9FZzRkuZ64oB9ORF/ahdbagS8YpQatd3dqmoJfzcZHTUmaiUvqhGiQpbUNB7zH6HUBwyrNwc71Gpp2BFed
b4GswGFWtoqIyI2lEfHsM0z6yQXNNL2WVlPUJw7OLAubjL2aVQbn3OBw6FYKAkScj/CnFt77c5IwLmCkT8G0tsw9QSwcIAYbHtaM
AAADnAAAAUEsBAhQAFAAICAgAIo8WV2eqM72CAAAAIAEAACIAAAAAAAAAAAAAAAAAAAAAAHVucGFja2FnZWQvY2xhc3Nlcy9QYWd
lZFJlc3VsdC5jbHNQSwECFAAUAAgICAAijxZXwX3rpIgAAACuAAAAKwAAAAAAAAAAAAAAAADSAAAAdW5wYWNrYWdlZC9jbGFzc2V
zL1BhZ2VkUmVzdWx0LmNscy1tZXRhLnhtbFBLAQIUABQACAgIACKPFlcBhse1owAAAOcAAAAWAAAAAAAAAAAAAAAAALMBAAB1bnB
hY2thZ2VkL3BhY2thZ2UueG1sUEsFBgAAAAADAAMA7QAAAJoCAAAAAA==`;
const zipFileContents = Buffer.from(zipFile, 'base64');
const usernameOrConnection = '[email protected]';
const fakeResults = { status: RequestStatus.Succeeded, zipFile } as MetadataApiRetrieveStatus;
let writeFileStub: SinonStub;
let openBufferStub: SinonStub;
let extractStub: SinonStub;
let mkdirStub: SinonStub;
const mdapiRetrieveExtractStub = $$.SANDBOX.stub().resolves({});

beforeEach(() => {
writeFileStub = $$.SANDBOX.stub(fs, 'writeFileSync');
extractStub = $$.SANDBOX.stub().resolves();
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-argument
openBufferStub = $$.SANDBOX.stub(unzipper.Open, 'buffer').resolves({ extract: extractStub } as any);
mkdirStub = $$.SANDBOX.stub(fs, 'mkdirSync');
});

it('should write the retrieved zip when format=metadata', async () => {
Expand All @@ -466,7 +475,7 @@ describe('MetadataApiRetrieve', () => {
expect(writeFileStub.calledOnce).to.be.true;
expect(writeFileStub.firstCall.args[0]).to.equal(join(output, 'unpackaged.zip'));
expect(writeFileStub.firstCall.args[1]).to.deep.equal(zipFileContents);
expect(openBufferStub.called).to.be.false;
expect(mkdirStub.called).to.be.false;
expect(mdapiRetrieveExtractStub.called).to.be.false;
});

Expand All @@ -476,11 +485,19 @@ describe('MetadataApiRetrieve', () => {
mdapiRetrieve.extract = mdapiRetrieveExtractStub;
await mdapiRetrieve.post(fakeResults);

expect(writeFileStub.calledOnce).to.be.true;
const unpkg1Dir = join(output, 'unpackaged');
const unpkg2Dir = join(unpkg1Dir, 'unpackaged/');
const classesDir = join(unpkg2Dir, 'classes/');

expect(writeFileStub.called).to.be.true;
expect(writeFileStub.firstCall.args[0]).to.equal(join(output, 'unpackaged.zip'));
expect(writeFileStub.firstCall.args[1]).to.deep.equal(zipFileContents);
expect(openBufferStub.called).to.be.true;
expect(extractStub.called).to.be.true;
expect(writeFileStub.secondCall.args[0]).to.equal(join(classesDir, 'PagedResult.cls'));
expect(writeFileStub.thirdCall.args[0]).to.equal(join(classesDir, 'PagedResult.cls-meta.xml'));
expect(mkdirStub.called).to.be.true;
expect(mkdirStub.firstCall.args[0]).to.equal(unpkg1Dir);
expect(mkdirStub.secondCall.args[0]).to.equal(unpkg2Dir);
expect(mkdirStub.thirdCall.args[0]).to.equal(classesDir);
expect(mdapiRetrieveExtractStub.called).to.be.false;
});

Expand All @@ -494,7 +511,7 @@ describe('MetadataApiRetrieve', () => {
expect(writeFileStub.calledOnce).to.be.true;
expect(writeFileStub.firstCall.args[0]).to.equal(join(output, zipFileName));
expect(writeFileStub.firstCall.args[1]).to.deep.equal(zipFileContents);
expect(openBufferStub.called).to.be.false;
expect(mkdirStub.called).to.be.false;
expect(mdapiRetrieveExtractStub.called).to.be.false;
});
});
Expand Down
Loading

0 comments on commit 6bc0c12

Please sign in to comment.