Skip to content

Commit a0172d5

Browse files
justinabrahmsjustinabrahms
committed
Additional hook tests, incl. functionality for error hooks & api hooks.
1 parent 6121daa commit a0172d5

File tree

7 files changed

+201
-20
lines changed

7 files changed

+201
-20
lines changed

‎lib/src/main/java/javasdk/FlagEvaluationOptions.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
import lombok.Singular;
66

77
import java.util.List;
8+
import java.util.Map;
89

910
@Data @Builder
1011
public class FlagEvaluationOptions {
1112
@Singular private List<Hook> hooks;
13+
private Map<String, Object> hookHints;
1214
}

‎lib/src/main/java/javasdk/Hook.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,5 @@ public abstract class Hook<T> {
55
void before(HookContext<T> ctx) {}
66
void after(HookContext<T> ctx, FlagEvaluationDetails<T> details) {}
77
void error(HookContext<T> ctx, Exception error) {}
8-
void afterAll(HookContext<T> ctx) {}
8+
void finallyAfter(HookContext<T> ctx) {}
99
}

‎lib/src/main/java/javasdk/OpenFeatureAPI.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,14 @@
44
import lombok.Setter;
55

66
import javax.annotation.Nullable;
7+
import java.util.ArrayList;
8+
import java.util.Arrays;
9+
import java.util.List;
710

811
public class OpenFeatureAPI {
912
@Getter @Setter private FeatureProvider provider;
1013
private static OpenFeatureAPI api;
14+
@Getter private List<Hook> apiHooks;
1115

1216
public static OpenFeatureAPI getInstance() {
1317
synchronized (OpenFeatureAPI.class) {
@@ -18,6 +22,10 @@ public static OpenFeatureAPI getInstance() {
1822
return api;
1923
}
2024

25+
public OpenFeatureAPI() {
26+
this.apiHooks = new ArrayList<>();
27+
}
28+
2129
public Client getClient() {
2230
return getClient(null, null);
2331
}
@@ -30,4 +38,11 @@ public Client getClient(@Nullable String name, @Nullable String version) {
3038
return new OpenFeatureClient(this, name, version);
3139
}
3240

41+
public void registerHooks(Hook... hooks) {
42+
this.apiHooks.addAll(Arrays.asList(hooks));
43+
}
44+
45+
public void clearHooks() {
46+
this.apiHooks.clear();
47+
}
3348
}

‎lib/src/main/java/javasdk/OpenFeatureClient.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package javasdk;
22

33
import com.google.common.collect.ImmutableList;
4+
import com.google.common.collect.Lists;
45
import javasdk.exceptions.GeneralError;
56
import javasdk.exceptions.OpenFeatureError;
67
import lombok.Getter;
@@ -44,6 +45,7 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key
4445
mergedHooks = ImmutableList.<Hook>builder()
4546
.addAll(options.getHooks())
4647
.addAll(clientHooks)
48+
.addAll(openfeatureApi.getApiHooks())
4749
.build();
4850
} else {
4951
mergedHooks = clientHooks;
@@ -75,16 +77,23 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key
7577
} else {
7678
details.errorCode = ErrorCode.GENERAL;
7779
}
80+
this.errorHooks(hookCtx, e, mergedHooks);
7881
} finally {
7982
this.afterAllHooks(hookCtx, mergedHooks);
8083
}
8184

8285
return details;
8386
}
8487

88+
private void errorHooks(HookContext hookCtx, Exception e, List<Hook> hooks) {
89+
for (Hook hook : hooks) {
90+
hook.error(hookCtx, e);
91+
}
92+
}
93+
8594
private void afterAllHooks(HookContext hookCtx, List<Hook> hooks) {
8695
for (Hook hook : hooks) {
87-
hook.afterAll(hookCtx);
96+
hook.finallyAfter(hookCtx);
8897
}
8998
}
9099

@@ -95,7 +104,8 @@ private <T> void afterHooks(HookContext hookContext, FlagEvaluationDetails<T> de
95104
}
96105

