Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
imanrahmani opened this issue May 7, 2024 · 4 comments
Open

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

imanrahmani opened this issue May 7, 2024 · 4 comments

Comments

@imanrahmani
Copy link

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.

@imanrahmani imanrahmani changed the title "IN" Operator Fails When Accessed via "VAR" Operator "IN" Operator Fails When Accessed Array via "VAR" Operator May 7, 2024
@jwadhams
Copy link
Owner

jwadhams commented May 7, 2024

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
Copy link
Owner

jwadhams commented May 7, 2024

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
Copy link

Tiglat-Pileser commented Jun 20, 2024

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
Copy link

birgerj commented Aug 19, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants