Supporting optional jsonValueValidator in Operator class?
Nobody has claimed this yet.
Assessment
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Newbie friendliness
- 42/100
- Issue type
- Feature
- Clarity
- Mostly clear
- Activity status
- Stale
- Tech stack
- javascript
- Domain
- backend
Research direction
Start with src/operator.js and the default operator setup in src/engine-default-operators.js, then reproduce the in/notIn and contains/notContains examples from the issue. Check how the proposed optional jsonValueValidator would affect operator evaluation; done means non-array values for in/notIn are handled gracefully and consistently with the documented behavior.
Written by the indexing model from the issue text.
Description
In the documentation on JSON rules engine operators it states that the fact passed to the contains/notContains operators must be arrays. And I see in the Operator class that a factValueValidator function can be supplied to the constructor to ensure that the fact supplied to this operator meets that expectation.
Similarly, the documentation states that for in/notIn operators, the value side of the comparison must be an array. But unlike contains/notContains, there's no validation in place to check that this is the case. And that means you don't have the same graceful handling when a non-array value is passed where an array is expected. A minimal-ish example to illustrate how in/notIn are not symmetrical to contains/notContains:
const { Engine } = require('json-rules-engine');
const facts = {
people: {
someguy: 'dave',
otherguys: ['hal', 'stanley', 'alex']
},
};
// This rule will throw an error because the path $.nonexistentPath evaluates to undefined and b.indexOf(a) throws an error
const operatorInWithNonexistentValuePathRule = {
conditions: {
all: [
{
fact: 'people',
path: '$.someguy',
operator: 'in', // notIn produces the same error
value: {
fact: 'people',
path: '$.nonexistentpath'
}
}
]
},
event: {
type: 'in-with-nonexistent-value-path-rule',
}
};
// This rule will NOT throw an error because contains operator is defined with a factValueValidator that will return false if the factValue is not an array
const operatorContainsWithNonexistentFactPathRule = {
conditions: {
all: [
{
fact: 'people',
path: '$.nonexistentpath',
operator: 'contains',
value: {
fact: 'people',
path: '$.someguy'
}
}
]
},
event: {
type: 'contains-with-nonexistent-fact-path-rule',
}
};
function runEngine(
rule,
facts
){
const engine = new Engine();
engine.addRule(rule);
engine
.run(facts)
.then(({ failureEvents }) => {
failureEvents.map(event => console.log(event));
})
.catch(console.error);
}
runEngine(operatorInWithNonexistentValuePathRule, facts);
runEngine(operatorContainsWithNonexistentFactPathRule, facts);
I'm curious if this is the intentional/desired behavior here? Naively I would've expected that in/notIn operators can (and would) validate values similarly to how contains/notContains validate facts. I think this could be accomplished with a minor rewrite to the Operator class, something like:
'use strict'
export default class Operator {
/**
* Constructor
* @param {string} name - operator identifier
* @param {function(factValue, jsonValue)} callback - operator evaluation method
* @param {function} [factValueValidator] - optional validator for asserting the data type of the fact
* @param {function} [jsonValueValidator] - optional validator for asserting the data type of the "value" property of the condition
* @returns {Operator} - instance
*/
constructor (name, cb, factValueValidator) {
this.name = String(name)
if (!name) throw new Error('Missing operator name')
if (typeof cb !== 'function') throw new Error('Missing operator callback')
this.cb = cb
this.factValueValidator = factValueValidator
if (!this.factValueValidator) this.factValueValidator = () => true
this.jsonValueValidator = jsonValueValidator
if (!this.jsonValueValidator) this.jsonValueValidator = () => true
}
/**
* Takes the fact result and compares it to the condition 'value', using the callback
* @param {mixed} factValue - fact result
* @param {mixed} jsonValue - "value" property of the condition
* @returns {Boolean} - whether the values pass the operator test
*/
evaluate (factValue, jsonValue) {
return this.factValueValidator(factValue) && this.jsonValueValidator(jsonValue) && this.cb(factValue, jsonValue)
}
}
And tweaking the initialization of the default engine operators. If there's interest in doing something along these lines I'd be glad to try and throw together a small PR.
- Dominant language
- JavaScript
- Stars
- 3.1k
- Forks
- 507
- PR merge metrics
- No merged PRs in 30d
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from CacheControl/json-rules-engine
-
Difficulty 4/5 3-5 days Newbie friendliness 35/100
CacheControl/json-rules-engine#427 · 1 reaction ·
-
Difficulty 4/5 3-5 days Newbie friendliness 35/100
-
Difficulty 3/5 1-2 days Newbie friendliness 48/100
CacheControl/json-rules-engine#424 · 1 comment ·
-
Difficulty 3/5 1-2 days Newbie friendliness 25/100
CacheControl/json-rules-engine#421 · 1 comment ·
-
Difficulty 3/5 1-2 days Newbie friendliness 38/100
CacheControl/json-rules-engine#417 · 1 reaction ·
All issues in CacheControl/json-rules-engine
Similar issues
-
ai-observability bug team/ai-observability
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
AllTheMods/ATM-10-L#19 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
SignalK/n2k-signalk#345 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
Difficulty 1/5 1-3 hours Newbie friendliness 88/100
JuliaPluto/PlutoPlotly.jl#72 ·