From 7078d35e0af4c0ef44cd63ae47f2f1361bab460c Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Sun, 17 Jan 2021 14:03:43 +0100 Subject: [PATCH] backend-common: UrlReader/Bitbucket implement SHA based caching Default branch detection was already implemented --- .../src/reading/BitbucketUrlReader.test.ts | 157 ++++++++++-------- .../src/reading/BitbucketUrlReader.ts | 13 +- .../src/reading/GithubUrlReader.test.ts | 20 --- .../src/reading/GitlabUrlReader.test.ts | 18 -- 4 files changed, 96 insertions(+), 112 deletions(-) diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts index e327770114..111aff753c 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.test.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.test.ts @@ -20,6 +20,7 @@ import fs from 'fs'; import { rest } from 'msw'; import { setupServer } from 'msw/node'; import path from 'path'; +import { NotModifiedError } from '../errors'; import { BitbucketUrlReader } from './BitbucketUrlReader'; import { ReadTreeResponseFactory } from './tree'; @@ -27,15 +28,24 @@ const treeResponseFactory = ReadTreeResponseFactory.create({ config: new ConfigReader({}), }); +const bitbucketProcessor = new BitbucketUrlReader( + { host: 'bitbucket.org', apiBaseUrl: 'https://api.bitbucket.org/2.0' }, + { treeResponseFactory }, +); + +const hostedBitbucketProcessor = new BitbucketUrlReader( + { + host: 'bitbucket.mycompany.net', + apiBaseUrl: 'https://api.bitbucket.mycompany.net/rest/api/1.0', + }, + { treeResponseFactory }, +); + describe('BitbucketUrlReader', () => { describe('implementation', () => { it('rejects unknown targets', async () => { - const processor = new BitbucketUrlReader( - { host: 'bitbucket.org', apiBaseUrl: 'https://api.bitbucket.org/2.0' }, - { treeResponseFactory }, - ); await expect( - processor.read('https://not.bitbucket.com/apa'), + bitbucketProcessor.read('https://not.bitbucket.com/apa'), ).rejects.toThrow( 'Incorrect URL: https://not.bitbucket.com/apa, Error: Invalid Bitbucket URL or file path', ); @@ -55,8 +65,30 @@ describe('BitbucketUrlReader', () => { ), ); - it('returns the wanted files from an archive', async () => { + const privateBitbucketRepoBuffer = fs.readFileSync( + path.resolve( + 'src', + 'reading', + '__fixtures__', + 'bitbucket-server-repo.zip', + ), + ); + + beforeEach(() => { worker.use( + rest.get( + 'https://api.bitbucket.org/2.0/repositories/backstage/mock', + (_, res, ctx) => + res( + ctx.status(200), + ctx.json({ + mainbranch: { + type: 'branch', + name: 'master', + }, + }), + ), + ), rest.get( 'https://bitbucket.org/backstage/mock/get/master.zip', (_, res, ctx) => @@ -76,17 +108,35 @@ describe('BitbucketUrlReader', () => { }), ), ), + rest.get( + 'https://api.bitbucket.mycompany.net/rest/api/1.0/projects/backstage/repos/mock/archive?format=zip&prefix=mock&path=docs', + (_, res, ctx) => + res( + ctx.status(200), + ctx.set('Content-Type', 'application/zip'), + ctx.body(privateBitbucketRepoBuffer), + ), + ), + rest.get( + 'https://api.bitbucket.mycompany.net/rest/api/1.0/repositories/backstage/mock/commits/some-branch', + (_, res, ctx) => + res( + ctx.status(200), + ctx.json({ + values: [{ hash: '12ab34cd56ef78gh90ij12kl34mn56op78qr90st' }], + }), + ), + ), ); + }); - const processor = new BitbucketUrlReader( - { host: 'bitbucket.org', apiBaseUrl: 'https://api.bitbucket.org/2.0' }, - { treeResponseFactory }, - ); - - const response = await processor.readTree( + it('returns the wanted files from an archive', async () => { + const response = await bitbucketProcessor.readTree( 'https://bitbucket.org/backstage/mock/src/master', ); + expect(response.sha).toBe('12ab34cd56ef'); + const files = await response.files(); expect(files.length).toBe(2); @@ -98,38 +148,12 @@ describe('BitbucketUrlReader', () => { }); it('uses private bitbucket host', async () => { - const privateBitbucketRepoBuffer = fs.readFileSync( - path.resolve( - 'src', - 'reading', - '__fixtures__', - 'bitbucket-server-repo.zip', - ), - ); - worker.use( - rest.get( - 'https://api.bitbucket.mycompany.net/rest/api/1.0/projects/backstage/repos/mock/archive?format=zip&prefix=mock&path=docs', - (_, res, ctx) => - res( - ctx.status(200), - ctx.set('Content-Type', 'application/zip'), - ctx.body(privateBitbucketRepoBuffer), - ), - ), - ); - - const processor = new BitbucketUrlReader( - { - host: 'bitbucket.mycompany.net', - apiBaseUrl: 'https://api.bitbucket.mycompany.net/rest/api/1.0', - }, - { treeResponseFactory }, - ); - - const response = await processor.readTree( + const response = await hostedBitbucketProcessor.readTree( 'https://bitbucket.mycompany.net/projects/backstage/repos/mock/browse/docs?at=some-branch', ); + expect(response.sha).toBe('12ab34cd56ef'); + const files = await response.files(); expect(files.length).toBe(1); @@ -139,37 +163,12 @@ describe('BitbucketUrlReader', () => { }); it('returns the wanted files from an archive with a subpath', async () => { - worker.use( - rest.get( - 'https://bitbucket.org/backstage/mock/get/master.zip', - (_, res, ctx) => - res( - ctx.status(200), - ctx.set('Content-Type', 'application/zip'), - ctx.body(repoBuffer), - ), - ), - rest.get( - 'https://api.bitbucket.org/2.0/repositories/backstage/mock/commits/master', - (_, res, ctx) => - res( - ctx.status(200), - ctx.json({ - values: [{ hash: '12ab34cd56ef78gh90ij12kl34mn56op78qr90st' }], - }), - ), - ), - ); - - const processor = new BitbucketUrlReader( - { host: 'bitbucket.org', apiBaseUrl: 'https://api.bitbucket.org/2.0' }, - { treeResponseFactory }, - ); - - const response = await processor.readTree( + const response = await bitbucketProcessor.readTree( 'https://bitbucket.org/backstage/mock/src/master/docs', ); + expect(response.sha).toBe('12ab34cd56ef'); + const files = await response.files(); expect(files.length).toBe(1); @@ -177,5 +176,25 @@ describe('BitbucketUrlReader', () => { expect(indexMarkdownFile.toString()).toBe('# Test\n'); }); + + it('throws a NotModifiedError when given a sha in options', async () => { + const fnBitbucket = async () => { + await bitbucketProcessor.readTree( + 'https://bitbucket.org/backstage/mock', + { sha: '12ab34cd56ef' }, + ); + }; + + await expect(fnBitbucket).rejects.toThrow(NotModifiedError); + }); + + it('should not throw a NotModifiedError when given an outdated sha in options', async () => { + const response = await bitbucketProcessor.readTree( + 'https://bitbucket.org/backstage/mock', + { sha: 'outdatedSha123abc' }, + ); + + expect(response.sha).toBe('12ab34cd56ef'); + }); }); }); diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.ts b/packages/backend-common/src/reading/BitbucketUrlReader.ts index 7f462e5bf0..606aecd023 100644 --- a/packages/backend-common/src/reading/BitbucketUrlReader.ts +++ b/packages/backend-common/src/reading/BitbucketUrlReader.ts @@ -25,7 +25,7 @@ import { import fetch from 'cross-fetch'; import parseGitUrl from 'git-url-parse'; import { Readable } from 'stream'; -import { NotFoundError } from '../errors'; +import { NotFoundError, NotModifiedError } from '../errors'; import { ReadTreeResponseFactory } from './tree'; import { ReaderFactory, @@ -105,6 +105,11 @@ export class BitbucketUrlReader implements UrlReader { url, ); + const lastCommitShortHash = await this.getLastCommitShortHash(url); + if (options?.sha && options.sha === lastCommitShortHash) { + throw new NotModifiedError(); + } + const isHosted = resource === 'bitbucket.org'; const downloadUrl = await getBitbucketDownloadUrl(url, this.config); @@ -122,7 +127,6 @@ export class BitbucketUrlReader implements UrlReader { let folderPath = `${project}-${repoName}`; if (isHosted) { - const lastCommitShortHash = await this.getLastCommitShortHash(url); folderPath = `${project}-${repoName}-${lastCommitShortHash}`; } @@ -133,8 +137,7 @@ export class BitbucketUrlReader implements UrlReader { }); const response = archiveResponse as ReadTreeResponse; - // TODO: Just a placeholder for now. - response.sha = ''; + response.sha = lastCommitShortHash; return response; } @@ -147,7 +150,7 @@ export class BitbucketUrlReader implements UrlReader { return `bitbucket{host=${host},authed=${authed}}`; } - private async getLastCommitShortHash(url: string): Promise { + private async getLastCommitShortHash(url: string): Promise { const { name: repoName, owner: project, ref } = parseGitUrl(url); let branch = ref; diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 24a49cf81c..a311da910a 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -133,7 +133,6 @@ describe('GithubUrlReader', () => { }; beforeEach(() => { - // For github.com host worker.use( rest.get( 'https://github.com/backstage/mock/archive/main.tar.gz', @@ -144,9 +143,6 @@ describe('GithubUrlReader', () => { ctx.body(repoBuffer), ), ), - ); - - worker.use( rest.get('https://api.github.com/repos/backstage/mock', (_, res, ctx) => res( ctx.status(200), @@ -154,9 +150,6 @@ describe('GithubUrlReader', () => { ctx.json(reposGithubApiResponse), ), ), - ); - - worker.use( rest.get( 'https://api.github.com/repos/backstage/mock/branches/main', (_, res, ctx) => @@ -166,17 +159,10 @@ describe('GithubUrlReader', () => { ctx.json(branchesApiResponse), ), ), - ); - - worker.use( rest.get( 'https://api.github.com/repos/backstage/mock/branches/branchDoesNotExist', (_, res, ctx) => res(ctx.status(404)), ), - ); - - // For a GHE host - worker.use( rest.get( 'https://ghe.github.com/backstage/mock/archive/main.tar.gz', (_, res, ctx) => @@ -186,9 +172,6 @@ describe('GithubUrlReader', () => { ctx.body(repoBuffer), ), ), - ); - - worker.use( rest.get( 'https://ghe.github.com/api/v3/repos/backstage/mock', (_, res, ctx) => @@ -198,9 +181,6 @@ describe('GithubUrlReader', () => { ctx.json(reposGheApiResponse), ), ), - ); - - worker.use( rest.get( 'https://ghe.github.com/api/v3/repos/backstage/mock/branches/main', (_, res, ctx) => diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index a2dda48b16..823555d37d 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -179,9 +179,6 @@ describe('GitlabUrlReader', () => { ctx.body(archiveBuffer), ), ), - ); - - worker.use( rest.get( 'https://gitlab.com/api/v4/projects/backstage%2Fmock', (_, res, ctx) => @@ -191,9 +188,6 @@ describe('GitlabUrlReader', () => { ctx.json(projectGitlabApiResponse), ), ), - ); - - worker.use( rest.get( 'https://gitlab.com/api/v4/projects/backstage%2Fmock/repository/branches/main', (_, res, ctx) => @@ -203,16 +197,10 @@ describe('GitlabUrlReader', () => { ctx.json(branchGitlabApiResponse), ), ), - ); - - worker.use( rest.get( 'https://gitlab.com/api/v4/projects/backstage%2Fmock/repository/branches/branchDoesNotExist', (_, res, ctx) => res(ctx.status(404)), ), - ); - - worker.use( rest.get( 'https://gitlab.mycompany.com/api/v4/projects/backstage%2Fmock', (_, res, ctx) => @@ -222,9 +210,6 @@ describe('GitlabUrlReader', () => { ctx.json(projectGitlabApiResponse), ), ), - ); - - worker.use( rest.get( 'https://gitlab.mycompany.com/api/v4/projects/backstage%2Fmock/repository/branches/main', (_, res, ctx) => @@ -234,9 +219,6 @@ describe('GitlabUrlReader', () => { ctx.json(branchGitlabApiResponse), ), ), - ); - - worker.use( rest.get( 'https://gitlab.mycompany.com/api/v4/projects/backstage%2Fmock/repository/archive.zip?sha=main', (_, res, ctx) =>