Skip to content

Commit d12c7d3

Browse files
lopitzlopitz
committed
fix/classcastexceptions
-fixes ClassCastException by only calling hooks, that support the flag value -moves out calling of hooks from OpenFeatureClient to dedicated class HookSupport -introduces method in Hok showing the supported flag value type -makes FlagEvaluationOptions immutable
1 parent 8cd60e0 commit d12c7d3

File tree

10 files changed

+345
-176
lines changed

10 files changed

+345
-176
lines changed

‎lib/src/main/java/dev/openfeature/javasdk/FlagEvaluationDetails.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@
1111
*/
1212
@Data @Builder
1313
public class FlagEvaluationDetails<T> implements BaseEvaluation<T> {
14-
String flagKey;
15-
T value;
16-
@Nullable String variant;
17-
Reason reason;
18-
@Nullable String errorCode;
14+
private String flagKey;
15+
private T value;
16+
@Nullable private String variant;
17+
private Reason reason;
18+
@Nullable private String errorCode;
1919

2020
public static <T> FlagEvaluationDetails<T> from(ProviderEvaluation<T> providerEval, String flagKey) {
2121
return FlagEvaluationDetails.<T>builder()

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

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

3-
import com.google.common.collect.ImmutableMap;
4-
import lombok.Builder;
5-
import lombok.Data;
6-
import lombok.Singular;
3+
import java.util.*;
74

8-
import java.util.List;
5+
import lombok.*;
96

10-
@Data @Builder
7+
@Value
8+
@Builder
119
public class FlagEvaluationOptions {
12-
@Singular private List<Hook> hooks;
10+
@Singular
11+
List<Hook> hooks;
1312
@Builder.Default
14-
private ImmutableMap<String, Object> hookHints = ImmutableMap.of();
13+
Map<String, Object> hookHints = Map.of();
1514
}

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

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,53 @@
11
package dev.openfeature.javasdk;
22

3-
import com.google.common.collect.ImmutableMap;
4-
5-
import java.util.Optional;
3+
import java.util.*;
64

75
/**
86
* An extension point which can run around flag resolution. They are intended to be used as a way to add custom logic
97
* to the lifecycle of flag evaluation.
8+
*
109
* @param <T> The type of the flag being evaluated.
1110
*/
1211
public interface Hook<T> {
1312
/**
1413
* Runs before flag is resolved.
15-
* @param ctx Information about the particular flag evaluation
14+
*
15+
* @param ctx Information about the particular flag evaluation
1616
* @param hints An immutable mapping of data for users to communicate to the hooks.
1717
* @return An optional {@link EvaluationContext}. If returned, it will be merged with the EvaluationContext instances from other hooks, the client and API.
1818
*/
19-
default Optional<EvaluationContext> before(HookContext<T> ctx, ImmutableMap<String, Object> hints) {
19+
default Optional<EvaluationContext> before(HookContext<T> ctx, Map<String, Object> hints) {
2020
return Optional.empty();
2121
}
2222

2323
/**
2424
* Runs after a flag is resolved.
25-
* @param ctx Information about the particular flag evaluation
25+
*
26+
* @param ctx Information about the particular flag evaluation
2627
* @param details Information about how the flag was resolved, including any resolved values.
27-
* @param hints An immutable mapping of data for users to communicate to the hooks.
28+
* @param hints An immutable mapping of data for users to communicate to the hooks.
2829
*/
29-
default void after(HookContext<T> ctx, FlagEvaluationDetails<T> details, ImmutableMap<String, Object> hints) {}
30+
default void after(HookContext<T> ctx, FlagEvaluationDetails<T> details, Map<String, Object> hints) {
31+
}
3032

3133
/**
3234
* Run when evaluation encounters an error. This will always run. Errors thrown will be swallowed.
33-
* @param ctx Information about the particular flag evaluation
35+
*
36+
* @param ctx Information about the particular flag evaluation
3437
* @param error The exception that was thrown.
3538
* @param hints An immutable mapping of data for users to communicate to the hooks.
3639
*/
37-
default void error(HookContext<T> ctx, Exception error, ImmutableMap<String, Object> hints) {}
40+
default void error(HookContext<T> ctx, Exception error, Map<String, Object> hints) {
41+
}
3842

3943
/**
4044
* Run after flag evaluation, including any error processing. This will always run. Errors will be swallowed.
41-
* @param ctx Information about the particular flag evaluation
45+
*
46+
* @param ctx Information about the particular flag evaluation
4247
* @param hints An immutable mapping of data for users to communicate to the hooks.
4348
*/
44-
default void finallyAfter(HookContext<T> ctx, ImmutableMap<String, Object> hints) {}
49+
default void finallyAfter(HookContext<T> ctx, Map<String, Object> hints) {
50+
}
51+
52+
FlagValueType supportsFlagValueType();
4553
}

‎lib/src/main/java/dev/openfeature/javasdk/HookSupport.java

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package dev.openfeature.javasdk;
2+
3+
import java.util.*;
4+
import java.util.function.Consumer;
5+
6+
import com.google.common.collect.Lists;
7+
import lombok.*;
8+
import org.slf4j.Logger;
9+
10+
@RequiredArgsConstructor
11+
class HookSupport {
12+
13+
private final Logger log;
14+
15+
public void errorHooks(FlagValueType flagValueType, HookContext hookCtx, Exception e, List<Hook> hooks, Map<String, Object> hints) {
16+
executeHooks(flagValueType, hooks, "error", hook -> hook.error(hookCtx, e, hints));
17+
}
18+
19+
public void afterAllHooks(FlagValueType flagValueType, HookContext hookCtx, List<Hook> hooks, Map<String, Object> hints) {
20+
executeHooks(flagValueType, hooks, "finally", hook -> hook.finallyAfter(hookCtx, hints));
21+
}
22+
23+
public void afterHooks(FlagValueType flagValueType, HookContext hookContext, FlagEvaluationDetails details, List<Hook> hooks, Map<String, Object> hints) {
24+
executeHooksUnsafe(flagValueType, hooks, hook -> hook.after(hookContext, details, hints));
25+
}
26+
27+
private <T> void executeHooks(
28+
FlagValueType flagValueType, List<Hook> hooks,
29+
String hookMethod,
30+
Consumer<Hook<T>> hookCode
31+
) {
32+
hooks
33+
.stream()
34+
.filter(hook -> hook.supportsFlagValueType() == flagValueType)
35+
.forEach(hook -> executeChecked(hook, hookCode, hookMethod));
36+
}
37+
38+
private <T> void executeHooksUnsafe(
39+
FlagValueType flagValueType, List<Hook> hooks,
40+
Consumer<Hook<T>> hookCode
41+
) {
42+
hooks
43+
.stream()
44+
.filter(hook -> hook.supportsFlagValueType() == flagValueType)
45+
.forEach(hookCode::accept);
46+
}
47+
48+
private <T> void executeChecked(Hook<T> hook, Consumer<Hook<T>> hookCode, String hookMethod) {
49+
try {
50+
hookCode.accept(hook);
51+
} catch (Exception exception) {
52+
log.error("Exception when running {} hooks {}", hookMethod, hook.getClass(), exception);
53+
}
54+
}
55+
56+
public EvaluationContext beforeHooks(HookContext hookCtx, List<Hook> hooks, Map<String, Object> hints) {
57+
// These traverse backwards from normal.
58+
EvaluationContext ctx = hookCtx.getCtx();
59+
for (Hook hook : Lists.reverse(hooks)) {
60+
Optional<EvaluationContext> newCtx = hook.before(hookCtx, hints);
61+
if (newCtx != null && newCtx.isPresent()) {
62+
ctx = EvaluationContext.merge(ctx, newCtx.get());
63+
hookCtx = hookCtx.withCtx(ctx);
64+
}
65+
}
66+
return ctx;
67+
}
68+
69+
}

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

Lines changed: 54 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -1,135 +1,98 @@
11
package dev.openfeature.javasdk;
22

3-
import com.google.common.collect.ImmutableList;
4-
import com.google.common.collect.ImmutableMap;
5-
import com.google.common.collect.Lists;
6-
import dev.openfeature.javasdk.exceptions.*;
7-
import lombok.Getter;
8-
import org.slf4j.Logger;
9-
import org.slf4j.LoggerFactory;
3+
import java.util.*;
104

11-
import java.util.ArrayList;
12-
import java.util.Arrays;
13-
import java.util.List;
14-
import java.util.Optional;
5+
import dev.openfeature.javasdk.exceptions.GeneralError;
6+
import dev.openfeature.javasdk.internal.ObjectUtils;
7+
import lombok.Getter;
8+
import org.slf4j.*;
159

16-
@SuppressWarnings("PMD.DataflowAnomalyAnalysis")
10+
@SuppressWarnings({"PMD.DataflowAnomalyAnalysis", "unchecked", "rawtypes"})
1711
public class OpenFeatureClient implements Client {
18-
private transient final OpenFeatureAPI openfeatureApi;
19-
@Getter private final String name;
20-
@Getter private final String version;
21-
@Getter private final List<Hook> clientHooks;
2212
private static final Logger log = LoggerFactory.getLogger(OpenFeatureClient.class);
2313

14+
private final OpenFeatureAPI openfeatureApi;
15+
@Getter
16+
private final String name;
17+
@Getter
18+
private final String version;
19+
@Getter
20+
private final List<Hook> clientHooks;
21+
private final HookSupport hookSupport;
22+
2423
public OpenFeatureClient(OpenFeatureAPI openFeatureAPI, String name, String version) {
2524
this.openfeatureApi = openFeatureAPI;
2625
this.name = name;
2726
this.version = version;
2827
this.clientHooks = new ArrayList<>();
28+
this.hookSupport = new HookSupport(log);
2929
}
3030

3131
@Override
3232
public void addHooks(Hook... hooks) {
33-
this.clientHooks.addAll(Arrays.asList(hooks));
33+
this.clientHooks.addAll(List.of(hooks));
3434
}
3535

36-
<T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key, T defaultValue, EvaluationContext ctx, FlagEvaluationOptions options) {
37-
FeatureProvider provider = this.openfeatureApi.getProvider();
38-
ImmutableMap<String, Object> hints = options.getHookHints();
36+
private <T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key, T defaultValue, EvaluationContext ctx, FlagEvaluationOptions options) {
37+
var flagOptions = ObjectUtils.defaultIfNull(options, () -> FlagEvaluationOptions.builder().build());
38+
FeatureProvider provider = openfeatureApi.getProvider();
39+
Map<String, Object> hints = Collections.unmodifiableMap(flagOptions.getHookHints());
3940
if (ctx == null) {
4041
ctx = new EvaluationContext();
4142
}
4243

4344
// merge of: API.context, client.context, invocation.context
4445

4546
// TODO: Context transformation?
46-
HookContext hookCtx = HookContext.from(key, type, this.getMetadata(), OpenFeatureAPI.getInstance().getProvider().getMetadata(), ctx, defaultValue);
47-
48-
List<Hook> mergedHooks;
49-
if (options != null && options.getHooks() != null) {
50-
mergedHooks = ImmutableList.<Hook>builder()
51-
.addAll(options.getHooks())
52-
.addAll(clientHooks)
53-
.addAll(openfeatureApi.getApiHooks())
54-
.build();
55-
} else {
56-
mergedHooks = clientHooks;
57-
}
47+
HookContext<T> hookCtx = HookContext.from(key, type, this.getMetadata(), openfeatureApi.getProvider().getMetadata(), ctx, defaultValue);
48+
49+
List<Hook> mergedHooks = ObjectUtils.merge(flagOptions.getHooks(), clientHooks, openfeatureApi.getApiHooks());
5850

5951
FlagEvaluationDetails<T> details = null;
6052
try {
61-
EvaluationContext ctxFromHook = this.beforeHooks(hookCtx, mergedHooks, hints);
53+
EvaluationContext ctxFromHook = hookSupport.beforeHooks(hookCtx, mergedHooks, hints);
6254
EvaluationContext invocationContext = EvaluationContext.merge(ctxFromHook, ctx);
6355

64-
ProviderEvaluation<T> providerEval;
65-
if (type == FlagValueType.BOOLEAN) {
66-
// TODO: Can we guarantee that defaultValue is a boolean? If not, how to we handle that?
67-
providerEval = (ProviderEvaluation<T>) provider.getBooleanEvaluation(key, (Boolean) defaultValue, invocationContext, options);
68-
} else if(type == FlagValueType.STRING) {
69-
providerEval = (ProviderEvaluation<T>) provider.getStringEvaluation(key, (String) defaultValue, invocationContext, options);
70-
} else if (type == FlagValueType.INTEGER) {
71-
providerEval = (ProviderEvaluation<T>) provider.getIntegerEvaluation(key, (Integer) defaultValue, invocationContext, options);
72-
} else if (type == FlagValueType.OBJECT) {
73-
providerEval = (ProviderEvaluation<T>) provider.getObjectEvaluation(key, defaultValue, invocationContext, options);
74-
} else {
75-
throw new GeneralError("Unknown flag type");
76-
}
56+
ProviderEvaluation<T> providerEval = (ProviderEvaluation<T>) createProviderEvaluation(type, key, defaultValue, options, provider, invocationContext);
7757

7858
details = FlagEvaluationDetails.from(providerEval, key);
79-
this.afterHooks(hookCtx, details, mergedHooks, hints);
59+
hookSupport.afterHooks(type, hookCtx, details, mergedHooks, hints);
8060
} catch (Exception e) {
8161
log.error("Unable to correctly evaluate flag with key {} due to exception {}", key, e.getMessage());
8262
if (details == null) {
83-
details = FlagEvaluationDetails.<T>builder().value(defaultValue).reason(Reason.ERROR).build();
63+
details = FlagEvaluationDetails.<T>builder().build();
8464
}
85-
details.value = defaultValue;
86-
details.reason = Reason.ERROR;
87-
details.errorCode = e.getMessage();
88-
this.errorHooks(hookCtx, e, mergedHooks, hints);
65+
details.setValue(defaultValue);
66+
details.setReason(Reason.ERROR);
67+
details.setErrorCode(e.getMessage());
68+
hookSupport.errorHooks(type, hookCtx, e, mergedHooks, hints);
8969
} finally {
90-
this.afterAllHooks(hookCtx, mergedHooks, hints);
70+
hookSupport.afterAllHooks(type, hookCtx, mergedHooks, hints);
9171
}
9272

9373
return details;
9474
}
9575

96-
private void errorHooks(HookContext hookCtx, Exception e, List<Hook> hooks, ImmutableMap<String, Object> hints) {
97-
for (Hook hook : hooks) {
98-
try {
99-
hook.error(hookCtx, e, hints);
100-
} catch (Exception inner_exception) {
101-
log.error("Exception when running error hooks " + hook.getClass().toString(), inner_exception);
102-
}
103-
}
104-
}
105-
106-
private void afterAllHooks(HookContext hookCtx, List<Hook> hooks, ImmutableMap<String, Object> hints) {
107-
for (Hook hook : hooks) {
108-
try {
109-
hook.finallyAfter(hookCtx, hints);
110-
} catch (Exception inner_exception) {
111-
log.error("Exception when running finally hooks " + hook.getClass().toString(), inner_exception);
112-
}
113-
}
114-
}
115-
116-
private <T> void afterHooks(HookContext hookContext, FlagEvaluationDetails<T> details, List<Hook> hooks, ImmutableMap<String, Object> hints) {
117-
for (Hook hook : hooks) {
118-
hook.after(hookContext, details, hints);
119-
}
120-
}
121-
122-
private EvaluationContext beforeHooks(HookContext hookCtx, List<Hook> hooks, ImmutableMap<String, Object> hints) {
123-
// These traverse backwards from normal.
124-
EvaluationContext ctx = hookCtx.getCtx();
125-
for (Hook hook : Lists.reverse(hooks)) {
126-
Optional<EvaluationContext> newCtx = hook.before(hookCtx, hints);
127-
if (newCtx != null && newCtx.isPresent()) {
128-
ctx = EvaluationContext.merge(ctx, newCtx.get());
129-
hookCtx = hookCtx.withCtx(ctx);
130-
}
76+
private <T> ProviderEvaluation<?> createProviderEvaluation(
77+
FlagValueType type,
78+
String key,
79+
T defaultValue,
80+
FlagEvaluationOptions options,
81+
FeatureProvider provider,
82+
EvaluationContext invocationContext
83+
) {
84+
switch (type) {
85+
case BOOLEAN:
86+
return provider.getBooleanEvaluation(key, (Boolean) defaultValue, invocationContext, options);
87+
case STRING:
88+
return provider.getStringEvaluation(key, (String) defaultValue, invocationContext, options);
89+
case INTEGER:
90+
return provider.getIntegerEvaluation(key, (Integer) defaultValue, invocationContext, options);
91+
case OBJECT:
92+
return provider.getObjectEvaluation(key, defaultValue, invocationContext, options);
93+
default:
94+
throw new GeneralError("Unknown flag type");
13195
}
132-
return ctx;
13396
}
13497

13598
@Override
@@ -179,7 +142,7 @@ public String getStringValue(String key, String defaultValue, EvaluationContext
179142

180143
@Override
181144
public FlagEvaluationDetails<String> getStringDetails(String key, String defaultValue) {
182-
return getStringDetails(key, defaultValue, null);
145+
return getStringDetails(key, defaultValue, null);
183146
}
184147

185148
@Override
@@ -254,11 +217,6 @@ public <T> FlagEvaluationDetails<T> getObjectDetails(String key, T defaultValue,
254217

255218
@Override
256219
public Metadata getMetadata() {
257-
return new Metadata() {
258-
@Override
259-
public String getName() {
260-
return name;
261-
}
262-
};
220+
return () -> name;
263221
}
264222
}

0 commit comments

Comments
 (0)