From f5162d91daa192a33d1ecc8eb30b4121604be7dd Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Mon, 27 Oct 2025 14:40:36 +0000 Subject: [PATCH 1/3] fix: Implicit identity key not supported for % Split operator --- .gitmodules | 2 +- flag_engine/segments/evaluator.py | 117 ++++++++++++------ tests/engine_tests/engine-test-data | 2 +- .../unit/segments/test_segments_evaluator.py | 27 ++-- 4 files changed, 95 insertions(+), 53 deletions(-) diff --git a/.gitmodules b/.gitmodules index 41aa8d9..4a5954e 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "tests/engine_tests/engine-test-data"] path = tests/engine_tests/engine-test-data url = https://github.com/flagsmith/engine-test-data.git - tag = v3.1.0 + branch = feat/implicit-identity-key-for-percentage-split diff --git a/flag_engine/segments/evaluator.py b/flag_engine/segments/evaluator.py index d7b4536..04a0ec7 100644 --- a/flag_engine/segments/evaluator.py +++ b/flag_engine/segments/evaluator.py @@ -36,11 +36,13 @@ from flag_engine.utils.types import SupportsStr, get_casting_function -class FeatureContextWithSegmentName(TypedDict, typing.Generic[FeatureMetadataT]): +class SegmentOverride(TypedDict, typing.Generic[FeatureMetadataT]): feature_context: FeatureContext[FeatureMetadataT] segment_name: str +SegmentOverrides = dict[str, SegmentOverride[FeatureMetadataT]] + # Type alias for EvaluationContext with any metadata types # used in internal evaluation logic _EvaluationContextAnyMeta = EvaluationContext[typing.Any, typing.Any] @@ -55,15 +57,45 @@ def get_evaluation_result( :param context: the evaluation context :return: EvaluationResult containing the context, flags, and segments """ - segments: list[SegmentResult[SegmentMetadataT]] = [] - flags: dict[str, FlagResult[FeatureMetadataT]] = {} + enrich_context(context) + segments, segment_overrides = evaluate_segments(context) + flags = evaluate_features(context, segment_overrides) + + return { + "flags": flags, + "segments": segments, + } + + +def enrich_context( + context: _EvaluationContextAnyMeta, +) -> None: + """ + Enrich the evaluation context in-place by ensuring that: + - `$.identity.key` is set + + :param context: the evaluation context to enrich + """ + if identity_context := context.get("identity"): + if not identity_context.get("key"): + identity_context["key"] = ( + f"{context['environment']['key']}_{identity_context['identifier']}" + ) - segment_feature_contexts: dict[ - SupportsStr, - FeatureContextWithSegmentName[FeatureMetadataT], - ] = {} - for segment_context in (context.get("segments") or {}).values(): +def evaluate_segments( + context: EvaluationContext[SegmentMetadataT, FeatureMetadataT], +) -> typing.Tuple[ + list[SegmentResult[SegmentMetadataT]], + SegmentOverrides[FeatureMetadataT], +]: + if not (segment_contexts := context.get("segments")): + return [], {} + + segment_results: list[SegmentResult[SegmentMetadataT]] = [] + segment_overrides: SegmentOverrides[FeatureMetadataT] = {} + + for segment_context in segment_contexts.values(): if not is_context_in_segment(context, segment_context): continue @@ -72,69 +104,75 @@ def get_evaluation_result( } if segment_metadata := segment_context.get("metadata"): segment_result["metadata"] = segment_metadata - segments.append(segment_result) + segment_results.append(segment_result) if overrides := segment_context.get("overrides"): for override_feature_context in overrides: feature_name = override_feature_context["name"] if ( - feature_name not in segment_feature_contexts + feature_name not in segment_overrides or override_feature_context.get( "priority", constants.DEFAULT_PRIORITY, ) - < (segment_feature_contexts[feature_name]["feature_context"]).get( + < (segment_overrides[feature_name]["feature_context"]).get( "priority", constants.DEFAULT_PRIORITY, ) ): - segment_feature_contexts[feature_name] = ( - FeatureContextWithSegmentName( - feature_context=override_feature_context, - segment_name=segment_context["name"], - ) + segment_overrides[feature_name] = SegmentOverride( + feature_context=override_feature_context, + segment_name=segment_context["name"], ) - identity_key = _get_identity_key(context) + return segment_results, segment_overrides + + +def evaluate_features( + context: EvaluationContext[typing.Any, FeatureMetadataT], + segment_overrides: SegmentOverrides[FeatureMetadataT], +) -> dict[str, FlagResult[FeatureMetadataT]]: + flags: dict[str, FlagResult[FeatureMetadataT]] = {} + for feature_context in (context.get("features") or {}).values(): feature_name = feature_context["name"] - if feature_context_with_segment_name := segment_feature_contexts.get( + if segment_override := segment_overrides.get( feature_context["name"], ): - feature_context = feature_context_with_segment_name["feature_context"] + feature_context = segment_override["feature_context"] flag_result: FlagResult[FeatureMetadataT] flags[feature_name] = flag_result = { "enabled": feature_context["enabled"], "name": feature_context["name"], - "reason": f"TARGETING_MATCH; segment={feature_context_with_segment_name['segment_name']}", + "reason": f"TARGETING_MATCH; segment={segment_override['segment_name']}", "value": feature_context.get("value"), } if feature_metadata := feature_context.get("metadata"): flag_result["metadata"] = feature_metadata continue - flags[feature_name] = get_flag_result_from_feature_context( - feature_context=feature_context, - key=identity_key, + flags[feature_name] = get_flag_result_from_context( + context=context, + feature_name=feature_name, ) - return { - "flags": flags, - "segments": segments, - } + return flags -def get_flag_result_from_feature_context( - feature_context: FeatureContext[FeatureMetadataT], - key: typing.Optional[SupportsStr], +def get_flag_result_from_context( + context: EvaluationContext[typing.Any, FeatureMetadataT], + feature_name: str, ) -> FlagResult[FeatureMetadataT]: """ - Get a feature value from the feature context - for a given key. + Get a feature value from the evaluation context + for a given feature name. - :param feature_context: the feature context - :param key: the key to get the value for - :return: the value for the key in the feature context + :param context: the evaluation context + :param feature_name: the feature name to get the value for + :return: the value for the feature name in the evaluation context """ + feature_context = context["features"][feature_name] + key = _get_identity_key(context) + flag_result: typing.Optional[FlagResult[FeatureMetadataT]] = None if key is not None and (variants := feature_context.get("variants")): @@ -253,8 +291,8 @@ def context_matches_condition( if condition["operator"] == constants.PERCENTAGE_SPLIT: if context_value is not None: object_ids = [segment_key, context_value] - elif identity_context := context.get("identity"): - object_ids = [segment_key, identity_context["key"]] + elif identity_key := _get_identity_key(context): + object_ids = [segment_key, identity_key] else: return False @@ -376,10 +414,7 @@ def _get_identity_key( context: _EvaluationContextAnyMeta, ) -> typing.Optional[SupportsStr]: if identity_context := context.get("identity"): - return ( - identity_context.get("key") - or f"{context['environment']['key']}_{identity_context['identifier']}" - ) + return identity_context.get("key") return None diff --git a/tests/engine_tests/engine-test-data b/tests/engine_tests/engine-test-data index 6ab57ec..49c9b63 160000 --- a/tests/engine_tests/engine-test-data +++ b/tests/engine_tests/engine-test-data @@ -1 +1 @@ -Subproject commit 6ab57ec67bc84659e8b5aa41534b04fe45cc4cbe +Subproject commit 49c9b63acd7feef80638426da6e1b79251ca61aa diff --git a/tests/unit/segments/test_segments_evaluator.py b/tests/unit/segments/test_segments_evaluator.py index 521f545..2f0a1c1 100644 --- a/tests/unit/segments/test_segments_evaluator.py +++ b/tests/unit/segments/test_segments_evaluator.py @@ -17,7 +17,7 @@ _matches_context_value, context_matches_condition, get_context_value, - get_flag_result_from_feature_context, + get_flag_result_from_context, is_context_in_segment, ) from flag_engine.segments.types import ConditionOperator @@ -828,7 +828,8 @@ def test_segment_condition_matches_context_value_for_modulo( ), ), ) -def test_get_flag_result_from_feature_context__calls_returns_expected( +def test_get_flag_result_from_context__calls_returns_expected( + context: EvaluationContext, percentage_value: int, expected_result: FlagResult, mocker: MockerFixture, @@ -855,11 +856,13 @@ def test_get_flag_result_from_feature_context__calls_returns_expected( {"value": "bar", "weight": 30, "priority": 2}, ], } + context["features"]["my_feature"] = feature_context + context["identity"] = {"identifier": expected_key, "key": expected_key} # When - result = get_flag_result_from_feature_context( - feature_context=feature_context, - key=expected_key, + result = get_flag_result_from_context( + context=context, + feature_name="my_feature", ) # the value of the feature state is correct based on the percentage value returned @@ -875,12 +878,11 @@ def test_get_flag_result_from_feature_context__calls_returns_expected( def test_get_flag_result_from_feature_context__null_key__calls_returns_expected( + context: EvaluationContext, mocker: MockerFixture, ) -> None: # Given expected_feature_context_key = "2" - # a None key is provided (no identity context present) - expected_key = None get_hashed_percentage_for_object_ids_mock = mocker.patch( "flag_engine.segments.evaluator.get_hashed_percentage_for_object_ids", @@ -897,10 +899,15 @@ def test_get_flag_result_from_feature_context__null_key__calls_returns_expected( ], } + context["features"]["my_feature"] = feature_context + + # no identity context present + context["identity"] = None + # When - result = get_flag_result_from_feature_context( - feature_context=feature_context, - key=expected_key, + result = get_flag_result_from_context( + context=context, + feature_name="my_feature", ) # Then From 327631b3e326fc97443765168bfd2091c8789daa Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Mon, 27 Oct 2025 14:42:51 +0000 Subject: [PATCH 2/3] bump --- .gitmodules | 2 +- tests/engine_tests/engine-test-data | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.gitmodules b/.gitmodules index 4a5954e..c26ad3a 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "tests/engine_tests/engine-test-data"] path = tests/engine_tests/engine-test-data url = https://github.com/flagsmith/engine-test-data.git - branch = feat/implicit-identity-key-for-percentage-split + tag = v3.2.0 diff --git a/tests/engine_tests/engine-test-data b/tests/engine_tests/engine-test-data index 49c9b63..97ab814 160000 --- a/tests/engine_tests/engine-test-data +++ b/tests/engine_tests/engine-test-data @@ -1 +1 @@ -Subproject commit 49c9b63acd7feef80638426da6e1b79251ca61aa +Subproject commit 97ab814316ecb95896878a0eaa26c9c5d0300361 From d8c68aa4361ccb6964dc41415de676e5f1bd7bb4 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Tue, 28 Oct 2025 11:48:27 +0000 Subject: [PATCH 3/3] avoid mutating input --- flag_engine/segments/evaluator.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/flag_engine/segments/evaluator.py b/flag_engine/segments/evaluator.py index 04a0ec7..8389b70 100644 --- a/flag_engine/segments/evaluator.py +++ b/flag_engine/segments/evaluator.py @@ -57,7 +57,7 @@ def get_evaluation_result( :param context: the evaluation context :return: EvaluationResult containing the context, flags, and segments """ - enrich_context(context) + context = get_enriched_context(context) segments, segment_overrides = evaluate_segments(context) flags = evaluate_features(context, segment_overrides) @@ -67,20 +67,27 @@ def get_evaluation_result( } -def enrich_context( - context: _EvaluationContextAnyMeta, -) -> None: +def get_enriched_context( + context: EvaluationContext[SegmentMetadataT, FeatureMetadataT], +) -> EvaluationContext[SegmentMetadataT, FeatureMetadataT]: """ - Enrich the evaluation context in-place by ensuring that: + Get an enriched version of the evaluation context by ensuring that: - `$.identity.key` is set :param context: the evaluation context to enrich + :return: the enriched evaluation context. If not modified, returns the original context. """ if identity_context := context.get("identity"): if not identity_context.get("key"): - identity_context["key"] = ( - f"{context['environment']['key']}_{identity_context['identifier']}" - ) + context = context.copy() + context["identity"] = { + **identity_context, + "key": ( + f"{context['environment']['key']}_{identity_context['identifier']}" + ), + } + + return context def evaluate_segments(