backend-common: slight refactoring in support of future search impl

.changeset/old-tools-exist.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-common': patch
+---
+
+Slight refactoring in support of a future search implementation in `UrlReader`. Mostly moving code around.

packages/backend-common/package.json

@@ -33,6 +33,7 @@
     "@backstage/config": "^0.1.2",
     "@backstage/config-loader": "^0.5.1",
     "@backstage/integration": "^0.3.2",
+    "@octokit/rest": "^18.0.12",
     "@types/cors": "^2.8.6",
     "@types/express": "^4.17.6",
     "archiver": "^5.0.2",

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

@@ -15,11 +15,12 @@
  */
 
 import {
-  GitHubIntegrationConfig,
-  readGitHubIntegrationConfigs,
   getGitHubFileFetchUrl,
   GithubCredentialsProvider,
+  GitHubIntegrationConfig,
+  readGitHubIntegrationConfigs,
 } from '@backstage/integration';
+import { RestEndpointMethodTypes } from '@octokit/rest';
 import fetch from 'cross-fetch';
 import parseGitUrl from 'git-url-parse';
 import { Readable } from 'stream';
@@ -98,74 +99,26 @@ export class GithubUrlReader implements UrlReader {
     url: string,
     options?: ReadTreeOptions,
   ): Promise<ReadTreeResponse> {
-    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,
-    });
-
-    // Get GitHub API urls for the repository
-    const repoGitHubResponse = await fetch(
-      new URL(`${this.config.apiBaseUrl}/repos/${full_name}`).toString(),
-      {
-        headers,
-      },
-    );
-    if (!repoGitHubResponse.ok) {
-      const message = `Failed to read tree (repository) from ${url}, ${repoGitHubResponse.status} ${repoGitHubResponse.statusText}`;
-      if (repoGitHubResponse.status === 404) {
-        throw new NotFoundError(message);
-      }
-      throw new Error(message);
-    }
-
-    const repoResponseJson = await repoGitHubResponse.json();
-
-    // 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;
-    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.
-    const branchGitHubResponse = await fetch(
-      // branchesApiUrl looks like "https://api.github.com/repos/owner/repo/branches{/branch}"
-      branchesApiUrl.replace('{/branch}', `/${branch}`),
-      {
-        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;
+    const repoDetails = await this.getRepoDetails(url);
+    const commitSha = repoDetails.branch.commit.sha!;
 
     if (options?.etag && options.etag === commitSha) {
       throw new NotModifiedError();
     }
 
-    const archive = await fetch(
-      // archiveApiUrl looks like "https://api.github.com/repos/owner/repo/{archive_format}{/ref}"
-      archiveApiUrl
+    const { headers } = await this.deps.credentialsProvider.getCredentials({
+      url,
+    });
+
+    // archive_url looks like "https://api.github.com/repos/owner/repo/{archive_format}{/ref}"
+    const archive = await this.fetchResponse(
+      repoDetails.repo.archive_url
         .replace('{archive_format}', 'tarball')
         .replace('{/ref}', `/${commitSha}`),
       { 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 { filepath } = parseGitUrl(url);
     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.
@@ -180,4 +133,59 @@ export class GithubUrlReader implements UrlReader {
     const { host, token } = this.config;
     return `github{host=${host},authed=${Boolean(token)}}`;
   }
+
+  private async getRepoDetails(
+    url: string,
+  ): Promise<{
+    repo: RestEndpointMethodTypes['repos']['get']['response']['data'];
+    branch: RestEndpointMethodTypes['repos']['getBranch']['response']['data'];
+  }> {
+    const parsed = parseGitUrl(url);
+    const { ref, full_name } = parsed;
+
+    // Caveat: The ref will totally be incorrect if the branch name includes a
+    // slash. Thus, some operations can not work on URLs containing branch
+    // names that have a slash in them.
+
+    const { headers } = await this.deps.credentialsProvider.getCredentials({
+      url,
+    });
+
+    const repo: RestEndpointMethodTypes['repos']['get']['response']['data'] = await this.fetchJson(
+      `${this.config.apiBaseUrl}/repos/${full_name}`,
+      { headers },
+    );
+
+    // branches_url looks like "https://api.github.com/repos/owner/repo/branches{/branch}"
+    const branch: RestEndpointMethodTypes['repos']['getBranch']['response']['data'] = await this.fetchJson(
+      repo.branches_url.replace('{/branch}', `/${ref || repo.default_branch}`),
+      { headers },
+    );
+
+    return { repo, branch };
+  }
+
+  private async fetchResponse(
+    url: string | URL,
+    init: RequestInit,
+  ): Promise<Response> {
+    const urlAsString = url.toString();
+
+    const response = await fetch(urlAsString, init);
+
+    if (!response.ok) {
+      const message = `Request failed for ${urlAsString}, ${response.status} ${response.statusText}`;
+      if (response.status === 404) {
+        throw new NotFoundError(message);
+      }
+      throw new Error(message);
+    }
+
+    return response;
+  }
+
+  private async fetchJson(url: string | URL, init: RequestInit): Promise<any> {
+    const response = await this.fetchResponse(url, init);
+    return await response.json();
+  }
 }

packages/backend-common/src/reading/tree/TarArchiveResponse.ts

@@ -14,16 +14,16 @@
  * limitations under the License.
  */
 
-import tar, { Parse, ParseStream, ReadEntry } from 'tar';
-import platformPath from 'path';
-import fs from 'fs-extra';
-import { Readable, pipeline as pipelineCb } from 'stream';
-import { promisify } from 'util';
 import concatStream from 'concat-stream';
+import fs from 'fs-extra';
+import platformPath from 'path';
+import { pipeline as pipelineCb, Readable } from 'stream';
+import tar, { Parse, ParseStream, ReadEntry } from 'tar';
+import { promisify } from 'util';
 import {
   ReadTreeResponse,
-  ReadTreeResponseFile,
   ReadTreeResponseDirOptions,
+  ReadTreeResponseFile,
 } from '../types';
 
 // Tar types for `Parse` is not a proper constructor, but it should be

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

@@ -14,15 +14,15 @@
  * limitations under the License.
  */
 
-import platformPath from 'path';
-import fs from 'fs-extra';
-import unzipper, { Entry } from 'unzipper';
 import archiver from 'archiver';
+import fs from 'fs-extra';
+import platformPath from 'path';
 import { Readable } from 'stream';
+import unzipper, { Entry } from 'unzipper';
 import {
   ReadTreeResponse,
-  ReadTreeResponseFile,
   ReadTreeResponseDirOptions,
+  ReadTreeResponseFile,
 } from '../types';
 
 // Matches a directory name + one `/` at the start of any string,

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

@@ -18,6 +18,32 @@ import { Logger } from 'winston';
 import { Config } from '@backstage/config';
 import { ReadTreeResponseFactory } from './tree';
 
+/**
+ * A generic interface for fetching plain data from URLs.
+ */
+export type UrlReader = {
+  read(url: string): Promise<Buffer>;
+  readTree(url: string, options?: ReadTreeOptions): Promise<ReadTreeResponse>;
+};
+
+export type UrlReaderPredicateTuple = {
+  predicate: (url: URL) => boolean;
+  reader: UrlReader;
+};
+
+/**
+ * A factory function that can read config to construct zero or more
+ * UrlReaders along with a predicate for when it should be used.
+ */
+export type ReaderFactory = (options: {
+  config: Config;
+  logger: Logger;
+  treeResponseFactory: ReadTreeResponseFactory;
+}) => UrlReaderPredicateTuple[];
+
+/**
+ * An options object for readTree operations.
+ */
 export type ReadTreeOptions = {
   /**
    * A filter that can be used to select which files should be included.
@@ -47,39 +73,6 @@ export type ReadTreeOptions = {
   etag?: string;
 };
 
-/**
- * A generic interface for fetching plain data from URLs.
- */
-export type UrlReader = {
-  read(url: string): Promise<Buffer>;
-  readTree(url: string, options?: ReadTreeOptions): Promise<ReadTreeResponse>;
-};
-
-export type UrlReaderPredicateTuple = {
-  predicate: (url: URL) => boolean;
-  reader: UrlReader;
-};
-
-/**
- * A factory function that can read config to construct zero or more
- * UrlReaders along with a predicate for when it should be used.
- */
-export type ReaderFactory = (options: {
-  config: Config;
-  logger: Logger;
-  treeResponseFactory: ReadTreeResponseFactory;
-}) => UrlReaderPredicateTuple[];
-
-export type ReadTreeResponseFile = {
-  path: string;
-  content(): Promise<Buffer>;
-};
-
-export type ReadTreeResponseDirOptions = {
-  /** The directory to write files to. Defaults to the OS tmpdir or `backend.workingDirectory` if set in config */
-  targetDir?: string;
-};
-
 export type ReadTreeResponse = {
   /**
    * files() returns an array of all the files inside the tree and corresponding functions to read their content.
@@ -97,3 +90,16 @@ export type ReadTreeResponse = {
    */
   etag: string;
 };
+
+export type ReadTreeResponseDirOptions = {
+  /** The directory to write files to. Defaults to the OS tmpdir or `backend.workingDirectory` if set in config */
+  targetDir?: string;
+};
+
+/**
+ * Represents a single file in a readTree response.
+ */
+export type ReadTreeResponseFile = {
+  path: string;
+  content(): Promise<Buffer>;
+};

packages/integration/src/github/GithubCredentialsProvider.ts

@@ -53,7 +53,7 @@ class Cache {
 
 /**
  * This accept header is required when calling App APIs in GitHub Enterprise.
- * It has no effect on calls to github.com and can probably be removed entierly
+ * It has no effect on calls to github.com and can probably be removed entirely
  * once GitHub Apps is out of preview.
  */
 const HEADERS = {