Skip to content

Implement "last feature flag wins" behavior for duplicate feature flag IDs #59

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
+209 −10
Open

Implement "last feature flag wins" behavior for duplicate feature flag IDs #59

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6c44174
Initial plan for issue
Copilot Jun 17, 2025
b00f288
Implement last feature flag wins behavior for duplicate IDs
Copilot Jun 17, 2025
b964759
Optimize _get_feature_flag to iterate backwards for better performance
Copilot Jun 17, 2025
e9e84e1
Optimize performance by caching reversed feature flags and simplify u…
Copilot Jun 17, 2025
71c3183
fix reverse search
mrm9084 Jun 19, 2025
1021dad
Update featuremanagement/_featuremanagerbase.py
mrm9084 Jun 19, 2025
ab22c6e
Update _featuremanagerbase.py
mrm9084 Jun 19, 2025
61ff753
fix is
mrm9084 Jun 19, 2025
4ec52ad
Replace dict.fromkeys() with set for simpler unique flag name dedupli…
Copilot Jun 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions featuremanagement/_featuremanagerbase.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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.
If multiple feature flags have the same id, the last one wins.

:param Mapping configuration: Configuration object.
:param str feature_flag_name: Name of the feature flag.
Expand All @@ -39,33 +40,33 @@ def _get_feature_flag(configuration: Mapping[str, Any], feature_flag_name: str)
feature_flags = feature_management.get(FEATURE_FLAG_KEY)
if not feature_flags or not isinstance(feature_flags, list):
return None

for feature_flag in feature_flags:
if feature_flag.get("id") == feature_flag_name:
return FeatureFlag.convert_from_json(feature_flag)

index = len(feature_flags) - 1

while index >= 0:
if feature_flags[index].get("id") == feature_flag_name:
return FeatureFlag.convert_from_json(feature_flags[index])
index -= 1
Copy link

Copilot AI Jun 19, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider replacing the manual index-based loop with Python’s reversed for better readability. For example:

for feature_flag in reversed(feature_flags):
    if feature_flag.get("id") == feature_flag_name:
        return FeatureFlag.convert_from_json(feature_flag)
Suggested change
index = len(feature_flags) - 1
while index >= 0:
if feature_flags[index].get("id") == feature_flag_name:
return FeatureFlag.convert_from_json(feature_flags[index])
index -= 1
for feature_flag in reversed(feature_flags):
if feature_flag.get("id") == feature_flag_name:
return FeatureFlag.convert_from_json(feature_flag)

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@mrm9084 mrm9084 Jun 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure if it maters given the data sizes of these. But going through backwards with the index will always be faster than calling reversed every time, then looking backwards. Thoughts?

@copilot


return None


def _list_feature_flag_names(configuration: Mapping[str, Any]) -> List[str]:
"""
List of all feature flag names.
List of feature flag names, with duplicates removed.

:param Mapping configuration: Configuration object.
:return: List of feature flag names.
"""
feature_flag_names = []
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_names.append(feature_flag.get("id"))

return feature_flag_names
flag_ids = [feature_flag.get("id") for feature_flag in feature_flags]
return list(dict.fromkeys(flag_ids))


class FeatureManagerBase(ABC):
Expand Down
126 changes: 126 additions & 0 deletions tests/test_feature_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,132 @@ def fake_telemetry_callback(self, evaluation_event):
assert evaluation_event
self.called_telemetry = True

# method: duplicate_feature_flag_handling
def test_duplicate_feature_flags_last_wins(self):
"""Test that when multiple feature flags have the same ID, the last one wins."""
feature_flags = {
"feature_management": {
"feature_flags": [
{
"id": "DuplicateFlag",
"description": "First",
"enabled": "true",
"conditions": {"client_filters": []},
},
{
"id": "DuplicateFlag",
"description": "Second",
"enabled": "false",
"conditions": {"client_filters": []},
},
{
"id": "DuplicateFlag",
"description": "Third",
"enabled": "true",
"conditions": {"client_filters": []},
},
]
}
}
feature_manager = FeatureManager(feature_flags)

# The last flag should win (enabled: true)
assert feature_manager.is_enabled("DuplicateFlag") == True

# Should only list unique names
flag_names = feature_manager.list_feature_flag_names()
assert "DuplicateFlag" in flag_names
# Count how many times DuplicateFlag appears in the list
duplicate_count = flag_names.count("DuplicateFlag")
assert duplicate_count == 1, f"Expected DuplicateFlag to appear once, but appeared {duplicate_count} times"

