From 2305296073b4a32ca096ceba46dd49c66173385f Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Sat, 16 Jan 2021 22:10:43 +0100 Subject: [PATCH] backend-common: Support SHA based caching in URL Reader's readTree readTree now takes a SHA in options, and throws a NotModifiedError if the SHA matches with the latest commit SHA on the tree. readTree response now contains a new 'sha' parameter. Also, the default branch will now be used if no branch is provided in the url's target --- catalog-info.yaml | 2 +- packages/backend-common/src/errors.ts | 5 + .../src/reading/AzureUrlReader.ts | 17 +- .../src/reading/BitbucketUrlReader.ts | 17 +- .../src/reading/GithubUrlReader.test.ts | 272 +++++++++++------- .../src/reading/GithubUrlReader.ts | 81 ++++-- .../src/reading/GitlabUrlReader.ts | 17 +- .../src/reading/UrlReaderPredicateMux.ts | 7 +- .../src/reading/__fixtures__/mock-main.tar.gz | Bin 0 -> 230 bytes .../src/reading/__fixtures__/repo.tar.gz | Bin 232 -> 0 bytes .../reading/tree/ReadTreeResponseFactory.ts | 10 +- .../reading/tree/TarArchiveResponse.test.ts | 16 +- .../src/reading/tree/TarArchiveResponse.ts | 4 +- .../src/reading/tree/ZipArchiveResponse.ts | 4 +- packages/backend-common/src/reading/types.ts | 19 +- packages/techdocs-common/src/helpers.test.ts | 1 + 16 files changed, 305 insertions(+), 167 deletions(-) create mode 100644 packages/backend-common/src/reading/__fixtures__/mock-main.tar.gz delete mode 100644 packages/backend-common/src/reading/__fixtures__/repo.tar.gz diff --git a/catalog-info.yaml b/catalog-info.yaml index 2144d1709c..7e60af5755 100644 --- a/catalog-info.yaml +++ b/catalog-info.yaml @@ -6,7 +6,7 @@ metadata: Backstage is an open-source developer portal that puts the developer experience first. annotations: github.com/project-slug: backstage/backstage - backstage.io/techdocs-ref: url:https://github.com/backstage/backstage/tree/master + backstage.io/techdocs-ref: url:https://github.com/backstage/backstage lighthouse.com/website-url: https://backstage.io spec: type: library diff --git a/packages/backend-common/src/errors.ts b/packages/backend-common/src/errors.ts index b68dc8e3f0..39703127e6 100644 --- a/packages/backend-common/src/errors.ts +++ b/packages/backend-common/src/errors.ts @@ -75,3 +75,8 @@ export class NotFoundError extends CustomErrorBase {} * resource. */ export class ConflictError extends CustomErrorBase {} + +/** + * The requested resource has not changed since last request. + */ +export class NotModifiedError extends CustomErrorBase {} diff --git a/packages/backend-common/src/reading/AzureUrlReader.ts b/packages/backend-common/src/reading/AzureUrlReader.ts index ea716a1d4a..4535b329d7 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.ts @@ -75,22 +75,27 @@ export class AzureUrlReader implements UrlReader { url: string, options?: ReadTreeOptions, ): Promise { - const response = await fetch( + const archiveAzureResponse = await fetch( getAzureDownloadUrl(url), getAzureRequestOptions(this.options, { Accept: 'application/zip' }), ); - if (!response.ok) { - const message = `Failed to read tree from ${url}, ${response.status} ${response.statusText}`; - if (response.status === 404) { + if (!archiveAzureResponse.ok) { + const message = `Failed to read tree from ${url}, ${archiveAzureResponse.status} ${archiveAzureResponse.statusText}`; + if (archiveAzureResponse.status === 404) { throw new NotFoundError(message); } throw new Error(message); } - return this.deps.treeResponseFactory.fromZipArchive({ - stream: (response.body as unknown) as Readable, + const archiveResponse = await this.deps.treeResponseFactory.fromZipArchive({ + stream: (archiveAzureResponse.body as unknown) as Readable, filter: options?.filter, }); + + const response = archiveResponse as ReadTreeResponse; + // TODO: Just a placeholder for now. + response.sha = ''; + return response; } toString() { diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.ts b/packages/backend-common/src/reading/BitbucketUrlReader.ts index 4578436521..7f462e5bf0 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.ts @@ -108,13 +108,13 @@ export class BitbucketUrlReader implements UrlReader { const isHosted = resource === 'bitbucket.org'; const downloadUrl = await getBitbucketDownloadUrl(url, this.config); - const response = await fetch( + const archiveBitbucketResponse = await fetch( downloadUrl, getBitbucketRequestOptions(this.config), ); - if (!response.ok) { - const message = `Failed to read tree from ${url}, ${response.status} ${response.statusText}`; - if (response.status === 404) { + if (!archiveBitbucketResponse.ok) { + const message = `Failed to read tree from ${url}, ${archiveBitbucketResponse.status} ${archiveBitbucketResponse.statusText}`; + if (archiveBitbucketResponse.status === 404) { throw new NotFoundError(message); } throw new Error(message); @@ -126,11 +126,16 @@ export class BitbucketUrlReader implements UrlReader { folderPath = `${project}-${repoName}-${lastCommitShortHash}`; } - return this.treeResponseFactory.fromZipArchive({ - stream: (response.body as unknown) as Readable, + const archiveResponse = await this.treeResponseFactory.fromZipArchive({ + stream: (archiveBitbucketResponse.body as unknown) as Readable, path: `${folderPath}/${filepath}`, filter: options?.filter, }); + + const response = archiveResponse as ReadTreeResponse; + // TODO: Just a placeholder for now. + response.sha = ''; + return response; } toString() { diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 1a0f27d28f..6fdab9dfa1 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -21,6 +21,7 @@ import fs from 'fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import path from 'path'; +import { NotModifiedError } from '../errors'; import { GithubUrlReader } from './GithubUrlReader'; import { ReadTreeResponseFactory } from './tree'; @@ -28,11 +29,27 @@ const treeResponseFactory = ReadTreeResponseFactory.create({ config: new ConfigReader({}), }); -describe('GithubUrlReader', () => { - const mockCredentialsProvider = ({ - getCredentials: jest.fn().mockResolvedValue({ headers: {} }), - } as unknown) as GithubCredentialsProvider; +const mockCredentialsProvider = ({ + getCredentials: jest.fn().mockResolvedValue({ headers: {} }), +} as unknown) as GithubCredentialsProvider; +const githubProcessor = new GithubUrlReader( + { + host: 'github.com', + apiBaseUrl: 'https://api.github.com', + }, + { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, +); + +const gheProcessor = new GithubUrlReader( + { + host: 'ghe.github.com', + apiBaseUrl: 'https://ghe.github.com/api/v3', + }, + { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, +); + +describe('GithubUrlReader', () => { const worker = setupServer(); msw.setupDefaultHandlers(worker); @@ -43,15 +60,8 @@ describe('GithubUrlReader', () => { describe('implementation', () => { it('rejects unknown targets', async () => { - const processor = new GithubUrlReader( - { - host: 'github.com', - apiBaseUrl: 'https://api.github.com', - }, - { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, - ); await expect( - processor.read('https://not.github.com/apa'), + githubProcessor.read('https://not.github.com/apa'), ).rejects.toThrow( 'Incorrect URL: https://not.github.com/apa, Error: Invalid GitHub URL or file path', ); @@ -73,7 +83,7 @@ describe('GithubUrlReader', () => { worker.use( rest.get( - 'https://api.github.com/repos/backstage/mock/tree/contents/?ref=repo', + 'https://api.github.com/repos/backstage/mock/tree/contents/?ref=main', (req, res, ctx) => { expect(req.headers.get('authorization')).toBe( mockHeaders.Authorization, @@ -90,28 +100,43 @@ describe('GithubUrlReader', () => { ), ); - const processor = new GithubUrlReader( - { - host: 'ghe.github.com', - apiBaseUrl: 'https://api.github.com', - }, - { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, - ); - await processor.read( - 'https://ghe.github.com/backstage/mock/tree/blob/repo', + await githubProcessor.read( + 'https://github.com/backstage/mock/tree/blob/main', ); }); }); describe('readTree', () => { const repoBuffer = fs.readFileSync( - path.resolve('src', 'reading', '__fixtures__', 'repo.tar.gz'), + path.resolve('src', 'reading', '__fixtures__', 'mock-main.tar.gz'), ); + const reposGithubApiResponse = { + id: '123', + full_name: 'backstage/mock', + default_branch: 'main', + branches_url: + 'https://api.github.com/repos/backstage/mock/branches{/branch}', + }; + + const reposGheApiResponse = { + ...reposGithubApiResponse, + branches_url: + 'https://ghe.github.com/api/v3/repos/backstage/mock/branches{/branch}', + }; + + const branchesApiResponse = { + name: 'main', + commit: { + sha: 'sha123abc', + }, + }; + beforeEach(() => { + // For github.com host worker.use( rest.get( - 'https://github.com/backstage/mock/archive/repo.tar.gz', + 'https://github.com/backstage/mock/archive/main.tar.gz', (_, res, ctx) => res( ctx.status(200), @@ -120,20 +145,73 @@ describe('GithubUrlReader', () => { ), ), ); + + worker.use( + rest.get('https://api.github.com/repos/backstage/mock', (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(reposGithubApiResponse), + ), + ), + ); + + worker.use( + rest.get( + 'https://api.github.com/repos/backstage/mock/branches/main', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(branchesApiResponse), + ), + ), + ); + + // For a GHE host + worker.use( + rest.get( + 'https://ghe.github.com/backstage/mock/archive/main.tar.gz', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/x-gzip'), + ctx.body(repoBuffer), + ), + ), + ); + + worker.use( + rest.get( + 'https://ghe.github.com/api/v3/repos/backstage/mock', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(reposGheApiResponse), + ), + ), + ); + + worker.use( + rest.get( + 'https://ghe.github.com/api/v3/repos/backstage/mock/branches/main', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(branchesApiResponse), + ), + ), + ); }); it('returns the wanted files from an archive', async () => { - const processor = new GithubUrlReader( - { - host: 'github.com', - apiBaseUrl: 'https://api.github.com', - }, - { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, + const response = await githubProcessor.readTree( + 'https://github.com/backstage/mock/tree/main', ); - const response = await processor.readTree( - 'https://github.com/backstage/mock/tree/repo', - ); + expect(response.sha).toBe('sha123abc'); const files = await response.files(); @@ -145,40 +223,6 @@ describe('GithubUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); - it('includes the subdomain in the github url', async () => { - worker.resetHandlers(); - worker.use( - rest.get( - 'https://ghe.github.com/backstage/mock/archive/repo.tar.gz', - (_, res, ctx) => - res( - ctx.status(200), - ctx.set('Content-Type', 'application/x-gzip'), - ctx.body(repoBuffer), - ), - ), - ); - - const processor = new GithubUrlReader( - { - host: 'ghe.github.com', - apiBaseUrl: 'https://api.github.com', - }, - { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, - ); - - const response = await processor.readTree( - 'https://ghe.github.com/backstage/mock/tree/repo/docs', - ); - - const files = await response.files(); - - expect(files.length).toBe(1); - const indexMarkdownFile = await files[0].content(); - - expect(indexMarkdownFile.toString()).toBe('# Test\n'); - }); - it('should use the headers from the credentials provider to the fetch request', async () => { expect.assertions(2); @@ -193,7 +237,7 @@ describe('GithubUrlReader', () => { worker.use( rest.get( - 'https://ghe.github.com/backstage/mock/archive/repo.tar.gz', + 'https://ghe.github.com/backstage/mock/archive/main.tar.gz', (req, res, ctx) => { expect(req.headers.get('authorization')).toBe( mockHeaders.Authorization, @@ -210,46 +254,14 @@ describe('GithubUrlReader', () => { ), ); - const processor = new GithubUrlReader( - { - host: 'ghe.github.com', - apiBaseUrl: 'https://api.github.com', - }, - { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, - ); - - await processor.readTree( - 'https://ghe.github.com/backstage/mock/tree/repo/docs', + await gheProcessor.readTree( + 'https://ghe.github.com/backstage/mock/tree/main', ); }); - it('must specify a branch', async () => { - const processor = new GithubUrlReader( - { - host: 'github.com', - apiBaseUrl: 'https://api.github.com', - }, - { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, - ); - - await expect( - processor.readTree('https://github.com/backstage/mock'), - ).rejects.toThrow( - 'GitHub URL must contain branch to be able to fetch tree', - ); - }); - - it('returns the wanted files from an archive with a subpath', async () => { - const processor = new GithubUrlReader( - { - host: 'github.com', - apiBaseUrl: 'https://api.github.com', - }, - { treeResponseFactory, credentialsProvider: mockCredentialsProvider }, - ); - - const response = await processor.readTree( - 'https://github.com/backstage/mock/tree/repo/docs', + it('includes the subdomain in the github url', async () => { + const response = await gheProcessor.readTree( + 'https://ghe.github.com/backstage/mock/tree/main/docs', ); const files = await response.files(); @@ -259,5 +271,55 @@ describe('GithubUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); + + it('returns the wanted files from an archive with a subpath', async () => { + const response = await githubProcessor.readTree( + 'https://github.com/backstage/mock/tree/main/docs', + ); + + const files = await response.files(); + + expect(files.length).toBe(1); + const indexMarkdownFile = await files[0].content(); + + expect(indexMarkdownFile.toString()).toBe('# Test\n'); + }); + + it('throws a NotModifiedError when given a sha in options', async () => { + const fnGithub = async () => { + await githubProcessor.readTree('https://github.com/backstage/mock', { + sha: 'sha123abc', + }); + }; + + const fnGhe = async () => { + await gheProcessor.readTree( + 'https://ghe.github.com/backstage/mock/tree/main/docs', + { + sha: 'sha123abc', + }, + ); + }; + + await expect(fnGithub).rejects.toThrow(NotModifiedError); + await expect(fnGhe).rejects.toThrow(NotModifiedError); + }); + + it('should not throw error when given an outdated sha in options', async () => { + const response = await githubProcessor.readTree( + 'https://github.com/backstage/mock/tree/main', + { + sha: 'outdatedSha123abc', + }, + ); + expect((await response.files()).length).toBe(2); + }); + + it('should detect the default branch', async () => { + const response = await githubProcessor.readTree( + 'https://github.com/backstage/mock', + ); + expect((await response.files()).length).toBe(2); + }); }); }); diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index 45ce8336e6..2acca10d4f 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -23,7 +23,7 @@ import { import fetch from 'cross-fetch'; import parseGitUrl from 'git-url-parse'; import { Readable } from 'stream'; -import { InputError, NotFoundError } from '../errors'; +import { NotFoundError, NotModifiedError } from '../errors'; import { ReadTreeResponseFactory } from './tree'; import { ReaderFactory, @@ -99,52 +99,83 @@ export class GithubUrlReader implements UrlReader { options?: ReadTreeOptions, ): Promise { const { - name: repoName, - ref, protocol, resource, - full_name, + name: repoName, + ref, filepath, + full_name, } = parseGitUrl(url); - if (!ref) { - // TODO(Rugvip): We should add support for defaulting to the default branch - throw new InputError( - 'GitHub URL must contain branch to be able to fetch tree', - ); - } - const { headers } = await this.deps.credentialsProvider.getCredentials({ url, }); - // TODO(Rugvip): use API to fetch URL instead - const response = await fetch( - new URL( - `${protocol}://${resource}/${full_name}/archive/${ref}.tar.gz`, - ).toString(), + + // Get GitHub API urls for the repository + const repoGitHubResponse = await fetch( + new URL(`${this.config.apiBaseUrl}/repos/${full_name}`).toString(), { - headers: { - ...headers, - }, + headers, }, ); - if (!response.ok) { - const message = `Failed to read tree from ${url}, ${response.status} ${response.statusText}`; - if (response.status === 404) { + if (!repoGitHubResponse.ok) { + const message = `Failed to read tree from ${url}, ${repoGitHubResponse.status} ${repoGitHubResponse.statusText}`; + if (repoGitHubResponse.status === 404) { throw new NotFoundError(message); } throw new Error(message); } - const path = `${repoName}-${ref}/${filepath}`; + const repoResponseJson = await repoGitHubResponse.json(); - return this.deps.treeResponseFactory.fromTarArchive({ + // ref is an empty string if no branch is set in provided url to readTree. + // Use GitHub API to get the default branch of the repository. + const branch = ref === '' ? repoResponseJson.default_branch : ref; + const branchesApiUrl = repoResponseJson.branches_url; + + // Fetch the latest commit in the provided or default branch to compare against + // the provided sha. + const branchGitHubResponse = await fetch( + // branchesApiUrl looks like "https://api.github.com/repos/owner/repo/branches{/branch}" + branchesApiUrl.replace('{/branch}', `/${branch}`), + { + headers, + }, + ); + const commitSha = (await branchGitHubResponse.json()).commit.sha; + + if (options?.sha && options.sha === commitSha) { + throw new NotModifiedError(); + } + + // Note: the API way of downloading an archive URL does not return a real time archive. + // https://github.community/t/archive-downloaded-via-v3-rest-api-is-not-real-time/14827 + // It looks like this https://api.github.com/repos/owner/repo/{archive_format}{/ref} + // and can be used from `repoResponseJson.archive_url`. + // Continue using the "direct" way i.e. https://github.com/:owner/:repo/archive/branch.tar.gz + // until the bug? is fixed. + const archive = await fetch( + new URL( + `${protocol}://${resource}/${full_name}/archive/${branch}.tar.gz`, + ).toString(), + { + headers, + }, + ); + + const path = `${repoName}-${branch}/${filepath}`; + + const archiveResponse = 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: (response.body as unknown) as Readable, + stream: (archive.body as unknown) as Readable, path, filter: options?.filter, }); + + const response = archiveResponse as ReadTreeResponse; + response.sha = commitSha; + return response; } toString() { diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts index 299bbc783b..76cfea7c38 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.ts @@ -94,13 +94,13 @@ export class GitlabUrlReader implements UrlReader { } const archive = `${protocol}://${resource}/${full_name}/-/archive/${ref}/${repoName}-${ref}.zip`; - const response = await fetch( + const archiveGitLabResponse = await fetch( archive, getGitLabRequestOptions(this.options), ); - if (!response.ok) { - const msg = `Failed to read tree from ${url}, ${response.status} ${response.statusText}`; - if (response.status === 404) { + if (!archiveGitLabResponse.ok) { + const msg = `Failed to read tree from ${url}, ${archiveGitLabResponse.status} ${archiveGitLabResponse.statusText}`; + if (archiveGitLabResponse.status === 404) { throw new NotFoundError(msg); } throw new Error(msg); @@ -108,11 +108,16 @@ export class GitlabUrlReader implements UrlReader { const path = filepath ? `${repoName}-${ref}/${filepath}/` : ''; - return this.treeResponseFactory.fromZipArchive({ - stream: (response.body as unknown) as Readable, + const archiveResponse = await this.treeResponseFactory.fromZipArchive({ + stream: (archiveGitLabResponse.body as unknown) as Readable, path, filter: options?.filter, }); + + const response = archiveResponse as ReadTreeResponse; + // TODO: Just a placeholder for now. + response.sha = ''; + return response; } toString() { diff --git a/packages/backend-common/src/reading/UrlReaderPredicateMux.ts b/packages/backend-common/src/reading/UrlReaderPredicateMux.ts index ca11400e4a..3183aa0c28 100644 --- a/packages/backend-common/src/reading/UrlReaderPredicateMux.ts +++ b/packages/backend-common/src/reading/UrlReaderPredicateMux.ts @@ -45,12 +45,15 @@ export class UrlReaderPredicateMux implements UrlReader { throw new NotAllowedError(`Reading from '${url}' is not allowed`); } - readTree(url: string, options?: ReadTreeOptions): Promise { + async readTree( + url: string, + options?: ReadTreeOptions, + ): Promise { const parsed = new URL(url); for (const { predicate, reader } of this.readers) { if (predicate(parsed)) { - return reader.readTree(url, options); + return await reader.readTree(url, options); } } diff --git a/packages/backend-common/src/reading/__fixtures__/mock-main.tar.gz b/packages/backend-common/src/reading/__fixtures__/mock-main.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..291690b447eae0995eda9c4215ffc21636d516c1 GIT binary patch literal 230 zcmV>lPUp5kJIk}HdFl55zWvjD0$=4n1-t*;egQ83=b-1m(n)&pF&VYT g$NUG`^WT<-F8}9X?PA~Ia5xsp4OF6Z-vAH*0G5MqdH?_b literal 0 HcmV?d00001 diff --git a/packages/backend-common/src/reading/__fixtures__/repo.tar.gz b/packages/backend-common/src/reading/__fixtures__/repo.tar.gz deleted file mode 100644 index 7a8e9902a24232a5f7130637a05304134abc3fec..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 232 zcmb2|=3oE==C@Zbay1!XSu&B+beL(LRxe2^Idk!eV446aOJ+o+S-o>~??20b^`}#YbLB3%Cll?@@AuK( zf9m={{U?{EUwv`;^MMVwzTe;Tuda?M|Lgvh`?I~>7XNQAm|j0i{htM|=koGBZ|a}E f`_yk@`Q^XBu@$utzd*@``2}nqTB%DJG#D5FAn { + async fromTarArchive( + options: FromArchiveOptions, + ): Promise { return new TarArchiveResponse( options.stream, options.path ?? '', @@ -49,7 +51,9 @@ export class ReadTreeResponseFactory { ); } - async fromZipArchive(options: FromArchiveOptions): Promise { + async fromZipArchive( + options: FromArchiveOptions, + ): Promise { return new ZipArchiveResponse( options.stream, options.path ?? '', diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts index 1bc0d3a386..bbeee00c70 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts @@ -20,7 +20,7 @@ import { resolve as resolvePath } from 'path'; import { TarArchiveResponse } from './TarArchiveResponse'; const archiveData = fs.readFileSync( - resolvePath(__filename, '../../__fixtures__/repo.tar.gz'), + resolvePath(__filename, '../../__fixtures__/mock-main.tar.gz'), ); describe('TarArchiveResponse', () => { @@ -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-repo/', '/tmp'); + const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp'); const files = await res.files(); expect(files).toEqual([ @@ -61,7 +61,7 @@ describe('TarArchiveResponse', () => { it('should read files with filter', async () => { const stream = fs.createReadStream('/test-archive.tar.gz'); - const res = new TarArchiveResponse(stream, 'mock-repo/', '/tmp', path => + const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp', path => path.endsWith('.yml'), ); const files = await res.files(); @@ -79,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-repo/', '/tmp'); + const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp'); const buffer = await res.archive(); await expect(res.archive()).rejects.toThrow( @@ -113,17 +113,17 @@ describe('TarArchiveResponse', () => { const dir = await res.dir(); await expect( - fs.readFile(resolvePath(dir, 'mock-repo/mkdocs.yml'), 'utf8'), + fs.readFile(resolvePath(dir, 'mock-main/mkdocs.yml'), 'utf8'), ).resolves.toBe('site_name: Test\n'); await expect( - fs.readFile(resolvePath(dir, 'mock-repo/docs/index.md'), 'utf8'), + fs.readFile(resolvePath(dir, 'mock-main/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-repo/docs/', '/tmp'); + const res = new TarArchiveResponse(stream, 'mock-main/docs/', '/tmp'); const dir = await res.dir(); expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/); @@ -135,7 +135,7 @@ 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-repo/', '/tmp', path => + const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp', path => path.endsWith('.yml'), ); const dir = await res.dir({ targetDir: '/tmp' }); diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.ts index 5d18ec7dc6..e8929fbe9b 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.ts @@ -21,7 +21,7 @@ import { Readable, pipeline as pipelineCb } from 'stream'; import { promisify } from 'util'; import concatStream from 'concat-stream'; import { - ReadTreeResponse, + ReadTreeArchiveResponse, ReadTreeResponseFile, ReadTreeResponseDirOptions, } from '../types'; @@ -34,7 +34,7 @@ const pipeline = promisify(pipelineCb); /** * Wraps a tar archive stream into a tree response reader. */ -export class TarArchiveResponse implements ReadTreeResponse { +export class TarArchiveResponse implements ReadTreeArchiveResponse { private read = false; constructor( diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts index 4106d49a11..48ba4f19ee 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts @@ -20,7 +20,7 @@ import unzipper, { Entry } from 'unzipper'; import archiver from 'archiver'; import { Readable } from 'stream'; import { - ReadTreeResponse, + ReadTreeArchiveResponse, ReadTreeResponseFile, ReadTreeResponseDirOptions, } from '../types'; @@ -28,7 +28,7 @@ import { /** * Wraps a zip archive stream into a tree response reader. */ -export class ZipArchiveResponse implements ReadTreeResponse { +export class ZipArchiveResponse implements ReadTreeArchiveResponse { private read = false; constructor( diff --git a/packages/backend-common/src/reading/types.ts b/packages/backend-common/src/reading/types.ts index f9dca3e1d5..c727ba078b 100644 --- a/packages/backend-common/src/reading/types.ts +++ b/packages/backend-common/src/reading/types.ts @@ -32,6 +32,19 @@ export type ReadTreeOptions = { * If no filter is provided all files are extracted. */ filter?(path: string): boolean; + + /** + * A commit SHA can be provided to check whether readTree's response has changed from a previous execution. + * + * In the readTree() response, a SHA is returned along with the tree blob. The SHA belongs to the + * latest commit on the target repository's branch that was used to read the blob. + * + * When a SHA is given in ReadTreeOptions, readTree will first compare the SHA against the latest commit + * on the target branch. If they match, it will throw a NotModifiedError indicating that the readTree + * response will not differ from the previous response which included this particular SHA. If they mismatch, + * readTree will return a new SHA along with the rest of ReadTreeResponse. + */ + sha?: string; }; /** @@ -67,8 +80,12 @@ export type ReadTreeResponseDirOptions = { targetDir?: string; }; -export type ReadTreeResponse = { +export type ReadTreeArchiveResponse = { files(): Promise; archive(): Promise; dir(options?: ReadTreeResponseDirOptions): Promise; }; + +export interface ReadTreeResponse extends ReadTreeArchiveResponse { + sha: string; +} diff --git a/packages/techdocs-common/src/helpers.test.ts b/packages/techdocs-common/src/helpers.test.ts index 88c4d8829e..88fdeed498 100644 --- a/packages/techdocs-common/src/helpers.test.ts +++ b/packages/techdocs-common/src/helpers.test.ts @@ -135,6 +135,7 @@ describe('getDocFilesFromRepository', () => { archive: async () => { return Readable.from(''); }, + sha: '', }; } }