paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 40 Packages Projects Releases Wiki Activity

backend: URL Reader readTree should not expect the extracted directory name

Browse Source 
If readTree can work without it receiving the top level directory name, it will be a relief. We won't have to deduce the directory name from the API responses or by any other means.
This commit is contained in:
Himanshu Mishra
2021-01-24 16:54:01 +01:00
parent c3ea694e84
commit f5ed88501b
9 changed files with 72 additions and 136 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/backend-common/src/reading/BitbucketUrlReader.ts
+1 -23
View File
@@ -121,31 +121,9 @@ export class BitbucketUrlReader implements UrlReader {
throw new Error(message);
}
// Get the filename of archive from the header of the response
const contentDispositionHeader = archiveBitbucketResponse.headers.get(
'content-disposition',
) as string;
if (!contentDispositionHeader) {
throw new Error(
`Failed to read tree from ${url}. ` +
'Bitbucket API response for downloading archive does not contain content-disposition header ',
);
}
const fileNameRegEx = new RegExp(
/^attachment; filename=(?<fileName>.*).zip$/,
);
const archiveFileName = contentDispositionHeader.match(fileNameRegEx)
?.groups?.fileName;
if (!archiveFileName) {
throw new Error(
`Failed to read tree from ${url}. Bitbucket API response for downloading archive has an unexpected ` +
`format of content-disposition header ${contentDispositionHeader} `,
);
}
return await this.treeResponseFactory.fromZipArchive({
stream: (archiveBitbucketResponse.body as unknown) as Readable,
path: `${archiveFileName}/${filepath}`,
subpath: filepath,
etag: lastCommitShortHash,
filter: options?.filter,
});
packages/backend-common/src/reading/GithubUrlReader.ts
+1 -27
View File
@@ -166,37 +166,11 @@ export class GithubUrlReader implements UrlReader {
throw new Error(message);
}
// Get the filename of archive from the header of the response
const contentDispositionHeader = archive.headers.get(
'content-disposition',
) as string;
if (!contentDispositionHeader) {
throw new Error(
`Failed to read tree from ${url}. ` +
'GitHub API response for downloading archive does not contain content-disposition header ',
);
}
const fileNameRegEx = new RegExp(
/^attachment; filename=(?<fileName>.*).tar.gz$/,
);
const archiveFileName = contentDispositionHeader.match(fileNameRegEx)
?.groups?.fileName;
if (!archiveFileName) {
throw new Error(
`Failed to read tree from ${url}. GitHub API response for downloading archive has an unexpected ` +
`format of content-disposition header ${contentDispositionHeader} `,
);
}
// The path includes the name of the directory inside the tarball and a sub path
// if requested in readTree.
const path = `${archiveFileName}/${filepath}`;
return await this.deps.treeResponseFactory.fromTarArchive({
// TODO(Rugvip): Underlying implementation of fetch will be node-fetch, we probably want
// to stick to using that in exclusively backend code.
stream: (archive.body as unknown) as Readable,
path,
subpath: filepath,
etag: commitSha,
filter: options?.filter,
});
packages/backend-common/src/reading/GitlabUrlReader.ts
+1 -25
View File
@@ -140,33 +140,9 @@ export class GitlabUrlReader implements UrlReader {
throw new Error(message);
}
// Get the filename of archive from the header of the response
const contentDispositionHeader = archiveGitLabResponse.headers.get(
'content-disposition',
) as string;
if (!contentDispositionHeader) {
throw new Error(
`Failed to read tree from ${url}. ` +
'GitLab API response for downloading archive does not contain content-disposition header ',
);
}
const fileNameRegEx = new RegExp(
/^attachment; filename="(?<fileName>.*).zip"$/,
);
const archiveFileName = contentDispositionHeader.match(fileNameRegEx)
?.groups?.fileName;
if (!archiveFileName) {
throw new Error(
`Failed to read tree from ${url}. GitLab API response for downloading archive has an unexpected ` +
`format of content-disposition header ${contentDispositionHeader} `,
);
}
const path = filepath ? `${archiveFileName}/${filepath}/` : '';
return await this.treeResponseFactory.fromZipArchive({
stream: (archiveGitLabResponse.body as unknown) as Readable,
path,
subpath: filepath,
etag: commitSha,
filter: options?.filter,
});
packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts
+5 -4
View File
@@ -24,8 +24,9 @@ import { ZipArchiveResponse } from './ZipArchiveResponse';
type FromArchiveOptions = {
// A binary stream of a tar archive.
stream: Readable;
// If set, the root of the tree will be set to the given directory path.
path?: string;
// If unset, the files at the root of the tree will be read.
// subpath must not contain the name of the top level directory.
subpath?: string;
// etag of the blob
etag: string;
// Filter passed on from the ReadTreeOptions
@@ -45,7 +46,7 @@ export class ReadTreeResponseFactory {
async fromTarArchive(options: FromArchiveOptions): Promise<ReadTreeResponse> {
return new TarArchiveResponse(
options.stream,
options.path ?? '',
options.subpath ?? '',
this.workDir,
options.etag,
options.filter,
@@ -55,7 +56,7 @@ export class ReadTreeResponseFactory {
async fromZipArchive(options: FromArchiveOptions): Promise<ReadTreeResponse> {
return new ZipArchiveResponse(
options.stream,
options.path ?? '',
options.subpath ?? '',
this.workDir,
options.etag,
options.filter,
packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts
+9 -23
View File
@@ -38,7 +38,7 @@ describe('TarArchiveResponse', () => {
it('should read files', async () => {
const stream = fs.createReadStream('/test-archive.tar.gz');
const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp', 'etag');
const res = new TarArchiveResponse(stream, '', '/tmp', 'etag');
const files = await res.files();
expect(files).toEqual([
@@ -61,12 +61,8 @@ describe('TarArchiveResponse', () => {
it('should read files with filter', async () => {
const stream = fs.createReadStream('/test-archive.tar.gz');
const res = new TarArchiveResponse(
stream,
'mock-main/',
'/tmp',
'etag',
path => path.endsWith('.yml'),
const res = new TarArchiveResponse(stream, '', '/tmp', 'etag', path =>
path.endsWith('.yml'),
);
const files = await res.files();
@@ -83,7 +79,7 @@ describe('TarArchiveResponse', () => {
it('should read as archive and files', async () => {
const stream = fs.createReadStream('/test-archive.tar.gz');
const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp', 'etag');
const res = new TarArchiveResponse(stream, '', '/tmp', 'etag');
const buffer = await res.archive();
await expect(res.archive()).rejects.toThrow(
@@ -115,24 +111,18 @@ describe('TarArchiveResponse', () => {
const res = new TarArchiveResponse(stream, '', '/tmp', 'etag');
const dir = await res.dir();
await expect(
fs.readFile(resolvePath(dir, 'mock-main/mkdocs.yml'), 'utf8'),
fs.readFile(resolvePath(dir, 'mkdocs.yml'), 'utf8'),
).resolves.toBe('site_name: Test\n');
await expect(
fs.readFile(resolvePath(dir, 'mock-main/docs/index.md'), 'utf8'),
fs.readFile(resolvePath(dir, 'docs/index.md'), 'utf8'),
).resolves.toBe('# Test\n');
});
it('should extract archive into directory with a subpath', async () => {
const stream = fs.createReadStream('/test-archive.tar.gz');
const res = new TarArchiveResponse(
stream,
'mock-main/docs/',
'/tmp',
'etag',
);
const res = new TarArchiveResponse(stream, 'docs', '/tmp', 'etag');
const dir = await res.dir();
expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/);
@@ -144,12 +134,8 @@ describe('TarArchiveResponse', () => {
it('should extract archive into directory with a subpath and filter', async () => {
const stream = fs.createReadStream('/test-archive.tar.gz');
const res = new TarArchiveResponse(
stream,
'mock-main/',
'/tmp',
'etag',
path => path.endsWith('.yml'),
const res = new TarArchiveResponse(stream, '', '/tmp', 'etag', path =>
path.endsWith('.yml'),
);
const dir = await res.dir({ targetDir: '/tmp' });
packages/backend-common/src/reading/tree/TarArchiveResponse.ts
+21 -5
View File
@@ -30,6 +30,10 @@ import {
const TarParseStream = (Parse as unknown) as { new (): ParseStream };
const pipeline = promisify(pipelineCb);
// Matches a directory name + one `/` at the start of any string,
// containing any character except `/` one or more times, and ending with a `/`
// e.g. Will match `dirA/` in `dirA/dirB/file.ext`
const directoryNameRegex = /^[^\/]+\//;
/**
* Wraps a tar archive stream into a tree response reader.
@@ -78,14 +82,18 @@ export class TarArchiveResponse implements ReadTreeResponse {
return;
}
// File path relative to the root extracted directory. Will remove the
// top level dir name from the path since its name is hard to predetermine.
const relativePath = entry.path.replace(directoryNameRegex, '');
if (this.subPath) {
if (!entry.path.startsWith(this.subPath)) {
if (!relativePath.startsWith(this.subPath)) {
entry.resume();
return;
}
}
const path = entry.path.slice(this.subPath.length);
const path = relativePath.slice(this.subPath.length);
if (this.filter) {
if (!this.filter(path)) {
entry.resume();
@@ -97,7 +105,10 @@ export class TarArchiveResponse implements ReadTreeResponse {
await pipeline(entry, concatStream(resolve));
});
files.push({ path, content: () => content });
files.push({
path,
content: () => content,
});
entry.resume();
});
@@ -138,7 +149,9 @@ export class TarArchiveResponse implements ReadTreeResponse {
options?.targetDir ??
(await fs.mkdtemp(path.join(this.workDir, 'backstage-')));
const strip = this.subPath ? this.subPath.split('/').length - 1 : 0;
// Equivalent of tar --strip-components=N
// When no subPath is given, remove just 1 top level directory
const strip = this.subPath ? this.subPath.split('/').length : 1;
await pipeline(
this.stream,
@@ -146,7 +159,10 @@ export class TarArchiveResponse implements ReadTreeResponse {
strip,
cwd: dir,
filter: path => {
if (this.subPath && !path.startsWith(this.subPath)) {
// File path relative to the root extracted directory. Will remove the
// top level dir name from the path since its name is hard to predetermine.
const relativePath = path.replace(directoryNameRegex, '');
if (this.subPath && !relativePath.startsWith(this.subPath)) {
return false;
}
if (this.filter) {
packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts
+9 -22
View File
@@ -38,7 +38,7 @@ describe('ZipArchiveResponse', () => {
it('should read files', async () => {
const stream = fs.createReadStream('/test-archive.zip');
const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', 'etag');
const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag');
const files = await res.files();
expect(files).toEqual([
@@ -61,12 +61,8 @@ describe('ZipArchiveResponse', () => {
it('should read files with filter', async () => {
const stream = fs.createReadStream('/test-archive.zip');
const res = new ZipArchiveResponse(
stream,
'mock-main/',
'/tmp',
'etag',
path => path.endsWith('.yml'),
const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag', path =>
path.endsWith('.yml'),
);
const files = await res.files();
@@ -83,7 +79,7 @@ describe('ZipArchiveResponse', () => {
it('should read as archive and files', async () => {
const stream = fs.createReadStream('/test-archive.zip');
const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', 'etag');
const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag');
const buffer = await res.archive();
await expect(res.archive()).rejects.toThrow(
@@ -117,22 +113,17 @@ describe('ZipArchiveResponse', () => {
const dir = await res.dir();
await expect(
fs.readFile(resolvePath(dir, 'mock-main/mkdocs.yml'), 'utf8'),
fs.readFile(resolvePath(dir, 'mkdocs.yml'), 'utf8'),
).resolves.toBe('site_name: Test\n');
await expect(
fs.readFile(resolvePath(dir, 'mock-main/docs/index.md'), 'utf8'),
fs.readFile(resolvePath(dir, 'docs/index.md'), 'utf8'),
).resolves.toBe('# Test\n');
});
it('should extract archive into directory with a subpath', async () => {
const stream = fs.createReadStream('/test-archive.zip');
const res = new ZipArchiveResponse(
stream,
'mock-main/docs/',
'/tmp',
'etag',
);
const res = new ZipArchiveResponse(stream, 'docs/', '/tmp', 'etag');
const dir = await res.dir();
expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/);
@@ -144,12 +135,8 @@ describe('ZipArchiveResponse', () => {
it('should extract archive into directory with a subpath and filter', async () => {
const stream = fs.createReadStream('/test-archive.zip');
const res = new ZipArchiveResponse(
stream,
'mock-main/',
'/tmp',
'etag',
path => path.endsWith('.yml'),
const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag', path =>
path.endsWith('.yml'),
);
const dir = await res.dir({ targetDir: '/tmp' });
packages/backend-common/src/reading/tree/ZipArchiveResponse.ts
+22 -7
View File
@@ -25,6 +25,11 @@ import {
ReadTreeResponseDirOptions,
} from '../types';
// Matches a directory name + one `/` at the start of any string,
// containing any character except / one or more times, and ending with a `/`
// e.g. Will match `dirA/` in `dirA/dirB/file.ext`
const directoryNameRegex = /^[^\/]+\//;
/**
* Wraps a zip archive stream into a tree response reader.
*/
@@ -60,18 +65,26 @@ export class ZipArchiveResponse implements ReadTreeResponse {
this.read = true;
}
private getPath(entry: Entry): string {
return entry.path.slice(this.subPath.length);
// Will remove the top level dir name from the path since its name is hard to predetermine.
private stripTopDirectory(path: string): string {
return path.replace(directoryNameRegex, '');
}
// File path relative to the root extracted directory or a sub directory if subpath is set.
private getInnerPath(path: string): string {
return path.slice(this.subPath.length);
}
private shouldBeIncluded(entry: Entry): boolean {
const strippedPath = this.stripTopDirectory(entry.path);
if (this.subPath) {
if (!entry.path.startsWith(this.subPath)) {
if (!strippedPath.startsWith(this.subPath)) {
return false;
}
}
if (this.filter) {
return this.filter(this.getPath(entry));
return this.filter(this.getInnerPath(entry.path));
}
return true;
}
@@ -91,7 +104,7 @@ export class ZipArchiveResponse implements ReadTreeResponse {
if (this.shouldBeIncluded(entry)) {
files.push({
path: this.getPath(entry),
path: this.getInnerPath(this.stripTopDirectory(entry.path)),
content: () => entry.buffer(),
});
} else {
@@ -115,7 +128,7 @@ export class ZipArchiveResponse implements ReadTreeResponse {
.pipe(unzipper.Parse())
.on('entry', (entry: Entry) => {
if (entry.type === 'File' && this.shouldBeIncluded(entry)) {
archive.append(entry, { name: this.getPath(entry) });
archive.append(entry, { name: this.getInnerPath(entry.path) });
} else {
entry.autodrain();
}
@@ -139,7 +152,9 @@ export class ZipArchiveResponse implements ReadTreeResponse {
// Ignore directory entries since we handle that with the file entries
// as a zip can have files with directories without directory entries
if (entry.type === 'File' && this.shouldBeIncluded(entry)) {
const entryPath = this.getPath(entry);
const entryPath = this.getInnerPath(
this.stripTopDirectory(entry.path),
);
const dirname = path.dirname(entryPath);
if (dirname) {
await fs.mkdirp(path.join(dir, dirname));
packages/backend-common/src/reading/types.ts
+3
View File
@@ -81,6 +81,9 @@ export type ReadTreeResponseDirOptions = {
};
export type ReadTreeResponse = {
/**
* files() returns an array of all the files inside the tree and corresponding functions to read their content.
*/
files(): Promise<ReadTreeResponseFile[]>;
archive(): Promise<NodeJS.ReadableStream>;