CacheControl / CacheControl/json-rules-engine

Supporting optional jsonValueValidator in Operator class?

未关闭
#412 3 条评论 1 个 reaction 已指派 0 人 在 GitHub 查看
主要语言
JavaScript
星标
3.1k
派生
507
PR 合并指标
30 天内没有已合并 PR

描述

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](https://github.com/CacheControl/json-rules-engine/blob/master/src/operator.js#L8) that a factValueValidator function can be [supplied to the constructor](https://github.com/CacheControl/json-rules-engine/blob/master/src/engine-default-operators.js) 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](https://github.com/CacheControl/json-rules-engine/blob/master/src/engine-default-operators.js). If there's interest in doing something along these lines I'd be glad to try and throw together a small PR.

贡献指南

这个仓库没有索引到贡献指南

调研方向

从 src/operator.js 和 src/engine-default-operators.js 中的默认 operator 设置开始,然后复现 issue 中的 in/notIn 和 contains/notContains 示例。检查建议的可选 jsonValueValidator 会如何影响 operator evaluation;完成的标准是,in/notIn 的非数组值能够按照文档说明的行为得到妥善且一致的处理。

由索引模型根据 Issue 内容生成。

评估

技术栈
javascript
领域
backend
Issue 类型
功能
难度
3/5
预计耗时
1-2 天
活跃度
停滞
描述清晰度
基本清楚
新手友好度
42/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。