Skip to content

Commit 4c4e7c3

Browse files
aepfliaepfli
committed
fixup: tests
Signed-off-by: Simon Schrottner <[email protected]>
1 parent 81f2e4f commit 4c4e7c3

File tree

3 files changed

+74
-42
lines changed

3 files changed

+74
-42
lines changed

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

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,10 @@
22

33
import java.util.ArrayList;
44
import java.util.Collections;
5-
import java.util.HashMap;
65
import java.util.List;
76
import java.util.Map;
87
import java.util.Optional;
98
import java.util.function.BiConsumer;
10-
import java.util.function.Consumer;
119
import lombok.RequiredArgsConstructor;
1210
import lombok.extern.slf4j.Slf4j;
1311
import org.apache.commons.lang3.tuple.Pair;
@@ -18,7 +16,10 @@
1816
class HookSupport {
1917

2018
public EvaluationContext beforeHooks(
21-
FlagValueType flagValueType, HookContext hookCtx, List<Pair<Hook, HookData>> hookDataPairs, Map<String, Object> hints) {
19+
FlagValueType flagValueType,
20+
HookContext hookCtx,
21+
List<Pair<Hook, HookData>> hookDataPairs,
22+
Map<String, Object> hints) {
2223
return callBeforeHooks(flagValueType, hookCtx, hookDataPairs, hints);
2324
}
2425

@@ -28,7 +29,8 @@ public void afterHooks(
2829
FlagEvaluationDetails details,
2930
List<Pair<Hook, HookData>> hookDataPairs,
3031
Map<String, Object> hints) {
31-
executeHooksUnchecked(flagValueType, hookDataPairs, hookContext, (hook, ctx) -> hook.after(ctx, details, hints));
32+
executeHooksUnchecked(
33+
flagValueType, hookDataPairs, hookContext, (hook, ctx) -> hook.after(ctx, details, hints));
3234
}
3335

3436
public void afterAllHooks(
@@ -37,7 +39,12 @@ public void afterAllHooks(
3739
FlagEvaluationDetails details,
3840
List<Pair<Hook, HookData>> hookDataPairs,
3941
Map<String, Object> hints) {
40-
executeHooks(flagValueType, hookDataPairs, hookCtx, "finally", (hook, ctx) -> hook.finallyAfter(ctx, details, hints));
42+
executeHooks(
43+
flagValueType,
44+
hookDataPairs,
45+
hookCtx,
46+
"finally",
47+
(hook, ctx) -> hook.finallyAfter(ctx, details, hints));
4148
}
4249

4350
public void errorHooks(
@@ -58,7 +65,11 @@ public List<Pair<Hook, HookData>> getHookDataPairs(List<Hook> hooks) {
5865
}
5966

6067
private <T> void executeHooks(
61-
FlagValueType flagValueType, List<Pair<Hook, HookData>> hookDataPairs, HookContext hookContext, String hookMethod, BiConsumer<Hook<T>, HookContext> hookCode) {
68+
FlagValueType flagValueType,
69+
List<Pair<Hook, HookData>> hookDataPairs,
70+
HookContext hookContext,
71+
String hookMethod,
72+
BiConsumer<Hook<T>, HookContext> hookCode) {
6273
if (hookDataPairs != null) {
6374
for (Pair<Hook, HookData> hookDataPair : hookDataPairs) {
6475
Hook hook = hookDataPair.getLeft();
@@ -71,7 +82,12 @@ private <T> void executeHooks(
7182
}
7283

7384
// before, error, and finally hooks shouldn't throw
74-
private <T> void executeChecked(Hook<T> hook, HookData hookData, HookContext hookContext, BiConsumer<Hook<T>, HookContext> hookCode, String hookMethod) {
85+
private <T> void executeChecked(
86+
Hook<T> hook,
87+
HookData hookData,
88+
HookContext hookContext,
89+
BiConsumer<Hook<T>, HookContext> hookCode,
90+
String hookMethod) {
7591
try {
7692
var hookCtxWithData = hookContext.withHookData(hookData);
7793
hookCode.accept(hook, hookCtxWithData);
@@ -85,7 +101,11 @@ private <T> void executeChecked(Hook<T> hook, HookData hookData, HookContext hoo
85101
}
86102

87103
// after hooks can throw in order to do validation
88-
private <T> void executeHooksUnchecked(FlagValueType flagValueType, List<Pair<Hook, HookData>> hookDataPairs, HookContext hookContext, BiConsumer<Hook<T>, HookContext> hookCode) {
104+
private <T> void executeHooksUnchecked(
105+
FlagValueType flagValueType,
106+
List<Pair<Hook, HookData>> hookDataPairs,
107+
HookContext hookContext,
108+
BiConsumer<Hook<T>, HookContext> hookCode) {
89109
if (hookDataPairs != null) {
90110
for (Pair<Hook, HookData> hookDataPair : hookDataPairs) {
91111
Hook hook = hookDataPair.getLeft();
@@ -99,7 +119,10 @@ private <T> void executeHooksUnchecked(FlagValueType flagValueType, List<Pair<Ho
99119
}
100120

101121
private EvaluationContext callBeforeHooks(
102-
FlagValueType flagValueType, HookContext hookCtx, List<Pair<Hook, HookData>> hookDataPairs, Map<String, Object> hints) {
122+
FlagValueType flagValueType,
123+
HookContext hookCtx,
124+
List<Pair<Hook, HookData>> hookDataPairs,
125+
Map<String, Object> hints) {
103126
// These traverse backwards from normal.
104127
List<Pair<Hook, HookData>> reversedHooks = new ArrayList<>(hookDataPairs);
105128
Collections.reverse(reversedHooks);

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

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,11 @@
3131
*/
3232
@Slf4j
3333
@SuppressWarnings({
34-
"PMD.DataflowAnomalyAnalysis",
35-
"PMD.BeanMembersShouldSerialize",
36-
"PMD.UnusedLocalVariable",
37-
"unchecked",
38-
"rawtypes"
34+
"PMD.DataflowAnomalyAnalysis",
35+
"PMD.BeanMembersShouldSerialize",
36+
"PMD.UnusedLocalVariable",
37+
"unchecked",
38+
"rawtypes"
3939
})
4040
@Deprecated() // TODO: eventually we will make this non-public. See issue #872
4141
public class OpenFeatureClient implements Client {
@@ -174,19 +174,20 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
174174
// provider must be accessed once to maintain a consistent reference
175175
var provider = stateManager.getProvider();
176176
var state = stateManager.getState();
177-
177+
afterHookContext = HookContext.from(
178+
key,
179+
type,
180+
this.getMetadata(),
181+
provider.getMetadata(),
182+
mergeEvaluationContext(ctx),
183+
defaultValue);
178184
mergedHooks = ObjectUtils.merge(
179-
provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, openfeatureApi.getMutableHooks());
185+
provider.getProviderHooks(), flagOptions.getHooks(), clientHooks,
186+
openfeatureApi.getMutableHooks());
180187
hookDataPairs = hookSupport.getHookDataPairs(mergedHooks);
181188
var mergedCtx = hookSupport.beforeHooks(
182189
type,
183-
HookContext.from(
184-
key,
185-
type,
186-
this.getMetadata(),
187-
provider.getMetadata(),
188-
mergeEvaluationContext(ctx),
189-
defaultValue),
190+
afterHookContext,
190191
hookDataPairs,
191192
hints);
192193

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

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,10 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() {
4040
HookSupport hookSupport = new HookSupport();
4141

4242
EvaluationContext result = hookSupport.beforeHooks(
43-
FlagValueType.STRING, hookContext, hookSupport.getHookDataPairs(Arrays.asList(hook1, hook2)), Collections.emptyMap());
43+
FlagValueType.STRING,
44+
hookContext,
45+
hookSupport.getHookDataPairs(Arrays.asList(hook1, hook2)),
46+
Collections.emptyMap());
4447

4548
assertThat(result.getValue("bla").asString()).isEqualTo("blubber");
4649
assertThat(result.getValue("foo").asString()).isEqualTo("bar");
@@ -65,8 +68,7 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) {
6568
.providerMetadata(() -> "provider")
6669
.build();
6770

68-
hookSupport.beforeHooks(
69-
flagValueType, hookContext, hookDataPairs, Collections.emptyMap());
71+
hookSupport.beforeHooks(flagValueType, hookContext, hookDataPairs, Collections.emptyMap());
7072
hookSupport.afterHooks(
7173
flagValueType,
7274
hookContext,
@@ -79,12 +81,7 @@ void shouldAlwaysCallGenericHook(FlagValueType flagValueType) {
7981
FlagEvaluationDetails.builder().build(),
8082
hookDataPairs,
8183
Collections.emptyMap());
82-
hookSupport.errorHooks(
83-
flagValueType,
84-
hookContext,
85-
expectedException,
86-
hookDataPairs,
87-
Collections.emptyMap());
84+
hookSupport.errorHooks(flagValueType, hookContext, expectedException, hookDataPairs, Collections.emptyMap());
8885

8986
verify(genericHook).before(any(), any());
9087
verify(genericHook).after(any(), any(), any());
@@ -102,7 +99,7 @@ void shouldPassDataAcrossStages(FlagValueType flagValueType) {
10299
TestHookWithData testHook = new TestHookWithData("test-key", "value");
103100
var pairs = hookSupport.getHookDataPairs(List.of(testHook));
104101

105-
callAllHooks(flagValueType, hookSupport, hookContext, pairs);
102+
callAllHooks(flagValueType, hookSupport, hookContext, testHook);
106103

107104
assertHookData(testHook, "value");
108105
}
@@ -135,13 +132,12 @@ void shouldIsolateDataBetweenSameHooks(FlagValueType flagValueType) {
135132
TestHookWithData testHook = new TestHookWithData("test-key", "value-1");
136133

137134
// run hooks first time
138-
var pairs = hookSupport.getHookDataPairs(List.of(testHook));
139-
callAllHooks(flagValueType, hookSupport, hookContext, pairs);
135+
callAllHooks(flagValueType, hookSupport, hookContext, testHook);
140136
assertHookData(testHook, "value-1");
141137

142138
// re-run with different value, will throw if HookData contains already data
143139
testHook.value = "value-2";
144-
callAllHooks(flagValueType, hookSupport, hookContext, pairs);
140+
callAllHooks(flagValueType, hookSupport, hookContext, testHook);
145141

146142
assertHookData(testHook, "value-2");
147143
}
@@ -166,15 +162,28 @@ private static void assertHookData(TestHookWithData testHook1, String expected)
166162
assertThat(testHook1.onErrorValue).isEqualTo(expected);
167163
}
168164

169-
private static void callAllHooks(FlagValueType flagValueType, HookSupport hookSupport, HookContext<Object> hookContext,
170-
List<Pair<Hook, HookData>> pairs) {
165+
private static void callAllHooks(
166+
FlagValueType flagValueType,
167+
HookSupport hookSupport,
168+
HookContext<Object> hookContext,
169+
TestHookWithData testHook) {
170+
var pairs = hookSupport.getHookDataPairs(List.of(testHook));
171+
callAllHooks(flagValueType, hookSupport, hookContext, pairs);
172+
}
173+
174+
private static void callAllHooks(
175+
FlagValueType flagValueType,
176+
HookSupport hookSupport,
177+
HookContext<Object> hookContext,
178+
List<Pair<Hook, HookData>> pairs ) {
171179
hookSupport.beforeHooks(flagValueType, hookContext, pairs, Collections.emptyMap());
172-
hookSupport.afterHooks(flagValueType, hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap());
180+
hookSupport.afterHooks(
181+
flagValueType, hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap());
173182
hookSupport.errorHooks(flagValueType, hookContext, new Exception(), pairs, Collections.emptyMap());
174-
hookSupport.afterAllHooks(flagValueType, hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap());
183+
hookSupport.afterAllHooks(
184+
flagValueType, hookContext, new FlagEvaluationDetails<>(), pairs, Collections.emptyMap());
175185
}
176186

177-
178187
private Object createDefaultValue(FlagValueType flagValueType) {
179188
switch (flagValueType) {
180189
case INTEGER:
@@ -239,6 +248,5 @@ public void error(HookContext ctx, Exception error, Map hints) {
239248
public void finallyAfter(HookContext ctx, FlagEvaluationDetails details, Map hints) {
240249
onFinallyAfterValue = ctx.getHookData().get(key);
241250
}
242-
243251
}
244252
}

0 commit comments

Comments
 (0)