Skip to content

Commit 56d9fd2

Browse files
committed
fixup! feat: Add evaluation details to finally hook stage open-feature#1246
Signed-off-by: christian.lutnik <[email protected]>
1 parent 49d7b95 commit 56d9fd2

File tree

5 files changed

+96
-68
lines changed

5 files changed

+96
-68
lines changed

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,7 @@ default void error(HookContext<T> ctx, Exception error, Map<String, Object> hint
4646
* @param ctx Information about the particular flag evaluation
4747
* @param hints An immutable mapping of data for users to communicate to the hooks.
4848
*/
49-
default void finallyAfter(HookContext<T> ctx, FlagEvaluationDetails<T> details, Map<String, Object> hints) {
50-
}
49+
default void finallyAfter(HookContext<T> ctx, FlagEvaluationDetails<T> details, Map<String, Object> hints) {}
5150

5251
default boolean supportsFlagValueType(FlagValueType flagValueType) {
5352
return true;

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

+6-2
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,12 @@ public void afterHooks(
2828
executeHooksUnchecked(flagValueType, hooks, hook -> hook.after(hookContext, details, hints));
2929
}
3030

31-
public void afterAllHooks(FlagValueType flagValueType, HookContext hookCtx, FlagEvaluationDetails details,
32-
List<Hook> hooks, Map<String, Object> hints) {
31+
public void afterAllHooks(
32+
FlagValueType flagValueType,
33+
HookContext hookCtx,
34+
FlagEvaluationDetails details,
35+
List<Hook> hooks,
36+
Map<String, Object> hints) {
3337
executeHooks(flagValueType, hooks, "finally", hook -> hook.finallyAfter(hookCtx, details, hints));
3438
}
3539

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

+51-40
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,5 @@
11
package dev.openfeature.sdk;
22

3-
import dev.openfeature.sdk.exceptions.FlagNotFoundError;
4-
import dev.openfeature.sdk.fixtures.HookFixtures;
5-
import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
6-
import dev.openfeature.sdk.testutils.TestEventsProvider;
7-
import lombok.SneakyThrows;
8-
import org.junit.jupiter.api.AfterEach;
9-
import org.junit.jupiter.api.Test;
10-
import org.mockito.ArgumentCaptor;
11-
import org.mockito.InOrder;
12-
13-
import java.util.ArrayList;
14-
import java.util.Arrays;
15-
import java.util.Collections;
16-
import java.util.HashMap;
17-
import java.util.List;
18-
import java.util.Map;
19-
import java.util.Optional;
20-
213
import static org.assertj.core.api.Assertions.assertThat;
224
import static org.assertj.core.api.Assertions.assertThatCode;
235
import static org.assertj.core.api.Assertions.fail;
@@ -34,6 +16,23 @@
3416
import static org.mockito.Mockito.verify;
3517
import static org.mockito.Mockito.when;
3618

19+
import dev.openfeature.sdk.exceptions.FlagNotFoundError;
20+
import dev.openfeature.sdk.fixtures.HookFixtures;
21+
import dev.openfeature.sdk.testutils.FeatureProviderTestUtils;
22+
import dev.openfeature.sdk.testutils.TestEventsProvider;
23+
import java.util.ArrayList;
24+
import java.util.Arrays;
25+
import java.util.Collections;
26+
import java.util.HashMap;
27+
import java.util.List;
28+
import java.util.Map;
29+
import java.util.Optional;
30+
import lombok.SneakyThrows;
31+
import org.junit.jupiter.api.AfterEach;
32+
import org.junit.jupiter.api.Test;
33+
import org.mockito.ArgumentCaptor;
34+
import org.mockito.InOrder;
35+
3736
class HookSpecTest implements HookFixtures {
3837
@AfterEach
3938
void emptyApiHooks() {
@@ -285,7 +284,10 @@ public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object>
285284
}
286285

287286
@Override
288-
public void finallyAfter(HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
287+
public void finallyAfter(
288+
HookContext<Boolean> ctx,
289+
FlagEvaluationDetails<Boolean> details,
290+
Map<String, Object> hints) {
289291
evalOrder.add("provider finally");
290292
}
291293
});
@@ -311,7 +313,8 @@ public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object>
311313
}
312314

313315
@Override
314-
public void finallyAfter(HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
316+
public void finallyAfter(
317+
HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
315318
evalOrder.add("api finally");
316319
}
317320
});
@@ -336,7 +339,8 @@ public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object>
336339
}
337340

