From 2305296073b4a32ca096ceba46dd49c66173385f Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Sat, 16 Jan 2021 22:10:43 +0100
Subject: [PATCH 01/11] backend-common: Support SHA based caching in URL
 Reader's readTree

readTree now takes a SHA in options, and throws a NotModifiedError if the SHA matches with the latest commit SHA on the tree. readTree response now contains a new 'sha' parameter.

Also, the default branch will now be used if no branch is provided in the url's target
---
 catalog-info.yaml                             |   2 +-
 packages/backend-common/src/errors.ts         |   5 +
 .../src/reading/AzureUrlReader.ts             |  17 +-
 .../src/reading/BitbucketUrlReader.ts         |  17 +-
 .../src/reading/GithubUrlReader.test.ts       | 272 +++++++++++-------
 .../src/reading/GithubUrlReader.ts            |  81 ++++--
 .../src/reading/GitlabUrlReader.ts            |  17 +-
 .../src/reading/UrlReaderPredicateMux.ts      |   7 +-
 .../src/reading/__fixtures__/mock-main.tar.gz | Bin 0 -> 230 bytes
 .../src/reading/__fixtures__/repo.tar.gz      | Bin 232 -> 0 bytes
 .../reading/tree/ReadTreeResponseFactory.ts   |  10 +-
 .../reading/tree/TarArchiveResponse.test.ts   |  16 +-
 .../src/reading/tree/TarArchiveResponse.ts    |   4 +-
 .../src/reading/tree/ZipArchiveResponse.ts    |   4 +-
 packages/backend-common/src/reading/types.ts  |  19 +-
 packages/techdocs-common/src/helpers.test.ts  |   1 +
 16 files changed, 305 insertions(+), 167 deletions(-)
 create mode 100644 packages/backend-common/src/reading/__fixtures__/mock-main.tar.gz
 delete mode 100644 packages/backend-common/src/reading/__fixtures__/repo.tar.gz

diff --git a/catalog-info.yaml b/catalog-info.yaml
index 2144d1709c..7e60af5755 100644
--- a/catalog-info.yaml
+++ b/catalog-info.yaml
@@ -6,7 +6,7 @@ metadata:
     Backstage is an open-source developer portal that puts the developer experience first.
   annotations:
     github.com/project-slug: backstage/backstage
-    backstage.io/techdocs-ref: url:https://github.com/backstage/backstage/tree/master
+    backstage.io/techdocs-ref: url:https://github.com/backstage/backstage
     lighthouse.com/website-url: https://backstage.io
 spec:
   type: library
diff --git a/packages/backend-common/src/errors.ts b/packages/backend-common/src/errors.ts
index b68dc8e3f0..39703127e6 100644
--- a/packages/backend-common/src/errors.ts
+++ b/packages/backend-common/src/errors.ts
@@ -75,3 +75,8 @@ export class NotFoundError extends CustomErrorBase {}
  * resource.
  */
 export class ConflictError extends CustomErrorBase {}
