From 2520f7c051315342ae4984909ad9e78bc20a4772 Mon Sep 17 00:00:00 2001 From: Justin Abrahms Date: Tue, 10 May 2022 15:51:09 -0700 Subject: [PATCH] Support for hook hints. --- .../java/javasdk/FlagEvaluationOptions.java | 5 ++- lib/src/main/java/javasdk/Hook.java | 10 +++-- .../main/java/javasdk/OpenFeatureClient.java | 28 +++++++------ .../java/javasdk/DeveloperExperienceTest.java | 6 +-- .../java/javasdk/FlagEvaluationSpecTests.java | 4 +- lib/src/test/java/javasdk/HookSpecTests.java | 40 +++++++++---------- 6 files changed, 50 insertions(+), 43 deletions(-) diff --git a/lib/src/main/java/javasdk/FlagEvaluationOptions.java b/lib/src/main/java/javasdk/FlagEvaluationOptions.java index f9420914..3aa2541a 100644 --- a/lib/src/main/java/javasdk/FlagEvaluationOptions.java +++ b/lib/src/main/java/javasdk/FlagEvaluationOptions.java @@ -1,14 +1,15 @@ package javasdk; +import com.google.common.collect.ImmutableMap; import lombok.Builder; import lombok.Data; import lombok.Singular; import java.util.List; -import java.util.Map; @Data @Builder public class FlagEvaluationOptions { @Singular private List hooks; - private Map hookHints; + @Builder.Default + private ImmutableMap hookHints = ImmutableMap.of(); } diff --git a/lib/src/main/java/javasdk/Hook.java b/lib/src/main/java/javasdk/Hook.java index f375456e..15651fa8 100644 --- a/lib/src/main/java/javasdk/Hook.java +++ b/lib/src/main/java/javasdk/Hook.java @@ -1,9 +1,11 @@ package javasdk; +import com.google.common.collect.ImmutableMap; + // TODO: interface? or abstract class? public abstract class Hook { - void before(HookContext ctx) {} - void after(HookContext ctx, FlagEvaluationDetails details) {} - void error(HookContext ctx, Exception error) {} - void finallyAfter(HookContext ctx) {} + void before(HookContext ctx, ImmutableMap hints) {} + void after(HookContext ctx, FlagEvaluationDetails details, ImmutableMap hints) {} + void error(HookContext ctx, Exception error, ImmutableMap hints) {} + void finallyAfter(HookContext ctx, ImmutableMap hints) {} } diff --git a/lib/src/main/java/javasdk/OpenFeatureClient.java b/lib/src/main/java/javasdk/OpenFeatureClient.java index 45a0f60f..69b69e2b 100644 --- a/lib/src/main/java/javasdk/OpenFeatureClient.java +++ b/lib/src/main/java/javasdk/OpenFeatureClient.java @@ -1,6 +1,7 @@ package javasdk; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import javasdk.exceptions.GeneralError; import javasdk.exceptions.OpenFeatureError; @@ -37,6 +38,9 @@ public class OpenFeatureClient implements Client { if (ctx == null) { ctx = new EvaluationContext(); } + + ImmutableMap hints = options.getHookHints(); + // TODO: Context transformation? HookContext hookCtx = HookContext.from(key, type, this, ctx, defaultValue); @@ -53,7 +57,7 @@ public class OpenFeatureClient implements Client { FlagEvaluationDetails details = null; try { - this.beforeHooks(hookCtx, mergedHooks); + this.beforeHooks(hookCtx, mergedHooks, hints); ProviderEvaluation providerEval; if (type == FlagValueType.BOOLEAN) { @@ -64,7 +68,7 @@ public class OpenFeatureClient implements Client { } details = FlagEvaluationDetails.from(providerEval, key, null); - this.afterHooks(hookCtx, details, mergedHooks); + this.afterHooks(hookCtx, details, mergedHooks, hints); } catch (Exception e) { log.error("Unable to correctly evaluate flag with key {} due to exception {}", key, e.getMessage()); if (details == null) { @@ -77,36 +81,36 @@ public class OpenFeatureClient implements Client { } else { details.errorCode = ErrorCode.GENERAL; } - this.errorHooks(hookCtx, e, mergedHooks); + this.errorHooks(hookCtx, e, mergedHooks, hints); } finally { - this.afterAllHooks(hookCtx, mergedHooks); + this.afterAllHooks(hookCtx, mergedHooks, hints); } return details; } - private void errorHooks(HookContext hookCtx, Exception e, List hooks) { + private void errorHooks(HookContext hookCtx, Exception e, List hooks, ImmutableMap hints) { for (Hook hook : hooks) { - hook.error(hookCtx, e); + hook.error(hookCtx, e, hints); } } - private void afterAllHooks(HookContext hookCtx, List hooks) { + private void afterAllHooks(HookContext hookCtx, List hooks, ImmutableMap hints) { for (Hook hook : hooks) { - hook.finallyAfter(hookCtx); + hook.finallyAfter(hookCtx, hints); } } - private void afterHooks(HookContext hookContext, FlagEvaluationDetails details, List hooks) { + private void afterHooks(HookContext hookContext, FlagEvaluationDetails details, List hooks, ImmutableMap hints) { for (Hook hook : hooks) { - hook.after(hookContext, details); + hook.after(hookContext, details, hints); } } - private HookContext beforeHooks(HookContext hookCtx, List hooks) { + private HookContext beforeHooks(HookContext hookCtx, List hooks, ImmutableMap hints) { // These traverse backwards from normal. for (Hook hook : Lists.reverse(hooks)) { - hook.before(hookCtx); + hook.before(hookCtx, hints); // TODO: Merge returned context w/ hook context object } return hookCtx; diff --git a/lib/src/test/java/javasdk/DeveloperExperienceTest.java b/lib/src/test/java/javasdk/DeveloperExperienceTest.java index 75e8775b..ae5a44fc 100644 --- a/lib/src/test/java/javasdk/DeveloperExperienceTest.java +++ b/lib/src/test/java/javasdk/DeveloperExperienceTest.java @@ -29,7 +29,7 @@ class DeveloperExperienceTest { Client client = api.getClient(); client.registerHooks(exampleHook); Boolean retval = client.getBooleanValue(flagKey, false); - verify(exampleHook, times(1)).finallyAfter(any()); + verify(exampleHook, times(1)).finallyAfter(any(), any()); assertFalse(retval); } @@ -43,8 +43,8 @@ class DeveloperExperienceTest { client.registerHooks(clientHook); Boolean retval = client.getBooleanValue(flagKey, false, new EvaluationContext(), FlagEvaluationOptions.builder().hook(evalHook).build()); - verify(clientHook, times(1)).finallyAfter(any()); - verify(evalHook, times(1)).finallyAfter(any()); + verify(clientHook, times(1)).finallyAfter(any(), any()); + verify(evalHook, times(1)).finallyAfter(any(), any()); assertFalse(retval); } diff --git a/lib/src/test/java/javasdk/FlagEvaluationSpecTests.java b/lib/src/test/java/javasdk/FlagEvaluationSpecTests.java index 7dc6ccf1..a3dd3a78 100644 --- a/lib/src/test/java/javasdk/FlagEvaluationSpecTests.java +++ b/lib/src/test/java/javasdk/FlagEvaluationSpecTests.java @@ -135,8 +135,8 @@ public class FlagEvaluationSpecTests { c.getBooleanValue("key", false, null, FlagEvaluationOptions.builder() .hook(invocationHook) .build()); - verify(clientHook, times(1)).before(any()); - verify(invocationHook, times(1)).before(any()); + verify(clientHook, times(1)).before(any(), any()); + verify(invocationHook, times(1)).before(any(), any()); } @Specification(spec="flag evaluation", number="1.18", text="Methods, functions, or operations on the client MUST " + diff --git a/lib/src/test/java/javasdk/HookSpecTests.java b/lib/src/test/java/javasdk/HookSpecTests.java index 2f7e7b52..375d83b2 100644 --- a/lib/src/test/java/javasdk/HookSpecTests.java +++ b/lib/src/test/java/javasdk/HookSpecTests.java @@ -147,7 +147,7 @@ public class HookSpecTests { client.getBooleanValue("key", false, new EvaluationContext(), FlagEvaluationOptions.builder().hook(evalHook).build()); - verify(evalHook, times(1)).before(any()); + verify(evalHook, times(1)).before(any(), any()); } @Specification(spec="hooks", number="5.1", text="Flag evalution options MUST contain a list of hooks to evaluate.") @@ -160,7 +160,7 @@ public class HookSpecTests { @Specification(spec="hooks", number="4.3", text="If an error occurs in the finally hook, it MUST NOT trigger the error hook.") @Test void errors_in_finally() { Hook h = mock(Hook.class); - doThrow(RuntimeException.class).when(h).finallyAfter(any()); + doThrow(RuntimeException.class).when(h).finallyAfter(any(), any()); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new NoOpProvider<>()); @@ -168,17 +168,17 @@ public class HookSpecTests { assertThrows(RuntimeException.class, () -> c.getBooleanValue("key", false, null, FlagEvaluationOptions.builder().hook(h).build())); - verify(h, times(1)).finallyAfter(any()); - verify(h, times(0)).error(any(), any()); + verify(h, times(1)).finallyAfter(any(), any()); + 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.") @Test void error_hook_run_during_non_finally_stage() { final boolean[] error_called = {false}; Hook h = mock(Hook.class); - doThrow(RuntimeException.class).when(h).finallyAfter(any()); + doThrow(RuntimeException.class).when(h).finallyAfter(any(), any()); - verify(h, times(0)).error(any(), any()); + verify(h, times(0)).error(any(), any(), any()); } @Specification(spec="hooks", number="4.2", text="Hooks MUST be evaluated in the following order:" + "before: API, Client, Invocation" + @@ -189,33 +189,33 @@ public class HookSpecTests { List evalOrder = new ArrayList(); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new NoOpProvider()); - api.registerHooks(new Hook() { + api.registerHooks(new Hook() { @Override - void before(HookContext ctx) { + void before(HookContext ctx, ImmutableMap hints) { evalOrder.add("api before"); } @Override - void after(HookContext ctx, FlagEvaluationDetails details) { + void after(HookContext ctx, FlagEvaluationDetails details, ImmutableMap hints) { evalOrder.add("api after"); throw new RuntimeException(); // trigger error flows. } @Override - void error(HookContext ctx, Exception error) { + void error(HookContext ctx, Exception error, ImmutableMap hints) { evalOrder.add("api error"); } @Override - void finallyAfter(HookContext ctx) { + void finallyAfter(HookContext ctx, ImmutableMap hints) { evalOrder.add("api finally"); } }); Client c = api.getClient(); - c.registerHooks(new Hook() { + c.registerHooks(new Hook() { @Override - void before(HookContext ctx) { + void before(HookContext ctx, ImmutableMap hints) { evalOrder.add("client before"); } @@ -225,35 +225,35 @@ public class HookSpecTests { } @Override - void error(HookContext ctx, Exception error) { + void error(HookContext ctx, Exception error, ImmutableMap hints) { evalOrder.add("client error"); } @Override - void finallyAfter(HookContext ctx) { + void finallyAfter(HookContext ctx, ImmutableMap hints) { evalOrder.add("client finally"); } }); c.getBooleanValue("key", false, null, FlagEvaluationOptions.builder() - .hook(new Hook() { + .hook(new Hook() { @Override - void before(HookContext ctx) { + void before(HookContext ctx, ImmutableMap hints) { evalOrder.add("invocation before"); } @Override - void after(HookContext ctx, FlagEvaluationDetails details) { + void after(HookContext ctx, FlagEvaluationDetails details, ImmutableMap hints) { evalOrder.add("invocation after"); } @Override - void error(HookContext ctx, Exception error) { + void error(HookContext ctx, Exception error, ImmutableMap hints) { evalOrder.add("invocation error"); } @Override - void finallyAfter(HookContext ctx) { + void finallyAfter(HookContext ctx, ImmutableMap hints) { evalOrder.add("invocation finally"); } })