From 965e200c61670758d6358b4ed58c3687f6f022c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Thu, 4 Feb 2021 14:19:17 +0100 Subject: [PATCH] backend-common: slight refactoring in support of future search impl --- .changeset/old-tools-exist.md | 5 + packages/backend-common/package.json | 1 + .../src/reading/GithubUrlReader.ts | 128 ++++++++++-------- .../src/reading/tree/TarArchiveResponse.ts | 12 +- .../src/reading/tree/ZipArchiveResponse.ts | 8 +- packages/backend-common/src/reading/types.ts | 72 +++++----- .../src/github/GithubCredentialsProvider.ts | 2 +- 7 files changed, 124 insertions(+), 104 deletions(-) create mode 100644 .changeset/old-tools-exist.md diff --git a/.changeset/old-tools-exist.md b/.changeset/old-tools-exist.md new file mode 100644 index 0000000000..a9365cb784 --- /dev/null +++ b/.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. diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 3053784dc3..faec36dab4 100644 --- a/packages/backend-common/package.json +++ b/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", diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index 4f1aa7c07b..99d641f2dd 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/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 { - 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 { + 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 { + const response = await this.fetchResponse(url, init); + return await response.json(); + } } diff --git a/packages/backend-common/src/reading/tree/TarArchiveResponse.ts b/packages/backend-common/src/reading/tree/TarArchiveResponse.ts index 24bfa3f1a8..d22529293f 100644 --- a/packages/backend-common/src/reading/tree/TarArchiveResponse.ts +++ b/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 diff --git a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts b/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts index 323f7b4778..2547c7b0e4 100644 --- a/packages/backend-common/src/reading/tree/ZipArchiveResponse.ts +++ b/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, diff --git a/packages/backend-common/src/reading/types.ts b/packages/backend-common/src/reading/types.ts index f9dfbe9aff..be788fc6e2 100644 --- a/packages/backend-common/src/reading/types.ts +++ b/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; + readTree(url: string, options?: ReadTreeOptions): Promise; +}; + +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; - readTree(url: string, options?: ReadTreeOptions): Promise; -}; - -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; -}; - -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; +}; diff --git a/packages/integration/src/github/GithubCredentialsProvider.ts b/packages/integration/src/github/GithubCredentialsProvider.ts index a22c69909b..0bf217c824 100644 --- a/packages/integration/src/github/GithubCredentialsProvider.ts +++ b/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 = {