diff --git a/src/main/java/dev/openfeature/sdk/HookContext.java b/src/main/java/dev/openfeature/sdk/HookContext.java index e14eeb643..b06d2e3fa 100644 --- a/src/main/java/dev/openfeature/sdk/HookContext.java +++ b/src/main/java/dev/openfeature/sdk/HookContext.java @@ -11,7 +11,7 @@ * @param the type for the flag being evaluated */ @Value -@Builder +@Builder(toBuilder = true) @With public class HookContext { @NonNull String flagKey; @@ -25,6 +25,12 @@ public class HookContext { ClientMetadata clientMetadata; Metadata providerMetadata; + /** + * Hook data provides a way for hooks to maintain state across their execution stages. + * Each hook instance gets its own isolated data store. + */ + HookData hookData; + /** * Builds a {@link HookContext} instances from request data. * @@ -51,6 +57,7 @@ public static HookContext from( .providerMetadata(providerMetadata) .ctx(ctx) .defaultValue(defaultValue) + .hookData(null) // Explicitly set to null for backward compatibility .build(); } } diff --git a/src/main/java/dev/openfeature/sdk/HookData.java b/src/main/java/dev/openfeature/sdk/HookData.java new file mode 100644 index 000000000..e952992d1 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/HookData.java @@ -0,0 +1,76 @@ +package dev.openfeature.sdk; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +/** + * Hook data provides a way for hooks to maintain state across their execution stages. + * Each hook instance gets its own isolated data store that persists only for the duration + * of a single flag evaluation. + */ +public interface HookData { + + /** + * Sets a value for the given key. + * + * @param key the key to store the value under + * @param value the value to store + */ + void set(String key, Object value); + + /** + * Gets the value for the given key. + * + * @param key the key to retrieve the value for + * @return the value, or null if not found + */ + Object get(String key); + + /** + * Gets the value for the given key, cast to the specified type. + * + * @param the type to cast to + * @param key the key to retrieve the value for + * @param type the class to cast to + * @return the value cast to the specified type, or null if not found + * @throws ClassCastException if the value cannot be cast to the specified type + */ + T get(String key, Class type); + + /** + * Default implementation using ConcurrentHashMap for thread safety. + */ + static HookData create() { + return new DefaultHookData(); + } + + /** + * Default thread-safe implementation of HookData. + */ + public class DefaultHookData implements HookData { + private final Map data = Collections.synchronizedMap(new HashMap<>()); + + @Override + public void set(String key, Object value) { + data.put(key, value); + } + + @Override + public Object get(String key) { + return data.get(key); + } + + @Override + public T get(String key, Class type) { + Object value = data.get(key); + if (value == null) { + return null; + } + if (!type.isInstance(value)) { + throw new ClassCastException("Value for key '" + key + "' is not of type " + type.getName()); + } + return type.cast(value); + } + } +} diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java index 73518ee8e..15cfd03c3 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ b/src/main/java/dev/openfeature/sdk/HookSupport.java @@ -2,6 +2,7 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -87,10 +88,21 @@ private EvaluationContext callBeforeHooks( List reversedHooks = new ArrayList<>(hooks); Collections.reverse(reversedHooks); EvaluationContext context = hookCtx.getCtx(); + + // Create hook data for each hook instance + Map hookDataMap = new HashMap<>(); + for (Hook hook : reversedHooks) { + if (hook.supportsFlagValueType(flagValueType)) { + hookDataMap.put(hook, HookData.create()); + } + } + for (Hook hook : reversedHooks) { if (hook.supportsFlagValueType(flagValueType)) { - Optional optional = - Optional.ofNullable(hook.before(hookCtx, hints)).orElse(Optional.empty()); + // Create a new context with this hook's data + HookContext contextWithHookData = hookCtx.withHookData(hookDataMap.get(hook)); + Optional optional = Optional.ofNullable(hook.before(contextWithHookData, hints)) + .orElse(Optional.empty()); if (optional.isPresent()) { context = context.merge(optional.get()); } diff --git a/src/test/java/dev/openfeature/sdk/HookContextTest.java b/src/test/java/dev/openfeature/sdk/HookContextTest.java index 2196b8b1f..fcf9715e5 100644 --- a/src/test/java/dev/openfeature/sdk/HookContextTest.java +++ b/src/test/java/dev/openfeature/sdk/HookContextTest.java @@ -1,6 +1,6 @@ package dev.openfeature.sdk; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.mock; import org.junit.jupiter.api.Test; @@ -29,4 +29,52 @@ void metadata_field_is_type_metadata() { "The before stage MUST run before flag resolution occurs. It accepts a hook context (required) and hook hints (optional) as parameters. It has no return value.") @Test void not_applicable_for_dynamic_context() {} + + @Test + void shouldCreateHookContextWithHookData() { + HookData hookData = HookData.create(); + hookData.set("test", "value"); + + HookContext context = HookContext.builder() + .flagKey("test-flag") + .type(FlagValueType.STRING) + .defaultValue("default") + .ctx(new ImmutableContext()) + .hookData(hookData) + .build(); + + assertNotNull(context.getHookData()); + assertEquals("value", context.getHookData().get("test")); + } + + @Test + void shouldCreateHookContextWithoutHookData() { + HookContext context = HookContext.builder() + .flagKey("test-flag") + .type(FlagValueType.STRING) + .defaultValue("default") + .ctx(new ImmutableContext()) + .build(); + + assertNull(context.getHookData()); + } + + @Test + void shouldCreateHookContextWithHookDataUsingWith() { + HookContext originalContext = HookContext.builder() + .flagKey("test-flag") + .type(FlagValueType.STRING) + .defaultValue("default") + .ctx(new ImmutableContext()) + .build(); + + HookData hookData = HookData.create(); + hookData.set("timing", System.currentTimeMillis()); + + HookContext contextWithHookData = originalContext.withHookData(hookData); + + assertNull(originalContext.getHookData()); + assertNotNull(contextWithHookData.getHookData()); + assertNotNull(contextWithHookData.getHookData().get("timing")); + } } diff --git a/src/test/java/dev/openfeature/sdk/HookDataTest.java b/src/test/java/dev/openfeature/sdk/HookDataTest.java new file mode 100644 index 000000000..cc748fe54 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/HookDataTest.java @@ -0,0 +1,111 @@ +package dev.openfeature.sdk; + +import static org.junit.jupiter.api.Assertions.*; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.Test; + +class HookDataTest { + + @Test + void shouldStoreAndRetrieveValues() { + HookData hookData = HookData.create(); + + hookData.set("key1", "value1"); + hookData.set("key2", 42); + hookData.set("key3", true); + + assertEquals("value1", hookData.get("key1")); + assertEquals(42, hookData.get("key2")); + assertEquals(true, hookData.get("key3")); + } + + @Test + void shouldReturnNullForMissingKeys() { + HookData hookData = HookData.create(); + + assertNull(hookData.get("nonexistent")); + } + + @Test + void shouldSupportTypeSafeRetrieval() { + HookData hookData = HookData.create(); + + hookData.set("string", "hello"); + hookData.set("integer", 123); + hookData.set("boolean", false); + + assertEquals("hello", hookData.get("string", String.class)); + assertEquals(Integer.valueOf(123), hookData.get("integer", Integer.class)); + assertEquals(Boolean.FALSE, hookData.get("boolean", Boolean.class)); + } + + @Test + void shouldReturnNullForMissingKeysWithType() { + HookData hookData = HookData.create(); + + assertNull(hookData.get("missing", String.class)); + } + + @Test + void shouldThrowClassCastExceptionForWrongType() { + HookData hookData = HookData.create(); + + hookData.set("string", "not a number"); + + assertThrows(ClassCastException.class, () -> { + hookData.get("string", Integer.class); + }); + } + + @Test + void shouldOverwriteExistingValues() { + HookData hookData = HookData.create(); + + hookData.set("key", "original"); + assertEquals("original", hookData.get("key")); + + hookData.set("key", "updated"); + assertEquals("updated", hookData.get("key")); + } + + @Test + void shouldBeThreadSafe() throws InterruptedException { + HookData hookData = HookData.create(); + int threadCount = 10; + int operationsPerThread = 100; + CountDownLatch latch = new CountDownLatch(threadCount); + ExecutorService executor = Executors.newFixedThreadPool(threadCount); + + for (int i = 0; i < threadCount; i++) { + final int threadId = i; + executor.submit(() -> { + try { + for (int j = 0; j < operationsPerThread; j++) { + String key = "thread-" + threadId + "-key-" + j; + String value = "thread-" + threadId + "-value-" + j; + hookData.set(key, value); + assertEquals(value, hookData.get(key)); + } + } finally { + latch.countDown(); + } + }); + } + + assertTrue(latch.await(10, TimeUnit.SECONDS)); + executor.shutdown(); + } + + @Test + void shouldSupportNullValues() { + HookData hookData = HookData.create(); + + hookData.set("nullKey", null); + assertNull(hookData.get("nullKey")); + assertNull(hookData.get("nullKey", String.class)); + } +} diff --git a/src/test/java/dev/openfeature/sdk/HookSupportTest.java b/src/test/java/dev/openfeature/sdk/HookSupportTest.java index 02a8ff90c..8b51ea1cc 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -23,8 +23,14 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() { Map attributes = new HashMap<>(); attributes.put("baseKey", new Value("baseValue")); EvaluationContext baseContext = new ImmutableContext(attributes); - HookContext hookContext = new HookContext<>( - "flagKey", FlagValueType.STRING, "defaultValue", baseContext, () -> "client", () -> "provider"); + HookContext hookContext = HookContext.builder() + .flagKey("flagKey") + .type(FlagValueType.STRING) + .defaultValue("defaultValue") + .ctx(baseContext) + .clientMetadata(() -> "client") + .providerMetadata(() -> "provider") + .build(); Hook hook1 = mockStringHook(); Hook hook2 = mockStringHook(); when(hook1.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("bla", "blubber"))); @@ -47,13 +53,14 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) { HookSupport hookSupport = new HookSupport(); EvaluationContext baseContext = new ImmutableContext(); IllegalStateException expectedException = new IllegalStateException("All fine, just a test"); - HookContext hookContext = new HookContext<>( - "flagKey", - flagValueType, - createDefaultValue(flagValueType), - baseContext, - () -> "client", - () -> "provider"); + HookContext hookContext = HookContext.builder() + .flagKey("flagKey") + .type(flagValueType) + .defaultValue(createDefaultValue(flagValueType)) + .ctx(baseContext) + .clientMetadata(() -> "client") + .providerMetadata(() -> "provider") + .build(); hookSupport.beforeHooks( flagValueType, hookContext, Collections.singletonList(genericHook), Collections.emptyMap()); @@ -105,4 +112,33 @@ private EvaluationContext evaluationContextWithValue(String key, String value) { EvaluationContext baseContext = new ImmutableContext(attributes); return baseContext; } + + private static class TestHook implements Hook { + boolean beforeCalled = false; + boolean afterCalled = false; + boolean errorCalled = false; + boolean finallyCalled = false; + + @Override + public Optional before(HookContext ctx, Map hints) { + beforeCalled = true; + return Optional.empty(); + } + + @Override + public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { + afterCalled = true; + } + + @Override + public void error(HookContext ctx, Exception error, Map hints) { + errorCalled = true; + } + + @Override + public void finallyAfter( + HookContext ctx, FlagEvaluationDetails details, Map hints) { + finallyCalled = true; + } + } }