From 9bf8e5164d82ae5050c2cff28ccdbb71709952f7 Mon Sep 17 00:00:00 2001 From: Joey Perrott Date: Thu, 8 Apr 2021 12:34:55 -0700 Subject: [PATCH] refactor(dev-infra): use a singleton for GitClient (#41515) Creates a singleton class for GitClient rather than relying on creating an instance to require being passed around throughout its usages. PR Close #41515 --- dev-infra/caretaker/check/base.spec.ts | 6 +- dev-infra/caretaker/check/base.ts | 5 +- dev-infra/caretaker/check/check.ts | 10 +- dev-infra/caretaker/check/ci.spec.ts | 15 +- dev-infra/caretaker/check/cli.ts | 6 +- dev-infra/caretaker/check/g3.spec.ts | 18 +- dev-infra/caretaker/check/github.spec.ts | 32 +- dev-infra/caretaker/check/github.ts | 10 +- dev-infra/caretaker/check/services.spec.ts | 9 +- dev-infra/ng-dev.js | 279 ++++++++++-------- .../check-target-branches.ts | 4 +- dev-infra/pr/common/checkout-pr.ts | 28 +- dev-infra/pr/discover-new-conflicts/index.ts | 33 +-- dev-infra/pr/merge/cli.ts | 4 +- dev-infra/pr/merge/index.ts | 11 +- dev-infra/pr/merge/pull-request.ts | 22 +- dev-infra/pr/merge/strategies/api-merge.ts | 2 +- dev-infra/pr/merge/strategies/strategy.ts | 7 +- dev-infra/pr/merge/task.ts | 2 +- dev-infra/pr/rebase/index.ts | 27 +- dev-infra/release/publish/actions.ts | 10 +- dev-infra/release/publish/index.ts | 4 +- .../release/publish/pull-request-state.ts | 7 +- .../publish/release-notes/release-notes.ts | 2 +- dev-infra/release/publish/test/test-utils.ts | 4 +- dev-infra/utils/git/github-urls.ts | 2 +- dev-infra/utils/git/github-yargs.ts | 4 + dev-infra/utils/git/github.ts | 65 ++-- dev-infra/utils/git/index.ts | 90 ++++-- dev-infra/utils/github.ts | 12 +- dev-infra/utils/testing/virtual-git-client.ts | 28 +- .../utils/testing/virtual-git-matchers.ts | 4 +- 32 files changed, 439 insertions(+), 323 deletions(-) diff --git a/dev-infra/caretaker/check/base.spec.ts b/dev-infra/caretaker/check/base.spec.ts index 32f2d9e897..58af0c2cf8 100644 --- a/dev-infra/caretaker/check/base.spec.ts +++ b/dev-infra/caretaker/check/base.spec.ts @@ -5,6 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {installVirtualGitClientSpies} from '../../utils/testing'; import {BaseModule} from './base'; /** Data mocking as the "retrieved data". */ @@ -23,17 +24,18 @@ describe('BaseModule', () => { beforeEach(() => { retrieveDataSpy = spyOn(ConcreteBaseModule.prototype, 'retrieveData'); + installVirtualGitClientSpies(); }); it('begins retrieving data during construction', () => { - new ConcreteBaseModule({} as any, {} as any); + new ConcreteBaseModule({} as any); expect(retrieveDataSpy).toHaveBeenCalled(); }); it('makes the data available via the data attribute', async () => { retrieveDataSpy.and.callThrough(); - const module = new ConcreteBaseModule({} as any, {} as any); + const module = new ConcreteBaseModule({} as any); expect(await module.data).toBe(exampleData); }); diff --git a/dev-infra/caretaker/check/base.ts b/dev-infra/caretaker/check/base.ts index 746c15cead..e60634b007 100644 --- a/dev-infra/caretaker/check/base.ts +++ b/dev-infra/caretaker/check/base.ts @@ -11,11 +11,12 @@ import {CaretakerConfig} from '../config'; /** The BaseModule to extend modules for caretaker checks from. */ export abstract class BaseModule { + /** The singleton instance of the GitClient. */ + protected git = GitClient.getAuthenticatedInstance(); /** The data for the module. */ readonly data = this.retrieveData(); - constructor( - protected git: GitClient, protected config: NgDevConfig<{caretaker: CaretakerConfig}>) {} + constructor(protected config: NgDevConfig<{caretaker: CaretakerConfig}>) {} /** Asyncronously retrieve data for the module. */ protected abstract retrieveData(): Promise; diff --git a/dev-infra/caretaker/check/check.ts b/dev-infra/caretaker/check/check.ts index 4202059b2f..c16eff89fd 100644 --- a/dev-infra/caretaker/check/check.ts +++ b/dev-infra/caretaker/check/check.ts @@ -23,15 +23,13 @@ const moduleList = [ ]; /** Check the status of services which Angular caretakers need to monitor. */ -export async function checkServiceStatuses(githubToken: string) { +export async function checkServiceStatuses() { + // Set the verbose logging state of the GitClient. + GitClient.getAuthenticatedInstance().setVerboseLoggingState(false); /** The configuration for the caretaker commands. */ const config = getCaretakerConfig(); - /** The GitClient for interacting with git and Github. */ - const git = new GitClient(githubToken, config); - // Prevent logging of the git commands being executed during the check. - GitClient.LOG_COMMANDS = false; /** List of instances of Caretaker Check modules */ - const caretakerCheckModules = moduleList.map(module => new module(git, config)); + const caretakerCheckModules = moduleList.map(module => new module(config)); // Module's `data` is casted as Promise because the data types of the `module`'s `data` // promises do not match typings, however our usage here is only to determine when the promise diff --git a/dev-infra/caretaker/check/ci.spec.ts b/dev-infra/caretaker/check/ci.spec.ts index 74c7ef30f2..4b21f0968b 100644 --- a/dev-infra/caretaker/check/ci.spec.ts +++ b/dev-infra/caretaker/check/ci.spec.ts @@ -7,11 +7,11 @@ * found in the LICENSE file at https://angular.io/license */ import {SemVer} from 'semver'; -import {ReleaseTrain} from '../../release/versioning'; +import {ReleaseTrain} from '../../release/versioning'; import * as versioning from '../../release/versioning/active-release-trains'; import * as console from '../../utils/console'; -import {buildVirtualGitClient, mockNgDevConfig, VirtualGitClient} from '../../utils/testing'; +import {installVirtualGitClientSpies, mockNgDevConfig} from '../../utils/testing'; import {CiModule} from './ci'; @@ -20,10 +20,9 @@ describe('CiModule', () => { let getBranchStatusFromCiSpy: jasmine.Spy; let infoSpy: jasmine.Spy; let debugSpy: jasmine.Spy; - let virtualGitClient: VirtualGitClient; beforeEach(() => { - virtualGitClient = buildVirtualGitClient(); + installVirtualGitClientSpies(); fetchActiveReleaseTrainsSpy = spyOn(versioning, 'fetchActiveReleaseTrains'); getBranchStatusFromCiSpy = spyOn(CiModule.prototype, 'getBranchStatusFromCi' as any); infoSpy = spyOn(console, 'info'); @@ -34,7 +33,7 @@ describe('CiModule', () => { it('handles active rc train', async () => { const trains = buildMockActiveReleaseTrains(true); fetchActiveReleaseTrainsSpy.and.resolveTo(trains); - const module = new CiModule(virtualGitClient, {caretaker: {}, ...mockNgDevConfig}); + const module = new CiModule({caretaker: {}, ...mockNgDevConfig}); await module.data; expect(getBranchStatusFromCiSpy).toHaveBeenCalledWith(trains.releaseCandidate.branchName); @@ -46,7 +45,7 @@ describe('CiModule', () => { it('handles an inactive rc train', async () => { const trains = buildMockActiveReleaseTrains(false); fetchActiveReleaseTrainsSpy.and.resolveTo(trains); - const module = new CiModule(virtualGitClient, {caretaker: {}, ...mockNgDevConfig}); + const module = new CiModule({caretaker: {}, ...mockNgDevConfig}); await module.data; expect(getBranchStatusFromCiSpy).toHaveBeenCalledWith(trains.latest.branchName); @@ -58,7 +57,7 @@ describe('CiModule', () => { const trains = buildMockActiveReleaseTrains(false); fetchActiveReleaseTrainsSpy.and.resolveTo(trains); getBranchStatusFromCiSpy.and.returnValue('success'); - const module = new CiModule(virtualGitClient, {caretaker: {}, ...mockNgDevConfig}); + const module = new CiModule({caretaker: {}, ...mockNgDevConfig}); const data = await module.data; expect(data[0]).toEqual( @@ -89,7 +88,7 @@ describe('CiModule', () => { ]); fetchActiveReleaseTrainsSpy.and.resolveTo([]); - const module = new CiModule(virtualGitClient, {caretaker: {}, ...mockNgDevConfig}); + const module = new CiModule({caretaker: {}, ...mockNgDevConfig}); Object.defineProperty(module, 'data', {value: fakeData}); await module.printToTerminal(); diff --git a/dev-infra/caretaker/check/cli.ts b/dev-infra/caretaker/check/cli.ts index 1988381423..92343638ec 100644 --- a/dev-infra/caretaker/check/cli.ts +++ b/dev-infra/caretaker/check/cli.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Arguments, Argv, CommandModule} from 'yargs'; +import {Argv, CommandModule} from 'yargs'; import {addGithubTokenOption} from '../../utils/git/github-yargs'; @@ -23,8 +23,8 @@ function builder(yargs: Argv) { } /** Handles the command. */ -async function handler({githubToken}: Arguments) { - await checkServiceStatuses(githubToken); +async function handler() { + await checkServiceStatuses(); } /** yargs command module for checking status information for the repository */ diff --git a/dev-infra/caretaker/check/g3.spec.ts b/dev-infra/caretaker/check/g3.spec.ts index e3b7d737ff..3fc9bec36c 100644 --- a/dev-infra/caretaker/check/g3.spec.ts +++ b/dev-infra/caretaker/check/g3.spec.ts @@ -9,7 +9,8 @@ import {SpawnSyncReturns} from 'child_process'; import * as console from '../../utils/console'; -import {buildVirtualGitClient, mockNgDevConfig, VirtualGitClient} from '../../utils/testing'; +import {GitClient} from '../../utils/git'; +import {installVirtualGitClientSpies, mockNgDevConfig} from '../../utils/testing'; import {G3Module, G3StatsData} from './g3'; @@ -18,10 +19,9 @@ describe('G3Module', () => { let getLatestShas: jasmine.Spy; let getDiffStats: jasmine.Spy; let infoSpy: jasmine.Spy; - let virtualGitClient: VirtualGitClient; beforeEach(() => { - virtualGitClient = buildVirtualGitClient(); + installVirtualGitClientSpies(); getG3FileIncludeAndExcludeLists = spyOn(G3Module.prototype, 'getG3FileIncludeAndExcludeLists' as any).and.returnValue(null); getLatestShas = spyOn(G3Module.prototype, 'getLatestShas' as any).and.returnValue(null); @@ -33,7 +33,7 @@ describe('G3Module', () => { it('unless the g3 merge config is not defined in the angular robot file', async () => { getG3FileIncludeAndExcludeLists.and.returnValue(null); getLatestShas.and.returnValue({g3: 'abc123', master: 'zxy987'}); - const module = new G3Module(virtualGitClient, {caretaker: {}, ...mockNgDevConfig}); + const module = new G3Module({caretaker: {}, ...mockNgDevConfig}); expect(getDiffStats).not.toHaveBeenCalled(); expect(await module.data).toBe(undefined); @@ -42,7 +42,7 @@ describe('G3Module', () => { it('unless the branch shas are not able to be retrieved', async () => { getLatestShas.and.returnValue(null); getG3FileIncludeAndExcludeLists.and.returnValue({include: ['file1'], exclude: []}); - const module = new G3Module(virtualGitClient, {caretaker: {}, ...mockNgDevConfig}); + const module = new G3Module({caretaker: {}, ...mockNgDevConfig}); expect(getDiffStats).not.toHaveBeenCalled(); expect(await module.data).toBe(undefined); @@ -52,7 +52,7 @@ describe('G3Module', () => { getLatestShas.and.returnValue({g3: 'abc123', master: 'zxy987'}); getG3FileIncludeAndExcludeLists.and.returnValue({include: ['project1/*'], exclude: []}); getDiffStats.and.callThrough(); - spyOn(virtualGitClient, 'run').and.callFake((args: string[]): any => { + spyOn(GitClient.prototype, 'run').and.callFake((args: string[]): any => { const output: Partial> = {}; if (args[0] === 'rev-list') { output.stdout = '3'; @@ -63,7 +63,7 @@ describe('G3Module', () => { return output; }); - const module = new G3Module(virtualGitClient, {caretaker: {}, ...mockNgDevConfig}); + const module = new G3Module({caretaker: {}, ...mockNgDevConfig}); const {insertions, deletions, files, commits} = (await module.data) as G3StatsData; expect(insertions).toBe(12); @@ -82,7 +82,7 @@ describe('G3Module', () => { commits: 2, }); - const module = new G3Module(virtualGitClient, {caretaker: {}, ...mockNgDevConfig}); + const module = new G3Module({caretaker: {}, ...mockNgDevConfig}); Object.defineProperty(module, 'data', {value: fakeData}); await module.printToTerminal(); @@ -98,7 +98,7 @@ describe('G3Module', () => { commits: 25, }); - const module = new G3Module(virtualGitClient, {caretaker: {}, ...mockNgDevConfig}); + const module = new G3Module({caretaker: {}, ...mockNgDevConfig}); Object.defineProperty(module, 'data', {value: fakeData}); await module.printToTerminal(); diff --git a/dev-infra/caretaker/check/github.spec.ts b/dev-infra/caretaker/check/github.spec.ts index 452e5595bf..9dfff39975 100644 --- a/dev-infra/caretaker/check/github.spec.ts +++ b/dev-infra/caretaker/check/github.spec.ts @@ -7,8 +7,8 @@ * found in the LICENSE file at https://angular.io/license */ import * as console from '../../utils/console'; -import {GithubGraphqlClient} from '../../utils/git/github'; -import {buildVirtualGitClient, mockNgDevConfig, VirtualGitClient} from '../../utils/testing'; +import {GithubClient} from '../../utils/git/github'; +import {installVirtualGitClientSpies, mockNgDevConfig} from '../../utils/testing'; import {GithubQueriesModule} from './github'; @@ -16,37 +16,35 @@ describe('GithubQueriesModule', () => { let githubApiSpy: jasmine.Spy; let infoSpy: jasmine.Spy; let infoGroupSpy: jasmine.Spy; - let virtualGitClient: VirtualGitClient; beforeEach(() => { - githubApiSpy = spyOn(GithubGraphqlClient.prototype, 'query') + githubApiSpy = spyOn(GithubClient.prototype, 'graphql') .and.throwError( 'The graphql query response must always be manually defined in a test.'); - virtualGitClient = buildVirtualGitClient(); + installVirtualGitClientSpies(); infoGroupSpy = spyOn(console.info, 'group'); infoSpy = spyOn(console, 'info'); }); describe('gathering stats', () => { it('unless githubQueries are `undefined`', async () => { - const module = new GithubQueriesModule( - virtualGitClient, {...mockNgDevConfig, caretaker: {githubQueries: undefined}}); + const module = + new GithubQueriesModule({...mockNgDevConfig, caretaker: {githubQueries: undefined}}); expect(await module.data).toBe(undefined); }); it('unless githubQueries are an empty array', async () => { - const module = new GithubQueriesModule( - virtualGitClient, {...mockNgDevConfig, caretaker: {githubQueries: []}}); + const module = new GithubQueriesModule({...mockNgDevConfig, caretaker: {githubQueries: []}}); expect(await module.data).toBe(undefined); }); - it('for the requestd Github queries', async () => { + it('for the requested Github queries', async () => { githubApiSpy.and.returnValue({ - 'keynamewithspaces': {issueCount: 1, nodes: [{url: 'http://gituhb.com/owner/name/issue/1'}]} + 'keynamewithspaces': {issueCount: 1, nodes: [{url: 'http://github.com/owner/name/issue/1'}]} }); - const module = new GithubQueriesModule(virtualGitClient, { + const module = new GithubQueriesModule({ ...mockNgDevConfig, caretaker: {githubQueries: [{name: 'key name with spaces', query: 'issue: yes'}]} }); @@ -55,7 +53,7 @@ describe('GithubQueriesModule', () => { queryName: 'key name with spaces', count: 1, queryUrl: 'https://github.com/owner/name/issues?q=issue:%20yes', - matchedUrls: ['http://gituhb.com/owner/name/issue/1'], + matchedUrls: ['http://github.com/owner/name/issue/1'], }]); }); }); @@ -78,7 +76,7 @@ describe('GithubQueriesModule', () => { ]); - const module = new GithubQueriesModule(virtualGitClient, {caretaker: {}, ...mockNgDevConfig}); + const module = new GithubQueriesModule({caretaker: {}, ...mockNgDevConfig}); Object.defineProperty(module, 'data', {value: fakeData}); await module.printToTerminal(); @@ -95,7 +93,7 @@ describe('GithubQueriesModule', () => { queryName: 'query1', count: 1, queryUrl: 'https://github.com/owner/name/issues?q=issue:%20yes', - matchedUrls: ['http://gituhb.com/owner/name/issue/1'], + matchedUrls: ['http://github.com/owner/name/issue/1'], }, { queryName: 'query2', @@ -105,7 +103,7 @@ describe('GithubQueriesModule', () => { }, ]); - const module = new GithubQueriesModule(virtualGitClient, {caretaker: {}, ...mockNgDevConfig}); + const module = new GithubQueriesModule({caretaker: {}, ...mockNgDevConfig}); Object.defineProperty(module, 'data', {value: fakeData}); await module.printToTerminal(); @@ -114,7 +112,7 @@ describe('GithubQueriesModule', () => { expect(infoSpy).toHaveBeenCalledWith('query1 1'); expect(infoGroupSpy) .toHaveBeenCalledWith('https://github.com/owner/name/issues?q=issue:%20yes'); - expect(infoSpy).toHaveBeenCalledWith('- http://gituhb.com/owner/name/issue/1'); + expect(infoSpy).toHaveBeenCalledWith('- http://github.com/owner/name/issue/1'); expect(infoSpy).toHaveBeenCalledWith('query2 0'); }); }); diff --git a/dev-infra/caretaker/check/github.ts b/dev-infra/caretaker/check/github.ts index 1b5be056e9..f0178c5ec1 100644 --- a/dev-infra/caretaker/check/github.ts +++ b/dev-infra/caretaker/check/github.ts @@ -56,7 +56,7 @@ export class GithubQueriesModule extends BaseModule { } /** The results of the generated github query. */ - const queryResult = await this.git.github.graphql.query(this.buildGraphqlQuery(queries)); + const queryResult = await this.git.github.graphql(this.buildGraphqlQuery(queries)); const results = Object.values(queryResult); const {owner, name: repo} = this.git.remoteConfig; @@ -74,16 +74,16 @@ export class GithubQueriesModule extends BaseModule { /** Build a Graphql query statement for the provided queries. */ private buildGraphqlQuery(queries: NonNullable) { /** The query object for graphql. */ - const graphQlQuery: GithubQueryResult = {}; + const graphqlQuery: GithubQueryResult = {}; const {owner, name: repo} = this.git.remoteConfig; /** The Github search filter for the configured repository. */ const repoFilter = `repo:${owner}/${repo}`; queries.forEach(({name, query}) => { - /** The name of the query, with spaces removed to match GraphQL requirements. */ + /** The name of the query, with spaces removed to match Graphql requirements. */ const queryKey = alias(name.replace(/ /g, ''), 'search'); - graphQlQuery[queryKey] = params( + graphqlQuery[queryKey] = params( { type: 'ISSUE', first: MAX_RETURNED_ISSUES, @@ -92,7 +92,7 @@ export class GithubQueriesModule extends BaseModule { {...GithubQueryResultFragment}); }); - return graphQlQuery; + return graphqlQuery; } async printToTerminal() { diff --git a/dev-infra/caretaker/check/services.spec.ts b/dev-infra/caretaker/check/services.spec.ts index ec2b64f7be..6c4688fe7d 100644 --- a/dev-infra/caretaker/check/services.spec.ts +++ b/dev-infra/caretaker/check/services.spec.ts @@ -8,7 +8,7 @@ */ import * as console from '../../utils/console'; -import {buildVirtualGitClient, mockNgDevConfig, VirtualGitClient} from '../../utils/testing'; +import {installVirtualGitClientSpies, mockNgDevConfig} from '../../utils/testing'; import {services, ServicesModule} from './services'; @@ -16,20 +16,19 @@ describe('ServicesModule', () => { let getStatusFromStandardApiSpy: jasmine.Spy; let infoSpy: jasmine.Spy; let infoGroupSpy: jasmine.Spy; - let virtualGitClient: VirtualGitClient; services.splice(0, Infinity, {url: 'fakeStatus.com/api.json', name: 'Service Name'}); beforeEach(() => { getStatusFromStandardApiSpy = spyOn(ServicesModule.prototype, 'getStatusFromStandardApi'); - virtualGitClient = buildVirtualGitClient(); + installVirtualGitClientSpies(); infoGroupSpy = spyOn(console.info, 'group'); infoSpy = spyOn(console, 'info'); }); describe('gathering status', () => { it('for each of the services', async () => { - new ServicesModule(virtualGitClient, {caretaker: {}, ...mockNgDevConfig}); + new ServicesModule({caretaker: {}, ...mockNgDevConfig}); expect(getStatusFromStandardApiSpy) .toHaveBeenCalledWith({url: 'fakeStatus.com/api.json', name: 'Service Name'}); @@ -54,7 +53,7 @@ describe('ServicesModule', () => { ]); - const module = new ServicesModule(virtualGitClient, {caretaker: {}, ...mockNgDevConfig}); + const module = new ServicesModule({caretaker: {}, ...mockNgDevConfig}); Object.defineProperty(module, 'data', {value: fakeData}); await module.printToTerminal(); diff --git a/dev-infra/ng-dev.js b/dev-infra/ng-dev.js index 5d86f09a9f..a66620f986 100755 --- a/dev-infra/ng-dev.js +++ b/dev-infra/ng-dev.js @@ -405,36 +405,6 @@ function getListCommitsInBranchUrl(_a, branchName) { return "https://github.com/" + remoteParams.owner + "/" + remoteParams.repo + "/commits/" + branchName; } -/** - * @license - * Copyright Google LLC All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ -/** Sets up the `github-token` command option for the given Yargs instance. */ -function addGithubTokenOption(yargs) { - return yargs - // 'github-token' is casted to 'githubToken' to properly set up typings to reflect the key in - // the Argv object being camelCase rather than kebob case due to the `camel-case-expansion` - // config: https://github.com/yargs/yargs-parser#camel-case-expansion - .option('github-token', { - type: 'string', - description: 'Github token. If not set, token is retrieved from the environment variables.', - coerce: function (token) { - var githubToken = token || process.env.GITHUB_TOKEN || process.env.TOKEN; - if (!githubToken) { - error(red('No Github token set. Please set the `GITHUB_TOKEN` environment variable.')); - error(red('Alternatively, pass the `--github-token` command line flag.')); - error(yellow("You can generate a token here: " + GITHUB_TOKEN_GENERATE_URL)); - process.exit(1); - } - return githubToken; - }, - }) - .default('github-token', '', ''); -} - /** * @license * Copyright Google LLC All Rights Reserved. @@ -477,6 +447,14 @@ var GithubApiRequestError = /** @class */ (function (_super) { } return GithubApiRequestError; }(Error)); +/** Error for failed Github API requests. */ +var GithubGraphqlClientError = /** @class */ (function (_super) { + tslib.__extends(GithubGraphqlClientError, _super); + function GithubGraphqlClientError() { + return _super !== null && _super.apply(this, arguments) || this; + } + return GithubGraphqlClientError; +}(Error)); /** * A Github client for interacting with the Github APIs. * @@ -485,21 +463,48 @@ var GithubApiRequestError = /** @class */ (function (_super) { **/ var GithubClient = /** @class */ (function (_super) { tslib.__extends(GithubClient, _super); + /** + * @param token The github authentication token for Github Rest and Graphql API requests. + */ function GithubClient(token) { var _this = // Pass in authentication token to base Octokit class. _super.call(this, { auth: token }) || this; + _this.token = token; /** The current user based on checking against the Github API. */ _this._currentUser = null; + /** The graphql instance with authentication set during construction. */ + _this._graphql = graphql.graphql.defaults({ headers: { authorization: "token " + _this.token } }); _this.hook.error('request', function (error) { // Wrap API errors in a known error class. This allows us to // expect Github API errors better and in a non-ambiguous way. throw new GithubApiRequestError(error.status, error.message); }); - // Create authenticated graphql client. - _this.graphql = new GithubGraphqlClient(token); + // Note: The prototype must be set explictly as Github's Octokit class is a non-standard class + // definition which adjusts the prototype chain. + // See: + // https://github.com/Microsoft/TypeScript/wiki/FAQ#why-doesnt-extending-built-ins-like-error-array-and-map-work + // https://github.com/octokit/rest.js/blob/7b51cee4a22b6e52adcdca011f93efdffa5df998/lib/constructor.js + Object.setPrototypeOf(_this, GithubClient.prototype); return _this; } + /** Perform a query using Github's Graphql API. */ + GithubClient.prototype.graphql = function (queryObject, params) { + if (params === void 0) { params = {}; } + return tslib.__awaiter(this, void 0, void 0, function () { + return tslib.__generator(this, function (_a) { + switch (_a.label) { + case 0: + if (this.token === undefined) { + throw new GithubGraphqlClientError('Cannot query via graphql without an authentication token set, use the authenticated ' + + '`GitClient` by calling `GitClient.getAuthenticatedInstance()`.'); + } + return [4 /*yield*/, this._graphql(typedGraphqlify.query(queryObject), params)]; + case 1: return [2 /*return*/, (_a.sent())]; + } + }); + }); + }; /** Retrieve the login of the current user from Github. */ GithubClient.prototype.getCurrentUser = function () { return tslib.__awaiter(this, void 0, void 0, function () { @@ -511,7 +516,7 @@ var GithubClient = /** @class */ (function (_super) { if (this._currentUser !== null) { return [2 /*return*/, this._currentUser]; } - return [4 /*yield*/, this.graphql.query({ + return [4 /*yield*/, this.graphql({ viewer: { login: typedGraphqlify.types.string, } @@ -525,34 +530,6 @@ var GithubClient = /** @class */ (function (_super) { }; return GithubClient; }(Octokit)); -/** A client for interacting with Github's GraphQL API. */ -var GithubGraphqlClient = /** @class */ (function () { - function GithubGraphqlClient(token) { - /** The Github GraphQL (v4) API. */ - this.graqhql = graphql.graphql; - // Set the default headers to include authorization with the provided token for all - // graphQL calls. - if (token) { - this.graqhql = this.graqhql.defaults({ headers: { authorization: "token " + token } }); - } - } - /** Perform a query using Github's GraphQL API. */ - GithubGraphqlClient.prototype.query = function (queryObject, params) { - if (params === void 0) { params = {}; } - return tslib.__awaiter(this, void 0, void 0, function () { - var queryString; - return tslib.__generator(this, function (_a) { - switch (_a.label) { - case 0: - queryString = typedGraphqlify.query(queryObject); - return [4 /*yield*/, this.graqhql(queryString, params)]; - case 1: return [2 /*return*/, (_a.sent())]; - } - }); - }); - }; - return GithubGraphqlClient; -}()); /** * @license @@ -585,20 +562,19 @@ var GitCommandError = /** @class */ (function (_super) { * the dev-infra configuration is loaded with its Github configuration. **/ var GitClient = /** @class */ (function () { + /** + * @param githubToken The github token used for authentication, if provided. + * @param _config The configuration, containing the github specific configuration. + * @param _projectRoot The full path to the root of the repository base. + */ function GitClient(githubToken, _config, _projectRoot) { if (_config === void 0) { _config = getConfig(); } if (_projectRoot === void 0) { _projectRoot = getRepoBaseDir(); } this.githubToken = githubToken; this._config = _config; this._projectRoot = _projectRoot; - /** Short-hand for accessing the default remote configuration. */ - this.remoteConfig = this._config.github; - /** Octokit request parameters object for targeting the configured remote. */ - this.remoteParams = { owner: this.remoteConfig.owner, repo: this.remoteConfig.name }; - /** Git URL that resolves to the configured repository. */ - this.repoGitUrl = getRepositoryGitUrl(this.remoteConfig, this.githubToken); - /** Instance of the authenticated Github octokit API. */ - this.github = new GithubClient(this.githubToken); + /** Whether verbose logging of Git actions should be used. */ + this.verboseLogging = true; /** The OAuth scopes available for the provided Github token. */ this._cachedOauthScopes = null; /** @@ -606,13 +582,51 @@ var GitClient = /** @class */ (function () { * sanitizing the token from Git child process output. */ this._githubTokenRegex = null; + /** Short-hand for accessing the default remote configuration. */ + this.remoteConfig = this._config.github; + /** Octokit request parameters object for targeting the configured remote. */ + this.remoteParams = { owner: this.remoteConfig.owner, repo: this.remoteConfig.name }; + /** Instance of the authenticated Github octokit API. */ + this.github = new GithubClient(this.githubToken); // If a token has been specified (and is not empty), pass it to the Octokit API and // also create a regular expression that can be used for sanitizing Git command output // so that it does not print the token accidentally. - if (githubToken != null) { + if (typeof githubToken === 'string') { this._githubTokenRegex = new RegExp(githubToken, 'g'); } } + /** + * Static method to get the singleton instance of the unauthorized GitClient, creating it if it + * has not yet been created. + */ + GitClient.getInstance = function () { + if (!GitClient.unauthenticated) { + GitClient.unauthenticated = new GitClient(undefined); + } + return GitClient.unauthenticated; + }; + /** + * Static method to get the singleton instance of the authenticated GitClient if it has been + * generated. + */ + GitClient.getAuthenticatedInstance = function () { + if (!GitClient.authenticated) { + throw Error('The authenticated GitClient has not yet been generated.'); + } + return GitClient.authenticated; + }; + /** Build the authenticated GitClient instance. */ + GitClient.authenticateWithToken = function (token) { + if (GitClient.authenticated) { + throw Error('Cannot generate new authenticated GitClient after one has already been generated.'); + } + GitClient.authenticated = new GitClient(token); + }; + /** Set the verbose logging state of the GitClient instance. */ + GitClient.prototype.setVerboseLoggingState = function (verbose) { + this.verboseLogging = verbose; + return this; + }; /** Executes the given git command. Throws if the command fails. */ GitClient.prototype.run = function (args, options) { var result = this.runGraceful(args, options); @@ -639,7 +653,7 @@ var GitClient = /** @class */ (function () { // To improve the debugging experience in case something fails, we print all executed Git // commands to better understand the git actions occuring. Depending on the command being // executed, this debugging information should be logged at different logging levels. - var printFn = (!GitClient.LOG_COMMANDS || options.stdio === 'ignore') ? debug : info; + var printFn = (!this.verboseLogging || options.stdio === 'ignore') ? debug : info; // Note that we do not want to print the token if it is contained in the command. It's common // to share errors with others if the tool failed, and we do not want to leak tokens. printFn('Executing: git', this.omitGithubTokenFromMessage(args.join(' '))); @@ -655,6 +669,10 @@ var GitClient = /** @class */ (function () { } return result; }; + /** Git URL that resolves to the configured repository. */ + GitClient.prototype.getRepoGitUrl = function () { + return getRepositoryGitUrl(this.remoteConfig, this.githubToken); + }; /** Whether the given branch contains the specified SHA. */ GitClient.prototype.hasCommit = function (branchName, sha) { return this.run(['branch', branchName, '--contains', sha]).stdout !== ''; @@ -760,11 +778,40 @@ var GitClient = /** @class */ (function () { return scopes.split(',').map(function (scope) { return scope.trim(); }); }); }; - /** Whether verbose logging of Git actions should be used. */ - GitClient.LOG_COMMANDS = true; return GitClient; }()); +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ +/** Sets up the `github-token` command option for the given Yargs instance. */ +function addGithubTokenOption(yargs) { + return yargs + // 'github-token' is casted to 'githubToken' to properly set up typings to reflect the key in + // the Argv object being camelCase rather than kebob case due to the `camel-case-expansion` + // config: https://github.com/yargs/yargs-parser#camel-case-expansion + .option('github-token', { + type: 'string', + description: 'Github token. If not set, token is retrieved from the environment variables.', + coerce: function (token) { + var githubToken = token || process.env.GITHUB_TOKEN || process.env.TOKEN; + if (!githubToken) { + error(red('No Github token set. Please set the `GITHUB_TOKEN` environment variable.')); + error(red('Alternatively, pass the `--github-token` command line flag.')); + error(yellow("You can generate a token here: " + GITHUB_TOKEN_GENERATE_URL)); + process.exit(1); + } + GitClient.authenticateWithToken(githubToken); + return githubToken; + }, + }) + .default('github-token', '', ''); +} + /** * @license * Copyright Google LLC All Rights Reserved. @@ -1091,9 +1138,10 @@ function getLtsNpmDistTagOfMajor(major) { /** The BaseModule to extend modules for caretaker checks from. */ class BaseModule { - constructor(git, config) { - this.git = git; + constructor(config) { this.config = config; + /** The singleton instance of the GitClient. */ + this.git = GitClient.getAuthenticatedInstance(); /** The data for the module. */ this.data = this.retrieveData(); } @@ -1332,7 +1380,7 @@ class GithubQueriesModule extends BaseModule { return; } /** The results of the generated github query. */ - const queryResult = yield this.git.github.graphql.query(this.buildGraphqlQuery(queries)); + const queryResult = yield this.git.github.graphql(this.buildGraphqlQuery(queries)); const results = Object.values(queryResult); const { owner, name: repo } = this.git.remoteConfig; return results.map((result, i) => { @@ -1348,20 +1396,20 @@ class GithubQueriesModule extends BaseModule { /** Build a Graphql query statement for the provided queries. */ buildGraphqlQuery(queries) { /** The query object for graphql. */ - const graphQlQuery = {}; + const graphqlQuery = {}; const { owner, name: repo } = this.git.remoteConfig; /** The Github search filter for the configured repository. */ const repoFilter = `repo:${owner}/${repo}`; queries.forEach(({ name, query }) => { - /** The name of the query, with spaces removed to match GraphQL requirements. */ + /** The name of the query, with spaces removed to match Graphql requirements. */ const queryKey = typedGraphqlify.alias(name.replace(/ /g, ''), 'search'); - graphQlQuery[queryKey] = typedGraphqlify.params({ + graphqlQuery[queryKey] = typedGraphqlify.params({ type: 'ISSUE', first: MAX_RETURNED_ISSUES, query: `"${repoFilter} ${query.replace(/"/g, '\\"')}"`, }, Object.assign({}, GithubQueryResultFragment)); }); - return graphQlQuery; + return graphqlQuery; } printToTerminal() { return tslib.__awaiter(this, void 0, void 0, function* () { @@ -1470,16 +1518,14 @@ const moduleList = [ G3Module, ]; /** Check the status of services which Angular caretakers need to monitor. */ -function checkServiceStatuses(githubToken) { +function checkServiceStatuses() { return tslib.__awaiter(this, void 0, void 0, function* () { + // Set the verbose logging state of the GitClient. + GitClient.getAuthenticatedInstance().setVerboseLoggingState(false); /** The configuration for the caretaker commands. */ const config = getCaretakerConfig(); - /** The GitClient for interacting with git and Github. */ - const git = new GitClient(githubToken, config); - // Prevent logging of the git commands being executed during the check. - GitClient.LOG_COMMANDS = false; /** List of instances of Caretaker Check modules */ - const caretakerCheckModules = moduleList.map(module => new module(git, config)); + const caretakerCheckModules = moduleList.map(module => new module(config)); // Module's `data` is casted as Promise because the data types of the `module`'s `data` // promises do not match typings, however our usage here is only to determine when the promise // resolves. @@ -1502,9 +1548,9 @@ function builder(yargs) { return addGithubTokenOption(yargs); } /** Handles the command. */ -function handler({ githubToken }) { +function handler() { return tslib.__awaiter(this, void 0, void 0, function* () { - yield checkServiceStatuses(githubToken); + yield checkServiceStatuses(); }); } /** yargs command module for checking status information for the repository */ @@ -2836,8 +2882,8 @@ function getTargetBranchesForPr(prNumber) { const config = getConfig(); /** Repo owner and name for the github repository. */ const { owner, name: repo } = config.github; - /** The git client to get a Github API service instance. */ - const git = new GitClient(undefined, config); + /** The singleton instance of the GitClient. */ + const git = GitClient.getInstance(); /** The validated merge config. */ const { config: mergeConfig, errors } = yield loadAndValidateConfig(config, git.github); if (errors !== undefined) { @@ -2931,7 +2977,7 @@ function getPr(prSchema, prNumber, git) { pullRequest: typedGraphqlify.params({ number: '$number' }, prSchema), }) }); - return [4 /*yield*/, git.github.graphql.query(PR_QUERY, { number: prNumber, owner: owner, name: name })]; + return [4 /*yield*/, git.github.graphql(PR_QUERY, { number: prNumber, owner: owner, name: name })]; case 1: result = (_b.sent()); return [2 /*return*/, result.repository.pullRequest]; @@ -2978,7 +3024,7 @@ function getPendingPrs(prSchema, git) { owner: owner, name: name, }; - return [4 /*yield*/, git.github.graphql.query(PRS_QUERY, params_1)]; + return [4 /*yield*/, git.github.graphql(PRS_QUERY, params_1)]; case 2: results = _b.sent(); prs.push.apply(prs, tslib.__spreadArray([], tslib.__read(results.repository.pullRequests.nodes))); @@ -2998,7 +3044,7 @@ function getPendingPrs(prSchema, git) { * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -/* GraphQL schema for the response body for a pending PR. */ +/* Graphql schema for the response body for a pending PR. */ const PR_SCHEMA = { state: typedGraphqlify.types.string, maintainerCanModify: typedGraphqlify.types.boolean, @@ -3037,8 +3083,8 @@ class MaintainerModifyAccessError extends Error { */ function checkOutPullRequestLocally(prNumber, githubToken, opts = {}) { return tslib.__awaiter(this, void 0, void 0, function* () { - /** Authenticated Git client for git and Github interactions. */ - const git = new GitClient(githubToken); + /** The singleton instance of the GitClient. */ + const git = GitClient.getAuthenticatedInstance(); // In order to preserve local changes, checkouts cannot occur if local changes are present in the // git environment. Checked before retrieving the PR to fail fast. if (git.hasLocalChanges()) { @@ -3132,7 +3178,7 @@ const CheckoutCommandModule = { * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -/* GraphQL schema for the response body for each pending PR. */ +/* Graphql schema for the response body for each pending PR. */ const PR_SCHEMA$1 = { headRef: { name: typedGraphqlify.types.string, @@ -3160,9 +3206,10 @@ function processPr(pr) { /** Name of a temporary local branch that is used for checking conflicts. **/ const tempWorkingBranch = '__NgDevRepoBaseAfterChange__'; /** Checks if the provided PR will cause new conflicts in other pending PRs. */ -function discoverNewConflictsForPr(newPrNumber, updatedAfter, config = getConfig()) { +function discoverNewConflictsForPr(newPrNumber, updatedAfter) { return tslib.__awaiter(this, void 0, void 0, function* () { - const git = new GitClient(); + /** The singleton instance of the GitClient. */ + const git = GitClient.getAuthenticatedInstance(); // If there are any local changes in the current repository state, the // check cannot run as it needs to move between branches. if (git.hasLocalChanges()) { @@ -3486,7 +3533,7 @@ function loadAndValidatePullRequest(_a, prNumber, ignoreNonFatalFailures) { }); }); } -/* GraphQL schema for the response body the requested pull request. */ +/* Graphql schema for the response body the requested pull request. */ var PR_SCHEMA$2 = { url: typedGraphqlify.types.string, number: typedGraphqlify.types.number, @@ -3723,7 +3770,7 @@ var MergeStrategy = /** @class */ (function () { }); // Fetch all target branches with a single command. We don't want to fetch them // individually as that could cause an unnecessary slow-down. - this.git.run(tslib.__spreadArray(tslib.__spreadArray(['fetch', '-q', '-f', this.git.repoGitUrl], tslib.__read(fetchRefspecs)), tslib.__read(extraRefspecs))); + this.git.run(tslib.__spreadArray(tslib.__spreadArray(['fetch', '-q', '-f', this.git.getRepoGitUrl()], tslib.__read(fetchRefspecs)), tslib.__read(extraRefspecs))); }; /** Pushes the given target branches upstream. */ MergeStrategy.prototype.pushTargetBranchesUpstream = function (names) { @@ -3734,7 +3781,7 @@ var MergeStrategy = /** @class */ (function () { }); // Push all target branches with a single command if we don't run in dry-run mode. // We don't want to push them individually as that could cause an unnecessary slow-down. - this.git.run(tslib.__spreadArray(['push', this.git.repoGitUrl], tslib.__read(pushRefspecs))); + this.git.run(tslib.__spreadArray(['push', this.git.getRepoGitUrl()], tslib.__read(pushRefspecs))); }; return MergeStrategy; }()); @@ -4206,7 +4253,7 @@ var PullRequestMergeTask = /** @class */ (function () { * @param projectRoot Path to the local Git project that is used for merging. * @param config Configuration for merging pull requests. */ -function mergePullRequest(prNumber, githubToken, flags) { +function mergePullRequest(prNumber, flags) { return tslib.__awaiter(this, void 0, void 0, function () { /** Performs the merge and returns whether it was successful or not. */ function performMerge(ignoreFatalErrors) { @@ -4322,7 +4369,7 @@ function mergePullRequest(prNumber, githubToken, flags) { // Set the environment variable to skip all git commit hooks triggered by husky. We are unable to // rely on `--no-verify` as some hooks still run, notably the `prepare-commit-msg` hook. process.env['HUSKY'] = '0'; - return [4 /*yield*/, createPullRequestMergeTask(githubToken, flags)]; + return [4 /*yield*/, createPullRequestMergeTask(flags)]; case 1: api = _a.sent(); return [4 /*yield*/, performMerge(false)]; @@ -4343,15 +4390,14 @@ function mergePullRequest(prNumber, githubToken, flags) { * and optional explicit configuration. An explicit configuration can be specified * when the merge script is used outside of a `ng-dev` configured repository. */ -function createPullRequestMergeTask(githubToken, flags) { +function createPullRequestMergeTask(flags) { return tslib.__awaiter(this, void 0, void 0, function () { - var projectRoot, devInfraConfig, git, _a, config, errors; + var devInfraConfig, git, _a, config, errors; return tslib.__generator(this, function (_b) { switch (_b.label) { case 0: - projectRoot = getRepoBaseDir(); devInfraConfig = getConfig(); - git = new GitClient(githubToken, devInfraConfig, projectRoot); + git = GitClient.getAuthenticatedInstance(); return [4 /*yield*/, loadAndValidateConfig(devInfraConfig, git.github)]; case 1: _a = _b.sent(), config = _a.config, errors = _a.errors; @@ -4396,11 +4442,11 @@ function builder$6(yargs) { } /** Handles the command. */ function handler$6(_a) { - var pr = _a.pr, githubToken = _a.githubToken, branchPrompt = _a.branchPrompt; + var pr = _a.pr, branchPrompt = _a.branchPrompt; return tslib.__awaiter(this, void 0, void 0, function () { return tslib.__generator(this, function (_b) { switch (_b.label) { - case 0: return [4 /*yield*/, mergePullRequest(pr, githubToken, { branchPrompt: branchPrompt })]; + case 0: return [4 /*yield*/, mergePullRequest(pr, { branchPrompt: branchPrompt })]; case 1: _b.sent(); return [2 /*return*/]; @@ -4423,7 +4469,7 @@ var MergeCommandModule = { * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -/* GraphQL schema for the response body for each pending PR. */ +/* Graphql schema for the response body for each pending PR. */ const PR_SCHEMA$3 = { state: typedGraphqlify.types.string, maintainerCanModify: typedGraphqlify.types.boolean, @@ -4450,7 +4496,8 @@ const PR_SCHEMA$3 = { */ function rebasePr(prNumber, githubToken, config = getConfig()) { return tslib.__awaiter(this, void 0, void 0, function* () { - const git = new GitClient(githubToken); + /** The singleton instance of the GitClient. */ + const git = GitClient.getAuthenticatedInstance(); // TODO: Rely on a common assertNoLocalChanges function. if (git.hasLocalChanges()) { error('Cannot perform rebase of PR with local changes.'); @@ -5747,7 +5794,7 @@ class ReleaseAction { return this._cachedForkRepo; } const { owner, name } = this.git.remoteConfig; - const result = yield this.git.github.graphql.query(findOwnedForksOfRepoQuery, { owner, name }); + const result = yield this.git.github.graphql(findOwnedForksOfRepoQuery, { owner, name }); const forks = result.repository.forks.nodes; if (forks.length === 0) { error(red(' ✘ Unable to find fork for currently authenticated user.')); @@ -5800,7 +5847,7 @@ class ReleaseAction { pushHeadToRemoteBranch(branchName) { return tslib.__awaiter(this, void 0, void 0, function* () { // Push the local `HEAD` to the remote branch in the configured project. - this.git.run(['push', this.git.repoGitUrl, `HEAD:refs/heads/${branchName}`]); + this.git.run(['push', this.git.getRepoGitUrl(), `HEAD:refs/heads/${branchName}`]); }); } /** @@ -5915,7 +5962,7 @@ class ReleaseAction { /** Checks out an upstream branch with a detached head. */ checkoutUpstreamBranch(branchName) { return tslib.__awaiter(this, void 0, void 0, function* () { - this.git.run(['fetch', '-q', this.git.repoGitUrl, branchName]); + this.git.run(['fetch', '-q', this.git.getRepoGitUrl(), branchName]); this.git.run(['checkout', 'FETCH_HEAD', '--detach']); }); } @@ -6551,8 +6598,8 @@ class ReleaseTool { this._github = _github; this._githubToken = _githubToken; this._projectRoot = _projectRoot; - /** Client for interacting with the Github API and the local Git command. */ - this._git = new GitClient(this._githubToken, { github: this._github }, this._projectRoot); + /** The singleton instance of the GitClient. */ + this._git = GitClient.getAuthenticatedInstance(); /** The previous git commit to return back to after the release tool runs. */ this.previousGitBranchOrRevision = this._git.getCurrentBranchOrRevision(); } diff --git a/dev-infra/pr/check-target-branches/check-target-branches.ts b/dev-infra/pr/check-target-branches/check-target-branches.ts index 09b96f435c..8b2a15cf9b 100644 --- a/dev-infra/pr/check-target-branches/check-target-branches.ts +++ b/dev-infra/pr/check-target-branches/check-target-branches.ts @@ -17,8 +17,8 @@ export async function getTargetBranchesForPr(prNumber: number) { const config = getConfig(); /** Repo owner and name for the github repository. */ const {owner, name: repo} = config.github; - /** The git client to get a Github API service instance. */ - const git = new GitClient(undefined, config); + /** The singleton instance of the GitClient. */ + const git = GitClient.getInstance(); /** The validated merge config. */ const {config: mergeConfig, errors} = await loadAndValidateConfig(config, git.github); if (errors !== undefined) { diff --git a/dev-infra/pr/common/checkout-pr.ts b/dev-infra/pr/common/checkout-pr.ts index ccea54730b..c6464b7a9f 100644 --- a/dev-infra/pr/common/checkout-pr.ts +++ b/dev-infra/pr/common/checkout-pr.ts @@ -6,31 +6,31 @@ * found in the LICENSE file at https://angular.io/license */ -import {types as graphQLTypes} from 'typed-graphqlify'; +import {types as graphqlTypes} from 'typed-graphqlify'; import {info} from '../../utils/console'; import {addTokenToGitHttpsUrl} from '../../utils/git/github-urls'; import {GitClient} from '../../utils/git/index'; import {getPr} from '../../utils/github'; -/* GraphQL schema for the response body for a pending PR. */ +/* Graphql schema for the response body for a pending PR. */ const PR_SCHEMA = { - state: graphQLTypes.string, - maintainerCanModify: graphQLTypes.boolean, - viewerDidAuthor: graphQLTypes.boolean, - headRefOid: graphQLTypes.string, + state: graphqlTypes.string, + maintainerCanModify: graphqlTypes.boolean, + viewerDidAuthor: graphqlTypes.boolean, + headRefOid: graphqlTypes.string, headRef: { - name: graphQLTypes.string, + name: graphqlTypes.string, repository: { - url: graphQLTypes.string, - nameWithOwner: graphQLTypes.string, + url: graphqlTypes.string, + nameWithOwner: graphqlTypes.string, }, }, baseRef: { - name: graphQLTypes.string, + name: graphqlTypes.string, repository: { - url: graphQLTypes.string, - nameWithOwner: graphQLTypes.string, + url: graphqlTypes.string, + nameWithOwner: graphqlTypes.string, }, }, }; @@ -62,8 +62,8 @@ export interface PullRequestCheckoutOptions { */ export async function checkOutPullRequestLocally( prNumber: number, githubToken: string, opts: PullRequestCheckoutOptions = {}) { - /** Authenticated Git client for git and Github interactions. */ - const git = new GitClient(githubToken); + /** The singleton instance of the GitClient. */ + const git = GitClient.getAuthenticatedInstance(); // In order to preserve local changes, checkouts cannot occur if local changes are present in the // git environment. Checked before retrieving the PR to fail fast. diff --git a/dev-infra/pr/discover-new-conflicts/index.ts b/dev-infra/pr/discover-new-conflicts/index.ts index d73c329b2d..be2a2cc2fd 100644 --- a/dev-infra/pr/discover-new-conflicts/index.ts +++ b/dev-infra/pr/discover-new-conflicts/index.ts @@ -7,38 +7,37 @@ */ import {Bar} from 'cli-progress'; -import {types as graphQLTypes} from 'typed-graphqlify'; +import {types as graphqlTypes} from 'typed-graphqlify'; -import {getConfig, NgDevConfig} from '../../utils/config'; import {error, info} from '../../utils/console'; import {GitClient} from '../../utils/git/index'; import {getPendingPrs} from '../../utils/github'; import {exec} from '../../utils/shelljs'; -/* GraphQL schema for the response body for each pending PR. */ +/* Graphql schema for the response body for each pending PR. */ const PR_SCHEMA = { headRef: { - name: graphQLTypes.string, + name: graphqlTypes.string, repository: { - url: graphQLTypes.string, - nameWithOwner: graphQLTypes.string, + url: graphqlTypes.string, + nameWithOwner: graphqlTypes.string, }, }, baseRef: { - name: graphQLTypes.string, + name: graphqlTypes.string, repository: { - url: graphQLTypes.string, - nameWithOwner: graphQLTypes.string, + url: graphqlTypes.string, + nameWithOwner: graphqlTypes.string, }, }, - updatedAt: graphQLTypes.string, - number: graphQLTypes.number, - mergeable: graphQLTypes.string, - title: graphQLTypes.string, + updatedAt: graphqlTypes.string, + number: graphqlTypes.number, + mergeable: graphqlTypes.string, + title: graphqlTypes.string, }; -/* Pull Request response from Github GraphQL query */ +/* Pull Request response from Github Graphql query */ type RawPullRequest = typeof PR_SCHEMA; /** Convert raw Pull Request response from Github to usable Pull Request object. */ @@ -53,9 +52,9 @@ type PullRequest = ReturnType; const tempWorkingBranch = '__NgDevRepoBaseAfterChange__'; /** Checks if the provided PR will cause new conflicts in other pending PRs. */ -export async function discoverNewConflictsForPr( - newPrNumber: number, updatedAfter: number, config: Pick = getConfig()) { - const git = new GitClient(); +export async function discoverNewConflictsForPr(newPrNumber: number, updatedAfter: number) { + /** The singleton instance of the GitClient. */ + const git = GitClient.getAuthenticatedInstance(); // If there are any local changes in the current repository state, the // check cannot run as it needs to move between branches. if (git.hasLocalChanges()) { diff --git a/dev-infra/pr/merge/cli.ts b/dev-infra/pr/merge/cli.ts index 0f1a13c74f..e752422d74 100644 --- a/dev-infra/pr/merge/cli.ts +++ b/dev-infra/pr/merge/cli.ts @@ -37,8 +37,8 @@ function builder(yargs: Argv) { } /** Handles the command. */ -async function handler({pr, githubToken, branchPrompt}: Arguments) { - await mergePullRequest(pr, githubToken, {branchPrompt}); +async function handler({pr, branchPrompt}: Arguments) { + await mergePullRequest(pr, {branchPrompt}); } /** yargs command module describing the command. */ diff --git a/dev-infra/pr/merge/index.ts b/dev-infra/pr/merge/index.ts index c2f5e1a193..d67542205c 100644 --- a/dev-infra/pr/merge/index.ts +++ b/dev-infra/pr/merge/index.ts @@ -29,13 +29,12 @@ import {MergeResult, MergeStatus, PullRequestMergeTask, PullRequestMergeTaskFlag * @param projectRoot Path to the local Git project that is used for merging. * @param config Configuration for merging pull requests. */ -export async function mergePullRequest( - prNumber: number, githubToken: string, flags: PullRequestMergeTaskFlags) { +export async function mergePullRequest(prNumber: number, flags: PullRequestMergeTaskFlags) { // Set the environment variable to skip all git commit hooks triggered by husky. We are unable to // rely on `--no-verify` as some hooks still run, notably the `prepare-commit-msg` hook. process.env['HUSKY'] = '0'; - const api = await createPullRequestMergeTask(githubToken, flags); + const api = await createPullRequestMergeTask(flags); // Perform the merge. Force mode can be activated through a command line flag. // Alternatively, if the merge fails with non-fatal failures, the script @@ -127,10 +126,10 @@ export async function mergePullRequest( * and optional explicit configuration. An explicit configuration can be specified * when the merge script is used outside of a `ng-dev` configured repository. */ -async function createPullRequestMergeTask(githubToken: string, flags: PullRequestMergeTaskFlags) { - const projectRoot = getRepoBaseDir(); +async function createPullRequestMergeTask(flags: PullRequestMergeTaskFlags) { const devInfraConfig = getConfig(); - const git = new GitClient(githubToken, devInfraConfig, projectRoot); + /** The singleton instance of the GitClient. */ + const git = GitClient.getAuthenticatedInstance(); const {config, errors} = await loadAndValidateConfig(devInfraConfig, git.github); if (errors) { diff --git a/dev-infra/pr/merge/pull-request.ts b/dev-infra/pr/merge/pull-request.ts index 07988287b9..ea38f4c4d0 100644 --- a/dev-infra/pr/merge/pull-request.ts +++ b/dev-infra/pr/merge/pull-request.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {params, types as graphQLTypes} from 'typed-graphqlify'; +import {params, types as graphqlTypes} from 'typed-graphqlify'; import {Commit, parseCommitMessage} from '../../commit-message/parse'; import {red, warn} from '../../utils/console'; @@ -133,28 +133,28 @@ export async function loadAndValidatePullRequest( }; } -/* GraphQL schema for the response body the requested pull request. */ +/* Graphql schema for the response body the requested pull request. */ const PR_SCHEMA = { - url: graphQLTypes.string, - number: graphQLTypes.number, + url: graphqlTypes.string, + number: graphqlTypes.number, // Only the last 100 commits from a pull request are obtained as we likely will never see a pull // requests with more than 100 commits. commits: params({last: 100}, { - totalCount: graphQLTypes.number, + totalCount: graphqlTypes.number, nodes: [{ commit: { status: { - state: graphQLTypes.oneOf(['FAILURE', 'PENDING', 'SUCCESS'] as const), + state: graphqlTypes.oneOf(['FAILURE', 'PENDING', 'SUCCESS'] as const), }, - message: graphQLTypes.string, + message: graphqlTypes.string, }, }], }), - baseRefName: graphQLTypes.string, - title: graphQLTypes.string, + baseRefName: graphqlTypes.string, + title: graphqlTypes.string, labels: params({first: 100}, { nodes: [{ - name: graphQLTypes.string, + name: graphqlTypes.string, }] }), }; @@ -163,7 +163,7 @@ const PR_SCHEMA = { /** Fetches a pull request from Github. Returns null if an error occurred. */ async function fetchPullRequestFromGithub( - git: GitClient, prNumber: number): Promise { + git: GitClient, prNumber: number): Promise { try { const x = await getPr(PR_SCHEMA, prNumber, git); return x; diff --git a/dev-infra/pr/merge/strategies/api-merge.ts b/dev-infra/pr/merge/strategies/api-merge.ts index 4268df8580..76b9798731 100644 --- a/dev-infra/pr/merge/strategies/api-merge.ts +++ b/dev-infra/pr/merge/strategies/api-merge.ts @@ -37,7 +37,7 @@ const COMMIT_HEADER_SEPARATOR = '\n\n'; * is properly set, but a notable downside is that PRs cannot use fixup or squash commits. */ export class GithubApiMergeStrategy extends MergeStrategy { - constructor(git: GitClient, private _config: GithubApiMergeStrategyConfig) { + constructor(git: GitClient, private _config: GithubApiMergeStrategyConfig) { super(git); } diff --git a/dev-infra/pr/merge/strategies/strategy.ts b/dev-infra/pr/merge/strategies/strategy.ts index ecbf845579..c5cd98c523 100644 --- a/dev-infra/pr/merge/strategies/strategy.ts +++ b/dev-infra/pr/merge/strategies/strategy.ts @@ -22,7 +22,7 @@ export const TEMP_PR_HEAD_BRANCH = 'merge_pr_head'; * merges it into the determined target branches. */ export abstract class MergeStrategy { - constructor(protected git: GitClient) {} + constructor(protected git: GitClient) {} /** * Prepares a merge of the given pull request. The strategy by default will @@ -124,7 +124,8 @@ export abstract class MergeStrategy { }); // Fetch all target branches with a single command. We don't want to fetch them // individually as that could cause an unnecessary slow-down. - this.git.run(['fetch', '-q', '-f', this.git.repoGitUrl, ...fetchRefspecs, ...extraRefspecs]); + this.git.run( + ['fetch', '-q', '-f', this.git.getRepoGitUrl(), ...fetchRefspecs, ...extraRefspecs]); } /** Pushes the given target branches upstream. */ @@ -135,6 +136,6 @@ export abstract class MergeStrategy { }); // Push all target branches with a single command if we don't run in dry-run mode. // We don't want to push them individually as that could cause an unnecessary slow-down. - this.git.run(['push', this.git.repoGitUrl, ...pushRefspecs]); + this.git.run(['push', this.git.getRepoGitUrl(), ...pushRefspecs]); } } diff --git a/dev-infra/pr/merge/task.ts b/dev-infra/pr/merge/task.ts index 19e2fd7dd9..9e8724b57f 100644 --- a/dev-infra/pr/merge/task.ts +++ b/dev-infra/pr/merge/task.ts @@ -51,7 +51,7 @@ export class PullRequestMergeTask { private flags: PullRequestMergeTaskFlags; constructor( - public config: MergeConfigWithRemote, public git: GitClient, + public config: MergeConfigWithRemote, public git: GitClient, flags: Partial) { // Update flags property with the provided flags values as patches to the default flag values. this.flags = {...defaultPullRequestMergeTaskFlags, ...flags}; diff --git a/dev-infra/pr/rebase/index.ts b/dev-infra/pr/rebase/index.ts index 5dac748e06..a892a7e452 100644 --- a/dev-infra/pr/rebase/index.ts +++ b/dev-infra/pr/rebase/index.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {types as graphQLTypes} from 'typed-graphqlify'; +import {types as graphqlTypes} from 'typed-graphqlify'; import {Commit} from '../../commit-message/parse'; import {getCommitsInRange} from '../../commit-message/utils'; @@ -16,24 +16,24 @@ import {addTokenToGitHttpsUrl} from '../../utils/git/github-urls'; import {GitClient} from '../../utils/git/index'; import {getPr} from '../../utils/github'; -/* GraphQL schema for the response body for each pending PR. */ +/* Graphql schema for the response body for each pending PR. */ const PR_SCHEMA = { - state: graphQLTypes.string, - maintainerCanModify: graphQLTypes.boolean, - viewerDidAuthor: graphQLTypes.boolean, - headRefOid: graphQLTypes.string, + state: graphqlTypes.string, + maintainerCanModify: graphqlTypes.boolean, + viewerDidAuthor: graphqlTypes.boolean, + headRefOid: graphqlTypes.string, headRef: { - name: graphQLTypes.string, + name: graphqlTypes.string, repository: { - url: graphQLTypes.string, - nameWithOwner: graphQLTypes.string, + url: graphqlTypes.string, + nameWithOwner: graphqlTypes.string, }, }, baseRef: { - name: graphQLTypes.string, + name: graphqlTypes.string, repository: { - url: graphQLTypes.string, - nameWithOwner: graphQLTypes.string, + url: graphqlTypes.string, + nameWithOwner: graphqlTypes.string, }, }, }; @@ -44,7 +44,8 @@ const PR_SCHEMA = { */ export async function rebasePr( prNumber: number, githubToken: string, config: Pick = getConfig()) { - const git = new GitClient(githubToken); + /** The singleton instance of the GitClient. */ + const git = GitClient.getAuthenticatedInstance(); // TODO: Rely on a common assertNoLocalChanges function. if (git.hasLocalChanges()) { error('Cannot perform rebase of PR with local changes.'); diff --git a/dev-infra/release/publish/actions.ts b/dev-infra/release/publish/actions.ts index 40b5aa1d29..556c605fb5 100644 --- a/dev-infra/release/publish/actions.ts +++ b/dev-infra/release/publish/actions.ts @@ -49,7 +49,7 @@ export interface ReleaseActionConstructor; /** Constructs a release action. */ - new(...args: [ActiveReleaseTrains, GitClient, ReleaseConfig, string]): T; + new(...args: [ActiveReleaseTrains, GitClient, ReleaseConfig, string]): T; } /** @@ -76,7 +76,7 @@ export abstract class ReleaseAction { private _cachedForkRepo: GithubRepo|null = null; constructor( - protected active: ActiveReleaseTrains, protected git: GitClient, + protected active: ActiveReleaseTrains, protected git: GitClient, protected config: ReleaseConfig, protected projectDir: string) {} /** Updates the version in the project top-level `package.json` file. */ @@ -182,7 +182,7 @@ export abstract class ReleaseAction { } const {owner, name} = this.git.remoteConfig; - const result = await this.git.github.graphql.query(findOwnedForksOfRepoQuery, {owner, name}); + const result = await this.git.github.graphql(findOwnedForksOfRepoQuery, {owner, name}); const forks = result.repository.forks.nodes; if (forks.length === 0) { @@ -232,7 +232,7 @@ export abstract class ReleaseAction { /** Pushes the current Git `HEAD` to the given remote branch in the configured project. */ protected async pushHeadToRemoteBranch(branchName: string) { // Push the local `HEAD` to the remote branch in the configured project. - this.git.run(['push', this.git.repoGitUrl, `HEAD:refs/heads/${branchName}`]); + this.git.run(['push', this.git.getRepoGitUrl(), `HEAD:refs/heads/${branchName}`]); } /** @@ -360,7 +360,7 @@ export abstract class ReleaseAction { /** Checks out an upstream branch with a detached head. */ protected async checkoutUpstreamBranch(branchName: string) { - this.git.run(['fetch', '-q', this.git.repoGitUrl, branchName]); + this.git.run(['fetch', '-q', this.git.getRepoGitUrl(), branchName]); this.git.run(['checkout', 'FETCH_HEAD', '--detach']); } diff --git a/dev-infra/release/publish/index.ts b/dev-infra/release/publish/index.ts index 9ce84c3571..63b46b75ae 100644 --- a/dev-infra/release/publish/index.ts +++ b/dev-infra/release/publish/index.ts @@ -28,8 +28,8 @@ export enum CompletionState { } export class ReleaseTool { - /** Client for interacting with the Github API and the local Git command. */ - private _git = new GitClient(this._githubToken, {github: this._github}, this._projectRoot); + /** The singleton instance of the GitClient. */ + private _git = GitClient.getAuthenticatedInstance(); /** The previous git commit to return back to after the release tool runs. */ private previousGitBranchOrRevision = this._git.getCurrentBranchOrRevision(); diff --git a/dev-infra/release/publish/pull-request-state.ts b/dev-infra/release/publish/pull-request-state.ts index 129ef2dba5..8a01def367 100644 --- a/dev-infra/release/publish/pull-request-state.ts +++ b/dev-infra/release/publish/pull-request-state.ts @@ -16,7 +16,8 @@ const THIRTY_SECONDS_IN_MS = 30000; export type PullRequestState = 'merged'|'closed'|'open'; /** Gets whether a given pull request has been merged. */ -export async function getPullRequestState(api: GitClient, id: number): Promise { +export async function getPullRequestState( + api: GitClient, id: number): Promise { const {data} = await api.github.pulls.get({...api.remoteParams, pull_number: id}); if (data.merged) { return 'merged'; @@ -38,7 +39,7 @@ export async function getPullRequestState(api: GitClient, id: number): Promise

, id: number) { const request = api.github.issues.listEvents.endpoint.merge({...api.remoteParams, issue_number: id}); const events: Octokit.IssuesListEventsResponse = await api.github.paginate(request); @@ -72,7 +73,7 @@ async function isPullRequestClosedWithAssociatedCommit(api: GitClient, id: numbe } /** Checks whether the specified commit is closing the given pull request. */ -async function isCommitClosingPullRequest(api: GitClient, sha: string, id: number) { +async function isCommitClosingPullRequest(api: GitClient, sha: string, id: number) { const {data} = await api.github.repos.getCommit({...api.remoteParams, ref: sha}); // Matches the closing keyword supported in commit messages. See: // https://docs.github.com/en/enterprise/2.16/user/github/managing-your-work-on-github/closing-issues-using-keywords. diff --git a/dev-infra/release/publish/release-notes/release-notes.ts b/dev-infra/release/publish/release-notes/release-notes.ts index 4861dcb8c7..15f6eb03df 100644 --- a/dev-infra/release/publish/release-notes/release-notes.ts +++ b/dev-infra/release/publish/release-notes/release-notes.ts @@ -38,7 +38,7 @@ export function getLocalChangelogFilePath(projectDir: string): string { /** Release note generation. */ export class ReleaseNotes { /** An instance of GitClient. */ - private git = new GitClient(); + private git = GitClient.getInstance(); /** The github configuration. */ private readonly github = getConfig().github; /** The configuration for the release notes generation. */ diff --git a/dev-infra/release/publish/test/test-utils.ts b/dev-infra/release/publish/test/test-utils.ts index 3b36e0f6de..76b254dd57 100644 --- a/dev-infra/release/publish/test/test-utils.ts +++ b/dev-infra/release/publish/test/test-utils.ts @@ -33,7 +33,7 @@ export const testTmpDir: string = process.env['TEST_TMPDIR']!; /** Interface describing a test release action. */ export interface TestReleaseAction { instance: T; - gitClient: VirtualGitClient; + gitClient: VirtualGitClient; repo: GithubTestingRepo; fork: GithubTestingRepo; testTmpDir: string; @@ -44,7 +44,7 @@ export interface TestReleaseAction { /** Gets necessary test mocks for running a release action. */ export function getTestingMocksForReleaseAction() { const githubConfig = {owner: 'angular', name: 'dev-infra-test'}; - const gitClient = new VirtualGitClient(undefined, {github: githubConfig}, testTmpDir); + const gitClient = VirtualGitClient.getAuthenticatedInstance({github: githubConfig}); const releaseConfig: ReleaseConfig = { npmPackages: [ '@angular/pkg1', diff --git a/dev-infra/utils/git/github-urls.ts b/dev-infra/utils/git/github-urls.ts index ff4e04810f..d6fc7bcd4f 100644 --- a/dev-infra/utils/git/github-urls.ts +++ b/dev-infra/utils/git/github-urls.ts @@ -37,6 +37,6 @@ export function getRepositoryGitUrl(config: GithubConfig, githubToken?: string): } /** Gets a Github URL that refers to a list of recent commits within a specified branch. */ -export function getListCommitsInBranchUrl({remoteParams}: GitClient, branchName: string) { +export function getListCommitsInBranchUrl({remoteParams}: GitClient, branchName: string) { return `https://github.com/${remoteParams.owner}/${remoteParams.repo}/commits/${branchName}`; } diff --git a/dev-infra/utils/git/github-yargs.ts b/dev-infra/utils/git/github-yargs.ts index ecd615d57e..e695bafd0a 100644 --- a/dev-infra/utils/git/github-yargs.ts +++ b/dev-infra/utils/git/github-yargs.ts @@ -7,8 +7,11 @@ */ import {Argv} from 'yargs'; + import {error, red, yellow} from '../console'; + import {GITHUB_TOKEN_GENERATE_URL} from './github-urls'; +import {GitClient} from './index'; export type ArgvWithGithubToken = Argv<{githubToken: string}>; @@ -29,6 +32,7 @@ export function addGithubTokenOption(yargs: Argv): ArgvWithGithubToken { error(yellow(`You can generate a token here: ${GITHUB_TOKEN_GENERATE_URL}`)); process.exit(1); } + GitClient.authenticateWithToken(githubToken); return githubToken; }, }) diff --git a/dev-infra/utils/git/github.ts b/dev-infra/utils/git/github.ts index 1d368bb9f0..b6326d9245 100644 --- a/dev-infra/utils/git/github.ts +++ b/dev-infra/utils/git/github.ts @@ -11,6 +11,12 @@ import * as Octokit from '@octokit/rest'; import {RequestParameters} from '@octokit/types'; import {query, types} from 'typed-graphqlify'; +/** + * An object representation of a Graphql Query to be used as a response type and + * to generate a Graphql query string. + */ +export type GraphqlQueryObject = Parameters[1]; + /** Interface describing a Github repository. */ export interface GithubRepo { /** Owner login of the repository. */ @@ -26,6 +32,9 @@ export class GithubApiRequestError extends Error { } } +/** Error for failed Github API requests. */ +export class GithubGraphqlClientError extends Error {} + /** * A Github client for interacting with the Github APIs. * @@ -33,13 +42,15 @@ export class GithubApiRequestError extends Error { * would provide value from memoized style responses. **/ export class GithubClient extends Octokit { - /** The Github GraphQL (v4) API. */ - graphql: GithubGraphqlClient; - /** The current user based on checking against the Github API. */ private _currentUser: string|null = null; + /** The graphql instance with authentication set during construction. */ + private _graphql = graphql.defaults({headers: {authorization: `token ${this.token}`}}); - constructor(token?: string) { + /** + * @param token The github authentication token for Github Rest and Graphql API requests. + */ + constructor(private token?: string) { // Pass in authentication token to base Octokit class. super({auth: token}); @@ -49,8 +60,22 @@ export class GithubClient extends Octokit { throw new GithubApiRequestError(error.status, error.message); }); - // Create authenticated graphql client. - this.graphql = new GithubGraphqlClient(token); + // Note: The prototype must be set explictly as Github's Octokit class is a non-standard class + // definition which adjusts the prototype chain. + // See: + // https://github.com/Microsoft/TypeScript/wiki/FAQ#why-doesnt-extending-built-ins-like-error-array-and-map-work + // https://github.com/octokit/rest.js/blob/7b51cee4a22b6e52adcdca011f93efdffa5df998/lib/constructor.js + Object.setPrototypeOf(this, GithubClient.prototype); + } + + /** Perform a query using Github's Graphql API. */ + async graphql(queryObject: T, params: RequestParameters = {}) { + if (this.token === undefined) { + throw new GithubGraphqlClientError( + 'Cannot query via graphql without an authentication token set, use the authenticated ' + + '`GitClient` by calling `GitClient.getAuthenticatedInstance()`.'); + } + return (await this._graphql(query(queryObject), params)) as T; } /** Retrieve the login of the current user from Github. */ @@ -59,7 +84,7 @@ export class GithubClient extends Octokit { if (this._currentUser !== null) { return this._currentUser; } - const result = await this.graphql.query({ + const result = await this.graphql({ viewer: { login: types.string, } @@ -67,29 +92,3 @@ export class GithubClient extends Octokit { return this._currentUser = result.viewer.login; } } - -/** - * An object representation of a GraphQL Query to be used as a response type and - * to generate a GraphQL query string. - */ -export type GraphQLQueryObject = Parameters[1]; - -/** A client for interacting with Github's GraphQL API. */ -export class GithubGraphqlClient { - /** The Github GraphQL (v4) API. */ - private graqhql = graphql; - - constructor(token?: string) { - // Set the default headers to include authorization with the provided token for all - // graphQL calls. - if (token) { - this.graqhql = this.graqhql.defaults({headers: {authorization: `token ${token}`}}); - } - } - - /** Perform a query using Github's GraphQL API. */ - async query(queryObject: T, params: RequestParameters = {}) { - const queryString = query(queryObject); - return (await this.graqhql(queryString, params)) as T; - } -} diff --git a/dev-infra/utils/git/index.ts b/dev-infra/utils/git/index.ts index e47b39aaa9..b1a6e5bbdb 100644 --- a/dev-infra/utils/git/index.ts +++ b/dev-infra/utils/git/index.ts @@ -10,7 +10,7 @@ import * as Octokit from '@octokit/rest'; import {spawnSync, SpawnSyncOptions, SpawnSyncReturns} from 'child_process'; import {Options as SemVerOptions, parse, SemVer} from 'semver'; -import {getConfig, getRepoBaseDir, NgDevConfig} from '../config'; +import {getConfig, getRepoBaseDir} from '../config'; import {debug, info, yellow} from '../console'; import {DryRunError, isDryRun} from '../dry-run'; import {GithubClient} from './github'; @@ -26,7 +26,7 @@ export type OAuthScopeTestFunction = (scopes: string[], missing: string[]) => vo /** Error for failed Git commands. */ export class GitCommandError extends Error { - constructor(client: GitClient, public args: string[]) { + constructor(client: GitClient, public args: string[]) { // Errors are not guaranteed to be caught. To ensure that we don't // accidentally leak the Github token that might be used in a command, // we sanitize the command that will be part of the error message. @@ -43,18 +43,49 @@ export class GitCommandError extends Error { * `config`: The dev-infra configuration containing information about the remote. By default * the dev-infra configuration is loaded with its Github configuration. **/ -export class GitClient { - /** Whether verbose logging of Git actions should be used. */ - static LOG_COMMANDS = true; - /** Short-hand for accessing the default remote configuration. */ - remoteConfig = this._config.github; - /** Octokit request parameters object for targeting the configured remote. */ - remoteParams = {owner: this.remoteConfig.owner, repo: this.remoteConfig.name}; - /** Git URL that resolves to the configured repository. */ - repoGitUrl = getRepositoryGitUrl(this.remoteConfig, this.githubToken); - /** Instance of the authenticated Github octokit API. */ - github = new GithubClient(this.githubToken); +export class GitClient { + /************************************************* + * Singleton definition and configuration. * + *************************************************/ + /** The singleton instance of the authenticated GitClient. */ + private static authenticated: GitClient; + /** The singleton instance of the unauthenticated GitClient. */ + private static unauthenticated: GitClient; + /** + * Static method to get the singleton instance of the unauthorized GitClient, creating it if it + * has not yet been created. + */ + static getInstance() { + if (!GitClient.unauthenticated) { + GitClient.unauthenticated = new GitClient(undefined); + } + return GitClient.unauthenticated; + } + + /** + * Static method to get the singleton instance of the authenticated GitClient if it has been + * generated. + */ + static getAuthenticatedInstance() { + if (!GitClient.authenticated) { + throw Error('The authenticated GitClient has not yet been generated.'); + } + return GitClient.authenticated; + } + + /** Build the authenticated GitClient instance. */ + static authenticateWithToken(token: string) { + if (GitClient.authenticated) { + throw Error( + 'Cannot generate new authenticated GitClient after one has already been generated.'); + } + GitClient.authenticated = new GitClient(token); + } + + + /** Whether verbose logging of Git actions should be used. */ + private verboseLogging = true; /** The OAuth scopes available for the provided Github token. */ private _cachedOauthScopes: Promise|null = null; /** @@ -62,18 +93,36 @@ export class GitClient { * sanitizing the token from Git child process output. */ private _githubTokenRegex: RegExp|null = null; + /** Short-hand for accessing the default remote configuration. */ + remoteConfig = this._config.github; + /** Octokit request parameters object for targeting the configured remote. */ + remoteParams = {owner: this.remoteConfig.owner, repo: this.remoteConfig.name}; + /** Instance of the authenticated Github octokit API. */ + github = new GithubClient(this.githubToken); - constructor( - public githubToken?: string, private _config: Pick = getConfig(), - private _projectRoot = getRepoBaseDir()) { + /** + * @param githubToken The github token used for authentication, if provided. + * @param _config The configuration, containing the github specific configuration. + * @param _projectRoot The full path to the root of the repository base. + */ + protected constructor(public githubToken: + Authenticated extends true? string: undefined, + private _config = getConfig(), + private _projectRoot = getRepoBaseDir()) { // If a token has been specified (and is not empty), pass it to the Octokit API and // also create a regular expression that can be used for sanitizing Git command output // so that it does not print the token accidentally. - if (githubToken != null) { + if (typeof githubToken === 'string') { this._githubTokenRegex = new RegExp(githubToken, 'g'); } } + /** Set the verbose logging state of the GitClient instance. */ + setVerboseLoggingState(verbose: boolean): this { + this.verboseLogging = verbose; + return this; + } + /** Executes the given git command. Throws if the command fails. */ run(args: string[], options?: SpawnSyncOptions): Omit, 'status'> { const result = this.runGraceful(args, options); @@ -102,7 +151,7 @@ export class GitClient { // To improve the debugging experience in case something fails, we print all executed Git // commands to better understand the git actions occuring. Depending on the command being // executed, this debugging information should be logged at different logging levels. - const printFn = (!GitClient.LOG_COMMANDS || options.stdio === 'ignore') ? debug : info; + const printFn = (!this.verboseLogging || options.stdio === 'ignore') ? debug : info; // Note that we do not want to print the token if it is contained in the command. It's common // to share errors with others if the tool failed, and we do not want to leak tokens. printFn('Executing: git', this.omitGithubTokenFromMessage(args.join(' '))); @@ -126,6 +175,11 @@ export class GitClient { return result; } + /** Git URL that resolves to the configured repository. */ + getRepoGitUrl() { + return getRepositoryGitUrl(this.remoteConfig, this.githubToken); + } + /** Whether the given branch contains the specified SHA. */ hasCommit(branchName: string, sha: string): boolean { return this.run(['branch', branchName, '--contains', sha]).stdout !== ''; diff --git a/dev-infra/utils/github.ts b/dev-infra/utils/github.ts index 6b79440124..62e3580f46 100644 --- a/dev-infra/utils/github.ts +++ b/dev-infra/utils/github.ts @@ -11,10 +11,10 @@ import {params, types} from 'typed-graphqlify'; import {GitClient} from './git/index'; /** Get a PR from github */ -export async function getPr(prSchema: PrSchema, prNumber: number, git: GitClient) { +export async function getPr(prSchema: PrSchema, prNumber: number, git: GitClient) { /** The owner and name of the repository */ const {owner, name} = git.remoteConfig; - /** The GraphQL query object to get a the PR */ + /** The Graphql query object to get a the PR */ const PR_QUERY = params( { $number: 'Int!', // The PR number @@ -27,15 +27,15 @@ export async function getPr(prSchema: PrSchema, prNumber: number, git: }) }); - const result = (await git.github.graphql.query(PR_QUERY, {number: prNumber, owner, name})); + const result = (await git.github.graphql(PR_QUERY, {number: prNumber, owner, name})); return result.repository.pullRequest; } /** Get all pending PRs from github */ -export async function getPendingPrs(prSchema: PrSchema, git: GitClient) { +export async function getPendingPrs(prSchema: PrSchema, git: GitClient) { /** The owner and name of the repository */ const {owner, name} = git.remoteConfig; - /** The GraphQL query object to get a page of pending PRs */ + /** The Graphql query object to get a page of pending PRs */ const PRS_QUERY = params( { $first: 'Int', // How many entries to get with each request @@ -75,7 +75,7 @@ export async function getPendingPrs(prSchema: PrSchema, git: GitClient owner, name, }; - const results = await git.github.graphql.query(PRS_QUERY, params) as typeof PRS_QUERY; + const results = await git.github.graphql(PRS_QUERY, params) as typeof PRS_QUERY; prs.push(...results.repository.pullRequests.nodes); hasNextPage = results.repository.pullRequests.pageInfo.hasNextPage; cursor = results.repository.pullRequests.pageInfo.endCursor; diff --git a/dev-infra/utils/testing/virtual-git-client.ts b/dev-infra/utils/testing/virtual-git-client.ts index e1fefe8bda..a1cbf1e2b5 100644 --- a/dev-infra/utils/testing/virtual-git-client.ts +++ b/dev-infra/utils/testing/virtual-git-client.ts @@ -58,7 +58,21 @@ export interface Commit { * Virtual git client that mocks Git commands and keeps track of the repository state * in memory. This allows for convenient test assertions with Git interactions. */ -export class VirtualGitClient extends GitClient { +export class VirtualGitClient extends GitClient { + static getInstance(config = mockNgDevConfig, tmpDir = testTmpDir): VirtualGitClient { + return new VirtualGitClient(undefined, config, tmpDir); + } + + static getAuthenticatedInstance(config = mockNgDevConfig, tmpDir = testTmpDir): + VirtualGitClient { + return new VirtualGitClient('abc123', config, tmpDir); + } + + private constructor(token: Authenticated extends true? string: undefined, config: NgDevConfig, + tmpDir: string) { + super(token, config, tmpDir); + } + /** Current Git HEAD that has been previously fetched. */ fetchHeadRef: RemoteRef|null = null; /** List of known branches in the repository. */ @@ -190,10 +204,10 @@ export class VirtualGitClient extends GitClient { } -/** - * Builds a Virtual Git Client instance with the provided config and set the temporary test - * directory. - */ -export function buildVirtualGitClient(config = mockNgDevConfig, tmpDir = testTmpDir) { - return (new VirtualGitClient(undefined, config, tmpDir)); +export function installVirtualGitClientSpies() { + const authenticatedVirtualGitClient = VirtualGitClient.getAuthenticatedInstance(); + spyOn(GitClient, 'getAuthenticatedInstance').and.returnValue(authenticatedVirtualGitClient); + + const unauthenticatedVirtualGitClient = VirtualGitClient.getInstance(); + spyOn(GitClient, 'getInstance').and.returnValue(unauthenticatedVirtualGitClient); } diff --git a/dev-infra/utils/testing/virtual-git-matchers.ts b/dev-infra/utils/testing/virtual-git-matchers.ts index 41c021aa6f..6e09d76fac 100644 --- a/dev-infra/utils/testing/virtual-git-matchers.ts +++ b/dev-infra/utils/testing/virtual-git-matchers.ts @@ -27,13 +27,13 @@ export function getBranchPushMatcher(options: BranchPushMatchParameters) { const {targetRepo, targetBranch, baseBranch, baseRepo, expectedCommits} = options; return jasmine.objectContaining({ remote: { - repoUrl: `https://github.com/${targetRepo.owner}/${targetRepo.name}.git`, + repoUrl: `https://abc123@github.com/${targetRepo.owner}/${targetRepo.name}.git`, name: `refs/heads/${targetBranch}` }, head: jasmine.objectContaining({ newCommits: expectedCommits, ref: { - repoUrl: `https://github.com/${baseRepo.owner}/${baseRepo.name}.git`, + repoUrl: `https://abc123@github.com/${baseRepo.owner}/${baseRepo.name}.git`, name: baseBranch, }, })