[fix] GitHub and Commenter should be per-request, not singletons

lib/plugins/jira/index.js

@@ -32,10 +32,13 @@ class JiraPlugin {
    * @public
    * @param {Object} data PR webhook data
    * @param {Object} config Configuration for plugin
+   * @param {Object} apis A set of APIs needed to process the request
+   * @param {Commenter} apis.commenter Commenter object for aggregating comments to post
+   * @param {GitHubClient} apis.github A GitHubClient instance for this request
    * @param {Function} done Continuation callback
    * @returns {Undefined} Nothing of significance
    */
-  processRequest(data, config, done) { // eslint-disable-line max-statements
+  processRequest(data, config, apis, done) { // eslint-disable-line max-statements
     const isEdit = data.action === 'edited';
     let oldTitle = null;
 
@@ -65,7 +68,7 @@ class JiraPlugin {
       return void done();
     }
 
-    this.findTicketsAndPost(ticketIds, done);
+    this.findTicketsAndPost(ticketIds, apis.commenter, done);
   }
 
   /**
@@ -74,9 +77,10 @@ class JiraPlugin {
    * @memberof JiraPlugin
    * @private
    * @param {String[]} ticketIds A list of ticket IDs
+   * @param {Commenter} commenter Commenter object for aggregating comments to post
    * @param {Function} done Continuation callback
    */
-  findTicketsAndPost(ticketIds, done) {
+  findTicketsAndPost(ticketIds, commenter, done) {
     const jql = `id in ('${ticketIds.join("', '")}')`;
 
     request.post(`${this.jiraConfig.protocol}://${this.jiraConfig.host}/rest/api/2/search`, {
@@ -106,7 +110,7 @@ class JiraPlugin {
 
       // call some API to post the comment on the PR
       const comment = `I found the following Jira ticket(s) referenced in this PR:\n${ticketList}`;
-      this.app.commenter.addComment(comment, Commenter.priority.Low);
+      commenter.addComment(comment, Commenter.priority.Low);
       done();
     });
   }

lib/plugins/required-file/index.js

@@ -30,16 +30,19 @@ class RequiredFilePlugin {
    * @param {Object} data PR webhook data
    * @param {Object} config Configuration for this plugin
    * @param {String[]} config.files File paths to require in the PR
+   * @param {Object} apis A set of APIs needed to process the request
+   * @param {Commenter} apis.commenter Commenter object for aggregating comments to post
+   * @param {GitHubClient} apis.github A GitHubClient instance for this request
    * @param {Function} done Continuation callback
    * @returns {Undefined} Nothing of significance
    */
-  processRequest(data, config, done) {
+  processRequest(data, config, apis, done) {
     const files = config && config.files;
     if (!files) {
       return done(new Error('Missing `files` field in plugin config'));
     }
 
-    async.each(files, this.checkFile.bind(this, data), done);
+    async.each(files, this.checkFile.bind(this, data, apis), done);
   }
 
   /**
@@ -53,32 +56,35 @@ class RequiredFilePlugin {
    * @memberof RequiredFilePlugin
    * @private
    * @param {Object} data PR webhook data
+   * @param {Object} apis A set of APIs needed to process the request
+   * @param {Commenter} apis.commenter Commenter object for aggregating comments to post
+   * @param {GitHubClient} apis.github A GitHubClient instance for this request
    * @param {String|RequiredFile} file The file object, or path to check, relative to the root of the repo
    * @param {Function} done Continuation callback
    * @returns {Undefined} Nothing of significance
    */
-  checkFile(data, file, done) {
+  checkFile(data, apis, file, done) {
     const filePath = typeof file === 'string' ? file : (file && file.path);
     if (!filePath) {
       return done(new Error('No file path specified for required file.'));
     }
     const message = file.message ||
       `You're missing a change to ${filePath}, which is a requirement for changes to this repo.`;
-    this.app.github.checkIfFileExists(data.repository.full_name, filePath, (checkFileExistsErr, exists) => {
+    apis.github.checkIfFileExists(data.repository.full_name, filePath, (checkFileExistsErr, exists) => {
       if (checkFileExistsErr) return done(checkFileExistsErr);
 
       if (!exists) {
         // File doesn't exist in this repo in the first place, nothing to do
         return done();
       }
 
-      this.app.github.getFilesInPullRequest(data.repository.full_name, data.pull_request.number, (getPRFilesErr, files) => {
+      apis.github.getFilesInPullRequest(data.repository.full_name, data.pull_request.number, (getPRFilesErr, files) => {
         if (getPRFilesErr) return done(getPRFilesErr);
 
         if (!files.some(f => {
           return f.filename === filePath;
         })) {
-          this.app.commenter.addComment(message, Commenter.priority.High);
+          apis.commenter.addComment(message, Commenter.priority.High);
         }
 
         return void done();

lib/plugins/reviewers/index.js

@@ -38,15 +38,18 @@ class ReviewerPlugin {
    *  package.json
    * @param {Number} [config.howMany] Number of reviewers to choose from the set of contributors and maintainers,
    *  defaults to choosing all of the possible reviewers
+   * @param {Object} apis A set of APIs needed to process the request
+   * @param {Commenter} apis.commenter Commenter object for aggregating comments to post
+   * @param {GitHubClient} apis.github A GitHubClient instance for this request
    * @param {Function} done Continuation callback
    */
-  processRequest(data, config, done) {
+  processRequest(data, config, apis, done) {
     config = config || {};
     const commentFormat = config.commentFormat || this.defaultCommentFormat;
     if (config.reviewers) {
-      this.requestReviews(data, config.reviewers, config.howMany, commentFormat, done);
+      this.requestReviews(data, config.reviewers, config.howMany, commentFormat, apis, done);
     } else {
-      this.getPackageJson(data.repository, (packageJsonErr, packageInfo) => {
+      this.getPackageJson(data.repository, apis, (packageJsonErr, packageInfo) => {
         if (packageJsonErr) return done(packageJsonErr);
         if (!packageInfo) {
           // No package.json, nothing to do
@@ -58,6 +61,7 @@ class ReviewerPlugin {
           reviewers,
           config.howMany,
           commentFormat,
+          apis,
           done);
       });
     }
@@ -120,16 +124,19 @@ class ReviewerPlugin {
    * @param {Number} [howMany] How many reviewers to select, default is all
    * @param {String} [commentFormat] An optional comment format string to use when posting a comment about
    *  the review request
+   * @param {Object} apis A set of APIs needed to process the request
+   * @param {Commenter} apis.commenter Commenter object for aggregating comments to post
+   * @param {GitHubClient} apis.github A GitHubClient instance for this request
    * @param {Function} done Continuation callback
    * @returns {Undefined} Nothing of significance
    */
-  requestReviews(data, reviewers, howMany, commentFormat, done) { // eslint-disable-line max-params
+  requestReviews(data, reviewers, howMany, commentFormat, apis, done) { // eslint-disable-line max-params
     if (!reviewers) {
       // No users to work with, nothing to do
       return void done();
     }
 
-    this.getUsersFromReviewersList(reviewers, data.pull_request, (err, userList) => {
+    this.getUsersFromReviewersList(reviewers, data.pull_request, apis, (err, userList) => {
       if (err) return done(err);
       if (!userList || userList.length === 0) {
         // No reviewers, nothing to do
@@ -144,13 +151,13 @@ class ReviewerPlugin {
 
       if (commentFormat) {
         const githubUsernames = usersToRequest.sort().map(username => `@${username}`).join(', ');
-        this.app.commenter.addComment(
+        apis.commenter.addComment(
           commentFormat.replace('%s', githubUsernames),
           Commenter.priority.High
         );
       }
 
-      this.app.github.requestReviewers(
+      apis.github.requestReviewers(
         data.repository.full_name,
         data.pull_request.number,
         usersToRequest,
@@ -165,10 +172,12 @@ class ReviewerPlugin {
    * @memberof ReviewerPlugin
    * @private
    * @param {Object} repoInfo Repository info
+   * @param {Object} apis A set of APIs needed to process the request
+   * @param {GitHubClient} apis.github A GitHubClient instance for this request
    * @param {Function} done Continuation callback
    */
-  getPackageJson(repoInfo, done) {
-    this.app.github.getFileContents(repoInfo.full_name, {
+  getPackageJson(repoInfo, apis, done) {
+    apis.github.getFileContents(repoInfo.full_name, {
       path: 'package.json'
     }, (getRcErr, pkg) => {
       if (getRcErr) {
@@ -178,7 +187,7 @@ class ReviewerPlugin {
         return done(getRcErr);
       }
 
-      this.app.github.parsePackageJson(pkg, done);
+      apis.github.parsePackageJson(pkg, done);
     });
   }
 
@@ -189,10 +198,12 @@ class ReviewerPlugin {
    * @private
    * @param {String[]} reviewers List of reviewers, often in the format `Joe Schmoe <jschmoe@domain.com>`
    * @param {Object} prInfo PR information
+   * @param {Object} apis A set of APIs needed to process the request
+   * @param {GitHubClient} apis.github A GitHubClient instance for this request
    * @param {Function} done Continuation callback
    * @returns {Undefined} Nothing of significance
    */
-  getUsersFromReviewersList(reviewers, prInfo, done) {
+  getUsersFromReviewersList(reviewers, prInfo, apis, done) {
     if (!reviewers || !reviewers.length) return done();
 
     const maybeUsers = Array.from(new Set(reviewers.map(r => {
@@ -214,7 +225,7 @@ class ReviewerPlugin {
     })));
 
     async.reduce(maybeUsers, [], (memo, user, next) => {
-      this.app.github.userExists(user, (userExistsErr, exists) => {
+      apis.github.userExists(user, (userExistsErr, exists) => {
         if (userExistsErr) return next(userExistsErr);
         next(null, exists ? memo.concat([user]) : memo);
       });

lib/preboots/index.js

@@ -26,8 +26,6 @@ function Preboots(app, opts, callback) {
     Prism.highlight(options.fn(), Prism.languages.json, 'json')));
   app.engine('hbs', hbs.__express);
 
-  app.preboot(require('./github'));
-  app.preboot(require('./commenter'));
   app.preboot(require('./processor'));
   app.preboot(require('./plugins'));
 

lib/processor.js

@@ -1,4 +1,5 @@
 const async = require('async');
+const Commenter = require('./commenter');
 
 /**
  * Processor for incoming webhook requests
@@ -19,10 +20,11 @@ class Processor {
    * @memberof Processor
    * @public
    * @param {HTTPRequest} req Request object from Slay
+   * @param {GitHubClient} github The GitHubClient instance for this request
    * @param {Function} done Continuation callback
    * @returns {Undefined} Nothing of significance
    */
-  processRequest(req, done) {
+  processRequest(req, github, done) {
     const eventType = req.headers['x-github-event'];
     if (eventType !== 'pull_request') {
       // Not a PR, nothing to do
@@ -40,7 +42,7 @@ class Processor {
 
     this.app.log.info('Processing PR', { repository: data.repository.full_name, number: data.number, requestId });
 
-    this.getRepoConfig(data.repository, (getRepoConfigErr, config) => {
+    this.getRepoConfig(data.repository, github, (getRepoConfigErr, config) => {
       if (getRepoConfigErr) {
         this.app.log.error('Error getting repository config', { error: getRepoConfigErr, requestId });
         return done(getRepoConfigErr);
@@ -53,6 +55,11 @@ class Processor {
         return done();
       }
 
+      const apis = {
+        commenter: new Commenter(),
+        github
+      };
+
       async.each(config.plugins, (pluginConfig, next) => {
         const pluginName = typeof pluginConfig === 'string' ? pluginConfig : pluginConfig.plugin;
         const plugin = this.app.plugins[pluginName];
@@ -66,7 +73,7 @@ class Processor {
           return next();
         }
 
-        plugin.processRequest(data, pluginConfig.config || {}, pluginProcessRequestErr => {
+        plugin.processRequest(data, pluginConfig.config || {}, apis, pluginProcessRequestErr => {
           if (pluginProcessRequestErr) {
             this.app.log.error('Error running plugin',
               { error: pluginProcessRequestErr, repository: data.repository.full_name, plugin: pluginName, requestId });
@@ -80,9 +87,9 @@ class Processor {
         this.app.log.info('Finished processing PR',
           { repository: data.repository.full_name, number: data.number, requestId });
 
-        const comment = this.app.commenter.flushToString();
+        const comment = apis.commenter.flushToString();
         if (comment) {
-          this.app.github.createIssueComment(data.repository.full_name, data.pull_request.number, comment, done);
+          github.createIssueComment(data.repository.full_name, data.pull_request.number, comment, done);
         } else {
           return done();
         }
@@ -96,10 +103,11 @@ class Processor {
    * @memberof Processor
    * @private
    * @param {Object} repoInfo Information about the target repo retrieved from the webhook payload
+   * @param {GitHubClient} github The GitHubClient instance for this request
    * @param {Function} done Continuation callback
    */
-  getRepoConfig(repoInfo, done) {
-    this.app.github.getFileContents(repoInfo.full_name, {
+  getRepoConfig(repoInfo, github, done) {
+    github.getFileContents(repoInfo.full_name, {
       path: '.pullierc'
     }, (getRcErr, pkg) => {
       if (getRcErr) {
@@ -109,7 +117,7 @@ class Processor {
         return done(getRcErr);
       }
 
-      this.app.github.parsePackageJson(pkg, done);
+      github.parsePackageJson(pkg, done);
     });
   }
 }

lib/routes/index.js

@@ -1,6 +1,7 @@
 const { normalizeError } = require('../utils');
 const resolveCwd = require('resolve-cwd');
 const packageJson = require('../../package.json');
+const GitHubClient = require('../github');
 
 /*
  * Setup the ordering for all of our routing in
@@ -37,17 +38,18 @@ function Routes(app, opts, callback) {
         });
         return next();
       }
-      app.github.login(req.body.installation.id, loginErr => {
+      const github = new GitHubClient(config);
+      github.login(req.body.installation.id, loginErr => {
         if (loginErr) {
-          app.github.logout();
+          github.logout();
           res.status(403).json({
             error: loginErr.toString()
           });
           return next();
         }
 
-        app.processor.processRequest(req, processErr => {
-          app.github.logout();
+        app.processor.processRequest(req, github, processErr => {
+          github.logout();
           if (processErr) {
             const status = processErr.status || 500;
             res.status(status).json({