Skip to content

Commit df686e5

Browse files
chrfwowchrfwow
committed
feat: Add evaluation details to finally hook stage #1246
Signed-off-by: christian.lutnik <[email protected]>
1 parent fc6f35e commit df686e5

File tree

7 files changed

+90
-93
lines changed

7 files changed

+90
-93
lines changed

‎src/main/java/dev/openfeature/sdk/Hook.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ default void error(HookContext<T> ctx, Exception error, Map<String, Object> hint
4848
* @param ctx Information about the particular flag evaluation
4949
* @param hints An immutable mapping of data for users to communicate to the hooks.
5050
*/
51-
default void finallyAfter(HookContext<T> ctx, Map<String, Object> hints) {
51+
default void finallyAfter(HookContext<T> ctx, FlagEvaluationDetails<T> details, Map<String, Object> hints) {
5252
}
5353

5454
default boolean supportsFlagValueType(FlagValueType flagValueType) {

‎src/main/java/dev/openfeature/sdk/HookSupport.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ public void afterHooks(FlagValueType flagValueType, HookContext hookContext, Fla
2525
executeHooksUnchecked(flagValueType, hooks, hook -> hook.after(hookContext, details, hints));
2626
}
2727

28-
public void afterAllHooks(FlagValueType flagValueType, HookContext hookCtx, List<Hook> hooks,
29-
Map<String, Object> hints) {
30-
executeHooks(flagValueType, hooks, "finally", hook -> hook.finallyAfter(hookCtx, hints));
28+
public void afterAllHooks(FlagValueType flagValueType, HookContext hookCtx, FlagEvaluationDetails details,
29+
List<Hook> hooks, Map<String, Object> hints) {
30+
executeHooks(flagValueType, hooks, "finally", hook -> hook.finallyAfter(hookCtx, details, hints));
3131
}
3232

3333
public void errorHooks(FlagValueType flagValueType, HookContext hookCtx, Exception e, List<Hook> hooks,

‎src/main/java/dev/openfeature/sdk/OpenFeatureClient.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key
218218
enrichDetailsWithErrorDefaults(defaultValue, details);
219219
hookSupport.errorHooks(type, afterHookContext, e, mergedHooks, hints);
220220
} finally {
221-
hookSupport.afterAllHooks(type, afterHookContext, mergedHooks, hints);
221+
hookSupport.afterAllHooks(type, afterHookContext, details, mergedHooks, hints);
222222
}
223223

224224
return details;

‎src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ void clientHooks() {
3636
Client client = api.getClient();
3737
client.addHooks(exampleHook);
3838
Boolean retval = client.getBooleanValue(flagKey, false);
39-
verify(exampleHook, times(1)).finallyAfter(any(), any());
39+
verify(exampleHook, times(1)).finallyAfter(any(), any(), any());
4040
assertFalse(retval);
4141
}
4242

@@ -51,8 +51,8 @@ void evalHooks() {
5151
client.addHooks(clientHook);
5252
Boolean retval = client.getBooleanValue(flagKey, false, null,
5353
FlagEvaluationOptions.builder().hook(evalHook).build());
54-
verify(clientHook, times(1)).finallyAfter(any(), any());
55-
verify(evalHook, times(1)).finallyAfter(any(), any());
54+
verify(clientHook, times(1)).finallyAfter(any(), any(), any());
55+
verify(evalHook, times(1)).finallyAfter(any(), any(), any());
5656
assertFalse(retval);
5757
}
5858

‎src/test/java/dev/openfeature/sdk/HookSpecTest.java‎

Lines changed: 68 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212

1313
import java.util.*;
1414

15-
import static org.assertj.core.api.Assertions.assertThatCode;
1615
import static org.assertj.core.api.Assertions.fail;
16+
import static org.assertj.core.api.Assertions.*;
1717
import static org.junit.jupiter.api.Assertions.*;
1818
import static org.mockito.ArgumentMatchers.any;
1919
import static org.mockito.Mockito.*;
@@ -115,7 +115,7 @@ void nullish_properties_on_hookcontext() {
115115

116116
}
117117

118-
@Specification(number = "4.1.2", text = "The hook context SHOULD provide: access to the client metadata and the provider metadata fields.")
118+
@Specification(number = "4.1.2", text = "The `hook context` SHOULD provide access to the `client metadata` and the `provider metadata` fields.")
119119
@Test
120120
void optional_properties() {
121121
// don't specify
@@ -170,7 +170,7 @@ void feo_has_hook_list() {
170170
void error_hook_run_during_non_finally_stage() {
171171
final boolean[] error_called = {false};
172172
Hook h = mockBooleanHook();
173-
doThrow(RuntimeException.class).when(h).finallyAfter(any(), any());
173+
doThrow(RuntimeException.class).when(h).finallyAfter(any(), any(), any());
174174

175175
verify(h, times(0)).error(any(), any(), any());
176176
}
@@ -201,7 +201,7 @@ void error_hook_must_run_if_resolution_details_returns_an_error_code() {
201201

202202
verify(hook, times(1)).before(any(), any());
203203
verify(hook, times(1)).error(any(), captor.capture(), any());
204-
verify(hook, times(1)).finallyAfter(any(), any());
204+
verify(hook, times(1)).finallyAfter(any(), any(), any());
205205
verify(hook, never()).after(any(), any(), any());
206206

207207
Exception exception = captor.getValue();
@@ -241,7 +241,7 @@ public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object>
241241
}
242242

243243
@Override
244-
public void finallyAfter(HookContext<Boolean> ctx, Map<String, Object> hints) {
244+
public void finallyAfter(HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
245245
evalOrder.add("provider finally");
246246
}
247247
});
@@ -266,7 +266,7 @@ public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object>
266266
}
267267

268268
@Override
269-
public void finallyAfter(HookContext<Boolean> ctx, Map<String, Object> hints) {
269+
public void finallyAfter(HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
270270
evalOrder.add("api finally");
271271
}
272272
});
@@ -290,7 +290,7 @@ public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object>
290290
}
291291

292292
@Override
293-
public void finallyAfter(HookContext<Boolean> ctx, Map<String, Object> hints) {
293+
public void finallyAfter(HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
294294
evalOrder.add("client finally");
295295
}
296296
});
@@ -315,7 +315,7 @@ public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object>
315315
}
316316

