diff --git a/lib/src/main/java/dev/openfeature/javasdk/EvaluationContext.java b/lib/src/main/java/dev/openfeature/javasdk/EvaluationContext.java index 58e62a27..74d28a0e 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/EvaluationContext.java +++ b/lib/src/main/java/dev/openfeature/javasdk/EvaluationContext.java @@ -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 integerAttributes; + private final Map 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 e : ctx1.integerAttributes.entrySet()) { + ec.addIntegerAttribute(e.getKey(), e.getValue()); + } + + for (Map.Entry e : ctx2.integerAttributes.entrySet()) { + ec.addIntegerAttribute(e.getKey(), e.getValue()); + } + + for (Map.Entry e : ctx1.stringAttributes.entrySet()) { + ec.addStringAttribute(e.getKey(), e.getValue()); + } + + for (Map.Entry 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; + } } diff --git a/lib/src/main/java/dev/openfeature/javasdk/Hook.java b/lib/src/main/java/dev/openfeature/javasdk/Hook.java index c4e9bb40..4976c5e3 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/Hook.java +++ b/lib/src/main/java/dev/openfeature/javasdk/Hook.java @@ -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 { - public void before(HookContext ctx, ImmutableMap hints) {} + public Optional before(HookContext ctx, ImmutableMap hints) { + return Optional.empty(); + } public void after(HookContext ctx, FlagEvaluationDetails details, ImmutableMap hints) {} public void error(HookContext ctx, Exception error, ImmutableMap hints) {} public void finallyAfter(HookContext ctx, ImmutableMap hints) {} diff --git a/lib/src/main/java/dev/openfeature/javasdk/HookContext.java b/lib/src/main/java/dev/openfeature/javasdk/HookContext.java index b3752a82..fe8a0eb2 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/HookContext.java +++ b/lib/src/main/java/dev/openfeature/javasdk/HookContext.java @@ -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 { @NonNull String flagKey; @NonNull FlagValueType type; diff --git a/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureClient.java b/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureClient.java index dc198c7a..238b714f 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureClient.java +++ b/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureClient.java @@ -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 { FlagEvaluationDetails evaluateFlag(FlagValueType type, String key, T defaultValue, EvaluationContext ctx, FlagEvaluationOptions options) { FeatureProvider provider = this.openfeatureApi.getProvider(); + ImmutableMap hints = options.getHookHints(); if (ctx == null) { ctx = new EvaluationContext(); } - ImmutableMap 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 details = null; try { - this.beforeHooks(hookCtx, mergedHooks, hints); + EvaluationContext ctxFromHook = this.beforeHooks(hookCtx, mergedHooks, hints); + EvaluationContext invocationContext = EvaluationContext.merge(ctxFromHook, ctx); ProviderEvaluation providerEval; if (type == FlagValueType.BOOLEAN) { // TODO: Can we guarantee that defaultValue is a boolean? If not, how to we handle that? - providerEval = (ProviderEvaluation) provider.getBooleanEvaluation(key, (Boolean) defaultValue, ctx, options); + providerEval = (ProviderEvaluation) 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 hooks, ImmutableMap hints) { + private EvaluationContext beforeHooks(HookContext hookCtx, List hooks, ImmutableMap 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 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 getStringDetails(String key, String defaultValue) { - return getStringDetails(key, defaultValue, new EvaluationContext()); + return getStringDetails(key, defaultValue, null); } @Override public FlagEvaluationDetails 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 diff --git a/lib/src/test/java/dev/openfeature/javasdk/DeveloperExperienceTest.java b/lib/src/test/java/dev/openfeature/javasdk/DeveloperExperienceTest.java index e9ada0e4..f103843b 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/DeveloperExperienceTest.java +++ b/lib/src/test/java/dev/openfeature/javasdk/DeveloperExperienceTest.java @@ -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()); diff --git a/lib/src/test/java/dev/openfeature/javasdk/EvalContextTests.java b/lib/src/test/java/dev/openfeature/javasdk/EvalContextTests.java new file mode 100644 index 00000000..92d9df7a --- /dev/null +++ b/lib/src/test/java/dev/openfeature/javasdk/EvalContextTests.java @@ -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() {} +} diff --git a/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java b/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java index cc0b541f..641af25c 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java +++ b/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java @@ -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(); } diff --git a/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java b/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java index da7175ab..65e1293a 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java +++ b/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java @@ -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.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.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() { @Override - public void before(HookContext ctx, ImmutableMap hints) { + public Optional before(HookContext ctx, ImmutableMap hints) { evalOrder.add("api before"); + return null; } @Override @@ -220,8 +222,9 @@ public class HookSpecTests { Client c = api.getClient(); c.registerHooks(new Hook() { @Override - public void before(HookContext ctx, ImmutableMap hints) { + public Optional before(HookContext ctx, ImmutableMap 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() { @Override - public void before(HookContext ctx, ImmutableMap hints) { + public Optional before(HookContext ctx, ImmutableMap hints) { evalOrder.add("invocation before"); + return null; } @Override @@ -297,8 +301,9 @@ public class HookSpecTests { Client client = api.getClient(); Hook mutatingHook = new Hook() { @Override - public void before(HookContext ctx, ImmutableMap hints) { + public Optional before(HookContext ctx, ImmutableMap 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 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.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 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);