Skip to content

Commit eef68c8

Browse files
nunogoissighphyrepre-commit-ci[bot]
authored
feat: add is_feature_enabled to variant response (Unleash#285)
* feat: add is_feature_enabled to variant response * style: linting * Update UnleashClient/features/Feature.py Co-authored-by: Simon Hornby <liquidwicked64@gmail.com> * Update UnleashClient/features/Feature.py Co-authored-by: Simon Hornby <liquidwicked64@gmail.com> * refactor: rename is_feature_enabled to feature_enabled * refactor: favor dict comprehension over copy * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * test: favor mock bootstrap over mock API * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Simon Hornby <liquidwicked64@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent 2ddb217 commit eef68c8

File tree

7 files changed

+86
-9
lines changed

7 files changed

+86
-9
lines changed

‎UnleashClient/constants.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
"Content-Type": "application/json",
1414
"Unleash-Client-Spec": CLIENT_SPEC_VERSION,
1515
}
16-
DISABLED_VARIATION = {"name": "disabled", "enabled": False}
16+
DISABLED_VARIATION = {"name": "disabled", "enabled": False, "feature_enabled": False}
1717

1818
# Paths
1919
REGISTER_URL = "/client/register"

‎UnleashClient/features/Feature.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
# pylint: disable=invalid-name
2-
import copy
32
from typing import Dict, Optional, cast
43

54
from UnleashClient.constants import DISABLED_VARIATION
@@ -111,14 +110,15 @@ def get_variant(self, context: dict = None, skip_stats: bool = False) -> dict:
111110
variant = (
112111
self.variants.get_variant(context, is_feature_enabled)
113112
if is_feature_enabled
114-
else copy.deepcopy(DISABLED_VARIATION)
113+
else DISABLED_VARIATION
115114
)
116115

117116
except Exception as variant_exception:
118117
LOGGER.warning("Error selecting variant: %s", variant_exception)
119118
if not skip_stats:
120119
self._count_variant(cast(str, variant["name"]))
121-
return variant
120+
121+
return {**variant, "feature_enabled": is_feature_enabled}
122122

