Skip to content

Commit

Permalink
feat: swap archiver for jszip
Browse files Browse the repository at this point in the history
  • Loading branch information
shetzel committed May 16, 2023
1 parent 5fda987 commit 630364a
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 44 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"got": "^11.8.6",
"graceful-fs": "^4.2.11",
"ignore": "^5.2.4",
"jszip": "^3.10.1",
"mime": "2.6.0",
"minimatch": "^5.1.6",
"proxy-agent": "^5.0.0",
Expand Down
47 changes: 31 additions & 16 deletions src/client/metadataApiDeploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
* 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, dirname, extname, join, posix, sep } from 'path';
import { basename, dirname, extname, join, posix, relative, resolve, sep } from 'path';
import { format } from 'util';
import { isString } from '@salesforce/ts-types';
import { create as createArchive } from 'archiver';
import * as JSZip from 'jszip';
import * as fs from 'graceful-fs';
import { Lifecycle, Messages, SfError } from '@salesforce/core';
import { ensureArray } from '@salesforce/kit';
Expand All @@ -16,7 +16,6 @@ import { MetadataConverter } from '../convert';
import { ComponentLike, SourceComponent } from '../resolve';
import { ComponentSet } from '../collections';
import { registry } from '../registry';
import { stream2buffer } from '../convert/streams';
import { MetadataTransfer, MetadataTransferOptions } from './metadataTransfer';
import {
AsyncResult,
Expand Down Expand Up @@ -437,20 +436,36 @@ export class MetadataApiDeploy extends MetadataTransfer<
}

