From 638db9c30ad937453076888764028d519bd6cbf6 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Fri, 17 May 2024 10:52:53 -0400 Subject: [PATCH 1/2] chore: javadoc and tests for api, context Signed-off-by: Todd Baert --- .../dev/openfeature/sdk/OpenFeatureAPI.java | 57 ++++++++++++++++--- .../openfeature/sdk/DoSomethingProvider.java | 10 ---- .../sdk/FlagEvaluationSpecTest.java | 55 ++++++++++++------ 3 files changed, 87 insertions(+), 35 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index bf7f24c5..49456e0c 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -48,30 +48,62 @@ public static OpenFeatureAPI getInstance() { return SingletonHolder.INSTANCE; } + /** + * Get metadata about the default provider. + * + * @return the provider metadata + */ public Metadata getProviderMetadata() { return getProvider().getMetadata(); } + /** + * Get metadata about a registered provider using the client name. + * An unbound or empty client name will return metadata from the default provider. + * + * @param domain an identifier which logically binds clients with providers + * @return the provider metadata + */ public Metadata getProviderMetadata(String domain) { return getProvider(domain).getMetadata(); } /** - * {@inheritDoc} + * A factory function for creating new, OpenFeature clients. + * Clients can contain their own state (e.g. logger, hook, context). + * Multiple clients can be used to segment feature flag configuration. + * All un-named or unbound clients use the default provider. + * + * @return a new client instance */ public Client getClient() { return getClient(null, null); } /** - * {@inheritDoc} + * A factory function for creating new domainless OpenFeature clients. + * Clients can contain their own state (e.g. logger, hook, context). + * Multiple clients can be used to segment feature flag configuration. + * If there is already a provider bound to this domain, this provider will be used. + * Otherwise, the default provider is used until a provider is assigned to that domain. + * + * @param name an identifier which logically binds clients with providers + * @return a new client instance */ public Client getClient(String domain) { return getClient(domain, null); } /** - * {@inheritDoc} + * A factory function for creating new domainless OpenFeature clients. + * Clients can contain their own state (e.g. logger, hook, context). + * Multiple clients can be used to segment feature flag configuration. + * If there is already a provider bound to this domain, this provider will be used. + * Otherwise, the default provider is used until a provider is assigned to that domain. + * + * @param name a identifier which logically binds clients with providers + * @param version a version identifier + * @return a new client instance */ public Client getClient(String domain, String version) { return new OpenFeatureClient(this, @@ -80,7 +112,10 @@ public Client getClient(String domain, String version) { } /** - * {@inheritDoc} + * Sets the global evaluation context, which will be used for all evaluations. + * + * @param evaluationContext the context + * @return api instance */ public OpenFeatureAPI setEvaluationContext(EvaluationContext evaluationContext) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { @@ -90,7 +125,9 @@ public OpenFeatureAPI setEvaluationContext(EvaluationContext evaluationContext) } /** - * {@inheritDoc} + * Gets the global evaluation context, which will be used for all evaluations. + * + * @return evaluation context */ public EvaluationContext getEvaluationContext() { try (AutoCloseableLock __ = lock.readLockAutoCloseable()) { @@ -250,7 +287,10 @@ public FeatureProvider getProvider(String domain) { } /** - * {@inheritDoc} + * Adds hooks for globally, used for all evaluations. + * Hooks are run in the order they're added in the before stage. They are run in reverse order for all other stages. + * + * @param hooks The hook to add. */ public void addHooks(Hook... hooks) { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { @@ -259,7 +299,8 @@ public void addHooks(Hook... hooks) { } /** - * {@inheritDoc} + * Fetch the hooks associated to this client. + * @return A list of {@link Hook}s. */ public List getHooks() { try (AutoCloseableLock __ = lock.readLockAutoCloseable()) { @@ -268,7 +309,7 @@ public List getHooks() { } /** - * {@inheritDoc} + * Removes all hooks. */ public void clearHooks() { try (AutoCloseableLock __ = lock.writeLockAutoCloseable()) { diff --git a/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java b/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java index 4fdc433b..32293446 100644 --- a/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java +++ b/src/test/java/dev/openfeature/sdk/DoSomethingProvider.java @@ -7,8 +7,6 @@ class DoSomethingProvider implements FeatureProvider { static final ImmutableMetadata DEFAULT_METADATA = ImmutableMetadata.builder().build(); private ImmutableMetadata flagMetadata; - private EvaluationContext savedContext; - public DoSomethingProvider() { this.flagMetadata = DEFAULT_METADATA; } @@ -17,10 +15,6 @@ public DoSomethingProvider(ImmutableMetadata flagMetadata) { this.flagMetadata = flagMetadata; } - EvaluationContext getMergedContext() { - return savedContext; - } - @Override public Metadata getMetadata() { return () -> name; @@ -28,7 +22,6 @@ public Metadata getMetadata() { @Override public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx) { - savedContext = ctx; return ProviderEvaluation.builder() .value(!defaultValue) .flagMetadata(flagMetadata) @@ -45,7 +38,6 @@ public ProviderEvaluation getStringEvaluation(String key, String default @Override public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx) { - savedContext = ctx; return ProviderEvaluation.builder() .value(defaultValue * 100) .flagMetadata(flagMetadata) @@ -54,7 +46,6 @@ public ProviderEvaluation getIntegerEvaluation(String key, Integer defa @Override public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) { - savedContext = ctx; return ProviderEvaluation.builder() .value(defaultValue * 100) .flagMetadata(flagMetadata) @@ -63,7 +54,6 @@ public ProviderEvaluation getDoubleEvaluation(String key, Double default @Override public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, EvaluationContext invocationContext) { - savedContext = invocationContext; return ProviderEvaluation.builder() .value(null) .flagMetadata(flagMetadata) diff --git a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java index d85d8825..b2a7510e 100644 --- a/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java +++ b/src/test/java/dev/openfeature/sdk/FlagEvaluationSpecTest.java @@ -11,7 +11,9 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -307,11 +309,30 @@ public void initialize(EvaluationContext evaluationContext) throws Exception { assertNotNull(result.getFlagMetadata()); } - @Specification(number="3.2.1.1", text="The API, Client and invocation MUST have a method for supplying evaluation context.") @Specification(number="3.2.2.1", text="The API MUST have a method for setting the global evaluation context.") + @Test void api_context() { + String contextKey = "some-key"; + String contextValue = "some-value"; + DoSomethingProvider provider = spy( new DoSomethingProvider()); + FeatureProviderTestUtils.setFeatureProvider(provider); + + Map attributes = new HashMap<>(); + attributes.put(contextKey, new Value(contextValue)); + EvaluationContext apiCtx = new ImmutableContext(attributes); + + // set the global context + api.setEvaluationContext(apiCtx); + Client client = api.getClient(); + client.getBooleanValue("any-flag", false); + + // assert that the value from the global context was passed to the provider + verify(provider).getBooleanEvaluation(any(), any(), argThat((arg) -> arg.getValue(contextKey).asString().equals(contextValue))); + } + + @Specification(number="3.2.1.1", text="The API, Client and invocation MUST have a method for supplying evaluation context.") @Specification(number="3.2.3", text="Evaluation context MUST be merged in the order: API (global; lowest precedence) -> transaction -> client -> invocation -> before hooks (highest precedence), with duplicate values being overwritten.") @Test void multi_layer_context_merges_correctly() { - DoSomethingProvider provider = new DoSomethingProvider(); + DoSomethingProvider provider = spy(new DoSomethingProvider()); FeatureProviderTestUtils.setFeatureProvider(provider); TransactionContextPropagator transactionContextPropagator = new ThreadLocalTransactionContextPropagator(); api.setTransactionContextPropagator(transactionContextPropagator); @@ -356,21 +377,21 @@ public void initialize(EvaluationContext evaluationContext) throws Exception { invocationAttributes.put("invocation", new Value("4")); EvaluationContext invocationCtx = new ImmutableContext(invocationAttributes); - // dosomethingprovider inverts this value. - assertTrue(c.getBooleanValue("key", false, invocationCtx)); - - EvaluationContext merged = provider.getMergedContext(); - assertEquals("1", merged.getValue("api").asString()); - assertEquals("2", merged.getValue("transaction").asString()); - assertEquals("3", merged.getValue("client").asString()); - assertEquals("4", merged.getValue("invocation").asString()); - assertEquals("2", merged.getValue("common1").asString(), "transaction merge is incorrect"); - assertEquals("3", merged.getValue("common2").asString(), "api client merge is incorrect"); - assertEquals("4", merged.getValue("common3").asString(), "invocation merge is incorrect"); - assertEquals("3", merged.getValue("common4").asString(), "api client merge is incorrect"); - assertEquals("4", merged.getValue("common5").asString(), "invocation merge is incorrect"); - assertEquals("4", merged.getValue("common6").asString(), "invocation merge is incorrect"); - + c.getBooleanValue("key", false, invocationCtx); + + // assert the connect overrides + verify(provider).getBooleanEvaluation(any(), any(), argThat((arg) -> { + return arg.getValue("api").asString().equals("1") && + arg.getValue("transaction").asString().equals("2") && + arg.getValue("client").asString().equals("3") && + arg.getValue("invocation").asString().equals("4") && + arg.getValue("common1").asString().equals("2") && + arg.getValue("common2").asString().equals("3") && + arg.getValue("common3").asString().equals("4") && + arg.getValue("common4").asString().equals("3") && + arg.getValue("common5").asString().equals("4") && + arg.getValue("common6").asString().equals("4"); + })); } @Specification(number="3.3.1.1", text="The API SHOULD have a method for setting a transaction context propagator.") From d406218f33e124a6ee68180419d99faeaf5f6112 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Fri, 14 Jun 2024 10:09:36 -0400 Subject: [PATCH 2/2] fixup: lint Signed-off-by: Todd Baert --- src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java index 49456e0c..f4d20caf 100644 --- a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java +++ b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java @@ -87,7 +87,7 @@ public Client getClient() { * If there is already a provider bound to this domain, this provider will be used. * Otherwise, the default provider is used until a provider is assigned to that domain. * - * @param name an identifier which logically binds clients with providers + * @param domain an identifier which logically binds clients with providers * @return a new client instance */ public Client getClient(String domain) { @@ -101,7 +101,7 @@ public Client getClient(String domain) { * If there is already a provider bound to this domain, this provider will be used. * Otherwise, the default provider is used until a provider is assigned to that domain. * - * @param name a identifier which logically binds clients with providers + * @param domain a identifier which logically binds clients with providers * @param version a version identifier * @return a new client instance */