Reimplement workflow with new plugin system

lib/dispatcher.js

@@ -1,5 +1,4 @@
 const Context = require('./context');
-const issues = require('./plugins/issues');
 
 class Dispatcher {
   constructor(github, event) {
@@ -11,28 +10,8 @@ class Dispatcher {
     // Get behaviors for the event
     const context = new Context(this.github, config, this.event);
 
-    // FIXME: have a better method to register evaluators
-    const evaluators = [
-      issues.Evaluator
-    ];
-
     // Handle all behaviors
-    return Promise.all(config.workflows.map(w => {
+    return Promise.all(config.workflows.map(w => w.execute(context)));
-      if (w.matches(this.event)) {
-        return evaluators.map(E => {
-          const evaluator = new E();
-          if (evaluator.checkIfEventApplies(context.event)) {
-            return evaluator.evaluate(w, context);
-          } else {
-            return [];
-          }
-        });
-      } else {
-        return false;
-      }
-    }).reduce((a, b) => {
-      return a.concat(b);
-    }, []));
   }
 }
 

lib/plugins/issues.js

@@ -1,114 +1,48 @@
 const handlebars = require('handlebars');
-const Evaluator = require('../evaluator');
 
-const IssuePlugin = superclass => class extends superclass {
+module.exports = class IssuesPlugin {
-  comment(comment) {
+  // checkIfEventApplies(event) {
-    this._setCommentData({comment});
+  //   return event.issue !== undefined || event.pull_request !== undefined;
-    return this;
+  // }
+
+  comment(context, content) {
+    const template = handlebars.compile(content)(context.payload);
+    return context.github.issues.createComment(context.payload.toIssue({body: template}));
   }
 
-  assign(...assignees) {
+  assign(context, ...assignees) {
-    this._setCommentData({assign: assignees});
+    return context.github.issues.addAssigneesToIssue(context.payload.toIssue({assignees}));
-    return this;
   }
 
-  unassign(...assignees) {
+  unassign(context, ...assignees) {
-    this._setCommentData({unassign: assignees});
+    return context.github.issues.removeAssigneesFromIssue(context.payload.toIssue({body: {assignees}}));
-    return this;
   }
 
-  label(...labels) {
+  label(context, ...labels) {
-    this._setCommentData({label: labels});
+    return context.github.issues.addLabels(context.payload.toIssue({body: labels}));
-    return this;
   }
 
-  unlabel(...labels) {
+  unlabel(context, ...labels) {
-    this._setCommentData({unlabel: labels});
-    return this;
-  }
-
-  lock() {
-    this._setCommentData({lock: true});
-    return this;
-  }
-
-  unlock() {
-    this._setCommentData({unlock: true});
-    return this;
-  }
-
-  open() {
-    this._setCommentData({open: true});
-    return this;
-  }
-
-  close() {
-    this._setCommentData({close: true});
-    return this;
-  }
-
-  _setCommentData(obj) {
-    if (this.issueActions === undefined) {
-      this.issueActions = {};
-    }
-    Object.assign(this.issueActions, obj);
-  }
-};
-
-// This is the function that implements all of the actions configured above.
-class IssueEvaluator extends Evaluator {
-
-  checkIfEventApplies(event) {
-    return event.issue !== undefined || event.pull_request !== undefined;
-  }
-
-  comment(content) {
-    const template = handlebars.compile(content)(this.payload);
-    return this.github.issues.createComment(this.payload.toIssue({body: template}));
-  }
-
-  assign(assignees) {
-    return this.github.issues.addAssigneesToIssue(this.payload.toIssue({assignees}));
-  }
-
-  unassign(assignees) {
-    return this.github.issues.removeAssigneesFromIssue(this.payload.toIssue({body: {assignees}}));
-  }
-
-  label(labels) {
-    return this.github.issues.addLabels(this.payload.toIssue({body: labels}));
-  }
-
-  unlabel(labels) {
     return labels.map(label => {
-      return this.github.issues.removeLabel(
+      return context.github.issues.removeLabel(
-        this.payload.toIssue({name: label})
+        context.payload.toIssue({name: label})
       );
     });
   }
 
-  lock() {
+  lock(context) {
-    return this.github.issues.lock(this.payload.toIssue({}));
+    return context.github.issues.lock(context.payload.toIssue({}));
   }
 
-  unlock() {
+  unlock(context) {
-    return this.github.issues.unlock(this.payload.toIssue({}));
+    return context.github.issues.unlock(context.payload.toIssue({}));
   }
 
-  open() {
+  open(context) {
-    return this.github.issues.edit(this.payload.toIssue({state: 'open'}));
+    return context.github.issues.edit(context.payload.toIssue({state: 'open'}));
   }
 
-  close() {
+  close(context) {
-    return this.github.issues.edit(this.payload.toIssue({state: 'closed'}));
+    return context.github.issues.edit(context.payload.toIssue({state: 'closed'}));
   }
-
-  pluginData(workflow) {
-    return workflow.issueActions;
-  }
-}
-
-module.exports = {
-  Plugin: IssuePlugin,
-  Evaluator: IssueEvaluator
 };

lib/sandbox.js

@@ -14,7 +14,7 @@ class Sandbox {
   on(...events) {
     const workflow = new Workflow(events);
     this.workflows.push(workflow);
-    return workflow;
+    return workflow.api;
   }
 
   execute() {

lib/workflow.js

@@ -1,14 +1,33 @@
-const issues = require('./plugins/issues');
+const Issues = require('./plugins/issues');
 
-class WorkflowCore {
+const plugins = [
+  new Issues()
+];
+
+module.exports = class Workflow {
   constructor(events) {
+    this.stack = [];
     this.events = events;
     this.filterFn = () => true;
+    this.api = {};
+
+    for (const plugin of plugins) {
+      // Get all the property names of the plugin
+      for (const method of Object.getOwnPropertyNames(plugin.constructor.prototype)) {
+        if (method !== 'constructor') {
+          // Define a new function in the API for this plugin method, forcing
+          // the binding to this to prevent any tampering.
+          this.api[method] = this.proxy(plugin, method).bind(this);
+        }
+      }
+    }
+
+    this.api.filter = this.filter.bind(this);
   }
 
   filter(fn) {
     this.filterFn = fn;
-    return this;
+    return this.api;
   }
 
   matches(event) {
@@ -17,18 +36,24 @@ class WorkflowCore {
       return name === event.event && (!action || action === event.payload.action);
     }) && this.filterFn(event);
   }
-}
 
-// FIXME: issues
+  proxy(plugin, method) {
-const plugins = [
+    // This function is what gets exposed to the sandbox
-  issues.Plugin
+    return (...args) => {
-];
+      // Push new function on the stack that calls the plugin method with a context.
+      this.stack.push(context => plugin[method](context, ...args));
 
-// Helper to combine an array of mixins into one class
+      // Return the API to allow methods to be chained.
-function mix(superclass, ...mixins) {
+      return this.api;
-  return mixins.reduce((c, mixin) => mixin(c), superclass);
+    };
-}
+  }
 
-class Workflow extends mix(WorkflowCore, ...plugins) {}
+  execute(context) {
-
+    if (this.matches(context.event)) {
-module.exports = Workflow;
+      // Reduce the stack to a chain of promises, each called with the given context
+      this.stack.reduce((promise, func) => {
+        return promise.then(func.bind(func, context));
+      }, Promise.resolve());
+    }
+  }
+};

scratchpad.js

@@ -40,7 +40,7 @@ class Workflow {
     this.stack = [];
     this.api = {};
 
-    for(const plugin of PLUGINS) {
+    for (const plugin of PLUGINS) {
       // Get all the property names of the plugin
       for (const method of Object.getOwnPropertyNames(plugin.constructor.prototype)) {
         if (method !== 'constructor') {

test/plugins/issues.js

@@ -1,36 +1,34 @@
 const expect = require('expect');
-const issues = require('../../lib/plugins/issues');
+const Issues = require('../../lib/plugins/issues');
-const Workflow = require('../../lib/workflow');
 const Context = require('../../lib/context');
 const payload = require('../fixtures/webhook/comment.created.json');
 
 const createSpy = expect.createSpy;
 
-const github = {
-  issues: {
-    lock: createSpy(),
-    unlock: createSpy(),
-    edit: createSpy(),
-    addLabels: createSpy(),
-    createComment: createSpy(),
-    addAssigneesToIssue: createSpy(),
-    removeAssigneesFromIssue: createSpy(),
-    removeLabel: createSpy()
-  }
-};
-const context = new Context(github, {}, {payload});
-
 describe('issues plugin', () => {
+  const github = {
+    issues: {
+      lock: createSpy().andReturn(Promise.resolve()),
+      unlock: createSpy().andReturn(Promise.resolve()),
+      edit: createSpy().andReturn(Promise.resolve()),
+      addLabels: createSpy().andReturn(Promise.resolve()),
+      createComment: createSpy().andReturn(Promise.resolve()),
+      addAssigneesToIssue: createSpy().andReturn(Promise.resolve()),
+      removeAssigneesFromIssue: createSpy().andReturn(Promise.resolve()),
+      removeLabel: createSpy().andReturn(Promise.resolve())
+    }
+  };
+
+  const context = new Context(github, {}, {payload});
+
   before(() => {
-    this.w = new Workflow();
+    this.issues = new Issues();
-    this.evaluator = new issues.Evaluator();
   });
 
   describe('locking', () => {
     it('locks', () => {
-      this.w.lock();
+      this.issues.lock(context);
 
-      Promise.all(this.evaluator.evaluate(this.w, context));
       expect(github.issues.lock).toHaveBeenCalledWith({
         owner: 'bkeepers-inc',
         repo: 'test',
@@ -39,9 +37,8 @@ describe('issues plugin', () => {
     });
 
     it('unlocks', () => {
-      this.w.unlock();
+      this.issues.unlock(context);
 
-      Promise.all(this.evaluator.evaluate(this.w, context));
       expect(github.issues.unlock).toHaveBeenCalledWith({
         owner: 'bkeepers-inc',
         repo: 'test',
@@ -52,9 +49,8 @@ describe('issues plugin', () => {
 
   describe('state', () => {
     it('opens an issue', () => {
-      this.w.open();
+      this.issues.open(context);
 
-      Promise.all(this.evaluator.evaluate(this.w, context));
       expect(github.issues.edit).toHaveBeenCalledWith({
         owner: 'bkeepers-inc',
         repo: 'test',
@@ -63,9 +59,8 @@ describe('issues plugin', () => {
       });
     });
     it('closes an issue', () => {
-      this.w.close();
+      this.issues.close(context);
 
-      Promise.all(this.evaluator.evaluate(this.w, context));
       expect(github.issues.edit).toHaveBeenCalledWith({
         owner: 'bkeepers-inc',
         repo: 'test',
@@ -77,9 +72,8 @@ describe('issues plugin', () => {
 
   describe('labels', () => {
     it('adds a label', () => {
-      this.w.label('hello');
+      this.issues.label(context, 'hello');
 
-      Promise.all(this.evaluator.evaluate(this.w, context));
       expect(github.issues.addLabels).toHaveBeenCalledWith({
         owner: 'bkeepers-inc',
         repo: 'test',
@@ -89,9 +83,8 @@ describe('issues plugin', () => {
     });
 
     it('adds multiple labels', () => {
-      this.w.label('hello', 'world');
+      this.issues.label(context, 'hello', 'world');
 
-      Promise.all(this.evaluator.evaluate(this.w, context));
       expect(github.issues.addLabels).toHaveBeenCalledWith({
         owner: 'bkeepers-inc',
         repo: 'test',
@@ -101,9 +94,8 @@ describe('issues plugin', () => {
     });
 
     it('removes a single label', () => {
-      this.w.unlabel('hello');
+      this.issues.unlabel(context, 'hello');
 
-      Promise.all(this.evaluator.evaluate(this.w, context));
       expect(github.issues.removeLabel).toHaveBeenCalledWith({
         owner: 'bkeepers-inc',
         repo: 'test',
@@ -113,9 +105,8 @@ describe('issues plugin', () => {
     });
 
     it('removes a multiple labels', () => {
-      this.w.unlabel('hello', 'goodbye');
+      this.issues.unlabel(context, 'hello', 'goodbye');
 
-      Promise.all(this.evaluator.evaluate(this.w, context));
       expect(github.issues.removeLabel).toHaveBeenCalledWith({
         owner: 'bkeepers-inc',
         repo: 'test',
@@ -134,9 +125,8 @@ describe('issues plugin', () => {
 
   describe('comments', () => {
     it('creates a comment', () => {
-      this.w.comment('Hello world!');
+      this.issues.comment(context, 'Hello world!');
 
-      Promise.all(this.evaluator.evaluate(this.w, context));
       expect(github.issues.createComment).toHaveBeenCalledWith({
         owner: 'bkeepers-inc',
         repo: 'test',
@@ -146,9 +136,8 @@ describe('issues plugin', () => {
     });
 
     it('evaluates templates with handlebars', () => {
-      this.w.comment('Hello @{{ sender.login }}!');
+      this.issues.comment(context, 'Hello @{{ sender.login }}!');
 
-      Promise.all(this.evaluator.evaluate(this.w, context));
       expect(github.issues.createComment).toHaveBeenCalledWith({
         owner: 'bkeepers-inc',
         repo: 'test',
@@ -160,9 +149,8 @@ describe('issues plugin', () => {
 
   describe('assignment', () => {
     it('assigns a user', () => {
-      this.w.assign('bkeepers');
+      this.issues.assign(context, 'bkeepers');
 
-      Promise.all(this.evaluator.evaluate(this.w, context));
       expect(github.issues.addAssigneesToIssue).toHaveBeenCalledWith({
         owner: 'bkeepers-inc',
         repo: 'test',
@@ -172,9 +160,8 @@ describe('issues plugin', () => {
     });
 
     it('assigns multiple users', () => {
-      this.w.assign('hello', 'world');
+      this.issues.assign(context, 'hello', 'world');
 
-      Promise.all(this.evaluator.evaluate(this.w, context));
       expect(github.issues.addAssigneesToIssue).toHaveBeenCalledWith({
         owner: 'bkeepers-inc',
         repo: 'test',
@@ -184,9 +171,8 @@ describe('issues plugin', () => {
     });
 
     it('unassigns a user', () => {
-      this.w.unassign('bkeepers');
+      this.issues.unassign(context, 'bkeepers');
 
-      Promise.all(this.evaluator.evaluate(this.w, context));
       expect(github.issues.removeAssigneesFromIssue).toHaveBeenCalledWith({
         owner: 'bkeepers-inc',
         repo: 'test',
@@ -196,9 +182,8 @@ describe('issues plugin', () => {
     });
 
     it('unassigns multiple users', () => {
-      this.w.unassign('hello', 'world');
+      this.issues.unassign(context, 'hello', 'world');
 
-      Promise.all(this.evaluator.evaluate(this.w, context));
       expect(github.issues.removeAssigneesFromIssue).toHaveBeenCalledWith({
         owner: 'bkeepers-inc',
         repo: 'test',