feat: Support mapping a client to a given provider. (#388)

* Support mapping a client to a given provider. Signed-off-by: Justin Abrahms <justin@abrah.ms> * Add a few javadocs. Signed-off-by: Justin Abrahms <justin@abrah.ms> * Special case the null client name Signed-off-by: Justin Abrahms <justin@abrah.ms> * Add some missing test cases. Signed-off-by: Justin Abrahms <justin@abrah.ms> * Moving to an object map unwraps the values. Signed-off-by: Justin Abrahms <justin@abrah.ms> * Fix equality test. Signed-off-by: Justin Abrahms <justin@abrah.ms> * Carry targeting key when copying over null object. Signed-off-by: Justin Abrahms <justin@abrah.ms> * Test provider name, not object equality. Signed-off-by: Justin Abrahms <justin@abrah.ms> * Client-based getProvider is now an overload; Use read lock, not write lock. Signed-off-by: Justin Abrahms <justin@abrah.ms> * Update src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java Co-authored-by: Lars Opitz <lars@lars-opitz.de> Signed-off-by: Justin Abrahms <justin@abrah.ms> * Simplify locking logic around providers. There's no such thing as "API without a provider set" anymore. We now default to NoOpProvider in the API (not client). Signed-off-by: Justin Abrahms <justin@abrah.ms> * Add a few missing tests Signed-off-by: Justin Abrahms <justin@abrah.ms> --------- Signed-off-by: Justin Abrahms <justin@abrah.ms> Co-authored-by: Lars Opitz <lars@lars-opitz.de> Co-authored-by: Michael Beemer <beeme1mr@users.noreply.github.com>
open-feature · May 19, 2023 · d4c43d7 · d4c43d7
1 parent 1af8e96
commit d4c43d7
Show file tree

Hide file tree

Showing 10 changed files with 113 additions and 39 deletions.
diff --git a/src/main/java/dev/openfeature/sdk/MutableContext.java b/src/main/java/dev/openfeature/sdk/MutableContext.java
@@ -4,6 +4,7 @@
 import java.util.List;
 import java.util.Map;
 
+import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.Setter;
 import lombok.ToString;
@@ -16,6 +17,7 @@
  * be modified after instantiation.
  */
 @ToString
+@EqualsAndHashCode
 @SuppressWarnings("PMD.BeanMembersShouldSerialize")
 public class MutableContext implements EvaluationContext {
 
@@ -88,7 +90,7 @@ public MutableContext add(String key, List<Value> value) {
     @Override
     public EvaluationContext merge(EvaluationContext overridingContext) {
         if (overridingContext == null) {
-            return new MutableContext(this.asMap());
+            return new MutableContext(this.targetingKey, this.asMap());
         }
 
         Map<String, Value> merged = this.merge(map -> new MutableStructure(map),

diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java b/src/main/java/dev/openfeature/sdk/OpenFeatureAPI.java
@@ -1,8 +1,7 @@
 package dev.openfeature.sdk;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
+import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
 
 import javax.annotation.Nullable;
 
@@ -16,11 +15,11 @@
 public class OpenFeatureAPI {
     // package-private multi-read/single-write lock
     static AutoCloseableReentrantReadWriteLock hooksLock = new AutoCloseableReentrantReadWriteLock();
-    static AutoCloseableReentrantReadWriteLock providerLock = new AutoCloseableReentrantReadWriteLock();
     static AutoCloseableReentrantReadWriteLock contextLock = new AutoCloseableReentrantReadWriteLock();
-    private FeatureProvider provider;
     private EvaluationContext evaluationContext;
-    private List<Hook> apiHooks;
+    private final List<Hook> apiHooks;
+    private FeatureProvider defaultProvider = new NoOpProvider();
+    private final Map<String, FeatureProvider> providers = new ConcurrentHashMap<>();
 
     private OpenFeatureAPI() {
         this.apiHooks = new ArrayList<>();
@@ -39,7 +38,11 @@ public static OpenFeatureAPI getInstance() {
     }
 
     public Metadata getProviderMetadata() {
-        return provider.getMetadata();
+        return defaultProvider.getMetadata();
+    }
+
+    public Metadata getProviderMetadata(String clientName) {
+        return getProvider(clientName).getMetadata();
     }
 
     public Client getClient() {
@@ -73,23 +76,44 @@ public EvaluationContext getEvaluationContext() {
     }
 
     /**
-     * {@inheritDoc}
+     * Set the default provider.
      */
     public void setProvider(FeatureProvider provider) {
-        try (AutoCloseableLock __ = providerLock.writeLockAutoCloseable()) {
-            this.provider = provider;
+        if (provider == null) {
+            throw new IllegalArgumentException("Provider cannot be null");
         }
+        defaultProvider = provider;
     }
 
     /**
-     * {@inheritDoc}
+     * Add a provider for a named client.
+     * @param clientName The name of the client.
+     * @param provider The provider to set.
      */
-    public FeatureProvider getProvider() {
-        try (AutoCloseableLock __ = providerLock.readLockAutoCloseable()) {
-            return this.provider;
+    public void setProvider(String clientName, FeatureProvider provider) {
+        if (provider == null) {
+            throw new IllegalArgumentException("Provider cannot be null");
         }
+        this.providers.put(clientName, provider);
     }
 
+    /**
+     * Return the default provider.
+     */
+    public FeatureProvider getProvider() {
+        return defaultProvider;
+    }
+
+    /**
+     * Fetch a provider for a named client. If not found, return the default.
+     * @param name The client name to look for.
+     * @return A named {@link FeatureProvider}
+     */
+    public FeatureProvider getProvider(String name) {
+        return Optional.ofNullable(name).map(this.providers::get).orElse(defaultProvider);
+    }
+
+
     /**
      * {@inheritDoc}
      */

diff --git a/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java b/src/main/java/dev/openfeature/sdk/OpenFeatureClient.java
@@ -99,17 +99,14 @@ private <T> FlagEvaluationDetails<T> evaluateFlag(FlagValueType type, String key
         FlagEvaluationDetails<T> details = null;
         List<Hook> mergedHooks = null;
         HookContext<T> hookCtx = null;
-        FeatureProvider provider = null;
+        FeatureProvider provider;
 
         try {
             final EvaluationContext apiContext;
             final EvaluationContext clientContext;
 
             // openfeatureApi.getProvider() must be called once to maintain a consistent reference
-            provider = ObjectUtils.defaultIfNull(openfeatureApi.getProvider(), () -> {
-                log.debug("No provider configured, using no-op provider.");
-                return new NoOpProvider();
-            });
+            provider = openfeatureApi.getProvider(this.name);
 
             mergedHooks = ObjectUtils.merge(provider.getProviderHooks(), flagOptions.getHooks(), clientHooks,
                     openfeatureApi.getHooks());

diff --git a/src/test/java/dev/openfeature/sdk/ClientProviderMappingTest.java b/src/test/java/dev/openfeature/sdk/ClientProviderMappingTest.java
@@ -0,0 +1,23 @@
+package dev.openfeature.sdk;
+
+import io.cucumber.java.eo.Do;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class ClientProviderMappingTest {
+    @Test
+    void clientProviderTest() {
+        OpenFeatureAPI api = OpenFeatureAPI.getInstance();
+
+        api.setProvider("client1", new DoSomethingProvider());
+        api.setProvider("client2", new NoOpProvider());
+
+        Client c1 = api.getClient("client1");
+        Client c2 = api.getClient("client2");
+
+        assertTrue(c1.getBooleanValue("test", false));
+        assertFalse(c2.getBooleanValue("test", false));
+    }
+}
diff --git a/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java b/src/test/java/dev/openfeature/sdk/DeveloperExperienceTest.java
@@ -19,15 +19,6 @@
 class DeveloperExperienceTest implements HookFixtures {
     transient String flagKey = "mykey";
 
-    @Test void noProviderSet() {
-        final String noOp = "no-op";
-        OpenFeatureAPI api = OpenFeatureAPI.getInstance();
-        api.setProvider(null);
-        Client client = api.getClient();
-        String retval = client.getStringValue(flagKey, noOp);
-        assertEquals(noOp, retval);
-    }
-
     @Test void simpleBooleanFlag() {
         OpenFeatureAPI api = OpenFeatureAPI.getInstance();
         api.setProvider(new NoOpProvider());

diff --git a/src/test/java/dev/openfeature/sdk/EvalContextTest.java b/src/test/java/dev/openfeature/sdk/EvalContextTest.java
@@ -162,6 +162,13 @@ public class EvalContextTest {
         assertEquals(key1, ctxMerged.getTargetingKey());
     }
 
+    @Test void merge_null_returns_value() {
+        MutableContext ctx1 = new MutableContext("key");
+        ctx1.add("mything", "value");
+        EvaluationContext result = ctx1.merge(null);
+        assertEquals(ctx1, result);
+    }
+
     @Test void merge_targeting_key() {
         String key1 = "key1";
         MutableContext ctx1 = new MutableContext(key1);

diff --git a/src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java b/src/test/java/dev/openfeature/sdk/ImmutableStructureTest.java
@@ -111,4 +111,15 @@ void GettingAMissingValueShouldReturnNull() {
         Object value = structure.getValue("missing");
         assertNull(value);
     }
+
+    @Test void objectMapTest() {
+        Map<String, Value> attrs = new HashMap<>();
+        attrs.put("test", new Value(45));
+        ImmutableStructure structure = new ImmutableStructure(attrs);
+
+        Map<String, Integer> expected = new HashMap<>();
+        expected.put("test", 45);
+
+        assertEquals(expected, structure.asObjectMap());
+    }
 }
diff --git a/src/test/java/dev/openfeature/sdk/LockingTest.java b/src/test/java/dev/openfeature/sdk/LockingTest.java
@@ -19,7 +19,6 @@ class LockingTest {
     private OpenFeatureClient client;
     private AutoCloseableReentrantReadWriteLock apiContextLock;
     private AutoCloseableReentrantReadWriteLock apiHooksLock;
-    private AutoCloseableReentrantReadWriteLock apiProviderLock;
     private AutoCloseableReentrantReadWriteLock clientContextLock;
     private AutoCloseableReentrantReadWriteLock clientHooksLock;
 
@@ -33,10 +32,8 @@ void beforeEach() {
         client = (OpenFeatureClient) api.getClient();
 
         apiContextLock = setupLock(apiContextLock, mockInnerReadLock(), mockInnerWriteLock());
-        apiProviderLock = setupLock(apiProviderLock, mockInnerReadLock(), mockInnerWriteLock());
         apiHooksLock = setupLock(apiHooksLock, mockInnerReadLock(), mockInnerWriteLock());
         OpenFeatureAPI.contextLock = apiContextLock;
-        OpenFeatureAPI.providerLock = apiProviderLock;
         OpenFeatureAPI.hooksLock = apiHooksLock;
 
         clientContextLock = setupLock(clientContextLock, mockInnerReadLock(), mockInnerWriteLock());
@@ -91,12 +88,6 @@ void getContextShouldReadLockAndUnlock() {
         verify(apiContextLock.readLock()).unlock();
     }
 
-    @Test
-    void setProviderShouldWriteLockAndUnlock() {
-        api.setProvider(new DoSomethingProvider());
-        verify(apiProviderLock.writeLock()).lock();
-        verify(apiProviderLock.writeLock()).unlock();
-    }
 
     @Test
     void clearHooksShouldWriteLockAndUnlock() {

diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureAPITest.java
@@ -0,0 +1,26 @@
+package dev.openfeature.sdk;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+public class OpenFeatureAPITest {
+    @Test
+    void namedProviderTest() {
+        OpenFeatureAPI api = OpenFeatureAPI.getInstance();
+        FeatureProvider provider = new NoOpProvider();
+        api.setProvider("namedProviderTest", provider);
+        assertEquals(provider.getMetadata().getName(), api.getProviderMetadata("namedProviderTest").getName());
+    }
+
+    @Test void settingDefaultProviderToNullErrors() {
+        OpenFeatureAPI api = OpenFeatureAPI.getInstance();
+        assertThrows(IllegalArgumentException.class, () -> api.setProvider(null));
+    }
+
+    @Test void settingNamedClientProviderToNullErrors() {
+        OpenFeatureAPI api = OpenFeatureAPI.getInstance();
+        assertThrows(IllegalArgumentException.class, () -> api.setProvider("client-name", null));
+    }
+}
diff --git a/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java b/src/test/java/dev/openfeature/sdk/OpenFeatureClientTest.java
@@ -27,7 +27,7 @@ class OpenFeatureClientTest implements HookFixtures {
     @DisplayName("should not throw exception if hook has different type argument than hookContext")
     void shouldNotThrowExceptionIfHookHasDifferentTypeArgumentThanHookContext() {
         OpenFeatureAPI api = mock(OpenFeatureAPI.class);
-        when(api.getProvider()).thenReturn(new DoSomethingProvider());
+        when(api.getProvider(any())).thenReturn(new DoSomethingProvider());
         when(api.getHooks()).thenReturn(Arrays.asList(mockBooleanHook(), mockStringHook()));
 
         OpenFeatureClient client = new OpenFeatureClient(api, "name", "version");
@@ -57,6 +57,8 @@ void mergeContextTest() {
             context -> context.getTargetingKey().equals(targetingKey)))).thenReturn(ProviderEvaluation.<Boolean>builder()
           .value(true).build());
         when(api.getProvider()).thenReturn(mockProvider);
+        when(api.getProvider(any())).thenReturn(mockProvider);
+
 
         OpenFeatureClient client = new OpenFeatureClient(api, "name", "version");
         client.setEvaluationContext(ctx);