feat!: use sdk-maintained state, require 1.12 (#964)

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
open-feature · Sep 25, 2024 · 4a041b0 · 4a041b0
1 parent 096d407
commit 4a041b0
Show file tree

Hide file tree

Showing 25 changed files with 602 additions and 957 deletions.
diff --git a/hooks/open-telemetry/pom.xml b/hooks/open-telemetry/pom.xml
@@ -38,13 +38,6 @@
     </dependencyManagement>
 
     <dependencies>
-        <dependency>
-            <groupId>dev.openfeature</groupId>
-            <artifactId>sdk</artifactId>
-            <version>[1.4,2.0)</version>
-            <scope>provided</scope>
-        </dependency>
-
         <dependency>
             <groupId>io.opentelemetry</groupId>
             <artifactId>opentelemetry-semconv</artifactId>

diff --git a/pom.xml b/pom.xml
@@ -62,10 +62,11 @@
     <dependencies>
         <dependency>
             <!-- provided -->
+            <!-- this can be overriden in child POMs to support specific SDK requirements -->
             <groupId>dev.openfeature</groupId>
             <artifactId>sdk</artifactId>
-            <!-- 1.0 <= v < 2.0 -->
-            <version>[1.0,2.0)</version>
+            <!-- 1.12 <= v < 2.0 -->
+            <version>[1.12,2.0)</version>
             <!-- use the version provided at runtime -->
             <scope>provided</scope>
         </dependency>

diff --git a/...onfigcat/src/main/java/dev/openfeature/contrib/providers/configcat/ConfigCatProvider.java b/...onfigcat/src/main/java/dev/openfeature/contrib/providers/configcat/ConfigCatProvider.java
@@ -1,24 +1,23 @@
 package dev.openfeature.contrib.providers.configcat;
 
+import java.util.ArrayList;
+import java.util.concurrent.atomic.AtomicBoolean;
+
 import com.configcat.ConfigCatClient;
 import com.configcat.EvaluationDetails;
 import com.configcat.User;
+
 import dev.openfeature.sdk.EvaluationContext;
 import dev.openfeature.sdk.EventProvider;
 import dev.openfeature.sdk.Metadata;
 import dev.openfeature.sdk.ProviderEvaluation;
 import dev.openfeature.sdk.ProviderEventDetails;
-import dev.openfeature.sdk.ProviderState;
 import dev.openfeature.sdk.Value;
 import dev.openfeature.sdk.exceptions.GeneralError;
-import dev.openfeature.sdk.exceptions.ProviderNotReadyError;
 import lombok.Getter;
 import lombok.SneakyThrows;
 import lombok.extern.slf4j.Slf4j;
 
-import java.util.ArrayList;
-import java.util.concurrent.atomic.AtomicBoolean;
-
 /**
  * Provider implementation for ConfigCat.
  */
@@ -36,9 +35,6 @@ public class ConfigCatProvider extends EventProvider {
     @Getter
     private ConfigCatClient configCatClient;
 
-    @Getter
-    private ProviderState state = ProviderState.NOT_READY;
-
     private AtomicBoolean isInitialized = new AtomicBoolean(false);
 
     /**
@@ -64,8 +60,7 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
         configCatClient = ConfigCatClient.get(configCatProviderConfig.getSdkKey(),
             configCatProviderConfig.getOptions());
         configCatProviderConfig.postInit();
-        state = ProviderState.READY;
-        log.info("finished initializing provider, state: {}", state);
+        log.info("finished initializing provider");
 
         configCatClient.getHooks().addOnClientReady(() -> {
             ProviderEventDetails providerEventDetails = ProviderEventDetails.builder()
@@ -123,12 +118,6 @@ public ProviderEvaluation<Value> getObjectEvaluation(String key, Value defaultVa
 
     private <T> ProviderEvaluation<T> getEvaluation(Class<T> classOfT, String key, T defaultValue,
            EvaluationContext ctx) {
-        if (!ProviderState.READY.equals(state)) {
-            if (ProviderState.NOT_READY.equals(state)) {
-                throw new ProviderNotReadyError(PROVIDER_NOT_YET_INITIALIZED);
-            }
-            throw new GeneralError(UNKNOWN_ERROR);
-        }
         User user = ctx == null ? null : ContextTransformer.transform(ctx);
         EvaluationDetails<T> evaluationDetails;
         T evaluatedValue;
@@ -157,6 +146,5 @@ public void shutdown() {
         if (configCatClient != null) {
             configCatClient.close();
         }
-        state = ProviderState.NOT_READY;
     }
 }
diff --git a/...gcat/src/test/java/dev/openfeature/contrib/providers/configcat/ConfigCatProviderTest.java b/...gcat/src/test/java/dev/openfeature/contrib/providers/configcat/ConfigCatProviderTest.java
@@ -1,27 +1,26 @@
 package dev.openfeature.contrib.providers.configcat;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.HashMap;
+
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
 import com.configcat.OverrideBehaviour;
 import com.configcat.OverrideDataSourceBuilder;
 import com.configcat.User;
+
 import dev.openfeature.sdk.Client;
 import dev.openfeature.sdk.ImmutableContext;
 import dev.openfeature.sdk.MutableContext;
 import dev.openfeature.sdk.OpenFeatureAPI;
 import dev.openfeature.sdk.ProviderEventDetails;
 import dev.openfeature.sdk.Value;
-import dev.openfeature.sdk.exceptions.GeneralError;
-import dev.openfeature.sdk.exceptions.ProviderNotReadyError;
 import lombok.SneakyThrows;
 import lombok.extern.slf4j.Slf4j;
-import org.junit.jupiter.api.AfterAll;
-import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.Test;
-
-import java.util.HashMap;
-
-import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 
 /**
  * ConfigCatProvider test, based on local config file evaluation.
@@ -181,27 +180,6 @@ void getStringEvaluationByUser() {
         assertEquals("fallback", client.getStringValue(USERS_FLAG_NAME + "Str", "111", evaluationContext));
     }
 
-    @SneakyThrows
-    @Test
-    void shouldThrowIfNotInitialized() {
-        ConfigCatProviderConfig configCatProviderConfig = ConfigCatProviderConfig.builder().sdkKey("configcat-sdk-1/TEST_KEY-0123456789012/1234567890123456789012").build();
-        ConfigCatProvider tempConfigCatProvider = new ConfigCatProvider(configCatProviderConfig);
-
-        assertThrows(ProviderNotReadyError.class, ()-> tempConfigCatProvider.getBooleanEvaluation("fail_not_initialized", false, new ImmutableContext()));
-
-        OpenFeatureAPI.getInstance().setProviderAndWait("tempConfigCatProvider", tempConfigCatProvider);
-
-        assertThrows(GeneralError.class, ()-> tempConfigCatProvider.initialize(null));
-
-        tempConfigCatProvider.shutdown();
-
-        assertThrows(ProviderNotReadyError.class, ()-> tempConfigCatProvider.getBooleanEvaluation("fail_not_initialized", false, new ImmutableContext()));
-        assertThrows(ProviderNotReadyError.class, ()-> tempConfigCatProvider.getDoubleEvaluation("fail_not_initialized", 0.1, new ImmutableContext()));
-        assertThrows(ProviderNotReadyError.class, ()-> tempConfigCatProvider.getIntegerEvaluation("fail_not_initialized", 3, new ImmutableContext()));
-        assertThrows(ProviderNotReadyError.class, ()-> tempConfigCatProvider.getObjectEvaluation("fail_not_initialized", null, new ImmutableContext()));
-        assertThrows(ProviderNotReadyError.class, ()-> tempConfigCatProvider.getStringEvaluation("fail_not_initialized", "", new ImmutableContext()));
-    }
-
     @Test
     void eventsTest() {
         configCatProvider.emitProviderReady(ProviderEventDetails.builder().build());

diff --git a/providers/flagd/pom.xml b/providers/flagd/pom.xml
@@ -32,16 +32,7 @@
     </developers>
 
     <dependencies>
-
         <!-- we inherent dev.openfeature.javasdk and the test dependencies from the parent pom -->
-        <!-- override parent definition -->
-        <dependency>
-            <groupId>dev.openfeature</groupId>
-            <artifactId>sdk</artifactId>
-            <version>[1.4,2.0)</version>
-            <scope>provided</scope>
-        </dependency>
-
         <dependency>
             <groupId>io.grpc</groupId>
             <artifactId>grpc-netty</artifactId>

diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProvider.java
@@ -1,5 +1,7 @@
 package dev.openfeature.contrib.providers.flagd;
 
+import java.util.List;
+
 import dev.openfeature.contrib.providers.flagd.resolver.Resolver;
 import dev.openfeature.contrib.providers.flagd.resolver.grpc.GrpcResolver;
 import dev.openfeature.contrib.providers.flagd.resolver.grpc.cache.Cache;
@@ -9,27 +11,19 @@
 import dev.openfeature.sdk.Metadata;
 import dev.openfeature.sdk.ProviderEvaluation;
 import dev.openfeature.sdk.ProviderEventDetails;
-import dev.openfeature.sdk.ProviderState;
 import dev.openfeature.sdk.Value;
 import lombok.extern.slf4j.Slf4j;
 
-import java.util.List;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
-
 /**
  * OpenFeature provider for flagd.
  */
 @Slf4j
-@SuppressWarnings({"PMD.TooManyStaticImports", "checkstyle:NoFinalizer"})
+@SuppressWarnings({ "PMD.TooManyStaticImports", "checkstyle:NoFinalizer" })
 public class FlagdProvider extends EventProvider {
     private static final String FLAGD_PROVIDER = "flagD Provider";
-
-    private final ReadWriteLock lock = new ReentrantReadWriteLock();
     private final Resolver flagResolver;
-    private ProviderState state = ProviderState.NOT_READY;
-    private boolean initialized = false;
+    private volatile boolean initialized = false;
+    private volatile boolean connected = false;
 
     private EvaluationContext evaluationContext;
 
@@ -52,14 +46,14 @@ public FlagdProvider() {
     public FlagdProvider(final FlagdOptions options) {
         switch (options.getResolverType().asString()) {
             case Config.RESOLVER_IN_PROCESS:
-                this.flagResolver = new InProcessResolver(options, this::setState);
+                this.flagResolver = new InProcessResolver(options, this::isConnected,
+                        this::onResolverConnectionChanged);
                 break;
             case Config.RESOLVER_RPC:
-                this.flagResolver =
-                        new GrpcResolver(options,
-                                new Cache(options.getCacheType(), options.getMaxCacheSize()),
-                                this::getState,
-                                this::setState);
+                this.flagResolver = new GrpcResolver(options,
+                        new Cache(options.getCacheType(), options.getMaxCacheSize()),
+                        this::isConnected,
+                        this::onResolverConnectionChanged);
                 break;
             default:
                 throw new IllegalStateException(
@@ -86,24 +80,13 @@ public synchronized void shutdown() {
 
         try {
             this.flagResolver.shutdown();
-            this.initialized = false;
         } catch (Exception e) {
             log.error("Error during shutdown {}", FLAGD_PROVIDER, e);
-        }
-    }
-
-    @Override
-    public ProviderState getState() {
-        Lock l = this.lock.readLock();
-        try {
-            l.lock();
-            return this.state;
         } finally {
-            l.unlock();
+            this.initialized = false;
         }
     }
 
-
     @Override
     public Metadata getMetadata() {
         return () -> FLAGD_PROVIDER;
@@ -142,49 +125,32 @@ private EvaluationContext mergeContext(final EvaluationContext clientCallCtx) {
         return clientCallCtx;
     }
 
-    private void setState(ProviderState newState, List<String> changedFlagsKeys) {
-        ProviderState oldState;
-        Lock l = this.lock.writeLock();
-        try {
-            l.lock();
-            oldState = this.state;
-            this.state = newState;
-        } finally {
-            l.unlock();
-        }
-        this.handleStateTransition(oldState, newState, changedFlagsKeys);
+    private boolean isConnected() {
+        return this.connected;
     }
 
-    private void handleStateTransition(ProviderState oldState, ProviderState newState, List<String> changedFlagKeys) {
-        // we got initialized
-        if (ProviderState.NOT_READY.equals(oldState) && ProviderState.READY.equals(newState)) {
-            // nothing to do, the SDK emits the events
-            log.debug("Init completed");
-            return;
-        }
-        // we got shutdown, not checking oldState as behavior remains the same for shutdown
-        if (ProviderState.NOT_READY.equals(newState)) {
-            // nothing to do
-            log.debug("shutdown completed");
-            return;
-        }
+    private void onResolverConnectionChanged(boolean newConnectedState, List<String> changedFlagKeys) {
+        boolean previous = connected;
+        boolean current = newConnectedState;
+        this.connected = newConnectedState;
+
         // configuration changed
-        if (ProviderState.READY.equals(oldState) && ProviderState.READY.equals(newState)) {
+        if (initialized && previous && current) {
             log.debug("Configuration changed");
             ProviderEventDetails details = ProviderEventDetails.builder().flagsChanged(changedFlagKeys)
                     .message("configuration changed").build();
             this.emitProviderConfigurationChanged(details);
             return;
         }
         // there was an error
-        if (ProviderState.READY.equals(oldState) && ProviderState.ERROR.equals(newState)) {
+        if (initialized && previous && !current) {
             log.debug("There has been an error");
             ProviderEventDetails details = ProviderEventDetails.builder().message("there has been an error").build();
             this.emitProviderError(details);
             return;
         }
-        // we recover from an error
-        if (ProviderState.ERROR.equals(oldState) && ProviderState.READY.equals(newState)) {
+        // we recovered from an error
+        if (initialized && !previous && current) {
             log.debug("Recovered from error");
             ProviderEventDetails details = ProviderEventDetails.builder().message("recovered from error").build();
             this.emitProviderReady(details);

diff --git a/...ers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/Util.java b/...ers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/Util.java
@@ -1,6 +1,6 @@
 package dev.openfeature.contrib.providers.flagd.resolver.common;
 
-import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Supplier;
 
 import dev.openfeature.sdk.exceptions.GeneralError;
 
@@ -14,20 +14,23 @@ private Util() {
 
     /**
      * A helper to block the caller for given conditions.
-     *
-     * @param deadline number of milliseconds to block
-     * @param check    {@link AtomicBoolean} to check for status true
+     * 
+     * @param deadline          number of milliseconds to block
+     * @param connectedSupplier func to check for status true
+     * @throws InterruptedException if interrupted
      */
-    public static void busyWaitAndCheck(final Long deadline, final AtomicBoolean check) throws InterruptedException {
+    public static void busyWaitAndCheck(final Long deadline, final Supplier<Boolean> connectedSupplier)
+            throws InterruptedException {
         long start = System.currentTimeMillis();
 
         do {
             if (deadline <= System.currentTimeMillis() - start) {
                 throw new GeneralError(
-                    String.format("Deadline exceeded. Condition did not complete within the %d deadline", deadline));
+                        String.format("Deadline exceeded. Condition did not complete within the %d deadline",
+                                deadline));
             }
 
             Thread.sleep(50L);
-        } while (!check.get());
+        } while (!connectedSupplier.get());
     }
 }