Skip to content

Commit f463deb

Browse files
joyeecheungjoyeecheung
committed
refactor the request class
1 parent ebb6a60 commit f463deb

File tree

6 files changed

+705
-101
lines changed

6 files changed

+705
-101
lines changed

‎bin/metadata.js

+6-8
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
'use strict';
33

44
const { EOL } = require('os');
5-
const { request, requestAll } = require('../lib/request');
6-
const requestPromise = require('request-promise-native');
5+
const Request = require('../lib/request');
6+
const auth = require('../lib/auth');
77

88
const loggerFactory = require('../lib/logger');
99
const PRData = require('../lib/pr_data');
@@ -17,10 +17,11 @@ const OWNER = process.argv[3] || 'nodejs';
1717
const REPO = process.argv[4] || 'node';
1818

1919
async function main(prid, owner, repo, logger) {
20-
const req = { request, requestAll, requestPromise };
21-
const data = new PRData(prid, owner, repo, logger, req);
20+
const credentials = await auth();
21+
const request = new Request(credentials);
22+
23+
const data = new PRData(prid, owner, repo, logger, request);
2224
await data.getAll();
23-
data.analyzeReviewers();
2425

2526
const metadata = new MetadataGenerator(data).getMetadata();
2627
const [SCISSOR_LEFT, SCISSOR_RIGHT] = MetadataGenerator.SCISSORS;
@@ -31,9 +32,6 @@ async function main(prid, owner, repo, logger) {
3132
if (!process.stdout.isTTY) {
3233
process.stdout.write(`${metadata}${EOL}`);
3334
}
34-
/**
35-
* TODO: put all these data into one object with a class
36-
*/
3735
const checker = new PRChecker(logger, data);
3836
checker.checkAll();
3937
}

‎lib/collaborators.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,9 @@ Collaborator.TYPES = {
4141
TSC, COLLABORATOR
4242
};
4343

44-
async function getCollaborators(requestPromise, logger, owner, repo) {
44+
async function getCollaborators(readme, logger, owner, repo) {
4545
// This is more or less taken from
4646
// https://github.com/rvagg/iojs-tools/blob/master/pr-metadata/pr-metadata.js
47-
const url = `https://raw.githubusercontent.com/${owner}/${repo}/master/README.md`;
48-
49-
const readme = await requestPromise({ url });
50-
5147
const members = new Map();
5248
let m;
5349

‎lib/pr_data.js

+23-29
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,30 @@
11
'use strict';
22

3-
const fs = require('fs');
4-
const path = require('path');
5-
6-
function loadQuery(file) {
7-
const filePath = path.resolve(__dirname, '..', 'queries', `${file}.gql`);
8-
return fs.readFileSync(filePath, 'utf8');
9-
}
10-
11-
const PR_QUERY = loadQuery('PR');
12-
const REVIEWS_QUERY = loadQuery('Reviews');
13-
const COMMENTS_QUERY = loadQuery('PRComments');
14-
const COMMITS_QUERY = loadQuery('PRCommits');
15-
const USER_QUERY = loadQuery('User');
16-
173
// TODO(joyeecheung): make it mockable with req.rp ?
184
const { getCollaborators } = require('../lib/collaborators');
195
const { ReviewAnalyzer } = require('../lib/reviews');
206

7+
// queries/*.gql file names
8+
const PR_QUERY = 'PR';
9+
const REVIEWS_QUERY = 'Reviews';
10+
const COMMENTS_QUERY = 'PRComments';
11+
const COMMITS_QUERY = 'PRCommits';
12+
const USER_QUERY = 'User';
13+
2114
class PRData {
2215
/**
2316
* @param {number} prid
2417
* @param {string} owner
2518
* @param {string} repo
2619
* @param {Object} logger
27-
* @param {Object} req
20+
* @param {Object} request
2821
*/
29-
constructor(prid, owner, repo, logger, req) {
22+
constructor(prid, owner, repo, logger, request) {
3023
this.prid = prid;
3124
this.owner = owner;
3225
this.repo = repo;
3326
this.logger = logger;
34-
this.request = req.request;
35-
this.requestAll = req.requestAll;
36-
this.requestPromise = req.requestPromise;
27+
this.request = request;
3728

3829
// Data
3930
this.collaborators = new Map();
@@ -52,53 +43,56 @@ class PRData {
5243
this.getComments(),
5344
this.getCommits()
5445
]);
46+
this.analyzeReviewers();
5547
}
5648

5749
analyzeReviewers() {
5850
this.reviewers = new ReviewAnalyzer(this).getReviewers();
5951
}
6052

6153
async getCollaborators() {
62-
const { owner, repo, logger, requestPromise } = this;
54+
const { owner, repo, logger, request } = this;
6355
logger.trace(`Getting collaborator contacts from README of ${owner}/${repo}`);
64-
this.collaborators = await getCollaborators(requestPromise, logger, owner, repo);
56+
const url = `https://raw.githubusercontent.com/${owner}/${repo}/master/README.md`;
57+
const readme = await request.promise({ url });
58+
this.collaborators = await getCollaborators(readme, logger, owner, repo);
6559
}
6660

6761
async getPR() {
6862
const { prid, owner, repo, logger, request } = this;
6963
logger.trace(`Getting PR from ${owner}/${repo}/pull/${prid}`);
70-
const prData = await request(PR_QUERY, { prid, owner, repo });
64+
const prData = await request.gql(PR_QUERY, { prid, owner, repo });
7165
const pr = this.pr = prData.repository.pullRequest;
7266
// Get the mail
7367
logger.trace(`Getting User information for ${pr.author.login}`);
74-
const userData = await request(USER_QUERY, { login: pr.author.login });
68+
const userData = await request.gql(USER_QUERY, { login: pr.author.login });
7569
const user = userData.user;
7670
Object.assign(this.pr.author, user);
7771
}
7872

7973
async getReviews() {
80-
const { prid, owner, repo, logger, requestAll } = this;
74+
const { prid, owner, repo, logger, request } = this;
8175
const vars = { prid, owner, repo };
8276
logger.trace(`Getting reviews from ${owner}/${repo}/pull/${prid}`);
83-
this.reviews = await requestAll(REVIEWS_QUERY, vars, [
77+
this.reviews = await request.gql(REVIEWS_QUERY, vars, [
8478
'repository', 'pullRequest', 'reviews'
8579
]);
8680
}
8781

8882
async getComments() {
89-
const { prid, owner, repo, logger, requestAll } = this;
83+
const { prid, owner, repo, logger, request } = this;
9084
const vars = { prid, owner, repo };
9185
logger.trace(`Getting comments from ${owner}/${repo}/pull/${prid}`);
92-
this.comments = await requestAll(COMMENTS_QUERY, vars, [
86+
this.comments = await request.gql(COMMENTS_QUERY, vars, [
9387
'repository', 'pullRequest', 'comments'
9488
]);
9589
}
9690

9791
async getCommits() {
98-
const { prid, owner, repo, logger, requestAll } = this;
92+
const { prid, owner, repo, logger, request } = this;
9993
const vars = { prid, owner, repo };
10094
logger.trace(`Getting commits from ${owner}/${repo}/pull/${prid}`);
101-
this.commits = await requestAll(COMMITS_QUERY, vars, [
95+
this.commits = await request.gql(COMMITS_QUERY, vars, [
10296
'repository', 'pullRequest', 'commits'
10397
]);
10498
}

‎lib/request.js

+79-55
Original file line numberDiff line numberDiff line change
@@ -1,66 +1,90 @@
11
'use strict';
22

33
const rp = require('request-promise-native');
4-
const auth = require('./auth');
4+
const fs = require('fs');
5+
const path = require('path');
56

6-
async function request(query, variables) {
7-
const options = {
8-
uri: 'https://api.github.com/graphql',
9-
method: 'POST',
10-
headers: {
11-
'Authorization': `Basic ${await auth()}`,
12-
'User-Agent': 'node-check-pr'
13-
},
14-
json: true,
15-
gzip: true,
16-
body: {
17-
query: query,
18-
variables: variables
19-
}
20-
};
21-
// console.log(options);
22-
const result = await rp(options);
23-
if (result.errors) {
24-
const err = new Error('GraphQL request Error');
25-
err.data = {
26-
// query: query,
27-
variables: variables,
28-
errors: result.errors
29-
};
30-
throw err;
7+
class Request {
8+
constructor(credentials) {
9+
this.credentials = credentials;
3110
}
32-
return result.data;
33-
}
3411

35-
async function requestAll(query, variables, path) {
36-
let after = null;
37-
let all = [];
38-
// first page
39-
do {
40-
const varWithPage = Object.assign({
41-
after
42-
}, variables);
43-
const data = await request(query, varWithPage);
44-
let current = data;
45-
for (const step of path) {
46-
current = current[step];
47-
}
48-
// current should have:
49-
// totalCount
50-
// pageInfo { hasNextPage, endCursor }
51-
// nodes
52-
all = all.concat(current.nodes);
53-
if (current.pageInfo.hasNextPage) {
54-
after = current.pageInfo.endCursor;
12+
loadQuery(file) {
13+
const filePath = path.resolve(__dirname, '..', 'queries', `${file}.gql`);
14+
return fs.readFileSync(filePath, 'utf8');
15+
}
16+
17+
async promise() {
18+
return rp(...arguments);
19+
}
20+
21+
async gql(name, variables, path) {
22+
const query = this.loadQuery(name);
23+
if (path) {
24+
const result = await this.requestAll(query, variables, path);
25+
return result;
5526
} else {
56-
after = null;
27+
const result = await this.request(query, variables);
28+
return result;
5729
}
58-
} while (after !== null);
30+
}
5931

60-
return all;
32+
async request(query, variables) {
33+
const options = {
34+
uri: 'https://api.github.com/graphql',
35+
method: 'POST',
36+
headers: {
37+
'Authorization': `Basic ${this.credentials}`,
38+
'User-Agent': 'node-core-utils'
39+
},
40+
json: true,
41+
gzip: true,
42+
body: {
43+
query: query,
44+
variables: variables
45+
}
46+
};
47+
// console.log(options);
48+
const result = await rp(options);
49+
if (result.errors) {
50+
const err = new Error('GraphQL request Error');
51+
err.data = {
52+
// query: query,
53+
variables: variables,
54+
errors: result.errors
55+
};
56+
throw err;
57+
}
58+
return result.data;
59+
}
60+
61+
async requestAll(query, variables, path) {
62+
let after = null;
63+
let all = [];
64+
// first page
65+
do {
66+
const varWithPage = Object.assign({
67+
after
68+
}, variables);
69+
const data = await this.request(query, varWithPage);
70+
let current = data;
71+
for (const step of path) {
72+
current = current[step];
73+
}
74+
// current should have:
75+
// totalCount
76+
// pageInfo { hasNextPage, endCursor }
77+
// nodes
78+
all = all.concat(current.nodes);
79+
if (current.pageInfo.hasNextPage) {
80+
after = current.pageInfo.endCursor;
81+
} else {
82+
after = null;
83+
}
84+
} while (after !== null);
85+
86+
return all;
87+
}
6188
}
6289

63-
module.exports = {
64-
request,
65-
requestAll
66-
};
90+
module.exports = Request;

0 commit comments

Comments
 (0)