def test_duplicate_feature_flags_last_wins_disabled(self):
"""Test that when multiple feature flags have the same ID, the last one wins even if disabled."""
feature_flags = {
"feature_management": {
"feature_flags": [
{
"id": "DuplicateFlag",
"description": "First",
"enabled": "true",
"conditions": {"client_filters": []},
},
{
"id": "DuplicateFlag",
"description": "Second",
"enabled": "true",
"conditions": {"client_filters": []},
},
{
"id": "DuplicateFlag",
"description": "Third",
"enabled": "false",
"conditions": {"client_filters": []},
},
]
}
}
feature_manager = FeatureManager(feature_flags)

# The last flag should win (enabled: false)
assert feature_manager.is_enabled("DuplicateFlag") == False

def test_duplicate_feature_flags_mixed_with_unique(self):
"""Test behavior with a mix of duplicate and unique feature flags."""
feature_flags = {
"feature_management": {
"feature_flags": [
{
"id": "UniqueFlag1",
"description": "First unique",
"enabled": "true",
"conditions": {"client_filters": []},
},
{
"id": "DuplicateFlag",
"description": "First duplicate",
"enabled": "false",
"conditions": {"client_filters": []},
},
{
"id": "UniqueFlag2",
"description": "Second unique",
"enabled": "false",
"conditions": {"client_filters": []},
},
{
"id": "DuplicateFlag",
"description": "Second duplicate",
"enabled": "true",
"conditions": {"client_filters": []},
},
{
"id": "UniqueFlag3",
"description": "Third unique",
"enabled": "true",
"conditions": {"client_filters": []},
},
]
}
}
feature_manager = FeatureManager(feature_flags)

# Test unique flags work as expected
assert feature_manager.is_enabled("UniqueFlag1") == True
assert feature_manager.is_enabled("UniqueFlag2") == False
assert feature_manager.is_enabled("UniqueFlag3") == True

# Test duplicate flag - last should win (enabled: true)
assert feature_manager.is_enabled("DuplicateFlag") == True

# Test list includes all unique names
flag_names = feature_manager.list_feature_flag_names()
expected_names = ["UniqueFlag1", "DuplicateFlag", "UniqueFlag2", "UniqueFlag3"]
assert set(flag_names) == set(expected_names)
# Ensure each name appears only once
for name in expected_names:
assert flag_names.count(name) == 1


class AlwaysOn(FeatureFilter):
def evaluate(self, context, **kwargs):
Expand Down
72 changes: 72 additions & 0 deletions tests/test_feature_manager_async.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,78 @@ async def fake_telemetry_callback_async(self, evaluation_event):
assert evaluation_event
self.called_telemetry = True

# method: duplicate_feature_flag_handling
@pytest.mark.asyncio
async def test_duplicate_feature_flags_last_wins_async(self):
"""Test that when multiple feature flags have the same ID, the last one wins."""
feature_flags = {
"feature_management": {
"feature_flags": [
{
"id": "DuplicateFlag",
"description": "First",
"enabled": "true",
"conditions": {"client_filters": []},
},
{
"id": "DuplicateFlag",
"description": "Second",
"enabled": "false",
"conditions": {"client_filters": []},
},
{
"id": "DuplicateFlag",
"description": "Third",
"enabled": "true",
"conditions": {"client_filters": []},
},
]
}
}
feature_manager = FeatureManager(feature_flags)

# The last flag should win (enabled: true)
assert await feature_manager.is_enabled("DuplicateFlag") == True

# Should only list unique names
flag_names = feature_manager.list_feature_flag_names()
assert "DuplicateFlag" in flag_names
# Count how many times DuplicateFlag appears in the list
duplicate_count = flag_names.count("DuplicateFlag")
assert duplicate_count == 1, f"Expected DuplicateFlag to appear once, but appeared {duplicate_count} times"

@pytest.mark.asyncio
async def test_duplicate_feature_flags_last_wins_disabled_async(self):
"""Test that when multiple feature flags have the same ID, the last one wins even if disabled."""
feature_flags = {
"feature_management": {
"feature_flags": [
{
"id": "DuplicateFlag",
"description": "First",
"enabled": "true",
"conditions": {"client_filters": []},
},
{
"id": "DuplicateFlag",
"description": "Second",
"enabled": "true",
"conditions": {"client_filters": []},
},
{
"id": "DuplicateFlag",
"description": "Third",
"enabled": "false",
"conditions": {"client_filters": []},
},
]
}
}
feature_manager = FeatureManager(feature_flags)

# The last flag should win (enabled: false)
assert await feature_manager.is_enabled("DuplicateFlag") == False


class AlwaysOn(FeatureFilter):
async def evaluate(self, context, **kwargs):
Expand Down
Loading