338341
@Override
339-
public void finallyAfter(HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
342+
public void finallyAfter(
343+
HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
340344
evalOrder.add("client finally");
341345
}
342346
});
@@ -367,12 +371,15 @@ public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object>
367371
evalOrder.add("invocation error");
368372
}
369373

370-
@Override
371-
public void finallyAfter(HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
372-
evalOrder.add("invocation finally");
373-
}
374-
})
375-
.build());
374+
@Override
375+
public void finallyAfter(
376+
HookContext<Boolean> ctx,
377+
FlagEvaluationDetails<Boolean> details,
378+
Map<String, Object> hints) {
379+
evalOrder.add("invocation finally");
380+
}
381+
})
382+
.build());
376383

377384
List<String> expectedOrder = Arrays.asList(
378385
"api before",
@@ -408,10 +415,11 @@ void error_stops_before() {
408415
api.setProviderAndWait(new AlwaysBrokenProvider());
409416
Client c = api.getClient();
410417

411-
c.getBooleanDetails("key", false, null, FlagEvaluationOptions.builder()
412-
.hook(h2)
413-
.hook(h)
414-
.build());
418+
c.getBooleanDetails(
419+
"key",
420+
false,
421+
null,
422+
FlagEvaluationOptions.builder().hook(h2).hook(h).build());
415423
verify(h, times(1)).before(any(), any());
416424
verify(h2, times(0)).before(any(), any());
417425
}
@@ -472,8 +480,10 @@ public void error(HookContext<Boolean> ctx, Exception error, Map<String, Object>
472480
}
473481

474482
@Override
475-
public void finallyAfter(HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
476-
assertThatCode(() -> hints.put(hintKey, "changed value")).isInstanceOf(UnsupportedOperationException.class);
483+
public void finallyAfter(
484+
HookContext<Boolean> ctx, FlagEvaluationDetails<Boolean> details, Map<String, Object> hints) {
485+
assertThatCode(() -> hints.put(hintKey, "changed value"))
486+
.isInstanceOf(UnsupportedOperationException.class);
477487
}
478488
};
479489

@@ -569,8 +579,7 @@ void erroneous_flagResolution_setsAppropriateFieldsInFlagEvaluationDetails() {
569579
flagKey,
570580
true,
571581
new ImmutableContext(),
572-
FlagEvaluationOptions.builder().hook(hook).build()
573-
);
582+
FlagEvaluationOptions.builder().hook(hook).build());
574583

575584
ArgumentCaptor<FlagEvaluationDetails<Boolean>> captor = ArgumentCaptor.forClass(FlagEvaluationDetails.class);
576585
verify(hook).finallyAfter(any(), captor.capture(), any());
@@ -582,7 +591,8 @@ void erroneous_flagResolution_setsAppropriateFieldsInFlagEvaluationDetails() {
582591
assertThat(evaluationDetails.getReason()).isEqualTo("ERROR");
583592
assertThat(evaluationDetails.getVariant()).isEqualTo("Passed in default");
584593
assertThat(evaluationDetails.getFlagKey()).isEqualTo(flagKey);
585-
assertThat(evaluationDetails.getFlagMetadata()).isEqualTo(ImmutableMetadata.builder().build());
594+
assertThat(evaluationDetails.getFlagMetadata())
595+
.isEqualTo(ImmutableMetadata.builder().build());
586596
assertThat(evaluationDetails.getValue()).isTrue();
587597
}
588598

@@ -595,8 +605,7 @@ void successful_flagResolution_setsAppropriateFieldsInFlagEvaluationDetails() {
595605
flagKey,
596606
true,
597607
new ImmutableContext(),
598-
FlagEvaluationOptions.builder().hook(hook).build()
599-
);
608+
FlagEvaluationOptions.builder().hook(hook).build());
600609

601610
ArgumentCaptor<FlagEvaluationDetails<Boolean>> captor = ArgumentCaptor.forClass(FlagEvaluationDetails.class);
602611
verify(hook).finallyAfter(any(), captor.capture(), any());
@@ -607,7 +616,8 @@ void successful_flagResolution_setsAppropriateFieldsInFlagEvaluationDetails() {
607616
assertThat(evaluationDetails.getReason()).isEqualTo("DEFAULT");
608617
assertThat(evaluationDetails.getVariant()).isEqualTo("Passed in default");
609618
assertThat(evaluationDetails.getFlagKey()).isEqualTo(flagKey);
610-
assertThat(evaluationDetails.getFlagMetadata()).isEqualTo(ImmutableMetadata.builder().build());
619+
assertThat(evaluationDetails.getFlagMetadata())
620+
.isEqualTo(ImmutableMetadata.builder().build());
611621
assertThat(evaluationDetails.getValue()).isTrue();
612622
}
613623

