From 9caabdf5625e97da08f5d334bc58bf4d1901662b Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Fri, 24 Oct 2025 16:25:08 +0100 Subject: [PATCH] feat: Remove `FeatureContext.feature_key`, `SegmentResult.key` --- .gitmodules | 2 +- flag_engine/context/types.py | 1 - flag_engine/result/types.py | 2 - flag_engine/segments/evaluator.py | 14 +++--- tests/engine_tests/engine-test-data | 2 +- tests/unit/conftest.py | 13 ++---- .../unit/segments/test_segments_evaluator.py | 6 --- tests/unit/test_engine.py | 43 +++---------------- 8 files changed, 18 insertions(+), 65 deletions(-) diff --git a/.gitmodules b/.gitmodules index fa58745..7aa2113 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 = v2.5.0 + tag = v3.0.0 diff --git a/flag_engine/context/types.py b/flag_engine/context/types.py index a6df2cf..d5e6824 100644 --- a/flag_engine/context/types.py +++ b/flag_engine/context/types.py @@ -57,7 +57,6 @@ class SegmentRule(TypedDict): class FeatureContext(TypedDict, Generic[FeatureMetadataT]): key: str - feature_key: str name: str enabled: bool value: Any diff --git a/flag_engine/result/types.py b/flag_engine/result/types.py index 21509e0..4c9d663 100644 --- a/flag_engine/result/types.py +++ b/flag_engine/result/types.py @@ -12,7 +12,6 @@ class FlagResult(TypedDict, Generic[FeatureMetadataT]): - feature_key: str name: str enabled: bool value: Any @@ -21,7 +20,6 @@ class FlagResult(TypedDict, Generic[FeatureMetadataT]): class SegmentResult(TypedDict, Generic[SegmentMetadataT]): - key: str name: str metadata: NotRequired[SegmentMetadataT] diff --git a/flag_engine/segments/evaluator.py b/flag_engine/segments/evaluator.py index 2788338..aee805b 100644 --- a/flag_engine/segments/evaluator.py +++ b/flag_engine/segments/evaluator.py @@ -63,7 +63,6 @@ def get_evaluation_result( continue segment_result: SegmentResult[SegmentMetadataT] = { - "key": segment_context["key"], "name": segment_context["name"], } if segment_metadata := segment_context.get("metadata"): @@ -72,19 +71,19 @@ def get_evaluation_result( if overrides := segment_context.get("overrides"): for override_feature_context in overrides: - feature_key = override_feature_context["feature_key"] + feature_name = override_feature_context["name"] if ( - feature_key not in segment_feature_contexts + feature_name not in segment_feature_contexts or override_feature_context.get( "priority", constants.DEFAULT_PRIORITY, ) - < (segment_feature_contexts[feature_key]["feature_context"]).get( + < (segment_feature_contexts[feature_name]["feature_context"]).get( "priority", constants.DEFAULT_PRIORITY, ) ): - segment_feature_contexts[feature_key] = ( + segment_feature_contexts[feature_name] = ( FeatureContextWithSegmentName( feature_context=override_feature_context, segment_name=segment_context["name"], @@ -99,13 +98,12 @@ def get_evaluation_result( for feature_context in (context.get("features") or {}).values(): feature_name = feature_context["name"] if feature_context_with_segment_name := segment_feature_contexts.get( - feature_context["feature_key"], + feature_context["name"], ): feature_context = feature_context_with_segment_name["feature_context"] flag_result: FlagResult[FeatureMetadataT] flags[feature_name] = flag_result = { "enabled": feature_context["enabled"], - "feature_key": feature_context["feature_key"], "name": feature_context["name"], "reason": f"TARGETING_MATCH; segment={feature_context_with_segment_name['segment_name']}", "value": feature_context.get("value"), @@ -153,7 +151,6 @@ def get_flag_result_from_feature_context( if start_percentage <= percentage_value < limit: flag_result = { "enabled": feature_context["enabled"], - "feature_key": feature_context["feature_key"], "name": feature_context["name"], "reason": f"SPLIT; weight={weight}", "value": variant["value"], @@ -165,7 +162,6 @@ def get_flag_result_from_feature_context( if flag_result is None: flag_result = { "enabled": feature_context["enabled"], - "feature_key": feature_context["feature_key"], "name": feature_context["name"], "reason": "DEFAULT", "value": feature_context["value"], diff --git a/tests/engine_tests/engine-test-data b/tests/engine_tests/engine-test-data index 41c2021..8d19e96 160000 --- a/tests/engine_tests/engine-test-data +++ b/tests/engine_tests/engine-test-data @@ -1 +1 @@ -Subproject commit 41c202145e375c712600e318c439456de5b221d7 +Subproject commit 8d19e9627013c0e0213c29f3318fd45a179868fa diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 0c8fd9a..e6f2d03 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -53,16 +53,12 @@ def segment(segment_rule: SegmentRule) -> SegmentContext: @pytest.fixture() def feature_state_1() -> FeatureContext: - return FeatureContext( - feature_key="1", key="1", name="feature_1", value=None, enabled=True - ) + return FeatureContext(key="1", name="feature_1", value=None, enabled=True) @pytest.fixture() def feature_state_2() -> FeatureContext: - return FeatureContext( - feature_key="2", key="2", name="feature_2", value=None, enabled=False - ) + return FeatureContext(key="2", name="feature_2", value=None, enabled=False) @pytest.fixture() @@ -89,8 +85,8 @@ def context( return { "environment": environment, "features": { - feature_state_1["feature_key"]: feature_state_1, - feature_state_2["feature_key"]: feature_state_2, + feature_state_1["name"]: feature_state_1, + feature_state_2["name"]: feature_state_2, }, "segments": {segment["key"]: segment}, "identity": identity, @@ -126,7 +122,6 @@ def context_in_segment( "overrides": [ { "key": "4", - "feature_key": "1", "name": "feature_1", "enabled": False, "value": "segment_override", diff --git a/tests/unit/segments/test_segments_evaluator.py b/tests/unit/segments/test_segments_evaluator.py index 8a95a5f..521f545 100644 --- a/tests/unit/segments/test_segments_evaluator.py +++ b/tests/unit/segments/test_segments_evaluator.py @@ -803,7 +803,6 @@ def test_segment_condition_matches_context_value_for_modulo( 10, { "enabled": False, - "feature_key": "1", "name": "my_feature", "reason": "SPLIT; weight=30", "value": "foo", @@ -813,7 +812,6 @@ def test_segment_condition_matches_context_value_for_modulo( 40, { "enabled": False, - "feature_key": "1", "name": "my_feature", "reason": "SPLIT; weight=30", "value": "bar", @@ -823,7 +821,6 @@ def test_segment_condition_matches_context_value_for_modulo( 70, { "enabled": False, - "feature_key": "1", "name": "my_feature", "reason": "DEFAULT", "value": "control", @@ -850,7 +847,6 @@ def test_get_flag_result_from_feature_context__calls_returns_expected( # and have a feature context with some multivariate feature options and associated values feature_context: FeatureContext = { "key": expected_feature_context_key, - "feature_key": "1", "enabled": False, "name": "my_feature", "value": "control", @@ -892,7 +888,6 @@ def test_get_flag_result_from_feature_context__null_key__calls_returns_expected( feature_context: FeatureContext = { "key": expected_feature_context_key, - "feature_key": "1", "enabled": False, "name": "my_feature", "value": "control", @@ -912,7 +907,6 @@ def test_get_flag_result_from_feature_context__null_key__calls_returns_expected( # the value of the feature state is the default one assert result == { "enabled": False, - "feature_key": "1", "name": "my_feature", "reason": "DEFAULT", "value": "control", diff --git a/tests/unit/test_engine.py b/tests/unit/test_engine.py index 54b26df..be0aae4 100644 --- a/tests/unit/test_engine.py +++ b/tests/unit/test_engine.py @@ -23,14 +23,12 @@ def test_get_evaluation_result__no_overrides__returns_expected( flags={ "feature_1": { "enabled": True, - "feature_key": "1", "name": "feature_1", "reason": "DEFAULT", "value": None, }, "feature_2": { "enabled": False, - "feature_key": "2", "name": "feature_2", "reason": "DEFAULT", "value": None, @@ -51,20 +49,18 @@ def test_get_evaluation_result__segment_override__returns_expected( "flags": { "feature_1": { "enabled": False, - "feature_key": "1", "name": "feature_1", "reason": "TARGETING_MATCH; segment=my_segment", "value": "segment_override", }, "feature_2": { "enabled": False, - "feature_key": "2", "name": "feature_2", "reason": "DEFAULT", "value": None, }, }, - "segments": [{"key": "1", "name": "my_segment"}], + "segments": [{"name": "my_segment"}], } @@ -91,7 +87,6 @@ def test_get_evaluation_result__identity_override__returns_expected( overrides=[ { "key": "5", - "feature_key": "1", "name": "feature_1", "enabled": True, "value": "overridden_for_identity", @@ -108,25 +103,18 @@ def test_get_evaluation_result__identity_override__returns_expected( "flags": { "feature_1": { "enabled": True, - "feature_key": "1", "name": "feature_1", "reason": "TARGETING_MATCH; segment=identity_overrides", "value": "overridden_for_identity", }, "feature_2": { "enabled": False, - "feature_key": "2", "name": "feature_2", "reason": "DEFAULT", "value": None, }, }, - "segments": [ - { - "key": "", - "name": "identity_overrides", - }, - ], + "segments": [{"name": "identity_overrides"}], } @@ -144,14 +132,12 @@ def test_get_evaluation_result__two_segments_override_same_feature__returns_expe "features": { "feature_1": { "key": "1", - "feature_key": "1", "name": "feature_1", "enabled": False, "value": None, }, "feature_2": { "key": "2", - "feature_key": "2", "name": "feature_2", "enabled": False, "value": None, @@ -173,7 +159,6 @@ def test_get_evaluation_result__two_segments_override_same_feature__returns_expe "overrides": [ { "key": "4", - "feature_key": "1", "name": "feature_1", "enabled": False, "value": "segment_override", @@ -197,7 +182,6 @@ def test_get_evaluation_result__two_segments_override_same_feature__returns_expe { "enabled": True, "key": "2", - "feature_key": "1", "name": "feature_1", "value": "segment_override_other", "priority": 1, @@ -215,22 +199,20 @@ def test_get_evaluation_result__two_segments_override_same_feature__returns_expe "flags": { "feature_1": { "enabled": True, - "feature_key": "1", "name": "feature_1", "reason": "TARGETING_MATCH; segment=higher_priority_segment", "value": "segment_override_other", }, "feature_2": { "enabled": False, - "feature_key": "2", "name": "feature_2", "reason": "DEFAULT", "value": None, }, }, "segments": [ - {"key": "1", "name": "my_segment"}, - {"key": "3", "name": "higher_priority_segment"}, + {"name": "my_segment"}, + {"name": "higher_priority_segment"}, ], } @@ -249,14 +231,12 @@ def test_get_evaluation_result__segment_override__no_priority__returns_expected( "features": { "feature_1": { "key": "1", - "feature_key": "1", "name": "feature_1", "enabled": False, "value": None, }, "feature_2": { "key": "2", - "feature_key": "2", "name": "feature_2", "enabled": False, "value": None, @@ -278,7 +258,6 @@ def test_get_evaluation_result__segment_override__no_priority__returns_expected( "overrides": [ { "key": "3", - "feature_key": "1", "name": "feature_1", "enabled": True, "value": "overridden_without_priority", @@ -300,7 +279,6 @@ def test_get_evaluation_result__segment_override__no_priority__returns_expected( "overrides": [ { "key": "4", - "feature_key": "1", "name": "feature_1", "enabled": True, "value": "overridden_with_priority", @@ -327,7 +305,6 @@ def test_get_evaluation_result__segment_override__no_priority__returns_expected( "overrides": [ { "key": "5", - "feature_key": "2", "name": "feature_2", "enabled": False, "value": "moose", @@ -345,23 +322,21 @@ def test_get_evaluation_result__segment_override__no_priority__returns_expected( "flags": { "feature_1": { "enabled": True, - "feature_key": "1", "name": "feature_1", "reason": "TARGETING_MATCH; segment=segment_with_override_priority", "value": "overridden_with_priority", }, "feature_2": { "enabled": False, - "feature_key": "2", "name": "feature_2", "reason": "TARGETING_MATCH; segment=another_segment", "value": "moose", }, }, "segments": [ - {"key": "1", "name": "segment_without_override_priority"}, - {"key": "2", "name": "segment_with_override_priority"}, - {"key": "3", "name": "another_segment"}, + {"name": "segment_without_override_priority"}, + {"name": "segment_with_override_priority"}, + {"name": "another_segment"}, ], } @@ -456,7 +431,6 @@ class CustomFeatureMetadata(TypedDict): "features": { "feature_1": { "key": "1", - "feature_key": "1", "name": "feature_1", "enabled": False, "value": None, @@ -483,7 +457,6 @@ class CustomFeatureMetadata(TypedDict): "overrides": [ { "key": "5", - "feature_key": "1", "name": "feature_1", "enabled": True, "value": "overridden_for_identity", @@ -512,7 +485,6 @@ def test_feature_metadata_generic_type__default__returns_expected() -> None: "features": { "feature_1": { "key": "1", - "feature_key": "1", "name": "feature_1", "enabled": False, "value": None, @@ -539,7 +511,6 @@ def test_feature_metadata_generic_type__default__returns_expected() -> None: "overrides": [ { "key": "5", - "feature_key": "1", "name": "feature_1", "enabled": True, "value": "overridden_for_identity",