Merge pull request #5 from open-feature/eval-context

Evaluation Context support
This commit is contained in:
Justin Abrahms 2022-05-28 00:41:06 -07:00 committed by GitHub
commit b57e55436d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 239 additions and 25 deletions

View File

@ -1,4 +1,94 @@
package dev.openfeature.javasdk;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.Setter;
import lombok.ToString;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.util.HashMap;
import java.util.Map;
@ToString @EqualsAndHashCode
@SuppressWarnings("PMD.BeanMembersShouldSerialize")
public class EvaluationContext {
@Setter @Getter private String targetingKey;
private final Map<String, Integer> integerAttributes;
private final Map<String, String> stringAttributes;
EvaluationContext() {
this.targetingKey = "";
this.integerAttributes = new HashMap<>();
this.stringAttributes = new HashMap<>();
}
public void addStringAttribute(String key, String value) {
stringAttributes.put(key, value);
}
public String getStringAttribute(String key) {
return stringAttributes.get(key);
}
public void addIntegerAttribute(String key, Integer value) {
integerAttributes.put(key, value);
}
public Integer getIntegerAttribute(String key) {
return integerAttributes.get(key);
}
public Boolean getBooleanAttribute(String key) {
return Boolean.valueOf(stringAttributes.get(key));
}
public void addBooleanAttribute(String key, Boolean b) {
stringAttributes.put(key, b.toString());
}
public void addDatetimeAttribute(String key, ZonedDateTime value) {
this.stringAttributes.put(key, value.format(DateTimeFormatter.ISO_ZONED_DATE_TIME));
}
// TODO: addStructure or similar.
public ZonedDateTime getDatetimeAttribute(String key) {
String attr = this.stringAttributes.get(key);
if (attr == null) {
return null;
}
return ZonedDateTime.parse(attr, DateTimeFormatter.ISO_ZONED_DATE_TIME);
}
/**
* Merges two EvaluationContext objects with the second overriding the first in case of conflict.
*/
public static EvaluationContext merge(EvaluationContext ctx1, EvaluationContext ctx2) {
EvaluationContext ec = new EvaluationContext();
for (Map.Entry<String, Integer> e : ctx1.integerAttributes.entrySet()) {
ec.addIntegerAttribute(e.getKey(), e.getValue());
}
for (Map.Entry<String, Integer> e : ctx2.integerAttributes.entrySet()) {
ec.addIntegerAttribute(e.getKey(), e.getValue());
}
for (Map.Entry<String, String> e : ctx1.stringAttributes.entrySet()) {
ec.addStringAttribute(e.getKey(), e.getValue());
}
for (Map.Entry<String, String> e : ctx2.stringAttributes.entrySet()) {
ec.addStringAttribute(e.getKey(), e.getValue());
}
if (ctx1.getTargetingKey() != null) {
ec.setTargetingKey(ctx1.getTargetingKey());
}
if (ctx2.getTargetingKey() != null) {
ec.setTargetingKey(ctx2.getTargetingKey());
}
return ec;
}
}

View File

