From 7e7fef2a43309534c23348bfe19b37482f0c77bc Mon Sep 17 00:00:00 2001 From: AdityaVallabh Date: Sun, 16 Mar 2025 06:25:43 +0000 Subject: [PATCH 1/4] fix(flagd): handle falsy target values correctly Signed-off-by: AdityaVallabh --- .../provider/flagd/resolvers/in_process.py | 2 - .../provider/flagd/resolvers/process/flags.py | 3 + .../tests/test_in_process.py | 178 ++++++++++++++++++ 3 files changed, 181 insertions(+), 2 deletions(-) create mode 100644 providers/openfeature-provider-flagd/tests/test_in_process.py diff --git a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/in_process.py b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/in_process.py index a555896d..70da24ea 100644 --- a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/in_process.py +++ b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/in_process.py @@ -121,8 +121,6 @@ def _resolve( ) variant, value = flag.get_variant(variant) - if not value: - raise ParseError(f"Resolved variant {variant} not in variants config.") return FlagResolutionDetails( value, diff --git a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/flags.py b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/flags.py index d8f93b36..cfc146eb 100644 --- a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/flags.py +++ b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/flags.py @@ -92,4 +92,7 @@ def get_variant( if isinstance(variant_key, bool): variant_key = str(variant_key).lower() + if variant_key not in self.variants: + raise ParseError(f"Resolved variant {variant_key} not in variants config.") + return variant_key, self.variants.get(variant_key) diff --git a/providers/openfeature-provider-flagd/tests/test_in_process.py b/providers/openfeature-provider-flagd/tests/test_in_process.py new file mode 100644 index 00000000..3467e6c3 --- /dev/null +++ b/providers/openfeature-provider-flagd/tests/test_in_process.py @@ -0,0 +1,178 @@ +import pytest +from unittest.mock import Mock, create_autospec +from openfeature.contrib.provider.flagd.resolvers.in_process import InProcessResolver +from openfeature.evaluation_context import EvaluationContext +from openfeature.exception import FlagNotFoundError +from openfeature.contrib.provider.flagd.resolvers.process.flags import Flag, FlagStore +from openfeature.contrib.provider.flagd.config import Config +from openfeature.exception import FlagNotFoundError, ParseError + +@pytest.fixture +def config(): + return create_autospec(Config) + +@pytest.fixture +def flag_store(): + return create_autospec(FlagStore) + +@pytest.fixture +def targeting(): + return { + "if": [ + {"==": [{"var": "targetingKey"}, "target_variant"]}, + "target_variant", + None, + ] + } + +@pytest.fixture +def flag(targeting): + return Flag( + key="flag", + state="ENABLED", + variants={"default_variant": False, "target_variant": True}, + default_variant="default_variant", + targeting=targeting + ) + +@pytest.fixture +def context(): + return EvaluationContext(targeting_key="target_variant") + +@pytest.fixture +def resolver(config): + config.offline_flag_source_path = 'flag.json' + config.deadline_ms = 100 + return InProcessResolver( + config=config, + emit_provider_ready=Mock(), + emit_provider_error=Mock(), + emit_provider_stale=Mock(), + emit_provider_configuration_changed=Mock() + ) + +@pytest.fixture +def flag(): + return Flag( + key="flag", + state="ENABLED", + variants={"default_variant": False}, + default_variant="default_variant", + targeting=None, + ) + +def targeting(): + return { + "if": [ + {"==": [{"var": "targetingKey"}, "target_variant"]}, + "target_variant", + None, + ] + } + +def context(targeting_key): + return EvaluationContext(targeting_key=targeting_key) + +def test_resolve_boolean_details_flag_not_found(resolver): + resolver.flag_store.get_flag = Mock(return_value=None) + with pytest.raises(FlagNotFoundError): + resolver.resolve_boolean_details("nonexistent_flag", False) + +def test_resolve_boolean_details_disabled_flag(flag, resolver): + flag.state = "DISABLED" + resolver.flag_store.get_flag = Mock(return_value=flag) + + result = resolver.resolve_boolean_details("disabled_flag", False) + + assert result.reason == "DISABLED" + assert result.variant == None + assert result.value == False + +def test_resolve_boolean_details_invalid_variant(resolver, flag): + flag.targeting = { + "var": ["targetingKey", "invalid_variant"] + } + + resolver.flag_store.get_flag = Mock(return_value=flag) + + with pytest.raises(ParseError): + resolver.resolve_boolean_details("flag", False) + +@pytest.mark.parametrize( + "variants, targeting," + "context, method, default_value," + "expected_reason, expected_variant, expected_value,", + [ + ( + {"default_variant": False, "target_variant": True}, None, + None, "resolve_boolean_details", False, + "STATIC", "default_variant", False, + ), + ( + {"default_variant": False, "target_variant": True}, targeting(), + context("no_target_variant"), "resolve_boolean_details", False, + "DEFAULT", "default_variant", False, + ), + ( + {"default_variant": False, "target_variant": True}, targeting(), + context("target_variant"), "resolve_boolean_details", False, + "TARGETING_MATCH", "target_variant", True, + ), + ( + {"default_variant": "default", "target_variant": "target"}, targeting(), + context("target_variant"), "resolve_string_details", "placeholder", + "TARGETING_MATCH", "target_variant", "target", + ), + ( + {"default_variant": 1.0, "target_variant": 2.0}, targeting(), + context("target_variant"), "resolve_float_details", 0.0, + "TARGETING_MATCH", "target_variant", 2.0, + ), + ( + {"default_variant": True, "target_variant": False}, targeting(), + context("target_variant"), "resolve_boolean_details", True, + "TARGETING_MATCH", "target_variant", False, + ), + ( + {"default_variant": 10, "target_variant": 0}, targeting(), + context("target_variant"), "resolve_integer_details", 1, + "TARGETING_MATCH", "target_variant", 0, + ), + ( + {"default_variant": {}, "target_variant": {}}, targeting(), + context("target_variant"), "resolve_object_details", {}, + "TARGETING_MATCH", "target_variant", {}, + ), + ( + {"default_variant": None, "target_variant": None}, targeting(), + context("target_variant"), "resolve_object_details", {}, + "TARGETING_MATCH", "target_variant", None, + ), + ], + ids=[ + "static_flag", + "boolean_default_fallback", + "boolean_targeting_match", + "string_targeting_match", + "float_targeting_match", + "boolean_falsy_target", + "integer_falsy_target", + "object_falsy_target", + "none_target_value", + ], +) +def test_resolver_details( + resolver, flag, + variants, targeting, + context, method, default_value, + expected_reason, expected_variant, expected_value + ): + flag.variants = variants + flag.targeting = targeting + resolver.flag_store.get_flag = Mock(return_value=flag) + + result = getattr(resolver, method)("flag", default_value, context) + + assert result.reason == expected_reason + assert result.variant == expected_variant + assert result.value == expected_value From 5137690b5a477b81a2decf69a26cfd9eb3cd8f27 Mon Sep 17 00:00:00 2001 From: AdityaVallabh Date: Mon, 17 Mar 2025 15:09:52 +0000 Subject: [PATCH 2/4] chore(flagd): run pre-commit reformatting Signed-off-by: AdityaVallabh --- .../tests/test_in_process.py | 150 ++++++++++++------ 1 file changed, 101 insertions(+), 49 deletions(-) diff --git a/providers/openfeature-provider-flagd/tests/test_in_process.py b/providers/openfeature-provider-flagd/tests/test_in_process.py index 3467e6c3..fbf9f949 100644 --- a/providers/openfeature-provider-flagd/tests/test_in_process.py +++ b/providers/openfeature-provider-flagd/tests/test_in_process.py @@ -1,20 +1,24 @@ -import pytest from unittest.mock import Mock, create_autospec + +import pytest + +from openfeature.contrib.provider.flagd.config import Config from openfeature.contrib.provider.flagd.resolvers.in_process import InProcessResolver -from openfeature.evaluation_context import EvaluationContext -from openfeature.exception import FlagNotFoundError from openfeature.contrib.provider.flagd.resolvers.process.flags import Flag, FlagStore -from openfeature.contrib.provider.flagd.config import Config +from openfeature.evaluation_context import EvaluationContext from openfeature.exception import FlagNotFoundError, ParseError + @pytest.fixture def config(): return create_autospec(Config) + @pytest.fixture def flag_store(): return create_autospec(FlagStore) + @pytest.fixture def targeting(): return { @@ -24,7 +28,8 @@ def targeting(): None, ] } - + + @pytest.fixture def flag(targeting): return Flag( @@ -32,25 +37,28 @@ def flag(targeting): state="ENABLED", variants={"default_variant": False, "target_variant": True}, default_variant="default_variant", - targeting=targeting + targeting=targeting, ) + @pytest.fixture def context(): return EvaluationContext(targeting_key="target_variant") + @pytest.fixture def resolver(config): - config.offline_flag_source_path = 'flag.json' + config.offline_flag_source_path = "flag.json" config.deadline_ms = 100 return InProcessResolver( config=config, emit_provider_ready=Mock(), emit_provider_error=Mock(), emit_provider_stale=Mock(), - emit_provider_configuration_changed=Mock() + emit_provider_configuration_changed=Mock(), ) + @pytest.fixture def flag(): return Flag( @@ -61,6 +69,7 @@ def flag(): targeting=None, ) + def targeting(): return { "if": [ @@ -70,14 +79,17 @@ def targeting(): ] } + def context(targeting_key): return EvaluationContext(targeting_key=targeting_key) + def test_resolve_boolean_details_flag_not_found(resolver): resolver.flag_store.get_flag = Mock(return_value=None) with pytest.raises(FlagNotFoundError): resolver.resolve_boolean_details("nonexistent_flag", False) + def test_resolve_boolean_details_disabled_flag(flag, resolver): flag.state = "DISABLED" resolver.flag_store.get_flag = Mock(return_value=flag) @@ -88,65 +100,100 @@ def test_resolve_boolean_details_disabled_flag(flag, resolver): assert result.variant == None assert result.value == False + def test_resolve_boolean_details_invalid_variant(resolver, flag): - flag.targeting = { - "var": ["targetingKey", "invalid_variant"] - } + flag.targeting = {"var": ["targetingKey", "invalid_variant"]} resolver.flag_store.get_flag = Mock(return_value=flag) with pytest.raises(ParseError): resolver.resolve_boolean_details("flag", False) + @pytest.mark.parametrize( - "variants, targeting," - "context, method, default_value," - "expected_reason, expected_variant, expected_value,", + "variants, targeting," + "context, method, default_value," + "expected_reason, expected_variant, expected_value,", [ ( - {"default_variant": False, "target_variant": True}, None, - None, "resolve_boolean_details", False, - "STATIC", "default_variant", False, - ), - ( - {"default_variant": False, "target_variant": True}, targeting(), - context("no_target_variant"), "resolve_boolean_details", False, - "DEFAULT", "default_variant", False, + {"default_variant": False, "target_variant": True}, + None, + None, + "resolve_boolean_details", + False, + "STATIC", + "default_variant", + False, ), ( - {"default_variant": False, "target_variant": True}, targeting(), - context("target_variant"), "resolve_boolean_details", False, - "TARGETING_MATCH", "target_variant", True, + {"default_variant": False, "target_variant": True}, + targeting(), + context("no_target_variant"), + "resolve_boolean_details", + False, + "DEFAULT", + "default_variant", + False, ), ( - {"default_variant": "default", "target_variant": "target"}, targeting(), - context("target_variant"), "resolve_string_details", "placeholder", - "TARGETING_MATCH", "target_variant", "target", + {"default_variant": False, "target_variant": True}, + targeting(), + context("target_variant"), + "resolve_boolean_details", + False, + "TARGETING_MATCH", + "target_variant", + True, ), ( - {"default_variant": 1.0, "target_variant": 2.0}, targeting(), - context("target_variant"), "resolve_float_details", 0.0, - "TARGETING_MATCH", "target_variant", 2.0, + {"default_variant": "default", "target_variant": "target"}, + targeting(), + context("target_variant"), + "resolve_string_details", + "placeholder", + "TARGETING_MATCH", + "target_variant", + "target", ), ( - {"default_variant": True, "target_variant": False}, targeting(), - context("target_variant"), "resolve_boolean_details", True, - "TARGETING_MATCH", "target_variant", False, + {"default_variant": 1.0, "target_variant": 2.0}, + targeting(), + context("target_variant"), + "resolve_float_details", + 0.0, + "TARGETING_MATCH", + "target_variant", + 2.0, ), ( - {"default_variant": 10, "target_variant": 0}, targeting(), - context("target_variant"), "resolve_integer_details", 1, - "TARGETING_MATCH", "target_variant", 0, + {"default_variant": True, "target_variant": False}, + targeting(), + context("target_variant"), + "resolve_boolean_details", + True, + "TARGETING_MATCH", + "target_variant", + False, ), ( - {"default_variant": {}, "target_variant": {}}, targeting(), - context("target_variant"), "resolve_object_details", {}, - "TARGETING_MATCH", "target_variant", {}, + {"default_variant": 10, "target_variant": 0}, + targeting(), + context("target_variant"), + "resolve_integer_details", + 1, + "TARGETING_MATCH", + "target_variant", + 0, ), ( - {"default_variant": None, "target_variant": None}, targeting(), - context("target_variant"), "resolve_object_details", {}, - "TARGETING_MATCH", "target_variant", None, + {"default_variant": {}, "target_variant": {}}, + targeting(), + context("target_variant"), + "resolve_object_details", + {}, + "TARGETING_MATCH", + "target_variant", + {}, ), ], ids=[ @@ -158,15 +205,20 @@ def test_resolve_boolean_details_invalid_variant(resolver, flag): "boolean_falsy_target", "integer_falsy_target", "object_falsy_target", - "none_target_value", ], ) def test_resolver_details( - resolver, flag, - variants, targeting, - context, method, default_value, - expected_reason, expected_variant, expected_value - ): + resolver, + flag, + variants, + targeting, + context, + method, + default_value, + expected_reason, + expected_variant, + expected_value, +): flag.variants = variants flag.targeting = targeting resolver.flag_store.get_flag = Mock(return_value=flag) From df130a65bf315e9ae01abf420a96186a9a2bc2a6 Mon Sep 17 00:00:00 2001 From: AdityaVallabh Date: Mon, 17 Mar 2025 15:10:20 +0000 Subject: [PATCH 3/4] fix(flagd): skip handling 'None' Signed-off-by: AdityaVallabh --- .../openfeature/contrib/provider/flagd/resolvers/in_process.py | 2 ++ .../contrib/provider/flagd/resolvers/process/flags.py | 3 --- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/in_process.py b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/in_process.py index 70da24ea..1e614773 100644 --- a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/in_process.py +++ b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/in_process.py @@ -121,6 +121,8 @@ def _resolve( ) variant, value = flag.get_variant(variant) + if value is None: + raise ParseError(f"Resolved variant {variant} not in variants config.") return FlagResolutionDetails( value, diff --git a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/flags.py b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/flags.py index cc0a818e..638535b4 100644 --- a/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/flags.py +++ b/providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/resolvers/process/flags.py @@ -90,7 +90,4 @@ def get_variant( if isinstance(variant_key, bool): variant_key = str(variant_key).lower() - if variant_key not in self.variants: - raise ParseError(f"Resolved variant {variant_key} not in variants config.") - return variant_key, self.variants.get(variant_key) From e7ad1f6e0e5b05a87174eb9de02b4832ef21c1b9 Mon Sep 17 00:00:00 2001 From: AdityaVallabh Date: Tue, 18 Mar 2025 12:54:30 +0000 Subject: [PATCH 4/4] chore(flagd): fix linting Signed-off-by: AdityaVallabh --- .../tests/test_in_process.py | 240 +++++++++--------- 1 file changed, 114 insertions(+), 126 deletions(-) diff --git a/providers/openfeature-provider-flagd/tests/test_in_process.py b/providers/openfeature-provider-flagd/tests/test_in_process.py index fbf9f949..cc9a67fd 100644 --- a/providers/openfeature-provider-flagd/tests/test_in_process.py +++ b/providers/openfeature-provider-flagd/tests/test_in_process.py @@ -9,17 +9,6 @@ from openfeature.exception import FlagNotFoundError, ParseError -@pytest.fixture -def config(): - return create_autospec(Config) - - -@pytest.fixture -def flag_store(): - return create_autospec(FlagStore) - - -@pytest.fixture def targeting(): return { "if": [ @@ -30,22 +19,31 @@ def targeting(): } +def context(targeting_key): + return EvaluationContext(targeting_key=targeting_key) + + @pytest.fixture -def flag(targeting): +def config(): + return create_autospec(Config) + + +@pytest.fixture +def flag_store(): + return create_autospec(FlagStore) + + +@pytest.fixture +def flag(): return Flag( key="flag", state="ENABLED", variants={"default_variant": False, "target_variant": True}, default_variant="default_variant", - targeting=targeting, + targeting=targeting(), ) -@pytest.fixture -def context(): - return EvaluationContext(targeting_key="target_variant") - - @pytest.fixture def resolver(config): config.offline_flag_source_path = "flag.json" @@ -59,31 +57,6 @@ def resolver(config): ) -@pytest.fixture -def flag(): - return Flag( - key="flag", - state="ENABLED", - variants={"default_variant": False}, - default_variant="default_variant", - targeting=None, - ) - - -def targeting(): - return { - "if": [ - {"==": [{"var": "targetingKey"}, "target_variant"]}, - "target_variant", - None, - ] - } - - -def context(targeting_key): - return EvaluationContext(targeting_key=targeting_key) - - def test_resolve_boolean_details_flag_not_found(resolver): resolver.flag_store.get_flag = Mock(return_value=None) with pytest.raises(FlagNotFoundError): @@ -97,8 +70,8 @@ def test_resolve_boolean_details_disabled_flag(flag, resolver): result = resolver.resolve_boolean_details("disabled_flag", False) assert result.reason == "DISABLED" - assert result.variant == None - assert result.value == False + assert result.variant is None + assert not result.value def test_resolve_boolean_details_invalid_variant(resolver, flag): @@ -111,89 +84,107 @@ def test_resolve_boolean_details_invalid_variant(resolver, flag): @pytest.mark.parametrize( - "variants, targeting," - "context, method, default_value," - "expected_reason, expected_variant, expected_value,", + "input_config, resolve_config, expected", [ ( - {"default_variant": False, "target_variant": True}, - None, - None, - "resolve_boolean_details", - False, - "STATIC", - "default_variant", - False, + { + "variants": {"default_variant": False, "target_variant": True}, + "targeting": None, + }, + { + "context": None, + "method": "resolve_boolean_details", + "default_value": False, + }, + {"reason": "STATIC", "variant": "default_variant", "value": False}, ), ( - {"default_variant": False, "target_variant": True}, - targeting(), - context("no_target_variant"), - "resolve_boolean_details", - False, - "DEFAULT", - "default_variant", - False, + { + "variants": {"default_variant": False, "target_variant": True}, + "targeting": targeting(), + }, + { + "context": context("no_target_variant"), + "method": "resolve_boolean_details", + "default_value": False, + }, + {"reason": "DEFAULT", "variant": "default_variant", "value": False}, ), ( - {"default_variant": False, "target_variant": True}, - targeting(), - context("target_variant"), - "resolve_boolean_details", - False, - "TARGETING_MATCH", - "target_variant", - True, + { + "variants": {"default_variant": False, "target_variant": True}, + "targeting": targeting(), + }, + { + "context": context("target_variant"), + "method": "resolve_boolean_details", + "default_value": False, + }, + {"reason": "TARGETING_MATCH", "variant": "target_variant", "value": True}, ), ( - {"default_variant": "default", "target_variant": "target"}, - targeting(), - context("target_variant"), - "resolve_string_details", - "placeholder", - "TARGETING_MATCH", - "target_variant", - "target", + { + "variants": {"default_variant": "default", "target_variant": "target"}, + "targeting": targeting(), + }, + { + "context": context("target_variant"), + "method": "resolve_string_details", + "default_value": "placeholder", + }, + { + "reason": "TARGETING_MATCH", + "variant": "target_variant", + "value": "target", + }, ), ( - {"default_variant": 1.0, "target_variant": 2.0}, - targeting(), - context("target_variant"), - "resolve_float_details", - 0.0, - "TARGETING_MATCH", - "target_variant", - 2.0, + { + "variants": {"default_variant": 1.0, "target_variant": 2.0}, + "targeting": targeting(), + }, + { + "context": context("target_variant"), + "method": "resolve_float_details", + "default_value": 0.0, + }, + {"reason": "TARGETING_MATCH", "variant": "target_variant", "value": 2.0}, ), ( - {"default_variant": True, "target_variant": False}, - targeting(), - context("target_variant"), - "resolve_boolean_details", - True, - "TARGETING_MATCH", - "target_variant", - False, + { + "variants": {"default_variant": True, "target_variant": False}, + "targeting": targeting(), + }, + { + "context": context("target_variant"), + "method": "resolve_boolean_details", + "default_value": True, + }, + {"reason": "TARGETING_MATCH", "variant": "target_variant", "value": False}, ), ( - {"default_variant": 10, "target_variant": 0}, - targeting(), - context("target_variant"), - "resolve_integer_details", - 1, - "TARGETING_MATCH", - "target_variant", - 0, + { + "variants": {"default_variant": 10, "target_variant": 0}, + "targeting": targeting(), + }, + { + "context": context("target_variant"), + "method": "resolve_integer_details", + "default_value": 1, + }, + {"reason": "TARGETING_MATCH", "variant": "target_variant", "value": 0}, ), ( - {"default_variant": {}, "target_variant": {}}, - targeting(), - context("target_variant"), - "resolve_object_details", - {}, - "TARGETING_MATCH", - "target_variant", - {}, + { + "variants": {"default_variant": {}, "target_variant": {}}, + "targeting": targeting(), + }, + { + "context": context("target_variant"), + "method": "resolve_object_details", + "default_value": {}, + }, + {"reason": "TARGETING_MATCH", "variant": "target_variant", "value": {}}, ), ], ids=[ @@ -210,21 +201,18 @@ def test_resolve_boolean_details_invalid_variant(resolver, flag): def test_resolver_details( resolver, flag, - variants, - targeting, - context, - method, - default_value, - expected_reason, - expected_variant, - expected_value, + input_config, + resolve_config, + expected, ): - flag.variants = variants - flag.targeting = targeting + flag.variants = input_config["variants"] + flag.targeting = input_config["targeting"] resolver.flag_store.get_flag = Mock(return_value=flag) - result = getattr(resolver, method)("flag", default_value, context) + result = getattr(resolver, resolve_config["method"])( + "flag", resolve_config["default_value"], resolve_config["context"] + ) - assert result.reason == expected_reason - assert result.variant == expected_variant - assert result.value == expected_value + assert result.reason == expected["reason"] + assert result.variant == expected["variant"] + assert result.value == expected["value"]