some cr changes

Author: ilanskogan

docs/engine.md

@@ -8,7 +8,8 @@ The Engine stores and executes rules, emits events, and maintains state.
     * [engine.addFact(String id, Function [definitionFunc], Object [options])](#engineaddfactstring-id-function-definitionfunc-object-options)
     * [engine.removeFact(String id)](#engineremovefactstring-id)
     * [engine.addRule(Rule instance|Object options)](#engineaddrulerule-instanceobject-options)
-    * [engine.removeRule(Rule instance)](#engineremoverulerule-instance)
+    * [engine.updateRule(Rule instance|Object options)](#engineupdaterulerule-instanceobject-options)
+    * [engine.removeRule(Rule instance | String ruleId)](#engineremoverulerule-instance)
     * [engine.addOperator(String operatorName, Function evaluateFunc(factValue, jsonValue))](#engineaddoperatorstring-operatorname-function-evaluatefuncfactvalue-jsonvalue)
     * [engine.removeOperator(String operatorName)](#engineremoveoperatorstring-operatorname)
     * [engine.run([Object facts], [Object options]) -> Promise ({ events: [], failureEvents: [], almanac: Almanac, results: [], failureResults: []})](#enginerunobject-facts-object-options---promise--events--failureevents--almanac-almanac-results--failureresults-)
@@ -44,8 +45,6 @@ undefined facts as `undefined`.  (default: false)
 
 `pathResolver` - Allows a custom object path resolution library to be used. (default: `json-path` syntax). See [custom path resolver](./rules.md#condition-helpers-custom-path-resolver) docs.
 
-`ruleKeyField` - By default this field is set to 'id', this field is the rule property that will be used when removing a rule by a key string or when updating rule
-
 ### engine.addFact(String id, Function [definitionFunc], Object [options])
 
 ```js
@@ -75,7 +74,7 @@ engine.removeFact('speed-of-light')
 ### engine.addRule(Rule instance|Object options)
 
 Adds a rule to the engine.  The engine will execute the rule upon the next ```run()```
-if the rule doesn't have value set at rule[engine.ruleKeyField], a random string will be set instead.
+if the rule doesn't have value set at rule.id, a random string will be set instead.
 
 ```js
 let Rule = require('json-rules-engine').Rule
@@ -117,6 +116,8 @@ engine.removeRule(rule.id)
 // adds a rule
 let rule = new Rule()
 engine.addRule(rule)
+
+// change rule condition
 rule.conditions.all = []
 
 //update it

src/engine.js

@@ -25,7 +25,6 @@ class Engine extends EventEmitter {
     this.operators = new Map()
     this.facts = new Map()
     this.status = READY
-    this.ruleKeyField = options.ruleKeyField || 'id'
     rules.map(r => this.addRule(r))
     defaultOperators.map(o => this.addOperator(o))
   }
@@ -51,7 +50,7 @@ class Engine extends EventEmitter {
       rule = new Rule(properties)
     }
     rule.setEngine(this)
-    if (!rule[this.ruleKeyField]) rule[this.ruleKeyField] = '_' + Math.random().toString(36).substr(2, 9)
+    if (!rule.id) rule.id = '_' + Math.random().toString(36).substr(2, 9)
     this.rules.push(rule)
     this.prioritizedRules = null
     return this
@@ -62,25 +61,27 @@ class Engine extends EventEmitter {
    * @param {object|Rule} rule - rule definition. Must be a instance of Rule
    */
   updateRule (rule) {
-    const ruleIndex = this.rules.findIndex(ruleInEngine => ruleInEngine[this.ruleKeyField] === rule[this.ruleKeyField])
+    const ruleIndex = this.rules.findIndex(ruleInEngine => (ruleInEngine.id === rule.id) && rule.id)
     if (ruleIndex > -1) {
       this.rules.splice(ruleIndex, 1)
       this.addRule(rule)
+      this.prioritizedRules = null
     } else {
       throw new Error('Engine: updateRule() rule not found')
     }
   }
 
   /**
    * Remove a rule from the engine
-   * @param {object|Rule||string} rule - rule definition. Must be a instance of Rule
+   * @param {object|Rule|string} rule - rule definition. Must be a instance of Rule
    */
   removeRule (rule) {
     if (!(rule instanceof Rule)) {
-      this.rules = this.rules.filter(ruleInEngine => ruleInEngine[this.ruleKeyField] !== rule)
+      this.rules = this.rules.filter(ruleInEngine => ruleInEngine.id !== rule)
       this.prioritizedRules = null
       return Boolean(this.rules.length)
     } else {
+      if (!rule) return false
       const index = this.rules.indexOf(rule)
       if (index === -1) return false
       this.prioritizedRules = null

src/rule.js

@@ -15,6 +15,7 @@ class Rule extends EventEmitter {
    * @param {string} options.event.params - parameters to pass to the event listener
    * @param {Object} options.conditions - conditions to evaluate when processing this rule
    * @param {any} options.name - identifier for a particular rule, particularly valuable in RuleResult output
+   * @param {any} options.id - identifier for a particular rule, particularly valuable in RuleResult output
    * @return {Rule} instance
    */
   constructor (options) {
@@ -35,6 +36,10 @@ class Rule extends EventEmitter {
       this.setName(options.name)
     }
 
+    if (options && (options.id)) {
+      this.setId(options.id)
+    }
+
     const priority = (options && options.priority) || 1
     this.setPriority(priority)
 
@@ -53,6 +58,18 @@ class Rule extends EventEmitter {
     return this
   }
 
+  /**
+   * Sets the unique id of the rule
+   * @param {any} id - any truthy input
+   */
+  setId (id) {
+    if (!id && id !== 0) {
+      throw new Error('Rule "id" must be defined')
+    }
+    this.id = id
+    return this
+  }
+
   /**
    * Sets the name of the rule
    * @param {any} name - any truthy input and zero is allowed

test/engine.test.js

@@ -93,6 +93,14 @@ describe('Engine', () => {
       engine.updateRule(rule)
       expect(engine.rules[0].conditions.all.length).to.equal(0)
     })
+    it('should throw error if rule not found', () => {
+      const rule1 = new Rule(factories.rule())
+      engine.addRule(rule1)
+      const rule2 = new Rule(factories.rule())
+      expect(() => {
+        engine.updateRule(rule2)
+      }).to.throw(/Engine: updateRule\(\) rule not found/)
+    })
   })
 
   describe('removeRule()', () => {