123123
def _get_evaluation_result(
124124
self, context: dict = None, skip_stats: bool = False

‎UnleashClient/variants/Variants.py

+2-4
Original file line numberDiff line numberDiff line change
@@ -84,16 +84,14 @@ def get_variant(self, context: dict, flag_status: Optional[bool] = None) -> dict
8484
:param flag_status:
8585
:return:
8686
"""
87-
fallback_variant = copy.deepcopy(DISABLED_VARIATION)
88-
8987
if self.variants:
9088
override_variant = self._apply_overrides(context)
9189
if override_variant:
9290
return self._format_variation(override_variant, flag_status)
9391

9492
total_weight = sum(x["weight"] for x in self.variants)
9593
if total_weight <= 0:
96-
return fallback_variant
94+
return DISABLED_VARIATION
9795

9896
stickiness_selector = (
9997
self.variants[0]["stickiness"]
@@ -115,4 +113,4 @@ def get_variant(self, context: dict, flag_status: Optional[bool] = None) -> dict
115113
return self._format_variation(variation, flag_status)
116114

117115
# Catch all return.
118-
return fallback_variant
116+
return DISABLED_VARIATION

‎tests/specification_tests/test_client_specs.py

+5-1
Original file line numberDiff line numberDiff line change
@@ -92,4 +92,8 @@ def test_spec(spec):
9292
expected = test_data["expectedResult"]
9393
context = test_data.get("context")
9494
variant = unleash_client.get_variant(toggle_name, context)
95-
assert variant == expected
95+
# Remove the feature_enabled key from the variant, as it is not part of the expected result in the spec
96+
variant_without_feature_enabled = {
97+
k: v for k, v in variant.items() if k != "feature_enabled"
98+
}
99+
assert variant_without_feature_enabled == expected

‎tests/unit_tests/test_client.py

+27
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
from tests.utilities.mocks.mock_all_features import MOCK_ALL_FEATURES
1313
from tests.utilities.mocks.mock_features import (
14+
MOCK_FEATURE_ENABLED_NO_VARIANTS_RESPONSE,
1415
MOCK_FEATURE_RESPONSE,
1516
MOCK_FEATURE_RESPONSE_PROJECT,
1617
MOCK_FEATURE_WITH_DEPENDENCIES_RESPONSE,
@@ -399,11 +400,36 @@ def test_uc_get_variant():
399400
variant = unleash_client.get_variant("testVariations", context={"userId": "2"})
400401
assert variant["name"] == "VarA"
401402
assert variant["enabled"]
403+
assert variant["feature_enabled"]
402404

403405
# If feature flag is not.
404406
variant = unleash_client.get_variant("testVariations", context={"userId": "3"})
405407
assert variant["name"] == "disabled"
406408
assert not variant["enabled"]
409+
assert not variant["feature_enabled"]
410+
411+
unleash_client.destroy()
412+
413+
414+
@responses.activate
415+
def test_uc_get_variant_feature_enabled_no_variants():
416+
cache = FileCache("MOCK_CACHE")
417+
cache.bootstrap_from_dict(MOCK_FEATURE_ENABLED_NO_VARIANTS_RESPONSE)
418+
unleash_client = UnleashClient(
419+
url=URL,
420+
app_name=APP_NAME,
421+
disable_metrics=True,
422+
disable_registration=True,
423+
cache=cache,
424+
environment="default",
425+
)
426+
unleash_client.initialize_client(fetch_toggles=False)
427+
428+
# If feature is enabled but has no variants, should return disabled variant with feature_enabled=True
429+
variant = unleash_client.get_variant("EnabledNoVariants")
430+
assert variant["name"] == "disabled"
431+
assert not variant["enabled"]
432+
assert variant["feature_enabled"]
407433

408434
unleash_client.destroy()
409435

@@ -414,6 +440,7 @@ def test_uc_not_initialized_getvariant():
414440
variant = unleash_client.get_variant("ThisFlagDoesn'tExist")
415441
assert not variant["enabled"]
416442
assert variant["name"] == "disabled"
443+
assert not variant["feature_enabled"]
417444

418445

419446
@responses.activate

‎tests/unit_tests/test_features.py

+29
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,17 @@ def test_feature_strategy_variants():
4646
yield Feature("My Feature", True, strategies, variants)
4747

4848

49+
@pytest.fixture()
50+
def test_feature_no_variants():
51+
strategies = [
52+
FlexibleRollout(
53+
BASE_FLEXIBLE_ROLLOUT_DICT["constraints"],
54+
BASE_FLEXIBLE_ROLLOUT_DICT["parameters"],
55+
)
56+
]
57+
yield Feature("My Feature", True, strategies)
58+
59+
4960
@pytest.fixture()
5061
def test_feature_dependencies():
5162
strategies = [Default()]
@@ -108,6 +119,7 @@ def test_select_variation_variation(test_feature_variants):
108119
selected_variant = test_feature_variants.get_variant({"userId": "2"})
109120
assert selected_variant["enabled"]
110121
assert selected_variant["name"] == "VarC"
122+
assert selected_variant["feature_enabled"]
111123

112124

113125
def test_variant_metrics_are_reset(test_feature_variants):
@@ -151,6 +163,23 @@ def test_strategy_variant_is_returned(test_feature_strategy_variants):
151163
"enabled": True,
152164
"name": "VarC",
153165
"payload": {"type": "string", "value": "Test 3"},
166+
"feature_enabled": True,
167+
}
168+
169+
170+
def test_feature_enabled_when_no_variants(test_feature_no_variants):
171+
context = {
172+
"userId": "122",
173+
"appName": "test",
174+
"environment": "prod",
175+
"customField": "1",
176+
}
177+
variant = test_feature_no_variants.get_variant(context)
178+
179+
assert variant == {
180+
"enabled": False,
181+
"name": "disabled",
182+
"feature_enabled": True,
154183
}
155184

156185

‎tests/utilities/mocks/mock_features.py

+19
Original file line numberDiff line numberDiff line change
@@ -252,3 +252,22 @@
252252
},
253253
],
254254
}
255+
256+
MOCK_FEATURE_ENABLED_NO_VARIANTS_RESPONSE = {
257+
"version": 1,
258+
"features": [
259+
{
260+
"name": "EnabledNoVariants",
261+
"description": "Enabled with no variants",
262+
"enabled": True,
263+
"strategies": [
264+
{
265+
"name": "default",
266+
"parameters": {},
267+
}
268+
],
269+
"createdAt": "2018-10-09T06:04:05.667Z",
270+
"impressionData": False,
271+
},
272+
],
273+
}

0 commit comments

Comments
 (0)