fix(hook): propagate evaluation context to subsequent before hooks (#628)

openfeature/hook/_hook_support.py

@@ -1,6 +1,5 @@
 import logging
 import typing
-from functools import reduce
 
 from openfeature.evaluation_context import EvaluationContext
 from openfeature.flag_evaluation import FlagEvaluationDetails, FlagType
@@ -59,19 +58,44 @@ def before_hooks(
     hooks_and_context: list[tuple[Hook, HookContext]],
     hints: HookHints | None = None,
 ) -> EvaluationContext:
-    kwargs = {"hints": hints}
-    executed_hooks = _execute_hooks_unchecked(
-        flag_type=flag_type,
-        hooks_and_context=hooks_and_context,
-        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)
+    # Requirement 4.3.4: Any evaluation context returned from a before hook MUST be
+    # passed to subsequent before hooks (via HookContext).
+    accumulated: EvaluationContext | None = None
+    supported_hooks_and_context = [
+        (hook, hook_context)
+        for (hook, hook_context) in hooks_and_context
+        if hook.supports_flag_value_type(flag_type)
+    ]
 
-    return EvaluationContext()
+    try:
+        for hook, hook_context in supported_hooks_and_context:
+            if accumulated is not None:
+                # Propagate the accumulated context into this hook's HookContext so that
+                # it can observe the evaluation context returned by earlier before hooks.
+                if isinstance(hook_context.evaluation_context, EvaluationContext):
+                    hook_context.evaluation_context = (
+                        hook_context.evaluation_context.merge(accumulated)
+                    )
+                else:
+                    hook_context.evaluation_context = accumulated
+
+            result = hook.before(hook_context=hook_context, hints=hints or {})
+
+            if isinstance(result, EvaluationContext):
+                accumulated = (
+                    accumulated.merge(result) if accumulated is not None else result
+                )
+    finally:
+        if accumulated is not None:
+            for _, hook_context in supported_hooks_and_context:
+                if isinstance(hook_context.evaluation_context, EvaluationContext):
+                    hook_context.evaluation_context = (
+                        hook_context.evaluation_context.merge(accumulated)
+                    )
+                else:
+                    hook_context.evaluation_context = accumulated
+
+    return accumulated if accumulated is not None else EvaluationContext()
 
 
 def _execute_hooks(

tests/hook/test_hook_support.py

@@ -122,6 +122,225 @@ def test_before_hooks_merges_evaluation_contexts():
     assert context == EvaluationContext("bar", {"key_1": "val_1", "key_2": "val_2"})
 
 
+def test_before_hooks_propagates_context_to_subsequent_hook():
+    # Given
+    initial_context = EvaluationContext(attributes={"initial": "present"})
+    hook_context_a = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+    hook_context_b = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+
+    received_by_hook_b: list[EvaluationContext] = []
+
+    hook_a = MagicMock(spec=Hook)
+    hook_a.before.return_value = EvaluationContext(
+        attributes={"from_hook_a": "visible"}
+    )
+
+    hook_b = MagicMock(spec=Hook)
+
+    def hook_b_before(hook_context, hints):
+        received_by_hook_b.append(hook_context.evaluation_context)
+        return None
+
+    hook_b.before.side_effect = hook_b_before
+
+    # When
+    before_hooks(
+        FlagType.BOOLEAN,
+        [(hook_a, hook_context_a), (hook_b, hook_context_b)],
+    )
+
+    # Then
+    assert len(received_by_hook_b) == 1
+    assert received_by_hook_b[0].attributes.get("from_hook_a") == "visible", (
+        "Hook B did not receive the evaluation context returned by Hook A"
+    )
+    assert received_by_hook_b[0].attributes.get("initial") == "present"
+
+
+def test_before_hooks_accumulates_context_across_three_hooks():
+    # Given
+    initial_context = EvaluationContext(attributes={"initial": "present"})
+    ctx_a = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+    ctx_b = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+    ctx_c = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+
+    received_by_hook_b: list[EvaluationContext] = []
+    received_by_hook_c: list[EvaluationContext] = []
+
+    hook_a = MagicMock(spec=Hook)
+    hook_a.before.return_value = EvaluationContext(attributes={"from_a": "A"})
+
+    hook_b = MagicMock(spec=Hook)
+
+    def hook_b_before(hook_context, hints):
+        received_by_hook_b.append(hook_context.evaluation_context)
+        return EvaluationContext(attributes={"from_b": "B"})
+
+    hook_b.before.side_effect = hook_b_before
+
+    hook_c = MagicMock(spec=Hook)
+
+    def hook_c_before(hook_context, hints):
+        received_by_hook_c.append(hook_context.evaluation_context)
+        return None
+
+    hook_c.before.side_effect = hook_c_before
+
+    # When
+    before_hooks(
+        FlagType.BOOLEAN,
+        [(hook_a, ctx_a), (hook_b, ctx_b), (hook_c, ctx_c)],
+    )
+
+    # Then
+    assert received_by_hook_b[0].attributes.get("from_a") == "A", (
+        "Hook B did not receive the evaluation context returned by Hook A"
+    )
+    assert received_by_hook_c[0].attributes.get("from_a") == "A", (
+        "Hook C did not receive the evaluation context returned by Hook A"
+    )
+    assert received_by_hook_c[0].attributes.get("from_b") == "B", (
+        "Hook C did not receive the evaluation context returned by Hook B"
+    )
+
+
+def test_before_hooks_later_hook_overrides_earlier_on_conflict():
+    # Given
+    initial_context = EvaluationContext(attributes={"initial": "present"})
+    ctx_a = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+    ctx_b = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+    ctx_c = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+
+    received_by_hook_c: list[EvaluationContext] = []
+
+    hook_a = MagicMock(spec=Hook)
+    hook_a.before.return_value = EvaluationContext(attributes={"shared": "A"})
+
+    hook_b = MagicMock(spec=Hook)
+    hook_b.before.return_value = EvaluationContext(attributes={"shared": "B"})
+
+    hook_c = MagicMock(spec=Hook)
+
+    def hook_c_before(hook_context, hints):
+        received_by_hook_c.append(hook_context.evaluation_context)
+        return None
+
+    hook_c.before.side_effect = hook_c_before
+
+    # When
+    before_hooks(
+        FlagType.BOOLEAN,
+        [(hook_a, ctx_a), (hook_b, ctx_b), (hook_c, ctx_c)],
+    )
+
+    # Then
+    assert received_by_hook_c[0].attributes.get("shared") == "B", (
+        "Later hook (B) result should override earlier hook (A) result for the same attribute"
+    )
+
+
+def test_before_hooks_none_result_does_not_corrupt_accumulated_context():
+    # Given
+    initial_context = EvaluationContext(attributes={"initial": "present"})
+    ctx_a = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+    ctx_b = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+    ctx_c = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+
+    received_by_hook_c: list[EvaluationContext] = []
+
+    hook_a = MagicMock(spec=Hook)
+    hook_a.before.return_value = EvaluationContext(attributes={"from_a": "A"})
+
+    hook_b = MagicMock(spec=Hook)
+    hook_b.before.return_value = None
+
+    hook_c = MagicMock(spec=Hook)
+
+    def hook_c_before(hook_context, hints):
+        received_by_hook_c.append(hook_context.evaluation_context)
+        return None
+
+    hook_c.before.side_effect = hook_c_before
+
+    # When
+    before_hooks(
+        FlagType.BOOLEAN,
+        [(hook_a, ctx_a), (hook_b, ctx_b), (hook_c, ctx_c)],
+    )
+
+    # Then
+    assert received_by_hook_c[0].attributes.get("from_a") == "A", (
+        "Hook C should still see Hook A's context even though Hook B returned None"
+    )
+
+
+def test_before_hooks_finalizes_hook_contexts_for_all_participating_hooks():
+    # Given
+    initial_context = EvaluationContext(attributes={"initial": "present"})
+    ctx_a = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+    ctx_b = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+
+    hook_a = MagicMock(spec=Hook)
+    hook_a.before.return_value = EvaluationContext(attributes={"from_a": "A"})
+    hook_b = MagicMock(spec=Hook)
+    hook_b.before.return_value = EvaluationContext(attributes={"from_b": "B"})
+
+    # When
+    before_hooks(FlagType.BOOLEAN, [(hook_a, ctx_a), (hook_b, ctx_b)])
+
+    # Then
+    expected = {"initial": "present", "from_a": "A", "from_b": "B"}
+    assert ctx_a.evaluation_context.attributes == expected
+    assert ctx_b.evaluation_context.attributes == expected
+
+
+def test_before_hooks_finalizes_hook_contexts_on_exception():
+    # Given
+    initial_context = EvaluationContext(attributes={"initial": "present"})
+    ctx_a = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+    ctx_b = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+    ctx_c = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+
+    hook_a = MagicMock(spec=Hook)
+    hook_a.before.return_value = EvaluationContext(attributes={"from_a": "A"})
+    hook_b = MagicMock(spec=Hook)
+    hook_b.before.side_effect = RuntimeError("hook_b error")
+    hook_c = MagicMock(spec=Hook)
+
+    # When
+    with pytest.raises(RuntimeError, match="hook_b error"):
+        before_hooks(
+            FlagType.BOOLEAN, [(hook_a, ctx_a), (hook_b, ctx_b), (hook_c, ctx_c)]
+        )
+
+    # Then
+    expected = {"initial": "present", "from_a": "A"}
+    assert hook_c.before.call_count == 0
+    assert ctx_a.evaluation_context.attributes == expected
+    assert ctx_b.evaluation_context.attributes == expected
+    assert ctx_c.evaluation_context.attributes == expected
+
+
+def test_before_hooks_unsupported_hook_context_is_not_finalized():
+    # Given
+    initial_context = EvaluationContext(attributes={"initial": "present"})
+    ctx_a = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+    ctx_b = HookContext("flag_key", FlagType.BOOLEAN, True, initial_context)
+
+    hook_a = MagicMock(spec=Hook)
+    hook_a.before.return_value = EvaluationContext(attributes={"from_a": "A"})
+    hook_b = MagicMock(spec=Hook)
+    hook_b.supports_flag_value_type.return_value = False
+
+    # When
+    before_hooks(FlagType.BOOLEAN, [(hook_a, ctx_a), (hook_b, ctx_b)])
+
+    # Then
+    assert hook_b.before.call_count == 0
+    assert ctx_a.evaluation_context.attributes.get("from_a") == "A"
+    assert ctx_b.evaluation_context.attributes.get("from_a") is None
+
+
 def test_after_hooks_run_after_method(mock_hook):
     # Given
     hook_context = HookContext("flag_key", FlagType.BOOLEAN, True, "")