97106
private HookContext beforeHooks(HookContext hookCtx, List<Hook> hooks) {
98-
for (Hook hook : hooks) {
107+
// These traverse backwards from normal.
108+
for (Hook hook : Lists.reverse(hooks)) {
99109
hook.before(hookCtx);
100110
// TODO: Merge returned context w/ hook context object
101111
}

‎lib/src/test/java/javasdk/DeveloperExperienceTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class DeveloperExperienceTest {
2929
Client client = api.getClient();
3030
client.registerHooks(exampleHook);
3131
Boolean retval = client.getBooleanValue(flagKey, false);
32-
verify(exampleHook, times(1)).afterAll(any());
32+
verify(exampleHook, times(1)).finallyAfter(any());
3333
assertFalse(retval);
3434
}
3535

@@ -43,8 +43,8 @@ class DeveloperExperienceTest {
4343
client.registerHooks(clientHook);
4444
Boolean retval = client.getBooleanValue(flagKey, false, new EvaluationContext(),
4545
FlagEvaluationOptions.builder().hook(evalHook).build());
46-
verify(clientHook, times(1)).afterAll(any());
47-
verify(evalHook, times(1)).afterAll(any());
46+
verify(clientHook, times(1)).finallyAfter(any());
47+
verify(evalHook, times(1)).finallyAfter(any());
4848
assertFalse(retval);
4949
}
5050

‎lib/src/test/java/javasdk/FlagEvaluationSpecTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,16 +75,16 @@ Client _client() {
7575
String key = "key";
7676
assertFalse(c.getBooleanValue(key, false));
7777
assertFalse(c.getBooleanValue(key, false, new EvaluationContext()));
78-
assertFalse(c.getBooleanValue(key, false, new EvaluationContext(), new FlagEvaluationOptions(null)));
78+
assertFalse(c.getBooleanValue(key, false, new EvaluationContext(), FlagEvaluationOptions.builder().build()));
7979

8080

8181
assertEquals("my-string", c.getStringValue(key, "my-string"));
8282
assertEquals("my-string", c.getStringValue(key, "my-string", new EvaluationContext()));
83-
assertEquals("my-string", c.getStringValue(key, "my-string", new EvaluationContext(), new FlagEvaluationOptions(null)));
83+
assertEquals("my-string", c.getStringValue(key, "my-string", new EvaluationContext(), FlagEvaluationOptions.builder().build()));
8484

8585
assertEquals(4, c.getIntegerValue(key, 4));
8686
assertEquals(4, c.getIntegerValue(key, 4, new EvaluationContext()));
87-
assertEquals(4, c.getIntegerValue(key, 4, new EvaluationContext(), new FlagEvaluationOptions(null)));
87+
assertEquals(4, c.getIntegerValue(key, 4, new EvaluationContext(), FlagEvaluationOptions.builder().build()));
8888

8989
}
9090

‎lib/src/test/java/javasdk/HookSpecTests.java

Lines changed: 165 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,25 @@
11
package javasdk;
22

3+
import lombok.SneakyThrows;
4+
import org.junit.jupiter.api.AfterEach;
5+
import org.junit.jupiter.api.Disabled;
36
import org.junit.jupiter.api.Test;
47

5-
import static org.junit.jupiter.api.Assertions.fail;
8+
import java.util.ArrayList;
9+
import java.util.Arrays;
10+
import java.util.List;
11+
12+
import static org.junit.jupiter.api.Assertions.*;
13+
import static org.mockito.ArgumentMatchers.any;
14+
import static org.mockito.Mockito.*;
615

716
public class HookSpecTests {
17+
@AfterEach
18+
void emptyApiHooks() {
19+
// it's a singleton. Don't pollute each test.
20+
OpenFeatureAPI.getInstance().clearHooks();
21+
}
22+
823
@Specification(spec="hooks", number="1.3", text="flag key, flag type, default value properties MUST be immutable. If the language does not support immutability, the hook MUST NOT modify these properties.")
924
@Test void immutableValues() {
1025
try {
@@ -122,31 +137,170 @@ public class HookSpecTests {
122137
.build();
123138
}
124139

125-
@Specification(spec="hooks", number="1.4", text="The evaluation context MUST be mutable only within the before hook.")
126-
@Specification(spec="hooks", number="2.1", text="HookHints MUST be a map of objects.")
127-
@Specification(spec="hooks", number="2.2", text="Condition: HookHints MUST be immutable.")
128-
@Specification(spec="hooks", number="3.1", text="Hooks MUST specify at least one stage.")
129140
@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.")
130-
@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.")
141+
@Test void before_runs_ahead_of_evaluation() {
142+
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
143+
api.setProvider(new AlwaysBrokenProvider());
144+
Client client = api.getClient();
145+
Hook<Boolean> evalHook = (Hook<Boolean>) mock(Hook.class);
146+
147+
client.getBooleanValue("key", false, new EvaluationContext(),
148+
FlagEvaluationOptions.builder().hook(evalHook).build());
149+
150+
verify(evalHook, times(1)).before(any());
151+
}
152+
153+
@Specification(spec="hooks", number="5.1", text="Flag evalution options MUST contain a list of hooks to evaluate.")
154+
@Test void feo_has_hook_list() {
155+
FlagEvaluationOptions feo = FlagEvaluationOptions.builder()
156+
.build();
157+
assertNotNull(feo.getHooks());
158+
}
159+
160+
@Specification(spec="hooks", number="4.3", text="If an error occurs in the finally hook, it MUST NOT trigger the error hook.")
161+
@Test void errors_in_finally() {
162+
Hook<Boolean> h = mock(Hook.class);
163+
doThrow(RuntimeException.class).when(h).finallyAfter(any());
164+
165+
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
166+
api.setProvider(new NoOpProvider<>());
167+
Client c= api.getClient();
168+
169+
assertThrows(RuntimeException.class, () -> c.getBooleanValue("key", false, null, FlagEvaluationOptions.builder().hook(h).build()));
170+
171+
verify(h, times(1)).finallyAfter(any());
172+
verify(h, times(0)).error(any(), any());
173+
}
174+
131175
@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.")
132-
@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.")
133-
@Specification(spec="hooks", number="3.6", text="Condition: If finally is a reserved word in the language, finallyAfter SHOULD be used.")
134-
@Specification(spec="hooks", number="4.1", text="The API, Client and invocation MUST have a method for registering hooks which accepts flag evaluation options")
176+
@Test void error_hook_run_during_non_finally_stage() {
177+
final boolean[] error_called = {false};
178+
Hook h = mock(Hook.class);
179+
doThrow(RuntimeException.class).when(h).finallyAfter(any());
180+
181+
verify(h, times(0)).error(any(), any());
182+
}
135183
@Specification(spec="hooks", number="4.2", text="Hooks MUST be evaluated in the following order:" +
136184
"before: API, Client, Invocation" +
137185
"after: Invocation, Client, API" +
138186
"error (if applicable): Invocation, Client, API" +
139187
"finally: Invocation, Client, API")
140-
@Specification(spec="hooks", number="4.3", text=" If an error occurs in the finally hook, it MUST NOT trigger the error hook.")
188+
@Test void eval_order() {
189+
List<String> evalOrder = new ArrayList<String>();
190+
OpenFeatureAPI api = OpenFeatureAPI.getInstance();
191+
api.setProvider(new NoOpProvider());
192+
api.registerHooks(new Hook() {
193+
@Override
194+
void before(HookContext ctx) {
195+
evalOrder.add("api before");
196+
}
197+
198+
@Override
199+
void after(HookContext ctx, FlagEvaluationDetails details) {
200+
evalOrder.add("api after");
201+
throw new RuntimeException(); // trigger error flows.
202+
}
203+
204+
@Override
205+
void error(HookContext ctx, Exception error) {
206+
evalOrder.add("api error");
207+
}
208+
209+
@Override
210+
void finallyAfter(HookContext ctx) {
211+
evalOrder.add("api finally");
212+
}
213+
});
214+
215+
Client c = api.getClient();
216+
c.registerHooks(new Hook() {
217+
@Override
218+
void before(HookContext ctx) {
219+
evalOrder.add("client before");
220+
}
221+
222+
@Override
223+
void after(HookContext ctx, FlagEvaluationDetails details) {
224+
evalOrder.add("client after");
225+
}
226+
227+
@Override
228+
void error(HookContext ctx, Exception error) {
229+
evalOrder.add("client error");
230+
}
231+
232+
@Override
233+
void finallyAfter(HookContext ctx) {
234+
evalOrder.add("client finally");
235+
}
236+
});
237+
238+
c.getBooleanValue("key", false, null, FlagEvaluationOptions.builder()
239+
.hook(new Hook() {
240+
@Override
241+
void before(HookContext ctx) {
242+
evalOrder.add("invocation before");
243+
}
244+
245+
@Override
246+
void after(HookContext ctx, FlagEvaluationDetails details) {
247+
evalOrder.add("invocation after");
248+
}
249+
250+
@Override
251+
void error(HookContext ctx, Exception error) {
252+
evalOrder.add("invocation error");
253+
}
254+
255+
@Override
256+
void finallyAfter(HookContext ctx) {
257+
evalOrder.add("invocation finally");
258+
}
259+
})
260+
.build());
261+
262+
ArrayList<String> expectedOrder = new ArrayList<String>();
263+
expectedOrder.addAll(Arrays.asList(
264+
"api before", "client before", "invocation before",
265+
"invocation after", "client after", "api after",
266+
"invocation error", "client error", "api error",
267+
"invocation finally", "client finally", "api finally"));
268+
assertEquals(expectedOrder, evalOrder);
269+
}
270+
271+
@Specification(spec="hooks", number="1.4", text="The evaluation context MUST be mutable only within the before hook.")
272+
@Specification(spec="hooks", number="2.1", text="HookHints MUST be a map of objects.")
273+
@Specification(spec="hooks", number="2.2", text="Condition: HookHints MUST be immutable.")
274+
@Specification(spec="hooks", number="3.1", text="Hooks MUST specify at least one stage.")
275+
@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.")
276+
@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.")
277+
@Specification(spec="hooks", number="4.1", text="The API, Client and invocation MUST have a method for registering hooks which accepts flag evaluation options")
141278
@Specification(spec="hooks", number="4.4", text="If an error occurs in the before or after hooks, the error hooks MUST be invoked.")
142279
@Specification(spec="hooks", number="4.5", text="If an error occurs during the evaluation of before or after hooks, any remaining hooks in the before or after stages MUST NOT be invoked.")
143280
@Specification(spec="hooks", number="4.6", text="If an error is encountered in the error stage, it MUST NOT be returned to the user.")
144-
@Specification(spec="hooks", number="5.1", text="Flag evalution options MUST contain a list of hooks to evaluate.")
145281
@Specification(spec="hooks", number="5.2", text="Flag evaluation options MAY contain HookHints, a map of data to be provided to hook invocations.")
146282
@Specification(spec="hooks", number="5.3", text="HookHints MUST be passed to each hook through a parameter. It is merged into the object in the precedence order API -> Client -> Invocation (last wins).")
147283
@Specification(spec="hooks", number="5.4", text="The hook MUST NOT alter the HookHints object.")
148284
@Specification(spec="hooks", number="6.1", text="HookHints MUST passed between each hook.")
149285
void todo() {}
150286

287+
@SneakyThrows
288+
@Specification(spec="hooks", number="3.6", text="Condition: If finally is a reserved word in the language, finallyAfter SHOULD be used.")
289+
@Disabled
290+
@Test void doesnt_use_finally() {
291+
// Class [] carr = new Class[1];
292+
// carr[0] = HookContext.class;
293+
//
294+
// try {
295+
// Hook.class.getMethod("finally", carr);
296+
// fail("Not possible. Finally is a reserved word.");
297+
// } catch (NoSuchMethodException e) {
298+
// // expected
299+
// }
300+
301+
Hook.class.getMethod("finallyAfter", HookContext.class);
302+
303+
}
304+
151305

152306
}

0 commit comments

Comments
 (0)