Merge pull request #32 from CacheControl/cached-fact-path-bugfix

fix almanac.factValue incorrectly interpretting path

Author: Cache Hamm

CHANGELOG.md

@@ -1,3 +1,8 @@
+1.5.1 / 2017-03-19
+==================
+
+  * Bugfix almanac.factValue skipping interpreting condition "path" for cached facts
+
 1.5.0 / 2017-03-12
 ==================
 

dist/almanac.js

@@ -38,7 +38,7 @@ var Almanac = function () {
     _classCallCheck(this, Almanac);
 
     this.factMap = new Map(factMap);
-    this.factResultsCache = new Map();
+    this.factResultsCache = new Map(); // { cacheKey:  Promise<factValu> }
 
     for (var factId in runtimeFacts) {
       var fact = void 0;
@@ -131,33 +131,40 @@ var Almanac = function () {
       var _ref = _asyncToGenerator(regeneratorRuntime.mark(function _callee(factId) {
         var params = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : {};
         var path = arguments.length > 2 && arguments[2] !== undefined ? arguments[2] : '';
-        var fact, cacheKey, cacheVal, factValue;
+        var factValue, fact, cacheKey, cacheVal;
         return regeneratorRuntime.wrap(function _callee$(_context) {
           while (1) {
             switch (_context.prev = _context.next) {
               case 0:
+                factValue = void 0;
                 fact = this._getFact(factId);
                 cacheKey = fact.getCacheKey(params);
                 cacheVal = cacheKey && this.factResultsCache.get(cacheKey);
 
                 if (!cacheVal) {
-                  _context.next = 6;
+                  _context.next = 11;
                   break;
                 }
 
-                cacheVal.then(function (val) {
-                  return debug('almanac::factValue cache hit for fact:' + factId + ' value: ' + JSON.stringify(val) + '<' + (typeof val === 'undefined' ? 'undefined' : _typeof(val)) + '>');
-                });
-                return _context.abrupt('return', cacheVal);
+                _context.next = 7;
+                return cacheVal;
 
-              case 6:
+              case 7:
+                factValue = _context.sent;
+
+                debug('almanac::factValue cache hit for fact:' + factId + ' value: ' + JSON.stringify(factValue) + '<' + (typeof factValue === 'undefined' ? 'undefined' : _typeof(factValue)) + '>');
+                _context.next = 15;
+                break;
+
+              case 11:
                 verbose('almanac::factValue cache miss for fact:' + factId + '; calculating');
-                _context.next = 9;
+                _context.next = 14;
                 return this._setFactValue(fact, params, fact.calculate(params, this));
 
-              case 9:
+              case 14:
                 factValue = _context.sent;
 
+              case 15:
                 if (path) {
                   if (isPlainObject(factValue) || Array.isArray(factValue)) {
                     factValue = selectn(path)(factValue);
@@ -168,7 +175,7 @@ var Almanac = function () {
                 }
                 return _context.abrupt('return', factValue);
 
-              case 12:
+              case 17:
               case 'end':
                 return _context.stop();
             }

package.json

@@ -6,6 +6,7 @@
   "scripts": {
     "test": "mocha && npm run lint --silent",
     "lint": "standard --verbose | snazzy || true",
+    "lint:fix": "standard --fix",
     "prepublish": "npm run compile",
     "compile": "babel --stage 1 -d dist/ src/ && regenerator --no-cache-dir --include-runtime src/generator-runtime.js > dist/generator-runtime.js"
   },

src/almanac.js

@@ -17,7 +17,7 @@ import { UndefinedFactError } from './errors'
 export default class Almanac {
   constructor (factMap, runtimeFacts = {}) {
     this.factMap = new Map(factMap)
-    this.factResultsCache = new Map()
+    this.factResultsCache = new Map() // { cacheKey:  Promise<factValu> }
 
     for (let factId in runtimeFacts) {
       let fact
@@ -89,15 +89,17 @@ export default class Almanac {
    * @return {Promise} a promise which will resolve with the fact computation.
    */
   async factValue (factId, params = {}, path = '') {
+    let factValue
     let fact = this._getFact(factId)
     let cacheKey = fact.getCacheKey(params)
     let cacheVal = cacheKey && this.factResultsCache.get(cacheKey)
     if (cacheVal) {
-      cacheVal.then(val => debug(`almanac::factValue cache hit for fact:${factId} value: ${JSON.stringify(val)}<${typeof val}>`))
-      return cacheVal
+      factValue = await cacheVal
+      debug(`almanac::factValue cache hit for fact:${factId} value: ${JSON.stringify(factValue)}<${typeof factValue}>`)
+    } else {
+      verbose(`almanac::factValue cache miss for fact:${factId}; calculating`)
+      factValue = await this._setFactValue(fact, params, fact.calculate(params, this))
     }
-    verbose(`almanac::factValue cache miss for fact:${factId}; calculating`)
-    let factValue = await this._setFactValue(fact, params, fact.calculate(params, this))
     if (path) {
       if (isPlainObject(factValue) || Array.isArray(factValue)) {
         factValue = selectn(path)(factValue)

test/engine-fact.test.js

@@ -57,9 +57,12 @@ describe('Engine: fact evaluation', () => {
   }
   let successSpy = sinon.spy()
   let failureSpy = sinon.spy()
-  function setup (conditions = baseConditions(), engineOptions = {}) {
+  beforeEach(() => {
     successSpy.reset()
     failureSpy.reset()
+  })
+
+  function setup (conditions = baseConditions(), engineOptions = {}) {
     engine = engineFactory([], engineOptions)
     let rule = factories.rule({ conditions, event })
     engine.addRule(rule)
@@ -157,14 +160,40 @@ describe('Engine: fact evaluation', () => {
       expect(successSpy).to.not.have.been.called()
     })
 
-    it('emits when complex object paths meet the conditions', async () => {
-      let complexCondition = conditions()
-      complexCondition.any[0].path = '.address.occupantHistory[0].year'
-      complexCondition.any[0].value = 2011
-      complexCondition.any[0].operator = 'equal'
-      setup(complexCondition)
-      await engine.run()
-      expect(successSpy).to.have.been.calledWith(event)
+    context('complex paths', () => {
+      it('correctly interprets "path" when dynamic facts return objects', async () => {
+        let complexCondition = conditions()
+        complexCondition.any[0].path = '.address.occupantHistory[0].year'
+        complexCondition.any[0].value = 2011
+        complexCondition.any[0].operator = 'equal'
+        setup(complexCondition)
+        await engine.run()
+        expect(successSpy).to.have.been.calledWith(event)
+      })
+
+      it('correctly interprets "path" with runtime fact objects', async () => {
+        let fact = { x: { y: 1 }, a: 2 }
+        let conditions = {
+          all: [{
+            fact: 'x',
+            path: '.y',
+            operator: 'equal',
+            value: 1
+          }]
+        }
+        let event = {
+          type: 'runtimeEvent'
+        }
+
+        engine = engineFactory([])
+        let rule = factories.rule({ conditions, event })
+        engine.addRule(rule)
+        engine.on('success', successSpy)
+        engine.on('failure', failureSpy)
+        await engine.run(fact)
+        expect(successSpy).to.have.been.calledWith(event)
+        expect(failureSpy).to.not.have.been.calledWith(event)
+      })
     })
 
     it('does not emit when complex object paths fail the condition', async () => {