Code review, minor suggestions/comment changes, questions. Looks great!

This commit is contained in:
Todd Baert 2022-06-15 16:05:21 -04:00
parent 95305ec513
commit dacf1cf9c2
No known key found for this signature in database
GPG Key ID: 6832CDB677D5E06D
46 changed files with 11 additions and 4 deletions

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

View File

@ -1,7 +1,7 @@
package dev.openfeature.javasdk;
/**
* We differ between the evaluation results that providers return and what is given to the end users. This is a common interface between them.
* This is the common interface between the evaluation results that providers return and what is given to the end users.
* @param <T> The type of flag being evaluated.
*/
public interface BaseEvaluation<T> {

View File

@ -1,6 +1,6 @@
package dev.openfeature.javasdk;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectMapper; // jackson is pretty standard, I'm glad you decided on this for object support
import lombok.*;
import java.time.ZonedDateTime;
@ -28,6 +28,8 @@ public class EvaluationContext {
}
// TODO Not sure if I should have sneakythrows or checked exceptions here..
// do you mean un-checked exceptions?
// I had no idea about SneakyThrows, but it seems like a nicer solution than swallowing in an unchecked exception.
@SneakyThrows
public <T> void addStructureAttribute(String key, T value) {
jsonAttributes.put(key, objMapper.writeValueAsString(value));

View File

@ -5,6 +5,7 @@ package dev.openfeature.javasdk;
*/
public interface FeatureProvider {
Metadata getMetadata();
// I think default methods are nicer than an abstract class, +1 to this.
ProviderEvaluation<Boolean> getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx, FlagEvaluationOptions options);
ProviderEvaluation<String> getStringEvaluation(String key, String defaultValue, EvaluationContext ctx, FlagEvaluationOptions options);
ProviderEvaluation<Integer> getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx, FlagEvaluationOptions options);

View File

@ -1,7 +1,7 @@
package dev.openfeature.javasdk;
/**
* An API for the type-specific fetch methods we offer end users.
* An API for the type-specific fetch methods offered to users.
*/
public interface Features {

View File

@ -9,7 +9,9 @@ import java.util.List;
@Data @Builder
public class FlagEvaluationOptions {
@Singular private List<Hook> hooks;
// I guess because we are using boxed types, we can pass "Object" as T everywhere to avoid the
// "raw types" warning, but I'm not sure if that's really too helpful beyond removing some warnings
@Singular private List<Hook<Object>> hooks;
@Builder.Default
private ImmutableMap<String, Object> hookHints = ImmutableMap.of();
}

View File

@ -93,6 +93,7 @@ public class OpenFeatureClient implements Client {
return details;
}
// errorHooks and afterAllHooks error handling seems consistent with spec, and what I implemented. I'm glad we landed here.
private void errorHooks(HookContext hookCtx, Exception e, List<Hook> hooks, ImmutableMap<String, Object> hints) {
for (Hook hook : hooks) {
try {
@ -122,6 +123,7 @@ public class OpenFeatureClient implements Client {
private EvaluationContext beforeHooks(HookContext hookCtx, List<Hook> hooks, ImmutableMap<String, Object> hints) {
// These traverse backwards from normal.
EvaluationContext ctx = hookCtx.getCtx();
// This is a bit counter-intuitive to me, just want to make sure... this is the stage that gets reversed because ArrayList.addAll() does a prepend, right?
for (Hook hook : Lists.reverse(hooks)) {
Optional<EvaluationContext> newCtx = hook.before(hookCtx, hints);
if (newCtx != null && newCtx.isPresent()) {