+
+/**
+ * The requested resource has not changed since last request.
+ */
+export class NotModifiedError extends CustomErrorBase {}
diff --git a/packages/backend-common/src/reading/AzureUrlReader.ts b/packages/backend-common/src/reading/AzureUrlReader.ts
index ea716a1d4a..4535b329d7 100644
--- a/packages/backend-common/src/reading/AzureUrlReader.ts
+++ b/packages/backend-common/src/reading/AzureUrlReader.ts
@@ -75,22 +75,27 @@ export class AzureUrlReader implements UrlReader {
     url: string,
     options?: ReadTreeOptions,
   ): Promise<ReadTreeResponse> {
-    const response = await fetch(
+    const archiveAzureResponse = await fetch(
       getAzureDownloadUrl(url),
       getAzureRequestOptions(this.options, { Accept: 'application/zip' }),
     );
-    if (!response.ok) {
-      const message = `Failed to read tree from ${url}, ${response.status} ${response.statusText}`;
-      if (response.status === 404) {
+    if (!archiveAzureResponse.ok) {
+      const message = `Failed to read tree from ${url}, ${archiveAzureResponse.status} ${archiveAzureResponse.statusText}`;
+      if (archiveAzureResponse.status === 404) {
         throw new NotFoundError(message);
       }
       throw new Error(message);
     }
 
-    return this.deps.treeResponseFactory.fromZipArchive({
-      stream: (response.body as unknown) as Readable,
+    const archiveResponse = await this.deps.treeResponseFactory.fromZipArchive({
+      stream: (archiveAzureResponse.body as unknown) as Readable,
       filter: options?.filter,
     });
+
+    const response = archiveResponse as ReadTreeResponse;
+    // TODO: Just a placeholder for now.
+    response.sha = '';
+    return response;
   }
 
   toString() {
diff --git a/packages/backend-common/src/reading/BitbucketUrlReader.ts b/packages/backend-common/src/reading/BitbucketUrlReader.ts
index 4578436521..7f462e5bf0 100644
--- a/packages/backend-common/src/reading/BitbucketUrlReader.ts
+++ b/packages/backend-common/src/reading/BitbucketUrlReader.ts
@@ -108,13 +108,13 @@ export class BitbucketUrlReader implements UrlReader {
     const isHosted = resource === 'bitbucket.org';
 
     const downloadUrl = await getBitbucketDownloadUrl(url, this.config);
-    const response = await fetch(
+    const archiveBitbucketResponse = await fetch(
       downloadUrl,
       getBitbucketRequestOptions(this.config),
     );
-    if (!response.ok) {
-      const message = `Failed to read tree from ${url}, ${response.status} ${response.statusText}`;
-      if (response.status === 404) {
+    if (!archiveBitbucketResponse.ok) {
+      const message = `Failed to read tree from ${url}, ${archiveBitbucketResponse.status} ${archiveBitbucketResponse.statusText}`;
+      if (archiveBitbucketResponse.status === 404) {
         throw new NotFoundError(message);
       }
       throw new Error(message);
@@ -126,11 +126,16 @@ export class BitbucketUrlReader implements UrlReader {
       folderPath = `${project}-${repoName}-${lastCommitShortHash}`;
     }
 
-    return this.treeResponseFactory.fromZipArchive({
-      stream: (response.body as unknown) as Readable,
+    const archiveResponse = await this.treeResponseFactory.fromZipArchive({
+      stream: (archiveBitbucketResponse.body as unknown) as Readable,
       path: `${folderPath}/${filepath}`,
       filter: options?.filter,
     });
+
+    const response = archiveResponse as ReadTreeResponse;
+    // TODO: Just a placeholder for now.
+    response.sha = '';
+    return response;
   }
 
   toString() {
diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts
index 1a0f27d28f..6fdab9dfa1 100644
--- a/packages/backend-common/src/reading/GithubUrlReader.test.ts
+++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts
@@ -21,6 +21,7 @@ import fs from 'fs';
 import { rest } from 'msw';
 import { setupServer } from 'msw/node';
 import path from 'path';
+import { NotModifiedError } from '../errors';
 import { GithubUrlReader } from './GithubUrlReader';
 import { ReadTreeResponseFactory } from './tree';
 
@@ -28,11 +29,27 @@ const treeResponseFactory = ReadTreeResponseFactory.create({
   config: new ConfigReader({}),
 });
 
-describe('GithubUrlReader', () => {
-  const mockCredentialsProvider = ({
-    getCredentials: jest.fn().mockResolvedValue({ headers: {} }),
-  } as unknown) as GithubCredentialsProvider;
+const mockCredentialsProvider = ({
+  getCredentials: jest.fn().mockResolvedValue({ headers: {} }),
+} as unknown) as GithubCredentialsProvider;
 
+const githubProcessor = new GithubUrlReader(
+  {
+    host: 'github.com',
+    apiBaseUrl: 'https://api.github.com',
+  },
+  { treeResponseFactory, credentialsProvider: mockCredentialsProvider },
+);
+
+const gheProcessor = new GithubUrlReader(
+  {
+    host: 'ghe.github.com',
+    apiBaseUrl: 'https://ghe.github.com/api/v3',
+  },
+  { treeResponseFactory, credentialsProvider: mockCredentialsProvider },
+);
+
+describe('GithubUrlReader', () => {
   const worker = setupServer();
 
   msw.setupDefaultHandlers(worker);
@@ -43,15 +60,8 @@ describe('GithubUrlReader', () => {
 
   describe('implementation', () => {
     it('rejects unknown targets', async () => {
-      const processor = new GithubUrlReader(
-        {
-          host: 'github.com',
-          apiBaseUrl: 'https://api.github.com',
-        },
-        { treeResponseFactory, credentialsProvider: mockCredentialsProvider },
-      );
       await expect(
-        processor.read('https://not.github.com/apa'),
+        githubProcessor.read('https://not.github.com/apa'),
       ).rejects.toThrow(
         'Incorrect URL: https://not.github.com/apa, Error: Invalid GitHub URL or file path',
       );
@@ -73,7 +83,7 @@ describe('GithubUrlReader', () => {
 
       worker.use(
         rest.get(
-          'https://api.github.com/repos/backstage/mock/tree/contents/?ref=repo',
+          'https://api.github.com/repos/backstage/mock/tree/contents/?ref=main',
           (req, res, ctx) => {
             expect(req.headers.get('authorization')).toBe(
               mockHeaders.Authorization,
@@ -90,28 +100,43 @@ describe('GithubUrlReader', () => {
         ),
       );
 
-      const processor = new GithubUrlReader(
-        {
-          host: 'ghe.github.com',
-          apiBaseUrl: 'https://api.github.com',
-        },
-        { treeResponseFactory, credentialsProvider: mockCredentialsProvider },
-      );
-      await processor.read(
-        'https://ghe.github.com/backstage/mock/tree/blob/repo',
+      await githubProcessor.read(
+        'https://github.com/backstage/mock/tree/blob/main',
       );
     });
   });
 
   describe('readTree', () => {
     const repoBuffer = fs.readFileSync(
-      path.resolve('src', 'reading', '__fixtures__', 'repo.tar.gz'),
+      path.resolve('src', 'reading', '__fixtures__', 'mock-main.tar.gz'),
     );
 
+    const reposGithubApiResponse = {
+      id: '123',
+      full_name: 'backstage/mock',
+      default_branch: 'main',
+      branches_url:
+        'https://api.github.com/repos/backstage/mock/branches{/branch}',
+    };
+
+    const reposGheApiResponse = {
+      ...reposGithubApiResponse,
+      branches_url:
+        'https://ghe.github.com/api/v3/repos/backstage/mock/branches{/branch}',
+    };
+
+    const branchesApiResponse = {
+      name: 'main',
+      commit: {
+        sha: 'sha123abc',
+      },
+    };
+
     beforeEach(() => {
+      // For github.com host
       worker.use(
         rest.get(
-          'https://github.com/backstage/mock/archive/repo.tar.gz',
+          'https://github.com/backstage/mock/archive/main.tar.gz',
           (_, res, ctx) =>
             res(
               ctx.status(200),
@@ -120,20 +145,73 @@ describe('GithubUrlReader', () => {
             ),
         ),
       );
+
+      worker.use(
+        rest.get('https://api.github.com/repos/backstage/mock', (_, res, ctx) =>
+          res(
+            ctx.status(200),
+            ctx.set('Content-Type', 'application/json'),
+            ctx.json(reposGithubApiResponse),
+          ),
+        ),
+      );
+
+      worker.use(
+        rest.get(
+          'https://api.github.com/repos/backstage/mock/branches/main',
+          (_, res, ctx) =>
+            res(
+              ctx.status(200),
+              ctx.set('Content-Type', 'application/json'),
+              ctx.json(branchesApiResponse),
+            ),
+        ),
+      );
+
+      // For a GHE host
+      worker.use(
+        rest.get(
+          'https://ghe.github.com/backstage/mock/archive/main.tar.gz',
+          (_, res, ctx) =>
+            res(
+              ctx.status(200),
+              ctx.set('Content-Type', 'application/x-gzip'),
+              ctx.body(repoBuffer),
+            ),
+        ),
+      );
+
+      worker.use(
+        rest.get(
+          'https://ghe.github.com/api/v3/repos/backstage/mock',
+          (_, res, ctx) =>
+            res(
+              ctx.status(200),
+              ctx.set('Content-Type', 'application/json'),
+              ctx.json(reposGheApiResponse),
+            ),
+        ),
+      );
+
+      worker.use(
+        rest.get(
+          'https://ghe.github.com/api/v3/repos/backstage/mock/branches/main',
+          (_, res, ctx) =>
+            res(
+              ctx.status(200),
+              ctx.set('Content-Type', 'application/json'),
+              ctx.json(branchesApiResponse),
+            ),
+        ),
+      );
     });
 
     it('returns the wanted files from an archive', async () => {
-      const processor = new GithubUrlReader(
-        {
-          host: 'github.com',
-          apiBaseUrl: 'https://api.github.com',
-        },
-        { treeResponseFactory, credentialsProvider: mockCredentialsProvider },
+      const response = await githubProcessor.readTree(
+        'https://github.com/backstage/mock/tree/main',
       );
 
-      const response = await processor.readTree(
-        'https://github.com/backstage/mock/tree/repo',
-      );
+      expect(response.sha).toBe('sha123abc');
 
       const files = await response.files();
 
@@ -145,40 +223,6 @@ describe('GithubUrlReader', () => {
       expect(indexMarkdownFile.toString()).toBe('# Test\n');
     });
 
-    it('includes the subdomain in the github url', async () => {
-      worker.resetHandlers();
-      worker.use(
-        rest.get(
-          'https://ghe.github.com/backstage/mock/archive/repo.tar.gz',
-          (_, res, ctx) =>
-            res(
-              ctx.status(200),
-              ctx.set('Content-Type', 'application/x-gzip'),
-              ctx.body(repoBuffer),
-            ),
-        ),
-      );
-
-      const processor = new GithubUrlReader(
-        {
-          host: 'ghe.github.com',
-          apiBaseUrl: 'https://api.github.com',
-        },
-        { treeResponseFactory, credentialsProvider: mockCredentialsProvider },
-      );
-
-      const response = await processor.readTree(
-        'https://ghe.github.com/backstage/mock/tree/repo/docs',
-      );
-
-      const files = await response.files();
-
-      expect(files.length).toBe(1);
-      const indexMarkdownFile = await files[0].content();
-
-      expect(indexMarkdownFile.toString()).toBe('# Test\n');
-    });
-
     it('should use the headers from the credentials provider to the fetch request', async () => {
       expect.assertions(2);
 
@@ -193,7 +237,7 @@ describe('GithubUrlReader', () => {
 
       worker.use(
         rest.get(
-          'https://ghe.github.com/backstage/mock/archive/repo.tar.gz',
+          'https://ghe.github.com/backstage/mock/archive/main.tar.gz',
           (req, res, ctx) => {
             expect(req.headers.get('authorization')).toBe(
               mockHeaders.Authorization,
@@ -210,46 +254,14 @@ describe('GithubUrlReader', () => {
         ),
       );
 
-      const processor = new GithubUrlReader(
-        {
-          host: 'ghe.github.com',
-          apiBaseUrl: 'https://api.github.com',
-        },
-        { treeResponseFactory, credentialsProvider: mockCredentialsProvider },
-      );
-
-      await processor.readTree(
-        'https://ghe.github.com/backstage/mock/tree/repo/docs',
+      await gheProcessor.readTree(
+        'https://ghe.github.com/backstage/mock/tree/main',
       );
     });
 
-    it('must specify a branch', async () => {
-      const processor = new GithubUrlReader(
-        {
-          host: 'github.com',
-          apiBaseUrl: 'https://api.github.com',
-        },
-        { treeResponseFactory, credentialsProvider: mockCredentialsProvider },
-      );
-
-      await expect(
-        processor.readTree('https://github.com/backstage/mock'),
-      ).rejects.toThrow(
-        'GitHub URL must contain branch to be able to fetch tree',
-      );
-    });
-
-    it('returns the wanted files from an archive with a subpath', async () => {
-      const processor = new GithubUrlReader(
-        {
-          host: 'github.com',
-          apiBaseUrl: 'https://api.github.com',
-        },
-        { treeResponseFactory, credentialsProvider: mockCredentialsProvider },
-      );
-
-      const response = await processor.readTree(
-        'https://github.com/backstage/mock/tree/repo/docs',
+    it('includes the subdomain in the github url', async () => {
+      const response = await gheProcessor.readTree(
+        'https://ghe.github.com/backstage/mock/tree/main/docs',
       );
 
       const files = await response.files();
@@ -259,5 +271,55 @@ describe('GithubUrlReader', () => {
 
       expect(indexMarkdownFile.toString()).toBe('# Test\n');
     });
+
+    it('returns the wanted files from an archive with a subpath', async () => {
+      const response = await githubProcessor.readTree(
+        'https://github.com/backstage/mock/tree/main/docs',
+      );
+
+      const files = await response.files();
+
+      expect(files.length).toBe(1);
+      const indexMarkdownFile = await files[0].content();
+
+      expect(indexMarkdownFile.toString()).toBe('# Test\n');
+    });
+
+    it('throws a NotModifiedError when given a sha in options', async () => {
+      const fnGithub = async () => {
+        await githubProcessor.readTree('https://github.com/backstage/mock', {
+          sha: 'sha123abc',
+        });
+      };
+
+      const fnGhe = async () => {
+        await gheProcessor.readTree(
+          'https://ghe.github.com/backstage/mock/tree/main/docs',
+          {
+            sha: 'sha123abc',
+          },
+        );
+      };
+
+      await expect(fnGithub).rejects.toThrow(NotModifiedError);
+      await expect(fnGhe).rejects.toThrow(NotModifiedError);
+    });
+
+    it('should not throw error when given an outdated sha in options', async () => {
+      const response = await githubProcessor.readTree(
+        'https://github.com/backstage/mock/tree/main',
+        {
+          sha: 'outdatedSha123abc',
+        },
+      );
+      expect((await response.files()).length).toBe(2);
+    });
+
+    it('should detect the default branch', async () => {
+      const response = await githubProcessor.readTree(
+        'https://github.com/backstage/mock',
+      );
+      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 45ce8336e6..2acca10d4f 100644
--- a/packages/backend-common/src/reading/GithubUrlReader.ts
+++ b/packages/backend-common/src/reading/GithubUrlReader.ts
@@ -23,7 +23,7 @@ import {
 import fetch from 'cross-fetch';
 import parseGitUrl from 'git-url-parse';
 import { Readable } from 'stream';
-import { InputError, NotFoundError } from '../errors';
+import { NotFoundError, NotModifiedError } from '../errors';
 import { ReadTreeResponseFactory } from './tree';
 import {
   ReaderFactory,
@@ -99,52 +99,83 @@ export class GithubUrlReader implements UrlReader {
     options?: ReadTreeOptions,
   ): Promise<ReadTreeResponse> {
     const {
-      name: repoName,
-      ref,
       protocol,
       resource,
-      full_name,
+      name: repoName,
+      ref,
       filepath,
+      full_name,
     } = parseGitUrl(url);
 
-    if (!ref) {
-      // TODO(Rugvip): We should add support for defaulting to the default branch
-      throw new InputError(
-        'GitHub URL must contain branch to be able to fetch tree',
-      );
-    }
-
     const { headers } = await this.deps.credentialsProvider.getCredentials({
       url,
     });
-    // TODO(Rugvip): use API to fetch URL instead
-    const response = await fetch(
-      new URL(
-        `${protocol}://${resource}/${full_name}/archive/${ref}.tar.gz`,
-      ).toString(),
+
+    // Get GitHub API urls for the repository
+    const repoGitHubResponse = await fetch(
+      new URL(`${this.config.apiBaseUrl}/repos/${full_name}`).toString(),
       {
-        headers: {
-          ...headers,
-        },
+        headers,
       },
     );
-    if (!response.ok) {
-      const message = `Failed to read tree from ${url}, ${response.status} ${response.statusText}`;
-      if (response.status === 404) {
+    if (!repoGitHubResponse.ok) {
+      const message = `Failed to read tree from ${url}, ${repoGitHubResponse.status} ${repoGitHubResponse.statusText}`;
+      if (repoGitHubResponse.status === 404) {
         throw new NotFoundError(message);
       }
       throw new Error(message);
     }
 
-    const path = `${repoName}-${ref}/${filepath}`;
+    const repoResponseJson = await repoGitHubResponse.json();
 
-    return this.deps.treeResponseFactory.fromTarArchive({
+    // ref is an empty string if no branch is set in provided url to readTree.
+    // Use GitHub API to get the default branch of the repository.
+    const branch = ref === '' ? repoResponseJson.default_branch : ref;
+    const branchesApiUrl = repoResponseJson.branches_url;
+
+    // Fetch the latest commit in the provided or default branch to compare against
+    // the provided sha.
+    const branchGitHubResponse = await fetch(
+      // branchesApiUrl looks like "https://api.github.com/repos/owner/repo/branches{/branch}"
+      branchesApiUrl.replace('{/branch}', `/${branch}`),
+      {
+        headers,
+      },
+    );
+    const commitSha = (await branchGitHubResponse.json()).commit.sha;
+
+    if (options?.sha && options.sha === commitSha) {
+      throw new NotModifiedError();
+    }
+
+    // Note: the API way of downloading an archive URL does not return a real time archive.
+    // https://github.community/t/archive-downloaded-via-v3-rest-api-is-not-real-time/14827
+    // It looks like this https://api.github.com/repos/owner/repo/{archive_format}{/ref}
+    // and can be used from `repoResponseJson.archive_url`.
+    // Continue using the "direct" way i.e. https://github.com/:owner/:repo/archive/branch.tar.gz
+    // until the bug? is fixed.
+    const archive = await fetch(
+      new URL(
+        `${protocol}://${resource}/${full_name}/archive/${branch}.tar.gz`,
+      ).toString(),
+      {
+        headers,
+      },
+    );
+
+    const path = `${repoName}-${branch}/${filepath}`;
+
+    const archiveResponse = 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: (response.body as unknown) as Readable,
+      stream: (archive.body as unknown) as Readable,
       path,
       filter: options?.filter,
     });
+
+    const response = archiveResponse as ReadTreeResponse;
+    response.sha = commitSha;
+    return response;
   }
 
   toString() {
diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts
index 299bbc783b..76cfea7c38 100644
--- a/packages/backend-common/src/reading/GitlabUrlReader.ts
+++ b/packages/backend-common/src/reading/GitlabUrlReader.ts
@@ -94,13 +94,13 @@ export class GitlabUrlReader implements UrlReader {
     }
 
     const archive = `${protocol}://${resource}/${full_name}/-/archive/${ref}/${repoName}-${ref}.zip`;
-    const response = await fetch(
+    const archiveGitLabResponse = await fetch(
       archive,
       getGitLabRequestOptions(this.options),
     );
-    if (!response.ok) {
-      const msg = `Failed to read tree from ${url}, ${response.status} ${response.statusText}`;
-      if (response.status === 404) {
+    if (!archiveGitLabResponse.ok) {
+      const msg = `Failed to read tree from ${url}, ${archiveGitLabResponse.status} ${archiveGitLabResponse.statusText}`;
+      if (archiveGitLabResponse.status === 404) {
         throw new NotFoundError(msg);
       }
       throw new Error(msg);
@@ -108,11 +108,16 @@ export class GitlabUrlReader implements UrlReader {
 
     const path = filepath ? `${repoName}-${ref}/${filepath}/` : '';
 
-    return this.treeResponseFactory.fromZipArchive({
-      stream: (response.body as unknown) as Readable,
+    const archiveResponse = await this.treeResponseFactory.fromZipArchive({
+      stream: (archiveGitLabResponse.body as unknown) as Readable,
       path,
       filter: options?.filter,
     });
+
+    const response = archiveResponse as ReadTreeResponse;
+    // TODO: Just a placeholder for now.
+    response.sha = '';
+    return response;
   }
 
   toString() {
diff --git a/packages/backend-common/src/reading/UrlReaderPredicateMux.ts b/packages/backend-common/src/reading/UrlReaderPredicateMux.ts
index ca11400e4a..3183aa0c28 100644
--- a/packages/backend-common/src/reading/UrlReaderPredicateMux.ts
+++ b/packages/backend-common/src/reading/UrlReaderPredicateMux.ts
@@ -45,12 +45,15 @@ export class UrlReaderPredicateMux implements UrlReader {
     throw new NotAllowedError(`Reading from '${url}' is not allowed`);
   }
 
-  readTree(url: string, options?: ReadTreeOptions): Promise<ReadTreeResponse> {
+  async readTree(
+    url: string,
+    options?: ReadTreeOptions,
+  ): Promise<ReadTreeResponse> {
     const parsed = new URL(url);
 
     for (const { predicate, reader } of this.readers) {
       if (predicate(parsed)) {
-        return reader.readTree(url, options);
+        return await reader.readTree(url, options);
       }
     }
 
diff --git a/packages/backend-common/src/reading/__fixtures__/mock-main.tar.gz b/packages/backend-common/src/reading/__fixtures__/mock-main.tar.gz
new file mode 100644
index 0000000000000000000000000000000000000000..291690b447eae0995eda9c4215ffc21636d516c1
GIT binary patch
literal 230
zcmV<C02%)uiwFP%XaisX1MQXF3W7iog}v@67P`(x_YZoG?nC^+#b$$6q29hNMMKI6
z)Cl*xIXA=3>>lPUp5kJIk<wdlG5|nwP97?1g^<<SKmsb602+h=ClD|Y(j$Djkvb|<
zMrS)pt}xOjukcNsEz7cjbDDd!`X^xPKNiU;HvSclWB0J{As8F*KYaF|TK@$I@Cb}O
z$SW4^zfom+&=ICPvQJI9`t>}HdFl55zWvjD0$=4n1-t*;egQ83=b-1m(n)&pF&VYT
g$NUG`^WT<-F8}9X?PA~Ia5xsp4OF6Z-vAH*0G5MqdH?_b

literal 0
HcmV?d00001

diff --git a/packages/backend-common/src/reading/__fixtures__/repo.tar.gz b/packages/backend-common/src/reading/__fixtures__/repo.tar.gz
deleted file mode 100644
index 7a8e9902a24232a5f7130637a05304134abc3fec..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001

literal 232
zcmb2|=3oE==C@Zbay1!<usw+VF4DYi_1d=<FD!TUUvNzN+!A%=_Hyx57r{oUtN-uv
z+(}@s`zCF1PObI+N1rclS7ib>XSu&B+beL(LRxe2^Idk!eV446aOJ+o+S<sq+?q$*
zDo@uW)D^wE`fobd^*_$j3S#{H6K{W&^*u0g>-o>~??20b^`}#YbLB3%Cll?@@AuK(
zf9m={{U?{EUwv`;^MMVwzTe;Tuda?M|Lgvh`?I~>7XNQAm|j0i{htM|=koGBZ|a}E
f`_yk@`Q^XBu@$utzd*@``2}nqTB%DJG#D5FAn<c{

diff --git a/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts b/packages/backend-common/src/reading/tree/ReadTreeResponseFactory.ts
index 986a0302bc..cc229e9d5a 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 { ReadTreeResponse } from '../types';
+import { ReadTreeArchiveResponse } from '../types';
 import { TarArchiveResponse } from './TarArchiveResponse';
 import { ZipArchiveResponse } from './ZipArchiveResponse';
 
@@ -40,7 +40,9 @@ export class ReadTreeResponseFactory {
 
   constructor(private readonly workDir: string) {}
 
-  async fromTarArchive(options: FromArchiveOptions): Promise<ReadTreeResponse> {
+  async fromTarArchive(
+    options: FromArchiveOptions,
+  ): Promise<ReadTreeArchiveResponse> {
     return new TarArchiveResponse(
       options.stream,
       options.path ?? '',
@@ -49,7 +51,9 @@ export class ReadTreeResponseFactory {
     );
   }
 
-  async fromZipArchive(options: FromArchiveOptions): Promise<ReadTreeResponse> {
+  async fromZipArchive(
+    options: FromArchiveOptions,
+  ): Promise<ReadTreeArchiveResponse> {
     return new ZipArchiveResponse(
       options.stream,
       options.path ?? '',
diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts
index 1bc0d3a386..bbeee00c70 100644
--- a/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts
+++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.test.ts
@@ -20,7 +20,7 @@ import { resolve as resolvePath } from 'path';
 import { TarArchiveResponse } from './TarArchiveResponse';
 
 const archiveData = fs.readFileSync(
-  resolvePath(__filename, '../../__fixtures__/repo.tar.gz'),
+  resolvePath(__filename, '../../__fixtures__/mock-main.tar.gz'),
 );
 
 describe('TarArchiveResponse', () => {
@@ -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-repo/', '/tmp');
+    const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp');
     const files = await res.files();
 
     expect(files).toEqual([
@@ -61,7 +61,7 @@ describe('TarArchiveResponse', () => {
   it('should read files with filter', async () => {
     const stream = fs.createReadStream('/test-archive.tar.gz');
 
-    const res = new TarArchiveResponse(stream, 'mock-repo/', '/tmp', path =>
+    const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp', path =>
       path.endsWith('.yml'),
     );
     const files = await res.files();
@@ -79,7 +79,7 @@ describe('TarArchiveResponse', () => {
   it('should read as archive and files', async () => {
     const stream = fs.createReadStream('/test-archive.tar.gz');
 
-    const res = new TarArchiveResponse(stream, 'mock-repo/', '/tmp');
+    const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp');
     const buffer = await res.archive();
 
     await expect(res.archive()).rejects.toThrow(
@@ -113,17 +113,17 @@ describe('TarArchiveResponse', () => {
     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.tar.gz');
 
-    const res = new TarArchiveResponse(stream, 'mock-repo/docs/', '/tmp');
+    const res = new TarArchiveResponse(stream, 'mock-main/docs/', '/tmp');
     const dir = await res.dir();
 
     expect(dir).toMatch(/^[\/\\]tmp[\/\\].*$/);
@@ -135,7 +135,7 @@ 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-repo/', '/tmp', path =>
+    const res = new TarArchiveResponse(stream, 'mock-main/', '/tmp', 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 5d18ec7dc6..e8929fbe9b 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 {
-  ReadTreeResponse,
+  ReadTreeArchiveResponse,
   ReadTreeResponseFile,
   ReadTreeResponseDirOptions,
 } from '../types';
@@ -34,7 +34,7 @@ const pipeline = promisify(pipelineCb);
 /**
  * Wraps a tar archive stream into a tree response reader.
  */
-export class TarArchiveResponse implements ReadTreeResponse {
+export class TarArchiveResponse implements ReadTreeArchiveResponse {
   private read = false;
 
   constructor(
diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts
index 4106d49a11..48ba4f19ee 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 {
-  ReadTreeResponse,
+  ReadTreeArchiveResponse,
   ReadTreeResponseFile,
   ReadTreeResponseDirOptions,
 } from '../types';
@@ -28,7 +28,7 @@ import {
 /**
  * Wraps a zip archive stream into a tree response reader.
  */
-export class ZipArchiveResponse implements ReadTreeResponse {
+export class ZipArchiveResponse implements ReadTreeArchiveResponse {
   private read = false;
 
   constructor(
diff --git a/packages/backend-common/src/reading/types.ts b/packages/backend-common/src/reading/types.ts
index f9dca3e1d5..c727ba078b 100644
--- a/packages/backend-common/src/reading/types.ts
+++ b/packages/backend-common/src/reading/types.ts
@@ -32,6 +32,19 @@ export type ReadTreeOptions = {
    * If no filter is provided all files are extracted.
    */
   filter?(path: string): boolean;
+
+  /**
+   * A commit SHA 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.
+   *
+   * 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.
+   */
+  sha?: string;
 };
 
 /**
@@ -67,8 +80,12 @@ export type ReadTreeResponseDirOptions = {
   targetDir?: string;
 };
 
-export type ReadTreeResponse = {
+export type ReadTreeArchiveResponse = {
   files(): Promise<ReadTreeResponseFile[]>;
   archive(): Promise<NodeJS.ReadableStream>;
   dir(options?: ReadTreeResponseDirOptions): Promise<string>;
 };
+
+export interface ReadTreeResponse extends ReadTreeArchiveResponse {
+  sha: string;
+}
diff --git a/packages/techdocs-common/src/helpers.test.ts b/packages/techdocs-common/src/helpers.test.ts
index 88c4d8829e..88fdeed498 100644
--- a/packages/techdocs-common/src/helpers.test.ts
+++ b/packages/techdocs-common/src/helpers.test.ts
@@ -135,6 +135,7 @@ describe('getDocFilesFromRepository', () => {
           archive: async () => {
             return Readable.from('');
           },
+          sha: '',
         };
       }
     }

From 294a70caba78729627b96759a61881b454069546 Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Sun, 17 Jan 2021 09:28:54 +0100
Subject: [PATCH 02/11] backend-common: Add changeset about SHA based caching
 in URL Reader

---
 .changeset/khaki-icons-trade.md | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 .changeset/khaki-icons-trade.md

diff --git a/.changeset/khaki-icons-trade.md b/.changeset/khaki-icons-trade.md
new file mode 100644
index 0000000000..37bafd8d33
--- /dev/null
+++ b/.changeset/khaki-icons-trade.md
@@ -0,0 +1,22 @@
+---
+'@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.
+
+An example -
+
+```ts
+const response = await reader.readTree(
+  'https://github.com/backstage/backstage',
+);
+
+const commitSha = response.sha;
+
+// Will throw a new NotModifiedError (exported from @backstage/backstage-common)
+await reader.readTree('https://github.com/backstage/backstage', {
+  sha: commitSha,
+});
+```
+
+2. URL Reader's readTree method can now detect the default branch. So, `url:https://github.com/org/repo/tree/master` can be replaced with `url:https://github.com/org/repo` in places like `backstage.io/techdocs-ref`.

From fa8ba330a86b518981fd6177203dc45590b65d3e Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Sun, 17 Jan 2021 10:43:39 +0100
Subject: [PATCH 03/11] integration: GitLab API should be added for default and
 should have a protocol

---
 .changeset/nervous-mails-repair.md             |  5 +++++
 packages/integration/src/gitlab/config.test.ts | 14 +++++++++++++-
 packages/integration/src/gitlab/config.ts      |  4 ++--
 3 files changed, 20 insertions(+), 3 deletions(-)
 create mode 100644 .changeset/nervous-mails-repair.md

diff --git a/.changeset/nervous-mails-repair.md b/.changeset/nervous-mails-repair.md
new file mode 100644
index 0000000000..f72e2b5e66
--- /dev/null
+++ b/.changeset/nervous-mails-repair.md
@@ -0,0 +1,5 @@
+---
+'@backstage/integration': patch
+---
+
+Fix GitLab API base URL and add it by default to the gitlab.com host
diff --git a/packages/integration/src/gitlab/config.test.ts b/packages/integration/src/gitlab/config.test.ts
index e817465b83..31643d74a6 100644
--- a/packages/integration/src/gitlab/config.test.ts
+++ b/packages/integration/src/gitlab/config.test.ts
@@ -43,7 +43,18 @@ describe('readGitLabIntegrationConfig', () => {
     const output = readGitLabIntegrationConfig(buildConfig({}));
     expect(output).toEqual({
       host: 'gitlab.com',
-      apiBaseUrl: 'gitlab.com/api/v4',
+      apiBaseUrl: 'https://gitlab.com/api/v4',
+    });
+  });
+
+  it('injects the correct GitLab API base URL when missing', () => {
+    const output = readGitLabIntegrationConfig(
+      buildConfig({ host: 'gitlab.com' }),
+    );
+
+    expect(output).toEqual({
+      host: 'gitlab.com',
+      apiBaseUrl: 'https://gitlab.com/api/v4',
     });
   });
 
@@ -86,6 +97,7 @@ describe('readGitLabIntegrationConfigs', () => {
     expect(output).toEqual([
       {
         host: 'gitlab.com',
+        apiBaseUrl: 'https://gitlab.com/api/v4',
       },
     ]);
   });
diff --git a/packages/integration/src/gitlab/config.ts b/packages/integration/src/gitlab/config.ts
index 2d9f4b39ab..fd52a436b6 100644
--- a/packages/integration/src/gitlab/config.ts
+++ b/packages/integration/src/gitlab/config.ts
@@ -18,7 +18,7 @@ import { Config } from '@backstage/config';
 import { isValidHost } from '../helpers';
 
 const GITLAB_HOST = 'gitlab.com';
-const GITLAB_API_BASE_URL = 'gitlab.com/api/v4';
+const GITLAB_API_BASE_URL = 'https://gitlab.com/api/v4';
 
 /**
  * The configuration parameters for a single GitLab integration.
@@ -89,7 +89,7 @@ export function readGitLabIntegrationConfigs(
   // As a convenience we always make sure there's at least an unauthenticated
   // reader for public gitlab repos.
   if (!result.some(c => c.host === GITLAB_HOST)) {
-    result.push({ host: GITLAB_HOST });
+    result.push({ host: GITLAB_HOST, apiBaseUrl: GITLAB_API_BASE_URL });
   }
 
   return result;

From f9ca2a3769989f653d3a439607099dd6369b4c71 Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Sun, 17 Jan 2021 12:10:32 +0100
Subject: [PATCH 04/11] backend-common: UrlReader/GitLab implement Sha based
 caching

Also use API to fetch archive.zip
---
 .../src/reading/AzureUrlReader.test.ts        |   6 +-
 .../src/reading/GithubUrlReader.test.ts       |  18 +-
 .../src/reading/GithubUrlReader.ts            |  16 +-
 .../src/reading/GitlabUrlReader.test.ts       | 197 ++++++++++++++----
 .../src/reading/GitlabUrlReader.ts            |  99 ++++++---
 .../reading/__fixtures__/gitlab-archive.zip   | Bin 0 -> 857 bytes
 .../src/reading/__fixtures__/mock-main.zip    | Bin 0 -> 777 bytes
 .../src/reading/__fixtures__/repo.zip         | Bin 387 -> 0 bytes
 .../reading/tree/ZipArchiveResponse.test.ts   |  28 +--
 packages/backend-common/src/reading/types.ts  |   4 +
 10 files changed, 275 insertions(+), 93 deletions(-)
 create mode 100644 packages/backend-common/src/reading/__fixtures__/gitlab-archive.zip
 create mode 100644 packages/backend-common/src/reading/__fixtures__/mock-main.zip
 delete mode 100644 packages/backend-common/src/reading/__fixtures__/repo.zip

diff --git a/packages/backend-common/src/reading/AzureUrlReader.test.ts b/packages/backend-common/src/reading/AzureUrlReader.test.ts
index 616cbaaadc..f729316223 100644
--- a/packages/backend-common/src/reading/AzureUrlReader.test.ts
+++ b/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');
diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts
index 6fdab9dfa1..24a49cf81c 100644
--- a/packages/backend-common/src/reading/GithubUrlReader.test.ts
+++ b/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);
+    });
   });
 });
diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts
index 2acca10d4f..8e0410642b 100644
--- a/packages/backend-common/src/reading/GithubUrlReader.ts
+++ b/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}`;
 
diff --git a/packages/backend-common/src/reading/GitlabUrlReader.test.ts b/packages/backend-common/src/reading/GitlabUrlReader.test.ts
index 6cc5e30dbd..a2dda48b16 100644
--- a/packages/backend-common/src/reading/GitlabUrlReader.test.ts
+++ b/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);
+    });
   });
 });
diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts
index 76cfea7c38..0c0ce4f565 100644
--- a/packages/backend-common/src/reading/GitlabUrlReader.ts
+++ b/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)}}`;
   }
 }
diff --git a/packages/backend-common/src/reading/__fixtures__/gitlab-archive.zip b/packages/backend-common/src/reading/__fixtures__/gitlab-archive.zip
new file mode 100644
index 0000000000000000000000000000000000000000..884ec200048ae719af7b6e9c352ce5ab062c0da2
GIT binary patch
literal 857
zcmWIWW@Zs#0D;aZ!yqsNN{BEhFy!VZXY1xBX6ES@XCxXL87C$s>xYK$GO#D}vm}7<
z32~N$(h6<{MwYLP3=CkC0>CD6FmN!euZ<3bnJ55c$l)+CH#;Rixmd3<HwSEXx6zFF
zVB@XvJ21`GKBwc^d*1tu@75p=PhGu-Obh|u>>OX)-s}?snh&xAVmruIbpJ@=upMMK
zs;976jPTSpBu}vetx?2hY-V0cYK2~I3fNyWfc^p*jm7xjFeL9x6FDEj2{ajGdVn`0
zlL#~J2m&ergSU<#ioEE8*Z_+#<iG?)7z`|F1f>paHV_|u_<RG39vFD*cmao9L`D|E
qOQ7gN4p2~Z!N8J6bw=!70$W3D<OO)MvVn|b0m3Cf1Lp%V0|Nj(!?5uH

literal 0
HcmV?d00001

diff --git a/packages/backend-common/src/reading/__fixtures__/mock-main.zip b/packages/backend-common/src/reading/__fixtures__/mock-main.zip
new file mode 100644
index 0000000000000000000000000000000000000000..beee59d3a075b3e439f5eb436051afae2fdb15f1
GIT binary patch
literal 777
zcmWIWW@Zs#0D;aZ!yqsNN^mhKFy!VZXY1xBX6EUKhVU}5C-bu;fN&NNmsW5yFtU7Q
zWMBa64*;9P!N9?=zBW1#W{v=mA%bE~Zgxt3a<N`zZVuSQZlf9T!NyzTL8hXasC`by
zv-iCB8Q-lz9G<#*51AMOyxBRvw!PUW1T+<7GQ>`hUFdG-N3j!RBdQ0mScmYyHY5+Q
z0}U6)G%PbOCAC5?HwEm689+b4LI%wb!C^>FpC)oXf)i*S$jkt5MkWzv+yM_%0tRm#
zK@=&`05KO95y-&>iU=53(&&L=F7eTV&*h+Chk>__dr)j3G7=EZ2So#Nkb$BB29`8(
dG9m{H*l=PaCBU1N4P+1t5Y_;VDF<Q(1_0rFlQ{qY

literal 0
HcmV?d00001

diff --git a/packages/backend-common/src/reading/__fixtures__/repo.zip b/packages/backend-common/src/reading/__fixtures__/repo.zip
deleted file mode 100644
index f66bf2d612eafc3c4b65216b83d1b3e884726236..0000000000000000000000000000000000000000
GIT binary patch
literal 0
HcmV?d00001

literal 387
zcmWIWW@h1H0D*0_(Sg%M&PT8V*&r;=Aj6QGpPa2*lv<FlpOT+kte=^el3Jmcn-Utr
z$-q3rXhwXn@z(g#3T_5QmKV$n3}8Z8Atbf9gew4U?gpT_U)$d769AeC!XhZ<=4OLT
z)~n3T0UO$l-O%F9lGOOT#N1RXnCXm6a?H5gEdjKJfq@Z-mo$P{IDNqi@dcU>0=yB1
jV>%w$@CX#ck-Y*m8RQiVlUdn74q^hr?Lc}Ph{FH?Ktx*D

diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts
index 6c2592ffce..345ec7a783 100644
--- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.test.ts
+++ b/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' });
diff --git a/packages/backend-common/src/reading/types.ts b/packages/backend-common/src/reading/types.ts
index c727ba078b..d5c24ed1db 100644
--- a/packages/backend-common/src/reading/types.ts
+++ b/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>;
 };
 

From 7078d35e0af4c0ef44cd63ae47f2f1361bab460c Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Sun, 17 Jan 2021 14:03:43 +0100
Subject: [PATCH 05/11] 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<String> {
+  private async getLastCommitShortHash(url: string): Promise<string> {
     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) =>

From 79f9a97428ea781136f46773c23769a52dfd460b Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Sun, 17 Jan 2021 15:02:55 +0100
Subject: [PATCH 06/11] backend-common: UrlReader/Azure implement SHA based
 caching

---
 .../src/reading/AzureUrlReader.test.ts        | 62 +++++++++++++++++--
 .../src/reading/AzureUrlReader.ts             | 25 +++++++-
 packages/integration/src/azure/core.ts        | 56 +++++++++++++++++
 packages/integration/src/azure/index.ts       |  1 +
 4 files changed, 136 insertions(+), 8 deletions(-)

diff --git a/packages/backend-common/src/reading/AzureUrlReader.test.ts b/packages/backend-common/src/reading/AzureUrlReader.test.ts
index f729316223..b492b26609 100644
--- a/packages/backend-common/src/reading/AzureUrlReader.test.ts
+++ b/packages/backend-common/src/reading/AzureUrlReader.test.ts
@@ -23,6 +23,7 @@ import { getVoidLogger } from '../logging';
 import { AzureUrlReader } from './AzureUrlReader';
 import { msw } from '@backstage/test-utils';
 import { ReadTreeResponseFactory } from './tree';
+import { NotModifiedError } from '../errors';
 
 const logger = getVoidLogger();
 
@@ -142,6 +143,11 @@ describe('AzureUrlReader', () => {
       path.resolve('src', 'reading', '__fixtures__', 'mock-main.zip'),
     );
 
+    const processor = new AzureUrlReader(
+      { host: 'dev.azure.com' },
+      { treeResponseFactory },
+    );
+
     beforeEach(() => {
       worker.use(
         rest.get(
@@ -153,19 +159,65 @@ describe('AzureUrlReader', () => {
               ctx.body(repoBuffer),
             ),
         ),
+        rest.get(
+          // https://docs.microsoft.com/en-us/rest/api/azure/devops/git/commits/get%20commits?view=azure-devops-rest-6.0#on-a-branch
+          'https://dev.azure.com/organization/project/_apis/git/repositories/repository/commits',
+          (_, res, ctx) =>
+            res(
+              ctx.status(200),
+              ctx.json({
+                count: 2,
+                value: [
+                  {
+                    commitId: '123abc2',
+                    comment: 'second commit',
+                  },
+                  {
+                    commitId: '123abc1',
+                    comment: 'first commit',
+                  },
+                ],
+              }),
+            ),
+        ),
       );
     });
 
     it('returns the wanted files from an archive', async () => {
-      const processor = new AzureUrlReader(
-        { host: 'dev.azure.com' },
-        { treeResponseFactory },
-      );
-
       const response = await processor.readTree(
         'https://dev.azure.com/organization/project/_git/repository',
       );
 
+      expect(response.sha).toBe('123abc2');
+
+      const files = await response.files();
+
+      expect(files.length).toBe(2);
+      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');
+    });
+
+    it('throws a NotModifiedError when given a sha in options', async () => {
+      const fnAzure = async () => {
+        await processor.readTree(
+          'https://dev.azure.com/organization/project/_git/repository',
+          { sha: '123abc2' },
+        );
+      };
+
+      await expect(fnAzure).rejects.toThrow(NotModifiedError);
+    });
+
+    it('should not throw a NotModifiedError when given an outdated sha in options', async () => {
+      const response = await processor.readTree(
+        'https://dev.azure.com/organization/project/_git/repository',
+        { sha: 'outdated123abc' },
+      );
+
+      expect(response.sha).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 4535b329d7..c40291ec85 100644
--- a/packages/backend-common/src/reading/AzureUrlReader.ts
+++ b/packages/backend-common/src/reading/AzureUrlReader.ts
@@ -20,10 +20,11 @@ import {
   getAzureFileFetchUrl,
   getAzureDownloadUrl,
   getAzureRequestOptions,
+  getAzureCommitsUrl,
 } from '@backstage/integration';
 import fetch from 'cross-fetch';
 import { Readable } from 'stream';
-import { NotFoundError } from '../errors';
+import { NotFoundError, NotModifiedError } from '../errors';
 import {
   ReaderFactory,
   ReadTreeOptions,
@@ -75,6 +76,25 @@ export class AzureUrlReader implements UrlReader {
     url: string,
     options?: ReadTreeOptions,
   ): Promise<ReadTreeResponse> {
+    // Get latest commit SHA
+
+    const commitsAzureResponse = await fetch(
+      getAzureCommitsUrl(url),
+      getAzureRequestOptions(this.options),
+    );
+    if (!commitsAzureResponse.ok) {
+      const message = `Failed to read tree from ${url}, ${commitsAzureResponse.status} ${commitsAzureResponse.statusText}`;
+      if (commitsAzureResponse.status === 404) {
+        throw new NotFoundError(message);
+      }
+      throw new Error(message);
+    }
+
+    const commitSha = (await commitsAzureResponse.json()).value[0].commitId;
+    if (options?.sha && options.sha === commitSha) {
+      throw new NotModifiedError();
+    }
+
     const archiveAzureResponse = await fetch(
       getAzureDownloadUrl(url),
       getAzureRequestOptions(this.options, { Accept: 'application/zip' }),
@@ -93,8 +113,7 @@ export class AzureUrlReader implements UrlReader {
     });
 
     const response = archiveResponse as ReadTreeResponse;
-    // TODO: Just a placeholder for now.
-    response.sha = '';
+    response.sha = commitSha;
     return response;
   }
 
diff --git a/packages/integration/src/azure/core.ts b/packages/integration/src/azure/core.ts
index 7900774c79..73ce4f83d7 100644
--- a/packages/integration/src/azure/core.ts
+++ b/packages/integration/src/azure/core.ts
@@ -108,6 +108,62 @@ export function getAzureDownloadUrl(url: string): string {
   return `${protocol}://${resource}/${organization}/${project}/_apis/git/repositories/${repoName}/items?recursionLevel=full&download=true&api-version=6.0${scopePath}`;
 }
 
+/**
+ * Given a URL, return the API URL to fetch commits on the branch.
+ *
+ * @param url A URL pointing to a repository or a sub-path
+ */
+export function getAzureCommitsUrl(url: string): string {
+  try {
+    const parsedUrl = new URL(url);
+
+    const [
+      empty,
+      userOrOrg,
+      project,
+      srcKeyword,
+      repoName,
+    ] = parsedUrl.pathname.split('/');
+
+    // Remove the "GB" from "GBmain" for example.
+    const ref = parsedUrl.searchParams.get('version')?.substr(2);
+
+    if (
+      empty !== '' ||
+      userOrOrg === '' ||
+      project === '' ||
+      srcKeyword !== '_git' ||
+      repoName === ''
+    ) {
+      throw new Error('Wrong Azure Devops URL');
+    }
+
+    // transform to commits api
+    parsedUrl.pathname = [
+      empty,
+      userOrOrg,
+      project,
+      '_apis',
+      'git',
+      'repositories',
+      repoName,
+      'commits',
+    ].join('/');
+
+    const queryParams = [];
+    if (ref) {
+      queryParams.push(`searchCriteria.itemVersion.version=${ref}`);
+    }
+    parsedUrl.search = queryParams.join('&');
+
+    parsedUrl.protocol = 'https';
+
+    return parsedUrl.toString();
+  } catch (e) {
+    throw new Error(`Incorrect URL: ${url}, ${e}`);
+  }
+}
+
 /**
  * Gets the request options necessary to make requests to a given provider.
  *
diff --git a/packages/integration/src/azure/index.ts b/packages/integration/src/azure/index.ts
index 365e4cdcdc..6d57437779 100644
--- a/packages/integration/src/azure/index.ts
+++ b/packages/integration/src/azure/index.ts
@@ -23,4 +23,5 @@ export {
   getAzureDownloadUrl,
   getAzureFileFetchUrl,
   getAzureRequestOptions,
+  getAzureCommitsUrl,
 } from './core';

From 8565ad2279c157cd0c404499efe51b6942a6b685 Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Mon, 18 Jan 2021 22:50:47 +0100
Subject: [PATCH 07/11] 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<ReadTreeArchiveResponse> {
+  async fromTarArchive(options: FromArchiveOptions): Promise<ReadTreeResponse> {
     return new TarArchiveResponse(
       options.stream,
       options.path ?? '',
       this.workDir,
+      options.etag,
       options.filter,
     );
   }
 
-  async fromZipArchive(
-    options: FromArchiveOptions,
-  ): Promise<ReadTreeArchiveResponse> {
+  async fromZipArchive(options: FromArchiveOptions): Promise<ReadTreeResponse> {
     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<ReadTreeResponseFile[]>;
   archive(): Promise<NodeJS.ReadableStream>;
 
@@ -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<string>;
-};
 
-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: '',
         };
       }
     }

From baeed36324f6a275f3885da8a86aea78b0cbd8d8 Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Mon, 18 Jan 2021 22:59:36 +0100
Subject: [PATCH 08/11] Convert NotModifiedError to 304 HTTP status code

---
 packages/backend-common/src/middleware/errorHandler.test.ts | 4 ++++
 packages/backend-common/src/middleware/errorHandler.ts      | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/packages/backend-common/src/middleware/errorHandler.test.ts b/packages/backend-common/src/middleware/errorHandler.test.ts
index a7a3d64bd1..6d22c2f175 100644
--- a/packages/backend-common/src/middleware/errorHandler.test.ts
+++ b/packages/backend-common/src/middleware/errorHandler.test.ts
@@ -72,6 +72,9 @@ describe('errorHandler', () => {
 
   it('handles well-known error classes', async () => {
     const app = express();
+    app.use('/NotModifiedError', () => {
+      throw new errors.NotModifiedError();
+    });
     app.use('/InputError', () => {
       throw new errors.InputError();
     });
@@ -90,6 +93,7 @@ describe('errorHandler', () => {
     app.use(errorHandler());
 
     const r = request(app);
+    expect((await r.get('/NotModifiedError')).status).toBe(304);
     expect((await r.get('/InputError')).status).toBe(400);
     expect((await r.get('/AuthenticationError')).status).toBe(401);
     expect((await r.get('/NotAllowedError')).status).toBe(403);
diff --git a/packages/backend-common/src/middleware/errorHandler.ts b/packages/backend-common/src/middleware/errorHandler.ts
index 7365ce8b93..a08849813d 100644
--- a/packages/backend-common/src/middleware/errorHandler.ts
+++ b/packages/backend-common/src/middleware/errorHandler.ts
@@ -101,6 +101,8 @@ function getStatusCode(error: Error): number {
 
   // Handle well-known error types
   switch (error.name) {
+    case errors.NotModifiedError.name:
+      return 304;
     case errors.InputError.name:
       return 400;
     case errors.AuthenticationError.name:

From 380dd626fb1f42c24c8ce77eddfc5b93caa389e5 Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Mon, 18 Jan 2021 23:04:31 +0100
Subject: [PATCH 09/11] Safe nullish check with git-url-parse library responses

---
 packages/backend-common/src/reading/GithubUrlReader.ts | 2 +-
 packages/backend-common/src/reading/GitlabUrlReader.ts | 2 +-
 packages/integration/src/azure/core.ts                 | 8 ++++----
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts
index 1a8133fceb..d6612a9da2 100644
--- a/packages/backend-common/src/reading/GithubUrlReader.ts
+++ b/packages/backend-common/src/reading/GithubUrlReader.ts
@@ -130,7 +130,7 @@ export class GithubUrlReader implements UrlReader {
 
     // ref is an empty string if no branch is set in provided url to readTree.
     // Use GitHub API to get the default branch of the repository.
-    const branch = ref === '' ? repoResponseJson.default_branch : ref;
+    const branch = ref || repoResponseJson.default_branch;
     const branchesApiUrl = repoResponseJson.branches_url;
 
     // Fetch the latest commit in the provided or default branch to compare against
diff --git a/packages/backend-common/src/reading/GitlabUrlReader.ts b/packages/backend-common/src/reading/GitlabUrlReader.ts
index 647c6fd644..72db901ac1 100644
--- a/packages/backend-common/src/reading/GitlabUrlReader.ts
+++ b/packages/backend-common/src/reading/GitlabUrlReader.ts
@@ -99,7 +99,7 @@ export class GitlabUrlReader implements UrlReader {
     const projectGitlabResponseJson = await projectGitlabResponse.json();
 
     // ref is an empty string if no branch is set in provided url to readTree.
-    const branch = ref === '' ? projectGitlabResponseJson.default_branch : ref;
+    const branch = ref || projectGitlabResponseJson.default_branch;
 
     // Fetch the latest commit in the provided or default branch to compare against
     // the provided sha.
diff --git a/packages/integration/src/azure/core.ts b/packages/integration/src/azure/core.ts
index 73ce4f83d7..b2878af89e 100644
--- a/packages/integration/src/azure/core.ts
+++ b/packages/integration/src/azure/core.ts
@@ -129,11 +129,11 @@ export function getAzureCommitsUrl(url: string): string {
     const ref = parsedUrl.searchParams.get('version')?.substr(2);
 
     if (
-      empty !== '' ||
-      userOrOrg === '' ||
-      project === '' ||
+      !!empty ||
+      !userOrOrg ||
+      !project ||
       srcKeyword !== '_git' ||
-      repoName === ''
+      !repoName
     ) {
       throw new Error('Wrong Azure Devops URL');
     }

From a65c4516ec854875e7d21139f9418bcc2bad1c11 Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Tue, 19 Jan 2021 10:41:40 +0100
Subject: [PATCH 10/11] Use GitHub API to download the archive

---
 .../src/reading/GithubUrlReader.test.ts       |  33 +++++++++-----
 .../src/reading/GithubUrlReader.ts            |  42 +++++++++---------
 .../backstage-mock-etag123.tar.gz             | Bin 0 -> 240 bytes
 3 files changed, 42 insertions(+), 33 deletions(-)
 create mode 100644 packages/backend-common/src/reading/__fixtures__/backstage-mock-etag123.tar.gz

diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts
index 26ba91bd29..0b7d56480f 100644
--- a/packages/backend-common/src/reading/GithubUrlReader.test.ts
+++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts
@@ -108,7 +108,12 @@ describe('GithubUrlReader', () => {
 
   describe('readTree', () => {
     const repoBuffer = fs.readFileSync(
-      path.resolve('src', 'reading', '__fixtures__', 'mock-main.tar.gz'),
+      path.resolve(
+        'src',
+        'reading',
+        '__fixtures__',
+        'backstage-mock-etag123.tar.gz',
+      ),
     );
 
     const reposGithubApiResponse = {
@@ -117,12 +122,16 @@ describe('GithubUrlReader', () => {
       default_branch: 'main',
       branches_url:
         'https://api.github.com/repos/backstage/mock/branches{/branch}',
+      archive_url:
+        'https://api.github.com/repos/backstage/mock/{archive_format}{/ref}',
     };
 
     const reposGheApiResponse = {
       ...reposGithubApiResponse,
       branches_url:
         'https://ghe.github.com/api/v3/repos/backstage/mock/branches{/branch}',
+      archive_url:
+        'https://ghe.github.com/api/v3/repos/backstage/mock/{archive_format}{/ref}',
     };
 
     const branchesApiResponse = {
@@ -134,15 +143,6 @@ describe('GithubUrlReader', () => {
 
     beforeEach(() => {
       worker.use(
-        rest.get(
-          'https://github.com/backstage/mock/archive/main.tar.gz',
-          (_, res, ctx) =>
-            res(
-              ctx.status(200),
-              ctx.set('Content-Type', 'application/x-gzip'),
-              ctx.body(repoBuffer),
-            ),
-        ),
         rest.get('https://api.github.com/repos/backstage/mock', (_, res, ctx) =>
           res(
             ctx.status(200),
@@ -159,12 +159,21 @@ describe('GithubUrlReader', () => {
               ctx.json(branchesApiResponse),
             ),
         ),
+        rest.get(
+          'https://api.github.com/repos/backstage/mock/tarball/etag123abc',
+          (_, res, ctx) =>
+            res(
+              ctx.status(200),
+              ctx.set('Content-Type', 'application/x-gzip'),
+              ctx.body(repoBuffer),
+            ),
+        ),
         rest.get(
           'https://api.github.com/repos/backstage/mock/branches/branchDoesNotExist',
           (_, res, ctx) => res(ctx.status(404)),
         ),
         rest.get(
-          'https://ghe.github.com/backstage/mock/archive/main.tar.gz',
+          'https://ghe.github.com/api/v3/repos/backstage/mock/tarball/etag123abc',
           (_, res, ctx) =>
             res(
               ctx.status(200),
@@ -224,7 +233,7 @@ describe('GithubUrlReader', () => {
 
       worker.use(
         rest.get(
-          'https://ghe.github.com/backstage/mock/archive/main.tar.gz',
+          'https://ghe.github.com/api/v3/repos/backstage/mock/tarball/etag123abc',
           (req, res, ctx) => {
             expect(req.headers.get('authorization')).toBe(
               mockHeaders.Authorization,
diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts
index d6612a9da2..b5b4e9c1a7 100644
--- a/packages/backend-common/src/reading/GithubUrlReader.ts
+++ b/packages/backend-common/src/reading/GithubUrlReader.ts
@@ -98,14 +98,9 @@ export class GithubUrlReader implements UrlReader {
     url: string,
     options?: ReadTreeOptions,
   ): Promise<ReadTreeResponse> {
-    const {
-      protocol,
-      resource,
-      name: repoName,
-      ref,
-      filepath,
-      full_name,
-    } = parseGitUrl(url);
+    const { ref, filepath, full_name } = parseGitUrl(url);
+    // Caveat: The ref will totally be incorrect if the branch name includes a /
+    // Thus, readTree can not work on url containing branch name that has a /
 
     const { headers } = await this.deps.credentialsProvider.getCredentials({
       url,
@@ -132,6 +127,7 @@ export class GithubUrlReader implements UrlReader {
     // Use GitHub API to get the default branch of the repository.
     const branch = ref || repoResponseJson.default_branch;
     const branchesApiUrl = repoResponseJson.branches_url;
+    const archiveApiUrl = repoResponseJson.archive_url;
 
     // Fetch the latest commit in the provided or default branch to compare against
     // the provided sha.
@@ -155,19 +151,12 @@ export class GithubUrlReader implements UrlReader {
       throw new NotModifiedError();
     }
 
-    // Note: the API way of downloading an archive URL does not return a real time archive.
-    // https://github.community/t/archive-downloaded-via-v3-rest-api-is-not-real-time/14827
-    // It looks like this https://api.github.com/repos/owner/repo/{archive_format}{/ref}
-    // and can be used from `repoResponseJson.archive_url`.
-    // Continue using the "direct" way i.e. https://github.com/:owner/:repo/archive/branch.tar.gz
-    // until the bug? is fixed.
     const archive = await fetch(
-      new URL(
-        `${protocol}://${resource}/${full_name}/archive/${branch}.tar.gz`,
-      ).toString(),
-      {
-        headers,
-      },
+      // archiveApiUrl looks like "https://api.github.com/repos/owner/repo/{archive_format}{/ref}"
+      archiveApiUrl
+        .replace('{archive_format}', 'tarball')
+        .replace('{/ref}', `/${commitSha}`),
+      { headers },
     );
     if (!archive.ok) {
       const message = `Failed to read tree (archive) from ${url}, ${archive.status} ${archive.statusText}`;
@@ -177,7 +166,18 @@ export class GithubUrlReader implements UrlReader {
       throw new Error(message);
     }
 
-    const path = `${repoName}-${branch}/${filepath}`;
+    // Note that repoResponseJson.full_name must be used over full_name because the path
+    // is case sensitive and full_name may not be inq the correct case.
+    // TODO(OrkoHunter): The directory name inside the tarball should be retrieved from the tar
+    // instead of being constructed here. Same goes for GitLab, Bitbucket and Azure.
+    const extractedDirName = `${repoResponseJson.full_name.replace(
+      '/',
+      '-',
+    )}-${commitSha.substr(0, 7)}`;
+
+    // The path includes the name of the directory inside the tarball and a sub path
+    // if requested in readTree.
+    const path = `${extractedDirName}/${filepath}`;
 
     return await this.deps.treeResponseFactory.fromTarArchive({
       // TODO(Rugvip): Underlying implementation of fetch will be node-fetch, we probably want
diff --git a/packages/backend-common/src/reading/__fixtures__/backstage-mock-etag123.tar.gz b/packages/backend-common/src/reading/__fixtures__/backstage-mock-etag123.tar.gz
new file mode 100644
index 0000000000000000000000000000000000000000..e1ac2579de3999e2847c562c9d0882f486b81e22
GIT binary patch
literal 240
zcmV<M01y8kiwFSKs0LsF1MQVv3xY5dhQ01r47%3NK81cq_u<D(u_-i#`uDArC`ct5
zM!4tZY;f2)XD@pfM@gxh=pcPnC#5gV5)-=dYytow=ja|q7lE)Y_8?3{CV*msIRY#Q
zr5@r|yBMG~)1OQ$d7+|8=dBvZGig~ia!x<TBmN2Td;L|J)`<=->S&>cxevkEi2meR
ze~QeW5auDUP<xVv>#vJO?yE@2E!s)ltbW~(V_pW{|Lyf#`vgAne+tI`LoC4g{~V0z
qUsS0)2P*xx#-#s4_^Lkv0mAwJ9ITz~I~)$jBDn%0oP%uu5C8yN%yPE?

literal 0
HcmV?d00001


From 662d9b9a225738bcf03908c7780a3561c40adc4c Mon Sep 17 00:00:00 2001
From: Himanshu Mishra <himanshu@orkohunter.net>
Date: Wed, 20 Jan 2021 14:33:06 +0100
Subject: [PATCH 11/11] backend-common: Move definition of etag property inside
 constructor

---
 packages/backend-common/src/reading/tree/TarArchiveResponse.ts | 3 +--
 packages/backend-common/src/reading/tree/ZipArchiveResponse.ts | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.ts
index f44adcc7b2..5927eb75a1 100644
--- a/packages/backend-common/src/reading/tree/TarArchiveResponse.ts
+++ b/packages/backend-common/src/reading/tree/TarArchiveResponse.ts
@@ -36,13 +36,12 @@ const pipeline = promisify(pipelineCb);
  */
 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,
+    public readonly etag: string,
     private readonly filter?: (path: string) => boolean,
   ) {
     if (subPath) {
diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts
index 7550dc1259..07d34faaa3 100644
--- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts
+++ b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts
@@ -30,13 +30,12 @@ import {
  */
 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,
+    public readonly etag: string,
     private readonly filter?: (path: string) => boolean,
   ) {
     if (subPath) {