From 3b0c4597b40a88712d3c8f6471531ff649f71ad7 Mon Sep 17 00:00:00 2001 From: leohoare Date: Wed, 13 Nov 2024 15:17:33 +1100 Subject: [PATCH] update to remove async hooks Signed-off-by: leohoare --- openfeature/client.py | 24 ++---- openfeature/hook/__init__.py | 63 --------------- openfeature/hook/_hook_support.py | 124 ------------------------------ tests/hook/conftest.py | 12 --- tests/hook/test_hook_support.py | 81 ------------------- tests/test_async_client.py | 27 +++---- 6 files changed, 19 insertions(+), 312 deletions(-) diff --git a/openfeature/client.py b/openfeature/client.py index 4adf2974..3168b2a2 100644 --- a/openfeature/client.py +++ b/openfeature/client.py @@ -23,13 +23,9 @@ from openfeature.hook import Hook, HookContext from openfeature.hook._hook_support import ( after_all_hooks, - after_all_hooks_async, after_hooks, - after_hooks_async, before_hooks, - before_hooks_async, error_hooks, - error_hooks_async, ) from openfeature.provider import FeatureProvider, ProviderStatus from openfeature.provider._registry import provider_registry @@ -673,7 +669,7 @@ async def evaluate_flag_details( # noqa: PLR0915 status = self.get_provider_status() if status == ProviderStatus.NOT_READY: - await error_hooks_async( + error_hooks( flag_type, hook_context, ProviderNotReadyError(), @@ -687,7 +683,7 @@ async def evaluate_flag_details( # noqa: PLR0915 error_code=ErrorCode.PROVIDER_NOT_READY, ) if status == ProviderStatus.FATAL: - await error_hooks_async( + error_hooks( flag_type, hook_context, ProviderFatalError(), @@ -706,7 +702,7 @@ async def evaluate_flag_details( # noqa: PLR0915 # Any resulting evaluation context from a before hook will overwrite # duplicate fields defined globally, on the client, or in the invocation. # Requirement 3.2.2, 4.3.4: API.context->client.context->invocation.context - invocation_context = await before_hooks_async( + invocation_context = before_hooks( flag_type, hook_context, merged_hooks, hook_hints ) @@ -726,7 +722,7 @@ async def evaluate_flag_details( # noqa: PLR0915 merged_context, ) - await after_hooks_async( + after_hooks( flag_type, hook_context, flag_evaluation, @@ -737,9 +733,7 @@ async def evaluate_flag_details( # noqa: PLR0915 return flag_evaluation except OpenFeatureError as err: - await error_hooks_async( - flag_type, hook_context, err, reversed_merged_hooks, hook_hints - ) + error_hooks(flag_type, hook_context, err, reversed_merged_hooks, hook_hints) return FlagEvaluationDetails( flag_key=flag_key, @@ -755,9 +749,7 @@ async def evaluate_flag_details( # noqa: PLR0915 "Unable to correctly evaluate flag with key: '%s'", flag_key ) - await error_hooks_async( - flag_type, hook_context, err, reversed_merged_hooks, hook_hints - ) + error_hooks(flag_type, hook_context, err, reversed_merged_hooks, hook_hints) error_message = getattr(err, "error_message", str(err)) return FlagEvaluationDetails( @@ -769,9 +761,7 @@ async def evaluate_flag_details( # noqa: PLR0915 ) finally: - await after_all_hooks_async( - flag_type, hook_context, reversed_merged_hooks, hook_hints - ) + after_all_hooks(flag_type, hook_context, reversed_merged_hooks, hook_hints) async def _create_provider_evaluation( self, diff --git a/openfeature/hook/__init__.py b/openfeature/hook/__init__.py index 546d8cab..e98301fa 100644 --- a/openfeature/hook/__init__.py +++ b/openfeature/hook/__init__.py @@ -128,66 +128,3 @@ def supports_flag_value_type(self, flag_type: FlagType) -> bool: or not (False) """ return True - -class AsyncHook: - async def before( - self, hook_context: HookContext, hints: HookHints - ) -> typing.Optional[EvaluationContext]: - """ - Runs before flag is resolved. - - :param hook_context: Information about the particular flag evaluation - :param hints: An immutable mapping of data for users to - communicate to the hooks. - :return: An EvaluationContext. It will be merged with the - EvaluationContext instances from other hooks, the client and API. - """ - return None - - async def after( - self, - hook_context: HookContext, - details: FlagEvaluationDetails[typing.Any], - hints: HookHints, - ) -> None: - """ - Runs after a flag is resolved. - - :param hook_context: Information about the particular flag evaluation - :param details: Information about how the flag was resolved, - including any resolved values. - :param hints: A mapping of data for users to communicate to the hooks. - """ - pass - - async def error( - self, hook_context: HookContext, exception: Exception, hints: HookHints - ) -> None: - """ - Run when evaluation encounters an error. Errors thrown will be swallowed. - - :param hook_context: Information about the particular flag evaluation - :param exception: The exception that was thrown - :param hints: A mapping of data for users to communicate to the hooks. - """ - pass - - async def finally_after(self, hook_context: HookContext, hints: HookHints) -> None: - """ - Run after flag evaluation, including any error processing. - This will always run. Errors will be swallowed. - - :param hook_context: Information about the particular flag evaluation - :param hints: A mapping of data for users to communicate to the hooks. - """ - pass - - def supports_flag_value_type(self, flag_type: FlagType) -> bool: - """ - Check to see if the hook supports the particular flag type. - - :param flag_type: particular type of the flag - :return: a boolean containing whether the flag type is supported (True) - or not (False) - """ - return True diff --git a/openfeature/hook/_hook_support.py b/openfeature/hook/_hook_support.py index ce817b36..349b25f3 100644 --- a/openfeature/hook/_hook_support.py +++ b/openfeature/hook/_hook_support.py @@ -22,19 +22,6 @@ def error_hooks( ) -async def error_hooks_async( - flag_type: FlagType, - hook_context: HookContext, - exception: Exception, - hooks: typing.List[Hook], - hints: typing.Optional[HookHints] = None, -) -> None: - kwargs = {"hook_context": hook_context, "exception": exception, "hints": hints} - await _execute_hooks_async( - flag_type=flag_type, hooks=hooks, hook_method=HookType.ERROR, **kwargs - ) - - def after_all_hooks( flag_type: FlagType, hook_context: HookContext, @@ -47,18 +34,6 @@ def after_all_hooks( ) -async def after_all_hooks_async( - flag_type: FlagType, - hook_context: HookContext, - hooks: typing.List[Hook], - hints: typing.Optional[HookHints] = None, -) -> None: - kwargs = {"hook_context": hook_context, "hints": hints} - await _execute_hooks_async( - flag_type=flag_type, hooks=hooks, hook_method=HookType.FINALLY_AFTER, **kwargs - ) - - def after_hooks( flag_type: FlagType, hook_context: HookContext, @@ -72,19 +47,6 @@ def after_hooks( ) -async def after_hooks_async( - 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, "details": details, "hints": hints} - await _execute_hooks_async_unchecked( - flag_type=flag_type, hooks=hooks, hook_method=HookType.AFTER, **kwargs - ) - - def before_hooks( flag_type: FlagType, hook_context: HookContext, @@ -103,23 +65,6 @@ def before_hooks( return EvaluationContext() -async def before_hooks_async( - flag_type: FlagType, - hook_context: HookContext, - hooks: typing.List[Hook], - hints: typing.Optional[HookHints] = None, -) -> EvaluationContext: - kwargs = {"hook_context": hook_context, "hints": hints} - executed_hooks = await _execute_hooks_async( - flag_type=flag_type, hooks=hooks, hook_method=HookType.BEFORE, **kwargs - ) - filtered_hooks = [result for result in executed_hooks if result is not None] - if filtered_hooks: - return reduce(lambda a, b: a.merge(b), filtered_hooks) - - return EvaluationContext() - - def _execute_hooks( flag_type: FlagType, hooks: typing.List[Hook], @@ -143,29 +88,6 @@ def _execute_hooks( ] -async def _execute_hooks_async( - flag_type: FlagType, - hooks: typing.List[Hook], - hook_method: HookType, - **kwargs: typing.Any, -) -> list: - """ - Run multiple hooks of any hook type. All of these hooks will be run through an - exception check. - - :param flag_type: particular type of flag - :param hooks: a list of hooks - :param hook_method: the type of hook that is being run - :param kwargs: arguments that need to be provided to the hook method - :return: a list of results from the applied hook methods - """ - return [ - await _execute_hook_checked_async(hook, hook_method, **kwargs) - for hook in hooks - if hook.supports_flag_value_type(flag_type) - ] - - def _execute_hooks_unchecked( flag_type: FlagType, hooks: typing.List[Hook], @@ -190,30 +112,6 @@ def _execute_hooks_unchecked( ] -async def _execute_hooks_async_unchecked( - flag_type: FlagType, - hooks: typing.List[Hook], - hook_method: HookType, - **kwargs: typing.Any, -) -> typing.List[typing.Optional[EvaluationContext]]: - """ - Execute a single hook without checking whether an exception is thrown. This is - used in the before and after hooks since any exception will be caught in the - client. - - :param flag_type: particular type of flag - :param hooks: a list of hooks - :param hook_method: the type of hook that is being run - :param kwargs: arguments that need to be provided to the hook method - :return: a list of results from the applied hook methods - """ - return [ - await getattr(hook, hook_method.value)(**kwargs) - for hook in hooks - if hook.supports_flag_value_type(flag_type) - ] - - def _execute_hook_checked( hook: Hook, hook_method: HookType, **kwargs: typing.Any ) -> typing.Optional[EvaluationContext]: @@ -234,25 +132,3 @@ def _execute_hook_checked( except Exception: # pragma: no cover logger.exception(f"Exception when running {hook_method.value} hooks") return None - - -async def _execute_hook_checked_async( - hook: Hook, hook_method: HookType, **kwargs: typing.Any -) -> typing.Optional[EvaluationContext]: - """ - Try and run a single hook and catch any exception thrown. This is used in the - after all and error hooks since any error thrown at this point needs to be caught. - - :param hook: a list of hooks - :param hook_method: the type of hook that is being run - :param kwargs: arguments that need to be provided to the hook method - :return: the result of the hook method - """ - try: - return typing.cast( - "typing.Optional[EvaluationContext]", - await getattr(hook, hook_method.value)(**kwargs), - ) - except Exception: # pragma: no cover - logger.exception(f"Exception when running {hook_method.value} hooks") - return None diff --git a/tests/hook/conftest.py b/tests/hook/conftest.py index bf4af84e..5a3d8092 100644 --- a/tests/hook/conftest.py +++ b/tests/hook/conftest.py @@ -3,7 +3,6 @@ import pytest from openfeature.evaluation_context import EvaluationContext -from openfeature.hook import AsyncHook @pytest.fixture() @@ -15,14 +14,3 @@ def mock_hook(): mock_hook.error.return_value = None mock_hook.finally_after.return_value = None return mock_hook - - -@pytest.fixture() -def mock_hook_async(): - mock_hook = AsyncHook() - mock_hook.supports_flag_value_type = mock.MagicMock(return_value=True) - mock_hook.before = mock.AsyncMock(return_value=None) - mock_hook.after = mock.AsyncMock(return_value=None) - mock_hook.error = mock.AsyncMock(return_value=None) - mock_hook.finally_after = mock.AsyncMock(return_value=None) - return mock_hook diff --git a/tests/hook/test_hook_support.py b/tests/hook/test_hook_support.py index 49997107..7dec419d 100644 --- a/tests/hook/test_hook_support.py +++ b/tests/hook/test_hook_support.py @@ -8,13 +8,9 @@ from openfeature.hook import AsyncHook, Hook, HookContext from openfeature.hook._hook_support import ( after_all_hooks, - after_all_hooks_async, after_hooks, - after_hooks_async, before_hooks, - before_hooks_async, error_hooks, - error_hooks_async, ) from openfeature.immutable_dict.mapping_proxy_type import MappingProxyType from openfeature.provider.metadata import Metadata @@ -119,23 +115,6 @@ def test_before_hooks_run_before_method(mock_hook): mock_hook.before.assert_called_with(hook_context=hook_context, hints=hook_hints) -@pytest.mark.asyncio -async def test_before_hooks_run_before_method_async(mock_hook_async): - # Given - hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "") - hook_hints = MappingProxyType({}) - # When - await before_hooks_async( - FlagType.BOOLEAN, hook_context, [mock_hook_async], hook_hints - ) - # Then - mock_hook_async.supports_flag_value_type.assert_called_once() - mock_hook_async.before.assert_called_once() - mock_hook_async.before.assert_called_with( - hook_context=hook_context, hints=hook_hints - ) - - def test_before_hooks_merges_evaluation_contexts(): # Given hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "") @@ -153,25 +132,6 @@ def test_before_hooks_merges_evaluation_contexts(): assert context == EvaluationContext("bar", {"key_1": "val_1", "key_2": "val_2"}) -@pytest.mark.asyncio -async def test_before_hooks_async_merges_evaluation_contexts(): - # Given - hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "") - hook_1 = AsyncHook() - hook_1.before = AsyncMock(return_value=EvaluationContext("foo", {"key_1": "val_1"})) - hook_2 = AsyncHook() - hook_2.before = AsyncMock(return_value=EvaluationContext("bar", {"key_2": "val_2"})) - hook_3 = AsyncHook() - hook_3.before = AsyncMock(return_value=None) - # When - context = await before_hooks_async( - FlagType.BOOLEAN, hook_context, [hook_1, hook_2, hook_3] - ) - - # Then - assert context == EvaluationContext("bar", {"key_1": "val_1", "key_2": "val_2"}) - - def test_after_hooks_run_after_method(mock_hook): # Given hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "") @@ -191,30 +151,6 @@ def test_after_hooks_run_after_method(mock_hook): ) -@pytest.mark.asyncio -async def test_after_hooks_run_after_method_async(mock_hook_async): - # Given - hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "") - flag_evaluation_details = FlagEvaluationDetails( - hook_context.flag_key, "val", "unknown" - ) - hook_hints = MappingProxyType({}) - # When - await after_hooks_async( - FlagType.BOOLEAN, - hook_context, - flag_evaluation_details, - [mock_hook_async], - hook_hints, - ) - # Then - mock_hook_async.supports_flag_value_type.assert_called_once() - mock_hook_async.after.assert_called_once() - mock_hook_async.after.assert_called_with( - hook_context=hook_context, details=flag_evaluation_details, hints=hook_hints - ) - - def test_finally_after_hooks_run_finally_after_method(mock_hook): # Given hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "") @@ -227,20 +163,3 @@ def test_finally_after_hooks_run_finally_after_method(mock_hook): mock_hook.finally_after.assert_called_with( hook_context=hook_context, hints=hook_hints ) - - -@pytest.mark.asyncio -async def test_finally_after_hooks_run_finally_after_method_async(mock_hook_async): - # Given - hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "") - hook_hints = MappingProxyType({}) - # When - await after_all_hooks_async( - FlagType.BOOLEAN, hook_context, [mock_hook_async], hook_hints - ) - # Then - mock_hook_async.supports_flag_value_type.assert_called_once() - mock_hook_async.finally_after.assert_called_once() - mock_hook_async.finally_after.assert_called_with( - hook_context=hook_context, hints=hook_hints - ) diff --git a/tests/test_async_client.py b/tests/test_async_client.py index 97a11bc7..49fc3355 100644 --- a/tests/test_async_client.py +++ b/tests/test_async_client.py @@ -1,7 +1,7 @@ import time import uuid from concurrent.futures import ThreadPoolExecutor -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import MagicMock import pytest @@ -9,16 +9,12 @@ from openfeature.client import AsyncOpenFeatureClient from openfeature.event import ProviderEvent, ProviderEventDetails from openfeature.exception import ErrorCode, OpenFeatureError -from openfeature.flag_evaluation import FlagResolutionDetails, Reason -from openfeature.hook import AsyncHook -from openfeature.provider import FeatureProvider, ProviderStatus +from openfeature.flag_evaluation import Reason +from openfeature.hook import Hook +from openfeature.provider import ProviderStatus from openfeature.provider.in_memory_provider import AsyncInMemoryProvider, InMemoryFlag from openfeature.provider.no_op_provider import NoOpProvider -async_hook = MagicMock(spec=AsyncHook) -async_hook.before = AsyncMock(return_value=None) -async_hook.after = AsyncMock(return_value=None) - @pytest.mark.parametrize( "default, variants, get_method, expected_value", @@ -40,7 +36,8 @@ async def test_flag_resolution_to_evaluation_details_async( default, variants, get_method, expected_value, clear_hooks_fixture ): # Given - add_hooks([async_hook]) + api_hook = MagicMock(spec=Hook) + add_hooks([api_hook]) provider = AsyncInMemoryProvider( { "Key": InMemoryFlag( @@ -52,7 +49,7 @@ async def test_flag_resolution_to_evaluation_details_async( ) set_provider(provider, "my-async-client") client = AsyncOpenFeatureClient("my-async-client", None) - client.add_hooks([async_hook]) + client.add_hooks([api_hook]) # When details = await getattr(client, f"{get_method}_details")( flag_key="Key", default_value=None @@ -144,8 +141,8 @@ async def test_should_shortcircuit_if_provider_is_not_ready( "get_provider_status", lambda: provider_status, ) - spy_hook = MagicMock(spec=AsyncHook) - spy_hook.before = AsyncMock(return_value=None) + spy_hook = MagicMock(spec=Hook) + spy_hook.before.return_value = None no_op_provider_client_async.add_hooks([spy_hook]) # When flag_details = await no_op_provider_client_async.get_boolean_details( @@ -177,9 +174,9 @@ async def test_handle_an_open_feature_exception_thrown_by_a_provider_async( no_op_provider_client_async, ): # Given - exception_hook = AsyncHook() - exception_hook.after = AsyncMock( - side_effect=OpenFeatureError(ErrorCode.GENERAL, "error_message") + exception_hook = MagicMock(spec=Hook) + exception_hook.after.side_effect = OpenFeatureError( + ErrorCode.GENERAL, "error_message" ) no_op_provider_client_async.add_hooks([exception_hook])