Skip to content

Commit a4fc956

Browse files
aepfliaepfli
committed
fixup: pr comments and extraction of own pair class
Signed-off-by: Simon Schrottner <[email protected]>
1 parent 6d4f700 commit a4fc956

File tree

5 files changed

+57
-62
lines changed

5 files changed

+57
-62
lines changed

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

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import java.util.function.BiConsumer;
99
import lombok.RequiredArgsConstructor;
1010
import lombok.extern.slf4j.Slf4j;
11-
import org.apache.commons.lang3.tuple.Pair;
1211

1312
@Slf4j
1413
@RequiredArgsConstructor
@@ -56,10 +55,12 @@ public void errorHooks(
5655
executeHooks(flagValueType, hookDataPairs, hookCtx, "error", (hook, ctx) -> hook.error(ctx, e, hints));
5756
}
5857

59-
public List<Pair<Hook, HookData>> getHookDataPairs(List<Hook> hooks) {
58+
public List<Pair<Hook, HookData>> getHookDataPairs(List<Hook> hooks, FlagValueType flagValueType) {
6059
var pairs = new ArrayList<Pair<Hook, HookData>>();
6160
for (Hook hook : hooks) {
62-
pairs.add(Pair.of(hook, HookData.create()));
61+
if (hook.supportsFlagValueType(flagValueType)) {
62+
pairs.add(Pair.of(hook, HookData.create()));
63+
}
6364
}
6465
return pairs;
6566
}
@@ -127,15 +128,6 @@ private EvaluationContext callBeforeHooks(
127128
List<Pair<Hook, HookData>> reversedHooks = new ArrayList<>(hookDataPairs);
128129
Collections.reverse(reversedHooks);
129130
EvaluationContext context = hookCtx.getCtx();
130-
/*
131-
// Create hook data for each hook instance
132-
Map<Hook, HookData> hookDataMap = new HashMap<>();
133-
for (Hook hook : reversedHooks) {
134-
if (hook.supportsFlagValueType(flagValueType)) {
135-
hookDataMap.put(hook, HookData.create());
136-
}
137-
}
138-
*/
139131

140132
for (Pair<Hook, HookData> hookDataPair : reversedHooks) {
141133
Hook hook = hookDataPair.getLeft();

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

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import java.util.function.Consumer;
2020
import lombok.Getter;
2121
import lombok.extern.slf4j.Slf4j;
22-
import org.apache.commons.lang3.tuple.Pair;
2322

2423
/**
2524
* OpenFeature Client implementation.
@@ -167,21 +166,21 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
167166
FlagEvaluationDetails<T> details = null;
168167
List<Hook> mergedHooks;
169168
List<Pair<Hook, HookData>> hookDataPairs = null;
170-
HookContext<T> afterHookContext = null;
169+
HookContext<T> hookContext = null;
171170

172171
try {
173172
final var stateManager = openfeatureApi.getFeatureProviderStateManager(this.domain);
174173
// provider must be accessed once to maintain a consistent reference
175174
final var provider = stateManager.getProvider();
176175
final var state = stateManager.getState();
177-
afterHookContext = HookContext.from(
176+
hookContext = HookContext.from(
178177
key, type, this.getMetadata(), provider.getMetadata(), mergeEvaluationContext(ctx), defaultValue);
179178
mergedHooks = ObjectUtils.merge(
180179
provider.getProviderHooks(), flagOptions.getHooks(), clientHooks, openfeatureApi.getMutableHooks());
181-
hookDataPairs = hookSupport.getHookDataPairs(mergedHooks);
182-
var mergedCtx = hookSupport.beforeHooks(type, afterHookContext, hookDataPairs, hints);
180+
hookDataPairs = hookSupport.getHookDataPairs(mergedHooks, type);
181+
var mergedCtx = hookSupport.beforeHooks(type, hookContext, hookDataPairs, hints);
183182

184-
afterHookContext =
183+
hookContext =
185184
HookContext.from(key, type, this.getMetadata(), provider.getMetadata(), mergedCtx, defaultValue);
186185

187186
// "short circuit" if the provider is in NOT_READY or FATAL state
@@ -200,9 +199,9 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
200199
var error =
201200
ExceptionUtils.instantiateErrorByErrorCode(details.getErrorCode(), details.getErrorMessage());
202201
enrichDetailsWithErrorDefaults(defaultValue, details);
203-
hookSupport.errorHooks(type, afterHookContext, error, hookDataPairs, hints);
202+
hookSupport.errorHooks(type, hookContext, error, hookDataPairs, hints);
204203
} else {
205-
hookSupport.afterHooks(type, afterHookContext, details, hookDataPairs, hints);
204+
hookSupport.afterHooks(type, hookContext, details, hookDataPairs, hints);
206205
}
207206
} catch (Exception e) {
208207
if (details == null) {
@@ -215,9 +214,9 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(
215214
}
216215
details.setErrorMessage(e.getMessage());
217216
enrichDetailsWithErrorDefaults(defaultValue, details);
218-
hookSupport.errorHooks(type, afterHookContext, e, hookDataPairs, hints);
217+
hookSupport.errorHooks(type, hookContext, e, hookDataPairs, hints);
219218
} finally {
220-
hookSupport.afterAllHooks(type, afterHookContext, details, hookDataPairs, hints);
219+
hookSupport.afterAllHooks(type, hookContext, details, hookDataPairs, hints);
221220
}
222221

223222
return details;

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
package dev.openfeature.sdk;
2+
3+
class Pair<K, V> {
4+
private final K key;
5+
private final V value;
6+
7+
private Pair(K key, V value) {
8+
this.key = key;
9+
this.value = value;
10+
}
11+
12+
public K getKey() {
13+
return key;
14+
}
15+
16+
public V getValue() {
17+
return value;
18+
}
19+
20+
public K getLeft() {
21+
return key;
22+
}
23+
24+
public V getRight() {
25+
return value;
26+
}
27+
28+
@Override
29+
public String toString() {
30+
return "Pair{" + "key=" + key + ", value=" + value + '}';
31+
}
32+
33+
public static <K, V> Pair<K, V> of(K key, V value) {
34+
return new Pair<>(key, value);
35+
}
36+
}

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

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

33
import static org.junit.jupiter.api.Assertions.*;
44

5-
import java.util.concurrent.CountDownLatch;
6-
import java.util.concurrent.ExecutorService;
7-
import java.util.concurrent.Executors;
8-
import java.util.concurrent.TimeUnit;
95
import org.junit.jupiter.api.Test;
106

117
class HookDataTest {
@@ -72,34 +68,6 @@ void shouldOverwriteExistingValues() {
7268
assertEquals("updated", hookData.get("key"));
7369
}
7470

75-
@Test
76-
void shouldBeThreadSafe() throws InterruptedException {
77-
HookData hookData = HookData.create();
78-
int threadCount = 10;
79-
int operationsPerThread = 100;
80-
CountDownLatch latch = new CountDownLatch(threadCount);
81-
ExecutorService executor = Executors.newFixedThreadPool(threadCount);
82-
83-
for (int i = 0; i < threadCount; i++) {
84-
final int threadId = i;
85-
executor.submit(() -> {
86-
try {
87-
for (int j = 0; j < operationsPerThread; j++) {
88-
String key = "thread-" + threadId + "-key-" + j;
89-
String value = "thread-" + threadId + "-value-" + j;
90-
hookData.set(key, value);
91-
assertEquals(value, hookData.get(key));
92-
}
93-
} finally {
94-
latch.countDown();
95-
}
96-
});
97-
}
98-
99-
assertTrue(latch.await(10, TimeUnit.SECONDS));
100-
executor.shutdown();
101-
}
102-
10371
@Test
10472
void shouldSupportNullValues() {
10573
HookData hookData = HookData.create();

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import java.util.List;
1313
import java.util.Map;
1414
import java.util.Optional;
15-
import org.apache.commons.lang3.tuple.Pair;
1615
import org.junit.jupiter.api.DisplayName;
1716
import org.junit.jupiter.api.Test;
1817
import org.junit.jupiter.params.ParameterizedTest;
@@ -25,9 +24,10 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() {
2524
Map<String, Value> attributes = new HashMap<>();
2625
attributes.put("baseKey", new Value("baseValue"));
2726
EvaluationContext baseContext = new ImmutableContext(attributes);
27+
FlagValueType valueType = FlagValueType.STRING;
2828
HookContext<String> hookContext = HookContext.<String>builder()
2929
.flagKey("flagKey")
30-
.type(FlagValueType.STRING)
30+
.type(valueType)
3131
.defaultValue("defaultValue")
3232
.ctx(baseContext)
3333
.clientMetadata(() -> "client")
@@ -40,9 +40,9 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() {
4040
HookSupport hookSupport = new HookSupport();
4141

4242
EvaluationContext result = hookSupport.beforeHooks(
43-
FlagValueType.STRING,
43+
valueType,
4444
hookContext,
45-
hookSupport.getHookDataPairs(Arrays.asList(hook1, hook2)),
45+
hookSupport.getHookDataPairs(Arrays.asList(hook1, hook2), valueType),
4646
Collections.emptyMap());
4747

4848
assertThat(result.getValue("bla").asString()).isEqualTo("blubber");
@@ -56,7 +56,7 @@ void shouldMergeEvaluationContextsOnBeforeHooksCorrectly() {
5656
void shouldAlwaysCallGenericHook(FlagValueType flagValueType) {
5757
Hook<?> genericHook = mockGenericHook();
5858
HookSupport hookSupport = new HookSupport();
59-
var hookDataPairs = hookSupport.getHookDataPairs(Collections.singletonList(genericHook));
59+
var hookDataPairs = hookSupport.getHookDataPairs(Collections.singletonList(genericHook), flagValueType);
6060
EvaluationContext baseContext = new ImmutableContext();
6161
IllegalStateException expectedException = new IllegalStateException("All fine, just a test");
6262
HookContext<Object> hookContext = HookContext.<Object>builder()
@@ -97,7 +97,7 @@ void shouldPassDataAcrossStages(FlagValueType flagValueType) {
9797
HookContext<Object> hookContext = getObjectHookContext(flagValueType);
9898

9999
TestHookWithData testHook = new TestHookWithData("test-key", "value");
100-
var pairs = hookSupport.getHookDataPairs(List.of(testHook));
100+
var pairs = hookSupport.getHookDataPairs(List.of(testHook), flagValueType);
101101

102102
callAllHooks(flagValueType, hookSupport, hookContext, testHook);
103103

@@ -113,7 +113,7 @@ void shouldIsolateDataBetweenHooks(FlagValueType flagValueType) {
113113

114114
TestHookWithData testHook1 = new TestHookWithData("test-key", "value-1");
115115
TestHookWithData testHook2 = new TestHookWithData("test-key", "value-2");
116-
var pairs = hookSupport.getHookDataPairs(List.of(testHook1, testHook2));
116+
var pairs = hookSupport.getHookDataPairs(List.of(testHook1, testHook2), flagValueType);
117117

118118
callAllHooks(flagValueType, hookSupport, hookContext, pairs);
119119

@@ -167,7 +167,7 @@ private static void callAllHooks(
167167
HookSupport hookSupport,
168168
HookContext<Object> hookContext,
169169
TestHookWithData testHook) {
170-
var pairs = hookSupport.getHookDataPairs(List.of(testHook));
170+
var pairs = hookSupport.getHookDataPairs(List.of(testHook), flagValueType);
171171
callAllHooks(flagValueType, hookSupport, hookContext, pairs);
172172
}
173173

0 commit comments

Comments
 (0)