Skip to content

feat: Added hook data #1506

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/main/java/dev/openfeature/sdk/HookContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* @param <T> the type for the flag being evaluated
*/
@Value
@Builder
@Builder(toBuilder = true)
@With
public class HookContext<T> {
@NonNull String flagKey;
Expand All @@ -25,6 +25,12 @@ public class HookContext<T> {
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this addition, the Constructor of the HookContext changes, which may be a breaking change @toddbaert


/**
* Builds a {@link HookContext} instances from request data.
*
Expand All @@ -51,6 +57,7 @@ public static <T> HookContext<T> from(
.providerMetadata(providerMetadata)
.ctx(ctx)
.defaultValue(defaultValue)
.hookData(null) // Explicitly set to null for backward compatibility
.build();
}
}
76 changes: 76 additions & 0 deletions src/main/java/dev/openfeature/sdk/HookData.java
Original file line number Diff line number Diff line change
@@ -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 <T> 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> T get(String key, Class<T> type);

/**
* Default implementation using ConcurrentHashMap for thread safety.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DefaultHookData uses a Collections.synchronizedMap(new HashMap<>()), not a ConcurrentHashMap

*/
static HookData create() {
return new DefaultHookData();
}

/**
* Default thread-safe implementation of HookData.
*/
public class DefaultHookData implements HookData {
private final Map<String, Object> data = Collections.synchronizedMap(new HashMap<>());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a thread safe implementation? If the hook data only lives for the duraion of a flag evalution, there will probably not be concurrent access to this object. IIRC, a flag evaluation happens synchronously.


@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> T get(String key, Class<T> 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);
}
}
}
16 changes: 14 additions & 2 deletions src/main/java/dev/openfeature/sdk/HookSupport.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -87,10 +88,21 @@ private EvaluationContext callBeforeHooks(
List<Hook> reversedHooks = new ArrayList<>(hooks);
Collections.reverse(reversedHooks);
EvaluationContext context = hookCtx.getCtx();

// Create hook data for each hook instance
Map<Hook, HookData> 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<EvaluationContext> 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<EvaluationContext> optional = Optional.ofNullable(hook.before(contextWithHookData, hints))
.orElse(Optional.empty());
if (optional.isPresent()) {
context = context.merge(optional.get());
}
Expand Down
50 changes: 49 additions & 1 deletion src/test/java/dev/openfeature/sdk/HookContextTest.java
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<String> context = HookContext.<String>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<String> context = HookContext.<String>builder()
.flagKey("test-flag")
.type(FlagValueType.STRING)
.defaultValue("default")
.ctx(new ImmutableContext())
.build();

assertNull(context.getHookData());
}

@Test
void shouldCreateHookContextWithHookDataUsingWith() {
HookContext<String> originalContext = HookContext.<String>builder()
.flagKey("test-flag")
.type(FlagValueType.STRING)
.defaultValue("default")
.ctx(new ImmutableContext())
.build();

HookData hookData = HookData.create();
hookData.set("timing", System.currentTimeMillis());

HookContext<String> contextWithHookData = originalContext.withHookData(hookData);

assertNull(originalContext.getHookData());
assertNotNull(contextWithHookData.getHookData());
assertNotNull(contextWithHookData.getHookData().get("timing"));
}
}
111 changes: 111 additions & 0 deletions src/test/java/dev/openfeature/sdk/HookDataTest.java
Original file line number Diff line number Diff line change
@@ -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));
}
}
54 changes: 45 additions & 9 deletions src/test/java/dev/openfeature/sdk/HookSupportTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,14 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() {
Map<String, Value> attributes = new HashMap<>();
attributes.put("baseKey", new Value("baseValue"));
EvaluationContext baseContext = new ImmutableContext(attributes);
HookContext<String> hookContext = new HookContext<>(
"flagKey", FlagValueType.STRING, "defaultValue", baseContext, () -> "client", () -> "provider");
HookContext<String> hookContext = HookContext.<String>builder()
.flagKey("flagKey")
.type(FlagValueType.STRING)
.defaultValue("defaultValue")
.ctx(baseContext)
.clientMetadata(() -> "client")
.providerMetadata(() -> "provider")
.build();
Hook<String> hook1 = mockStringHook();
Hook<String> hook2 = mockStringHook();
when(hook1.before(any(), any())).thenReturn(Optional.of(evaluationContextWithValue("bla", "blubber")));
Expand All @@ -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<Object> hookContext = new HookContext<>(
"flagKey",
flagValueType,
createDefaultValue(flagValueType),
baseContext,
() -> "client",
() -> "provider");
HookContext<Object> hookContext = HookContext.<Object>builder()
.flagKey("flagKey")
.type(flagValueType)
.defaultValue(createDefaultValue(flagValueType))
.ctx(baseContext)
.clientMetadata(() -> "client")
.providerMetadata(() -> "provider")
.build();

hookSupport.beforeHooks(
flagValueType, hookContext, Collections.singletonList(genericHook), Collections.emptyMap());
Expand Down Expand Up @@ -105,4 +112,33 @@ private EvaluationContext evaluationContextWithValue(String key, String value) {
EvaluationContext baseContext = new ImmutableContext(attributes);
return baseContext;
}

private static class TestHook implements Hook<String> {
boolean beforeCalled = false;
boolean afterCalled = false;
boolean errorCalled = false;
boolean finallyCalled = false;

@Override
public Optional<EvaluationContext> before(HookContext<String> ctx, Map<String, Object> hints) {
beforeCalled = true;
return Optional.empty();
}

@Override
public void after(HookContext<String> ctx, FlagEvaluationDetails<String> details, Map<String, Object> hints) {
afterCalled = true;
}

@Override
public void error(HookContext<String> ctx, Exception error, Map<String, Object> hints) {
errorCalled = true;
}

@Override
public void finallyAfter(
HookContext<String> ctx, FlagEvaluationDetails<String> details, Map<String, Object> hints) {
finallyCalled = true;
}
}
}
Loading