From 8b8b8b36334729051206861959c841b31b575e12 Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Thu, 5 Aug 2021 10:36:22 +0300 Subject: [PATCH 1/5] feat (feature flags): Add not_in action and rename contains to in --- .../utilities/feature_flags/feature_flags.py | 11 ++++-- .../utilities/feature_flags/schema.py | 5 ++- .../feature_flags/test_feature_flags.py | 38 +++++++++++++++---- .../feature_flags/test_schema_validation.py | 7 +++- 4 files changed, 47 insertions(+), 14 deletions(-) diff --git a/aws_lambda_powertools/utilities/feature_flags/feature_flags.py b/aws_lambda_powertools/utilities/feature_flags/feature_flags.py index 3d913de98d4..566155b3882 100644 --- a/aws_lambda_powertools/utilities/feature_flags/feature_flags.py +++ b/aws_lambda_powertools/utilities/feature_flags/feature_flags.py @@ -46,7 +46,8 @@ def _match_by_action(action: str, condition_value: Any, context_value: Any) -> b schema.RuleAction.EQUALS.value: lambda a, b: a == b, schema.RuleAction.STARTSWITH.value: lambda a, b: a.startswith(b), schema.RuleAction.ENDSWITH.value: lambda a, b: a.endswith(b), - schema.RuleAction.CONTAINS.value: lambda a, b: a in b, + schema.RuleAction.IN.value: lambda a, b: a in b, + schema.RuleAction.NOT_IN.value: lambda a, b: a not in b, } try: @@ -65,10 +66,12 @@ def _evaluate_conditions( for condition in conditions: context_value = context.get(str(condition.get(schema.CONDITION_KEY))) - cond_action = condition.get(schema.CONDITION_ACTION, "") - cond_value = condition.get(schema.CONDITION_VALUE) + condition_action = condition.get(schema.CONDITION_ACTION, "") + condition_value = condition.get(schema.CONDITION_VALUE) - if not self._match_by_action(action=cond_action, condition_value=cond_value, context_value=context_value): + if not self._match_by_action( + action=condition_action, condition_value=condition_value, context_value=context_value + ): logger.debug( f"rule did not match action, rule_name={rule_name}, rule_value={rule_match_value}, " f"name={feature_name}, context_value={str(context_value)} " diff --git a/aws_lambda_powertools/utilities/feature_flags/schema.py b/aws_lambda_powertools/utilities/feature_flags/schema.py index 6f2d2bf713c..3efe448546e 100644 --- a/aws_lambda_powertools/utilities/feature_flags/schema.py +++ b/aws_lambda_powertools/utilities/feature_flags/schema.py @@ -20,7 +20,8 @@ class RuleAction(str, Enum): EQUALS = "EQUALS" STARTSWITH = "STARTSWITH" ENDSWITH = "ENDSWITH" - CONTAINS = "CONTAINS" + IN = "IN" + NOT_IN = "NOT_IN" class SchemaValidator(BaseValidator): @@ -79,7 +80,7 @@ class SchemaValidator(BaseValidator): The value MUST contain the following members: * **action**: `str`. Operation to perform to match a key and value. - The value MUST be either EQUALS, STARTSWITH, ENDSWITH, CONTAINS + The value MUST be either EQUALS, STARTSWITH, ENDSWITH, IN * **key**: `str`. Key in given context to perform operation * **value**: `Any`. Value in given context that should match action operation. diff --git a/tests/functional/feature_flags/test_feature_flags.py b/tests/functional/feature_flags/test_feature_flags.py index d2150268062..6e4e12ac995 100644 --- a/tests/functional/feature_flags/test_feature_flags.py +++ b/tests/functional/feature_flags/test_feature_flags.py @@ -263,7 +263,7 @@ def test_flags_conditions_rule_match_multiple_actions_multiple_rules_multiple_co # check a case where the feature exists but the rule doesn't match so we revert to the default value of the feature -def test_flags_match_rule_with_contains_action(mocker, config): +def test_flags_match_rule_with_in_action(mocker, config): expected_value = True mocked_app_config_schema = { "my_feature": { @@ -273,7 +273,7 @@ def test_flags_match_rule_with_contains_action(mocker, config): "when_match": expected_value, "conditions": [ { - "action": RuleAction.CONTAINS.value, + "action": RuleAction.IN.value, "key": "tenant_id", "value": ["6", "2"], } @@ -287,7 +287,7 @@ def test_flags_match_rule_with_contains_action(mocker, config): assert toggle == expected_value -def test_flags_no_match_rule_with_contains_action(mocker, config): +def test_flags_no_match_rule_with_in_action(mocker, config): expected_value = False mocked_app_config_schema = { "my_feature": { @@ -297,7 +297,7 @@ def test_flags_no_match_rule_with_contains_action(mocker, config): "when_match": True, "conditions": [ { - "action": RuleAction.CONTAINS.value, + "action": RuleAction.IN.value, "key": "tenant_id", "value": ["8", "2"], } @@ -310,6 +310,30 @@ def test_flags_no_match_rule_with_contains_action(mocker, config): toggle = feature_flags.evaluate(name="my_feature", context={"tenant_id": "6", "username": "a"}, default=False) assert toggle == expected_value +def test_flags_match_rule_with_not_in_action(mocker, config): + expected_value = True + mocked_app_config_schema = { + "my_feature": { + "default": False, + "rules": { + "tenant id is contained in [8, 2]": { + "when_match": expected_value, + "conditions": [ + { + "action": RuleAction.NOT_IN.value, + "key": "tenant_id", + "value": ["10", "4"], + } + ], + } + }, + } + } + feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config) + toggle = feature_flags.evaluate(name="my_feature", context={"tenant_id": "6", "username": "a"}, default=False) + assert toggle == expected_value + + def test_multiple_features_enabled(mocker, config): expected_value = ["my_feature", "my_feature2"] @@ -321,7 +345,7 @@ def test_multiple_features_enabled(mocker, config): "when_match": True, "conditions": [ { - "action": RuleAction.CONTAINS.value, + "action": RuleAction.IN.value, "key": "tenant_id", "value": ["6", "2"], } @@ -351,7 +375,7 @@ def test_multiple_features_only_some_enabled(mocker, config): "when_match": True, "conditions": [ { - "action": RuleAction.CONTAINS.value, + "action": RuleAction.IN.value, "key": "tenant_id", "value": ["6", "2"], } @@ -464,7 +488,7 @@ def test_features_jmespath_envelope(mocker, config): assert toggle == expected_value -# test_match_rule_with_contains_action +# test_match_rule_with_equals_action def test_match_condition_with_dict_value(mocker, config): expected_value = True mocked_app_config_schema = { diff --git a/tests/functional/feature_flags/test_schema_validation.py b/tests/functional/feature_flags/test_schema_validation.py index 2c33d3c61cc..ce85494afce 100644 --- a/tests/functional/feature_flags/test_schema_validation.py +++ b/tests/functional/feature_flags/test_schema_validation.py @@ -211,10 +211,15 @@ def test_valid_condition_all_actions(): CONDITION_VALUE: "a", }, { - CONDITION_ACTION: RuleAction.CONTAINS.value, + CONDITION_ACTION: RuleAction.IN.value, CONDITION_KEY: "username", CONDITION_VALUE: ["a", "b"], }, + { + CONDITION_ACTION: RuleAction.NOT_IN.value, + CONDITION_KEY: "username", + CONDITION_VALUE: ["c"], + }, ], } }, From 5db73c2d614898690f9e72d838fe1ec1f666f090 Mon Sep 17 00:00:00 2001 From: Ran Isenberg <60175085+risenberg-cyberark@users.noreply.github.com> Date: Thu, 5 Aug 2021 10:46:34 +0300 Subject: [PATCH 2/5] Update aws_lambda_powertools/utilities/feature_flags/feature_flags.py Co-authored-by: Heitor Lessa --- .../utilities/feature_flags/feature_flags.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws_lambda_powertools/utilities/feature_flags/feature_flags.py b/aws_lambda_powertools/utilities/feature_flags/feature_flags.py index 566155b3882..c8b8b7b8ed5 100644 --- a/aws_lambda_powertools/utilities/feature_flags/feature_flags.py +++ b/aws_lambda_powertools/utilities/feature_flags/feature_flags.py @@ -66,8 +66,8 @@ def _evaluate_conditions( for condition in conditions: context_value = context.get(str(condition.get(schema.CONDITION_KEY))) - condition_action = condition.get(schema.CONDITION_ACTION, "") - condition_value = condition.get(schema.CONDITION_VALUE) + cond_action = condition.get(schema.CONDITION_ACTION, "") + cond_value = condition.get(schema.CONDITION_VALUE) if not self._match_by_action( action=condition_action, condition_value=condition_value, context_value=context_value From 382ac3a94d055b090f906a67c445eb0b7e6346b2 Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Thu, 5 Aug 2021 10:47:09 +0300 Subject: [PATCH 3/5] add extra test for not matching not_in --- .../utilities/feature_flags/schema.py | 2 +- .../feature_flags/test_feature_flags.py | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/aws_lambda_powertools/utilities/feature_flags/schema.py b/aws_lambda_powertools/utilities/feature_flags/schema.py index 3efe448546e..efce82018db 100644 --- a/aws_lambda_powertools/utilities/feature_flags/schema.py +++ b/aws_lambda_powertools/utilities/feature_flags/schema.py @@ -80,7 +80,7 @@ class SchemaValidator(BaseValidator): The value MUST contain the following members: * **action**: `str`. Operation to perform to match a key and value. - The value MUST be either EQUALS, STARTSWITH, ENDSWITH, IN + The value MUST be either EQUALS, STARTSWITH, ENDSWITH, IN, NOT_IN * **key**: `str`. Key in given context to perform operation * **value**: `Any`. Value in given context that should match action operation. diff --git a/tests/functional/feature_flags/test_feature_flags.py b/tests/functional/feature_flags/test_feature_flags.py index 6e4e12ac995..c0f463c78d0 100644 --- a/tests/functional/feature_flags/test_feature_flags.py +++ b/tests/functional/feature_flags/test_feature_flags.py @@ -310,6 +310,7 @@ def test_flags_no_match_rule_with_in_action(mocker, config): toggle = feature_flags.evaluate(name="my_feature", context={"tenant_id": "6", "username": "a"}, default=False) assert toggle == expected_value + def test_flags_match_rule_with_not_in_action(mocker, config): expected_value = True mocked_app_config_schema = { @@ -334,6 +335,29 @@ def test_flags_match_rule_with_not_in_action(mocker, config): assert toggle == expected_value +def test_flags_no_match_rule_with_not_in_action(mocker, config): + expected_value = False + mocked_app_config_schema = { + "my_feature": { + "default": expected_value, + "rules": { + "tenant id is contained in [8, 2]": { + "when_match": True, + "conditions": [ + { + "action": RuleAction.NOT_IN.value, + "key": "tenant_id", + "value": ["6", "4"], + } + ], + } + }, + } + } + feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config) + toggle = feature_flags.evaluate(name="my_feature", context={"tenant_id": "6", "username": "a"}, default=False) + assert toggle == expected_value + def test_multiple_features_enabled(mocker, config): expected_value = ["my_feature", "my_feature2"] From 8246b2ec4132e07f8af5cbbe55f5ae3a57fb73cf Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Thu, 5 Aug 2021 10:52:36 +0300 Subject: [PATCH 4/5] lint --- .../utilities/feature_flags/feature_flags.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/aws_lambda_powertools/utilities/feature_flags/feature_flags.py b/aws_lambda_powertools/utilities/feature_flags/feature_flags.py index c8b8b7b8ed5..3009b367aad 100644 --- a/aws_lambda_powertools/utilities/feature_flags/feature_flags.py +++ b/aws_lambda_powertools/utilities/feature_flags/feature_flags.py @@ -66,12 +66,10 @@ def _evaluate_conditions( for condition in conditions: context_value = context.get(str(condition.get(schema.CONDITION_KEY))) - cond_action = condition.get(schema.CONDITION_ACTION, "") + cond_value = condition.get(schema.CONDITION_ACTION, "") cond_value = condition.get(schema.CONDITION_VALUE) - if not self._match_by_action( - action=condition_action, condition_value=condition_value, context_value=context_value - ): + if not self._match_by_action(action=cond_value, condition_value=cond_value, context_value=context_value): logger.debug( f"rule did not match action, rule_name={rule_name}, rule_value={rule_match_value}, " f"name={feature_name}, context_value={str(context_value)} " From 8fa11f5918d7f7d5494abb93669d203eff361164 Mon Sep 17 00:00:00 2001 From: Ran Isenberg Date: Thu, 5 Aug 2021 10:54:25 +0300 Subject: [PATCH 5/5] fix rename --- .../utilities/feature_flags/feature_flags.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws_lambda_powertools/utilities/feature_flags/feature_flags.py b/aws_lambda_powertools/utilities/feature_flags/feature_flags.py index 3009b367aad..1606933bbc2 100644 --- a/aws_lambda_powertools/utilities/feature_flags/feature_flags.py +++ b/aws_lambda_powertools/utilities/feature_flags/feature_flags.py @@ -66,10 +66,10 @@ def _evaluate_conditions( for condition in conditions: context_value = context.get(str(condition.get(schema.CONDITION_KEY))) - cond_value = condition.get(schema.CONDITION_ACTION, "") + cond_action = condition.get(schema.CONDITION_ACTION, "") cond_value = condition.get(schema.CONDITION_VALUE) - if not self._match_by_action(action=cond_value, condition_value=cond_value, context_value=context_value): + if not self._match_by_action(action=cond_action, condition_value=cond_value, context_value=context_value): logger.debug( f"rule did not match action, rule_name={rule_name}, rule_value={rule_match_value}, " f"name={feature_name}, context_value={str(context_value)} "