Repository: CacheControl/json-rules-engine
Pull request #265 (2021-05-31 13:48:11)

refactored remove rule and added updateRule
@CacheControl as we talked, I kept engine.rules as an array. 
with this PR we expect that each rule will have a unique key. the name of the field is customizable in the engine's constructor as you can pass "ruleKeyField" default is ('id').
when adding rule if I see there is no unique key at rule[engine.ruleKeyField] I generate a random string so you can still remove it by key if you want to.
also added updateRule() which is removing the rule and adding him again.
let me know if there is something more I need to do. 
My project needs this functionality as each user in my app can create/remove/update his rules which are stored in MongoDB i need to be able to remove or update them by id.
all unit tests passed and I added 2 more.


Patch:
diff --git a/docs/engine.md b/docs/engine.md
index 2988465..593a2fd 100644
--- a/docs/engine.md
+++ b/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]) -&gt; 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))
diff --git a/src/engine.js b/src/engine.js
index 899c71a..9d42f49 100644
--- a/src/engine.js
+++ b/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)
+    }
   }
 
   /**
diff --git a/src/rule.js b/src/rule.js
index 5db0191..2dc99ed 100644
--- a/src/rule.js
+++ b/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 patch:
diff --git a/test/engine.test.js b/test/engine.test.js
index 4328a44..58d7959 100644
--- a/test/engine.test.js
+++ b/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()', () => {