private async getZipBuffer(): Promise<Buffer> {
if (this.options.mdapiPath) {
if (!fs.existsSync(this.options.mdapiPath) || !fs.lstatSync(this.options.mdapiPath).isDirectory()) {
throw messages.createError('error_directory_not_found_or_not_directory', [this.options.mdapiPath]);
const mdapiPath = this.options.mdapiPath;
if (mdapiPath) {
if (!fs.existsSync(mdapiPath) || !fs.lstatSync(mdapiPath).isDirectory()) {
throw messages.createError('error_directory_not_found_or_not_directory', [mdapiPath]);
}
// make a zip from the given directory
const zip = createArchive('zip', { zlib: { level: 9 } });
// anywhere not at the root level is fine
zip.directory(this.options.mdapiPath, 'zip');
// archiver/zip.finalize looks like it is async, because it extends streams, but it is not meant to be used that way
// the typings on it are misleading and unintended. More info https://github.com/archiverjs/node-archiver/issues/476
// If you await it, bad things happen, like the convert process exiting silently. https://github.com/forcedotcom/cli/issues/1791
// leave the void as it is
void zip.finalize();
return stream2buffer(zip);

const zip = JSZip();

const zipDirRecursive = (dir: string) => {
const list = fs.readdirSync(dir);
for (const file of list) {
const fullPath = resolve(dir, file);
const stat = fs.statSync(fullPath);
if (stat.isDirectory()) {
zipDirRecursive(fullPath);
} else {
// Add relative file paths to a root of "zip" for MDAPI.
const relPath = join('zip', relative(mdapiPath, fullPath));
zip.file(relPath, fs.createReadStream(fullPath));
}
}
};
console.log('Zipping directory for metadata deploy:', mdapiPath);
zipDirRecursive(mdapiPath);

return zip.generateAsync({
type: "nodebuffer",
compression: 'DEFLATE',
compressionOptions: { level: 9 },
});
}
// read the zip into a buffer
if (this.options.zipPath) {
Expand Down
1 change: 1 addition & 0 deletions src/convert/metadataConverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export class MetadataConverter {
mergeSet,
tasks = [],
} = await getConvertIngredients(output, cs, targetFormatIsSource);
console.log('convert to path:', packagePath);

const conversionPipeline = pipeline(
Readable.from(components),
Expand Down
25 changes: 20 additions & 5 deletions src/convert/streams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import { basename, dirname, isAbsolute, join } from 'path';
import { pipeline as cbPipeline, Readable, Stream, Transform, Writable } from 'stream';
import { promisify } from 'util';
import { Messages, SfError } from '@salesforce/core';
import { Archiver, create as createArchive } from 'archiver';
// import { Archiver, create as createArchive } from 'archiver';
import * as JSZip from 'jszip';
import { createWriteStream, existsSync } from 'graceful-fs';
import { JsonMap } from '@salesforce/ts-types';
import { XMLBuilder } from 'fast-xml-parser';
Expand Down Expand Up @@ -186,12 +187,23 @@ export class ZipWriter extends ComponentWriter {
// compression-/speed+ (0)<---(3)---------->(9) compression+/speed-
// 3 appears to be a decent balance of compression and speed. It felt like
// higher values = diminishing returns on compression and made conversion slower
private zip: Archiver = createArchive('zip', { zlib: { level: 3 } });
// private zip: Archiver = createArchive('zip', { zlib: { level: 3 } });
private zip = JSZip();
private buffers: Buffer[] = [];

public constructor(rootDestination?: SourcePath) {
super(rootDestination);
void pipeline(this.zip, this.getOutputStream());
// void pipeline(this.zip, this.getOutputStream());
console.log(`generating zip for: ${rootDestination}`);
const zipStream = this.zip.generateNodeStream({
compression: 'DEFLATE',
compressionOptions: { level: 9 },
streamFiles: true,
})
.on('end', () => {
console.log(`zip complete for: ${rootDestination}`);
});
void pipeline(zipStream, this.getOutputStream());
}

public get buffer(): Buffer | undefined {
Expand Down Expand Up @@ -227,15 +239,18 @@ export class ZipWriter extends ComponentWriter {
try {
// Unlike the other 2 places where zip.finalize is called, this one DOES need to be awaited
// the e2e replacement nuts will fail otherwise
await this.zip.finalize();
// await this.zip.finalize();
console.log('now we are really done');
} catch (e) {
err = e as Error;
}
callback(err);
}

public addToZip(contents: string | Readable | Buffer, path: SourcePath): void {
this.zip.append(contents, { name: path });
// this.zip.append(contents, { name: path });
console.log('adding to zip:', path);
this.zip.file(path, contents);
}

private getOutputStream(): Writable {
Expand Down
3 changes: 3 additions & 0 deletions src/convert/transformers/baseMetadataTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,18 @@ import { MetadataTransformer, WriteInfo } from '../types';
import { ConvertContext } from '../convertContext';
import { SourceComponent } from '../../resolve';
import { RegistryAccess } from '../../registry';
import { Logger } from '@salesforce/core';

export abstract class BaseMetadataTransformer implements MetadataTransformer {
public readonly context: ConvertContext;
public defaultDirectory?: string;
protected registry: RegistryAccess;
protected logger: Logger;

public constructor(registry = new RegistryAccess(), context = new ConvertContext()) {
this.registry = registry;
this.context = context;
this.logger = Logger.childFromRoot(this.constructor.name);
}

public abstract toMetadataFormat(component: SourceComponent): Promise<WriteInfo[]>;
Expand Down
44 changes: 22 additions & 22 deletions src/convert/transformers/staticResourceMetadataTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
import { basename, dirname, isAbsolute, join, relative } from 'path';
import { Readable } from 'stream';
import { create as createArchive, Archiver } from 'archiver';
import * as JSZip from 'jszip';
import { getExtension } from 'mime';
import { CentralDirectory, Open } from 'unzipper';
import { JsonMap } from '@salesforce/ts-types';
Expand Down Expand Up @@ -41,33 +41,33 @@ export class StaticResourceMetadataTransformer extends BaseMetadataTransformer {
if (!xml) {
throw messages.createError('error_parsing_xml', [component.fullName, component.type.name]);
}
// archiver/zip.finalize looks like it is async, because it extends streams, but it is not meant to be used that way
// the typings on it are misleading and unintended. More info https://github.com/archiverjs/node-archiver/issues/476
// If you await it, bad things happen, like the convert process exiting silently. https://github.com/forcedotcom/cli/issues/1791
// leave the void as it is
// eslint-disable-next-line @typescript-eslint/require-await
const zipIt = async (): Promise<Archiver> => {
// toolbelt was using level 9 for static resources, so we'll do the same.
// Otherwise, you'll see errors like https://github.com/forcedotcom/cli/issues/1098
const zip = createArchive('zip', { zlib: { level: 9 } });
if (!component.replacements) {
// the easy way...no replacements required
zip.directory(content, false);
} else {
// the hard way--we have to walk the content and do replacements on each of the files.
for (const path of component.walkContent()) {
const replacementStream = getReplacementStreamForReadable(component, path);
zip.append(replacementStream, { name: relative(content, path) });
}

// Zip the static resource from disk to a stream, compressing at level 9.
const zipIt = (): Readable => {
this.logger.debug(`zipping static resource: ${component.content}`);
const zip = JSZip();

// JSZip does not have an API for adding a directory of files recursively so we always
// have to walk the component content. Replacements only happen if set on the component.
for (const path of component.walkContent()) {
const replacementStream = getReplacementStreamForReadable(component, path);
zip.file(relative(content, path), replacementStream);
}
void zip.finalize();
return zip;

return new Readable().wrap(zip.generateNodeStream({
compression: 'DEFLATE',
compressionOptions: { level: 9 },
streamFiles: true,
})
.on('end', () => {
this.logger.debug(`zip complete for: ${component.content}`);
}));
};

return [
{
source: (await componentIsExpandedArchive(component))
? await zipIt()
? zipIt()
: getReplacementStreamForReadable(component, content),
output: join(type.directoryName, `${baseName(content)}.${type.suffix}`),
},
Expand Down
29 changes: 28 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3979,6 +3979,11 @@ ignore@^5.1.4, ignore@^5.2.0, ignore@^5.2.4:
resolved "https://registry.yarnpkg.com/ignore/-/ignore-5.2.4.tgz#a291c0c6178ff1b960befe47fcdec301674a6324"
integrity sha512-MAb38BcSbH0eHNBxn7ql2NH/kX33OkB3lZ1BNdh7ENeRChHTYsTvWrMubiIAMNS2llXEEgZ1MUOBtXChP3kaFQ==

immediate@~3.0.5:
version "3.0.6"
resolved "https://registry.yarnpkg.com/immediate/-/immediate-3.0.6.tgz#9db1dbd0faf8de6fbe0f5dd5e56bb606280de69b"
integrity sha512-XXOFtyqDjNDAQxVfYxuF7g9Il/IbWmmlQg2MYKOH8ExIT1qg6xc4zyS3HaEEATgs1btfzxq15ciUiY7gjSXRGQ==

import-fresh@^3.0.0, import-fresh@^3.2.1:
version "3.3.0"
resolved "https://registry.yarnpkg.com/import-fresh/-/import-fresh-3.3.0.tgz#37162c25fcb9ebaa2e6e53d5b4d88ce17d9e0c2b"
Expand Down Expand Up @@ -4604,6 +4609,16 @@ jsonwebtoken@9.0.0:
ms "^2.1.1"
semver "^7.3.8"

jszip@^3.10.1:
version "3.10.1"
resolved "https://registry.yarnpkg.com/jszip/-/jszip-3.10.1.tgz#34aee70eb18ea1faec2f589208a157d1feb091c2"
integrity sha512-xXDvecyTpGLrqFrvkrUSoxxfJI5AH7U8zxxtVclpsUtMCq4JQ290LY8AW5c7Ggnr/Y/oK+bQMbqK2qmtk3pN4g==
dependencies:
lie "~3.3.0"
pako "~1.0.2"
readable-stream "~2.3.6"
setimmediate "^1.0.5"

just-diff-apply@^5.2.0:
version "5.5.0"
resolved "https://registry.yarnpkg.com/just-diff-apply/-/just-diff-apply-5.5.0.tgz#771c2ca9fa69f3d2b54e7c3f5c1dfcbcc47f9f0f"
Expand Down Expand Up @@ -4671,6 +4686,13 @@ levn@~0.3.0:
prelude-ls "~1.1.2"
type-check "~0.3.2"

lie@~3.3.0:
version "3.3.0"
resolved "https://registry.yarnpkg.com/lie/-/lie-3.3.0.tgz#dcf82dee545f46074daf200c7c1c5a08e0f40f6a"
integrity sha512-UaiMJzeWRlEujzAuw5LokY1L5ecNQYZKfmyZ9L7wDHb/p5etKaxXhohBcrw0EYby+G/NA52vRSN4N39dxHAIwQ==
dependencies:
immediate "~3.0.5"

lines-and-columns@^1.1.6:
version "1.2.4"
resolved "https://registry.yarnpkg.com/lines-and-columns/-/lines-and-columns-1.2.4.tgz#eca284f75d2965079309dc0ad9255abb2ebc1632"
Expand Down Expand Up @@ -5833,6 +5855,11 @@ pad-component@0.0.1:
resolved "https://registry.yarnpkg.com/pad-component/-/pad-component-0.0.1.tgz#ad1f22ce1bf0fdc0d6ddd908af17f351a404b8ac"
integrity sha512-8EKVBxCRSvLnsX1p2LlSFSH3c2/wuhY9/BXXWu8boL78FbVKqn2L5SpURt1x5iw6Gq8PTqJ7MdPoe5nCtX3I+g==

pako@~1.0.2:
version "1.0.11"
resolved "https://registry.yarnpkg.com/pako/-/pako-1.0.11.tgz#6c9599d340d54dfd3946380252a35705a6b992bf"
integrity sha512-4hLB8Py4zZce5s4yd9XzopqwVv/yGNhV1Bl8NTmCq1763HeK2+EwVTv+leGeL13Dnh2wfbqowVPXCIO0z4taYw==

param-case@^3.0.4:
version "3.0.4"
resolved "https://registry.yarnpkg.com/param-case/-/param-case-3.0.4.tgz#7d17fe4aa12bde34d4a77d91acfb6219caad01c5"
Expand Down Expand Up @@ -6509,7 +6536,7 @@ set-blocking@^2.0.0:
resolved "https://registry.yarnpkg.com/set-blocking/-/set-blocking-2.0.0.tgz#045f9782d011ae9a6803ddd382b24392b3d890f7"
integrity sha512-KiKBS8AnWGEyLzofFfmvKwpdPzqiy16LvQfK3yv/fVH7Bj13/wl3JSR1J+rfgRE9q7xUJK4qvgS8raSOeLUehw==

setimmediate@~1.0.4:
setimmediate@^1.0.5, setimmediate@~1.0.4:
version "1.0.5"
resolved "https://registry.yarnpkg.com/setimmediate/-/setimmediate-1.0.5.tgz#290cbb232e306942d7d7ea9b83732ab7856f8285"
integrity sha512-MATJdZp8sLqDl/68LfQmbP8zKPLQNV6BIZoIgrscFDQ+RsvK/BxeDQOgyxKKoh0y/8h3BqVFnCqQ/gd+reiIXA==
Expand Down

2 comments on commit 630364a

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Benchmark suite Current: 630364a Previous: 27e8549 Ratio
eda-componentSetCreate-linux 334 ms 277 ms 1.21
eda-sourceToMdapi-linux 7712 ms 7571 ms 1.02
eda-sourceToZip-linux 4516 ms 5044 ms 0.90
eda-mdapiToSource-linux 6705 ms 5266 ms 1.27
lotsOfClasses-componentSetCreate-linux 667 ms 520 ms 1.28
lotsOfClasses-sourceToMdapi-linux 12657 ms 9879 ms 1.28
lotsOfClasses-sourceToZip-linux 6660 ms 7485 ms 0.89
lotsOfClasses-mdapiToSource-linux 8572 ms 5777 ms 1.48
lotsOfClassesOneDir-componentSetCreate-linux 1196 ms 891 ms 1.34
lotsOfClassesOneDir-sourceToMdapi-linux 19745 ms 14050 ms 1.41
lotsOfClassesOneDir-sourceToZip-linux 10114 ms 11857 ms 0.85
lotsOfClassesOneDir-mdapiToSource-linux 15187 ms 10126 ms 1.50

This comment was automatically generated by workflow using github-action-benchmark.

@svc-cli-bot
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Benchmark

Benchmark suite Current: 630364a Previous: 27e8549 Ratio
eda-componentSetCreate-win32 437 ms 394 ms 1.11
eda-sourceToMdapi-win32 8502 ms 7915 ms 1.07
eda-sourceToZip-win32 6443 ms 5704 ms 1.13
eda-mdapiToSource-win32 8708 ms 7695 ms 1.13
lotsOfClasses-componentSetCreate-win32 870 ms 858 ms 1.01
lotsOfClasses-sourceToMdapi-win32 13220 ms 10934 ms 1.21
lotsOfClasses-sourceToZip-win32 7446 ms 7467 ms 1.00
lotsOfClasses-mdapiToSource-win32 10927 ms 9259 ms 1.18
lotsOfClassesOneDir-componentSetCreate-win32 1530 ms 1495 ms 1.02
lotsOfClassesOneDir-sourceToMdapi-win32 22455 ms 18292 ms 1.23
lotsOfClassesOneDir-sourceToZip-win32 10124 ms 11790 ms 0.86
lotsOfClassesOneDir-mdapiToSource-win32 19965 ms 16653 ms 1.20

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.