317317
@Override
318-
public void finallyAfter(HookContext<Boolean> ctx, Map<String, Object> hints) {
318+
public void finallyAfter(HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
319319
evalOrder.add("invocation finally");
320320
}
321321
})
@@ -344,8 +344,8 @@ void error_stops_before() {
344344
.hook(h2)
345345
.hook(h)
346346
.build());
347-
verify(h, times(1)).before(any(), any());
348-
verify(h2, times(0)).before(any(), any());
347+
verify(h, times(1)).before(any(), any());
348+
verify(h2, times(0)).before(any(), any());
349349
}
350350

351351
@Specification(number = "4.4.6", 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.")
@@ -393,7 +393,7 @@ public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object>
393393
}
394394

395395
@Override
396-
public void finallyAfter(HookContext<Boolean> ctx, Map<String, Object> hints) {
396+
public void finallyAfter(HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
397397
assertThatCode(() -> hints.put(hintKey, "changed value")).isInstanceOf(UnsupportedOperationException.class);
398398
}
399399
};
@@ -435,7 +435,7 @@ void flag_eval_hook_order() {
435435
order.verify(hook).before(any(), any());
436436
order.verify(provider).getBooleanEvaluation(any(), any(), any());
437437
order.verify(hook).after(any(), any(), any());
438-
order.verify(hook).finallyAfter(any(), any());
438+
order.verify(hook).finallyAfter(any(), any(), any());
439439
}
440440

441441
@Specification(number = "4.4.5", text = "If an error occurs in the before or after hooks, the error hooks MUST be invoked.")
@@ -464,6 +464,58 @@ void error_hooks__after() {
464464
verify(hook, times(1)).error(any(), any(), any());
465465
}
466466

