refactor(dev-infra): migrate github api in GitClient to rely on GithubClient (#37593)

GitClient now uses GithubClient for github API interactions.  GithubClient is
a class which extends Octokit and provides a member which allows for GraphQL
requests against the Github GraphQL api, as well as providing convenience methods
for common/repeated Github API requests.

PR Close #37593
This commit is contained in:
Joey Perrott 2020-06-15 09:20:05 -07:00 committed by Misko Hevery
parent 7e0eccc4ef
commit a8af8551ec
8 changed files with 127 additions and 27 deletions

View File

@ -67,7 +67,7 @@ export async function loadAndValidatePullRequest(
} }
const {data: {state}} = const {data: {state}} =
await git.api.repos.getCombinedStatusForRef({...git.remoteParams, ref: prData.head.sha}); await git.github.repos.getCombinedStatusForRef({...git.remoteParams, ref: prData.head.sha});
if (state === 'failure' && !ignoreNonFatalFailures) { if (state === 'failure' && !ignoreNonFatalFailures) {
return PullRequestFailure.failingCiJobs(); return PullRequestFailure.failingCiJobs();
@ -102,7 +102,7 @@ export async function loadAndValidatePullRequest(
async function fetchPullRequestFromGithub( async function fetchPullRequestFromGithub(
git: GitClient, prNumber: number): Promise<Octokit.PullsGetResponse|null> { git: GitClient, prNumber: number): Promise<Octokit.PullsGetResponse|null> {
try { try {
const result = await git.api.pulls.get({...git.remoteParams, pull_number: prNumber}); const result = await git.github.pulls.get({...git.remoteParams, pull_number: prNumber});
return result.data; return result.data;
} catch (e) { } catch (e) {
// If the pull request could not be found, we want to return `null` so // If the pull request could not be found, we want to return `null` so

View File

@ -94,7 +94,7 @@ export class GithubApiMergeStrategy extends MergeStrategy {
try { try {
// Merge the pull request using the Github API into the selected base branch. // Merge the pull request using the Github API into the selected base branch.
const result = await this.git.api.pulls.merge(mergeOptions); const result = await this.git.github.pulls.merge(mergeOptions);
mergeStatusCode = result.status; mergeStatusCode = result.status;
targetSha = result.data.sha; targetSha = result.data.sha;
@ -189,9 +189,9 @@ export class GithubApiMergeStrategy extends MergeStrategy {
/** Gets all commit messages of commits in the pull request. */ /** Gets all commit messages of commits in the pull request. */
private async _getPullRequestCommitMessages({prNumber}: PullRequest) { private async _getPullRequestCommitMessages({prNumber}: PullRequest) {
const request = this.git.api.pulls.listCommits.endpoint.merge( const request = this.git.github.pulls.listCommits.endpoint.merge(
{...this.git.remoteParams, pull_number: prNumber}); {...this.git.remoteParams, pull_number: prNumber});
const allCommits: PullsListCommitsResponse = await this.git.api.paginate(request); const allCommits: PullsListCommitsResponse = await this.git.github.paginate(request);
return allCommits.map(({commit}) => commit.message); return allCommits.map(({commit}) => commit.message);
} }

View File

@ -11,6 +11,7 @@
"dependencies": { "dependencies": {
"@angular/benchpress": "0.2.0", "@angular/benchpress": "0.2.0",
"@octokit/graphql": "<from-root>", "@octokit/graphql": "<from-root>",
"@octokit/types": "<from-root>",
"brotli": "<from-root>", "brotli": "<from-root>",
"chalk": "<from-root>", "chalk": "<from-root>",
"cli-progress": "<from-root>", "cli-progress": "<from-root>",

View File

@ -2,12 +2,16 @@ load("@npm_bazel_typescript//:index.bzl", "ts_library")
ts_library( ts_library(
name = "utils", name = "utils",
srcs = glob(["*.ts"]), srcs = glob([
"*.ts",
"git/*.ts",
]),
module_name = "@angular/dev-infra-private/utils", module_name = "@angular/dev-infra-private/utils",
visibility = ["//dev-infra:__subpackages__"], visibility = ["//dev-infra:__subpackages__"],
deps = [ deps = [
"@npm//@octokit/graphql", "@npm//@octokit/graphql",
"@npm//@octokit/rest", "@npm//@octokit/rest",
"@npm//@octokit/types",
"@npm//@types/inquirer", "@npm//@types/inquirer",
"@npm//@types/node", "@npm//@types/node",
"@npm//@types/shelljs", "@npm//@types/shelljs",

View File

@ -0,0 +1,100 @@
/**
* @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
*/
/****************************************************************************
****************************************************************************
** DO NOT IMPORT THE GithubClient DIRECTLY, INSTEAD IMPORT GitClient from **
** ./index.ts and access the GithubClient via the `.github` member. **
****************************************************************************
****************************************************************************/
import {graphql} from '@octokit/graphql';
import * as Octokit from '@octokit/rest';
import {RequestParameters} from '@octokit/types';
import {query, types} from 'typed-graphqlify';
/** Error for failed Github API requests. */
export class GithubApiRequestError extends Error {
constructor(public status: number, message: string) {
super(message);
}
}
/**
* A Github client for interacting with the Github APIs.
*
* Additionally, provides convienience methods for actions which require multiple requests, or
* would provide value from memoized style responses.
**/
export class _GithubClient extends Octokit {
/** The Github GraphQL (v4) API. */
graqhql: GithubGraphqlClient;
/** The current user based on checking against the Github API. */
private _currentUser: string|null = null;
constructor(token?: string) {
// Pass in authentication token to base Octokit class.
super({auth: token});
this.hook.error('request', 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.graqhql = new GithubGraphqlClient(token);
}
/** Retrieve the login of the current user from Github. */
async getCurrentUser() {
// If the current user has already been retrieved return the current user value again.
if (this._currentUser !== null) {
return this._currentUser;
}
const result = await this.graqhql.query({
viewer: {
login: types.string,
}
});
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.
*/
type GraphQLQueryObject = Parameters<typeof query>[1];
/**
* A client for interacting with Github's GraphQL API.
*
* This class is intentionally not exported as it should always be access/used via a
* _GithubClient instance.
*/
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.defaults({headers: {authorization: `token ${token}`}});
}
}
/** Perform a query using Github's GraphQL API. */
async query<T extends GraphQLQueryObject>(queryObject: T, params: RequestParameters = {}) {
const queryString = query(queryObject);
return (await this.graqhql(queryString, params)) as T;
}
}

View File

@ -9,22 +9,18 @@
import * as Octokit from '@octokit/rest'; import * as Octokit from '@octokit/rest';
import {spawnSync, SpawnSyncOptions, SpawnSyncReturns} from 'child_process'; import {spawnSync, SpawnSyncOptions, SpawnSyncReturns} from 'child_process';
import {getConfig, getRepoBaseDir, NgDevConfig} from './config'; import {getConfig, getRepoBaseDir, NgDevConfig} from '../config';
import {info, yellow} from './console'; import {info, yellow} from '../console';
import {_GithubClient} from './_github';
// Re-export GithubApiRequestError
export {GithubApiRequestError} from './_github';
/** Github response type extended to include the `x-oauth-scopes` headers presence. */ /** Github response type extended to include the `x-oauth-scopes` headers presence. */
type RateLimitResponseWithOAuthScopeHeader = Octokit.Response<Octokit.RateLimitGetResponse>&{ type RateLimitResponseWithOAuthScopeHeader = Octokit.Response<Octokit.RateLimitGetResponse>&{
headers: {'x-oauth-scopes': string}; headers: {'x-oauth-scopes': string};
}; };
/** Error for failed Github API requests. */
export class GithubApiRequestError extends Error {
constructor(public status: number, message: string) {
super(message);
}
}
/** Error for failed Git commands. */ /** Error for failed Git commands. */
export class GitCommandError extends Error { export class GitCommandError extends Error {
constructor(client: GitClient, public args: string[]) { constructor(client: GitClient, public args: string[]) {
@ -55,7 +51,7 @@ export class GitClient {
`https://${this._githubToken}@github.com/${this.remoteConfig.owner}/${ `https://${this._githubToken}@github.com/${this.remoteConfig.owner}/${
this.remoteConfig.name}.git`; this.remoteConfig.name}.git`;
/** Instance of the authenticated Github octokit API. */ /** Instance of the authenticated Github octokit API. */
api: Octokit; github = new _GithubClient(this._githubToken);
/** The file path of project's root directory. */ /** The file path of project's root directory. */
private _projectRoot = getRepoBaseDir(); private _projectRoot = getRepoBaseDir();
@ -75,13 +71,6 @@ export class GitClient {
if (_githubToken != null) { if (_githubToken != null) {
this._githubTokenRegex = new RegExp(_githubToken, 'g'); this._githubTokenRegex = new RegExp(_githubToken, 'g');
} }
this.api = new Octokit({auth: _githubToken});
this.api.hook.error('request', 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);
});
} }
/** Executes the given git command. Throws if the command fails. */ /** Executes the given git command. Throws if the command fails. */
@ -186,10 +175,8 @@ export class GitClient {
return {error}; return {error};
} }
/** /**
* Retrieves the OAuth scopes for the loaded Github token, returning the already * Retrieve the OAuth scopes for the loaded Github token.
* retrieved list of OAuth scopes if available.
**/ **/
private async getAuthScopesForToken() { private async getAuthScopesForToken() {
// If the OAuth scopes have already been loaded, return the Promise containing them. // If the OAuth scopes have already been loaded, return the Promise containing them.
@ -198,7 +185,7 @@ export class GitClient {
} }
// OAuth scopes are loaded via the /rate_limit endpoint to prevent // OAuth scopes are loaded via the /rate_limit endpoint to prevent
// usage of a request against that rate_limit for this lookup. // usage of a request against that rate_limit for this lookup.
return this._oauthScopes = this.api.rateLimit.get().then(_response => { return this._oauthScopes = this.github.rateLimit.get().then(_response => {
const response = _response as RateLimitResponseWithOAuthScopeHeader; const response = _response as RateLimitResponseWithOAuthScopeHeader;
const scopes: string = response.headers['x-oauth-scopes'] || ''; const scopes: string = response.headers['x-oauth-scopes'] || '';
return scopes.split(',').map(scope => scope.trim()); return scopes.split(',').map(scope => scope.trim());

View File

@ -62,6 +62,7 @@
"@bazel/typescript": "1.7.0", "@bazel/typescript": "1.7.0",
"@microsoft/api-extractor": "7.7.11", "@microsoft/api-extractor": "7.7.11",
"@octokit/rest": "16.28.7", "@octokit/rest": "16.28.7",
"@octokit/types": "^5.0.1",
"@schematics/angular": "10.0.0-rc.2", "@schematics/angular": "10.0.0-rc.2",
"@types/angular": "^1.6.47", "@types/angular": "^1.6.47",
"@types/babel__core": "^7.1.6", "@types/babel__core": "^7.1.6",

View File

@ -1955,6 +1955,13 @@
dependencies: dependencies:
"@types/node" ">= 8" "@types/node" ">= 8"
"@octokit/types@^5.0.1":
version "5.0.1"
resolved "https://registry.yarnpkg.com/@octokit/types/-/types-5.0.1.tgz#5459e9a5e9df8565dcc62c17a34491904d71971e"
integrity sha512-GorvORVwp244fGKEt3cgt/P+M0MGy4xEDbckw+K5ojEezxyMDgCaYPKVct+/eWQfZXOT7uq0xRpmrl/+hliabA==
dependencies:
"@types/node" ">= 8"
"@protobufjs/aspromise@^1.1.1", "@protobufjs/aspromise@^1.1.2": "@protobufjs/aspromise@^1.1.1", "@protobufjs/aspromise@^1.1.2":
version "1.1.2" version "1.1.2"
resolved "https://registry.yarnpkg.com/@protobufjs/aspromise/-/aspromise-1.1.2.tgz#9b8b0cc663d669a7d8f6f5d0893a14d348f30fbf" resolved "https://registry.yarnpkg.com/@protobufjs/aspromise/-/aspromise-1.1.2.tgz#9b8b0cc663d669a7d8f6f5d0893a14d348f30fbf"