From 8565ad2279c157cd0c404499efe51b6942a6b685 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Mon, 18 Jan 2021 22:50:47 +0100 Subject: [PATCH] 1. Use etag over sha for the identifier 2. Re-combine Resonse types into one --- .changeset/khaki-icons-trade.md | 10 ++++-- .github/styles/vocab.txt | 1 + .../src/reading/AzureUrlReader.test.ts | 12 +++---- .../src/reading/AzureUrlReader.ts | 9 ++---- .../src/reading/BitbucketUrlReader.test.ts | 16 +++++----- .../src/reading/BitbucketUrlReader.ts | 9 ++---- .../src/reading/GithubUrlReader.test.ts | 14 ++++----- .../src/reading/GithubUrlReader.ts | 9 ++---- .../src/reading/GitlabUrlReader.test.ts | 10 +++--- .../src/reading/GitlabUrlReader.ts | 9 ++---- .../reading/tree/ReadTreeResponseFactory.ts | 14 ++++----- .../reading/tree/TarArchiveResponse.test.ts | 31 +++++++++++++------ .../src/reading/tree/TarArchiveResponse.ts | 8 +++-- .../reading/tree/ZipArchiveResponse.test.ts | 31 +++++++++++++------ .../src/reading/tree/ZipArchiveResponse.ts | 8 +++-- packages/backend-common/src/reading/types.ts | 27 ++++++++-------- packages/techdocs-common/src/helpers.test.ts | 2 +- 17 files changed, 124 insertions(+), 96 deletions(-) diff --git a/.changeset/khaki-icons-trade.md b/.changeset/khaki-icons-trade.md index 37bafd8d33..4d30c3977e 100644 --- a/.changeset/khaki-icons-trade.md +++ b/.changeset/khaki-icons-trade.md @@ -2,7 +2,11 @@ '@backstage/backend-common': patch --- -1. URL Reader's `readTree` method now returns a `sha` in the response along with the blob. The SHA belongs to the latest commit on the target. `readTree` also takes an optional `sha` in its options and throws a `NotModifiedError` if the SHA matches with the latest commit SHA on the url's target. This can be used in building a cache when working with URL Reader. +1. URL Reader's `readTree` method now returns an `etag` in the response along with the blob. The etag is an identifier of the blob and will only change if the blob is modified on the target. Usually it is set to the latest commit SHA on the target. + +`readTree` also takes an optional `etag` in its options and throws a `NotModifiedError` if the etag matches with the etag of the resource. + +So, the `etag` can be used in building a cache when working with URL Reader. An example - @@ -11,11 +15,11 @@ const response = await reader.readTree( 'https://github.com/backstage/backstage', ); -const commitSha = response.sha; +const etag = response.etag; // Will throw a new NotModifiedError (exported from @backstage/backstage-common) await reader.readTree('https://github.com/backstage/backstage', { - sha: commitSha, + etag, }); ``` diff --git a/.github/styles/vocab.txt b/.github/styles/vocab.txt index c3c67ab66e..96581d4b79 100644 --- a/.github/styles/vocab.txt +++ b/.github/styles/vocab.txt @@ -67,6 +67,7 @@ Dominik dtuite dzolotusky Ek +etag env Env eslint diff --git a/packages/backend-common/src/reading/AzureUrlReader.test.ts b/packages/backend-common/src/reading/AzureUrlReader.test.ts index b492b26609..20f8feba42 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.test.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.test.ts @@ -188,7 +188,7 @@ describe('AzureUrlReader', () => { 'https://dev.azure.com/organization/project/_git/repository', ); - expect(response.sha).toBe('123abc2'); + expect(response.etag).toBe('123abc2'); const files = await response.files(); @@ -200,24 +200,24 @@ describe('AzureUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); - it('throws a NotModifiedError when given a sha in options', async () => { + it('throws a NotModifiedError when given a etag in options', async () => { const fnAzure = async () => { await processor.readTree( 'https://dev.azure.com/organization/project/_git/repository', - { sha: '123abc2' }, + { etag: '123abc2' }, ); }; await expect(fnAzure).rejects.toThrow(NotModifiedError); }); - it('should not throw a NotModifiedError when given an outdated sha in options', async () => { + it('should not throw a NotModifiedError when given an outdated etag in options', async () => { const response = await processor.readTree( 'https://dev.azure.com/organization/project/_git/repository', - { sha: 'outdated123abc' }, + { etag: 'outdated123abc' }, ); - expect(response.sha).toBe('123abc2'); + expect(response.etag).toBe('123abc2'); const files = await response.files(); expect(files.length).toBe(2); diff --git a/packages/backend-common/src/reading/AzureUrlReader.ts b/packages/backend-common/src/reading/AzureUrlReader.ts index c40291ec85..59e437607f 100644 --- a/packages/backend-common/src/reading/AzureUrlReader.ts +++ b/packages/backend-common/src/reading/AzureUrlReader.ts @@ -91,7 +91,7 @@ export class AzureUrlReader implements UrlReader { } const commitSha = (await commitsAzureResponse.json()).value[0].commitId; - if (options?.sha && options.sha === commitSha) { + if (options?.etag && options.etag === commitSha) { throw new NotModifiedError(); } @@ -107,14 +107,11 @@ export class AzureUrlReader implements UrlReader { throw new Error(message); } - const archiveResponse = await this.deps.treeResponseFactory.fromZipArchive({ + return await this.deps.treeResponseFactory.fromZipArchive({ stream: (archiveAzureResponse.body as unknown) as Readable, + etag: commitSha, filter: options?.filter, }); - - const response = archiveResponse as ReadTreeResponse; - response.sha = commitSha; - return response; } toString() { diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts index 111aff753c..3542e822e1 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts @@ -135,7 +135,7 @@ describe('BitbucketUrlReader', () => { 'https://bitbucket.org/backstage/mock/src/master', ); - expect(response.sha).toBe('12ab34cd56ef'); + expect(response.etag).toBe('12ab34cd56ef'); const files = await response.files(); @@ -152,7 +152,7 @@ describe('BitbucketUrlReader', () => { 'https://bitbucket.mycompany.net/projects/backstage/repos/mock/browse/docs?at=some-branch', ); - expect(response.sha).toBe('12ab34cd56ef'); + expect(response.etag).toBe('12ab34cd56ef'); const files = await response.files(); @@ -167,7 +167,7 @@ describe('BitbucketUrlReader', () => { 'https://bitbucket.org/backstage/mock/src/master/docs', ); - expect(response.sha).toBe('12ab34cd56ef'); + expect(response.etag).toBe('12ab34cd56ef'); const files = await response.files(); @@ -177,24 +177,24 @@ describe('BitbucketUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); - it('throws a NotModifiedError when given a sha in options', async () => { + it('throws a NotModifiedError when given a etag in options', async () => { const fnBitbucket = async () => { await bitbucketProcessor.readTree( 'https://bitbucket.org/backstage/mock', - { sha: '12ab34cd56ef' }, + { etag: '12ab34cd56ef' }, ); }; await expect(fnBitbucket).rejects.toThrow(NotModifiedError); }); - it('should not throw a NotModifiedError when given an outdated sha in options', async () => { + it('should not throw a NotModifiedError when given an outdated etag in options', async () => { const response = await bitbucketProcessor.readTree( 'https://bitbucket.org/backstage/mock', - { sha: 'outdatedSha123abc' }, + { etag: 'outdatedetag123abc' }, ); - expect(response.sha).toBe('12ab34cd56ef'); + expect(response.etag).toBe('12ab34cd56ef'); }); }); }); diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.ts b/packages/backend-common/src/reading/BitbucketUrlReader.ts index 606aecd023..869870f248 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.ts @@ -106,7 +106,7 @@ export class BitbucketUrlReader implements UrlReader { ); const lastCommitShortHash = await this.getLastCommitShortHash(url); - if (options?.sha && options.sha === lastCommitShortHash) { + if (options?.etag && options.etag === lastCommitShortHash) { throw new NotModifiedError(); } @@ -130,15 +130,12 @@ export class BitbucketUrlReader implements UrlReader { folderPath = `${project}-${repoName}-${lastCommitShortHash}`; } - const archiveResponse = await this.treeResponseFactory.fromZipArchive({ + return await this.treeResponseFactory.fromZipArchive({ stream: (archiveBitbucketResponse.body as unknown) as Readable, path: `${folderPath}/${filepath}`, + etag: lastCommitShortHash, filter: options?.filter, }); - - const response = archiveResponse as ReadTreeResponse; - response.sha = lastCommitShortHash; - return response; } toString() { diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index a311da910a..26ba91bd29 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -128,7 +128,7 @@ describe('GithubUrlReader', () => { const branchesApiResponse = { name: 'main', commit: { - sha: 'sha123abc', + sha: 'etag123abc', }, }; @@ -198,7 +198,7 @@ describe('GithubUrlReader', () => { 'https://github.com/backstage/mock/tree/main', ); - expect(response.sha).toBe('sha123abc'); + expect(response.etag).toBe('etag123abc'); const files = await response.files(); @@ -272,10 +272,10 @@ describe('GithubUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); - it('throws a NotModifiedError when given a sha in options', async () => { + it('throws a NotModifiedError when given a etag in options', async () => { const fnGithub = async () => { await githubProcessor.readTree('https://github.com/backstage/mock', { - sha: 'sha123abc', + etag: 'etag123abc', }); }; @@ -283,7 +283,7 @@ describe('GithubUrlReader', () => { await gheProcessor.readTree( 'https://ghe.github.com/backstage/mock/tree/main/docs', { - sha: 'sha123abc', + etag: 'etag123abc', }, ); }; @@ -292,11 +292,11 @@ describe('GithubUrlReader', () => { await expect(fnGhe).rejects.toThrow(NotModifiedError); }); - it('should not throw error when given an outdated sha in options', async () => { + it('should not throw error when given an outdated etag in options', async () => { const response = await githubProcessor.readTree( 'https://github.com/backstage/mock/tree/main', { - sha: 'outdatedSha123abc', + etag: 'outdatedetag123abc', }, ); 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 8e0410642b..1a8133fceb 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -151,7 +151,7 @@ export class GithubUrlReader implements UrlReader { } const commitSha = (await branchGitHubResponse.json()).commit.sha; - if (options?.sha && options.sha === commitSha) { + if (options?.etag && options.etag === commitSha) { throw new NotModifiedError(); } @@ -179,17 +179,14 @@ export class GithubUrlReader implements UrlReader { const path = `${repoName}-${branch}/${filepath}`; - const archiveResponse = await this.deps.treeResponseFactory.fromTarArchive({ + 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, + etag: commitSha, filter: options?.filter, }); - - const response = archiveResponse as ReadTreeResponse; - response.sha = commitSha; - return response; } toString() { diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index 823555d37d..2e66794397 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -284,10 +284,10 @@ describe('GitlabUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); - it('throws a NotModifiedError when given a sha in options', async () => { + it('throws a NotModifiedError when given a etag in options', async () => { const fnGitlab = async () => { await gitlabProcessor.readTree('https://gitlab.com/backstage/mock', { - sha: 'sha123abc', + etag: 'sha123abc', }); }; @@ -295,7 +295,7 @@ describe('GitlabUrlReader', () => { await hostedGitlabProcessor.readTree( 'https://gitlab.mycompany.com/backstage/mock', { - sha: 'sha123abc', + etag: 'sha123abc', }, ); }; @@ -304,11 +304,11 @@ describe('GitlabUrlReader', () => { await expect(fnHostedGitlab).rejects.toThrow(NotModifiedError); }); - it('should not throw error when given an outdated sha in options', async () => { + it('should not throw error when given an outdated etag in options', async () => { const response = await gitlabProcessor.readTree( 'https://gitlab.com/backstage/mock/tree/main', { - sha: 'outdatedSha123abc', + etag: 'outdatedsha123abc', }, ); expect((await response.files()).length).toBe(2); diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts index 0c0ce4f565..647c6fd644 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.ts @@ -121,7 +121,7 @@ export class GitlabUrlReader implements UrlReader { const commitSha = (await branchGitlabResponse.json()).commit.id; - if (options?.sha && options.sha === commitSha) { + if (options?.etag && options.etag === commitSha) { throw new NotModifiedError(); } @@ -144,15 +144,12 @@ export class GitlabUrlReader implements UrlReader { ? `${repoName}-${branch}-${commitSha}/${filepath}/` : ''; - const archiveResponse = await this.treeResponseFactory.fromZipArchive({ + return await this.treeResponseFactory.fromZipArchive({ stream: (archiveGitLabResponse.body as unknown) as Readable, path, + etag: commitSha, filter: options?.filter, }); - - const response = archiveResponse as ReadTreeResponse; - response.sha = commitSha; - return response; } toString() { diff --git a/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts b/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts index cc229e9d5a..7332154d09 100644 --- a/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts +++ b/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts @@ -17,7 +17,7 @@ import os from 'os'; import { Readable } from 'stream'; import { Config } from '@backstage/config'; -import { ReadTreeArchiveResponse } from '../types'; +import { ReadTreeResponse } from '../types'; import { TarArchiveResponse } from './TarArchiveResponse'; import { ZipArchiveResponse } from './ZipArchiveResponse'; @@ -26,6 +26,8 @@ type FromArchiveOptions = { stream: Readable; // If set, the root of the tree will be set to the given directory path. path?: string; + // etag of the blob + etag: string; // Filter passed on from the ReadTreeOptions filter?: (path: string) => boolean; }; @@ -40,24 +42,22 @@ export class ReadTreeResponseFactory { constructor(private readonly workDir: string) {} - async fromTarArchive( - options: FromArchiveOptions, - ): Promise { + async fromTarArchive(options: FromArchiveOptions): Promise { return new TarArchiveResponse( options.stream, options.path ?? '', this.workDir, + options.etag, options.filter, ); } - async fromZipArchive( - options: FromArchiveOptions, - ): Promise { + async fromZipArchive(options: FromArchiveOptions): Promise { return new ZipArchiveResponse( options.stream, options.path ?? '', this.workDir, + options.etag, options.filter, ); } diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts index bbeee00c70..2cbfc4a89e 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts @@ -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'); + const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp', 'etag'); const files = await res.files(); expect(files).toEqual([ @@ -61,8 +61,12 @@ 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', path => - path.endsWith('.yml'), + const res = new TarArchiveResponse( + stream, + 'mock-main/', + '/tmp', + 'etag', + path => path.endsWith('.yml'), ); const files = await res.files(); @@ -79,14 +83,14 @@ 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'); + const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp', 'etag'); const buffer = await res.archive(); await expect(res.archive()).rejects.toThrow( 'Response has already been read', ); - const res2 = new TarArchiveResponse(buffer, '', '/tmp'); + const res2 = new TarArchiveResponse(buffer, '', '/tmp', 'etag'); const files = await res2.files(); expect(files).toEqual([ @@ -109,7 +113,7 @@ describe('TarArchiveResponse', () => { it('should extract entire archive into directory', async () => { const stream = fs.createReadStream('/test-archive.tar.gz'); - const res = new TarArchiveResponse(stream, '', '/tmp'); + const res = new TarArchiveResponse(stream, '', '/tmp', 'etag'); const dir = await res.dir(); await expect( @@ -123,7 +127,12 @@ describe('TarArchiveResponse', () => { 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'); + const res = new TarArchiveResponse( + stream, + 'mock-main/docs/', + '/tmp', + 'etag', + ); const dir = await res.dir(); expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/); @@ -135,8 +144,12 @@ 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', path => - path.endsWith('.yml'), + const res = new TarArchiveResponse( + stream, + 'mock-main/', + '/tmp', + 'etag', + 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 e8929fbe9b..f44adcc7b2 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 { - ReadTreeArchiveResponse, + ReadTreeResponse, ReadTreeResponseFile, ReadTreeResponseDirOptions, } from '../types'; @@ -34,13 +34,15 @@ const pipeline = promisify(pipelineCb); /** * Wraps a tar archive stream into a tree response reader. */ -export class TarArchiveResponse implements ReadTreeArchiveResponse { +export class TarArchiveResponse implements ReadTreeResponse { private read = false; + public readonly etag; constructor( private readonly stream: Readable, private readonly subPath: string, private readonly workDir: string, + etag: string, private readonly filter?: (path: string) => boolean, ) { if (subPath) { @@ -53,6 +55,8 @@ export class TarArchiveResponse implements ReadTreeArchiveResponse { ); } } + + this.etag = etag; } // Make sure the input stream is only read once diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts index 345ec7a783..b42ec79d81 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts +++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts @@ -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'); + const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', 'etag'); const files = await res.files(); expect(files).toEqual([ @@ -61,8 +61,12 @@ describe('ZipArchiveResponse', () => { it('should read files with filter', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', path => - path.endsWith('.yml'), + const res = new ZipArchiveResponse( + stream, + 'mock-main/', + '/tmp', + 'etag', + path => path.endsWith('.yml'), ); const files = await res.files(); @@ -79,14 +83,14 @@ 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'); + const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', 'etag'); const buffer = await res.archive(); await expect(res.archive()).rejects.toThrow( 'Response has already been read', ); - const res2 = new ZipArchiveResponse(buffer, '', '/tmp'); + const res2 = new ZipArchiveResponse(buffer, '', '/tmp', 'etag'); const files = await res2.files(); expect(files).toEqual([ @@ -109,7 +113,7 @@ describe('ZipArchiveResponse', () => { it('should extract entire archive into directory', async () => { const stream = fs.createReadStream('/test-archive.zip'); - const res = new ZipArchiveResponse(stream, '', '/tmp'); + const res = new ZipArchiveResponse(stream, '', '/tmp', 'etag'); const dir = await res.dir(); await expect( @@ -123,7 +127,12 @@ describe('ZipArchiveResponse', () => { 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'); + const res = new ZipArchiveResponse( + stream, + 'mock-main/docs/', + '/tmp', + 'etag', + ); const dir = await res.dir(); expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/); @@ -135,8 +144,12 @@ 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', path => - path.endsWith('.yml'), + const res = new ZipArchiveResponse( + stream, + 'mock-main/', + '/tmp', + 'etag', + path => path.endsWith('.yml'), ); const dir = await res.dir({ targetDir: '/tmp' }); diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts index 48ba4f19ee..7550dc1259 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 { - ReadTreeArchiveResponse, + ReadTreeResponse, ReadTreeResponseFile, ReadTreeResponseDirOptions, } from '../types'; @@ -28,13 +28,15 @@ import { /** * Wraps a zip archive stream into a tree response reader. */ -export class ZipArchiveResponse implements ReadTreeArchiveResponse { +export class ZipArchiveResponse implements ReadTreeResponse { private read = false; + public readonly etag; constructor( private readonly stream: Readable, private readonly subPath: string, private readonly workDir: string, + etag: string, private readonly filter?: (path: string) => boolean, ) { if (subPath) { @@ -47,6 +49,8 @@ export class ZipArchiveResponse implements ReadTreeArchiveResponse { ); } } + + this.etag = etag; } // Make sure the input stream is only read once diff --git a/packages/backend-common/src/reading/types.ts b/packages/backend-common/src/reading/types.ts index d5c24ed1db..e98f760d8f 100644 --- a/packages/backend-common/src/reading/types.ts +++ b/packages/backend-common/src/reading/types.ts @@ -34,17 +34,17 @@ export type ReadTreeOptions = { filter?(path: string): boolean; /** - * A commit SHA can be provided to check whether readTree's response has changed from a previous execution. + * An etag 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. + * In the readTree() response, an etag is returned along with the tree blob. The etag is a unique identifer + * of the tree blob, usually the commit SHA or etag from the target. * - * 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. + * When a etag is given in ReadTreeOptions, readTree will first compare the etag against the etag + * on the target branch. If they match, readTree will throw a NotModifiedError indicating that the readTree + * response will not differ from the previous response which included this particular etag. If they mismatch, + * readTree will return the rest of ReadTreeResponse along with a new etag. */ - sha?: string; + etag?: string; }; /** @@ -80,7 +80,7 @@ export type ReadTreeResponseDirOptions = { targetDir?: string; }; -export type ReadTreeArchiveResponse = { +export type ReadTreeResponse = { files(): Promise; archive(): Promise; @@ -88,8 +88,9 @@ export type ReadTreeArchiveResponse = { * dir() extracts the tree response into a directory and returns the path of the directory. */ dir(options?: ReadTreeResponseDirOptions): Promise; -}; -export interface ReadTreeResponse extends ReadTreeArchiveResponse { - sha: string; -} + /** + * A unique identifer of the tree blob, usually the commit SHA or etag from the target. + */ + etag: string; +}; diff --git a/packages/techdocs-common/src/helpers.test.ts b/packages/techdocs-common/src/helpers.test.ts index 88fdeed498..740a08aaa2 100644 --- a/packages/techdocs-common/src/helpers.test.ts +++ b/packages/techdocs-common/src/helpers.test.ts @@ -135,7 +135,7 @@ describe('getDocFilesFromRepository', () => { archive: async () => { return Readable.from(''); }, - sha: '', + etag: '', }; } }