Merge pull request #265 from Workiz/remove_update_rule_by_key

Author: CacheControl

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-)
@@ -73,6 +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.id, a random string will be set instead.
 
 ```js
 let Rule = require('json-rules-engine').Rule
@@ -102,6 +104,24 @@ engine.addRule(rule)
 
 //remove it
 engine.removeRule(rule)
+//or
+engine.removeRule(rule.id)
+```
+
+ ### engine.updateRule(Rule instance|Object options)
+
+ Updates a rule in the engine.
+
+```javascript
+// adds a rule
+let rule = new Rule()
+engine.addRule(rule)
+
+// change rule condition
+rule.conditions.all = []
+
+//update it in the engine
+engine.updateRule(rule)
 ```
 
 ### engine.addOperator(String operatorName, Function evaluateFunc(factValue, jsonValue))

src/engine.js

@@ -47,27 +47,45 @@ class Engine extends EventEmitter {
     } else {
       if (!Object.prototype.hasOwnProperty.call(properties, 'event')) throw new Error('Engine: addRule() argument requires "event" property')
       if (!Object.prototype.hasOwnProperty.call(properties, 'conditions')) throw new Error('Engine: addRule() argument requires "conditions" property')
-
       rule = new Rule(properties)
     }
     rule.setEngine(this)
-
     this.rules.push(rule)
     this.prioritizedRules = null
     return this
   }
 
   /**
-   * Remove a rule from the engine
+   * update a rule in the engine
    * @param {object|Rule} rule - rule definition. Must be a instance of Rule
    */
-  removeRule (rule) {
-    if ((rule instanceof Rule) === false) throw new Error('Engine: removeRule() rule must be a instance of Rule')
+  updateRule (rule) {
+    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')
+    }
+  }
 
-    const index = this.rules.indexOf(rule)
-    if (index === -1) return false
-    this.prioritizedRules = null
-    return Boolean(this.rules.splice(index, 1).length)
+  /**
+   * Remove a rule from the engine
+   * @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.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
+      return Boolean(this.rules.splice(index, 1).length)
+    }
   }
 
   /**

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,8 @@ class Rule extends EventEmitter {
       this.setName(options.name)
     }
 
+    this.setId(options ? options.id : null)
+
     const priority = (options && options.priority) || 1
     this.setPriority(priority)
 
@@ -53,6 +56,19 @@ class Rule extends EventEmitter {
     return this
   }
 
+  /**
+   * Sets the unique id of the rule
+   * @param {any} id - any truthy input
+   */
+  setId (id) {
+    if (!id) {
+      this.id = '_' + Math.random().toString(36).substr(2, 9)
+    } else {
+      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

@@ -84,6 +84,30 @@ describe('Engine', () => {
     })
   })
 
+  describe('updateRule()', () => {
+    it('updates rule', () => {
+      const rule = new Rule(factories.rule())
+      engine.addRule(rule)
+      expect(engine.rules[0].conditions.all.length).to.equal(2)
+      rule.conditions = { all: [] }
+      engine.updateRule(rule)
+      expect(engine.rules[0].conditions.all.length).to.equal(0)
+    })
+    it('should generate id for rule if not provided', () => {
+      const rule = new Rule(factories.rule())
+      expect(rule.id).to.not.equal(null)
+      expect(rule.id).to.not.equal(undefined)
+    })
+    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()', () => {
     describe('rule instance', () => {
       it('removes the rule', () => {
@@ -96,14 +120,6 @@ describe('Engine', () => {
       })
     })
 
-    describe('required fields', () => {
-      it('.conditions', () => {
-        expect(() => {
-          engine.removeRule([])
-        }).to.throw(/Engine: removeRule\(\) rule must be a instance of Rule/)
-      })
-    })
-
     it('can only remove added rules', () => {
       expect(engine.rules.length).to.equal(0)
       const rule = new Rule(factories.rule())
@@ -118,6 +134,14 @@ describe('Engine', () => {
       engine.removeRule(rule)
       expect(engine.prioritizedRules).to.equal(null)
     })
+    it('removes rule based on ruleKey', () => {
+      const rule = new Rule(factories.rule())
+      engine.addRule(rule)
+      expect(engine.rules.length).to.equal(1)
+      engine.removeRule(rule.id)
+      expect(engine.rules.length).to.equal(0)
+      expect(engine.prioritizedRules).to.equal(null)
+    })
   })
 
   describe('addOperator()', () => {