Merge pull request #4231 from backstage/orkohunter/readTree-get-rid-of-directory-name-dependency

fix(backend): URL Reader readTree gets rid of archive directory name dependency
This commit is contained in:
Fredrik Adelöw
2021-01-28 11:41:00 +01:00
committed by GitHub
14 changed files with 224 additions and 140 deletions
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/backend-common': patch
---
URL Reader's readTree: Fix bug with github.com URLs.
@@ -14,7 +14,9 @@
* limitations under the License.
*/
import fs from 'fs';
import * as os from 'os';
import fs from 'fs-extra';
import mockFs from 'mock-fs';
import path from 'path';
import { rest } from 'msw';
import { setupServer } from 'msw/node';
@@ -31,6 +33,8 @@ const treeResponseFactory = ReadTreeResponseFactory.create({
config: new ConfigReader({}),
});
const tmpDir = os.platform() === 'win32' ? 'C:\\tmp' : '/tmp';
describe('AzureUrlReader', () => {
const worker = setupServer();
msw.setupDefaultHandlers(worker);
@@ -139,6 +143,16 @@ describe('AzureUrlReader', () => {
});
describe('readTree', () => {
beforeEach(() => {
mockFs({
[tmpDir]: mockFs.directory(),
});
});
afterEach(() => {
mockFs.restore();
});
const repoBuffer = fs.readFileSync(
path.resolve('src', 'reading', '__fixtures__', 'mock-main.zip'),
);
@@ -200,6 +214,21 @@ describe('AzureUrlReader', () => {
expect(indexMarkdownFile.toString()).toBe('# Test\n');
});
it('creates a directory with the wanted files', async () => {
const response = await processor.readTree(
'https://dev.azure.com/organization/project/_git/repository',
);
const dir = await response.dir({ targetDir: tmpDir });
await expect(
fs.readFile(path.join(dir, 'mkdocs.yml'), 'utf8'),
).resolves.toBe('site_name: Test\n');
await expect(
fs.readFile(path.join(dir, 'docs', 'index.md'), 'utf8'),
).resolves.toBe('# Test\n');
});
it('throws a NotModifiedError when given a etag in options', async () => {
const fnAzure = async () => {
await processor.readTree(
@@ -16,7 +16,8 @@
import { ConfigReader } from '@backstage/config';
import { msw } from '@backstage/test-utils';
import fs from 'fs';
import fs from 'fs-extra';
import mockFs from 'mock-fs';
import { rest } from 'msw';
import { setupServer } from 'msw/node';
import path from 'path';
@@ -53,6 +54,16 @@ describe('BitbucketUrlReader', () => {
});
describe('readTree', () => {
beforeEach(() => {
mockFs({
'/tmp': mockFs.directory(),
});
});
afterEach(() => {
mockFs.restore();
});
const worker = setupServer();
msw.setupDefaultHandlers(worker);
@@ -155,6 +166,21 @@ describe('BitbucketUrlReader', () => {
expect(mkDocsFile.toString()).toBe('site_name: Test\n');
});
it('creates a directory with the wanted files', async () => {
const response = await bitbucketProcessor.readTree(
'https://bitbucket.org/backstage/mock',
);
const dir = await response.dir({ targetDir: '/tmp' });
await expect(
fs.readFile(path.join(dir, 'mkdocs.yml'), 'utf8'),
).resolves.toBe('site_name: Test\n');
await expect(
fs.readFile(path.join(dir, 'docs', 'index.md'), 'utf8'),
).resolves.toBe('# Test\n');
});
it('uses private bitbucket host', async () => {
const response = await hostedBitbucketProcessor.readTree(
'https://bitbucket.mycompany.net/projects/backstage/repos/mock/browse/docs?at=some-branch',
@@ -185,6 +211,18 @@ describe('BitbucketUrlReader', () => {
expect(indexMarkdownFile.toString()).toBe('# Test\n');
});
it('creates a directory with the wanted files with a subpath', async () => {
const response = await bitbucketProcessor.readTree(
'https://bitbucket.org/backstage/mock/src/master/docs',
);
const dir = await response.dir({ targetDir: '/tmp' });
await expect(
fs.readFile(path.join(dir, 'index.md'), 'utf8'),
).resolves.toBe('# Test\n');
});
it('throws a NotModifiedError when given a etag in options', async () => {
const fnBitbucket = async () => {
await bitbucketProcessor.readTree(
@@ -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,
});
@@ -17,7 +17,8 @@
import { ConfigReader } from '@backstage/config';
import { GithubCredentialsProvider } from '@backstage/integration';
import { msw } from '@backstage/test-utils';
import fs from 'fs';
import fs from 'fs-extra';
import mockFs from 'mock-fs';
import { rest } from 'msw';
import { setupServer } from 'msw/node';
import path from 'path';
@@ -107,6 +108,16 @@ describe('GithubUrlReader', () => {
});
describe('readTree', () => {
beforeEach(() => {
mockFs({
'/tmp': mockFs.directory(),
});
});
afterEach(() => {
mockFs.restore();
});
const repoBuffer = fs.readFileSync(
path.resolve(
'src',
@@ -227,6 +238,21 @@ describe('GithubUrlReader', () => {
expect(indexMarkdownFile.toString()).toBe('# Test\n');
});
it('creates a directory with the wanted files', async () => {
const response = await githubProcessor.readTree(
'https://github.com/backstage/mock',
);
const dir = await response.dir({ targetDir: '/tmp' });
await expect(
fs.readFile(path.join(dir, 'mkdocs.yml'), 'utf8'),
).resolves.toBe('site_name: Test\n');
await expect(
fs.readFile(path.join(dir, 'docs', 'index.md'), 'utf8'),
).resolves.toBe('# Test\n');
});
it('should use the headers from the credentials provider to the fetch request', async () => {
expect.assertions(2);
@@ -293,6 +319,18 @@ describe('GithubUrlReader', () => {
expect(indexMarkdownFile.toString()).toBe('# Test\n');
});
it('creates a directory with the wanted files with subpath', async () => {
const response = await githubProcessor.readTree(
'https://github.com/backstage/mock/tree/main/docs',
);
const dir = await response.dir({ targetDir: '/tmp' });
await expect(
fs.readFile(path.join(dir, 'index.md'), 'utf8'),
).resolves.toBe('# Test\n');
});
it('throws a NotModifiedError when given a etag in options', async () => {
const fnGithub = async () => {
await githubProcessor.readTree('https://github.com/backstage/mock', {
@@ -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,
});
@@ -16,7 +16,8 @@
import { ConfigReader } from '@backstage/config';
import { msw } from '@backstage/test-utils';
import fs from 'fs';
import fs from 'fs-extra';
import mockFs from 'mock-fs';
import { rest } from 'msw';
import { setupServer } from 'msw/node';
import path from 'path';
@@ -153,6 +154,16 @@ describe('GitlabUrlReader', () => {
});
describe('readTree', () => {
beforeEach(() => {
mockFs({
'/tmp': mockFs.directory(),
});
});
afterEach(() => {
mockFs.restore();
});
const archiveBuffer = fs.readFileSync(
path.resolve('src', 'reading', '__fixtures__', 'gitlab-archive.zip'),
);
@@ -254,6 +265,21 @@ describe('GitlabUrlReader', () => {
expect(indexMarkdownFile.toString()).toBe('# Test\n');
});
it('creates a directory with the wanted files', async () => {
const response = await gitlabProcessor.readTree(
'https://gitlab.com/backstage/mock',
);
const dir = await response.dir({ targetDir: '/tmp' });
await expect(
fs.readFile(path.join(dir, 'mkdocs.yml'), 'utf8'),
).resolves.toBe('site_name: Test\n');
await expect(
fs.readFile(path.join(dir, 'docs', 'index.md'), 'utf8'),
).resolves.toBe('# Test\n');
});
it('returns the wanted files from hosted gitlab', async () => {
worker.use(
rest.get(
@@ -296,6 +322,18 @@ describe('GitlabUrlReader', () => {
expect(indexMarkdownFile.toString()).toBe('# Test\n');
});
it('creates a directory with the wanted files with subpath', async () => {
const response = await gitlabProcessor.readTree(
'https://gitlab.com/backstage/mock/tree/main/docs',
);
const dir = await response.dir({ targetDir: '/tmp' });
await expect(
fs.readFile(path.join(dir, 'index.md'), 'utf8'),
).resolves.toBe('# Test\n');
});
it('throws a NotModifiedError when given a etag in options', async () => {
const fnGitlab = async () => {
await gitlabProcessor.readTree('https://gitlab.com/backstage/mock', {
@@ -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,
});
@@ -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,
@@ -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' });
@@ -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) {
@@ -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' });
@@ -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));
@@ -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>;