Merge pull request #16948 from Andy2003/feature/gitlab-url-loader-imrpove-search

Improve GitlabUrlReader to only load requested sub-path

.changeset/ten-hounds-bathe.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-common': patch
+---
+
+Improve GitlabUrlReader to only load requested sub-path

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(

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<SearchResponse> {
     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)}}`;