open-feature
/
java-sdk
mirror of https://github.com/open-feature/java-sdk.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix: updated context not passed to all hooks (#1049) 

* fix: updated context not passed to all hooks

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>

* fixup: checkstyle

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>

---------

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
This commit is contained in:
Todd Baert 2024-08-20 15:33:34 -04:00 committed by GitHub
parent 7cac1984f8
commit dbf967a860
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 72 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/main/java/dev/openfeature/sdk/ImmutableContext.java
View File

@ -73,7 +73,7 @@ public final class ImmutableContext implements EvaluationContext {
* Merges this EvaluationContext object with the passed EvaluationContext, overriding in case of conflict.
*
* @param overridingContext overriding context
* @return resulting merged context
* @return new, resulting merged context
*/
@Override
public EvaluationContext merge(EvaluationContext overridingContext) {

27
src/main/java/dev/openfeature/sdk/OpenFeatureClient.java
View File

@ -105,7 +105,7 @@ public class OpenFeatureClient implements Client {
FlagEvaluationDetails<T> details = null;
List<Hook> mergedHooks = null;
HookContext<T> hookCtx = null;
HookContext<T> afterHookContext = null;
FeatureProvider provider;
try {
@ -115,12 +115,11 @@ public class OpenFeatureClient implements Client {
mergedHooks = ObjectUtils.merge(provider.getProviderHooks(), flagOptions.getHooks(), clientHooks,
openfeatureApi.getHooks());
hookCtx = HookContext.from(key, type, this.getMetadata(),
provider.getMetadata(), ctx, defaultValue);
EvaluationContext mergedCtx = hookSupport.beforeHooks(type, HookContext.from(key, type, this.getMetadata(),
provider.getMetadata(), mergeEvaluationContext(ctx), defaultValue), mergedHooks, hints);
EvaluationContext ctxFromHook = hookSupport.beforeHooks(type, hookCtx, mergedHooks, hints);
EvaluationContext mergedCtx = mergeEvaluationContext(ctxFromHook, ctx);
afterHookContext = HookContext.from(key, type, this.getMetadata(),
provider.getMetadata(), mergedCtx, defaultValue);
ProviderEvaluation<T> providerEval = (ProviderEvaluation<T>) createProviderEvaluation(type, key,
defaultValue, provider, mergedCtx);
@ -129,7 +128,7 @@ public class OpenFeatureClient implements Client {
if (details.getErrorCode() != null) {
throw ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage());
} else {
hookSupport.afterHooks(type, hookCtx, details, mergedHooks, hints);
hookSupport.afterHooks(type, afterHookContext, details, mergedHooks, hints);
}
} catch (Exception e) {
log.error("Unable to correctly evaluate flag with key '{}'", key, e);
@ -144,24 +143,22 @@ public class OpenFeatureClient implements Client {
details.setErrorMessage(e.getMessage());
details.setValue(defaultValue);
details.setReason(Reason.ERROR.toString());
hookSupport.errorHooks(type, hookCtx, e, mergedHooks, hints);
hookSupport.errorHooks(type, afterHookContext, e, mergedHooks, hints);
} finally {
hookSupport.afterAllHooks(type, hookCtx, mergedHooks, hints);
hookSupport.afterAllHooks(type, afterHookContext, mergedHooks, hints);
}
return details;
}
/**
* Merge hook and invocation contexts with API, transaction and client contexts.
* Merge invocation contexts with API, transaction and client contexts.
* Does not merge before context.
*
* @param hookContext hook context
* @param invocationContext invocation context
* @return merged evaluation context
*/
private EvaluationContext mergeEvaluationContext(
EvaluationContext hookContext,
EvaluationContext invocationContext) {
private EvaluationContext mergeEvaluationContext(EvaluationContext invocationContext) {
final EvaluationContext apiContext = openfeatureApi.getEvaluationContext() != null
? openfeatureApi.getEvaluationContext()
: new ImmutableContext();
@ -172,7 +169,7 @@ public class OpenFeatureClient implements Client {
? openfeatureApi.getTransactionContext()
: new ImmutableContext();
return apiContext.merge(transactionContext.merge(clientContext.merge(invocationContext.merge(hookContext))));
return apiContext.merge(transactionContext.merge(clientContext.merge(invocationContext)));
}
private <T> ProviderEvaluation<?> createProviderEvaluation(

56
src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java
View File

@ -2,7 +2,6 @@ package dev.openfeature.sdk;
import static dev.openfeature.sdk.DoSomethingProvider.DEFAULT_METADATA;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
@ -20,6 +19,7 @@ import static org.mockito.Mockito.verify;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import org.awaitility.Awaitility;
import org.junit.jupiter.api.AfterEach;
@ -336,11 +336,25 @@ class FlagEvaluationSpecTest implements HookFixtures {
FeatureProviderTestUtils.setFeatureProvider(provider);
TransactionContextPropagator transactionContextPropagator = new ThreadLocalTransactionContextPropagator();
api.setTransactionContextPropagator(transactionContextPropagator);
Hook<Boolean> hook = spy(new Hook<Boolean>() {
@Override
public Optional<EvaluationContext> before(HookContext<Boolean> ctx, Map<String, Object> hints) {
Map<String, Value> attrs = ctx.getCtx().asMap();
attrs.put("before", new Value("5"));
attrs.put("common7", new Value("5"));
return Optional.ofNullable(new ImmutableContext(attrs));
}
@Override
public void after(HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
Hook.super.after(ctx, details, hints);
}
});
Map<String, Value> apiAttributes = new HashMap<>();
apiAttributes.put("common1", new Value("1"));
apiAttributes.put("common2", new Value("1"));
apiAttributes.put("common3", new Value("1"));
apiAttributes.put("common7", new Value("1"));
apiAttributes.put("api", new Value("1"));
EvaluationContext apiCtx = new ImmutableContext(apiAttributes);
@ -377,21 +391,55 @@ class FlagEvaluationSpecTest implements HookFixtures {
invocationAttributes.put("invocation", new Value("4"));
EvaluationContext invocationCtx = new ImmutableContext(invocationAttributes);
c.getBooleanValue("key", false, invocationCtx);
c.getBooleanValue("key", false, invocationCtx, FlagEvaluationOptions.builder().hook(hook).build());
// assert the connect overrides
// assert the correct overrides in before hook
verify(hook).before(argThat((arg) -> {
EvaluationContext evaluationContext = arg.getCtx();
return evaluationContext.getValue("api").asString().equals("1") &&
evaluationContext.getValue("transaction").asString().equals("2") &&
evaluationContext.getValue("client").asString().equals("3") &&
evaluationContext.getValue("invocation").asString().equals("4") &&
evaluationContext.getValue("common1").asString().equals("2") &&
evaluationContext.getValue("common2").asString().equals("3") &&
evaluationContext.getValue("common3").asString().equals("4") &&
evaluationContext.getValue("common4").asString().equals("3") &&
evaluationContext.getValue("common5").asString().equals("4") &&
evaluationContext.getValue("common6").asString().equals("4");
}), any());
// assert the correct overrides in evaluation
verify(provider).getBooleanEvaluation(any(), any(), argThat((arg) -> {
return arg.getValue("api").asString().equals("1") &&
arg.getValue("transaction").asString().equals("2") &&
arg.getValue("client").asString().equals("3") &&
arg.getValue("invocation").asString().equals("4") &&
arg.getValue("before").asString().equals("5") &&
arg.getValue("common1").asString().equals("2") &&
arg.getValue("common2").asString().equals("3") &&
arg.getValue("common3").asString().equals("4") &&
arg.getValue("common4").asString().equals("3") &&
arg.getValue("common5").asString().equals("4") &&
arg.getValue("common6").asString().equals("4");
arg.getValue("common6").asString().equals("4") &&
arg.getValue("common7").asString().equals("5");
}));
// assert the correct overrides in after hook
verify(hook).after(argThat((arg) -> {
EvaluationContext evaluationContext = arg.getCtx();
return evaluationContext.getValue("api").asString().equals("1") &&
evaluationContext.getValue("transaction").asString().equals("2") &&
evaluationContext.getValue("client").asString().equals("3") &&
evaluationContext.getValue("invocation").asString().equals("4") &&
evaluationContext.getValue("before").asString().equals("5") &&
evaluationContext.getValue("common1").asString().equals("2") &&
evaluationContext.getValue("common2").asString().equals("3") &&
evaluationContext.getValue("common3").asString().equals("4") &&
evaluationContext.getValue("common4").asString().equals("3") &&
evaluationContext.getValue("common5").asString().equals("4") &&
evaluationContext.getValue("common6").asString().equals("4") &&
evaluationContext.getValue("common7").asString().equals("5");
}), any(), any());
}
@Specification(number="3.3.1.1", text="The API SHOULD have a method for setting a transaction context propagator.")

13
src/test/java/dev/openfeature/sdk/HookSpecTest.java
View File

@ -484,10 +484,11 @@ class HookSpecTest implements HookFixtures {
@Specification(number = "4.1.4", text = "The evaluation context MUST be mutable only within the before hook.")
@Specification(number = "4.3.4", text = "Any `evaluation context` returned from a `before` hook MUST be passed to subsequent `before` hooks (via `HookContext`).")
@Test void beforeContextUpdated() {
EvaluationContext ctx = new ImmutableContext();
Hook hook = mockBooleanHook();
String targetingKey = "test-key";
EvaluationContext ctx = new ImmutableContext(targetingKey);
Hook<Boolean> hook = mockBooleanHook();
when(hook.before(any(), any())).thenReturn(Optional.of(ctx));
Hook hook2 = mockBooleanHook();
Hook<Boolean> hook2 = mockBooleanHook();
when(hook.before(any(), any())).thenReturn(Optional.empty());
InOrder order = inOrder(hook, hook2);
@ -499,11 +500,11 @@ class HookSpecTest implements HookFixtures {
.build());
order.verify(hook).before(any(), any());
ArgumentCaptor<HookContext> captor = ArgumentCaptor.forClass(HookContext.class);
ArgumentCaptor<HookContext<Boolean>> captor = ArgumentCaptor.forClass(HookContext.class);
order.verify(hook2).before(captor.capture(), any());
HookContext hc = captor.getValue();
assertEquals(hc.getCtx(), ctx);
HookContext<Boolean> hc = captor.getValue();
assertEquals(hc.getCtx().getTargetingKey(), targetingKey);
}