467+
@Test
468+
void erroneous_flagResolution_setsAppropriateFieldsInFlagEvaluationDetails() {
469+
Hook hook = mockBooleanHook();
470+
doThrow(RuntimeException.class).when(hook).after(any(), any(), any());
471+
String flagKey = "test-flag-key";
472+
Client client = getClient(TestEventsProvider.newInitializedTestEventsProvider());
473+
client.getBooleanValue(
474+
flagKey,
475+
true,
476+
new ImmutableContext(),
477+
FlagEvaluationOptions.builder().hook(hook).build()
478+
);
479+
480+
ArgumentCaptor<FlagEvaluationDetails<Boolean>> captor = ArgumentCaptor.forClass(FlagEvaluationDetails.class);
481+
verify(hook).finallyAfter(any(), captor.capture(), any());
482+
483+
FlagEvaluationDetails<Boolean> evaluationDetails = captor.getValue();
484+
assertThat(evaluationDetails).isNotNull();
485+
486+
assertThat(evaluationDetails.getErrorCode()).isEqualTo(ErrorCode.GENERAL);
487+
assertThat(evaluationDetails.getReason()).isEqualTo("ERROR");
488+
assertThat(evaluationDetails.getVariant()).isEqualTo("Passed in default");
489+
assertThat(evaluationDetails.getFlagKey()).isEqualTo(flagKey);
490+
assertThat(evaluationDetails.getFlagMetadata()).isEqualTo(ImmutableMetadata.builder().build());
491+
assertThat(evaluationDetails.getValue()).isEqualTo(true);
492+
}
493+
494+
@Test
495+
void successful_flagResolution_setsAppropriateFieldsInFlagEvaluationDetails() {
496+
Hook hook = mockBooleanHook();
497+
String flagKey = "test-flag-key";
498+
Client client = getClient(TestEventsProvider.newInitializedTestEventsProvider());
499+
client.getBooleanValue(
500+
flagKey,
501+
true,
502+
new ImmutableContext(),
503+
FlagEvaluationOptions.builder().hook(hook).build()
504+
);
505+
506+
ArgumentCaptor<FlagEvaluationDetails<Boolean>> captor = ArgumentCaptor.forClass(FlagEvaluationDetails.class);
507+
verify(hook).finallyAfter(any(), captor.capture(), any());
508+
509+
FlagEvaluationDetails<Boolean> evaluationDetails = captor.getValue();
510+
assertThat(evaluationDetails).isNotNull();
511+
assertThat(evaluationDetails.getErrorCode()).isNull();
512+
assertThat(evaluationDetails.getReason()).isEqualTo("DEFAULT");
513+
assertThat(evaluationDetails.getVariant()).isEqualTo("Passed in default");
514+
assertThat(evaluationDetails.getFlagKey()).isEqualTo(flagKey);
515+
assertThat(evaluationDetails.getFlagMetadata()).isEqualTo(ImmutableMetadata.builder().build());
516+
assertThat(evaluationDetails.getValue()).isEqualTo(true);
517+
}
518+
467519
@Test
468520
void multi_hooks_early_out__before() {
469521
Hook<Boolean> hook = mockBooleanHook();
@@ -556,7 +608,7 @@ void mergeHappensCorrectly() {
556608
void first_finally_broken() {
557609
Hook hook = mockBooleanHook();
558610
doThrow(RuntimeException.class).when(hook).before(any(), any());
559-
doThrow(RuntimeException.class).when(hook).finallyAfter(any(), any());
611+
doThrow(RuntimeException.class).when(hook).finallyAfter(any(), any(), any());
560612
Hook hook2 = mockBooleanHook();
561613
InOrder order = inOrder(hook, hook2);
562614

@@ -568,8 +620,8 @@ void first_finally_broken() {
568620
.build());
569621

570622
order.verify(hook).before(any(), any());
571-
order.verify(hook2).finallyAfter(any(), any());
572-
order.verify(hook).finallyAfter(any(), any());
623+
order.verify(hook2).finallyAfter(any(), any(), any());
624+
order.verify(hook).finallyAfter(any(), any(), any());
573625
}
574626

575627
@Specification(number = "4.4.4", text = "If an error hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining error hooks.")
@@ -616,8 +668,7 @@ void doesnt_use_finally() {
616668
.as("Not possible. Finally is a reserved word.")
617669
.isInstanceOf(NoSuchMethodException.class);
618670

619-
assertThatCode(() -> Hook.class.getMethod("finallyAfter", HookContext.class, Map.class))
671+
assertThatCode(() -> Hook.class.getMethod("finallyAfter", HookContext.class, FlagEvaluationDetails.class, Map.class))
620672
.doesNotThrowAnyException();
621673
}
622-
623674
}

‎src/test/java/dev/openfeature/sdk/HookSupportTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,12 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) {
5151

5252
hookSupport.beforeHooks(flagValueType, hookContext, Collections.singletonList(genericHook), Collections.emptyMap());
5353
hookSupport.afterHooks(flagValueType, hookContext, FlagEvaluationDetails.builder().build(), Collections.singletonList(genericHook), Collections.emptyMap());
54-
hookSupport.afterAllHooks(flagValueType, hookContext, Collections.singletonList(genericHook), Collections.emptyMap());
54+
hookSupport.afterAllHooks(flagValueType, hookContext, FlagEvaluationDetails.builder().build(), Collections.singletonList(genericHook), Collections.emptyMap());
5555
hookSupport.errorHooks(flagValueType, hookContext, expectedException, Collections.singletonList(genericHook), Collections.emptyMap());
5656

5757
verify(genericHook).before(any(), any());
5858
verify(genericHook).after(any(), any(), any());
59-
verify(genericHook).finallyAfter(any(), any());
59+
verify(genericHook).finallyAfter(any(), any(), any());
6060
verify(genericHook).error(any(), any(), any());
6161
}
6262

