backend-common: UrlReader/GitLab implement Sha based caching

Also use API to fetch archive.zip

packages/backend-common/src/reading/AzureUrlReader.test.ts

@@ -139,7 +139,7 @@ describe('AzureUrlReader', () => {
 
   describe('readTree', () => {
     const repoBuffer = fs.readFileSync(
-      path.resolve('src', 'reading', '__fixtures__', 'repo.zip'),
+      path.resolve('src', 'reading', '__fixtures__', 'mock-main.zip'),
     );
 
     beforeEach(() => {
@@ -169,8 +169,8 @@ describe('AzureUrlReader', () => {
       const files = await response.files();
 
       expect(files.length).toBe(2);
-      const mkDocsFile = await files[1].content();
-      const indexMarkdownFile = await files[0].content();
+      const mkDocsFile = await files[0].content();
+      const indexMarkdownFile = await files[1].content();
 
       expect(mkDocsFile.toString()).toBe('site_name: Test\n');
       expect(indexMarkdownFile.toString()).toBe('# Test\n');

packages/backend-common/src/reading/GithubUrlReader.test.ts

@@ -21,7 +21,7 @@ import fs from 'fs';
 import { rest } from 'msw';
 import { setupServer } from 'msw/node';
 import path from 'path';
-import { NotModifiedError } from '../errors';
+import { NotFoundError, NotModifiedError } from '../errors';
 import { GithubUrlReader } from './GithubUrlReader';
 import { ReadTreeResponseFactory } from './tree';
 
@@ -168,6 +168,13 @@ describe('GithubUrlReader', () => {
         ),
       );
 
+      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(
@@ -321,5 +328,14 @@ describe('GithubUrlReader', () => {
       );
       expect((await response.files()).length).toBe(2);
     });
+
+    it('should throw error on missing branch', async () => {
+      const fnGithub = async () => {
+        await githubProcessor.readTree(
+          'https://github.com/backstage/mock/tree/branchDoesNotExist',
+        );
+      };
+      await expect(fnGithub).rejects.toThrow(NotFoundError);
+    });
   });
 });

packages/backend-common/src/reading/GithubUrlReader.ts

@@ -119,7 +119,7 @@ export class GithubUrlReader implements UrlReader {
       },
     );
     if (!repoGitHubResponse.ok) {
-      const message = `Failed to read tree from ${url}, ${repoGitHubResponse.status} ${repoGitHubResponse.statusText}`;
+      const message = `Failed to read tree (repository) from ${url}, ${repoGitHubResponse.status} ${repoGitHubResponse.statusText}`;
       if (repoGitHubResponse.status === 404) {
         throw new NotFoundError(message);
       }
@@ -142,6 +142,13 @@ export class GithubUrlReader implements UrlReader {
         headers,
       },
     );
+    if (!branchGitHubResponse.ok) {
+      const message = `Failed to read tree (branch) from ${url}, ${branchGitHubResponse.status} ${branchGitHubResponse.statusText}`;
+      if (branchGitHubResponse.status === 404) {
+        throw new NotFoundError(message);
+      }
+      throw new Error(message);
+    }
     const commitSha = (await branchGitHubResponse.json()).commit.sha;
 
     if (options?.sha && options.sha === commitSha) {
@@ -162,6 +169,13 @@ export class GithubUrlReader implements UrlReader {
         headers,
       },
     );
+    if (!archive.ok) {
+      const message = `Failed to read tree (archive) from ${url}, ${archive.status} ${archive.statusText}`;
+      if (archive.status === 404) {
+        throw new NotFoundError(message);
+      }
+      throw new Error(message);
+    }
 
     const path = `${repoName}-${branch}/${filepath}`;
 

packages/backend-common/src/reading/GitlabUrlReader.test.ts

@@ -23,6 +23,7 @@ import path from 'path';
 import { getVoidLogger } from '../logging';
 import { GitlabUrlReader } from './GitlabUrlReader';
 import { ReadTreeResponseFactory } from './tree';
+import { NotModifiedError, NotFoundError } from '../errors';
 
 const logger = getVoidLogger();
 
@@ -30,6 +31,22 @@ const treeResponseFactory = ReadTreeResponseFactory.create({
   config: new ConfigReader({}),
 });
 
+const gitlabProcessor = new GitlabUrlReader(
+  {
+    host: 'gitlab.com',
+    apiBaseUrl: 'https://gitlab.com/api/v4',
+  },
+  { treeResponseFactory },
+);
+
+const hostedGitlabProcessor = new GitlabUrlReader(
+  {
+    host: 'gitlab.mycompany.com',
+    apiBaseUrl: 'https://gitlab.mycompany.com/api/v4',
+  },
+  { treeResponseFactory },
+);
+
 describe('GitlabUrlReader', () => {
   const worker = setupServer();
   msw.setupDefaultHandlers(worker);
@@ -136,39 +153,112 @@ describe('GitlabUrlReader', () => {
   });
 
   describe('readTree', () => {
-    const repoBuffer = fs.readFileSync(
-      path.resolve('src', 'reading', '__fixtures__', 'repo.zip'),
+    const archiveBuffer = fs.readFileSync(
+      path.resolve('src', 'reading', '__fixtures__', 'gitlab-archive.zip'),
     );
 
+    const projectGitlabApiResponse = {
+      id: 11111111,
+      default_branch: 'main',
+    };
+
+    const branchGitlabApiResponse = {
+      commit: {
+        id: 'sha123abc',
+      },
+    };
+
     beforeEach(() => {
       worker.use(
         rest.get(
-          'https://gitlab.com/backstage/mock/-/archive/repo/mock-repo.zip',
+          'https://gitlab.com/api/v4/projects/backstage%2Fmock/repository/archive.zip?sha=main',
           (_, res, ctx) =>
             res(
               ctx.status(200),
               ctx.set('Content-Type', 'application/zip'),
-              ctx.body(repoBuffer),
+              ctx.body(archiveBuffer),
+            ),
+        ),
+      );
+
+      worker.use(
+        rest.get(
+          'https://gitlab.com/api/v4/projects/backstage%2Fmock',
+          (_, res, ctx) =>
+            res(
+              ctx.status(200),
+              ctx.set('Content-Type', 'application/json'),
+              ctx.json(projectGitlabApiResponse),
+            ),
+        ),
+      );
+
+      worker.use(
+        rest.get(
+          'https://gitlab.com/api/v4/projects/backstage%2Fmock/repository/branches/main',
+          (_, res, ctx) =>
+            res(
+              ctx.status(200),
+              ctx.set('Content-Type', 'application/json'),
+              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) =>
+            res(
+              ctx.status(200),
+              ctx.set('Content-Type', 'application/json'),
+              ctx.json(projectGitlabApiResponse),
+            ),
+        ),
+      );
+
+      worker.use(
+        rest.get(
+          'https://gitlab.mycompany.com/api/v4/projects/backstage%2Fmock/repository/branches/main',
+          (_, res, ctx) =>
+            res(
+              ctx.status(200),
+              ctx.set('Content-Type', 'application/json'),
+              ctx.json(branchGitlabApiResponse),
+            ),
+        ),
+      );
+
+      worker.use(
+        rest.get(
+          'https://gitlab.mycompany.com/api/v4/projects/backstage%2Fmock/repository/archive.zip?sha=main',
+          (_, res, ctx) =>
+            res(
+              ctx.status(200),
+              ctx.set('Content-Type', 'application/zip'),
+              ctx.body(archiveBuffer),
             ),
         ),
       );
     });
 
     it('returns the wanted files from an archive', async () => {
-      const processor = new GitlabUrlReader(
-        { host: 'gitlab.com' },
-        { treeResponseFactory },
-      );
-
-      const response = await processor.readTree(
-        'https://gitlab.com/backstage/mock/tree/repo',
+      const response = await gitlabProcessor.readTree(
+        'https://gitlab.com/backstage/mock/tree/main',
       );
 
       const files = await response.files();
       expect(files.length).toBe(2);
 
-      const indexMarkdownFile = await files[0].content();
-      const mkDocsFile = await files[1].content();
+      const mkDocsFile = await files[0].content();
+      const indexMarkdownFile = await files[1].content();
 
       expect(mkDocsFile.toString()).toBe('site_name: Test\n');
       expect(indexMarkdownFile.toString()).toBe('# Test\n');
@@ -177,23 +267,18 @@ describe('GitlabUrlReader', () => {
     it('returns the wanted files from hosted gitlab', async () => {
       worker.use(
         rest.get(
-          'https://git.mycompany.com/backstage/mock/-/archive/repo/mock-repo.zip',
+          'https://gitlab.mycompany.com/backstage/mock/-/archive/main.zip',
           (_, res, ctx) =>
             res(
               ctx.status(200),
               ctx.set('Content-Type', 'application/zip'),
-              ctx.body(repoBuffer),
+              ctx.body(archiveBuffer),
             ),
         ),
       );
 
-      const processor = new GitlabUrlReader(
-        { host: 'git.mycompany.com' },
-        { treeResponseFactory },
-      );
-
-      const response = await processor.readTree(
-        'https://git.mycompany.com/backstage/mock/tree/repo/docs',
+      const response = await hostedGitlabProcessor.readTree(
+        'https://gitlab.mycompany.com/backstage/mock/tree/main/docs',
       );
 
       const files = await response.files();
@@ -204,27 +289,9 @@ describe('GitlabUrlReader', () => {
       expect(indexMarkdownFile.toString()).toBe('# Test\n');
     });
 
-    it('throws an error when branch is not specified', async () => {
-      const processor = new GitlabUrlReader(
-        { host: 'gitlab.com' },
-        { treeResponseFactory },
-      );
-
-      await expect(
-        processor.readTree('https://gitlab.com/backstage/mock'),
-      ).rejects.toThrow(
-        'GitLab URL must contain a branch to be able to fetch its tree',
-      );
-    });
-
     it('returns the wanted files from an archive with a subpath', async () => {
-      const processor = new GitlabUrlReader(
-        { host: 'gitlab.com' },
-        { treeResponseFactory },
-      );
-
-      const response = await processor.readTree(
-        'https://gitlab.com/backstage/mock/tree/repo/docs',
+      const response = await gitlabProcessor.readTree(
+        'https://gitlab.com/backstage/mock/tree/main/docs',
       );
 
       const files = await response.files();
@@ -234,5 +301,51 @@ describe('GitlabUrlReader', () => {
 
       expect(indexMarkdownFile.toString()).toBe('# Test\n');
     });
+
+    it('throws a NotModifiedError when given a sha in options', async () => {
+      const fnGitlab = async () => {
+        await gitlabProcessor.readTree('https://gitlab.com/backstage/mock', {
+          sha: 'sha123abc',
+        });
+      };
+
+      const fnHostedGitlab = async () => {
+        await hostedGitlabProcessor.readTree(
+          'https://gitlab.mycompany.com/backstage/mock',
+          {
+            sha: 'sha123abc',
+          },
+        );
+      };
+
+      await expect(fnGitlab).rejects.toThrow(NotModifiedError);
+      await expect(fnHostedGitlab).rejects.toThrow(NotModifiedError);
+    });
+
+    it('should not throw error when given an outdated sha in options', async () => {
+      const response = await gitlabProcessor.readTree(
+        'https://gitlab.com/backstage/mock/tree/main',
+        {
+          sha: 'outdatedSha123abc',
+        },
+      );
+      expect((await response.files()).length).toBe(2);
+    });
+
+    it('should detect the default branch', async () => {
+      const response = await gitlabProcessor.readTree(
+        'https://gitlab.com/backstage/mock',
+      );
+      expect((await response.files()).length).toBe(2);
+    });
+
+    it('should throw error on missing branch', async () => {
+      const fnGithub = async () => {
+        await gitlabProcessor.readTree(
+          'https://gitlab.com/backstage/mock/tree/branchDoesNotExist',
+        );
+      };
+      await expect(fnGithub).rejects.toThrow(NotFoundError);
+    });
   });
 });

packages/backend-common/src/reading/GitlabUrlReader.ts

@@ -21,7 +21,7 @@ import {
   readGitLabIntegrationConfigs,
 } from '@backstage/integration';
 import fetch from 'cross-fetch';
-import { InputError, NotFoundError } from '../errors';
+import { NotFoundError, NotModifiedError } from '../errors';
 import { ReadTreeResponseFactory } from './tree';
 import {
   ReaderFactory,
@@ -39,26 +39,26 @@ export class GitlabUrlReader implements UrlReader {
     const configs = readGitLabIntegrationConfigs(
       config.getOptionalConfigArray('integrations.gitlab') ?? [],
     );
-    return configs.map(options => {
-      const reader = new GitlabUrlReader(options, { treeResponseFactory });
-      const predicate = (url: URL) => url.host === options.host;
+    return configs.map(provider => {
+      const reader = new GitlabUrlReader(provider, { treeResponseFactory });
+      const predicate = (url: URL) => url.host === provider.host;
       return { reader, predicate };
     });
   };
 
   constructor(
-    private readonly options: GitLabIntegrationConfig,
+    private readonly config: GitLabIntegrationConfig,
     deps: { treeResponseFactory: ReadTreeResponseFactory },
   ) {
     this.treeResponseFactory = deps.treeResponseFactory;
   }
 
   async read(url: string): Promise<Buffer> {
-    const builtUrl = await getGitLabFileFetchUrl(url, this.options);
+    const builtUrl = await getGitLabFileFetchUrl(url, this.config);
 
     let response: Response;
     try {
-      response = await fetch(builtUrl, getGitLabRequestOptions(this.options));
+      response = await fetch(builtUrl, getGitLabRequestOptions(this.config));
     } catch (e) {
       throw new Error(`Unable to read ${url}, ${e}`);
     }
@@ -78,35 +78,71 @@ export class GitlabUrlReader implements UrlReader {
     url: string,
     options?: ReadTreeOptions,
   ): Promise<ReadTreeResponse> {
-    const {
-      name: repoName,
-      ref,
-      protocol,
-      resource,
-      full_name,
-      filepath,
-    } = parseGitUrl(url);
+    const { name: repoName, ref, full_name, filepath } = parseGitUrl(url);
 
-    if (!ref) {
-      throw new InputError(
-        'GitLab URL must contain a branch to be able to fetch its tree',
-      );
-    }
-
-    const archive = `${protocol}://${resource}/${full_name}/-/archive/${ref}/${repoName}-${ref}.zip`;
-    const archiveGitLabResponse = await fetch(
-      archive,
-      getGitLabRequestOptions(this.options),
+    // Use GitLab API to get the default branch
+    // encodeURIComponent is required for GitLab API
+    // https://docs.gitlab.com/ee/api/README.html#namespaced-path-encoding
+    const projectGitlabResponse = await fetch(
+      new URL(
+        `${this.config.apiBaseUrl}/projects/${encodeURIComponent(full_name)}`,
+      ).toString(),
+      getGitLabRequestOptions(this.config),
     );
-    if (!archiveGitLabResponse.ok) {
-      const msg = `Failed to read tree from ${url}, ${archiveGitLabResponse.status} ${archiveGitLabResponse.statusText}`;
-      if (archiveGitLabResponse.status === 404) {
+    if (!projectGitlabResponse.ok) {
+      const msg = `Failed to read tree from ${url}, ${projectGitlabResponse.status} ${projectGitlabResponse.statusText}`;
+      if (projectGitlabResponse.status === 404) {
         throw new NotFoundError(msg);
       }
       throw new Error(msg);
     }
+    const projectGitlabResponseJson = await projectGitlabResponse.json();
 
-    const path = filepath ? `${repoName}-${ref}/${filepath}/` : '';
+    // ref is an empty string if no branch is set in provided url to readTree.
+    const branch = ref === '' ? projectGitlabResponseJson.default_branch : ref;
+
+    // Fetch the latest commit in the provided or default branch to compare against
+    // the provided sha.
+    const branchGitlabResponse = await fetch(
+      new URL(
+        `${this.config.apiBaseUrl}/projects/${encodeURIComponent(
+          full_name,
+        )}/repository/branches/${branch}`,
+      ).toString(),
+      getGitLabRequestOptions(this.config),
+    );
+    if (!branchGitlabResponse.ok) {
+      const message = `Failed to read tree (branch) from ${url}, ${branchGitlabResponse.status} ${branchGitlabResponse.statusText}`;
+      if (branchGitlabResponse.status === 404) {
+        throw new NotFoundError(message);
+      }
+      throw new Error(message);
+    }
+
+    const commitSha = (await branchGitlabResponse.json()).commit.id;
+
+    if (options?.sha && options.sha === commitSha) {
+      throw new NotModifiedError();
+    }
+
+    // https://docs.gitlab.com/ee/api/repositories.html#get-file-archive
+    const archiveGitLabResponse = await fetch(
+      `${this.config.apiBaseUrl}/projects/${encodeURIComponent(
+        full_name,
+      )}/repository/archive.zip?sha=${branch}`,
+      getGitLabRequestOptions(this.config),
+    );
+    if (!archiveGitLabResponse.ok) {
+      const message = `Failed to read tree (archive) from ${url}, ${archiveGitLabResponse.status} ${archiveGitLabResponse.statusText}`;
+      if (archiveGitLabResponse.status === 404) {
+        throw new NotFoundError(message);
+      }
+      throw new Error(message);
+    }
+
+    const path = filepath
+      ? `${repoName}-${branch}-${commitSha}/${filepath}/`
+      : '';
 
     const archiveResponse = await this.treeResponseFactory.fromZipArchive({
       stream: (archiveGitLabResponse.body as unknown) as Readable,
@@ -115,13 +151,12 @@ export class GitlabUrlReader implements UrlReader {
     });
 
     const response = archiveResponse as ReadTreeResponse;
-    // TODO: Just a placeholder for now.
-    response.sha = '';
+    response.sha = commitSha;
     return response;
   }
 
   toString() {
-    const { host, token } = this.options;
+    const { host, token } = this.config;
     return `gitlab{host=${host},authed=${Boolean(token)}}`;
   }
 }

packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts

@@ -20,7 +20,7 @@ import { resolve as resolvePath } from 'path';
 import { ZipArchiveResponse } from './ZipArchiveResponse';
 
 const archiveData = fs.readFileSync(
-  resolvePath(__filename, '../../__fixtures__/repo.zip'),
+  resolvePath(__filename, '../../__fixtures__/mock-main.zip'),
 );
 
 describe('ZipArchiveResponse', () => {
@@ -38,30 +38,30 @@ describe('ZipArchiveResponse', () => {
   it('should read files', async () => {
     const stream = fs.createReadStream('/test-archive.zip');
 
-    const res = new ZipArchiveResponse(stream, 'mock-repo/', '/tmp');
+    const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp');
     const files = await res.files();
 
     expect(files).toEqual([
       {
-        path: 'docs/index.md',
+        path: 'mkdocs.yml',
         content: expect.any(Function),
       },
       {
-        path: 'mkdocs.yml',
+        path: 'docs/index.md',
         content: expect.any(Function),
       },
     ]);
     const contents = await Promise.all(files.map(f => f.content()));
     expect(contents.map(c => c.toString('utf8').trim())).toEqual([
-      '# Test',
       'site_name: Test',
+      '# Test',
     ]);
   });
 
   it('should read files with filter', async () => {
     const stream = fs.createReadStream('/test-archive.zip');
 
-    const res = new ZipArchiveResponse(stream, 'mock-repo/', '/tmp', path =>
+    const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', path =>
       path.endsWith('.yml'),
     );
     const files = await res.files();
@@ -79,7 +79,7 @@ describe('ZipArchiveResponse', () => {
   it('should read as archive and files', async () => {
     const stream = fs.createReadStream('/test-archive.zip');
 
-    const res = new ZipArchiveResponse(stream, 'mock-repo/', '/tmp');
+    const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp');
     const buffer = await res.archive();
 
     await expect(res.archive()).rejects.toThrow(
@@ -91,18 +91,18 @@ describe('ZipArchiveResponse', () => {
 
     expect(files).toEqual([
       {
-        path: 'docs/index.md',
+        path: 'mkdocs.yml',
         content: expect.any(Function),
       },
       {
-        path: 'mkdocs.yml',
+        path: 'docs/index.md',
         content: expect.any(Function),
       },
     ]);
     const contents = await Promise.all(files.map(f => f.content()));
     expect(contents.map(c => c.toString('utf8').trim())).toEqual([
-      '# Test',
       'site_name: Test',
+      '# Test',
     ]);
   });
 
@@ -113,17 +113,17 @@ describe('ZipArchiveResponse', () => {
     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.zip');
 
-    const res = new ZipArchiveResponse(stream, 'mock-repo/docs/', '/tmp');
+    const res = new ZipArchiveResponse(stream, 'mock-main/docs/', '/tmp');
     const dir = await res.dir();
 
     expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/);
@@ -135,7 +135,7 @@ 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-repo/', '/tmp', path =>
+    const res = new ZipArchiveResponse(stream, 'mock-main/', '/tmp', path =>
       path.endsWith('.yml'),
     );
     const dir = await res.dir({ targetDir: '/tmp' });

packages/backend-common/src/reading/types.ts

@@ -83,6 +83,10 @@ export type ReadTreeResponseDirOptions = {
 export type ReadTreeArchiveResponse = {
   files(): Promise<ReadTreeResponseFile[]>;
   archive(): Promise<NodeJS.ReadableStream>;
+
+  /**
+   * dir() extracts the tree response into a directory and returns the path of the directory.
+   */
   dir(options?: ReadTreeResponseDirOptions): Promise<string>;
 };