diff --git a/openfeature/client.py b/openfeature/client.py index fc6e65a0..cd82694b 100644 --- a/openfeature/client.py +++ b/openfeature/client.py @@ -366,13 +366,14 @@ def evaluate_flag_details( # noqa: PLR0915 except OpenFeatureError as err: error_hooks(flag_type, hook_context, err, reversed_merged_hooks, hook_hints) - return FlagEvaluationDetails( + flag_evaluation = FlagEvaluationDetails( flag_key=flag_key, value=default_value, reason=Reason.ERROR, error_code=err.error_code, error_message=err.error_message, ) + return flag_evaluation # Catch any type of exception here since the user can provide any exception # in the error hooks except Exception as err: # pragma: no cover @@ -383,16 +384,23 @@ def evaluate_flag_details( # noqa: PLR0915 error_hooks(flag_type, hook_context, err, reversed_merged_hooks, hook_hints) error_message = getattr(err, "error_message", str(err)) - return FlagEvaluationDetails( + flag_evaluation = FlagEvaluationDetails( flag_key=flag_key, value=default_value, reason=Reason.ERROR, error_code=ErrorCode.GENERAL, error_message=error_message, ) + return flag_evaluation finally: - after_all_hooks(flag_type, hook_context, reversed_merged_hooks, hook_hints) + after_all_hooks( + flag_type, + hook_context, + flag_evaluation, + reversed_merged_hooks, + hook_hints, + ) def _create_provider_evaluation( self, diff --git a/openfeature/hook/__init__.py b/openfeature/hook/__init__.py index 77190dc0..03d8c865 100644 --- a/openfeature/hook/__init__.py +++ b/openfeature/hook/__init__.py @@ -109,7 +109,12 @@ def error( """ pass - def finally_after(self, hook_context: HookContext, hints: HookHints) -> None: + def finally_after( + self, + hook_context: HookContext, + details: FlagEvaluationDetails[typing.Any], + hints: HookHints, + ) -> None: """ Run after flag evaluation, including any error processing. This will always run. Errors will be swallowed. diff --git a/openfeature/hook/_hook_support.py b/openfeature/hook/_hook_support.py index 349b25f3..2c151ae1 100644 --- a/openfeature/hook/_hook_support.py +++ b/openfeature/hook/_hook_support.py @@ -25,10 +25,11 @@ def error_hooks( def after_all_hooks( flag_type: FlagType, hook_context: HookContext, + details: FlagEvaluationDetails[typing.Any], hooks: typing.List[Hook], hints: typing.Optional[HookHints] = None, ) -> None: - kwargs = {"hook_context": hook_context, "hints": hints} + kwargs = {"hook_context": hook_context, "details": details, "hints": hints} _execute_hooks( flag_type=flag_type, hooks=hooks, hook_method=HookType.FINALLY_AFTER, **kwargs ) diff --git a/pyproject.toml b/pyproject.toml index 2e076acd..50abed87 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -49,7 +49,7 @@ cov = [ ] e2e = [ "git submodule add --force https://github.com/open-feature/spec.git spec", - "cp -r spec/specification/assets/gherkin/evaluation.feature tests/features/", + "cp spec/specification/assets/gherkin/* tests/features/", "behave tests/features/", "rm tests/features/*.feature", ] diff --git a/tests/features/steps/hooks_steps.py b/tests/features/steps/hooks_steps.py new file mode 100644 index 00000000..bc7e156b --- /dev/null +++ b/tests/features/steps/hooks_steps.py @@ -0,0 +1,72 @@ +from unittest.mock import MagicMock + +from behave import then, when + +from openfeature.exception import ErrorCode +from openfeature.hook import Hook + + +@when("a hook is added to the client") +def step_impl_add_hook(context): + hook = MagicMock(spec=Hook) + hook.before = MagicMock() + hook.after = MagicMock() + hook.error = MagicMock() + hook.finally_after = MagicMock() + context.hook = hook + context.client.add_hooks([hook]) + + +@then("error hooks should be called") +def step_impl_call_error(context): + assert context.hook.before.called + assert context.hook.error.called + assert context.hook.finally_after.called + + +@then("non-error hooks should be called") +def step_impl_call_non_error(context): + assert context.hook.before.called + assert context.hook.after.called + assert context.hook.finally_after.called + + +def get_hook_from_name(context, hook_name): + if hook_name.lower() == "before": + return context.hook.before + elif hook_name.lower() == "after": + return context.hook.after + elif hook_name.lower() == "error": + return context.hook.error + elif hook_name.lower() == "finally": + return context.hook.finally_after + else: + raise ValueError(str(hook_name) + " is not a valid hook name") + + +def convert_value_from_flag_type(value, flag_type): + if value == "None": + return None + if flag_type.lower() == "boolean": + return bool(value) + elif flag_type.lower() == "integer": + return int(value) + elif flag_type.lower() == "float": + return float(value) + return value + + +@then('"{hook_names}" hooks should have evaluation details') +def step_impl_should_have_eval_details(context, hook_names): + for hook_name in hook_names.split(", "): + hook = get_hook_from_name(context, hook_name) + for row in context.table: + flag_type, key, value = row + + value = convert_value_from_flag_type(value, flag_type) + + actual = hook.call_args[1]["details"].__dict__[key] + if isinstance(actual, ErrorCode): + actual = str(actual) + + assert actual == value diff --git a/tests/features/steps/steps.py b/tests/features/steps/steps.py index dd8f264c..5d9d38fd 100644 --- a/tests/features/steps/steps.py +++ b/tests/features/steps/steps.py @@ -43,7 +43,8 @@ def step_impl_provider(context): '"{default_value}"' ) def step_impl_evaluated_with_details(context, flag_type, key, default_value): - context.client = get_client() + if context.client is None: + context.client = get_client() if flag_type == "boolean": context.boolean_flag_details = context.client.get_boolean_details( key, default_value diff --git a/tests/hook/test_hook_support.py b/tests/hook/test_hook_support.py index 64bb8f6f..19a86ec0 100644 --- a/tests/hook/test_hook_support.py +++ b/tests/hook/test_hook_support.py @@ -137,12 +137,17 @@ def test_after_hooks_run_after_method(mock_hook): def test_finally_after_hooks_run_finally_after_method(mock_hook): # Given hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "") + flag_evaluation_details = FlagEvaluationDetails( + hook_context.flag_key, "val", "unknown" + ) hook_hints = MappingProxyType({}) # When - after_all_hooks(FlagType.BOOLEAN, hook_context, [mock_hook], hook_hints) + after_all_hooks( + FlagType.BOOLEAN, hook_context, flag_evaluation_details, [mock_hook], hook_hints + ) # Then mock_hook.supports_flag_value_type.assert_called_once() mock_hook.finally_after.assert_called_once() mock_hook.finally_after.assert_called_with( - hook_context=hook_context, hints=hook_hints + hook_context=hook_context, details=flag_evaluation_details, hints=hook_hints )