From 420164593cfdc8459b2138b911b25cb81c294aa6 Mon Sep 17 00:00:00 2001 From: Andreas Berger Date: Fri, 17 Mar 2023 19:56:04 +0100 Subject: [PATCH] Improve GitlabUrlReader to only load requested sub-path Signed-off-by: Andreas Berger --- .changeset/ten-hounds-bathe.md | 5 ++ .../src/reading/GitlabUrlReader.test.ts | 50 +++++++++++++++-- .../src/reading/GitlabUrlReader.ts | 53 +++++++++++++----- .../gitlab-subpath-archive.tar.gz | Bin 0 -> 346 bytes 4 files changed, 90 insertions(+), 18 deletions(-) create mode 100644 .changeset/ten-hounds-bathe.md create mode 100644 packages/backend-common/src/reading/__fixtures__/gitlab-subpath-archive.tar.gz diff --git a/.changeset/ten-hounds-bathe.md b/.changeset/ten-hounds-bathe.md new file mode 100644 index 0000000000..85bda89138 --- /dev/null +++ b/.changeset/ten-hounds-bathe.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Improve GitlabUrlReader to only load requested sub-path diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts index d926b2be9b..5af4faf7e5 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.test.ts @@ -556,6 +556,10 @@ describe('GitlabUrlReader', () => { path.resolve(__dirname, '__fixtures__/gitlab-archive.tar.gz'), ); + const archiveSubPathBuffer = fs.readFileSync( + path.resolve(__dirname, '__fixtures__/gitlab-subpath-archive.tar.gz'), + ); + const projectGitlabApiResponse = { id: 11111111, default_branch: 'main', @@ -566,21 +570,34 @@ describe('GitlabUrlReader', () => { id: 'sha123abc', }, ]; + const commitsOfSubPathGitlabApiResponse = [ + { + id: 'sha456abc', + }, + ]; beforeEach(() => { worker.use( rest.get( 'https://gitlab.com/api/v4/projects/backstage%2Fmock/repository/archive', - (_, res, ctx) => - res( + (req, res, ctx) => { + const filepath = req.url.searchParams.get('path'); + let filename = 'mock-main-sha123abc.zip'; + let body = archiveBuffer; + if (filepath === 'docs') { + filename = 'gitlab-subpath-archive.tar.gz'; + body = archiveSubPathBuffer; + } + return res( ctx.status(200), ctx.set('Content-Type', 'application/zip'), ctx.set( 'content-disposition', - 'attachment; filename="mock-main-sha123abc.zip"', + `attachment; filename="${filename}"`, ), - ctx.body(archiveBuffer), - ), + ctx.body(body), + ); + }, ), rest.get( 'https://gitlab.com/api/v4/projects/backstage%2Fmock', @@ -596,6 +613,14 @@ describe('GitlabUrlReader', () => { (req, res, ctx) => { const refName = req.url.searchParams.get('ref_name'); if (refName === 'main') { + const filepath = req.url.searchParams.get('path'); + if (filepath === 'docs') { + return res( + ctx.status(200), + ctx.set('Content-Type', 'application/json'), + ctx.json(commitsOfSubPathGitlabApiResponse), + ); + } return res( ctx.status(200), ctx.set('Content-Type', 'application/json'), @@ -622,6 +647,21 @@ describe('GitlabUrlReader', () => { ); }); + it('load only relevant path', async () => { + const result = await gitlabProcessor.search( + 'https://gitlab.com/backstage/mock/tree/main/docs/**/index.*', + ); + + expect(result.etag).toBe('sha456abc'); + expect(result.files.length).toBe(1); + expect(result.files[0].url).toBe( + 'https://gitlab.com/backstage/mock/tree/main/docs/index.md', + ); + await expect(result.files[0].content()).resolves.toEqual( + Buffer.from('# Test Subpath\n'), + ); + }); + it('throws NotModifiedError when same etag', async () => { await expect( gitlabProcessor.search( diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts index df8441c1f7..ab59c19b16 100644 --- a/packages/backend-common/src/reading/GitlabUrlReader.ts +++ b/packages/backend-common/src/reading/GitlabUrlReader.ts @@ -26,7 +26,6 @@ import parseGitUrl from 'git-url-parse'; import { Minimatch } from 'minimatch'; import { Readable } from 'stream'; import { NotFoundError, NotModifiedError } from '@backstage/errors'; -import { stripFirstDirectoryFromPath } from './tree/util'; import { ReadTreeResponseFactory, ReaderFactory, @@ -163,7 +162,7 @@ export class GitlabUrlReader implements UrlReader { // ref is an empty string if no branch is set in provided url to readTree. const branch = ref || projectGitlabResponseJson.default_branch; - // Fetch the latest commit that modifies the the filepath in the provided or default branch + // Fetch the latest commit that modifies the filepath in the provided or default branch // to compare against the provided sha. const commitsReqParams = new URLSearchParams(); commitsReqParams.set('ref_name', branch); @@ -200,11 +199,16 @@ export class GitlabUrlReader implements UrlReader { throw new NotModifiedError(); } + const archiveReqParams = new URLSearchParams(); + archiveReqParams.set('sha', branch); + if (!!filepath) { + archiveReqParams.set('path', filepath); + } // https://docs.gitlab.com/ee/api/repositories.html#get-file-archive const archiveGitLabResponse = await fetch( `${this.integration.config.apiBaseUrl}/projects/${encodeURIComponent( repoFullName, - )}/repository/archive?sha=${branch}`, + )}/repository/archive?${archiveReqParams.toString()}`, { ...getGitLabRequestOptions(this.integration.config), // TODO(freben): The signal cast is there because pre-3.x versions of @@ -234,31 +238,54 @@ export class GitlabUrlReader implements UrlReader { async search(url: string, options?: SearchOptions): Promise { const { filepath } = parseGitUrl(url); + const staticPart = this.getStaticPart(filepath); const matcher = new Minimatch(filepath); - - // TODO(freben): For now, read the entire repo and filter through that. In - // a future improvement, we could be smart and try to deduce that non-glob - // prefixes (like for filepaths such as some-prefix/**/a.yaml) can be used - // to get just that part of the repo. - const treeUrl = trimEnd(url.replace(filepath, ''), '/'); - + const treeUrl = trimEnd(url.replace(filepath, staticPart), `/`); + const pathPrefix = staticPart ? `${staticPart}/` : ''; const tree = await this.readTree(treeUrl, { etag: options?.etag, signal: options?.signal, - filter: path => matcher.match(stripFirstDirectoryFromPath(path)), + filter: path => matcher.match(`${pathPrefix}${path}`), }); - const files = await tree.files(); + const files = await tree.files(); return { etag: tree.etag, files: files.map(file => ({ - url: this.integration.resolveUrl({ url: `/${file.path}`, base: url }), + url: this.integration.resolveUrl({ + url: `/${pathPrefix}${file.path}`, + base: url, + }), content: file.content, lastModifiedAt: file.lastModifiedAt, })), }; } + /** + * This function splits the input globPattern string into segments using the path separator /. It then iterates over + * the segments from the end of the array towards the beginning, checking if the concatenated string up to that + * segment matches the original globPattern using the minimatch function. If a match is found, it continues iterating. + * If no match is found, it returns the concatenated string up to the current segment, which is the static part of the + * glob pattern. + * + * E.g. `catalog/foo/*.yaml` will return `catalog/foo`. + * + * @param globPattern the glob pattern + * @private + */ + private getStaticPart(globPattern: string) { + const segments = globPattern.split('/'); + let i = segments.length; + while ( + i > 0 && + new Minimatch(segments.slice(0, i).join('/')).match(globPattern) + ) { + i--; + } + return segments.slice(0, i).join('/'); + } + toString() { const { host, token } = this.integration.config; return `gitlab{host=${host},authed=${Boolean(token)}}`; diff --git a/packages/backend-common/src/reading/__fixtures__/gitlab-subpath-archive.tar.gz b/packages/backend-common/src/reading/__fixtures__/gitlab-subpath-archive.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..c41714b4527dd224ef71cd80af75c1785cfb64ee GIT binary patch literal 346 zcmV-g0j2&QiwFRBwG?Ck1MQYwYlAQphQ02u2<&?8iJp_J7rWnOVfzEA5p`^K_%UR^ ze$mc;$hsm#M==jV2!c7u;d$dm)lZSqQND`P@=a~;RURhkXXIG~03;FfWXo}cjzBPu zAOwO$@)>X;2|QNYEaDPXQL5aO@|tFGn5f