‎src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java‎

Lines changed: 12 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,8 @@
11
package dev.openfeature.sdk;
22

3-
import static org.assertj.core.api.Assertions.assertThat;
4-
import static org.junit.jupiter.api.Assertions.assertEquals;
5-
import static org.junit.jupiter.api.Assertions.assertThrows;
6-
import static org.mockito.ArgumentMatchers.any;
7-
import static org.mockito.ArgumentMatchers.anyString;
8-
import static org.mockito.Mockito.*;
9-
10-
import java.util.HashMap;
11-
import java.util.concurrent.atomic.AtomicBoolean;
12-
3+
import dev.openfeature.sdk.exceptions.FatalError;
4+
import dev.openfeature.sdk.fixtures.HookFixtures;
5+
import dev.openfeature.sdk.testutils.TestEventsProvider;
136
import org.junit.jupiter.api.AfterEach;
147
import org.junit.jupiter.api.BeforeEach;
158
import org.junit.jupiter.api.DisplayName;
@@ -18,9 +11,15 @@
1811
import org.simplify4u.slf4jmock.LoggerMock;
1912
import org.slf4j.Logger;
2013

21-
import dev.openfeature.sdk.exceptions.FatalError;
22-
import dev.openfeature.sdk.fixtures.HookFixtures;
23-
import dev.openfeature.sdk.testutils.TestEventsProvider;
14+
import java.util.HashMap;
15+
16+
import static org.assertj.core.api.Assertions.assertThat;
17+
import static org.junit.jupiter.api.Assertions.assertEquals;
18+
import static org.junit.jupiter.api.Assertions.assertThrows;
19+
import static org.mockito.ArgumentMatchers.any;
20+
import static org.mockito.ArgumentMatchers.anyString;
21+
import static org.mockito.Mockito.mock;
22+
import static org.mockito.Mockito.never;
2423

2524
class OpenFeatureClientTest implements HookFixtures {
2625

@@ -102,57 +101,4 @@ void shouldNotCallEvaluationMethodsWhenProviderIsInNotReadyState() {
102101

103102
assertThat(details.getErrorCode()).isEqualTo(ErrorCode.PROVIDER_NOT_READY);
104103
}
105-
106-
private static class MockProvider implements FeatureProvider {
107-
private final AtomicBoolean evaluationCalled = new AtomicBoolean();
108-
private final ProviderState providerState;
109-
110-
public MockProvider(ProviderState providerState) {
111-
this.providerState = providerState;
112-
}
113-
114-
public boolean isEvaluationCalled() {
115-
return evaluationCalled.get();
116-
}
117-
118-
@Override
119-
public ProviderState getState() {
120-
return providerState;
121-
}
122-
123-
@Override
124-
public Metadata getMetadata() {
125-
return null;
126-
}
127-
128-
@Override
129-
public ProviderEvaluation<Boolean> getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx) {
130-
evaluationCalled.set(true);
131-
return null;
132-
}
133-
134-
@Override
135-
public ProviderEvaluation<String> getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) {
136-
evaluationCalled.set(true);
137-
return null;
138-
}
139-
140-
@Override
141-
public ProviderEvaluation<Integer> getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx) {
142-
evaluationCalled.set(true);
143-
return null;
144-
}
145-
146-
@Override
147-
public ProviderEvaluation<Double> getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) {
148-
evaluationCalled.set(true);
149-
return null;
150-
}
151-
152-
@Override
153-
public ProviderEvaluation<Value> getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx) {
154-
evaluationCalled.set(true);
155-
return null;
156-
}
157-
}
158104
}

0 commit comments

Comments
 (0)