From 30c5ac4834a6ddac4322f9eb3fa6cecbec2099f7 Mon Sep 17 00:00:00 2001 From: Justin Abrahms Date: Thu, 9 Jun 2022 15:11:19 -0500 Subject: [PATCH 1/4] Support for spec validation --- .../openfeature/javasdk/EvalContextTests.java | 6 +- .../javasdk/FlagEvaluationSpecTests.java | 82 ++++---------- .../openfeature/javasdk/HookSpecTests.java | 53 ++++----- .../javasdk/ProviderSpecTests.java | 26 ++--- .../openfeature/javasdk/Specification.java | 1 - spec_finder.py | 107 ++++++++++++++++++ 6 files changed, 171 insertions(+), 104 deletions(-) create mode 100644 spec_finder.py diff --git a/lib/src/test/java/dev/openfeature/javasdk/EvalContextTests.java b/lib/src/test/java/dev/openfeature/javasdk/EvalContextTests.java index c71ffeb2..e8aa2c8a 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/EvalContextTests.java +++ b/lib/src/test/java/dev/openfeature/javasdk/EvalContextTests.java @@ -7,7 +7,7 @@ import java.time.ZonedDateTime; import static org.junit.jupiter.api.Assertions.assertEquals; public class EvalContextTests { - @Specification(spec="Evaluation Context", number="3.1", + @Specification(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() { @@ -16,7 +16,7 @@ public class EvalContextTests { assertEquals("targeting-key", ec.getTargetingKey()); } - @Specification(spec="Evaluation Context", number="3.2", text="The evaluation context MUST support the inclusion of " + + @Specification(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() { @@ -36,7 +36,7 @@ public class EvalContextTests { assertEquals(dt, ec.getDatetimeAttribute("dt")); } - @Specification(spec="Evaluation Context", number="3.2", text="The evaluation context MUST support the inclusion of " + + @Specification(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__structure() { diff --git a/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java b/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java index 514690f4..702d8f05 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java +++ b/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java @@ -16,18 +16,10 @@ public class FlagEvaluationSpecTests { return api.getClient(); } - @Specification(spec="flag evaluation", number="1.1", - text="The API, and any state it maintains SHOULD exist as a global singleton, " + - "even in cases wherein multiple versions of the API are present at runtime.") @Test void global_singleton() { assertSame(OpenFeatureAPI.getInstance(), OpenFeatureAPI.getInstance()); } - @Specification(spec="flag evaluation", number="1.2", - text="The API MUST provide a function to set the global provider singleton, " + - "which accepts an API-conformant provider implementation.") - @Specification(spec="flag evaluation", number="1.4", - text="The API MUST provide a function for retrieving the provider implementation.") @Test void provider() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); FeatureProvider mockProvider = mock(FeatureProvider.class); @@ -35,9 +27,6 @@ public class FlagEvaluationSpecTests { assertEquals(mockProvider, api.getProvider()); } - @Specification(spec="flag evaluation", number="1.3", text="The API MUST provide a function to add hooks which " + - "accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks." + - "When new hooks are added, previously added hooks are not removed.") @Test void hook_addition() { Hook h1 = mock(Hook.class); Hook h2 = mock(Hook.class); @@ -52,17 +41,12 @@ public class FlagEvaluationSpecTests { assertEquals(h2, api.getApiHooks().get(1)); } - @Specification(spec="flag evaluation", number="1.5", text="The API MUST provide a function for creating a client " + - "which accepts the following options: name (optional): A logical string identifier for the client.") @Test void namedClient() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); Client c = api.getClient("Sir Calls-a-lot"); // TODO: Doesn't say that you can *get* the client name.. which seems useful? } - @Specification(spec="flag evaluation", number="1.6", text="The client MUST provide a method to add hooks which " + - "accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. " + - "When new hooks are added, previously added hooks are not removed.") @Test void hookRegistration() { Client c = _client(); Hook m1 = mock(Hook.class); @@ -75,11 +59,6 @@ public class FlagEvaluationSpecTests { assertTrue(hooks.contains(m2)); } - @Specification(spec="flag evaluation", number="1.7", text="The client MUST provide methods for flag evaluation, with" + - " parameters flag key (string, required), default value (boolean | number | string | structure, required), " + - "evaluation context (optional), and evaluation options (optional), which returns the flag value.") - @Specification(spec="flag evaluation", number="1.8.1",text="The client MUST provide methods for typed flag " + - "evaluation, including boolean, numeric, string, and structure.") @Test void value_flags() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new DoSomethingProvider()); @@ -104,17 +83,6 @@ public class FlagEvaluationSpecTests { assertEquals(null, c.getObjectValue(key, new Node(), new EvaluationContext(), FlagEvaluationOptions.builder().build())); } - @Specification(spec="flag evaluation", number="1.9", text="The client MUST provide methods for detailed flag value " + - "evaluation with parameters flag key (string, required), default value (boolean | number | string | " + - "structure, required), evaluation context (optional), and evaluation options (optional), which returns an " + - "evaluation details structure.") - @Specification(spec="flag evaluation", number="1.10", text="The evaluation details structure's value field MUST " + - "contain the evaluated flag value.") - @Specification(spec="flag evaluation", number="1.11.1", text="The evaluation details structure SHOULD accept a " + - "generic argument (or use an equivalent language feature) which indicates the type of the wrapped value " + - "field.") - @Specification(spec="flag evaluation", number="1.12", text="The evaluation details structure's flag key field MUST " + - "contain the flag key argument passed to the detailed flag evaluation method.") @Test void detail_flags() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new DoSomethingProvider()); @@ -148,9 +116,6 @@ public class FlagEvaluationSpecTests { assertEquals(id, c.getIntegerDetails(key, 4, new EvaluationContext(), FlagEvaluationOptions.builder().build())); } - @Specification(spec="flag evaluation", number="1.17", text="The evaluation options structure's hooks field denotes " + - "a collection of hooks that the client MUST execute for the respective flag evaluation, in addition to " + - "those already configured.") @Test void hooks() { Client c = _client(); Hook clientHook = mock(Hook.class); @@ -163,10 +128,6 @@ public class FlagEvaluationSpecTests { verify(invocationHook, times(1)).before(any(), any()); } - @Specification(spec="flag evaluation", number="1.18", text="Methods, functions, or operations on the client MUST " + - "NOT throw exceptions, or otherwise abnormally terminate. Flag evaluation calls must always return the " + - "default value in the event of abnormal execution. Exceptions include functions or methods for the " + - "purposes for configuration or setup.") @Test void broken_provider() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new AlwaysBrokenProvider()); @@ -174,31 +135,38 @@ public class FlagEvaluationSpecTests { assertFalse(c.getBooleanValue("key", false)); } - @Specification(spec="flag evaluation", number="1.19", text="In the case of abnormal execution, the client SHOULD " + - "log an informative error message.") @Disabled("Not actually sure how to mock out the slf4j logger") @Test void log_on_error() throws NotImplementedException { throw new NotImplementedException(); } - @Specification(spec="flag evaluation", number="1.21", text="The client MUST transform the evaluation context using " + - "the provider's context transformer function, before passing the result of the transformation to the " + - "provider's flag resolution functions.") - @Specification(spec="flag evaluation", number="1.13", text="In cases of normal execution, the evaluation details " + - "structure's variant field MUST contain the value of the variant field in the flag resolution structure " + - "returned by the configured provider, if the field is set.") - @Specification(spec="flag evaluation", number="1.14", text="In cases of normal execution, the evaluation details " + - "structure's reason field MUST contain the value of the reason field in the flag resolution structure " + - "returned by the configured provider, if the field is set.") - @Specification(spec="flag evaluation", number="1.15", text="In cases of abnormal execution, the evaluation details " + - "structure's error code field MUST identify an error occurred during flag evaluation, having possible " + - "values PROVIDER_NOT_READY, FLAG_NOT_FOUND, PARSE_ERROR, TYPE_MISMATCH, or GENERAL.") - @Specification(spec="flag evaluation", number="1.16", text="In cases of abnormal execution (network failure, " + - "unhandled error, etc) the reason field in the evaluation details SHOULD indicate an error.") + @Specification(number="1.1.1", text="The API, and any state it maintains SHOULD exist as a global singleton, even in cases wherein multiple versions of the API are present at runtime.") + @Specification(number="1.1.2", text="The API MUST provide a function to set the global provider singleton, which accepts an API-conformant provider implementation.") + @Specification(number="1.1.3", text="The API MUST provide a function to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed.") + @Specification(number="1.1.4", text="The API MUST provide a function for retrieving the metadata field of the configured provider.") + @Specification(number="1.1.5", text="The API MUST provide a function for creating a client which accepts the following options: - name (optional): A logical string identifier for the client.") + @Specification(number="1.1.6", text="The client creation function MUST NOT throw, or otherwise abnormally terminate.") + @Specification(number="1.2.1", text="The client MUST provide a method to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed.") + @Specification(number="1.2.2", text="The client interface MUST define a metadata member or accessor, containing an immutable name field or accessor of type string, which corresponds to the name value supplied during client creation.") + @Specification(number="1.3.1", text="The client MUST provide methods for flag evaluation, with parameters flag key (string, required), default value (boolean | number | string | structure, required), evaluation context (optional), and evaluation options (optional), which returns the flag value.") + @Specification(number="1.3.2.1", text="The client MUST provide methods for typed flag evaluation, including boolean, numeric, string, and structure.") + @Specification(number="1.3.3", text="The client SHOULD guarantee the returned value of any typed flag evaluation method is of the expected type. If the value returned by the underlying provider implementation does not match the expected type, it's to be considered abnormal execution, and the supplied default value should be returned.") + @Specification(number="1.4.1", text="The client MUST provide methods for detailed flag value evaluation with parameters flag key (string, required), default value (boolean | number | string | structure, required), evaluation context (optional), and evaluation options (optional), which returns an evaluation details structure.") + @Specification(number="1.4.2", text="The evaluation details structure's value field MUST contain the evaluated flag value.") + @Specification(number="1.4.3.1", text="The evaluation details structure SHOULD accept a generic argument (or use an equivalent language feature) which indicates the type of the wrapped value field.") + @Specification(number="1.4.4", text="The evaluation details structure's flag key field MUST contain the flag key argument passed to the detailed flag evaluation method.") + @Specification(number="1.4.5", text="In cases of normal execution, the evaluation details structure's variant field MUST contain the value of the variant field in the flag resolution structure returned by the configured provider, if the field is set.") + @Specification(number="1.4.6", text="In cases of normal execution, the evaluation details structure's reason field MUST contain the value of the reason field in the flag resolution structure returned by the configured provider, if the field is set.") + @Specification(number="1.4.7", text="In cases of abnormal execution, the evaluation details structure's error code field MUST contain a string identifying an error occurred during flag evaluation and the nature of the error.") + @Specification(number="1.4.8", text="In cases of abnormal execution (network failure, unhandled error, etc) the reason field in the evaluation details SHOULD indicate an error.") + @Specification(number="1.4.9", text="Methods, functions, or operations on the client MUST NOT throw exceptions, or otherwise abnormally terminate. Flag evaluation calls must always return the default value in the event of abnormal execution. Exceptions include functions or methods for the purposes for configuration or setup.") + @Specification(number="1.4.10", text="In the case of abnormal execution, the client SHOULD log an informative error message.") + @Specification(number="1.4.11", text="The client SHOULD provide asynchronous or non-blocking mechanisms for flag evaluation.") + @Specification(number="1.5.1", text="The evaluation options structure's hooks field denotes an ordered collection of hooks that the client MUST execute for the respective flag evaluation, in addition to those already configured.") + @Specification(number="1.6.1", text="The client SHOULD transform the evaluation context using the provider's context transformer function if one is defined, before passing the result of the transformation to the provider's flag resolution functions.") + @Test @Disabled void todo() {} - @Specification(spec="flag evaluation", number="1.20", text="The client SHOULD provide asynchronous or non-blocking " + - "mechanisms for flag evaluation.") @Disabled("We're operating in a one request per thread model") @Test void explicitly_not_doing() { diff --git a/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java b/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java index ee6daee8..72dd61e4 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java +++ b/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java @@ -24,7 +24,7 @@ public class HookSpecTests { OpenFeatureAPI.getInstance().clearHooks(); } - @Specification(spec="hooks", number="1.3", text="flag key, flag type, default value properties MUST be immutable. If the language does not support immutability, the hook MUST NOT modify these properties.") + @Specification(number="4.1.3", text="The flag key, flag type, and default value properties MUST be immutable. If the language does not support immutability, the hook MUST NOT modify these properties.") @Test void immutableValues() { try { HookContext.class.getMethod("setFlagKey"); @@ -48,7 +48,7 @@ public class HookSpecTests { } } - @Specification(spec="hooks", number="1.1", text="Hook context MUST provide: the flag key, flag type, evaluation context, and the default value.") + @Specification(number="4.1.1", text="Hook context MUST provide: the flag key, flag value type, evaluation context, and the default value.") @Test void nullish_properties_on_hookcontext() { // missing ctx try { @@ -112,7 +112,6 @@ public class HookSpecTests { } - @Specification(spec="hooks", number="1.2", text="Hook context SHOULD provide: provider (instance) and client (instance)") @Test void optional_properties() { // don't specify HookContext.builder() @@ -141,7 +140,6 @@ public class HookSpecTests { .build(); } - @Specification(spec="hooks", number="3.2", text="The before stage MUST run before flag evaluation occurs. It accepts a hook context (required) and HookHints (optional) as parameters and returns either a EvaluationContext or nothing.") @Test void before_runs_ahead_of_evaluation() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new AlwaysBrokenProvider()); @@ -154,14 +152,12 @@ public class HookSpecTests { verify(evalHook, times(1)).before(any(), any()); } - @Specification(spec="hooks", number="5.1", text="Flag evalution options MUST contain a list of hooks to evaluate.") @Test void feo_has_hook_list() { FlagEvaluationOptions feo = FlagEvaluationOptions.builder() .build(); assertNotNull(feo.getHooks()); } - @Specification(spec="hooks", number="4.3", text="If an error occurs in the finally hook, it MUST NOT trigger the error hook.") @Test void errors_in_finally() { Hook h = mock(Hook.class); doThrow(RuntimeException.class).when(h).finallyAfter(any(), any()); @@ -176,7 +172,6 @@ public class HookSpecTests { verify(h, times(0)).error(any(), any(), any()); } - @Specification(spec="hooks", number="3.6", text="The error hook MUST run when errors are encountered in the before stage, the after stage or during flag evaluation. It accepts hook context (required), exception for what went wrong (required), and HookHints (optional). It has no return value.") @Test void error_hook_run_during_non_finally_stage() { final boolean[] error_called = {false}; Hook h = mock(Hook.class); @@ -185,12 +180,7 @@ public class HookSpecTests { verify(h, times(0)).error(any(), any(), any()); } - @Specification(spec="hooks", number="4.1", text="The API, Client and invocation MUST have a method for registering hooks which accepts flag evaluation options") - @Specification(spec="hooks", number="4.2", text="Hooks MUST be evaluated in the following order:" + - "before: API, Client, Invocation" + - "after: Invocation, Client, API" + - "error (if applicable): Invocation, Client, API" + - "finally: Invocation, Client, API") + @Specification(number="4.4.1", text="The API, Client and invocation MUST have a method for registering hooks which accepts flag evaluation options") @Test void hook_eval_order() { List evalOrder = new ArrayList(); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); @@ -277,7 +267,7 @@ public class HookSpecTests { assertEquals(expectedOrder, evalOrder); } - @Specification(spec="hooks", number="4.6", text="If an error is encountered in the error stage, it MUST NOT be returned to the user.") + @Specification(number="4.4.6", text="If an error occurs during the evaluation of before or after hooks, any remaining hooks in the before or after stages MUST NOT be invoked.") @Disabled("Not actually sure what 'returned to the user' means in this context. There is no exception information returned.") @Test void error_in_error_stage() { Hook h = mock(Hook.class); @@ -291,10 +281,6 @@ public class HookSpecTests { } - @Specification(spec="hooks", number="2.1", text="HookHints MUST be a map of objects.") - @Specification(spec="hooks", number="2.2", text="Condition: HookHints MUST be immutable.") - @Specification(spec="hooks", number="5.4", text="The hook MUST NOT alter the HookHints object.") - @Specification(spec="hooks", number="5.3", text="HookHints MUST be passed to each hook.") @Test void hook_hints() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new NoOpProvider()); @@ -330,15 +316,12 @@ public class HookSpecTests { .build()); } - @Specification(spec="hooks", number="5.2", text="Flag evaluation options MAY contain HookHints, a map of data to be provided to hook invocations.") @Test void missing_hook_hints() { FlagEvaluationOptions feo = FlagEvaluationOptions.builder().build(); assertNotNull(feo.getHookHints()); assertTrue(feo.getHookHints().isEmpty()); } - @Specification(spec="hooks", number="3.5", text="The after stage MUST run after flag evaluation occurs. It accepts a hook context (required), flag evaluation details (required) and HookHints (optional). It has no return value.") - @Specification(spec="hooks", number="3.7", text="The finally hook MUST run after the before, after, and error stages. It accepts a hook context (required) and HookHints (optional). There is no return value.") @Test void flag_eval_hook_order() { Hook hook = mock(Hook.class); FeatureProvider provider = mock(FeatureProvider.class); @@ -360,7 +343,6 @@ public class HookSpecTests { order.verify(hook).finallyAfter(any(),any()); } - @Specification(spec="hooks", number="4.4", text="If an error occurs in the before or after hooks, the error hooks MUST be invoked.") @Test void error_hooks() { Hook hook = mock(Hook.class); doThrow(RuntimeException.class).when(hook).before(any(), any()); @@ -373,7 +355,6 @@ public class HookSpecTests { verify(hook, times(1)).error(any(), any(), any()); } - @Specification(spec="hooks", number="4.5", text="If an error occurs during the evaluation of before or after hooks, any remaining hooks in the before or after stages MUST NOT be invoked.") @Test void multi_hooks_early_out__before() { Hook hook = mock(Hook.class); Hook hook2 = mock(Hook.class); @@ -396,7 +377,6 @@ public class HookSpecTests { verify(hook2, times(1)).error(any(), any(), any()); } - @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); @@ -424,7 +404,6 @@ public class HookSpecTests { 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"); @@ -457,12 +436,30 @@ public class HookSpecTests { } - @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.") + @Specification(number="4.1.2", text="The hook context SHOULD provide: access to the client metadata and the provider metadata fields.") + @Specification(number="4.1.4", text="The evaluation context MUST be mutable only within the before hook.") + @Specification(number="4.2.1", text="hook hints MUST be a structure supports definition of arbitrary properties, with keys of type string, and values of type boolean | string | number | datetime | structure..") + @Specification(number="4.2.2.1", text="Condition: Hook hints MUST be immutable.") + @Specification(number="4.2.2.2", text="Condition: The client metadata field in the hook context MUST be immutable.") + @Specification(number="4.2.2.3", text="Condition: The provider metadata field in the hook context MUST be immutable.") + @Specification(number="4.3.1", text="Hooks MUST specify at least one stage.") + @Specification(number="4.3.2", text="The before stage MUST run before flag resolution occurs. It accepts a hook context (required) and hook hints (optional) as parameters and returns either an evaluation context or nothing.") + @Specification(number="4.3.3", text="Any evaluation context returned from a before hook MUST be passed to subsequent before hooks (via HookContext).") + @Specification(number="4.3.4", text="When before hooks have finished executing, any resulting evaluation context MUST be merged with the invocation evaluation context with the invocation evaluation context taking precedence in the case of any conflicts.") + @Specification(number="4.3.5", text="The after stage MUST run after flag resolution occurs. It accepts a hook context (required), flag evaluation details (required) and hook hints (optional). It has no return value.") + @Specification(number="4.3.6", text="The error hook MUST run when errors are encountered in the before stage, the after stage or during flag resolution. It accepts hook context (required), exception representing what went wrong (required), and hook hints (optional). It has no return value.") + @Specification(number="4.3.7", text="The finally hook MUST run after the before, after, and error stages. It accepts a hook context (required) and hook hints (optional). There is no return value.") + @Specification(number="4.3.8.1", text="Instead of finally, finallyAfter SHOULD be used.") + @Specification(number="4.4.2", text="Hooks MUST be evaluated in the following order: - before: API, Client, Invocation - after: Invocation, Client, API - error (if applicable): Invocation, Client, API - finally: Invocation, Client, API") + @Specification(number="4.4.3", text="If a finally hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining finally hooks.") + @Specification(number="4.4.4", text="If an error hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining error hooks.") + @Specification(number="4.4.5", text="If an error occurs in the before or after hooks, the error hooks MUST be invoked.") + @Specification(number="4.5.1", text="Flag evaluation options MAY contain hook hints, a map of data to be provided to hook invocations.") + @Specification(number="4.5.2", text="hook hints MUST be passed to each hook.") + @Specification(number="4.5.3", text="The hook MUST NOT alter the hook hints structure.") @Test @Disabled void todo() {} @SneakyThrows - @Specification(spec="hooks", number="3.8", text="Condition: If finally is a reserved word in the language, finallyAfter SHOULD be used.") @Test void doesnt_use_finally() { try { Hook.class.getMethod("finally", HookContext.class, ImmutableMap.class); diff --git a/lib/src/test/java/dev/openfeature/javasdk/ProviderSpecTests.java b/lib/src/test/java/dev/openfeature/javasdk/ProviderSpecTests.java index d4921b99..b92d021e 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/ProviderSpecTests.java +++ b/lib/src/test/java/dev/openfeature/javasdk/ProviderSpecTests.java @@ -8,21 +8,20 @@ import static org.junit.jupiter.api.Assertions.*; public class ProviderSpecTests { NoOpProvider p = new NoOpProvider(); - @Specification(spec="provider", number="2.1", text="The provider interface MUST define a name field or accessor, " + - "which identifies the provider implementation.") + @Specification(number="2.1", text="The provider interface MUST define a metadata member or accessor, containing a name field or accessor of type string, which identifies the provider implementation.") @Test void name_accessor() { assertNotNull(p.getName()); } - @Specification(spec="provider", number="2.3.1", text="The feature provider interface MUST define methods for typed " + + @Specification(number="2.3.1", text="The feature provider interface MUST define methods for typed " + "flag resolution, including boolean, numeric, string, and structure.") - @Specification(spec="provider", number="2.4", text="In cases of normal execution, the provider MUST populate the " + + @Specification(number="2.4", text="In cases of normal execution, the provider MUST populate the " + "flag resolution structure's value field with the resolved flag value.") - @Specification(spec="provider", number="2.2", text="The feature provider interface MUST define methods to resolve " + + @Specification(number="2.2", text="The feature provider interface MUST define methods to resolve " + "flag values, with parameters flag key (string, required), default value " + "(boolean | number | string | structure, required), evaluation context (optional), and " + "evaluation options (optional), which returns a flag resolution structure.") - @Specification(spec="provider", number="2.9.1", text="The flag resolution structure SHOULD accept a generic " + + @Specification(number="2.9.1", text="The flag resolution structure SHOULD accept a generic " + "argument (or use an equivalent language feature) which indicates the type of the wrapped value field.") @Test void flag_value_set() { ProviderEvaluation int_result = p.getIntegerEvaluation("key", 4, new EvaluationContext(), FlagEvaluationOptions.builder().build()); @@ -38,21 +37,21 @@ public class ProviderSpecTests { assertNotNull(boolean_result.getValue()); } - @Specification(spec="provider", number="2.6", text="The provider SHOULD populate the flag resolution structure's " + + @Specification(number="2.6", text="The provider SHOULD populate the flag resolution structure's " + "reason field with a string indicating the semantic reason for the returned flag value.") @Test void has_reason() { ProviderEvaluation result = p.getBooleanEvaluation("key", false, new EvaluationContext(), FlagEvaluationOptions.builder().build()); assertEquals(Reason.DEFAULT, result.getReason()); } - @Specification(spec="provider", number="2.7", text="In cases of normal execution, the provider MUST NOT populate " + + @Specification(number="2.7", text="In cases of normal execution, the provider MUST NOT populate " + "the flag resolution structure's error code field, or otherwise must populate it with a null or falsy value.") @Test void no_error_code_by_default() { ProviderEvaluation result = p.getBooleanEvaluation("key", false, new EvaluationContext(), FlagEvaluationOptions.builder().build()); assertNull(result.getErrorCode()); } - @Specification(spec="provider", number="2.8", text="In cases of abnormal execution, the provider MUST indicate an " + + @Specification(number="2.8", text="In cases of abnormal execution, the provider MUST indicate an " + "error using the idioms of the implementation language, with an associated error code having possible " + "values PROVIDER_NOT_READY, FLAG_NOT_FOUND, PARSE_ERROR, TYPE_MISMATCH, or GENERAL.") @Disabled("I don't think we expect the provider to do all the exception catching.. right?") @@ -62,7 +61,7 @@ public class ProviderSpecTests { assertEquals(ErrorCode.GENERAL, result.getErrorCode()); } - @Specification(spec="provider", number="2.5", text="In cases of normal execution, the provider SHOULD populate the " + + @Specification(number="2.5", text="In cases of normal execution, the provider SHOULD populate the " + "flag resolution structure's variant field with a string identifier corresponding to the returned flag value.") @Test void variant_set() { ProviderEvaluation int_result = p.getIntegerEvaluation("key", 4, new EvaluationContext(), FlagEvaluationOptions.builder().build()); @@ -75,11 +74,8 @@ public class ProviderSpecTests { assertNotNull(boolean_result.getReason()); } - @Specification(spec="provider", number="2.11.1", text="If the implementation includes a context transformer, the " + - "provider SHOULD accept a generic argument (or use an equivalent language feature) indicating the type of " + - "the transformed context. If such type information is supplied, more accurate type information can be " + - "supplied in the flag resolution methods.") - @Specification(spec="provider", number="2.10", text="The provider interface MAY define a context transformer method " + + @Specification(number="2.11.1", text="If the implementation includes a context transformer, the provider SHOULD accept a generic argument (or use an equivalent language feature) indicating the type of the transformed context. If such type information is supplied, more accurate type information can be supplied in the flag resolution methods.") + @Specification(number="2.10", text="The provider interface MAY define a context transformer method " + "or function, which can be optionally implemented in order to transform the evaluation context prior to " + "flag value resolution.") @Disabled("I don't think we should do that until we figure out the call signature differences") diff --git a/lib/src/test/java/dev/openfeature/javasdk/Specification.java b/lib/src/test/java/dev/openfeature/javasdk/Specification.java index 16e9af8f..3d8bbd1c 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/Specification.java +++ b/lib/src/test/java/dev/openfeature/javasdk/Specification.java @@ -4,7 +4,6 @@ import java.lang.annotation.Repeatable; @Repeatable(Specifications.class) public @interface Specification { - String spec(); String number(); String text(); } diff --git a/spec_finder.py b/spec_finder.py new file mode 100644 index 00000000..58caced3 --- /dev/null +++ b/spec_finder.py @@ -0,0 +1,107 @@ +import urllib.request +import json +import re +import difflib +import os +import sys + +def _demarkdown(t): + return t.replace('**', '').replace('`', '').replace('"', '') + +def get_spec(force_refresh=False): + spec_path = './specification.json' + data = "" + if os.path.exists(spec_path): + with open(spec_path) as f: + data = ''.join(f.readlines()) + else: + # TODO: Status code check + spec_response = urllib.request.urlopen('https://raw.githubusercontent.com/open-feature/spec/main/specification.json') + raw = [] + for i in spec_response.readlines(): + raw.append(i.decode('utf-8')) + data = ''.join(raw) + with open(spec_path, 'w') as f: + f.write(data) + return json.loads(data) + + +def main(refresh_spec=False, diff_output=False, limit_numbers=None): + actual_spec = get_spec(refresh_spec) + + spec_map = {} + for entry in actual_spec['rules']: + number = re.search('[\d.]+', entry['id']).group() + if 'requirement' in entry['machine_id']: + spec_map[number] = _demarkdown(entry['content']) + + if len(entry['children']) > 0: + for ch in entry['children']: + number = re.search('[\d.]+', ch['id']).group() + if 'requirement' in ch['machine_id']: + spec_map[number] = _demarkdown(ch['content']) + + java_specs = {} + missing = set(spec_map.keys()) + + + import os + for root, dirs, files in os.walk(".", topdown=False): + for name in files: + F = os.path.join(root, name) + if '.java' not in name: + continue + with open(F) as f: + data = ''.join(f.readlines()) + + for match in re.findall('@Specification\((?P.*?)"\)', data.replace('\n', ''), re.MULTILINE | re.DOTALL): + number = re.findall('number="(.*?)"', match)[0] + if number in missing: + missing.remove(number) + text_with_concat_chars = re.findall('text=(.*)', match) + try: + # We have to match for ") to capture text with parens inside, so we add the trailing " back in. + text = _demarkdown(eval(''.join(text_with_concat_chars) + '"')) + entry = java_specs[number] = { + 'number': number, + 'text': text, + } + except: + print(f"Skipping {match} b/c we couldn't parse it") + + bad_num = len(missing) + for number, entry in java_specs.items(): + if limit_numbers is not None and len(limit_numbers) > 0 and number not in limit_numbers: + continue + if number in spec_map: + txt = entry['text'] + if txt == spec_map[number]: + # print(f'{number} is good') + continue + else: + print(f"{number} is bad") + bad_num += 1 + if diff_output: + print(number + '\n' + '\n'.join([li for li in difflib.ndiff([txt], [spec_map[number]]) if not li.startswith(' ')])) + continue + + print(f"Couldn't find the number {number}") + print("\n\n") + + if len(missing) > 0: + print('Missing: ', missing) + + sys.exit(bad_num) + + +if __name__ == '__main__': + import argparse + + parser = argparse.ArgumentParser(description='Parse the spec to make sure our tests cover it') + parser.add_argument('--refresh-spec', action='store_true', help='Re-downloads the spec') + parser.add_argument('--diff-output', action='store_true', help='print the text differences') + parser.add_argument('specific_numbers', metavar='num', type=str, nargs='*', + help='limit this to specific numbers') + + args = parser.parse_args() + main(refresh_spec=args.refresh_spec, diff_output=args.diff_output, limit_numbers=args.specific_numbers) From 448795920dd2247a3ae8619d0d5516a3196a2541 Mon Sep 17 00:00:00 2001 From: Justin Abrahms Date: Thu, 9 Jun 2022 16:59:23 -0500 Subject: [PATCH 2/4] Spec annotations in the right place, approximately --- .../openfeature/javasdk/HookSpecTests.java | 50 +++++++++++++------ 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java b/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java index 72dd61e4..e9ccddb4 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java +++ b/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java @@ -140,6 +140,7 @@ public class HookSpecTests { .build(); } + @Specification(number="4.3.2", text="The before stage MUST run before flag resolution occurs. It accepts a hook context (required) and hook hints (optional) as parameters and returns either an evaluation context or nothing.") @Test void before_runs_ahead_of_evaluation() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new AlwaysBrokenProvider()); @@ -180,7 +181,12 @@ public class HookSpecTests { verify(h, times(0)).error(any(), any(), any()); } + @Specification(number="4.4.1", text="The API, Client and invocation MUST have a method for registering hooks which accepts flag evaluation options") + @Specification(number="4.3.5", text="The after stage MUST run after flag resolution occurs. It accepts a hook context (required), flag evaluation details (required) and hook hints (optional). It has no return value.") + @Specification(number="4.4.2", text="Hooks MUST be evaluated in the following order: - before: API, Client, Invocation - after: Invocation, Client, API - error (if applicable): Invocation, Client, API - finally: Invocation, Client, API") + @Specification(number="4.3.6", text="The error hook MUST run when errors are encountered in the before stage, the after stage or during flag resolution. It accepts hook context (required), exception representing what went wrong (required), and hook hints (optional). It has no return value.") + @Specification(number="4.3.7", text="The finally hook MUST run after the before, after, and error stages. It accepts a hook context (required) and hook hints (optional). There is no return value.") @Test void hook_eval_order() { List evalOrder = new ArrayList(); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); @@ -281,6 +287,8 @@ public class HookSpecTests { } + @Specification(number="4.2.1", text="hook hints MUST be a structure supports definition of arbitrary properties, with keys of type string, and values of type boolean | string | number | datetime | structure..") + @Specification(number="4.5.2", text="hook hints MUST be passed to each hook.") @Test void hook_hints() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new NoOpProvider()); @@ -316,6 +324,7 @@ public class HookSpecTests { .build()); } + @Specification(number="4.5.1", text="Flag evaluation options MAY contain hook hints, a map of data to be provided to hook invocations.") @Test void missing_hook_hints() { FlagEvaluationOptions feo = FlagEvaluationOptions.builder().build(); assertNotNull(feo.getHookHints()); @@ -343,7 +352,8 @@ public class HookSpecTests { order.verify(hook).finallyAfter(any(),any()); } - @Test void error_hooks() { + @Specification(number="4.4.5", text="If an error occurs in the before or after hooks, the error hooks MUST be invoked.") + @Test void error_hooks__before() { Hook hook = mock(Hook.class); doThrow(RuntimeException.class).when(hook).before(any(), any()); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); @@ -355,6 +365,19 @@ public class HookSpecTests { verify(hook, times(1)).error(any(), any(), any()); } + @Specification(number="4.4.5", text="If an error occurs in the before or after hooks, the error hooks MUST be invoked.") + @Test void error_hooks__after() { + Hook hook = mock(Hook.class); + doThrow(RuntimeException.class).when(hook).after(any(), any(), any()); + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + api.setProvider(new NoOpProvider()); + Client client = api.getClient(); + client.getBooleanValue("key", false, new EvaluationContext(), + FlagEvaluationOptions.builder().hook(hook).build()); + verify(hook, times(1)).after(any(), any(), any()); + verify(hook, times(1)).error(any(), any(), any()); + } + @Test void multi_hooks_early_out__before() { Hook hook = mock(Hook.class); Hook hook2 = mock(Hook.class); @@ -377,6 +400,8 @@ public class HookSpecTests { verify(hook2, times(1)).error(any(), any(), any()); } + @Specification(number="4.1.4", text="The evaluation context MUST be mutable only within the before hook.") + @Specification(number="4.3.3", text="Any evaluation context 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); @@ -404,6 +429,8 @@ public class HookSpecTests { assertEquals(hc.getCtx(), ctx); } + + @Specification(number="4.3.4", text="When before hooks have finished executing, any resulting evaluation context MUST be merged with the invocation evaluation context with the invocation evaluation context taking precedence in the case of any conflicts.") @Test void mergeHappensCorrectly() { EvaluationContext hookCtx = new EvaluationContext(); hookCtx.addStringAttribute("test", "broken"); @@ -437,28 +464,21 @@ public class HookSpecTests { @Specification(number="4.1.2", text="The hook context SHOULD provide: access to the client metadata and the provider metadata fields.") - @Specification(number="4.1.4", text="The evaluation context MUST be mutable only within the before hook.") - @Specification(number="4.2.1", text="hook hints MUST be a structure supports definition of arbitrary properties, with keys of type string, and values of type boolean | string | number | datetime | structure..") @Specification(number="4.2.2.1", text="Condition: Hook hints MUST be immutable.") @Specification(number="4.2.2.2", text="Condition: The client metadata field in the hook context MUST be immutable.") @Specification(number="4.2.2.3", text="Condition: The provider metadata field in the hook context MUST be immutable.") - @Specification(number="4.3.1", text="Hooks MUST specify at least one stage.") - @Specification(number="4.3.2", text="The before stage MUST run before flag resolution occurs. It accepts a hook context (required) and hook hints (optional) as parameters and returns either an evaluation context or nothing.") - @Specification(number="4.3.3", text="Any evaluation context returned from a before hook MUST be passed to subsequent before hooks (via HookContext).") - @Specification(number="4.3.4", text="When before hooks have finished executing, any resulting evaluation context MUST be merged with the invocation evaluation context with the invocation evaluation context taking precedence in the case of any conflicts.") - @Specification(number="4.3.5", text="The after stage MUST run after flag resolution occurs. It accepts a hook context (required), flag evaluation details (required) and hook hints (optional). It has no return value.") - @Specification(number="4.3.6", text="The error hook MUST run when errors are encountered in the before stage, the after stage or during flag resolution. It accepts hook context (required), exception representing what went wrong (required), and hook hints (optional). It has no return value.") - @Specification(number="4.3.7", text="The finally hook MUST run after the before, after, and error stages. It accepts a hook context (required) and hook hints (optional). There is no return value.") - @Specification(number="4.3.8.1", text="Instead of finally, finallyAfter SHOULD be used.") - @Specification(number="4.4.2", text="Hooks MUST be evaluated in the following order: - before: API, Client, Invocation - after: Invocation, Client, API - error (if applicable): Invocation, Client, API - finally: Invocation, Client, API") + + @Specification(number="4.4.3", text="If a finally hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining finally hooks.") @Specification(number="4.4.4", text="If an error hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining error hooks.") - @Specification(number="4.4.5", text="If an error occurs in the before or after hooks, the error hooks MUST be invoked.") - @Specification(number="4.5.1", text="Flag evaluation options MAY contain hook hints, a map of data to be provided to hook invocations.") - @Specification(number="4.5.2", text="hook hints MUST be passed to each hook.") + @Specification(number="4.5.3", text="The hook MUST NOT alter the hook hints structure.") @Test @Disabled void todo() {} + @Specification(number="4.3.1", text="Hooks MUST specify at least one stage.") + @Test void default_methods_so_impossible() {} + + @Specification(number="4.3.8.1", text="Instead of finally, finallyAfter SHOULD be used.") @SneakyThrows @Test void doesnt_use_finally() { try { From a2063fe5a458b6c8334c463373bc5df98a1b5158 Mon Sep 17 00:00:00 2001 From: Justin Abrahms Date: Sun, 12 Jun 2022 20:19:34 -0700 Subject: [PATCH 3/4] Tests updated to the latest spec. This drove a few changes within the code base. --- .../openfeature/javasdk/BaseEvaluation.java | 2 +- .../java/dev/openfeature/javasdk/Client.java | 3 +- .../openfeature/javasdk/FeatureProvider.java | 2 +- .../javasdk/FlagEvaluationDetails.java | 2 +- .../dev/openfeature/javasdk/HookContext.java | 9 +- .../dev/openfeature/javasdk/Metadata.java | 5 + .../dev/openfeature/javasdk/NoOpProvider.java | 10 ++ .../openfeature/javasdk/OpenFeatureAPI.java | 4 + .../javasdk/OpenFeatureClient.java | 30 ++-- .../javasdk/ProviderEvaluation.java | 2 +- .../javasdk/AlwaysBrokenProvider.java | 17 ++- .../javasdk/DeveloperExperienceTest.java | 2 +- .../javasdk/DoSomethingProvider.java | 10 +- .../javasdk/FlagEvaluationSpecTests.java | 82 ++++++----- .../openfeature/javasdk/HookContextTest.java | 26 ++++ .../openfeature/javasdk/HookSpecTests.java | 130 +++++++++++------- .../dev/openfeature/javasdk/MetadataTest.java | 19 +++ .../javasdk/NotImplementedException.java | 4 +- .../javasdk/ProviderSpecTests.java | 1 - 19 files changed, 254 insertions(+), 106 deletions(-) create mode 100644 lib/src/main/java/dev/openfeature/javasdk/Metadata.java create mode 100644 lib/src/test/java/dev/openfeature/javasdk/HookContextTest.java create mode 100644 lib/src/test/java/dev/openfeature/javasdk/MetadataTest.java diff --git a/lib/src/main/java/dev/openfeature/javasdk/BaseEvaluation.java b/lib/src/main/java/dev/openfeature/javasdk/BaseEvaluation.java index fa9db4da..9f06853d 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/BaseEvaluation.java +++ b/lib/src/main/java/dev/openfeature/javasdk/BaseEvaluation.java @@ -23,5 +23,5 @@ public interface BaseEvaluation { * The error code, if applicable. Should only be set when the Reason is ERROR. * @return {ErrorCode} */ - ErrorCode getErrorCode(); + String getErrorCode(); } diff --git a/lib/src/main/java/dev/openfeature/javasdk/Client.java b/lib/src/main/java/dev/openfeature/javasdk/Client.java index bc05f5ef..b57c344d 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/Client.java +++ b/lib/src/main/java/dev/openfeature/javasdk/Client.java @@ -1,4 +1,5 @@ package dev.openfeature.javasdk; -public interface Client extends FlagEvaluationLifecycle, Features{ +public interface Client extends FlagEvaluationLifecycle, Features { + Metadata getMetadata(); } diff --git a/lib/src/main/java/dev/openfeature/javasdk/FeatureProvider.java b/lib/src/main/java/dev/openfeature/javasdk/FeatureProvider.java index eb7ebd93..014b5ccd 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/FeatureProvider.java +++ b/lib/src/main/java/dev/openfeature/javasdk/FeatureProvider.java @@ -1,7 +1,7 @@ package dev.openfeature.javasdk; public interface FeatureProvider { - String getName(); + Metadata getMetadata(); ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx, FlagEvaluationOptions options); ProviderEvaluation getStringEvaluation(String key, String defaultValue, EvaluationContext ctx, FlagEvaluationOptions options); ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx, FlagEvaluationOptions options); diff --git a/lib/src/main/java/dev/openfeature/javasdk/FlagEvaluationDetails.java b/lib/src/main/java/dev/openfeature/javasdk/FlagEvaluationDetails.java index 4d0eb493..1df0a144 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/FlagEvaluationDetails.java +++ b/lib/src/main/java/dev/openfeature/javasdk/FlagEvaluationDetails.java @@ -11,7 +11,7 @@ public class FlagEvaluationDetails implements BaseEvaluation { T value; @Nullable String variant; Reason reason; - @Nullable ErrorCode errorCode; + @Nullable String errorCode; public static FlagEvaluationDetails from(ProviderEvaluation providerEval, String flagKey) { return FlagEvaluationDetails.builder() diff --git a/lib/src/main/java/dev/openfeature/javasdk/HookContext.java b/lib/src/main/java/dev/openfeature/javasdk/HookContext.java index fe8a0eb2..e61e0fc9 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/HookContext.java +++ b/lib/src/main/java/dev/openfeature/javasdk/HookContext.java @@ -11,14 +11,15 @@ public class HookContext { @NonNull FlagValueType type; @NonNull T defaultValue; @NonNull EvaluationContext ctx; - Client client; - FeatureProvider provider; + Metadata clientMetadata; + Metadata providerMetadata; - public static HookContext from(String key, FlagValueType type, Client client, EvaluationContext ctx, T defaultValue) { + public static HookContext from(String key, FlagValueType type, Metadata clientMetadata, Metadata providerMetadata, EvaluationContext ctx, T defaultValue) { return HookContext.builder() .flagKey(key) .type(type) - .client(client) + .clientMetadata(clientMetadata) + .providerMetadata(providerMetadata) .ctx(ctx) .defaultValue(defaultValue) .build(); diff --git a/lib/src/main/java/dev/openfeature/javasdk/Metadata.java b/lib/src/main/java/dev/openfeature/javasdk/Metadata.java new file mode 100644 index 00000000..c0c06e96 --- /dev/null +++ b/lib/src/main/java/dev/openfeature/javasdk/Metadata.java @@ -0,0 +1,5 @@ +package dev.openfeature.javasdk; + +public interface Metadata { + public String getName(); +} diff --git a/lib/src/main/java/dev/openfeature/javasdk/NoOpProvider.java b/lib/src/main/java/dev/openfeature/javasdk/NoOpProvider.java index cc0684d7..f3365520 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/NoOpProvider.java +++ b/lib/src/main/java/dev/openfeature/javasdk/NoOpProvider.java @@ -7,6 +7,16 @@ public class NoOpProvider implements FeatureProvider { @Getter private final String name = "No-op Provider"; + @Override + public Metadata getMetadata() { + return new Metadata() { + @Override + public String getName() { + return name; + } + }; + } + @Override public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx, FlagEvaluationOptions options) { return ProviderEvaluation.builder() diff --git a/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureAPI.java b/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureAPI.java index 8ff53d66..aca5d61f 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureAPI.java +++ b/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureAPI.java @@ -30,6 +30,10 @@ public class OpenFeatureAPI { return getClient(null, null); } + public Metadata getProviderMetadata() { + return provider.getMetadata(); + } + public Client getClient(@Nullable String name) { return getClient(name, null); } diff --git a/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureClient.java b/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureClient.java index 04ba6291..d7b6313c 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureClient.java +++ b/lib/src/main/java/dev/openfeature/javasdk/OpenFeatureClient.java @@ -43,7 +43,7 @@ public class OpenFeatureClient implements Client { // merge of: API.context, client.context, invocation.context // TODO: Context transformation? - HookContext hookCtx = HookContext.from(key, type, this, ctx, defaultValue); + HookContext hookCtx = HookContext.from(key, type, this.getMetadata(), OpenFeatureAPI.getInstance().getProvider().getMetadata(), ctx, defaultValue); List mergedHooks; if (options != null && options.getHooks() != null) { @@ -84,11 +84,7 @@ public class OpenFeatureClient implements Client { } details.value = defaultValue; details.reason = Reason.ERROR; - if (e instanceof OpenFeatureError) { //NOPMD - suppressed AvoidInstanceofChecksInCatchClause - Don't want to duplicate detail creation logic. - details.errorCode = ((OpenFeatureError) e).getErrorCode(); - } else { - details.errorCode = ErrorCode.GENERAL; - } + details.errorCode = e.getMessage(); this.errorHooks(hookCtx, e, mergedHooks, hints); } finally { this.afterAllHooks(hookCtx, mergedHooks, hints); @@ -99,13 +95,21 @@ public class OpenFeatureClient implements Client { private void errorHooks(HookContext hookCtx, Exception e, List hooks, ImmutableMap hints) { for (Hook hook : hooks) { - hook.error(hookCtx, e, hints); + try { + hook.error(hookCtx, e, hints); + } catch (Exception inner_exception) { + log.error("Exception when running error hooks " + hook.getClass().toString(), inner_exception); + } } } private void afterAllHooks(HookContext hookCtx, List hooks, ImmutableMap hints) { for (Hook hook : hooks) { - hook.finallyAfter(hookCtx, hints); + try { + hook.finallyAfter(hookCtx, hints); + } catch (Exception inner_exception) { + log.error("Exception when running finally hooks " + hook.getClass().toString(), inner_exception); + } } } @@ -247,4 +251,14 @@ public class OpenFeatureClient implements Client { public FlagEvaluationDetails getObjectDetails(String key, T defaultValue, EvaluationContext ctx, FlagEvaluationOptions options) { return this.evaluateFlag(FlagValueType.OBJECT, key, defaultValue, ctx, options); } + + @Override + public Metadata getMetadata() { + return new Metadata() { + @Override + public String getName() { + return name; + } + }; + } } diff --git a/lib/src/main/java/dev/openfeature/javasdk/ProviderEvaluation.java b/lib/src/main/java/dev/openfeature/javasdk/ProviderEvaluation.java index 905d78f5..e2d884cd 100644 --- a/lib/src/main/java/dev/openfeature/javasdk/ProviderEvaluation.java +++ b/lib/src/main/java/dev/openfeature/javasdk/ProviderEvaluation.java @@ -10,5 +10,5 @@ public class ProviderEvaluation implements BaseEvaluation { T value; @Nullable String variant; Reason reason; - @Nullable ErrorCode errorCode; + @Nullable String errorCode; } diff --git a/lib/src/test/java/dev/openfeature/javasdk/AlwaysBrokenProvider.java b/lib/src/test/java/dev/openfeature/javasdk/AlwaysBrokenProvider.java index 39eedbb9..1a24235a 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/AlwaysBrokenProvider.java +++ b/lib/src/test/java/dev/openfeature/javasdk/AlwaysBrokenProvider.java @@ -3,27 +3,32 @@ package dev.openfeature.javasdk; public class AlwaysBrokenProvider implements FeatureProvider { @Override - public String getName() { - throw new NotImplementedException(); + public Metadata getMetadata() { + return new Metadata() { + @Override + public String getName() { + throw new NotImplementedException("BORK"); + } + }; } @Override public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx, FlagEvaluationOptions options) { - throw new NotImplementedException(); + throw new NotImplementedException("BORK"); } @Override public ProviderEvaluation getStringEvaluation(String key, String defaultValue, EvaluationContext ctx, FlagEvaluationOptions options) { - throw new NotImplementedException(); + throw new NotImplementedException("BORK"); } @Override public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx, FlagEvaluationOptions options) { - throw new NotImplementedException(); + throw new NotImplementedException("BORK"); } @Override public ProviderEvaluation getObjectEvaluation(String key, T defaultValue, EvaluationContext invocationContext, FlagEvaluationOptions options) { - throw new NotImplementedException(); + throw new NotImplementedException("BORK"); } } diff --git a/lib/src/test/java/dev/openfeature/javasdk/DeveloperExperienceTest.java b/lib/src/test/java/dev/openfeature/javasdk/DeveloperExperienceTest.java index 690fa77d..a105c260 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/DeveloperExperienceTest.java +++ b/lib/src/test/java/dev/openfeature/javasdk/DeveloperExperienceTest.java @@ -53,7 +53,7 @@ class DeveloperExperienceTest { api.setProvider(new AlwaysBrokenProvider()); Client client = api.getClient(); FlagEvaluationDetails retval = client.getBooleanDetails(flagKey, false); - assertEquals(ErrorCode.GENERAL, retval.getErrorCode()); + assertEquals("BORK", retval.getErrorCode()); assertEquals(Reason.ERROR, retval.getReason()); assertFalse(retval.getValue()); } diff --git a/lib/src/test/java/dev/openfeature/javasdk/DoSomethingProvider.java b/lib/src/test/java/dev/openfeature/javasdk/DoSomethingProvider.java index 69abb347..6cff0a3a 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/DoSomethingProvider.java +++ b/lib/src/test/java/dev/openfeature/javasdk/DoSomethingProvider.java @@ -1,9 +1,15 @@ package dev.openfeature.javasdk; public class DoSomethingProvider implements FeatureProvider { + @Override - public String getName() { - return "test"; + public Metadata getMetadata() { + return new Metadata() { + @Override + public String getName() { + return "test"; + } + }; } @Override diff --git a/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java b/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java index 702d8f05..24f7157d 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java +++ b/lib/src/test/java/dev/openfeature/javasdk/FlagEvaluationSpecTests.java @@ -16,10 +16,12 @@ public class FlagEvaluationSpecTests { return api.getClient(); } + @Specification(number="1.1.1", text="The API, and any state it maintains SHOULD exist as a global singleton, even in cases wherein multiple versions of the API are present at runtime.") @Test void global_singleton() { assertSame(OpenFeatureAPI.getInstance(), OpenFeatureAPI.getInstance()); } + @Specification(number="1.1.2", text="The API MUST provide a function to set the global provider singleton, which accepts an API-conformant provider implementation.") @Test void provider() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); FeatureProvider mockProvider = mock(FeatureProvider.class); @@ -27,6 +29,14 @@ public class FlagEvaluationSpecTests { assertEquals(mockProvider, api.getProvider()); } + @Specification(number="1.1.4", text="The API MUST provide a function for retrieving the metadata field of the configured provider.") + @Test void provider_metadata() { + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + api.setProvider(new DoSomethingProvider()); + assertEquals("test", api.getProviderMetadata().getName()); + } + + @Specification(number="1.1.3", text="The API MUST provide a function to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed.") @Test void hook_addition() { Hook h1 = mock(Hook.class); Hook h2 = mock(Hook.class); @@ -41,12 +51,14 @@ public class FlagEvaluationSpecTests { assertEquals(h2, api.getApiHooks().get(1)); } + @Specification(number="1.1.5", text="The API MUST provide a function for creating a client which accepts the following options: - name (optional): A logical string identifier for the client.") @Test void namedClient() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); Client c = api.getClient("Sir Calls-a-lot"); // TODO: Doesn't say that you can *get* the client name.. which seems useful? } + @Specification(number="1.2.1", text="The client MUST provide a method to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed.") @Test void hookRegistration() { Client c = _client(); Hook m1 = mock(Hook.class); @@ -58,7 +70,8 @@ public class FlagEvaluationSpecTests { assertTrue(hooks.contains(m1)); assertTrue(hooks.contains(m2)); } - + @Specification(number="1.3.1", text="The client MUST provide methods for flag evaluation, with parameters flag key (string, required), default value (boolean | number | string | structure, required), evaluation context (optional), and evaluation options (optional), which returns the flag value.") + @Specification(number="1.3.2.1", text="The client MUST provide methods for typed flag evaluation, including boolean, numeric, string, and structure.") @Test void value_flags() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new DoSomethingProvider()); @@ -83,6 +96,12 @@ public class FlagEvaluationSpecTests { assertEquals(null, c.getObjectValue(key, new Node(), new EvaluationContext(), FlagEvaluationOptions.builder().build())); } + @Specification(number="1.4.1", text="The client MUST provide methods for detailed flag value evaluation with parameters flag key (string, required), default value (boolean | number | string | structure, required), evaluation context (optional), and evaluation options (optional), which returns an evaluation details structure.") + @Specification(number="1.4.2", text="The evaluation details structure's value field MUST contain the evaluated flag value.") + @Specification(number="1.4.3.1", text="The evaluation details structure SHOULD accept a generic argument (or use an equivalent language feature) which indicates the type of the wrapped value field.") + @Specification(number="1.4.4", text="The evaluation details structure's flag key field MUST contain the flag key argument passed to the detailed flag evaluation method.") + @Specification(number="1.4.5", text="In cases of normal execution, the evaluation details structure's variant field MUST contain the value of the variant field in the flag resolution structure returned by the configured provider, if the field is set.") + @Specification(number="1.4.6", text="In cases of normal execution, the evaluation details structure's reason field MUST contain the value of the reason field in the flag resolution structure returned by the configured provider, if the field is set.") @Test void detail_flags() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new DoSomethingProvider()); @@ -114,8 +133,11 @@ public class FlagEvaluationSpecTests { assertEquals(id, c.getIntegerDetails(key, 4)); assertEquals(id, c.getIntegerDetails(key, 4, new EvaluationContext())); assertEquals(id, c.getIntegerDetails(key, 4, new EvaluationContext(), FlagEvaluationOptions.builder().build())); + + // TODO: Structure detail tests. } + @Specification(number="1.5.1", text="The evaluation options structure's hooks field denotes an ordered collection of hooks that the client MUST execute for the respective flag evaluation, in addition to those already configured.") @Test void hooks() { Client c = _client(); Hook clientHook = mock(Hook.class); @@ -128,47 +150,45 @@ public class FlagEvaluationSpecTests { verify(invocationHook, times(1)).before(any(), any()); } + @Specification(number="1.4.9", text="Methods, functions, or operations on the client MUST NOT throw exceptions, or otherwise abnormally terminate. Flag evaluation calls must always return the default value in the event of abnormal execution. Exceptions include functions or methods for the purposes for configuration or setup.") + @Specification(number="1.4.7", text="In cases of abnormal execution, the evaluation details structure's error code field MUST contain a string identifying an error occurred during flag evaluation and the nature of the error.") @Test void broken_provider() { OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new AlwaysBrokenProvider()); Client c = api.getClient(); assertFalse(c.getBooleanValue("key", false)); - + FlagEvaluationDetails details = c.getBooleanDetails("key", false); + assertEquals("BORK", details.getErrorCode()); } + @Specification(number="1.4.10", text="In the case of abnormal execution, the client SHOULD log an informative error message.") @Disabled("Not actually sure how to mock out the slf4j logger") @Test void log_on_error() throws NotImplementedException { - throw new NotImplementedException(); + throw new NotImplementedException("Dunno."); } - @Specification(number="1.1.1", text="The API, and any state it maintains SHOULD exist as a global singleton, even in cases wherein multiple versions of the API are present at runtime.") - @Specification(number="1.1.2", text="The API MUST provide a function to set the global provider singleton, which accepts an API-conformant provider implementation.") - @Specification(number="1.1.3", text="The API MUST provide a function to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed.") - @Specification(number="1.1.4", text="The API MUST provide a function for retrieving the metadata field of the configured provider.") - @Specification(number="1.1.5", text="The API MUST provide a function for creating a client which accepts the following options: - name (optional): A logical string identifier for the client.") - @Specification(number="1.1.6", text="The client creation function MUST NOT throw, or otherwise abnormally terminate.") - @Specification(number="1.2.1", text="The client MUST provide a method to add hooks which accepts one or more API-conformant hooks, and appends them to the collection of any previously added hooks. When new hooks are added, previously added hooks are not removed.") + @Specification(number="1.2.2", text="The client interface MUST define a metadata member or accessor, containing an immutable name field or accessor of type string, which corresponds to the name value supplied during client creation.") - @Specification(number="1.3.1", text="The client MUST provide methods for flag evaluation, with parameters flag key (string, required), default value (boolean | number | string | structure, required), evaluation context (optional), and evaluation options (optional), which returns the flag value.") - @Specification(number="1.3.2.1", text="The client MUST provide methods for typed flag evaluation, including boolean, numeric, string, and structure.") - @Specification(number="1.3.3", text="The client SHOULD guarantee the returned value of any typed flag evaluation method is of the expected type. If the value returned by the underlying provider implementation does not match the expected type, it's to be considered abnormal execution, and the supplied default value should be returned.") - @Specification(number="1.4.1", text="The client MUST provide methods for detailed flag value evaluation with parameters flag key (string, required), default value (boolean | number | string | structure, required), evaluation context (optional), and evaluation options (optional), which returns an evaluation details structure.") - @Specification(number="1.4.2", text="The evaluation details structure's value field MUST contain the evaluated flag value.") - @Specification(number="1.4.3.1", text="The evaluation details structure SHOULD accept a generic argument (or use an equivalent language feature) which indicates the type of the wrapped value field.") - @Specification(number="1.4.4", text="The evaluation details structure's flag key field MUST contain the flag key argument passed to the detailed flag evaluation method.") - @Specification(number="1.4.5", text="In cases of normal execution, the evaluation details structure's variant field MUST contain the value of the variant field in the flag resolution structure returned by the configured provider, if the field is set.") - @Specification(number="1.4.6", text="In cases of normal execution, the evaluation details structure's reason field MUST contain the value of the reason field in the flag resolution structure returned by the configured provider, if the field is set.") - @Specification(number="1.4.7", text="In cases of abnormal execution, the evaluation details structure's error code field MUST contain a string identifying an error occurred during flag evaluation and the nature of the error.") - @Specification(number="1.4.8", text="In cases of abnormal execution (network failure, unhandled error, etc) the reason field in the evaluation details SHOULD indicate an error.") - @Specification(number="1.4.9", text="Methods, functions, or operations on the client MUST NOT throw exceptions, or otherwise abnormally terminate. Flag evaluation calls must always return the default value in the event of abnormal execution. Exceptions include functions or methods for the purposes for configuration or setup.") - @Specification(number="1.4.10", text="In the case of abnormal execution, the client SHOULD log an informative error message.") - @Specification(number="1.4.11", text="The client SHOULD provide asynchronous or non-blocking mechanisms for flag evaluation.") - @Specification(number="1.5.1", text="The evaluation options structure's hooks field denotes an ordered collection of hooks that the client MUST execute for the respective flag evaluation, in addition to those already configured.") - @Specification(number="1.6.1", text="The client SHOULD transform the evaluation context using the provider's context transformer function if one is defined, before passing the result of the transformation to the provider's flag resolution functions.") - - @Test @Disabled void todo() {} - - @Disabled("We're operating in a one request per thread model") - @Test void explicitly_not_doing() { + @Test void clientMetadata() { + Client c = _client(); + assertNull(c.getMetadata().getName()); + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + api.setProvider(new AlwaysBrokenProvider()); + Client c2 = api.getClient("test"); + assertEquals("test", c2.getMetadata().getName()); } + + @Specification(number="1.4.8", text="In cases of abnormal execution (network failure, unhandled error, etc) the reason field in the evaluation details SHOULD indicate an error.") + @Test @Disabled void question_pending() {} + + @Specification(number="1.3.3", text="The client SHOULD guarantee the returned value of any typed flag evaluation method is of the expected type. If the value returned by the underlying provider implementation does not match the expected type, it's to be considered abnormal execution, and the supplied default value should be returned.") + @Specification(number="1.1.6", text="The client creation function MUST NOT throw, or otherwise abnormally terminate.") + @Disabled("Not sure how to test?") + @Test void not_sure_how_to_test() {} + + @Specification(number="1.6.1", text="The client SHOULD transform the evaluation context using the provider's context transformer function if one is defined, before passing the result of the transformation to the provider's flag resolution functions.") + @Test void not_doing_unless_someone_has_a_good_reason_why() {} + + @Specification(number="1.4.11", text="The client SHOULD provide asynchronous or non-blocking mechanisms for flag evaluation.") + @Test void one_thread_per_request_model() {} } diff --git a/lib/src/test/java/dev/openfeature/javasdk/HookContextTest.java b/lib/src/test/java/dev/openfeature/javasdk/HookContextTest.java new file mode 100644 index 00000000..0d2dd2a7 --- /dev/null +++ b/lib/src/test/java/dev/openfeature/javasdk/HookContextTest.java @@ -0,0 +1,26 @@ +package dev.openfeature.javasdk; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +class HookContextTest { + @Specification(number="4.2.2.2", text="Condition: The client metadata field in the hook context MUST be immutable.") + @Specification(number="4.2.2.3", text="Condition: The provider metadata field in the hook context MUST be immutable.") + @Test void metadata_field_is_type_metadata() { + Metadata meta = mock(Metadata.class); + HookContext hc = HookContext.from( + "key", + FlagValueType.BOOLEAN, + meta, + meta, + new EvaluationContext(), + false + ); + + assertTrue(Metadata.class.isAssignableFrom(hc.getClientMetadata().getClass())); + assertTrue(Metadata.class.isAssignableFrom(hc.getProviderMetadata().getClass())); + } + +} \ No newline at end of file diff --git a/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java b/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java index e9ccddb4..52233a16 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java +++ b/lib/src/test/java/dev/openfeature/javasdk/HookSpecTests.java @@ -3,7 +3,6 @@ package dev.openfeature.javasdk; import com.google.common.collect.ImmutableMap; 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; @@ -112,6 +111,7 @@ public class HookSpecTests { } + @Specification(number="4.1.2", text="The hook context SHOULD provide: access to the client metadata and the provider metadata fields.") @Test void optional_properties() { // don't specify HookContext.builder() @@ -126,7 +126,7 @@ public class HookSpecTests { .flagKey("key") .type(FlagValueType.INTEGER) .ctx(new EvaluationContext()) - .provider(new NoOpProvider()) + .providerMetadata(new NoOpProvider().getMetadata()) .defaultValue(1) .build(); @@ -136,7 +136,7 @@ public class HookSpecTests { .type(FlagValueType.INTEGER) .ctx(new EvaluationContext()) .defaultValue(1) - .client(OpenFeatureAPI.getInstance().getClient()) + .clientMetadata(OpenFeatureAPI.getInstance().getClient().getMetadata()) .build(); } @@ -159,20 +159,6 @@ public class HookSpecTests { assertNotNull(feo.getHooks()); } - @Test void errors_in_finally() { - Hook h = mock(Hook.class); - doThrow(RuntimeException.class).when(h).finallyAfter(any(), any()); - - OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(new NoOpProvider()); - Client c= api.getClient(); - - assertThrows(RuntimeException.class, () -> c.getBooleanValue("key", false, null, FlagEvaluationOptions.builder().hook(h).build())); - - verify(h, times(1)).finallyAfter(any(), any()); - verify(h, times(0)).error(any(), any(), any()); - } - @Test void error_hook_run_during_non_finally_stage() { final boolean[] error_called = {false}; Hook h = mock(Hook.class); @@ -274,25 +260,45 @@ public class HookSpecTests { } @Specification(number="4.4.6", text="If an error occurs during the evaluation of before or after hooks, any remaining hooks in the before or after stages MUST NOT be invoked.") - @Disabled("Not actually sure what 'returned to the user' means in this context. There is no exception information returned.") - @Test void error_in_error_stage() { + @Test void error_stops_before() { Hook h = mock(Hook.class); - doThrow(RuntimeException.class).when(h).error(any(), any(), any()); + doThrow(RuntimeException.class).when(h).before(any(), any()); + Hook h2 = mock(Hook.class); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); api.setProvider(new AlwaysBrokenProvider()); Client c = api.getClient(); - FlagEvaluationDetails details = c.getBooleanDetails("key", false, null, FlagEvaluationOptions.builder().hook(h).build()); + c.getBooleanDetails("key", false, null, FlagEvaluationOptions.builder() + .hook(h2) + .hook(h) + .build()); + verify(h, times(1)).before(any(), any()); + verify(h2, times(0)).before(any(), any()); } + @Specification(number="4.4.6", text="If an error occurs during the evaluation of before or after hooks, any remaining hooks in the before or after stages MUST NOT be invoked.") + @Test void error_stops_after() { + Hook h = mock(Hook.class); + doThrow(RuntimeException.class).when(h).after(any(), any(), any()); + Hook h2 = mock(Hook.class); + + Client c = getClient(null); + + c.getBooleanDetails("key", false, null, FlagEvaluationOptions.builder() + .hook(h) + .hook(h2) + .build()); + verify(h, times(1)).after(any(), any(), any()); + verify(h2, times(0)).after(any(), any(), any()); + } @Specification(number="4.2.1", text="hook hints MUST be a structure supports definition of arbitrary properties, with keys of type string, and values of type boolean | string | number | datetime | structure..") @Specification(number="4.5.2", text="hook hints MUST be passed to each hook.") + @Specification(number="4.2.2.1", text="Condition: Hook hints MUST be immutable.") + @Specification(number="4.5.3", text="The hook MUST NOT alter the hook hints structure.") @Test void hook_hints() { - OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(new NoOpProvider()); - Client client = api.getClient(); + Client client = getClient(null); Hook mutatingHook = new Hook() { @Override public Optional before(HookContext ctx, ImmutableMap hints) { @@ -356,9 +362,7 @@ public class HookSpecTests { @Test void error_hooks__before() { Hook hook = mock(Hook.class); doThrow(RuntimeException.class).when(hook).before(any(), any()); - OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(new NoOpProvider()); - Client client = api.getClient(); + Client client = getClient(null); client.getBooleanValue("key", false, new EvaluationContext(), FlagEvaluationOptions.builder().hook(hook).build()); verify(hook, times(1)).before(any(), any()); @@ -369,9 +373,7 @@ public class HookSpecTests { @Test void error_hooks__after() { Hook hook = mock(Hook.class); doThrow(RuntimeException.class).when(hook).after(any(), any(), any()); - OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(new NoOpProvider()); - Client client = api.getClient(); + Client client = getClient(null); client.getBooleanValue("key", false, new EvaluationContext(), FlagEvaluationOptions.builder().hook(hook).build()); verify(hook, times(1)).after(any(), any(), any()); @@ -383,9 +385,7 @@ public class HookSpecTests { Hook hook2 = mock(Hook.class); doThrow(RuntimeException.class).when(hook).before(any(), any()); - OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(new NoOpProvider()); - Client client = api.getClient(); + Client client = getClient(null); client.getBooleanValue("key", false, new EvaluationContext(), FlagEvaluationOptions.builder() @@ -411,10 +411,7 @@ public class HookSpecTests { InOrder order = inOrder(hook, hook2); - - OpenFeatureAPI api = OpenFeatureAPI.getInstance(); - api.setProvider(new NoOpProvider()); - Client client = api.getClient(); + Client client = getClient(null); client.getBooleanValue("key", false, new EvaluationContext(), FlagEvaluationOptions.builder() .hook(hook2) @@ -462,18 +459,57 @@ public class HookSpecTests { assertEquals("exists", ec.getStringAttribute("another")); } - - @Specification(number="4.1.2", text="The hook context SHOULD provide: access to the client metadata and the provider metadata fields.") - @Specification(number="4.2.2.1", text="Condition: Hook hints MUST be immutable.") - @Specification(number="4.2.2.2", text="Condition: The client metadata field in the hook context MUST be immutable.") - @Specification(number="4.2.2.3", text="Condition: The provider metadata field in the hook context MUST be immutable.") - - @Specification(number="4.4.3", text="If a finally hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining finally hooks.") - @Specification(number="4.4.4", text="If an error hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining error hooks.") + @Test void first_finally_broken() { + Hook hook = mock(Hook.class); + doThrow(RuntimeException.class).when(hook).before(any(), any()); + doThrow(RuntimeException.class).when(hook).finallyAfter(any(), any()); + Hook hook2 = mock(Hook.class); + InOrder order = inOrder(hook, hook2); - @Specification(number="4.5.3", text="The hook MUST NOT alter the hook hints structure.") - @Test @Disabled void todo() {} + Client client = getClient(null); + client.getBooleanValue("key", false, new EvaluationContext(), + FlagEvaluationOptions.builder() + .hook(hook2) + .hook(hook) + .build()); + + order.verify(hook).before(any(), any()); + order.verify(hook2).finallyAfter(any(), any()); + order.verify(hook).finallyAfter(any(), any()); + } + + @Specification(number="4.4.4", text="If an error hook abnormally terminates, evaluation MUST proceed, including the execution of any remaining error hooks.") + @Test void first_error_broken() { + + Hook hook = mock(Hook.class); + doThrow(RuntimeException.class).when(hook).before(any(), any()); + doThrow(RuntimeException.class).when(hook).error(any(), any(), any()); + Hook hook2 = mock(Hook.class); + InOrder order = inOrder(hook, hook2); + + Client client = getClient(null); + client.getBooleanValue("key", false, new EvaluationContext(), + FlagEvaluationOptions.builder() + .hook(hook2) + .hook(hook) + .build()); + + order.verify(hook).before(any(), any()); + order.verify(hook2).error(any(), any(), any()); + order.verify(hook).error(any(), any(), any()); + } + + private Client getClient(FeatureProvider provider) { + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + if (provider == null) { + api.setProvider(new NoOpProvider()); + } else { + api.setProvider(provider); + } + Client client = api.getClient(); + return client; + } @Specification(number="4.3.1", text="Hooks MUST specify at least one stage.") @Test void default_methods_so_impossible() {} diff --git a/lib/src/test/java/dev/openfeature/javasdk/MetadataTest.java b/lib/src/test/java/dev/openfeature/javasdk/MetadataTest.java new file mode 100644 index 00000000..bbca73f7 --- /dev/null +++ b/lib/src/test/java/dev/openfeature/javasdk/MetadataTest.java @@ -0,0 +1,19 @@ +package dev.openfeature.javasdk; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.fail; + +class MetadataTest { + @Specification(number="4.2.2.2", text="Condition: The client metadata field in the hook context MUST be immutable.") + @Specification(number="4.2.2.3", text="Condition: The provider metadata field in the hook context MUST be immutable.") + @Test + void metadata_is_immutable() { + try { + Metadata.class.getMethod("setName", String.class); + fail("Not expected to be mutable."); + } catch (NoSuchMethodException e) { + // Pass + } + } +} \ No newline at end of file diff --git a/lib/src/test/java/dev/openfeature/javasdk/NotImplementedException.java b/lib/src/test/java/dev/openfeature/javasdk/NotImplementedException.java index 6a22247b..149de019 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/NotImplementedException.java +++ b/lib/src/test/java/dev/openfeature/javasdk/NotImplementedException.java @@ -4,5 +4,7 @@ public class NotImplementedException extends RuntimeException { private static final long serialVersionUID = 1L; - public NotImplementedException(){} + public NotImplementedException(String message){ + super(message); + } } diff --git a/lib/src/test/java/dev/openfeature/javasdk/ProviderSpecTests.java b/lib/src/test/java/dev/openfeature/javasdk/ProviderSpecTests.java index b92d021e..04621f5c 100644 --- a/lib/src/test/java/dev/openfeature/javasdk/ProviderSpecTests.java +++ b/lib/src/test/java/dev/openfeature/javasdk/ProviderSpecTests.java @@ -78,6 +78,5 @@ public class ProviderSpecTests { @Specification(number="2.10", text="The provider interface MAY define a context transformer method " + "or function, which can be optionally implemented in order to transform the evaluation context prior to " + "flag value resolution.") - @Disabled("I don't think we should do that until we figure out the call signature differences") @Test void not_doing() {} } From 87701362f72912fc132d76e69e5e6c68c5cabb56 Mon Sep 17 00:00:00 2001 From: Justin Abrahms Date: Sun, 12 Jun 2022 20:20:33 -0700 Subject: [PATCH 4/4] Ignore any specification files (used for linting) --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index c68362cb..31dea4a4 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ # Ignore Gradle build output directory build .idea +specification.json