From 13dbeeefeafcc75c15fe8e515c9ebf1675e228ad Mon Sep 17 00:00:00 2001 From: mdxabu Date: Sun, 6 Jul 2025 16:35:16 +0530 Subject: [PATCH 1/3] Added hook data Signed-off-by: mdxabu --- .../java/dev/openfeature/sdk/HookContext.java | 9 +- .../java/dev/openfeature/sdk/HookData.java | 75 ++++++++++++ .../java/dev/openfeature/sdk/HookSupport.java | 14 ++- .../dev/openfeature/sdk/HookContextTest.java | 50 +++++++- .../dev/openfeature/sdk/HookDataTest.java | 112 ++++++++++++++++++ .../dev/openfeature/sdk/HookSupportTest.java | 53 +++++++-- 6 files changed, 301 insertions(+), 12 deletions(-) create mode 100644 src/main/java/dev/openfeature/sdk/HookData.java create mode 100644 src/test/java/dev/openfeature/sdk/HookDataTest.java 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..5c22ea799 --- /dev/null +++ b/src/main/java/dev/openfeature/sdk/HookData.java @@ -0,0 +1,75 @@ +package dev.openfeature.sdk; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +/** + * 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. + */ + class DefaultHookData implements HookData { + private final ConcurrentMap data = new ConcurrentHashMap<>(); + + @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..df6115c57 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)) { + // Create a new context with this hook's data + HookContext contextWithHookData = hookCtx.withHookData(hookDataMap.get(hook)); Optional optional = - Optional.ofNullable(hook.before(hookCtx, hints)).orElse(Optional.empty()); + 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..d9e3dcc59 --- /dev/null +++ b/src/test/java/dev/openfeature/sdk/HookDataTest.java @@ -0,0 +1,112 @@ +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..2cf220de9 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,32 @@ 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; + } + } } From 549e97cd9fba8aef6b67d433a93077a96c24f11d Mon Sep 17 00:00:00 2001 From: mdxabu Date: Sun, 6 Jul 2025 17:37:06 +0530 Subject: [PATCH 2/3] to support null values while maintaining thread safety, you must avoid ConcurrentHashMap and use a synchronized HashMap instead Signed-off-by: mdxabu --- src/main/java/dev/openfeature/sdk/HookData.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/HookData.java b/src/main/java/dev/openfeature/sdk/HookData.java index 5c22ea799..a7429cc78 100644 --- a/src/main/java/dev/openfeature/sdk/HookData.java +++ b/src/main/java/dev/openfeature/sdk/HookData.java @@ -1,5 +1,8 @@ package dev.openfeature.sdk; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -47,8 +50,8 @@ static HookData create() { /** * Default thread-safe implementation of HookData. */ - class DefaultHookData implements HookData { - private final ConcurrentMap data = new ConcurrentHashMap<>(); + public class DefaultHookData implements HookData { + private final Map data = Collections.synchronizedMap(new HashMap<>()); @Override public void set(String key, Object value) { From c06c679967be28dfb1746cfe4d68d2477f595cfe Mon Sep 17 00:00:00 2001 From: mdxabu Date: Tue, 8 Jul 2025 15:35:52 +0530 Subject: [PATCH 3/3] Build successfully runned:spotless plugin Signed-off-by: mdxabu --- src/main/java/dev/openfeature/sdk/HookData.java | 2 -- src/main/java/dev/openfeature/sdk/HookSupport.java | 4 ++-- src/test/java/dev/openfeature/sdk/HookDataTest.java | 1 - src/test/java/dev/openfeature/sdk/HookSupportTest.java | 3 ++- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/HookData.java b/src/main/java/dev/openfeature/sdk/HookData.java index a7429cc78..e952992d1 100644 --- a/src/main/java/dev/openfeature/sdk/HookData.java +++ b/src/main/java/dev/openfeature/sdk/HookData.java @@ -3,8 +3,6 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; /** * Hook data provides a way for hooks to maintain state across their execution stages. diff --git a/src/main/java/dev/openfeature/sdk/HookSupport.java b/src/main/java/dev/openfeature/sdk/HookSupport.java index df6115c57..15cfd03c3 100644 --- a/src/main/java/dev/openfeature/sdk/HookSupport.java +++ b/src/main/java/dev/openfeature/sdk/HookSupport.java @@ -101,8 +101,8 @@ private EvaluationContext callBeforeHooks( if (hook.supportsFlagValueType(flagValueType)) { // 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()); + 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/HookDataTest.java b/src/test/java/dev/openfeature/sdk/HookDataTest.java index d9e3dcc59..cc748fe54 100644 --- a/src/test/java/dev/openfeature/sdk/HookDataTest.java +++ b/src/test/java/dev/openfeature/sdk/HookDataTest.java @@ -109,4 +109,3 @@ void shouldSupportNullValues() { 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 2cf220de9..8b51ea1cc 100644 --- a/src/test/java/dev/openfeature/sdk/HookSupportTest.java +++ b/src/test/java/dev/openfeature/sdk/HookSupportTest.java @@ -136,7 +136,8 @@ public void error(HookContext ctx, Exception error, Map } @Override - public void finallyAfter(HookContext ctx, FlagEvaluationDetails details, Map hints) { + public void finallyAfter( + HookContext ctx, FlagEvaluationDetails details, Map hints) { finallyCalled = true; } }