diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 658c4be..2813451 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,35 +1,93 @@ -# Contributing Guide +# Contributing -We welcome all contributions! +## Local setup -## Type checking - -We use MyPy for type checking. +Python 3.9+ is supported; CI tests 3.9–3.13. From a cloned checkout: ```bash -make type-check +python3.13 -m venv .venv +source .venv/bin/activate +python -m pip install --upgrade pip +python -m pip install -e . -r requirements.txt -r requirements_dev.txt ``` -## Tests +Alternatively, use `uv venv --python 3.13 .venv` and `uv pip install` with the +same dependencies. Configure your editor to use `.venv/bin/python`. Unit tests +need no GrowthBook credentials or running server. + +## Checks -Run the test suite with +With the virtual environment active, run from the repository root: ```bash -make test +pytest -q +mypy +flake8 . --extend-exclude=.venv --count --select=E9,F63,F7,F82 --show-source --statistics ``` -## Linting +For typing changes, also run Pyright with the version pinned in CI: + +```bash +npm install -g pyright@1.1.403 +pyright +pytest tests/test_typing.py -q +``` -Lint the code with flake8 +`make test` and `make type-check` wrap pytest and MyPy. `make coverage` produces +an HTML report. `make lint` uses older rules and fails on existing style issues; +the flake8 command above matches CI's blocking check. + +## Working on the SDK + +- `growthbook/core.py` contains evaluation shared by the sync `GrowthBook` + (`growthbook.py`) and async `GrowthBookClient` (`growthbook_client.py`). Test + both clients, including loading and refresh when payload handling changes. +- Keep library code compatible with Python 3.9 and add type annotations. + Public API changes should include typing regressions in `tests/typing/`. +- Mock external HTTP calls and close clients created in tests. Async tests use + `@pytest.mark.asyncio`; async fixtures use `@pytest_asyncio.fixture`. +- Follow the [JavaScript reference SDK](https://github.com/growthbook/growthbook/tree/main/packages/sdk-js) + for evaluation behavior. Preserve Python-local cases when syncing + `tests/cases.json` with the shared corpus. + +For evaluation or corpus changes, check parity with JS `main` (requires network): ```bash -make lint +python tests/scripts/check_corpus_freshness.py ``` -## Releasing +Use `--js-source /path/to/cases.json` for a local reference. Missing or changed +shared cases fail unless listed with a reason in `tests/scripts/corpus_skiplist.json`. + +For performance changes, compare both checkouts using the same interpreter and +machine. Benchmark commands and integration checks are in +[tests/scripts/README.md](tests/scripts/README.md). + +## Pull requests + +Discuss public API changes in an issue first. Branch from `main`, include +regression tests and relevant usage docs, and run the checks above. Describe +behavior changes, compatibility concerns, and validation in the PR. + +Use conventional commit subjects (`feat:`, `fix:`, etc.); Release Please uses +them to determine versions and changelog entries. Keep the merge or squash +subject consistent with the change. + +## Releases + +The [Release Please workflow](.github/workflows/release-please.yml) creates a +release PR after changes reach `main`. Merging that PR creates the tag and +GitHub release, then tests, builds, and publishes the package to PyPI. + +`setup.py` reads the version from `growthbook/__init__.py`. Contributors normally +leave version and changelog updates to Release Please. The legacy `make release` +command uploads directly to PyPI; it is not a verification command. + +## Help + +- [SDK docs](https://docs.growthbook.io/lib/python) +- [Issues](https://github.com/growthbook/growthbook-python/issues) +- [Slack](https://slack.growthbook.io?ref=contributing) -1. Bump the version in `setup.cfg` and `setup.py` -2. Merge code to `main` -3. Create a new release on GitHub with your version as the tag (e.g. `v0.2.0`) -4. Run `make dist` to create a distribution file -5. Run `make release` to upload to pypi +Report vulnerabilities to security@growthbook.io. Follow the +[Code of Conduct](https://github.com/growthbook/growthbook/blob/main/CODE_OF_CONDUCT.md). diff --git a/README.md b/README.md index 48fe16a..411b098 100644 --- a/README.md +++ b/README.md @@ -394,6 +394,31 @@ gb = GrowthBook( Note: When doing this, you do not need to specify your `api_host` or `client_key` and you don't need to call `gb.load_features()`. +### Saved group references + +Both `GrowthBook` and `GrowthBookClient` support `savedGroupReferencesV2` for +local evaluation. The `savedGroups` payload map can contain legacy ID arrays +or typed entries: + +```json +{ + "savedGroups": { + "beta": {"type": "list", "attributeKey": "id", "values": ["u1", "u2"]}, + "power": {"type": "condition", "condition": {"plan": "pro"}} + } +} +``` + +A rule references either type with `{"$savedGroup": {"id": "beta"}}`. List +references can override `attributeKey`; condition groups can reference other +groups, with cycle detection. Legacy `$inGroup` and `$notInGroup` also accept +typed list entries. + +When loading a payload yourself, use `gb.set_payload(payload)` or +`await client.set_payload(payload)` to load both features and saved groups. +Receiving v2 payloads requires a registered SDK version with this capability +and **All Saved Groups** under the SDK Connection's **Pass Saved Groups by reference**. + ## GrowthBook class The GrowthBook constructor has the following parameters: diff --git a/growthbook/core.py b/growthbook/core.py index d8ddfea..e5f7310 100644 --- a/growthbook/core.py +++ b/growthbook/core.py @@ -29,38 +29,111 @@ # errored). Matches CONTEXTUAL_BANDIT_FALLBACK_LEAF_ID in the JS SDK. CONTEXTUAL_BANDIT_FALLBACK_LEAF_ID = -1 -def evalCondition(attributes: Dict[str, Any], condition: Dict[str, Any], savedGroups: Optional[Dict[str, Any]] = None) -> bool: +def evalCondition( + attributes: Dict[str, Any], condition: Dict[str, Any], + savedGroups: Optional[Dict[str, Any]] = None, visited: Optional[Set[str]] = None, +) -> bool: + try: + return _eval_condition(attributes, condition, savedGroups, visited) + except (TypeError, ValueError, AttributeError, IndexError, RecursionError): + # Fail the entire condition so $not cannot turn an evaluation error into a match. + return False + + +def _eval_condition( + attributes: Dict[str, Any], condition: Dict[str, Any], + savedGroups: Optional[Dict[str, Any]], visited: Optional[Set[str]], +) -> bool: + """Evaluate recursively, leaving evaluation error handling to the outer caller.""" for key, value in condition.items(): if key == "$or": - if not evalOr(attributes, value, savedGroups): + if not evalOr(attributes, value, savedGroups, visited): return False elif key == "$nor": - if evalOr(attributes, value, savedGroups): + if evalOr(attributes, value, savedGroups, visited): return False elif key == "$and": - if not evalAnd(attributes, value, savedGroups): + if not evalAnd(attributes, value, savedGroups, visited): return False elif key == "$not": - if evalCondition(attributes, value, savedGroups): + if _eval_condition(attributes, value, savedGroups, visited): + return False + elif key == "$savedGroup": + if not _eval_saved_group(attributes, value, savedGroups, visited): return False - elif not evalConditionValue(value, getPath(attributes, key), savedGroups): + elif key == "$savedGroups": + return False + elif not evalConditionValue(value, getPath(attributes, key), savedGroups, visited=visited): return False return True -def evalOr(attributes: Dict[str, Any], conditions: List[Any], savedGroups: Optional[Dict[str, Any]]) -> bool: +def _eval_saved_group( + attributes: Dict[str, Any], reference: Any, saved_groups: Optional[Dict[str, Any]], + visited: Optional[Set[str]], +) -> bool: + """Resolve a typed saved group, keeping cycle detection local to this branch.""" + if not isinstance(reference, dict): + return False + group_id = reference.get("id") + if not isinstance(group_id, str) or (visited is not None and group_id in visited): + return False + if "attributeKey" in reference and not isinstance(reference["attributeKey"], str): + return False + + entry = saved_groups.get(group_id) if saved_groups is not None else None + if not isinstance(entry, dict): + return False + + if entry.get("type") == "list": + key = reference.get("attributeKey", entry.get("attributeKey")) + values = entry.get("values") + if not isinstance(key, str) or not isinstance(values, list): + return False + return isIn(values, getPath(attributes, key)) + if entry.get("type") == "condition": + condition = entry.get("condition") + if not isinstance(condition, dict): + return False + # Allocate the branch guard only when entering a condition group. + next_visited = visited | {group_id} if visited else {group_id} + return _eval_condition(attributes, condition, saved_groups, next_visited) + return False + + +def _saved_group_values(saved_groups: Optional[Dict[str, Any]], group_id: str) -> Optional[List[Any]]: + """Distinguish an absent group (empty list) from a malformed entry (None).""" + if saved_groups is None or group_id not in saved_groups: + return [] + entry = saved_groups[group_id] + if isinstance(entry, list): + return entry + if isinstance(entry, dict) and entry.get("type") == "list": + values = entry.get("values") + if isinstance(values, list): + return values + return None + + +def evalOr( + attributes: Dict[str, Any], conditions: List[Any], savedGroups: Optional[Dict[str, Any]], + visited: Optional[Set[str]] = None, +) -> bool: if len(conditions) == 0: return True for condition in conditions: - if evalCondition(attributes, condition, savedGroups): + if _eval_condition(attributes, condition, savedGroups, visited): return True return False -def evalAnd(attributes: Dict[str, Any], conditions: List[Any], savedGroups: Optional[Dict[str, Any]]) -> bool: +def evalAnd( + attributes: Dict[str, Any], conditions: List[Any], savedGroups: Optional[Dict[str, Any]], + visited: Optional[Set[str]] = None, +) -> bool: for condition in conditions: - if not evalCondition(attributes, condition, savedGroups): + if not _eval_condition(attributes, condition, savedGroups, visited): return False return True @@ -97,10 +170,13 @@ def getPath(attributes: Dict[str, Any], path: str) -> Any: return None return current -def evalConditionValue(conditionValue: Any, attributeValue: Any, savedGroups: Optional[Dict[str, Any]], insensitive: bool = False) -> bool: +def evalConditionValue( + conditionValue: Any, attributeValue: Any, savedGroups: Optional[Dict[str, Any]], + insensitive: bool = False, visited: Optional[Set[str]] = None, +) -> bool: if isinstance(conditionValue, dict) and isOperatorObject(conditionValue): for key, value in conditionValue.items(): - if not evalOperatorCondition(key, attributeValue, value, savedGroups): + if not evalOperatorCondition(key, attributeValue, value, savedGroups, visited): return False return True @@ -110,16 +186,19 @@ def evalConditionValue(conditionValue: Any, attributeValue: Any, savedGroups: Op return bool(conditionValue == attributeValue) -def elemMatch(condition: Dict[str, Any], attributeValue: Any, savedGroups: Optional[Dict[str, Any]]) -> bool: +def elemMatch( + condition: Dict[str, Any], attributeValue: Any, savedGroups: Optional[Dict[str, Any]], + visited: Optional[Set[str]] = None, +) -> bool: if not isinstance(attributeValue, list): return False for item in attributeValue: if isOperatorObject(condition): - if evalConditionValue(condition, item, savedGroups): + if evalConditionValue(condition, item, savedGroups, visited=visited): return True else: - if evalCondition(item, condition, savedGroups): + if _eval_condition(item, condition, savedGroups, visited): return True return False @@ -178,7 +257,10 @@ def _js_strict_equal(a: Any, b: Any) -> bool: return bool(a == b) -def evalOperatorCondition(operator: str, attributeValue: Any, conditionValue: Any, savedGroups: Any) -> bool: +def evalOperatorCondition( + operator: str, attributeValue: Any, conditionValue: Any, savedGroups: Any, + visited: Optional[Set[str]] = None, +) -> bool: if operator == "$eq": return _js_strict_equal(attributeValue, conditionValue) elif operator == "$ne": @@ -215,18 +297,16 @@ def evalOperatorCondition(operator: str, attributeValue: Any, conditionValue: An return paddedVersionString(attributeValue) > paddedVersionString(conditionValue) elif operator == "$vgte": return paddedVersionString(attributeValue) >= paddedVersionString(conditionValue) - elif operator == "$inGroup": + elif operator in ("$inGroup", "$notInGroup"): if not isinstance(conditionValue, str): return False - if not conditionValue in savedGroups: - return False - return isIn(savedGroups[conditionValue] or [], attributeValue) - elif operator == "$notInGroup": - if not isinstance(conditionValue, str): + values = savedGroups.get(conditionValue) if savedGroups is not None else None + if not isinstance(values, list): + values = _saved_group_values(savedGroups, conditionValue) + if values is None: return False - if not conditionValue in savedGroups: - return True - return not isIn(savedGroups[conditionValue] or [], attributeValue) + matches = isIn(values, attributeValue) + return matches if operator == "$inGroup" else not matches elif operator == "$regex": try: r = re.compile(conditionValue) @@ -271,19 +351,19 @@ def evalOperatorCondition(operator: str, attributeValue: Any, conditionValue: An return False return not isIn(conditionValue, attributeValue, insensitive=True) elif operator == "$elemMatch": - return elemMatch(conditionValue, attributeValue, savedGroups) + return elemMatch(conditionValue, attributeValue, savedGroups, visited) elif operator == "$size": if not isinstance(attributeValue, list): return False - return evalConditionValue(conditionValue, len(attributeValue), savedGroups) + return evalConditionValue(conditionValue, len(attributeValue), savedGroups, visited=visited) elif operator == "$all": if not isinstance(conditionValue, list): return False - return isInAll(conditionValue, attributeValue, savedGroups, insensitive=False) + return isInAll(conditionValue, attributeValue, savedGroups, insensitive=False, visited=visited) elif operator == "$alli": if not isinstance(conditionValue, list): return False - return isInAll(conditionValue, attributeValue, savedGroups, insensitive=True) + return isInAll(conditionValue, attributeValue, savedGroups, insensitive=True, visited=visited) elif operator == "$exists": if not conditionValue: return attributeValue is None @@ -291,7 +371,7 @@ def evalOperatorCondition(operator: str, attributeValue: Any, conditionValue: An elif operator == "$type": return bool(getType(attributeValue) == conditionValue) elif operator == "$not": - return not evalConditionValue(conditionValue, attributeValue, savedGroups) + return not evalConditionValue(conditionValue, attributeValue, savedGroups, visited=visited) return False def paddedVersionString(input: Any) -> str: @@ -339,18 +419,50 @@ def case_fold(val: Any) -> Any: # Do an intersection if attribute is an array (insensitive) if isinstance(attributeValue, list): return any( - case_fold(el) == case_fold(exp) + case_fold(el) == case_fold(exp) and isinstance(el, bool) == isinstance(exp, bool) for el in attributeValue for exp in conditionValue ) - return any(case_fold(attributeValue) == case_fold(exp) for exp in conditionValue) + return any( + case_fold(attributeValue) == case_fold(exp) + and isinstance(attributeValue, bool) == isinstance(exp, bool) + for exp in conditionValue + ) - # Case-sensitive behavior (original) if isinstance(attributeValue, list): - return bool(set(conditionValue) & set(attributeValue)) + try: + matches = set(conditionValue) & set(attributeValue) + # Python collapses booleans with 0/1; any other match is unambiguous. + if not matches or matches.difference((0, 1)): + return bool(matches) + return any(_contains_boolean_or_number(conditionValue, value) + for value in attributeValue if value == 0 or value == 1) + except TypeError: + # JSON arrays may contain objects or arrays, which cannot be hashed. + pass + return any(_js_strict_equal(value, expected) + for value in attributeValue for expected in conditionValue) + if attributeValue == 0 or attributeValue == 1: + return _contains_boolean_or_number(conditionValue, attributeValue) return attributeValue in conditionValue -def isInAll(conditionValue: List[Any], attributeValue: Any, savedGroups: Optional[Dict[str, Any]], insensitive: bool = False) -> bool: + +def _contains_boolean_or_number(values: List[Any], value: Any) -> bool: + """Find 0/1 or False/True without Python's boolean/number equality aliasing.""" + start = 0 + while True: + try: + index = values.index(value, start) + except ValueError: + return False + if isinstance(values[index], bool) == isinstance(value, bool): + return True + start = index + 1 + +def isInAll( + conditionValue: List[Any], attributeValue: Any, savedGroups: Optional[Dict[str, Any]], + insensitive: bool = False, visited: Optional[Set[str]] = None, +) -> bool: """Check if attributeValue (array) contains all elements in conditionValue""" if not isinstance(attributeValue, list): return False @@ -358,7 +470,7 @@ def isInAll(conditionValue: List[Any], attributeValue: Any, savedGroups: Optiona for cond in conditionValue: passing = False for attr in attributeValue: - if evalConditionValue(cond, attr, savedGroups, insensitive): + if evalConditionValue(cond, attr, savedGroups, insensitive, visited): passing = True break if not passing: diff --git a/tests/cases.json b/tests/cases.json index 5d4e57f..f8ac7f4 100644 --- a/tests/cases.json +++ b/tests/cases.json @@ -1,5 +1,5 @@ { - "specVersion": "0.8.0", + "specVersion": "0.9.0", "evalCondition": [ [ "$not - pass", @@ -4160,6 +4160,300 @@ ] }, true + ], + [ + "$vgt - date-based version", + { + "version": { + "$vgt": "20260909.9.9" + } + }, + { + "version": "20260910.1.1" + }, + true + ], + [ + "$vlt - oversized condition operand", + { + "version": { + "$vlt": "123456789" + } + }, + { + "version": "1.0.0" + }, + true + ], + [ + "$veq - oversized on both sides", + { + "version": { + "$veq": "123456789" + } + }, + { + "version": "123456789" + }, + true + ], + [ + "$vgt - oversized pre-release segment", + { + "version": { + "$vgt": "1.2.3-rc.123455" + } + }, + { + "version": "1.2.3-rc.123456" + }, + true + ], + [ + "$vlte - oversized equal segments", + { + "version": { + "$vlte": "20260910.1.0" + } + }, + { + "version": "20260910.1.0" + }, + true + ], + [ + "$vlt - absent attribute is below any release", + { + "version": { + "$vlt": "0.0.1" + } + }, + {}, + true + ], + [ + "$vgt - absent attribute is not above a release", + { + "version": { + "$vgt": "0.0.1" + } + }, + {}, + false + ], + [ + "$veq - absent attribute equals 0", + { + "version": { + "$veq": "0" + } + }, + {}, + true + ], + [ + "$vne - absent attribute differs from a real version", + { + "version": { + "$vne": "1.2.3" + } + }, + {}, + true + ], + [ + "$vgt - numeric attribute is stringified", + { + "version": { + "$vgt": "1.0.0" + } + }, + { + "version": 2 + }, + true + ], + [ + "$veq - numeric attribute equals numeric condition", + { + "version": { + "$veq": 2 + } + }, + { + "version": 2 + }, + true + ], + [ + "$veq - float attribute splits on the decimal point", + { + "version": { + "$veq": "1.5" + } + }, + { + "version": 1.5 + }, + true + ], + [ + "$veq - empty string attribute is version 0", + { + "version": { + "$veq": "0" + } + }, + { + "version": "" + }, + true + ], + [ + "$veq - boolean attribute is version 0", + { + "version": { + "$veq": "0" + } + }, + { + "version": true + }, + true + ], + [ + "$veq - array attribute is version 0", + { + "version": { + "$veq": "0" + } + }, + { + "version": [ + "1.0.0" + ] + }, + true + ], + [ + "$veq - null attribute is version 0", + { + "version": { + "$veq": "0" + } + }, + { + "version": null + }, + true + ], + [ + "$veq - leading v is ignored", + { + "version": { + "$veq": "1.2.3" + } + }, + { + "version": "v1.2.3" + }, + true + ], + [ + "$veq - v inside a pre-release tag is kept", + { + "version": { + "$veq": "1.2.3-dev" + } + }, + { + "version": "1.2.3-dev" + }, + true + ], + [ + "$vne - 1.2.3-dev is not 1.2.3-de", + { + "version": { + "$vne": "1.2.3-de" + } + }, + { + "version": "1.2.3-dev" + }, + true + ], + [ + "$veq - v inside a dotted pre-release is kept", + { + "version": { + "$veq": "1.0.0-alpha.v1" + } + }, + { + "version": "1.0.0-alpha.v1" + }, + true + ], + [ + "$veq - empty middle segment", + { + "version": { + "$veq": "1..2" + } + }, + { + "version": "1..2" + }, + true + ], + [ + "$vne - empty segment changes the part count", + { + "version": { + "$vne": "1.2" + } + }, + { + "version": "1..2" + }, + true + ], + [ + "$veq - leading separator", + { + "version": { + "$veq": "-1.2.3" + } + }, + { + "version": "-1.2.3" + }, + true + ], + [ + "$vne - non-ASCII digit is not the ASCII digit", + { + "version": { + "$vne": "1.2.3-3" + } + }, + { + "version": "1.2.3-\u0663" + }, + true + ], + [ + "$veq - non-ASCII digits compare as a tag", + { + "version": { + "$veq": "\u0661.\u0662.\u0663" + } + }, + { + "version": "\u0661.\u0662.\u0663" + }, + true ] ], "hash": [ @@ -14094,5 +14388,1467 @@ "ramped" ] ] - ] + ], + "savedGroupReferencesV2": { + "evalCondition": [ + [ + "list entry matches when the attribute is in the list", + { + "$savedGroup": { "id": "grp_list" } + }, + { + "id": "u_1" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "list entry does not match when the attribute is absent from the list", + { + "$savedGroup": { "id": "grp_list" } + }, + { + "id": "u_9" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "condition entry matches when its condition passes", + { + "$savedGroup": { "id": "grp_cond" } + }, + { + "plan": "pro" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "condition entry does not match when its condition fails", + { + "$savedGroup": { "id": "grp_cond" } + }, + { + "plan": "free" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "follows a chain of condition groups", + { + "$savedGroup": { "id": "grp_a" } + }, + { + "country": "US" + }, + true, + { + "grp_a": { + "type": "condition", + "condition": { + "$savedGroup": { "id": "grp_b" } + } + }, + "grp_b": { + "type": "condition", + "condition": { + "$savedGroup": { "id": "grp_c" } + } + }, + "grp_c": { + "type": "condition", + "condition": { + "country": "US" + } + } + } + ], + [ + "follows a chain and fails at the end", + { + "$savedGroup": { "id": "grp_a" } + }, + { + "country": "CA" + }, + false, + { + "grp_a": { + "type": "condition", + "condition": { + "$savedGroup": { "id": "grp_b" } + } + }, + "grp_b": { + "type": "condition", + "condition": { + "$savedGroup": { "id": "grp_c" } + } + }, + "grp_c": { + "type": "condition", + "condition": { + "country": "US" + } + } + } + ], + [ + "resolves inside $not", + { + "$not": { + "$savedGroup": { "id": "grp_list" } + } + }, + { + "id": "u_9" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "resolves inside $and", + { + "$and": [ + { + "$savedGroup": { "id": "grp_list" } + }, + { + "$savedGroup": { "id": "grp_cond" } + } + ] + }, + { + "id": "u_1", + "plan": "pro" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "resolves inside $or", + { + "$or": [ + { + "$savedGroup": { "id": "grp_list" } + }, + { + "$savedGroup": { "id": "grp_cond" } + } + ] + }, + { + "id": "u_9", + "plan": "pro" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "match all: passes only when in every group", + { + "$and": [ + { + "$savedGroup": { "id": "grp_list" } + }, + { + "$savedGroup": { "id": "grp_cond" } + } + ] + }, + { + "id": "u_1", + "plan": "free" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "match any: passes when in one group", + { + "$or": [ + { + "$savedGroup": { "id": "grp_list" } + }, + { + "$savedGroup": { "id": "grp_cond" } + } + ] + }, + { + "id": "u_9", + "plan": "free" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "match none: an AND of NOTs passes when in neither group", + { + "$and": [ + { + "$not": { + "$savedGroup": { "id": "grp_list" } + } + }, + { + "$not": { + "$savedGroup": { "id": "grp_cond" } + } + } + ] + }, + { + "id": "u_9", + "plan": "free" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "match none: an AND of NOTs fails when in one group", + { + "$and": [ + { + "$not": { + "$savedGroup": { "id": "grp_list" } + } + }, + { + "$not": { + "$savedGroup": { "id": "grp_cond" } + } + } + ] + }, + { + "id": "u_1", + "plan": "free" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "a negated pair passes when in only one group, where an AND of NOTs fails", + { + "$not": { + "$and": [ + { + "$savedGroup": { "id": "grp_list" } + }, + { + "$savedGroup": { "id": "grp_cond" } + } + ] + } + }, + { + "id": "u_1", + "plan": "free" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "combines with plain attribute targeting", + { + "$and": [ + { + "country": "US" + }, + { + "$savedGroup": { "id": "grp_list" } + } + ] + }, + { + "country": "US", + "id": "u_1" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "combines with plain attribute targeting and fails on the attribute", + { + "$and": [ + { + "country": "US" + }, + { + "$savedGroup": { "id": "grp_list" } + } + ] + }, + { + "country": "CA", + "id": "u_1" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "list entry intersects when the attribute is an array", + { + "$savedGroup": { "id": "grp_list" } + }, + { + "id": ["u_9", "u_2"] + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "list entry matches numeric values", + { + "$savedGroup": { "id": "grp_num" } + }, + { + "age": 21 + }, + true, + { + "grp_num": { + "type": "list", + "attributeKey": "age", + "values": [21, 30] + } + } + ], + [ + "a cycle resolves to false instead of hanging", + { + "$savedGroup": { "id": "grp_x" } + }, + { + "id": "u_1" + }, + false, + { + "grp_x": { + "type": "condition", + "condition": { + "$savedGroup": { "id": "grp_y" } + } + }, + "grp_y": { + "type": "condition", + "condition": { + "$savedGroup": { "id": "grp_x" } + } + } + } + ], + [ + "an unknown group type matches nobody", + { + "$savedGroup": { "id": "grp_future" } + }, + { + "id": "u_1" + }, + false, + { + "grp_future": { + "type": "somethingNew", + "values": ["u_1"] + } + } + ], + [ + "an id that is not in the payload matches nobody", + { + "$savedGroup": { "id": "grp_missing" } + }, + { + "id": "u_1" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "a non-string operator value matches nobody", + { + "$savedGroup": ["grp_list"] + }, + { + "id": "u_1" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "a list entry with no values matches nobody", + { + "$savedGroup": { "id": "grp_bad" } + }, + { + "id": "u_1" + }, + false, + { + "grp_bad": { + "type": "list", + "attributeKey": "id" + } + } + ], + [ + "a condition entry with no condition matches nobody", + { + "$savedGroup": { "id": "grp_bad" } + }, + { + "id": "u_1" + }, + false, + { + "grp_bad": { + "type": "condition" + } + } + ], + [ + "$savedGroup cannot resolve a legacy bare array", + { + "$savedGroup": { "id": "grp_legacy" } + }, + { + "id": "u_1" + }, + false, + { + "grp_legacy": ["u_1"], + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "$inGroup still works beside typed entries", + { + "id": { + "$inGroup": "grp_legacy" + } + }, + { + "id": "u_1" + }, + true, + { + "grp_legacy": ["u_1"], + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "$inGroup resolves a typed list entry", + { + "id": { + "$inGroup": "grp_list" + } + }, + { + "id": "u_1" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "$inGroup uses the attribute it sits on, not the one in the entry", + { + "backup_id": { + "$inGroup": "grp_list" + } + }, + { + "id": "u_9", + "backup_id": "u_1" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "$inGroup matches nobody for a typed condition entry", + { + "id": { + "$inGroup": "grp_cond" + } + }, + { + "id": "u_1", + "plan": "pro" + }, + false, + { + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "$notInGroup resolves a typed list entry and fails for a member", + { + "id": { + "$notInGroup": "grp_list" + } + }, + { + "id": "u_1" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "$notInGroup resolves a typed list entry and passes for a non-member", + { + "id": { + "$notInGroup": "grp_list" + } + }, + { + "id": "u_9" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "$notInGroup fails closed for a typed condition entry rather than passing everyone", + { + "id": { + "$notInGroup": "grp_cond" + } + }, + { + "id": "u_1", + "plan": "free" + }, + false, + { + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "$notInGroup fails closed for a group type added after the SDK was built", + { + "id": { + "$notInGroup": "grp_future" + } + }, + { + "id": "u_1" + }, + false, + { + "grp_future": { + "type": "somethingNew", + "values": ["u_1"] + } + } + ], + [ + "$notInGroup fails closed for a malformed entry rather than passing everyone", + { + "id": { + "$notInGroup": "grp_bad" + } + }, + { + "id": "u_1" + }, + false, + { + "grp_bad": null + } + ], + [ + "$inGroup matches nobody for a malformed entry", + { + "id": { + "$inGroup": "grp_bad" + } + }, + { + "id": "u_1" + }, + false, + { + "grp_bad": null + } + ], + [ + "$notInGroup still passes for an id that is not in the payload", + { + "id": { + "$notInGroup": "grp_missing" + } + }, + { + "id": "u_1" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "the authoring operator $savedGroups is not understood", + { + "$savedGroups": ["grp_list"] + }, + { + "id": "u_1" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "an attributeKey override checks that attribute instead of the entry's", + { + "$savedGroup": { + "id": "grp_list", + "attributeKey": "backup_id" + } + }, + { + "id": "u_9", + "backup_id": "u_1" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "an attributeKey override does not fall back to the entry's attribute", + { + "$savedGroup": { + "id": "grp_list", + "attributeKey": "backup_id" + } + }, + { + "id": "u_1", + "backup_id": "u_9" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "an attributeKey override resolves inside $not", + { + "$not": { + "$savedGroup": { + "id": "grp_list", + "attributeKey": "backup_id" + } + } + }, + { + "id": "u_1", + "backup_id": "u_9" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "a non-string attributeKey matches nobody rather than using the entry's", + { + "$savedGroup": { + "id": "grp_list", + "attributeKey": 42 + } + }, + { + "id": "u_1" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "an attributeKey override is ignored for a condition entry", + { + "$savedGroup": { + "id": "grp_cond", + "attributeKey": "backup_id" + } + }, + { + "plan": "pro" + }, + true, + { + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + ], + [ + "an unknown key inside the reference is ignored", + { + "$savedGroup": { + "id": "grp_list", + "somethingNew": "ignore me" + } + }, + { + "id": "u_1" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "a bare string id is not a reference and matches nobody", + { + "$savedGroup": "grp_list" + }, + { + "id": "u_1" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "a reference with no id matches nobody", + { + "$savedGroup": { + "attributeKey": "id" + } + }, + { + "id": "u_1" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "an error marker matches nobody", + { + "__sgInvalid__": "grp_cond" + }, + { + "id": "u_1" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "a negated error marker passes everyone, as an unresolved exclusion did before", + { + "$not": { + "__sgUnknown__": "grp_missing" + } + }, + { + "id": "u_1" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "an error marker fails the $and it sits in", + { + "$and": [ + { + "country": "US" + }, + { + "__sgInvalid__": "grp_cond" + } + ] + }, + { + "id": "u_1", + "country": "US" + }, + false, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ], + [ + "an error marker does not fail an $or whose other branch passes", + { + "$or": [ + { + "country": "US" + }, + { + "__sgInvalid__": "grp_cond" + } + ] + }, + { + "id": "u_1", + "country": "US" + }, + true, + { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + } + } + ] + ], + "feature": [ + [ + "a rule condition using a saved group reference", + { + "attributes": { + "id": "u_1", + "plan": "pro" + }, + "savedGroups": { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + }, + "features": { + "flag": { + "defaultValue": false, + "rules": [ + { + "condition": { + "$savedGroup": { "id": "grp_cond" } + }, + "force": true, + "id": "rule_1" + } + ] + } + } + }, + "flag", + { + "value": true, + "on": true, + "off": false, + "source": "force", + "ruleId": "rule_1" + } + ], + [ + "a rule condition using a saved group reference that does not match", + { + "attributes": { + "id": "u_1", + "plan": "free" + }, + "savedGroups": { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + }, + "features": { + "flag": { + "defaultValue": false, + "rules": [ + { + "condition": { + "$savedGroup": { "id": "grp_cond" } + }, + "force": true, + "id": "rule_1" + } + ] + } + } + }, + "flag", + { + "value": false, + "on": false, + "off": true, + "source": "defaultValue", + "ruleId": "" + } + ], + [ + "a prerequisite gate that itself uses a saved group reference", + { + "attributes": { + "id": "u_1" + }, + "savedGroups": { + "grp_parent_on": { + "type": "condition", + "condition": { + "value": true + } + } + }, + "features": { + "parent": { + "defaultValue": true + }, + "child": { + "defaultValue": false, + "rules": [ + { + "parentConditions": [ + { + "id": "parent", + "condition": { + "$savedGroup": { "id": "grp_parent_on" } + } + } + ], + "force": true, + "id": "child_rule" + } + ] + } + } + }, + "child", + { + "value": true, + "on": true, + "off": false, + "source": "force", + "ruleId": "child_rule" + } + ], + [ + "a prerequisite gate whose saved group reference does not match", + { + "attributes": { + "id": "u_1" + }, + "savedGroups": { + "grp_parent_on": { + "type": "condition", + "condition": { + "value": true + } + } + }, + "features": { + "parent": { + "defaultValue": false + }, + "child": { + "defaultValue": false, + "rules": [ + { + "parentConditions": [ + { + "id": "parent", + "condition": { + "$savedGroup": { "id": "grp_parent_on" } + } + } + ], + "force": true, + "id": "child_rule" + } + ] + } + } + }, + "child", + { + "value": false, + "on": false, + "off": true, + "source": "defaultValue", + "ruleId": "" + } + ] + ], + "run": [ + [ + "an experiment targeted by a saved group reference", + { + "attributes": { + "id": "u_1", + "plan": "pro" + }, + "savedGroups": { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + }, + { + "key": "sg-exp", + "variations": [0, 1], + "condition": { + "$savedGroup": { "id": "grp_cond" } + } + }, + 0, + true, + true + ], + [ + "an experiment skipped because the saved group does not match", + { + "attributes": { + "id": "u_1", + "plan": "free" + }, + "savedGroups": { + "grp_list": { + "type": "list", + "attributeKey": "id", + "values": ["u_1", "u_2"] + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + } + }, + { + "key": "sg-exp", + "variations": [0, 1], + "condition": { + "$savedGroup": { "id": "grp_cond" } + } + }, + 0, + false, + false + ], + [ + "an experiment gated by a prerequisite that uses a saved group reference", + { + "attributes": { + "id": "u_1", + "plan": "pro" + }, + "savedGroups": { + "grp_parent_on": { + "type": "condition", + "condition": { + "value": true + } + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + }, + "features": { + "parent": { + "defaultValue": true + } + } + }, + { + "key": "sg-prereq-exp", + "variations": [0, 1], + "parentConditions": [ + { + "id": "parent", + "condition": { + "$savedGroup": { "id": "grp_parent_on" } + } + } + ] + }, + 0, + true, + true + ], + [ + "an experiment blocked because its prerequisite saved group does not match", + { + "attributes": { + "id": "u_1", + "plan": "pro" + }, + "savedGroups": { + "grp_parent_on": { + "type": "condition", + "condition": { + "value": true + } + }, + "grp_cond": { + "type": "condition", + "condition": { + "plan": "pro" + } + } + }, + "features": { + "parent": { + "defaultValue": false + } + } + }, + { + "key": "sg-prereq-exp", + "variations": [0, 1], + "parentConditions": [ + { + "id": "parent", + "condition": { + "$savedGroup": { "id": "grp_parent_on" } + } + } + ] + }, + 0, + false, + false + ] + ] + } } diff --git a/tests/scripts/README.md b/tests/scripts/README.md index db25089..eb7c1c5 100644 --- a/tests/scripts/README.md +++ b/tests/scripts/README.md @@ -9,6 +9,16 @@ Also in this directory (documented in their own docstrings): `GrowthBookClient` with sync/async sticky bucket services. Reports throughput, latency percentiles, and event-loop lag. Run with `PYTHONPATH=. python3 tests/scripts/benchmark_async_client.py`. +- `benchmark_saved_groups.py` — sync/async condition and saved-group benchmarks. + Run `python3 tests/scripts/benchmark_saved_groups.py`; use `--help` for options. + Compare checkouts using the same Python environment: + + ```bash + python tests/scripts/benchmark_saved_groups.py --sdk-path /path/to/base --legacy-only + python tests/scripts/benchmark_saved_groups.py --sdk-path /path/to/change --legacy-only + ``` + + Run sequentially, alternate checkout order, and repeat. Keep results in the PR description. - `check_corpus_freshness.py` — cases.json drift check vs the JS SDK (runs as its own CI job). diff --git a/tests/scripts/benchmark_saved_groups.py b/tests/scripts/benchmark_saved_groups.py new file mode 100644 index 0000000..d58d03c --- /dev/null +++ b/tests/scripts/benchmark_saved_groups.py @@ -0,0 +1,132 @@ +"""Measure condition evaluation through both public clients, without network I/O. + +Compare the same workloads on two checkouts using the same interpreter: + + python tests/scripts/benchmark_saved_groups.py --sdk-path /path/to/base --legacy-only + python tests/scripts/benchmark_saved_groups.py --sdk-path /path/to/change --legacy-only + python tests/scripts/benchmark_saved_groups.py # also measure v2 payloads + +Run comparisons sequentially and alternate checkout order to reduce interference +and thermal bias. JSON output includes the imported SDK path and every sample. +Payload loading, correctness checks, and cleanup are outside the timed region. +This measures CPU cost per evaluation; benchmark_async_client.py measures +concurrent requests with simulated sticky-bucket I/O and event-loop lag. +""" + +import argparse +import asyncio +import json +import platform +import statistics +import sys +import time +from copy import deepcopy +from pathlib import Path + + +def scenarios(): + """Return equivalent legacy/v2 conditions plus ordinary evaluation controls.""" + attributes = {"id": "u999", "plan": "pro", "country": "US", "tags": ["u998", "u999"]} + plain = {"plan": "pro", "country": "US"} + nested = {"$and": [plain, {"$not": {"plan": "free"}}, {"$or": [{"country": "CA"}, plain]}]} + cases = [ + ("default", None, {}, False), + ("experiment", None, {}, False), + ("plain-condition", plain, {}, False), + ("nested-condition", nested, {}, False), + ] + for size in (10, 1000): + values = [f"u{i}" for i in range(1000 - size, 1000)] + cases.extend([ + (f"legacy-list-{size}", {"id": {"$inGroup": "g"}}, {"g": values}, False), + (f"v2-list-{size}", {"$savedGroup": {"id": "g"}}, { + "g": {"type": "list", "attributeKey": "id", "values": values}, + }, True), + ]) + values = [f"u{i}" for i in range(1000)] + cases.extend([ + ("legacy-array-1000", {"tags": {"$inGroup": "g"}}, {"g": values}, False), + ("legacy-not-in-group", {"id": {"$notInGroup": "g"}}, {"g": ["other"]}, False), + ("v2-condition", {"$savedGroup": {"id": "g"}}, { + "g": {"type": "condition", "condition": plain}, + }, True), + ("v2-nested-condition", {"$savedGroup": {"id": "g"}}, { + "g": {"type": "condition", "condition": nested}, + }, True), + ]) + for name, condition, groups, v2 in cases: + feature = {"defaultValue": True} + if name == "experiment": + feature = {"defaultValue": False, "rules": [{"key": "bench", "variations": [True, True], "coverage": 1}]} + elif condition is not None: + feature = {"defaultValue": False, "rules": [{"condition": condition, "force": True}]} + yield name, attributes, {"features": {"flag": feature}, "savedGroups": groups}, v2 + + +async def benchmark(args): + """Load the selected checkout before importing either client.""" + sdk_path = Path(args.sdk_path).resolve() + sys.path.insert(0, str(sdk_path)) + import growthbook + from growthbook import GrowthBook, GrowthBookClient, Options, UserContext + + assert Path(growthbook.__file__).resolve().parent == sdk_path / "growthbook" + for name, attributes, payload, v2 in scenarios(): + if args.legacy_only and v2: + continue + sync_client = GrowthBook(attributes=deepcopy(attributes)) + async_client = GrowthBookClient(Options()) + user = UserContext(attributes=deepcopy(attributes)) + try: + sync_client.set_payload(payload) + await async_client.set_payload(payload) + for client_name in ("sync", "async"): + expected_source = "defaultValue" if name == "default" else "experiment" if name == "experiment" else "force" + if client_name == "sync": + result = sync_client.eval_feature("flag") + else: + result = await async_client.eval_feature("flag", user) + assert result.value is True and result.source == expected_source, (name, client_name, result.to_dict()) + + samples = [] + for round_index in range(args.rounds + 1): + count = min(args.iterations, 1000) if round_index == 0 else args.iterations + start = time.perf_counter_ns() + if client_name == "sync": + for _ in range(count): + sync_client.eval_feature("flag") + else: + for _ in range(count): + await async_client.eval_feature("flag", user) + elapsed = time.perf_counter_ns() - start + if round_index: + samples.append(elapsed / count / 1000) + print(json.dumps({ + "scenario": name, + "client": client_name, + "sdk_path": str(sdk_path), + "python": platform.python_version(), + "iterations": args.iterations, + "median_us": statistics.median(samples), + "min_us": min(samples), + "samples_us": samples, + }), flush=True) + finally: + sync_client.destroy() + await async_client.close() + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--sdk-path", default=str(Path(__file__).resolve().parents[2])) + parser.add_argument("--legacy-only", action="store_true") + parser.add_argument("--iterations", type=int, default=20000) + parser.add_argument("--rounds", type=int, default=7) + args = parser.parse_args() + if args.iterations < 1 or args.rounds < 1: + parser.error("--iterations and --rounds must be positive") + asyncio.run(benchmark(args)) + + +if __name__ == "__main__": + main() diff --git a/tests/scripts/check_corpus_freshness.py b/tests/scripts/check_corpus_freshness.py index ca6838f..75146f2 100644 --- a/tests/scripts/check_corpus_freshness.py +++ b/tests/scripts/check_corpus_freshness.py @@ -55,7 +55,7 @@ "packages/sdk-js/test/cases.json" ) -# Top-level keys to diff. Other keys in cases.json (specVersion, decrypt +# Suite paths to diff. Other keys in cases.json (specVersion, decrypt # binary blobs, urlRedirect which Python doesn't yet wire) are skipped # either because they're scalar metadata or because the divergence is # tracked separately. @@ -71,6 +71,9 @@ "inNamespace", "getEqualWeights", "stickyBucket", + "savedGroupReferencesV2.evalCondition", + "savedGroupReferencesV2.feature", + "savedGroupReferencesV2.run", # Python-local extension today (reported as extras); once the JS SDK # adopts the section, drift checking picks it up automatically. "trackingCalls", @@ -191,10 +194,8 @@ def _diff( drift_skip = skip.get("drift", {}) for key in KEYS_TO_DIFF: - js_list = js_cases.get(key, []) - py_list = py_cases.get(key, []) - if not isinstance(js_list, list) or not isinstance(py_list, list): - continue + js_list = _section_cases(js_cases, key) + py_list = _section_cases(py_cases, key) js_grouped = _case_signatures_grouped(js_list) py_grouped = _case_signatures_grouped(py_list) @@ -238,6 +239,16 @@ def _diff( return actionable_missing, skipped_missing, extras, actionable_drift, skipped_drift +def _section_cases(corpus: dict, key: str) -> list: + """Read a suite, including capability suites nested under a top-level key.""" + section = corpus + for part in key.split("."): + if not isinstance(section, dict): + return [] + section = section.get(part, []) + return section if isinstance(section, list) else [] + + def _spec_versions(js_cases: dict, py_cases: dict) -> Tuple[str, str]: return ( str(js_cases.get("specVersion", "")), diff --git a/tests/test_corpus_freshness.py b/tests/test_corpus_freshness.py new file mode 100644 index 0000000..cc2ba3b --- /dev/null +++ b/tests/test_corpus_freshness.py @@ -0,0 +1,37 @@ +"""Capability sub-suites must participate in the corpus drift check.""" + +import runpy +from pathlib import Path + +import pytest + + +diff_corpora = runpy.run_path( + str(Path(__file__).parent / "scripts" / "check_corpus_freshness.py") +)["_diff"] + + +@pytest.mark.parametrize("suite", ["evalCondition", "feature", "run"]) +def test_nested_suites_report_missing_drift_and_local_cases(suite): + upstream = {"savedGroupReferencesV2": {suite: [["missing", True], ["changed", True]]}} + local = {"savedGroupReferencesV2": {suite: [["changed", False], ["local", True]]}} + key = "savedGroupReferencesV2." + suite + missing, _, extras, drift, _ = diff_corpora(upstream, local, {}) + assert missing[key] == ["missing"] + assert drift[key] == ["changed"] + assert extras[key] == ["local"] + + missing, skipped, _, drift, skipped_drift = diff_corpora(upstream, local, { + "missing": {key: {"missing"}}, "drift": {key: {"changed"}}, + }) + assert missing[key] == drift[key] == [] + assert skipped[key] == ["missing"] + assert skipped_drift[key] == ["changed"] + + +@pytest.mark.parametrize("capability", [None, {}, [], {"evalCondition": {}}]) +def test_missing_or_malformed_capability_suite_cannot_hide_missing_cases(capability): + upstream = {"savedGroupReferencesV2": {"evalCondition": [["new", True]]}} + local = {"savedGroupReferencesV2": capability} + missing, _, _, _, _ = diff_corpora(upstream, local, {}) + assert missing["savedGroupReferencesV2.evalCondition"] == ["new"] diff --git a/tests/test_saved_groups.py b/tests/test_saved_groups.py new file mode 100644 index 0000000..f7d0aa1 --- /dev/null +++ b/tests/test_saved_groups.py @@ -0,0 +1,444 @@ +"""Saved group references v2 conformance and Python-specific regressions.""" + +import asyncio +import json +import os +import sys +from base64 import b64decode, b64encode +from copy import deepcopy +from pathlib import Path + +import pytest +from cryptography.hazmat.primitives import padding +from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes +from unittest.mock import AsyncMock + +from growthbook import Experiment, FeatureRepository, GrowthBook, GrowthBookClient, Options, UserContext, feature_repo +from growthbook.core import evalCondition +from growthbook.growthbook_client import EnhancedFeatureRepository + + +CASES = json.loads(Path(__file__).with_name("cases.json").read_text())[ + "savedGroupReferencesV2" +] + + +@pytest.mark.parametrize("case", CASES["evalCondition"], ids=lambda case: case[0]) +def test_condition_conformance(case): + _, condition, attributes, expected, saved_groups = case + assert evalCondition(attributes, condition, saved_groups) is expected + + +@pytest.mark.parametrize("case", CASES["feature"], ids=lambda case: case[0]) +def test_sync_feature_conformance(case): + _, context, key, expected = deepcopy(case) + client = GrowthBook(**context) + try: + assert client.eval_feature(key).to_dict() == expected + finally: + client.destroy() + + +@pytest.mark.parametrize("case", CASES["run"], ids=lambda case: case[0]) +def test_sync_experiment_conformance(case): + _, context, experiment, value, in_experiment, hash_used = deepcopy(case) + client = GrowthBook(**context) + try: + result = client.run(Experiment(**experiment)) + assert (result.value, result.inExperiment, result.hashUsed) == ( + value, in_experiment, hash_used + ) + finally: + client.destroy() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("case", CASES["feature"], ids=lambda case: case[0]) +async def test_async_feature_conformance(case): + _, context, key, expected = deepcopy(case) + client = GrowthBookClient(Options()) + try: + await client.set_payload(context) + result = await client.eval_feature(key, UserContext(attributes=context["attributes"])) + assert result.to_dict() == expected + finally: + await client.close() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("case", CASES["run"], ids=lambda case: case[0]) +async def test_async_experiment_conformance(case): + _, context, experiment, value, in_experiment, hash_used = deepcopy(case) + client = GrowthBookClient(Options()) + try: + await client.set_payload(context) + result = await client.run(Experiment(**experiment), UserContext(attributes=context["attributes"])) + assert (result.value, result.inExperiment, result.hashUsed) == ( + value, in_experiment, hash_used + ) + finally: + await client.close() + + +@pytest.mark.parametrize("entry", [ + {"type": "list", "attributeKey": "id", "values": ["u1"]}, + {"type": "condition", "condition": {"id": "u1"}}, +]) +@pytest.mark.parametrize("reference", [ + None, False, 7, "g", [], ["g"], {}, + {"id": None}, {"id": 7}, {"id": []}, {"id": {}}, + *({"id": "g", "attributeKey": key} for key in [None, 7, False, [], {}]), +]) +def test_invalid_references_fail_closed(reference, entry): + assert evalCondition({"id": "u1"}, {"$savedGroup": reference}, {"g": entry}) is False + + +@pytest.mark.parametrize("entry", [ + None, False, 7, "g", {}, {"type": "future"}, {"type": []}, + {"type": "list", "attributeKey": "id"}, + *({"type": "list", "attributeKey": "id", "values": values} for values in [None, 7, {}, "u1"]), + {"type": "condition"}, + *({"type": "condition", "condition": condition} for condition in [ + None, 7, [], "g", {"$and": 7}, {"$not": None}, {"id": {"": True}}, + ]), +]) +def test_malformed_entries_fail_closed(entry): + groups = {"bad": entry, "good": {"type": "condition", "condition": {}}} + attributes = {"id": "u1"} + for condition in [ + {"$savedGroup": {"id": "bad"}}, + {"id": {"$inGroup": "bad"}}, + {"id": {"$notInGroup": "bad"}}, + ]: + assert evalCondition(attributes, condition, groups) is False + assert evalCondition(attributes, {"$savedGroup": {"id": "good"}}, groups) is True + assert evalCondition(attributes, {"id": {"$notInGroup": "missing"}}, groups) is True + + +@pytest.mark.asyncio +@pytest.mark.parametrize("invalid", [{"$or": 5}, {"$not": None}, {"id": {"": True}}]) +async def test_condition_errors_fail_closed_under_negation(invalid): + groups = {"bad": {"type": "condition", "condition": invalid}} + sync_client = GrowthBook(attributes={"id": "u1"}) + async_client = GrowthBookClient(Options()) + try: + for target in (invalid, {"$savedGroup": {"id": "bad"}}): + condition = {"$not": target} + payload = {"savedGroups": groups, "features": {"flag": { + "defaultValue": False, "rules": [{"condition": condition, "force": True}], + }}} + sync_client.set_payload(payload) + await async_client.set_payload(payload) + assert not evalCondition({"id": "u1"}, condition, groups) + assert not sync_client.is_on("flag") + assert not await async_client.is_on("flag", UserContext(attributes={"id": "u1"})) + finally: + sync_client.destroy() + await async_client.close() + + +@pytest.mark.parametrize("as_array", [False, True]) +@pytest.mark.parametrize("attribute, values, expected", [ + (True, [1], False), (1, [True], False), + (False, [0], False), (0, [False], False), + (True, [1.0], False), (1.0, [True], False), + (True, [True], True), (False, [False], True), + (1, [1.0], True), (0, [0.0], True), + (True, [1, True], True), (1, [True, 1], True), + (False, [0, False], True), (0, [False, 0], True), +]) +def test_membership_distinguishes_booleans_from_numbers(as_array, attribute, values, expected): + attributes = {"id": [attribute] if as_array else attribute} + for operator in ("$in", "$ini", "$nin", "$nini"): + result = not expected if operator in ("$nin", "$nini") else expected + assert evalCondition(attributes, {"id": {operator: values}}) is result + for entry in (values, {"type": "list", "attributeKey": "id", "values": values}): + groups = {"g": entry} + assert evalCondition(attributes, {"id": {"$inGroup": "g"}}, groups) is expected + assert evalCondition(attributes, {"id": {"$notInGroup": "g"}}, groups) is (not expected) + if isinstance(entry, dict): + assert evalCondition(attributes, {"$savedGroup": {"id": "g"}}, groups) is expected + + +@pytest.mark.parametrize("groups", [None, {}]) +def test_legacy_operators_without_saved_groups(groups): + assert evalCondition({"id": "u1"}, {"id": {"$inGroup": "missing"}}, groups) is False + assert evalCondition({"id": "u1"}, {"id": {"$notInGroup": "missing"}}, groups) is True + + +@pytest.mark.parametrize("entry_key", [None, 7, "id"]) +def test_override_and_legacy_operators_supply_the_attribute(entry_key): + entry = {"type": "list", "attributeKey": entry_key, "values": ["u1"]} + groups = {"g": entry} + attributes = {"profile": {"id": "u1"}, "id": "u2"} + assert evalCondition(attributes, {"$savedGroup": {"id": "g", "attributeKey": "profile.id"}}, groups) + assert evalCondition(attributes, {"profile.id": {"$inGroup": "g"}}, groups) + assert not evalCondition(attributes, {"$savedGroup": {"id": "g"}}, groups) + + +def test_overrides_and_visited_groups_do_not_leak_to_siblings(): + groups = { + "a": {"type": "condition", "condition": {"$savedGroup": {"id": "b"}}}, + "b": {"type": "list", "attributeKey": "id", "values": ["u1"]}, + } + original = deepcopy(groups) + attributes = {"id": "u1", "backup": "u2"} + reference = {"$savedGroup": {"id": "a", "future": {"key": True}}} + assert evalCondition(attributes, {"$and": [reference, reference]}, groups) + assert evalCondition(attributes, {"$or": [{"$and": [reference, {"missing": True}]}, reference]}, groups) + assert evalCondition(attributes, {"$and": [ + {"$not": {"$savedGroup": {"id": "b", "attributeKey": "backup"}}}, + {"$savedGroup": {"id": "a", "attributeKey": "backup"}}, + ]}, groups) + assert groups == original + + +@pytest.mark.parametrize("condition, expected", [ + ({"$savedGroup": {"id": "g"}}, False), + ({"$and": [{"$savedGroup": {"id": "g"}}]}, False), + ({"$or": [{"$savedGroup": {"id": "g"}}]}, False), + ({"$nor": [{"$savedGroup": {"id": "g"}}]}, True), + ({"$not": {"$savedGroup": {"id": "g"}}}, True), + ({"$not": {"$not": {"$savedGroup": {"id": "g"}}}}, False), +]) +def test_cycles_preserve_boolean_composition(condition, expected): + groups = {"g": {"type": "condition", "condition": condition}} + assert evalCondition({}, {"$savedGroup": {"id": "g"}}, groups) is expected + + +@pytest.mark.parametrize("operator", ["$elemMatch", "$all", "$alli", "$not"]) +def test_visited_groups_survive_nested_value_evaluation(operator): + nested = {"$elemMatch": {"marker": True, "$savedGroup": {"id": "g"}}} + items = [{"marker": True, "terminal": True}] + if operator in ("$all", "$alli"): + nested = {operator: [nested]} + items = [items] + elif operator == "$not": + nested = {"$not": {"$not": nested}} + groups = {"g": {"type": "condition", "condition": {"$or": [ + {"terminal": True}, {"items": nested}, + ]}}} + # Losing the visited set would let the nested element's terminal branch pass. + assert not evalCondition({"items": items}, {"$savedGroup": {"id": "g"}}, groups) + + +def test_long_acyclic_chain_has_no_sdk_depth_limit(): + groups = {str(i): {"type": "condition", "condition": {"$savedGroup": {"id": str(i + 1)}}} + for i in range(150)} + groups["150"] = {"type": "condition", "condition": {"id": "u1"}} + assert evalCondition({"id": "u1"}, {"$savedGroup": {"id": "0"}}, groups) + + +def test_cycle_longer_than_python_stack_fails_closed(): + count = sys.getrecursionlimit() + groups = {str(i): {"type": "condition", "condition": {"$savedGroup": {"id": str((i + 1) % count)}}} + for i in range(count)} + assert not evalCondition({}, {"$savedGroup": {"id": "0"}}, groups) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("condition", [ + {"$savedGroup": {"id": "0"}}, + {"$not": {"$savedGroup": {"id": "0"}}}, + {"$and": [{"$savedGroup": {"id": "0"}}]}, + {"$or": [{"$savedGroup": {"id": "0"}}, {}]}, + {"$nor": [{"$savedGroup": {"id": "0"}}]}, + {"items": {"$elemMatch": {"marker": True, "$savedGroup": {"id": "0"}}}}, + {"items": {"$not": {"$elemMatch": {"marker": True, "$savedGroup": {"id": "0"}}}}}, +], ids=["reference", "not", "and", "or", "nor", "elemMatch", "value-not"]) +async def test_recursion_overflow_rejects_the_entire_rule(condition): + count = sys.getrecursionlimit() + 50 + groups = {str(i): {"type": "condition", "condition": {"$savedGroup": {"id": str(i + 1)}}} + for i in range(count)} + groups[str(count)] = {"type": "condition", "condition": {}} + attributes = {"id": "u1", "items": [{"marker": True}]} + payload = {"savedGroups": groups, "features": {"flag": { + "defaultValue": True, "rules": [{"condition": condition, "force": True}, {"force": False}], + }}} + experiment = Experiment(key="deep", variations=[0, 1], condition=condition) + sync_client = GrowthBook(attributes=dict(attributes)) + async_client = GrowthBookClient(Options()) + try: + sync_client.set_payload(payload) + await async_client.set_payload(payload) + user = UserContext(attributes=attributes) + assert evalCondition(attributes, condition, groups) is False + assert sync_client.is_on("flag") is False + assert await async_client.is_on("flag", user) is False + assert sync_client.run(experiment).inExperiment is False + assert (await async_client.run(experiment, user)).inExperiment is False + assert evalCondition(attributes, {"$or": [{}, condition]}, groups) is True + finally: + sync_client.destroy() + await async_client.close() + + +@pytest.mark.parametrize("marker", ["__sgInvalid__", "__sgUnknown__", "__sgCycle__", "__sgMaxDepth__"]) +def test_error_markers_keep_boolean_semantics(marker): + condition = {marker: "g"} + assert not evalCondition({}, condition) + assert evalCondition({}, {"$not": condition}) + assert not evalCondition({}, {"$and": [{}, condition]}) + assert evalCondition({}, {"$or": [condition, {}]}) + + +def test_saved_group_is_only_a_top_level_operator(): + groups = {"g": {"type": "condition", "condition": {}}} + assert not evalCondition({"id": "u1"}, {"id": {"$savedGroup": {"id": "g"}}}, groups) + assert not evalCondition({}, {"$savedGroups": None}, groups) + + +def test_array_membership_handles_unhashable_attributes(): + attributes = {"id": [{"nested": True}, ["u2"], "u1"]} + groups = {"g": {"type": "list", "attributeKey": "id", "values": ["u1"]}} + assert evalCondition(attributes, {"id": {"$in": ["u1"]}}) + assert evalCondition(attributes, {"id": {"$inGroup": "g"}}, groups) + assert not evalCondition(attributes, {"id": {"$notInGroup": "g"}}, groups) + assert evalCondition(attributes, {"$savedGroup": {"id": "g"}}, groups) + + +@pytest.mark.parametrize("value", [{"id": "u1"}, ["u1"]]) +@pytest.mark.parametrize("primitive_match", [False, True]) +def test_container_membership_does_not_use_structural_equality(value, primitive_match): + attributes = {"id": [deepcopy(value), "u1"]} + values = [deepcopy(value), "u1" if primitive_match else "u2"] + for entry in [values, {"type": "list", "attributeKey": "id", "values": values}]: + groups = {"g": entry} + for operator, expected in [("$in", primitive_match), ("$nin", not primitive_match)]: + assert evalCondition(attributes, {"id": {operator: values}}) is expected + assert evalCondition(attributes, {"id": {"$inGroup": "g"}}, groups) is primitive_match + assert evalCondition(attributes, {"id": {"$notInGroup": "g"}}, groups) is (not primitive_match) + if isinstance(entry, dict): + assert evalCondition(attributes, {"$savedGroup": {"id": "g"}}, groups) is primitive_match + + + +@pytest.mark.asyncio +@pytest.mark.parametrize("attributes, expected", [ + ({"id": "u1", "plan": "pro"}, True), + ({"id": "u2", "plan": "pro"}, False), + ({"id": "u1", "plan": "free"}, False), +]) +async def test_switching_payload_formats_preserves_results(attributes, expected): + legacy = { + "savedGroups": {"beta": ["u1"]}, + "features": {"flag": {"defaultValue": False, "rules": [{ + "condition": {"id": {"$inGroup": "beta"}, "plan": "pro"}, "force": True, + }]}}, + } + typed = { + "savedGroups": { + "beta": {"type": "list", "attributeKey": "id", "values": ["u1"]}, + "power": {"type": "condition", "condition": {"plan": "pro"}}, + }, + "features": {"flag": {"defaultValue": False, "rules": [{ + "condition": {"$and": [{"$savedGroup": {"id": "beta"}}, {"$savedGroup": {"id": "power"}}]}, + "force": True, + }]}}, + } + sync_client = GrowthBook(attributes=attributes) + async_client = GrowthBookClient(Options()) + user = UserContext(attributes=attributes) + try: + for payload in [legacy, typed, legacy]: + sync_client.set_payload(deepcopy(payload)) + await async_client.set_payload(deepcopy(payload)) + assert sync_client.is_on("flag") is expected + assert await async_client.is_on("flag", user) is expected + finally: + sync_client.destroy() + await async_client.close() + + +def _encrypt_section(value, key): + """Encode a test payload using the SDK's AES-CBC wire format.""" + iv = os.urandom(16) + padder = padding.PKCS7(128).padder() + padded = padder.update(json.dumps(value).encode()) + padder.finalize() + encryptor = Cipher(algorithms.AES(b64decode(key)), modes.CBC(iv)).encryptor() + ciphertext = encryptor.update(padded) + encryptor.finalize() + return b64encode(iv).decode() + "." + b64encode(ciphertext).decode() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("encrypted", [False, True]) +@pytest.mark.parametrize("transport", ["set_payload", "fetch"]) +async def test_both_clients_load_and_refresh_typed_groups(mocker, encrypted, transport): + mocker.patch.object(EnhancedFeatureRepository, "_instances", {}) + key = b64encode(b"0123456789abcdef").decode() + payload = { + "features": { + "flag": {"defaultValue": False, "rules": [{ + "condition": {"$savedGroup": {"id": "eligible"}}, "force": True, + }]}, + "unsafe": {"defaultValue": False, "rules": [{ + "condition": {"id": {"$notInGroup": "bad"}}, "force": True, + }]}, + }, + "savedGroups": { + "members": {"type": "list", "attributeKey": "id", "values": ["u1"]}, + "eligible": {"type": "condition", "condition": { + "$and": [{"$savedGroup": {"id": "members"}}, {"plan": "pro"}], + }}, + "bad": None, + }, + } + wire = {} + sync_fetch = mocker.patch.object( + FeatureRepository, "_fetch_and_decode", side_effect=lambda *args: deepcopy(wire), + ) + async_fetch = mocker.patch.object( + FeatureRepository, "_fetch_and_decode_async", new_callable=AsyncMock, + side_effect=lambda *args: deepcopy(wire), + ) + mocker.patch.object(EnhancedFeatureRepository, "start_feature_refresh", new_callable=AsyncMock) + sync_client = GrowthBook(client_key="sdk-saved-sync" if transport == "fetch" else "", decryption_key=key) + async_client = GrowthBookClient(Options(client_key="sdk-saved-async" if transport == "fetch" else "", decryption_key=key)) + users = [{"id": "u1", "plan": "pro"}, {"id": "u2", "plan": "pro"}, {"id": "u1", "plan": "free"}] + experiment = Experiment(key="saved-group", variations=[0, 1], condition={"$savedGroup": {"id": "eligible"}}) + + async def check_user(attributes, member): + user = UserContext(attributes=attributes) + expected = attributes["id"] == member and attributes["plan"] == "pro" + assert await async_client.is_on("flag", user) is expected + assert await async_client.is_on("unsafe", user) is False + assert (await async_client.run(experiment, user)).inExperiment is expected + + feature_repo.clear_cache() + try: + for index, member in enumerate(("u1", "u2")): + payload["savedGroups"]["members"]["values"] = [member] + wire = ({ + "encryptedFeatures": _encrypt_section(payload["features"], key), + "encryptedSavedGroups": _encrypt_section(payload["savedGroups"], key), + } if encrypted else deepcopy(payload)) + if transport == "set_payload": + sync_client.set_payload(deepcopy(wire)) + await async_client.set_payload(deepcopy(wire)) + else: + sync_client.load_features(force_refresh=True) + if index == 0: + assert await async_client.initialize() + else: + repo = async_client._features_repository + updated = await repo.load_features_async( + "https://cdn.growthbook.io", "sdk-saved-async", key, force_refresh=True, + ) + await repo._handle_feature_update(updated) + for attributes in users: + expected = attributes["id"] == member and attributes["plan"] == "pro" + sync_client.set_attributes(dict(attributes)) + assert sync_client.is_on("flag") is expected + assert sync_client.is_on("unsafe") is False + assert sync_client.run(experiment).inExperiment is expected + await asyncio.gather(*(check_user(dict(attributes), member) for _ in range(10) for attributes in users)) + if transport == "fetch": + assert sync_fetch.call_count == 2 + assert async_fetch.await_count == 2 + else: + sync_fetch.assert_not_called() + async_fetch.assert_not_called() + finally: + sync_client.destroy() + await async_client.close() + feature_repo.clear_cache()