Skip to content

Commit

Permalink
fix: read directory with resolved path
Browse files Browse the repository at this point in the history
  • Loading branch information
shetzel committed Oct 24, 2023
1 parent c47ad10 commit 9b41af2
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 14 deletions.
33 changes: 24 additions & 9 deletions src/resolve/treeContainers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,17 +143,17 @@ export class ZipTreeContainer extends TreeContainer {
public isDirectory(fsPath: string): boolean {
const resolvedPath = this.match(fsPath);
if (resolvedPath) {
// JSZip can have directory entries or only file entries (with virtual directory entries)
const zipObj = this.zip.file(resolvedPath);
return zipObj?.dir === true || !zipObj;
return this.ensureDirectory(resolvedPath);
}
throw new SfError(messages.getMessage('error_path_not_found', [fsPath]), 'LibraryError');
}

public readDirectory(fsPath: string): string[] {
if (this.isDirectory(fsPath)) {
// remove trailing path sep if it exists
const dirPath = fsPath.endsWith(sep) ? fsPath.slice(0, -1) : fsPath;
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));
Expand Down Expand Up @@ -194,14 +194,29 @@ export class ZipTreeContainer extends TreeContainer {
// 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((f) => {
if (normalize(basename(f)) === fsPathBasename || fsPath === '.') {
return normalize(dirname(f)) === fsPathDirname;
return Object.keys(this.zip.files).find((filePath) => {
const normFilePath = normalize(filePath);
if (basename(normFilePath) === fsPathBasename) {
return dirname(normFilePath) === fsPathDirname;
}
});
}

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
15 changes: 10 additions & 5 deletions test/resolve/treeContainers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,20 @@ describe('Tree Containers', () => {
let tree: ZipTreeContainer;
let zipBuffer: Buffer;

const filesRoot = join('.', 'main', 'default');
const moreFiles = join(filesRoot, 'morefiles');
//
// NOTE: All files in zips use a forward slash as a file separator, so we build
// the zip using paths with hard-coded forward slashes, not OS specific seps.
//

const filesRoot = 'main/default';
const moreFiles = `${filesRoot}/morefiles`;

before(async () => {
const zip = new JSZip();
zip
?.file(join(filesRoot, 'test.txt'), 'test text')
?.file(join(filesRoot, 'test2.txt'), 'test text 2')
?.file(join(moreFiles, 'test3.txt'), 'test text 3');
?.file(`${filesRoot}/test.txt`, 'test text')
?.file(`${filesRoot}/test2.txt`, 'test text 2')
?.file(`${moreFiles}/test3.txt`, 'test text 3');

zipBuffer = await zip.generateAsync({
type: 'nodebuffer',
Expand Down

2 comments on commit 9b41af2

@svc-cli-bot
Copy link
Collaborator

Choose a reason for hiding this comment

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

Benchmark

Benchmark suite Current: 9b41af2 Previous: 7c2e83c Ratio
eda-componentSetCreate-linux 446 ms 390 ms 1.14
eda-sourceToMdapi-linux 9049 ms 6388 ms 1.42
eda-sourceToZip-linux 7438 ms 6034 ms 1.23
eda-mdapiToSource-linux 6402 ms 5227 ms 1.22
lotsOfClasses-componentSetCreate-linux 906 ms 718 ms 1.26
lotsOfClasses-sourceToMdapi-linux 14395 ms 11707 ms 1.23
lotsOfClasses-sourceToZip-linux 9928 ms 9480 ms 1.05
lotsOfClasses-mdapiToSource-linux 8080 ms 5967 ms 1.35
lotsOfClassesOneDir-componentSetCreate-linux 1443 ms 1188 ms 1.21
lotsOfClassesOneDir-sourceToMdapi-linux 20109 ms 16507 ms 1.22
lotsOfClassesOneDir-sourceToZip-linux 15910 ms 16140 ms 0.99
lotsOfClassesOneDir-mdapiToSource-linux 13794 ms 11174 ms 1.23

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

@svc-cli-bot
Copy link
Collaborator

Choose a reason for hiding this comment

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

Benchmark

Benchmark suite Current: 9b41af2 Previous: 7c2e83c Ratio
eda-componentSetCreate-win32 601 ms 551 ms 1.09
eda-sourceToMdapi-win32 10376 ms 9802 ms 1.06
eda-sourceToZip-win32 8158 ms 6981 ms 1.17
eda-mdapiToSource-win32 9434 ms 9414 ms 1.00
lotsOfClasses-componentSetCreate-win32 1274 ms 1302 ms 0.98
lotsOfClasses-sourceToMdapi-win32 17604 ms 15929 ms 1.11
lotsOfClasses-sourceToZip-win32 11438 ms 10908 ms 1.05
lotsOfClasses-mdapiToSource-win32 11246 ms 11824 ms 0.95
lotsOfClassesOneDir-componentSetCreate-win32 2311 ms 2261 ms 1.02
lotsOfClassesOneDir-sourceToMdapi-win32 26570 ms 27331 ms 0.97
lotsOfClassesOneDir-sourceToZip-win32 18642 ms 18659 ms 1.00
lotsOfClassesOneDir-mdapiToSource-win32 20488 ms 20661 ms 0.99

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

Please sign in to comment.