@ -2,9 +2,13 @@ package dev.openfeature.javasdk;
import com.google.common.collect.ImmutableMap;
import java.util.Optional;
// TODO: interface? or abstract class?
public abstract class Hook<T> {
public void before(HookContext<T> ctx, ImmutableMap<String, Object> hints) {}
public Optional<EvaluationContext> before(HookContext<T> ctx, ImmutableMap<String, Object> hints) {
return Optional.empty();
}
public void after(HookContext<T> ctx, FlagEvaluationDetails<T> details, ImmutableMap<String, Object> hints) {}
public void error(HookContext<T> ctx, Exception error, ImmutableMap<String, Object> hints) {}
public void finallyAfter(HookContext<T> ctx, ImmutableMap<String, Object> hints) {}

View File

@ -3,8 +3,9 @@ package dev.openfeature.javasdk;
import lombok.Builder;
import lombok.NonNull;
import lombok.Value;
import lombok.With;
@Value @Builder
@Value @Builder @With
public class HookContext<T> {
@NonNull String flagKey;
@NonNull FlagValueType type;

View File

@ -11,6 +11,7 @@ import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
@SuppressWarnings("PMD.DataflowAnomalyAnalysis")
public class OpenFeatureClient implements Client {
@ -34,11 +35,12 @@ public class OpenFeatureClient implements Client {
<T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key, T defaultValue, EvaluationContext ctx, FlagEvaluationOptions options) {
FeatureProvider provider = this.openfeatureApi.getProvider();
ImmutableMap<String, Object> hints = options.getHookHints();
if (ctx == null) {
ctx = new EvaluationContext();
}
ImmutableMap<String, Object> hints = options.getHookHints();
// merge of: API.context, client.context, invocation.context
// TODO: Context transformation?
HookContext hookCtx = HookContext.from(key, type, this, ctx, defaultValue);
@ -56,13 +58,15 @@ public class OpenFeatureClient implements Client {
FlagEvaluationDetails<T> details = null;
try {
this.beforeHooks(hookCtx, mergedHooks, hints);
EvaluationContext ctxFromHook = this.beforeHooks(hookCtx, mergedHooks, hints);
EvaluationContext invocationContext = EvaluationContext.merge(ctxFromHook, ctx);
ProviderEvaluation<T> providerEval;
if (type == FlagValueType.BOOLEAN) {
// TODO: Can we guarantee that defaultValue is a boolean? If not, how to we handle that?
providerEval = (ProviderEvaluation<T>) provider.getBooleanEvaluation(key, (Boolean) defaultValue, ctx, options);
providerEval = (ProviderEvaluation<T>) provider.getBooleanEvaluation(key, (Boolean) defaultValue, invocationContext, options);
} else {
// TODO: Support other flag types.
throw new GeneralError("Unknown flag type");
}
@ -106,13 +110,17 @@ public class OpenFeatureClient implements Client {
}
}
private HookContext beforeHooks(HookContext hookCtx, List<Hook> hooks, ImmutableMap<String, Object> hints) {
private EvaluationContext beforeHooks(HookContext hookCtx, List<Hook> hooks, ImmutableMap<String, Object> hints) {
// These traverse backwards from normal.
EvaluationContext ctx = hookCtx.getCtx();
for (Hook hook : Lists.reverse(hooks)) {
hook.before(hookCtx, hints);
// TODO: Merge returned context w/ hook context object
Optional<EvaluationContext> newCtx = hook.before(hookCtx, hints);
if (newCtx != null && newCtx.isPresent()) {
ctx = EvaluationContext.merge(ctx, newCtx.get());
hookCtx = hookCtx.withCtx(ctx);
}
return hookCtx;
}
return ctx;
}
@Override
@ -162,12 +170,12 @@ public class OpenFeatureClient implements Client {
@Override
public FlagEvaluationDetails<String> getStringDetails(String key, String defaultValue) {
return getStringDetails(key, defaultValue, new EvaluationContext());
return getStringDetails(key, defaultValue, null);
}
@Override
public FlagEvaluationDetails<String> getStringDetails(String key, String defaultValue, EvaluationContext ctx) {
return getStringDetails(key, defaultValue, new EvaluationContext(), FlagEvaluationOptions.builder().build());
return getStringDetails(key, defaultValue, ctx, FlagEvaluationOptions.builder().build());
}
@Override

View File

@ -41,7 +41,7 @@ class DeveloperExperienceTest {
api.setProvider(new NoOpProvider());
Client client = api.getClient();
client.registerHooks(clientHook);
Boolean retval = client.getBooleanValue(flagKey, false, new EvaluationContext(),
Boolean retval = client.getBooleanValue(flagKey, false, null,
FlagEvaluationOptions.builder().hook(evalHook).build());
verify(clientHook, times(1)).finallyAfter(any(), any());
verify(evalHook, times(1)).finallyAfter(any(), any());

View File

@ -0,0 +1,45 @@
package dev.openfeature.javasdk;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import java.time.ZonedDateTime;
import static org.junit.jupiter.api.Assertions.assertEquals;
public class EvalContextTests {
@Specification(spec="Evaluation Context", number="3.1",
text="The `evaluation context` structure **MUST** define an optional `targeting key` field of " +
"type string, identifying the subject of the flag evaluation.")
@Test void requires_targeting_key() {
EvaluationContext ec = new EvaluationContext();
ec.setTargetingKey("targeting-key");
assertEquals("targeting-key", ec.getTargetingKey());
}
@Specification(spec="Evaluation Context", number="3.2", text="The evaluation context MUST support the inclusion of " +
"custom fields, having keys of type `string`, and " +
"values of type `boolean | string | number | datetime | structure`.")
@Test void eval_context() {
EvaluationContext ec = new EvaluationContext();
ec.addStringAttribute("str", "test");
assertEquals("test", ec.getStringAttribute("str"));
ec.addBooleanAttribute("bool", true);
assertEquals(true, ec.getBooleanAttribute("bool"));
ec.addIntegerAttribute("int", 4);
assertEquals(4, ec.getIntegerAttribute("int"));
ZonedDateTime dt = ZonedDateTime.now();
ec.addDatetimeAttribute("dt", dt);
assertEquals(dt, ec.getDatetimeAttribute("dt"));
}
@Specification(spec="Evaluation Context", number="3.2", text="The evaluation context MUST support the inclusion of " +
"custom fields, having keys of type `string`, and " +
"values of type `boolean | string | number | datetime | structure`.")
@Disabled("Structure support")
@Test void eval_context__structure() {}
}

View File

@ -1,6 +1,5 @@
package dev.openfeature.javasdk;
import dev.openfeature.javasdk.*;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
@ -132,6 +131,7 @@ public class FlagEvaluationSpecTests {
"unhandled error, etc) the reason field in the evaluation details SHOULD indicate an error.")
@Disabled
@Test void detail_flags() {
// TODO: Add tests re: detail functions.
throw new NotImplementedException();
}

View File

@ -1,16 +1,17 @@
package dev.openfeature.javasdk;
import com.google.common.collect.ImmutableMap;
import dev.openfeature.javasdk.*;
import lombok.SneakyThrows;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.InOrder;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.any;
@ -65,7 +66,7 @@ public class HookSpecTests {
try {
HookContext.<Integer>builder()
.flagKey("key")
.ctx(new EvaluationContext())
.ctx(null)
.defaultValue(1)
.build();
fail("Missing type shouldn't be valid");
@ -77,7 +78,7 @@ public class HookSpecTests {
try {
HookContext.<Integer>builder()
.type(FlagValueType.INTEGER)
.ctx(new EvaluationContext())
.ctx(null)
.defaultValue(1)
.build();
fail("Missing key shouldn't be valid");
@ -140,7 +141,7 @@ public class HookSpecTests {
.build();
}
@Specification(spec="hooks", number="3.2", text="The before stage MUST run before flag evaluation occurs. It accepts a hook context (required) and HookHints (optional) as parameters and returns either a HookContext or nothing.")
@Specification(spec="hooks", number="3.2", text="The before stage MUST run before flag evaluation occurs. It accepts a hook context (required) and HookHints (optional) as parameters and returns either a EvaluationContext or nothing.")
@Test void before_runs_ahead_of_evaluation() {
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
api.setProvider(new AlwaysBrokenProvider());
@ -175,7 +176,7 @@ public class HookSpecTests {
verify(h, times(0)).error(any(), any(), any());
}
@Specification(spec="hooks", number="3.4", text="The error hook MUST run when errors are encountered in the before stage, the after stage or during flag evaluation. It accepts hook context (required), exception for what went wrong (required), and HookHints (optional). It has no return value.")
@Specification(spec="hooks", number="3.6", text="The error hook MUST run when errors are encountered in the before stage, the after stage or during flag evaluation. It accepts hook context (required), exception for what went wrong (required), and HookHints (optional). It has no return value.")
@Test void error_hook_run_during_non_finally_stage() {
final boolean[] error_called = {false};
Hook h = mock(Hook.class);
@ -196,8 +197,9 @@ public class HookSpecTests {
api.setProvider(new NoOpProvider());
api.registerHooks(new Hook<Boolean>() {
@Override
public void before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
public Optional<EvaluationContext> before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
evalOrder.add("api before");
return null;
}
@Override
@ -220,8 +222,9 @@ public class HookSpecTests {
Client c = api.getClient();
c.registerHooks(new Hook<Boolean>() {
@Override
public void before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
public Optional<EvaluationContext> before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
evalOrder.add("client before");
return null;
}
@Override
@ -243,8 +246,9 @@ public class HookSpecTests {
c.getBooleanValue("key", false, null, FlagEvaluationOptions.builder()
.hook(new Hook<Boolean>() {
@Override
public void before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
public Optional<EvaluationContext> before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
evalOrder.add("invocation before");
return null;
}
@Override
@ -297,8 +301,9 @@ public class HookSpecTests {
Client client = api.getClient();
Hook<Boolean> mutatingHook = new Hook<Boolean>() {
@Override
public void before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
public Optional<EvaluationContext> before(HookContext<Boolean> ctx, ImmutableMap<String, Object> hints) {
assertTrue(hints instanceof ImmutableMap);
return null;
}
@Override
@ -332,8 +337,8 @@ public class HookSpecTests {
assertTrue(feo.getHookHints().isEmpty());
}
@Specification(spec="hooks", number="3.3", text="The after stage MUST run after flag evaluation occurs. It accepts a hook context (required), flag evaluation details (required) and HookHints (optional). It has no return value.")
@Specification(spec="hooks", number="3.5", text="The finally hook MUST run after the before, after, and error stages. It accepts a hook context (required) and HookHints (optional). There is no return value.")
@Specification(spec="hooks", number="3.5", text="The after stage MUST run after flag evaluation occurs. It accepts a hook context (required), flag evaluation details (required) and HookHints (optional). It has no return value.")
@Specification(spec="hooks", number="3.7", text="The finally hook MUST run after the before, after, and error stages. It accepts a hook context (required) and HookHints (optional). There is no return value.")
@Test void flag_eval_hook_order() {
Hook hook = mock(Hook.class);
FeatureProvider provider = mock(FeatureProvider.class);
@ -391,12 +396,73 @@ public class HookSpecTests {
verify(hook2, times(1)).error(any(), any(), any());
}
@Specification(spec="hooks", number="3.3", text="Any `EvaluationContext` returned from a `before` hook **MUST** be passed to subsequent `before` hooks (via `HookContext`).")
@Test void beforeContextUpdated() {
EvaluationContext ctx = new EvaluationContext();
Hook hook = mock(Hook.class);
when(hook.before(any(), any())).thenReturn(Optional.of(ctx));
Hook hook2 = mock(Hook.class);
when(hook.before(any(), any())).thenReturn(Optional.empty());
InOrder order = inOrder(hook, hook2);
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
api.setProvider(new NoOpProvider());
Client client = api.getClient();
client.getBooleanValue("key", false, new EvaluationContext(),
FlagEvaluationOptions.builder()
.hook(hook2)
.hook(hook)
.build());
order.verify(hook).before(any(), any());
ArgumentCaptor<HookContext> captor = ArgumentCaptor.forClass(HookContext.class);
order.verify(hook2).before(captor.capture(), any());
HookContext hc = captor.getValue();
assertEquals(hc.getCtx(), ctx);
}
@Specification(spec="hooks", number="3.4", text="When `before` hooks have finished executing, any resulting `EvaluationContext` **MUST** be merged with the invocation `EvaluationContext` wherein the invocation `EvaluationContext` win any conflicts.")
@Test void mergeHappensCorrectly() {
EvaluationContext hookCtx = new EvaluationContext();
hookCtx.addStringAttribute("test", "broken");
hookCtx.addStringAttribute("another", "exists");
EvaluationContext invocationCtx = new EvaluationContext();
invocationCtx.addStringAttribute("test", "works");
Hook hook = mock(Hook.class);
when(hook.before(any(), any())).thenReturn(Optional.of(hookCtx));
FeatureProvider provider = mock(FeatureProvider.class);
when(provider.getBooleanEvaluation(any(),any(),any(),any())).thenReturn(ProviderEvaluation.<Boolean>builder()
.value(true)
.build());
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
api.setProvider(provider);
Client client = api.getClient();
client.getBooleanValue("key", false, invocationCtx,
FlagEvaluationOptions.builder()
.hook(hook)
.build());
ArgumentCaptor<EvaluationContext> captor = ArgumentCaptor.forClass(EvaluationContext.class);
verify(provider).getBooleanEvaluation(any(), any(), captor.capture(), any());
EvaluationContext ec = captor.getValue();
assertEquals("works", ec.getStringAttribute("test"));
assertEquals("exists", ec.getStringAttribute("another"));
}
@Specification(spec="hooks", number="1.4", text="The evaluation context MUST be mutable only within the before hook.")
@Specification(spec="hooks", number="3.1", text="Hooks MUST specify at least one stage.")
@Test @Disabled void todo() {}
@SneakyThrows
@Specification(spec="hooks", number="3.6", text="Condition: If finally is a reserved word in the language, finallyAfter SHOULD be used.")
@Specification(spec="hooks", number="3.8", text="Condition: If finally is a reserved word in the language, finallyAfter SHOULD be used.")
@Test void doesnt_use_finally() {
try {
Hook.class.getMethod("finally", HookContext.class, ImmutableMap.class);