Condition Sharing

[chris-pardy]

This introduces the concept of Condition sharing which allows regular conditions that are given a well-known name and to be added to the engine. Rules can reference the condition by including a condition property on a condition.

example based on #296

engine.setCondition('isMinor', {
  all: [
    {
        fact: 'age',
        operator: 'lessThan',
       value: { fact: 'drinkingAge' }
    }
  ]
});

engine.addRule({
   conditions: {
      all: [
          { not: { condition: 'isMinor' } },
         {
             fact: 'currentTime',
             operator: 'lessThanInclusive',
             value: { fact: 'lastCall' }
         }
     ]
  },
    event: {
      type: 'AlcoholicBeverageSelection'
   }
});

// condition references can be top-level conditions so it's possible for a rule to just be a condition reference.
engine.addRule({
 conditions: {
    condition: 'isMinor'
  },
  event: {
     type: 'SoftDrinkSelection'
  }
})

This addresses #336, #296, and #263

A few things related to this:

Why do we need this at-all

The ability to capture common conditions and compose rules is useful as a first-class function of this rules engine. As rules reach increased levels of complexity common patterns emerge that should be re-used rather than repeated. One option to do this is to add a "pre-processing" step outside the rules engine that would build up rules from component parts. However this means that someone inspecting the rules as they're run would need to understand the behavior of this pre-processing. Additionally having this be supported by the rules engine allows for more universal editing tools to adopt this as something that can be edited.

Condition references vs. rule-chaining

The primary difference here is that the composition can be communicated using JSON because conditions are fully expressible as JSON. This prevents the javascript code from needing to know about specific rules to setup specific event handlers. This allows us to cleanly separate javascript code that configures the rules engine from the rules themselves. Additionally there is no need to specify rule priority when using conditions as the condition references will be resolved at runtime.

Why condition references and not something with facts

I had originally suggested using a "conditional fact" or other construct to capture the idea of re-used logic. This was nice because it matched concepts that already existed in the rules engine. However it fails the equivalency test. Extracting re-used conditions into a fact would cause the rule results to be different. I liked that the use of Conditions and the approach of de-referencing the condition references at runtime would keep the same structure to the rule results even after refactoring.

Concern with duplicated work.

Condition references are dereferenced and evaluated within each rule they're part of. That means that if a condition is used in 10 rules it will be evaluated 10 times. I'm not concerned about this as the majority of the work will be captured by the Fact value caching in the almanac. The work that is left to be done ought to be very quick. While the possibility of result caching does exist as a future improvement adding it here seemed unnecessarily complex.

[RoboCafaz]

A very simple solution for adding additional composability options to the rules engine. I'm a fan.

The lack of result caching is a bit of a bummer but that's something that could be iterated upon in future updates.

[matthew-mahony]

Such a simple solution to something that will be super useful!

[CacheControl]

Underlying implementation looks good.

Per comments let's think hard about the naming and api we're exposing.

[CacheControl]

I see this as an advanced feature that grows in importance as users begin using the library more extensively; seems like a highly specific thing to mention up front in the readme. Recommend deleting this line

[CacheControl]

why not simply addCondition()? Adding new terminology into the mix suggests to the user they need to understand the difference between a shared condition vs a non-shared condition; wondering if we can maintain the single concept of a condition and simply offer this as an upgrade and additional way to use conditions.

[chris-pardy]

I was doing some googling to try to see what this was called in other rules engines and there doesn't seem to be a good name for these in wide use. So I'm ok with condition as the name I actually like that in a future state this could be used to add new Condition subclass constructors which would let make it even more extensible.

[CacheControl]

wondering if condition: would be more ergonomic, as it clarifies to the user the type of the identifier they need to place in the value, and mentally ties it back to engine.addSharedCondition(). It'd also be consistent with other existing properties such as fact: <id>, operator: <id>. thoughts?

[chris-pardy]

Yeah the more time I spend with it the less I like uses. I'm fine with either condition or ref as I think there a more natural term. The one thing with condition is that it means a rule could be { conditions: { condition: <id> } } how do we feel about that?

[CacheControl]

👍

[CacheControl]

I think you'll see to add a line to the toJSON helper in order to ensure the uses: properly serializes

[chris-pardy]

@CacheControl all comments addressed, looking for a final sign-off before I merge this and put together a point release.