From 1250ab67dde8c2685d33692046d26f2bb7e5ed72 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Mon, 6 Jul 2026 11:50:58 -0700 Subject: [PATCH 1/5] Last One Wins --- .gitignore | 1 + featuremanagement/_featuremanagerbase.py | 15 +++++++++++---- tests/test_feature_manager.py | 17 +++++++++++++++++ tests/test_feature_manager_async.py | 18 ++++++++++++++++++ 4 files changed, 47 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index cab6cf5..190177c 100644 --- a/.gitignore +++ b/.gitignore @@ -413,3 +413,4 @@ docs/doctrees docs/html package-lock.json package.json +.env diff --git a/featuremanagement/_featuremanagerbase.py b/featuremanagement/_featuremanagerbase.py index fac7f34..f29aac5 100644 --- a/featuremanagement/_featuremanagerbase.py +++ b/featuremanagement/_featuremanagerbase.py @@ -41,11 +41,15 @@ def _get_feature_flag(configuration: Mapping[str, Any], feature_flag_name: str) if not feature_flags or not isinstance(feature_flags, list): return None + matching_feature_flag = None for feature_flag in feature_flags: if feature_flag.get("id") == feature_flag_name: - return FeatureFlag.convert_from_json(feature_flag) + # If multiple feature flags share the same id, the last one defined wins. + matching_feature_flag = feature_flag - return None + if matching_feature_flag is None: + return None + return FeatureFlag.convert_from_json(matching_feature_flag) def _list_feature_flag_names(configuration: Mapping[str, Any]) -> List[str]: @@ -55,7 +59,7 @@ def _list_feature_flag_names(configuration: Mapping[str, Any]) -> List[str]: :param Mapping configuration: Configuration object. :return: List of feature flag names. """ - feature_flag_names = [] + feature_flag_names: List[str] = [] feature_management = configuration.get(FEATURE_MANAGEMENT_KEY) if not feature_management or not isinstance(feature_management, Mapping): return [] @@ -64,7 +68,10 @@ def _list_feature_flag_names(configuration: Mapping[str, Any]) -> List[str]: return [] for feature_flag in feature_flags: - feature_flag_names.append(feature_flag.get("id")) + feature_flag_name = feature_flag.get("id") + # Feature flags with the same id are treated as one, so only list each name once. + if feature_flag_name not in feature_flag_names: + feature_flag_names.append(feature_flag_name) return feature_flag_names diff --git a/tests/test_feature_manager.py b/tests/test_feature_manager.py index b9a0fc6..36fb9ed 100644 --- a/tests/test_feature_manager.py +++ b/tests/test_feature_manager.py @@ -112,6 +112,23 @@ def test_list_feature_flags(self): assert not feature_manager.is_enabled("Beta") assert len(feature_manager.list_feature_flag_names()) == 2 + # method: is_enabled + def test_duplicate_feature_flag_last_wins(self): + feature_flags = { + "feature_management": { + "feature_flags": [ + {"id": "Alpha", "description": "", "enabled": "true", "conditions": {"client_filters": []}}, + {"id": "Alpha", "description": "", "enabled": "false", "conditions": {"client_filters": []}}, + ] + } + } + feature_manager = FeatureManager(feature_flags) + assert feature_manager is not None + # The last feature flag with the same id should win. + assert not feature_manager.is_enabled("Alpha") + # Duplicate ids should only be listed once. + assert feature_manager.list_feature_flag_names() == ["Alpha"] + # method: is_enabled def test_unknown_feature_filter(self): feature_flags = { diff --git a/tests/test_feature_manager_async.py b/tests/test_feature_manager_async.py index d224ba9..7df4154 100644 --- a/tests/test_feature_manager_async.py +++ b/tests/test_feature_manager_async.py @@ -119,6 +119,24 @@ async def test_list_feature_flags(self): assert not await feature_manager.is_enabled("Beta") assert len(feature_manager.list_feature_flag_names()) == 2 + # method: is_enabled + @pytest.mark.asyncio + async def test_duplicate_feature_flag_last_wins(self): + feature_flags = { + "feature_management": { + "feature_flags": [ + {"id": "Alpha", "description": "", "enabled": "true", "conditions": {"client_filters": []}}, + {"id": "Alpha", "description": "", "enabled": "false", "conditions": {"client_filters": []}}, + ] + } + } + feature_manager = FeatureManager(feature_flags) + assert feature_manager is not None + # The last feature flag with the same id should win. + assert not await feature_manager.is_enabled("Alpha") + # Duplicate ids should only be listed once. + assert feature_manager.list_feature_flag_names() == ["Alpha"] + # method: is_enabled @pytest.mark.asyncio async def test_unknown_feature_filter(self): From f463c7a5845dd031b7a2c97cd847a9c2704aaa7a Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Mon, 6 Jul 2026 11:56:26 -0700 Subject: [PATCH 2/5] Update _featuremanagerbase.py --- featuremanagement/_featuremanagerbase.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/featuremanagement/_featuremanagerbase.py b/featuremanagement/_featuremanagerbase.py index f29aac5..d78c7f0 100644 --- a/featuremanagement/_featuremanagerbase.py +++ b/featuremanagement/_featuremanagerbase.py @@ -41,15 +41,12 @@ def _get_feature_flag(configuration: Mapping[str, Any], feature_flag_name: str) if not feature_flags or not isinstance(feature_flags, list): return None - matching_feature_flag = None - for feature_flag in feature_flags: + for feature_flag in reversed(feature_flags): + # If multiple feature flags share the same id, the last one defined wins. if feature_flag.get("id") == feature_flag_name: - # If multiple feature flags share the same id, the last one defined wins. - matching_feature_flag = feature_flag + return FeatureFlag.convert_from_json(feature_flag) - if matching_feature_flag is None: - return None - return FeatureFlag.convert_from_json(matching_feature_flag) + return None def _list_feature_flag_names(configuration: Mapping[str, Any]) -> List[str]: From f2ac7e246cce02fde17e67f49ac9d3c4437db02a Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Mon, 6 Jul 2026 14:31:21 -0700 Subject: [PATCH 3/5] Cleanup plus missing test --- featuremanagement/_featuremanagerbase.py | 89 +++++++++++------------- tests/test_feature_manager.py | 13 ++++ tests/test_feature_manager_async.py | 14 ++++ 3 files changed, 66 insertions(+), 50 deletions(-) diff --git a/featuremanagement/_featuremanagerbase.py b/featuremanagement/_featuremanagerbase.py index d78c7f0..99e15f0 100644 --- a/featuremanagement/_featuremanagerbase.py +++ b/featuremanagement/_featuremanagerbase.py @@ -25,54 +25,6 @@ logger = logging.getLogger(__name__) -def _get_feature_flag(configuration: Mapping[str, Any], feature_flag_name: str) -> Optional[FeatureFlag]: - """ - Gets the FeatureFlag json from the configuration, if it exists it gets converted to a FeatureFlag object. - - :param Mapping configuration: Configuration object. - :param str feature_flag_name: Name of the feature flag. - :return: FeatureFlag - :rtype: FeatureFlag - """ - feature_management = configuration.get(FEATURE_MANAGEMENT_KEY) - if not feature_management or not isinstance(feature_management, Mapping): - return None - feature_flags = feature_management.get(FEATURE_FLAG_KEY) - if not feature_flags or not isinstance(feature_flags, list): - return None - - for feature_flag in reversed(feature_flags): - # If multiple feature flags share the same id, the last one defined wins. - if feature_flag.get("id") == feature_flag_name: - return FeatureFlag.convert_from_json(feature_flag) - - return None - - -def _list_feature_flag_names(configuration: Mapping[str, Any]) -> List[str]: - """ - List of all feature flag names. - - :param Mapping configuration: Configuration object. - :return: List of feature flag names. - """ - feature_flag_names: List[str] = [] - feature_management = configuration.get(FEATURE_MANAGEMENT_KEY) - if not feature_management or not isinstance(feature_management, Mapping): - return [] - feature_flags = feature_management.get(FEATURE_FLAG_KEY) - if not feature_flags or not isinstance(feature_flags, list): - return [] - - for feature_flag in feature_flags: - feature_flag_name = feature_flag.get("id") - # Feature flags with the same id are treated as one, so only list each name once. - if feature_flag_name not in feature_flag_names: - feature_flag_names.append(feature_flag_name) - - return feature_flag_names - - class FeatureManagerBase(ABC): """ Base class for Feature Manager. This class is responsible for all shared logic between the sync and async. @@ -277,7 +229,7 @@ def _check_feature_base(self, feature_flag_id: str) -> Tuple[EvaluationEvent, bo self._copy = self._configuration.get(FEATURE_MANAGEMENT_KEY) if not self._cache.get(feature_flag_id): - feature_flag = _get_feature_flag(self._configuration, feature_flag_id) + feature_flag = self._get_feature_flag(feature_flag_id) self._cache[feature_flag_id] = feature_flag else: feature_flag = self._cache.get(feature_flag_id) @@ -304,4 +256,41 @@ def list_feature_flag_names(self) -> List[str]: """ List of all feature flag names. """ - return _list_feature_flag_names(self._configuration) + feature_flag_names: Dict[str, None] = {} + for feature_flag in self._get_feature_flags(): + feature_flag_name = feature_flag.get("id") + # Only include entries with a valid string id; duplicates are listed once. + if isinstance(feature_flag_name, str): + feature_flag_names[feature_flag_name] = None + + return list(feature_flag_names) + + def _get_feature_flag(self, feature_flag_name: str) -> Optional[FeatureFlag]: + """ + Gets the FeatureFlag json from the configuration, if it exists it gets converted to a FeatureFlag object. + + :param str feature_flag_name: Name of the feature flag. + :return: FeatureFlag + :rtype: FeatureFlag + """ + for feature_flag in reversed(self._get_feature_flags()): + # If multiple feature flags share the same id, the last one defined wins. + if feature_flag.get("id") == feature_flag_name: + return FeatureFlag.convert_from_json(feature_flag) + + return None + + def _get_feature_flags(self) -> List[Any]: + """ + Gets the list of raw feature flag definitions from the configuration. + + :return: List of feature flag definitions, or an empty list if none are configured. + :rtype: list + """ + feature_management = self._configuration.get(FEATURE_MANAGEMENT_KEY) + if not feature_management or not isinstance(feature_management, Mapping): + return [] + feature_flags = feature_management.get(FEATURE_FLAG_KEY) + if not feature_flags or not isinstance(feature_flags, list): + return [] + return feature_flags diff --git a/tests/test_feature_manager.py b/tests/test_feature_manager.py index 36fb9ed..2e93219 100644 --- a/tests/test_feature_manager.py +++ b/tests/test_feature_manager.py @@ -112,6 +112,19 @@ def test_list_feature_flags(self): assert not feature_manager.is_enabled("Beta") assert len(feature_manager.list_feature_flag_names()) == 2 + # method: list_feature_flags + def test_list_feature_flags_no_feature_flags(self): + # feature_management is present but has no valid feature_flags list. + feature_manager = FeatureManager({"feature_management": {"feature_flags": None}}) + assert feature_manager is not None + assert feature_manager.list_feature_flag_names() == [] + assert not feature_manager.is_enabled("Alpha") + + # feature_flags is present but is not a list. + feature_manager = FeatureManager({"feature_management": {"feature_flags": "not-a-list"}}) + assert feature_manager.list_feature_flag_names() == [] + assert not feature_manager.is_enabled("Alpha") + # method: is_enabled def test_duplicate_feature_flag_last_wins(self): feature_flags = { diff --git a/tests/test_feature_manager_async.py b/tests/test_feature_manager_async.py index 7df4154..39639b3 100644 --- a/tests/test_feature_manager_async.py +++ b/tests/test_feature_manager_async.py @@ -119,6 +119,20 @@ async def test_list_feature_flags(self): assert not await feature_manager.is_enabled("Beta") assert len(feature_manager.list_feature_flag_names()) == 2 + # method: list_feature_flags + @pytest.mark.asyncio + async def test_list_feature_flags_no_feature_flags(self): + # feature_management is present but has no valid feature_flags list. + feature_manager = FeatureManager({"feature_management": {"feature_flags": None}}) + assert feature_manager is not None + assert feature_manager.list_feature_flag_names() == [] + assert not await feature_manager.is_enabled("Alpha") + + # feature_flags is present but is not a list. + feature_manager = FeatureManager({"feature_management": {"feature_flags": "not-a-list"}}) + assert feature_manager.list_feature_flag_names() == [] + assert not await feature_manager.is_enabled("Alpha") + # method: is_enabled @pytest.mark.asyncio async def test_duplicate_feature_flag_last_wins(self): From 0845845768ed242e8f69369db7a919a7355b4fdb Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Mon, 6 Jul 2026 14:34:19 -0700 Subject: [PATCH 4/5] Update _featuremanagerbase.py --- featuremanagement/_featuremanagerbase.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/featuremanagement/_featuremanagerbase.py b/featuremanagement/_featuremanagerbase.py index 99e15f0..8ec7ba1 100644 --- a/featuremanagement/_featuremanagerbase.py +++ b/featuremanagement/_featuremanagerbase.py @@ -8,7 +8,7 @@ import hashlib import logging from abc import ABC -from typing import List, Optional, Dict, Tuple, Any, Mapping, Callable +from typing import List, Optional, Dict, Tuple, Any, Mapping, Callable, cast from ._models import FeatureFlag, Variant, VariantAssignmentReason, TargetingContext, EvaluationEvent, VariantReference FEATURE_MANAGEMENT_KEY = "feature_management" @@ -293,4 +293,4 @@ def _get_feature_flags(self) -> List[Any]: feature_flags = feature_management.get(FEATURE_FLAG_KEY) if not feature_flags or not isinstance(feature_flags, list): return [] - return feature_flags + return cast(List[Any], feature_flags) From dcfc0f73b7556b4d0b00d431aef8bd7f49143940 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Mon, 6 Jul 2026 14:46:24 -0700 Subject: [PATCH 5/5] fixing tests --- tests/test_feature_manager.py | 4 ++-- tests/test_feature_manager_async.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_feature_manager.py b/tests/test_feature_manager.py index 2e93219..4b6234a 100644 --- a/tests/test_feature_manager.py +++ b/tests/test_feature_manager.py @@ -117,12 +117,12 @@ def test_list_feature_flags_no_feature_flags(self): # feature_management is present but has no valid feature_flags list. feature_manager = FeatureManager({"feature_management": {"feature_flags": None}}) assert feature_manager is not None - assert feature_manager.list_feature_flag_names() == [] + assert not feature_manager.list_feature_flag_names() assert not feature_manager.is_enabled("Alpha") # feature_flags is present but is not a list. feature_manager = FeatureManager({"feature_management": {"feature_flags": "not-a-list"}}) - assert feature_manager.list_feature_flag_names() == [] + assert not feature_manager.list_feature_flag_names() assert not feature_manager.is_enabled("Alpha") # method: is_enabled diff --git a/tests/test_feature_manager_async.py b/tests/test_feature_manager_async.py index 39639b3..bd2c154 100644 --- a/tests/test_feature_manager_async.py +++ b/tests/test_feature_manager_async.py @@ -125,12 +125,12 @@ async def test_list_feature_flags_no_feature_flags(self): # feature_management is present but has no valid feature_flags list. feature_manager = FeatureManager({"feature_management": {"feature_flags": None}}) assert feature_manager is not None - assert feature_manager.list_feature_flag_names() == [] + assert not feature_manager.list_feature_flag_names() assert not await feature_manager.is_enabled("Alpha") # feature_flags is present but is not a list. feature_manager = FeatureManager({"feature_management": {"feature_flags": "not-a-list"}}) - assert feature_manager.list_feature_flag_names() == [] + assert not feature_manager.list_feature_flag_names() assert not await feature_manager.is_enabled("Alpha") # method: is_enabled