@@ -772,7 +782,8 @@ void doesnt_use_finally() {
772782
.as("Not possible. Finally is a reserved word.")
773783
.isInstanceOf(NoSuchMethodException.class);
774784

775-
assertThatCode(() -> Hook.class.getMethod("finallyAfter", HookContext.class, FlagEvaluationDetails.class, Map.class))
785+
assertThatCode(() ->
786+
Hook.class.getMethod("finallyAfter", HookContext.class, FlagEvaluationDetails.class, Map.class))
776787
.doesNotThrowAnyException();
777788
}
778789
}

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

+29-14
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,20 @@
11
package dev.openfeature.sdk;
22

3-
import dev.openfeature.sdk.fixtures.HookFixtures;
4-
import org.junit.jupiter.api.DisplayName;
5-
import org.junit.jupiter.api.Test;
6-
import org.junit.jupiter.params.ParameterizedTest;
7-
import org.junit.jupiter.params.provider.EnumSource;
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.mockito.ArgumentMatchers.any;
5+
import static org.mockito.Mockito.verify;
6+
import static org.mockito.Mockito.when;
87

8+
import dev.openfeature.sdk.fixtures.HookFixtures;
99
import java.util.Arrays;
1010
import java.util.Collections;
1111
import java.util.HashMap;
1212
import java.util.Map;
1313
import java.util.Optional;
14-
15-
import static org.assertj.core.api.Assertions.assertThat;
16-
import static org.mockito.ArgumentMatchers.any;
17-
import static org.mockito.Mockito.verify;
18-
import static org.mockito.Mockito.when;
14+
import org.junit.jupiter.api.DisplayName;
15+
import org.junit.jupiter.api.Test;
16+
import org.junit.jupiter.params.ParameterizedTest;
17+
import org.junit.jupiter.params.provider.EnumSource;
1918

2019
class HookSupportTest implements HookFixtures {
2120
@Test
@@ -56,10 +55,26 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) {
5655
() -> "client",
5756
() -> "provider");
5857

59-
hookSupport.beforeHooks(flagValueType, hookContext, Collections.singletonList(genericHook), Collections.emptyMap());
60-
hookSupport.afterHooks(flagValueType, hookContext, FlagEvaluationDetails.builder().build(), Collections.singletonList(genericHook), Collections.emptyMap());
61-
hookSupport.afterAllHooks(flagValueType, hookContext, FlagEvaluationDetails.builder().build(), Collections.singletonList(genericHook), Collections.emptyMap());
62-
hookSupport.errorHooks(flagValueType, hookContext, expectedException, Collections.singletonList(genericHook), Collections.emptyMap());
58+
hookSupport.beforeHooks(
59+
flagValueType, hookContext, Collections.singletonList(genericHook), Collections.emptyMap());
60+
hookSupport.afterHooks(
61+
flagValueType,
62+
hookContext,
63+
FlagEvaluationDetails.builder().build(),
64+
Collections.singletonList(genericHook),
65+
Collections.emptyMap());
66+
hookSupport.afterAllHooks(
67+
flagValueType,
68+
hookContext,
69+
FlagEvaluationDetails.builder().build(),
70+
Collections.singletonList(genericHook),
71+
Collections.emptyMap());
72+
hookSupport.errorHooks(
73+
flagValueType,
74+
hookContext,
75+
expectedException,
76+
Collections.singletonList(genericHook),
77+
Collections.emptyMap());
6378

6479
verify(genericHook).before(any(), any());
6580
verify(genericHook).after(any(), any(), any());

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

+9-10
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,17 @@
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.mock;
9+
import static org.mockito.Mockito.never;
10+
311
import dev.openfeature.sdk.exceptions.FatalError;
412
import dev.openfeature.sdk.fixtures.HookFixtures;
513
import dev.openfeature.sdk.testutils.TestEventsProvider;
14+
import java.util.HashMap;
615
import org.junit.jupiter.api.AfterEach;
716
import org.junit.jupiter.api.BeforeEach;
817
import org.junit.jupiter.api.DisplayName;
@@ -11,16 +20,6 @@
1120
import org.simplify4u.slf4jmock.LoggerMock;
1221
import org.slf4j.Logger;
1322

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;
23-
2423
class OpenFeatureClientTest implements HookFixtures {
2524

2625
private Logger logger;

0 commit comments

Comments
 (0)