fix bug where a rule would emitting succes/failure multiple times

Author: Cache Hamm

src/rule.js

@@ -161,11 +161,6 @@ class Rule extends EventEmitter {
         }
       }
       condition.result = passes
-      if (passes) {
-        this.emit('success', this.event, almanac, ruleResult)
-      } else {
-        this.emit('failure', this.event, almanac, ruleResult)
-      }
       return passes
     }
 
@@ -245,14 +240,23 @@ class Rule extends EventEmitter {
       return prioritizeAndRun(conditions, 'all')
     }
 
-    if (ruleResult.conditions.any) {
-      let result = await any(ruleResult.conditions.any)
+    /**
+     * Emits based on rule evaluation result, and decorates ruleResult with 'result' property
+     * @param {Boolean} result
+     */
+    let processResult = (result) => {
       ruleResult.result = result
+      if (result) this.emit('success', ruleResult.event, almanac, ruleResult)
+      else this.emit('failure', ruleResult.event, almanac, ruleResult)
       return ruleResult
+    }
+
+    if (ruleResult.conditions.any) {
+      let result = await any(ruleResult.conditions.any)
+      return processResult(result)
     } else {
       let result = await all(ruleResult.conditions.all)
-      ruleResult.result = result
-      return ruleResult
+      return processResult(result)
     }
   }
 }

test/engine-event.test.js

@@ -18,28 +18,39 @@ describe('Engine: event', () => {
       fact: 'age',
       operator: 'greaterThanInclusive',
       value: 21
+    }, {
+      fact: 'qualified',
+      operator: 'equal',
+      value: true
     }]
   }
   beforeEach(() => {
     engine = engineFactory()
     let ruleOptions = { conditions, event, priority: 100 }
     let determineDrinkingAgeRule = factories.rule(ruleOptions)
     engine.addRule(determineDrinkingAgeRule)
+    // age will succeed because 21 >= 21
     engine.addFact('age', 21)
+    // set 'qualified' to fail. rule will succeed because of 'any'
+    engine.addFact('qualified', false)
   })
 
-  describe('engine events', () => {
-    it('passes the event type and params', (done) => {
-      engine.on('success', function (a, almanac, ruleResult) {
-        try {
-          expect(a).to.eql(event)
-          expect(almanac).to.be.an.instanceof(Almanac)
-          expect(ruleResult.result).to.be.true()
-          expect(ruleResult.conditions.any[0].result).to.be.true()
-        } catch (e) { return done(e) }
-        done()
+  context('engine events', () => {
+    it('passes the event type and params', async () => {
+      let failureSpy = sinon.spy()
+      let successSpy = sinon.spy()
+      engine.on('success', function (e, almanac, ruleResult) {
+        expect(e).to.eql(event)
+        expect(almanac).to.be.an.instanceof(Almanac)
+        expect(ruleResult.result).to.be.true()
+        expect(ruleResult.conditions.any[0].result).to.be.true()
+        expect(ruleResult.conditions.any[1].result).to.be.false()
+        successSpy()
       })
-      engine.run()
+      engine.on('failure', failureSpy)
+      await engine.run()
+      expect(failureSpy.callCount).to.equal(0)
+      expect(successSpy.callCount).to.equal(1)
     })
 
     it('emits using the event "type"', (done) => {
@@ -89,38 +100,44 @@ describe('Engine: event', () => {
       })
     })
   })
-  describe('rule events', () => {
-    it('on-success, it passes the event type and params', (done) => {
+
+  context('rule events', () => {
+    it('on-success, it passes the event type and params', async () => {
       let failureSpy = sinon.spy()
+      let successSpy = sinon.spy()
       let rule = engine.rules[0]
       rule.on('success', function (e, almanac, ruleResult) {
-        try {
-          expect(e).to.eql(event)
-          expect(almanac).to.be.an.instanceof(Almanac)
-          expect(failureSpy.callCount).to.equal(0)
-          expect(ruleResult.conditions.any[0].result).to.be.true()
-        } catch (err) { return done(err) }
-        done()
+        expect(e).to.eql(event)
+        expect(almanac).to.be.an.instanceof(Almanac)
+        expect(failureSpy.callCount).to.equal(0)
+        expect(ruleResult.result).to.be.true()
+        expect(ruleResult.conditions.any[0].result).to.be.true()
+        expect(ruleResult.conditions.any[1].result).to.be.false()
+        successSpy()
       })
       rule.on('failure', failureSpy)
-      engine.run()
+      await engine.run()
+      expect(successSpy.callCount).to.equal(1)
     })
 
-    it('on-failure, it passes the event type and params', (done) => {
+    it('on-failure, it passes the event type and params', async () => {
       let successSpy = sinon.spy()
+      let failureSpy = sinon.spy()
       let rule = engine.rules[0]
       rule.on('failure', function (e, almanac, ruleResult) {
-        try {
-          expect(e).to.eql(event)
-          expect(almanac).to.be.an.instanceof(Almanac)
-          expect(successSpy.callCount).to.equal(0)
-          expect(ruleResult.conditions.any[0].result).to.be.false()
-        } catch (err) { return done(err) }
-        done()
+        expect(e).to.eql(event)
+        expect(almanac).to.be.an.instanceof(Almanac)
+        expect(successSpy.callCount).to.equal(0)
+        expect(ruleResult.result).to.be.false()
+        expect(ruleResult.conditions.any[0].result).to.be.false()
+        expect(ruleResult.conditions.any[1].result).to.be.false()
+        failureSpy()
       })
       rule.on('success', successSpy)
+      // both conditions will fail
       engine.addFact('age', 10)
-      engine.run()
+      await engine.run()
+      expect(failureSpy.callCount).to.equal(1)
     })
   })
 })