From aaa924f94c13123479a4ab88b4579059a970113c Mon Sep 17 00:00:00 2001 From: Justin Abrahms Date: Wed, 25 May 2022 17:57:32 -0700 Subject: [PATCH] Support for eval context & merging relevant thereof --- .../javasdk/EvaluationContext.java | 147 ++++-------------- .../java/dev/openfeature/javasdk/Hook.java | 2 +- .../dev/openfeature/javasdk/HookContext.java | 3 +- .../javasdk/OpenFeatureClient.java | 11 +- .../openfeature/javasdk/EvalContextTests.java | 79 ++-------- .../javasdk/FlagEvaluationSpecTests.java | 2 +- .../openfeature/javasdk/HookSpecTests.java | 60 +++++++ 7 files changed, 114 insertions(+), 190 deletions(-) diff --git a/lib/src/main/java/dev/openfeature/javasdk/EvaluationContext.java b/lib/src/main/java/dev/openfeature/javasdk/EvaluationContext.java index 34e4fd07..7b4792ff 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/EvaluationContext.java +++ b/lib/src/main/java/dev/openfeature/javasdk/EvaluationContext.java @@ -1,33 +1,21 @@ package dev.openfeature.javasdk; +import lombok.EqualsAndHashCode; import lombok.Getter; +import lombok.Setter; +import lombok.ToString; import java.time.ZonedDateTime; import java.time.format.DateTimeFormatter; import java.util.HashMap; import java.util.Map; +@ToString @EqualsAndHashCode public class EvaluationContext { - @Getter private final String targetingKey; + @Setter @Getter private String targetingKey; private final Map integerAttributes; private final Map stringAttributes; - - private enum KNOWN_KEYS { - EMAIL, - FIRST_NAME, - LAST_NAME, - NAME, - IP, - TZ, - LOCALE, - COUNTRY_CODE, - ENVIRONMENT, - APPLICATION, - VERSION, - TIMESTAMP, - } - EvaluationContext() { this.targetingKey = ""; this.integerAttributes = new HashMap<>(); @@ -62,6 +50,8 @@ public class EvaluationContext { this.stringAttributes.put(key, value.format(DateTimeFormatter.ISO_ZONED_DATE_TIME)); } + // TODO: addStructure or similar. + public ZonedDateTime getDatetimeAttribute(String key) { String attr = this.stringAttributes.get(key); if (attr == null) { @@ -70,107 +60,34 @@ public class EvaluationContext { return ZonedDateTime.parse(attr, DateTimeFormatter.ISO_ZONED_DATE_TIME); } - public String getEmail() { - return this.stringAttributes.get(KNOWN_KEYS.EMAIL.toString()); - } - - public String getFirstName() { - return this.stringAttributes.get(KNOWN_KEYS.FIRST_NAME.toString()); - } - - public String getLastName() { - return this.stringAttributes.get(KNOWN_KEYS.LAST_NAME.toString()); - } - - public String getName() { - return this.stringAttributes.get(KNOWN_KEYS.NAME.toString()); - } - - public String getIp() { - return this.stringAttributes.get(KNOWN_KEYS.IP.toString()); - } - - public String getTz() { - return this.stringAttributes.get(KNOWN_KEYS.TZ.toString()); - } - - public String getLocale() { - return this.stringAttributes.get(KNOWN_KEYS.LOCALE.toString()); - } - - public String getCountryCode() { - return this.stringAttributes.get(KNOWN_KEYS.COUNTRY_CODE.toString()); - } - - public String getEnvironment() { - return this.stringAttributes.get(KNOWN_KEYS.ENVIRONMENT.toString()); - } - - public String getApplication() { - return this.stringAttributes.get(KNOWN_KEYS.APPLICATION.toString()); - } - - public String getVersion() { - return this.stringAttributes.get(KNOWN_KEYS.VERSION.toString()); - } - - public ZonedDateTime getTimestamp() { - return getDatetimeAttribute(KNOWN_KEYS.TIMESTAMP.toString()); - } - - public void setEmail(String email) { - this.stringAttributes.put(KNOWN_KEYS.EMAIL.toString(), email); - } - - public void setFirstName(String firstname) { - this.stringAttributes.put(KNOWN_KEYS.FIRST_NAME.toString(), firstname); - } - - public void setLastName(String lastname) { - this.stringAttributes.put(KNOWN_KEYS.LAST_NAME.toString(), lastname); - } - - public void setName(String name) { - this.stringAttributes.put(KNOWN_KEYS.NAME.toString(), name); - } - - public void setIp(String ip) { - this.stringAttributes.put(KNOWN_KEYS.IP.toString(), ip); - } - - public void setTz(String tz) { - this.stringAttributes.put(KNOWN_KEYS.TZ.toString(), tz); - } - - public void setLocale(String locale) { - this.stringAttributes.put(KNOWN_KEYS.LOCALE.toString(), locale); - } - - public void setCountryCode(String countryCode) { - this.stringAttributes.put(KNOWN_KEYS.COUNTRY_CODE.toString(), countryCode); - } - - public void setEnvironment(String environment) { - this.stringAttributes.put(KNOWN_KEYS.ENVIRONMENT.toString(), environment); - } - - public void setApplication(String application) { - this.stringAttributes.put(KNOWN_KEYS.APPLICATION.toString(), application); - } - - public void setVersion(String version) { - this.stringAttributes.put(KNOWN_KEYS.VERSION.toString(), version); - } - - public void setTimestamp(ZonedDateTime timestamp) { - addDatetimeAttribute(KNOWN_KEYS.TIMESTAMP.toString(), timestamp); - } - /** * Merges two EvaluationContext objects with the second overriding the first in case of conflict. */ public static EvaluationContext merge(EvaluationContext ctx1, EvaluationContext ctx2) { - // TODO(abrahms): Actually implement this when we know what the fields of EC are. - return ctx1; + EvaluationContext ec = new EvaluationContext(); + for (Map.Entry e : ctx1.integerAttributes.entrySet()) { + ec.addIntegerAttribute(e.getKey(), e.getValue()); + } + + for (Map.Entry e : ctx2.integerAttributes.entrySet()) { + ec.addIntegerAttribute(e.getKey(), e.getValue()); + } + + for (Map.Entry e : ctx1.stringAttributes.entrySet()) { + ec.addStringAttribute(e.getKey(), e.getValue()); + } + + for (Map.Entry e : ctx2.stringAttributes.entrySet()) { + ec.addStringAttribute(e.getKey(), e.getValue()); + } + if (ctx1.getTargetingKey() != null) { + ec.setTargetingKey(ctx1.getTargetingKey()); + } + + if (ctx2.getTargetingKey() != null) { + ec.setTargetingKey(ctx2.getTargetingKey()); + } + + return ec; } } diff --git a/lib/src/main/java/dev/openfeature/javasdk/Hook.java b/lib/src/main/java/dev/openfeature/javasdk/Hook.java index 932c9591..4976c5e3 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/Hook.java +++ b/lib/src/main/java/dev/openfeature/javasdk/Hook.java @@ -7,7 +7,7 @@ import java.util.Optional; // TODO: interface? or abstract class? public abstract class Hook { public Optional before(HookContext ctx, ImmutableMap hints) { - return null; + return Optional.empty(); } public void after(HookContext ctx, FlagEvaluationDetails details, ImmutableMap hints) {} public void error(HookContext ctx, Exception error, ImmutableMap hints) {} diff --git a/lib/src/main/java/dev/openfeature/javasdk/HookContext.java b/lib/src/main/java/dev/openfeature/javasdk/HookContext.java index 3773a26c..fe8a0eb2 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/HookContext.java +++ b/lib/src/main/java/dev/openfeature/javasdk/HookContext.java @@ -4,14 +4,13 @@ import lombok.Builder; import lombok.NonNull; import lombok.Value; import lombok.With; -import javax.annotation.Nullable; @Value @Builder @With public class HookContext { @NonNull String flagKey; @NonNull FlagValueType type; @NonNull T defaultValue; - @Nullable EvaluationContext ctx; + @NonNull EvaluationContext ctx; Client client; FeatureProvider provider; diff --git a/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureClient.java b/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureClient.java index 4e3d8e29..238b714f 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureClient.java +++ b/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureClient.java @@ -36,11 +36,14 @@ public class OpenFeatureClient implements Client { FlagEvaluationDetails evaluateFlag(FlagValueType type, String key, T defaultValue, EvaluationContext ctx, FlagEvaluationOptions options) { FeatureProvider provider = this.openfeatureApi.getProvider(); ImmutableMap hints = options.getHookHints(); + if (ctx == null) { + ctx = new EvaluationContext(); + } // merge of: API.context, client.context, invocation.context // TODO: Context transformation? - HookContext hookCtx = HookContext.from(key, type, this, null, defaultValue); + HookContext hookCtx = HookContext.from(key, type, this, ctx, defaultValue); List mergedHooks; if (options != null && options.getHooks() != null) { @@ -112,7 +115,7 @@ public class OpenFeatureClient implements Client { EvaluationContext ctx = hookCtx.getCtx(); for (Hook hook : Lists.reverse(hooks)) { Optional newCtx = hook.before(hookCtx, hints); - if (newCtx.isPresent()) { + if (newCtx != null && newCtx.isPresent()) { ctx = EvaluationContext.merge(ctx, newCtx.get()); hookCtx = hookCtx.withCtx(ctx); } @@ -137,7 +140,7 @@ public class OpenFeatureClient implements Client { @Override public FlagEvaluationDetails getBooleanDetails(String key, Boolean defaultValue) { - return getBooleanDetails(key, defaultValue, null); + return getBooleanDetails(key, defaultValue, new EvaluationContext()); } @Override @@ -197,7 +200,7 @@ public class OpenFeatureClient implements Client { @Override public FlagEvaluationDetails getIntegerDetails(String key, Integer defaultValue) { - return getIntegerDetails(key, defaultValue, null); + return getIntegerDetails(key, defaultValue, new EvaluationContext()); } @Override diff --git a/lib/src/test/java/dev/openfeature/javasdk/EvalContextTests.java b/lib/src/test/java/dev/openfeature/javasdk/EvalContextTests.java index d777a74c..92d9df7a 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/EvalContextTests.java +++ b/lib/src/test/java/dev/openfeature/javasdk/EvalContextTests.java @@ -6,21 +6,20 @@ import org.junit.jupiter.api.Test; import java.time.ZonedDateTime; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNull; public class EvalContextTests { - @Specification(spec="flag evaluation", number="3.1", - text="The `evaluation context` structure MUST define a required `targeting key` " + - "field of type string, identifying the subject of the flag evaluation.") - @Disabled("https://github.com/open-feature/spec/pull/60/files#r872827439") + @Specification(spec="Evaluation Context", number="3.1", + text="The `evaluation context` structure **MUST** define an optional `targeting key` field of " + + "type string, identifying the subject of the flag evaluation.") @Test void requires_targeting_key() { EvaluationContext ec = new EvaluationContext(); + ec.setTargetingKey("targeting-key"); assertEquals("targeting-key", ec.getTargetingKey()); } - @Specification(spec="flag evaluation", number="3.3", text="The evaluation context MUST support the inclusion " + - "of custom fields, having keys of type `string`, and values of " + - "type `boolean | string | number | datetime`.") + @Specification(spec="Evaluation Context", number="3.2", text="The evaluation context MUST support the inclusion of " + + "custom fields, having keys of type `string`, and " + + "values of type `boolean | string | number | datetime | structure`.") @Test void eval_context() { EvaluationContext ec = new EvaluationContext(); @@ -38,63 +37,9 @@ public class EvalContextTests { assertEquals(dt, ec.getDatetimeAttribute("dt")); } - - @Specification(spec="flag evaluation", number="3.2", text="The evaluation context MUST define the " + - "following optional fields: `email` (string), `first name` (string), `last name`(string), " + - "`name`(string), `ip`(string), `tz`(string), `locale`(string), `country code` (string), " + - "`timestamp`(date), `environment`(string), `application`(string), and `version`(string).") - @Test void mandated_fields() { - EvaluationContext ec = new EvaluationContext(); - - assertNull(ec.getEmail()); - ec.setEmail("Test"); - assertEquals("Test", ec.getEmail()); - - assertNull(ec.getFirstName()); - ec.setFirstName("Test"); - assertEquals("Test", ec.getFirstName()); - - assertNull(ec.getLastName()); - ec.setLastName("Test"); - assertEquals("Test", ec.getLastName()); - - assertNull(ec.getName()); - ec.setName("Test"); - assertEquals("Test", ec.getName()); - - assertNull(ec.getIp()); - ec.setIp("Test"); - assertEquals("Test", ec.getIp()); - - assertNull(ec.getTz()); - ec.setTz("Test"); - assertEquals("Test", ec.getTz()); - - assertNull(ec.getLocale()); - ec.setLocale("Test"); - assertEquals("Test", ec.getLocale()); - - assertNull(ec.getCountryCode()); - ec.setCountryCode("Test"); - assertEquals("Test", ec.getCountryCode()); - - assertNull(ec.getTimestamp()); - ZonedDateTime dt = ZonedDateTime.now(); - ec.setTimestamp(dt); - assertEquals(dt, ec.getTimestamp()); - - assertNull(ec.getEnvironment()); - ec.setEnvironment("Test"); - assertEquals("Test", ec.getEnvironment()); - - - assertNull(ec.getApplication()); - ec.setApplication("Test"); - assertEquals("Test", ec.getApplication()); - - - assertNull(ec.getVersion()); - ec.setVersion("Test"); - assertEquals("Test", ec.getVersion()); - } + @Specification(spec="Evaluation Context", number="3.2", text="The evaluation context MUST support the inclusion of " + + "custom fields, having keys of type `string`, and " + + "values of type `boolean | string | number | datetime | structure`.") + @Disabled("Structure support") + @Test void eval_context__structure() {} } diff --git a/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java b/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java index cc0b541f..641af25c 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java +++ b/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java @@ -1,6 +1,5 @@ package dev.openfeature.javasdk; -import dev.openfeature.javasdk.*; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; @@ -132,6 +131,7 @@ public class FlagEvaluationSpecTests { "unhandled error, etc) the reason field in the evaluation details SHOULD indicate an error.") @Disabled @Test void detail_flags() { + // TODO: Add tests re: detail functions. throw new NotImplementedException(); } diff --git a/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java b/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java index 8190b9a6..65e1293a 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java +++ b/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java @@ -5,6 +5,7 @@ import lombok.SneakyThrows; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; import org.mockito.InOrder; import java.util.ArrayList; @@ -396,7 +397,66 @@ public class HookSpecTests { } @Specification(spec="hooks", number="3.3", text="Any `EvaluationContext` returned from a `before` hook **MUST** be passed to subsequent `before` hooks (via `HookContext`).") + @Test void beforeContextUpdated() { + EvaluationContext ctx = new EvaluationContext(); + Hook hook = mock(Hook.class); + when(hook.before(any(), any())).thenReturn(Optional.of(ctx)); + Hook hook2 = mock(Hook.class); + when(hook.before(any(), any())).thenReturn(Optional.empty()); + InOrder order = inOrder(hook, hook2); + + + + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + api.setProvider(new NoOpProvider()); + Client client = api.getClient(); + client.getBooleanValue("key", false, new EvaluationContext(), + FlagEvaluationOptions.builder() + .hook(hook2) + .hook(hook) + .build()); + + order.verify(hook).before(any(), any()); + ArgumentCaptor captor = ArgumentCaptor.forClass(HookContext.class); + order.verify(hook2).before(captor.capture(), any()); + + HookContext hc = captor.getValue(); + assertEquals(hc.getCtx(), ctx); + + } @Specification(spec="hooks", number="3.4", text="When `before` hooks have finished executing, any resulting `EvaluationContext` **MUST** be merged with the invocation `EvaluationContext` wherein the invocation `EvaluationContext` win any conflicts.") + @Test void mergeHappensCorrectly() { + EvaluationContext hookCtx = new EvaluationContext(); + hookCtx.addStringAttribute("test", "broken"); + hookCtx.addStringAttribute("another", "exists"); + + EvaluationContext invocationCtx = new EvaluationContext(); + invocationCtx.addStringAttribute("test", "works"); + + Hook hook = mock(Hook.class); + when(hook.before(any(), any())).thenReturn(Optional.of(hookCtx)); + + FeatureProvider provider = mock(FeatureProvider.class); + when(provider.getBooleanEvaluation(any(),any(),any(),any())).thenReturn(ProviderEvaluation.builder() + .value(true) + .build()); + + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + api.setProvider(provider); + Client client = api.getClient(); + client.getBooleanValue("key", false, invocationCtx, + FlagEvaluationOptions.builder() + .hook(hook) + .build()); + + ArgumentCaptor captor = ArgumentCaptor.forClass(EvaluationContext.class); + verify(provider).getBooleanEvaluation(any(), any(), captor.capture(), any()); + EvaluationContext ec = captor.getValue(); + assertEquals("works", ec.getStringAttribute("test")); + assertEquals("exists", ec.getStringAttribute("another")); + } + + @Specification(spec="hooks", number="1.4", text="The evaluation context MUST be mutable only within the before hook.") @Specification(spec="hooks", number="3.1", text="Hooks MUST specify at least one stage.") @Test @Disabled void todo() {}