Skip to content

Commit

Permalink
JS SDK - Fix operator precedence in advanced attribute targeting (#2516)
Browse files Browse the repository at this point in the history
  • Loading branch information
Kevin-Chant committed Jun 12, 2024
1 parent acd0466 commit eb28379
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 24 deletions.
20 changes: 12 additions & 8 deletions docs/docs/lib/build-your-own.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -446,15 +446,16 @@ There is only one public method `evalCondition` and everything else is a private

### public evalCondition(attributes: Attributes, condition: Condition): boolean

This is the main function used to evaluate a condition.
This is the main function used to evaluate a condition. It loops through the condition key/value pairs
and checks each entry:

1. If condition has a key `$or`, return `evalOr(attributes, condition["$or"])`
2. If condition has a key `$nor`, return `!evalOr(attributes, condition["$nor"])`
3. If condition has a key `$and`, return `evalAnd(attributes, condition["$and"])`
4. If condition has a key `$not`, return `!evalCondition(attributes, condition["$not"])`
5. Loop through the condition key/value pairs
1. If `evalConditionValue(value, getPath(attributes, key))` is false, break out of loop and return false
6. Return true
1. If condition key is `$or`, check if `evalOr(attributes, condition["$or"])` is false. If so, break out of the loop and return false
2. If condition key is `$nor`, check if `!evalOr(attributes, condition["$nor"])` is false. If so, break out of the loop and return false
3. If condition key is `$and`, check if `evalAnd(attributes, condition["$and"])` is false. If so, break out of the loop and return false
4. If condition key is `$not`, check if `!evalCondition(attributes, condition["$not"])` is false. If so, break out of the loop and return false
5. Otherwise, check if `evalConditionValue(value, getPath(attributes, key))` is false. If so, break out of the loop and return false

If none of the entries failed their checks, `evalCondition` returns true

### private evalOr(attributes: Attributes, conditions: Condition[]): boolean

Expand Down Expand Up @@ -1236,3 +1237,6 @@ Open a [GitHub issue](https://github.com/growthbook/growthbook/issues) with a li
- Remove `versionCompare` test cases (these are now just included as part of `evalCondition`)
- Tweak to `isIncludedInRollout` to handle an edge case when coverage is zero. Also added test case for this.
- Add `id` property to feature rules (reserved for future use)
- **v0.6.1** 2024-05-13
- Update logic in `evalCondition` to allow for and/or/not/nor operators to appear at the same level as other conditions
- Added test cases for multiple operators on the same level
34 changes: 18 additions & 16 deletions packages/sdk-js/src/mongrule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,23 +17,25 @@ export function evalCondition(
obj: TestedObj,
condition: ConditionInterface
): boolean {
// Recursive condition
if ("$or" in condition) {
return evalOr(obj, condition["$or"] as ConditionInterface[]);
}
if ("$nor" in condition) {
return !evalOr(obj, condition["$nor"] as ConditionInterface[]);
}
if ("$and" in condition) {
return evalAnd(obj, condition["$and"] as ConditionInterface[]);
}
if ("$not" in condition) {
return !evalCondition(obj, condition["$not"] as ConditionInterface);
}

// Condition is an object, keys are object paths, values are the condition for that path
// Condition is an object, keys are either specific operators or object paths
// values are either arguments for operators or conditions for paths
for (const [k, v] of Object.entries(condition)) {
if (!evalConditionValue(v, getPath(obj, k))) return false;
switch (k) {
case "$or":
if (!evalOr(obj, v as ConditionInterface[])) return false;
break;
case "$nor":
if (evalOr(obj, v as ConditionInterface[])) return false;
break;
case "$and":
if (!evalAnd(obj, v as ConditionInterface[])) return false;
break;
case "$not":
if (evalCondition(obj, v as ConditionInterface)) return false;
break;
default:
if (!evalConditionValue(v, getPath(obj, k))) return false;
}
}
return true;
}
Expand Down
53 changes: 53 additions & 0 deletions packages/sdk-js/test/cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -2775,6 +2775,59 @@
"version": "1.2.3.4"
},
true
],
[
"$or pass but second condition fail",
{
"$or": [{ "foo": 1 }, { "bar": 1 }],
"baz": 2
},
{
"foo": 1,
"bar": 2,
"baz": 1
},
false
],
[
"$or and second condition both pass",
{
"$or": [{ "foo": 1 }, { "bar": 1 }],
"baz": 2
},
{
"foo": 1,
"bar": 2,
"baz": 2
},
true
],
[
"$and condition pass but $or fail",
{
"$and": [{ "foo": 1 }, { "bar": 1 }],
"$or": [{ "baz": 1 }, { "empty": 1 }]
},
{
"foo": 1,
"bar": 1,
"baz": 2
},
false
],
[
"$and and $or both pass",
{
"$and": [{ "foo": 1 }, { "bar": 1 }],
"$or": [{ "baz": 1 }, { "empty": 1 }]
},
{
"foo": 1,
"bar": 1,
"baz": 2,
"empty": 1
},
true
]
],
"hash": [
Expand Down

1 comment on commit eb28379

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deploy preview for docs ready!

✅ Preview
https://docs-3670puv5a-growthbook.vercel.app

Built with commit eb28379.
This pull request is being automatically deployed with vercel-action

Please sign in to comment.