From b461b367d7cf9c99f7b39fd89309f7bb8deec6b0 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 29 Sep 2020 18:46:25 +0200 Subject: [PATCH] backend-common: port GithubUrlReader --- packages/backend-common/package.json | 3 + .../src/reading/GithubUrlReader.test.ts | 37 +++----- .../src/reading/GithubUrlReader.ts | 88 +++++++------------ 3 files changed, 43 insertions(+), 85 deletions(-) diff --git a/packages/backend-common/package.json b/packages/backend-common/package.json index 72f047761d..b2550c6551 100644 --- a/packages/backend-common/package.json +++ b/packages/backend-common/package.json @@ -39,11 +39,13 @@ "express": "^4.17.1", "express-prom-bundle": "^6.1.0", "express-promise-router": "^3.0.3", + "git-url-parse": "^11.2.0", "helmet": "^4.0.0", "knex": "^0.21.1", "lodash": "^4.17.15", "logform": "^2.1.1", "morgan": "^1.10.0", + "node-fetch": "^2.6.0", "prom-client": "^12.0.0", "selfsigned": "^1.10.7", "stoppable": "^1.1.0", @@ -62,6 +64,7 @@ "@types/compression": "^1.7.0", "@types/http-errors": "^1.6.3", "@types/morgan": "^1.9.0", + "@types/node-fetch": "^2.5.7", "@types/stoppable": "^1.1.0", "@types/supertest": "^2.0.8", "@types/webpack-env": "^1.15.2", diff --git a/packages/backend-common/src/reading/GithubUrlReader.test.ts b/packages/backend-common/src/reading/GithubUrlReader.test.ts index 8eb46db967..513d66c09b 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.test.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.test.ts @@ -14,20 +14,19 @@ * limitations under the License. */ -import { getVoidLogger } from '@backstage/backend-common'; -import { LocationSpec } from '@backstage/catalog-model'; import { ConfigReader } from '@backstage/config'; +import { getVoidLogger } from '../logging'; import { getApiRequestOptions, getApiUrl, getRawRequestOptions, getRawUrl, - GithubReaderProcessor, + GithubUrlReader, ProviderConfig, readConfig, -} from './GithubReaderProcessor'; +} from './GithubUrlReader'; -describe('GithubReaderProcessor', () => { +describe('GithubUrlReader', () => { describe('getApiRequestOptions', () => { it('sets the correct API version', () => { const config: ProviderConfig = { target: '', apiBaseUrl: '' }; @@ -238,31 +237,15 @@ describe('GithubReaderProcessor', () => { }); describe('implementation', () => { - it('rejects unknown types', async () => { - const processor = new GithubReaderProcessor([ - { target: 'https://github.com', apiBaseUrl: 'https://api.github.com' }, - ]); - const location: LocationSpec = { - type: 'not-github', - target: 'https://github.com', - }; - await expect( - processor.readLocation(location, false, () => {}), - ).resolves.toBeFalsy(); - }); - it('rejects unknown targets', async () => { - const processor = new GithubReaderProcessor([ - { target: 'https://github.com', apiBaseUrl: 'https://api.github.com' }, - ]); - const location: LocationSpec = { - type: 'github', - target: 'https://not.github.com/apa', - }; + const processor = new GithubUrlReader({ + target: 'https://github.com', + apiBaseUrl: 'https://api.github.com', + }); await expect( - processor.readLocation(location, false, () => {}), + processor.read('https://not.github.com/apa'), ).rejects.toThrow( - /There is no GitHub provider that matches https:\/\/not.github.com\/apa/, + 'Incorrect URL: https://not.github.com/apa, Error: Invalid GitHub URL or file path', ); }); }); diff --git a/packages/backend-common/src/reading/GithubUrlReader.ts b/packages/backend-common/src/reading/GithubUrlReader.ts index cc50e53240..33b5c68cae 100644 --- a/packages/backend-common/src/reading/GithubUrlReader.ts +++ b/packages/backend-common/src/reading/GithubUrlReader.ts @@ -14,13 +14,12 @@ * limitations under the License. */ -import { LocationSpec } from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import parseGitUri from 'git-url-parse'; -import fetch, { HeadersInit, RequestInit } from 'node-fetch'; +import fetch, { HeadersInit, RequestInit, Response } from 'node-fetch'; import { Logger } from 'winston'; -import * as result from './results'; -import { LocationProcessor, LocationProcessorEmit } from './types'; +import { NotFoundError } from '../errors'; +import { UrlReader } from './types'; /** * The configuration parameters for a single GitHub API provider. @@ -101,7 +100,7 @@ export function getApiUrl(target: string, provider: ProviderConfig): URL { !ref || (filepathtype !== 'blob' && filepathtype !== 'raw') ) { - throw new Error('Wrong URL or invalid file path'); + throw new Error('Invalid GitHub URL or file path'); } const pathWithoutSlash = filepath.replace(/^\//, ''); @@ -126,7 +125,7 @@ export function getRawUrl(target: string, provider: ProviderConfig): URL { !ref || (filepathtype !== 'blob' && filepathtype !== 'raw') ) { - throw new Error('Wrong URL or invalid file path'); + throw new Error('Invalid GitHub URL or file path'); } const pathWithoutSlash = filepath.replace(/^\//, ''); @@ -205,65 +204,38 @@ export function readConfig(config: Config, logger: Logger): ProviderConfig[] { * A processor that adds the ability to read files from GitHub v3 APIs, such as * the one exposed by GitHub itself. */ -export class GithubReaderProcessor implements LocationProcessor { - private providers: ProviderConfig[]; +export class GithubUrlReader implements UrlReader { + private config: ProviderConfig; - static fromConfig(config: Config, logger: Logger) { - return new GithubReaderProcessor(readConfig(config, logger)); + constructor(config: ProviderConfig) { + this.config = config; } - constructor(providers: ProviderConfig[]) { - this.providers = providers; - } - - async readLocation( - location: LocationSpec, - optional: boolean, - emit: LocationProcessorEmit, - ): Promise { - // The github/api type is for backward compatibility - if (location.type !== 'github' && location.type !== 'github/api') { - return false; - } - - const provider = this.providers.find(p => - location.target.startsWith(`${p.target}/`), - ); - if (!provider) { - throw new Error( - `There is no GitHub provider that matches ${location.target}. Please add a configuration entry for it under catalog.processors.github.providers.`, - ); - } + async read(url: string): Promise { + const useApi = + this.config.apiBaseUrl && (this.config.token || !this.config.rawBaseUrl); + const ghUrl = useApi + ? getApiUrl(url, this.config) + : getRawUrl(url, this.config); + const options = useApi + ? getApiRequestOptions(this.config) + : getRawRequestOptions(this.config); + let response: Response; try { - const useApi = - provider.apiBaseUrl && (provider.token || !provider.rawBaseUrl); - const url = useApi - ? getApiUrl(location.target, provider) - : getRawUrl(location.target, provider); - const options = useApi - ? getApiRequestOptions(provider) - : getRawRequestOptions(provider); - const response = await fetch(url.toString(), options); - - if (response.ok) { - const data = await response.buffer(); - emit(result.data(location, data)); - } else { - const message = `${location.target} could not be read as ${url}, ${response.status} ${response.statusText}`; - if (response.status === 404) { - if (!optional) { - emit(result.notFoundError(location, message)); - } - } else { - emit(result.generalError(location, message)); - } - } + response = await fetch(ghUrl.toString(), options); } catch (e) { - const message = `Unable to read ${location.type} ${location.target}, ${e}`; - emit(result.generalError(location, message)); + throw new Error(`Unable to read ${url}, ${e}`); } - return true; + if (response.ok) { + return response.buffer(); + } + + const message = `${url} could not be read as ${ghUrl}, ${response.status} ${response.statusText}`; + if (response.status === 404) { + throw new NotFoundError(message); + } + throw new Error(message); } }