jwadhams / json-logic-js

Build complex rules, serialize them as JSON, and execute them in JavaScript
MIT License
1.26k stars 140 forks source link

"IN" Operator Fails When Accessed Array via "VAR" Operator #135

Open imanrahmani opened 4 months ago

imanrahmani commented 4 months ago

Problem Statement:

While trying to access a subset array from within a superset array using the "All" operator, I found that the "IN" operator does not function correctly when an array is accessed via the "VAR" operator.

Observed Behavior: In the rule example below, the "IN" operator fails to correctly validate if all elements of the subset array are contained within the superset array:

Rule:

{ "all": [ {"var": "subset"}, {"in": [{"var": ""}, {"var": "superset"}]} ] }

Data:

{ "subset": ["apple", "banana", "orange"], "superset": ["apple", "banana", "orange", "grape", "pear"] }

Expected Outcome: The evaluation of this rule should yield True, as the subset array is fully included in the superset array.

Actual Outcome: The rule incorrectly returns False.

Hint / Workaround: When I specify the array directly within the rule instead of referencing it with the "VAR" operator, the "IN" operator behaves as expected:

Working Rule: { "all": [ {"var": "subset"}, {"in": [{"var": ""}, ["apple", "banana", "orange", "grape", "pear"]]} ] }

I suspect there might be a bug in the way the "IN" operator processes references to arrays. Your assistance in identifying and resolving the root cause of this behavior would be greatly appreciated.

jwadhams commented 4 months ago

The in operator isn't documented as supporting "first arg array is a subset of second arg array" -- we're not even attempting that. The JavaScript implementation is actually banking on the fact that both arrays and strings (in the second arg) support the indexOf method: https://github.com/jwadhams/json-logic-js/blob/master/logic.js#L77

If JavaScript has an easy way to just evaluate "array is subset of array" this could be a simple patch, but it would be an extension to the language, not a bug.

jwadhams commented 4 months ago

Of course, if you just wanted to add an isSubset command to your local implementation, that's even easier:


jsonLogic.add_operation("isSubset", function(a,b) {
    for (i in a) {
        if(b.indexOf(a[i]) === -1) {
            return false;
        }
    }
    return true;
));
Tiglat-Pileser commented 2 months ago

The in operator isn't documented as supporting "first arg array is a subset of second arg array" -- we're not even attempting that. The JavaScript implementation is actually banking on the fact that both arrays and strings (in the second arg) support the indexOf method: https://github.com/jwadhams/json-logic-js/blob/master/logic.js#L77

If JavaScript has an easy way to just evaluate "array is subset of array" this could be a simple patch, but it would be an extension to the language, not a bug.

But the first argument of the IN in this example is not an array, is it? When used within ALL the {"var": ""}-expression should act as a reference to the elements of the array being tested.

EDIT: I think the actual problem is that inside expressions like all, some or map, the "var" operator restricts its domain to elements of the array passed as the first argument, so it cannot be used to refer to other parts of the json in the second argument, same problem as in #130, #61, #48, #134. A possible solution might be to restrict this special behaviour of var to the first argument of all, some, map, ... only instead of the entirety if these functions' arguments.

birgerj commented 3 weeks ago

I also came across this issue. It would be nice if this somehow could be supported. I do understand the current implementation but it misses a crucial (to me) scenario.