fix: Hook methods should have default non-abstract implementations (#216)

* fix: Hook methods should have default non-abstract implementations

Signed-off-by: Federico Bond <federicobond@gmail.com>

* fix: use correct return type for Hook.before method

Signed-off-by: Federico Bond <federicobond@gmail.com>

* feat: make EvaluationContext a dataclass

Signed-off-by: Federico Bond <federicobond@gmail.com>

* test: add unit test for evaluation context merging in before_hooks

Signed-off-by: Federico Bond <federicobond@gmail.com>

---------

Signed-off-by: Federico Bond <federicobond@gmail.com>
Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>

openfeature/evaluation_context.py

@@ -1,14 +1,11 @@
 import typing
+from dataclasses import dataclass, field
 
 
+@dataclass
 class EvaluationContext:
-    def __init__(
+    targeting_key: typing.Optional[str] = None
-        self,
+    attributes: dict = field(default_factory=dict)
-        targeting_key: typing.Optional[str] = None,
-        attributes: typing.Optional[dict] = None,
-    ):
-        self.targeting_key = targeting_key
-        self.attributes = attributes or {}
 
     def merge(self, ctx2: "EvaluationContext") -> "EvaluationContext":
         if not (self and ctx2):

openfeature/hook/__init__.py

@@ -1,7 +1,6 @@
 from __future__ import annotations
 
 import typing
-from abc import abstractmethod
 from dataclasses import dataclass
 from enum import Enum
 
@@ -27,8 +26,9 @@ class HookContext:
 
 
 class Hook:
-    @abstractmethod
+    def before(
-    def before(self, hook_context: HookContext, hints: dict) -> EvaluationContext:
+        self, hook_context: HookContext, hints: dict
+    ) -> typing.Optional[EvaluationContext]:
         """
         Runs before flag is resolved.
 
@@ -38,9 +38,8 @@ class Hook:
         :return: An EvaluationContext. It will be merged with the
         EvaluationContext instances from other hooks, the client and API.
         """
-        pass
+        return None
 
-    @abstractmethod
     def after(
         self, hook_context: HookContext, details: FlagEvaluationDetails, hints: dict
     ):
@@ -54,7 +53,6 @@ class Hook:
         """
         pass
 
-    @abstractmethod
     def error(self, hook_context: HookContext, exception: Exception, hints: dict):
         """
         Run when evaluation encounters an error. Errors thrown will be swallowed.
@@ -65,7 +63,6 @@ class Hook:
         """
         pass
 
-    @abstractmethod
     def finally_after(self, hook_context: HookContext, hints: dict):
         """
         Run after flag evaluation, including any error processing.
@@ -76,7 +73,6 @@ class Hook:
         """
         pass
 
-    @abstractmethod
     def supports_flag_value_type(self, flag_type: FlagType) -> bool:
         """
         Check to see if the hook supports the particular flag type.

tests/hook/test_hook_support.py

@@ -1,7 +1,8 @@
-from unittest.mock import ANY
+from unittest.mock import ANY, MagicMock
 
+from openfeature.evaluation_context import EvaluationContext
 from openfeature.flag_evaluation import FlagEvaluationDetails, FlagType
-from openfeature.hook import HookContext
+from openfeature.hook import Hook, HookContext
 from openfeature.hook.hook_support import (
     after_all_hooks,
     after_hooks,
@@ -37,6 +38,23 @@ def test_before_hooks_run_before_method(mock_hook):
     mock_hook.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, "")
+    hook_1 = MagicMock(spec=Hook)
+    hook_1.before.return_value = EvaluationContext("foo", {"key_1": "val_1"})
+    hook_2 = MagicMock(spec=Hook)
+    hook_2.before.return_value = EvaluationContext("bar", {"key_2": "val_2"})
+    hook_3 = MagicMock(spec=Hook)
+    hook_3.before.return_value = None
+
+    # When
+    context = before_hooks(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, "")