Supporting optional jsonValueValidator in Operator class?
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 3/5
- Tiempo estimado
- 1-2 días
- Aptitud para principiantes
- 42/100
- Tipo de issue
- Nueva funcionalidad
- Claridad
- Bastante claro
- Estado de actividad
- Estancado
- Stack tecnológico
- javascript
- Área
- backend
Línea de trabajo
Empieza con src/operator.js y la configuración predeterminada de operadores en src/engine-default-operators.js, y después reproduce los ejemplos de in/notIn y contains/notContains del issue. Comprueba cómo afectaría el jsonValueValidator opcional propuesto a la evaluación de operadores; se considera terminado cuando los valores que no son arrays para in/notIn se gestionan correctamente y de forma coherente con el comportamiento documentado.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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.
- Lenguaje dominante
- JavaScript
- Estrellas
- 3.1k
- Forks
- 507
- Métricas de merge de PR
- Sin PR fusionados en 30 d
Guía de contribución
No hay ninguna guía de contribución indexada para este repositorio
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de CacheControl/json-rules-engine
-
Dificultad 4/5 3-5 días Aptitud para principiantes 35/100
CacheControl/json-rules-engine#427 · 1 reacción ·
-
Dificultad 4/5 3-5 días Aptitud para principiantes 35/100
-
Dificultad 3/5 1-2 días Aptitud para principiantes 48/100
CacheControl/json-rules-engine#424 · 1 comentario ·
-
How to handle array of objects? Abierto
Dificultad 3/5 1-2 días Aptitud para principiantes 25/100
CacheControl/json-rules-engine#421 · 1 comentario ·
-
Dificultad 3/5 1-2 días Aptitud para principiantes 38/100
CacheControl/json-rules-engine#417 · 1 reacción ·
Todos los issues de CacheControl/json-rules-engine
Issues similares
-
bug
Dificultad 1/5 Menos de una hora Aptitud para principiantes 90/100
apache/cloudstack#14222 ·
-
Browser Waiting for: Product Owner
Dificultad 2/5 1-3 horas Aptitud para principiantes 85/100
getsentry/sentry-javascript#24577 · 1 comentario ·
-
curation good first issue
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100
amponce/archive-movie-browser#186 ·
-
light
Dificultad 2/5 1-3 horas Aptitud para principiantes 85/100
aemdemos/patients-stryker#253 ·
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 86/100
clerk/javascript#9852 ·