feat(flagd): migrate file to own provider type

Signed-off-by: Simon Schrottner <[email protected]>
open-feature · Jan 22, 2025 · c233a4f · c233a4f
1 parent 23c5e69
commit c233a4f
Show file tree

Hide file tree

Showing 14 changed files with 131 additions and 50 deletions.
diff --git a/providers/flagd/spec b/providers/flagd/spec
diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/Config.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/Config.java
@@ -40,6 +40,7 @@ public final class Config {
 
     static final String RESOLVER_RPC = "rpc";
     static final String RESOLVER_IN_PROCESS = "in-process";
+    static final String RESOLVER_FILE = "file";
 
     public static final String STATIC_REASON = "STATIC";
     public static final String CACHED_REASON = "CACHED";
@@ -87,6 +88,8 @@ static Resolver fromValueProvider(Function<String, String> provider) {
                 return Resolver.IN_PROCESS;
             case "rpc":
                 return Resolver.RPC;
+            case "file":
+                return Resolver.FILE;
             default:
                 log.warn("Unsupported resolver variable: {}", resolverVar);
                 return DEFAULT_RESOLVER_TYPE;
@@ -143,6 +146,11 @@ public String asString() {
             public String asString() {
                 return RESOLVER_IN_PROCESS;
             }
+        },
+        FILE {
+            public String asString() {
+                return RESOLVER_FILE;
+            }
         }
     }
 }
diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdOptions.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdOptions.java
@@ -12,6 +12,7 @@
 import java.util.function.Function;
 import lombok.Builder;
 import lombok.Getter;
+import org.apache.commons.lang3.StringUtils;
 
 /**
  * FlagdOptions is a builder to build flagd provider options.
@@ -119,8 +120,7 @@ public class FlagdOptions {
      * File source of flags to be used by offline mode.
      * Setting this enables the offline mode of the in-process provider.
      */
-    @Builder.Default
-    private String offlineFlagSourcePath = fallBackToEnvOrDefault(Config.OFFLINE_SOURCE_PATH, null);
+    private String offlineFlagSourcePath;
 
     /**
      * gRPC custom target string.
@@ -193,7 +193,21 @@ void prebuild() {
                 resolverType = fromValueProvider(System::getenv);
             }
 
-            if (port == 0) {
+
+            if (StringUtils.isEmpty(offlineFlagSourcePath)) {
+                offlineFlagSourcePath = fallBackToEnvOrDefault(Config.OFFLINE_SOURCE_PATH, null);
+            }
+
+            if (!StringUtils.isEmpty(offlineFlagSourcePath) && resolverType == Config.Resolver.IN_PROCESS) {
+                resolverType = Config.Resolver.FILE;
+            }
+
+            // We need a file path for FILE Provider
+            if (StringUtils.isEmpty(offlineFlagSourcePath) && resolverType == Config.Resolver.FILE) {
+                throw new IllegalArgumentException("Resolver Type 'FILE' requires a offlineFlagSourcePath");
+            }
+
+            if (port == 0 && resolverType != Config.Resolver.FILE) {
                 port = Integer.parseInt(
                         fallBackToEnvOrDefault(Config.PORT_ENV_VAR_NAME, determineDefaultPortForResolver()));
             }

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
@@ -80,6 +80,7 @@ public FlagdProvider() {
      */
     public FlagdProvider(final FlagdOptions options) {
         switch (options.getResolverType().asString()) {
+            case Config.RESOLVER_FILE:
             case Config.RESOLVER_IN_PROCESS:
                 this.flagResolver = new InProcessResolver(options, this::onProviderEvent);
                 break;
@@ -137,14 +138,14 @@ public void initialize(EvaluationContext evaluationContext) throws Exception {
     public void shutdown() {
         synchronized (eventsLock) {
             if (!eventsLock.initialized) {
-                return;
+            return;
+        }
+        try {
+            this.flagResolver.shutdown();
+            if (errorExecutor != null) {
+                errorExecutor.shutdownNow();
+                errorExecutor.awaitTermination(deadline, TimeUnit.MILLISECONDS);
             }
-            try {
-                this.flagResolver.shutdown();
-                if (errorExecutor != null) {
-                    errorExecutor.shutdownNow();
-                    errorExecutor.awaitTermination(deadline, TimeUnit.MILLISECONDS);
-                }
             } catch (Exception e) {
                 log.error("Error during shutdown {}", FLAGD_PROVIDER, e);
             } finally {

diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdOptionsTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdOptionsTest.java
@@ -60,12 +60,9 @@ void TestBuilderOptions() {
                 .certPath("etc/cert/ca.crt")
                 .cacheType("lru")
                 .maxCacheSize(100)
-                .selector("app=weatherApp")
-                .offlineFlagSourcePath("some-path")
-                .openTelemetry(openTelemetry)
-                .customConnector(connector)
                 .resolverType(Resolver.IN_PROCESS)
                 .targetUri("dns:///localhost:8016")
+                .customConnector(connector)
                 .keepAlive(1000)
                 .build();
 
@@ -75,9 +72,6 @@ void TestBuilderOptions() {
         assertEquals("etc/cert/ca.crt", flagdOptions.getCertPath());
         assertEquals("lru", flagdOptions.getCacheType());
         assertEquals(100, flagdOptions.getMaxCacheSize());
-        assertEquals("app=weatherApp", flagdOptions.getSelector());
-        assertEquals("some-path", flagdOptions.getOfflineFlagSourcePath());
-        assertEquals(openTelemetry, flagdOptions.getOpenTelemetry());
         assertEquals(connector, flagdOptions.getCustomConnector());
         assertEquals(Resolver.IN_PROCESS, flagdOptions.getResolverType());
         assertEquals("dns:///localhost:8016", flagdOptions.getTargetUri());

diff --git a/...ders/flagd/e2e/RunConfigCucumberTest.java → ...roviders/flagd/RunConfigCucumberTest.java b/...ders/flagd/e2e/RunConfigCucumberTest.java → ...roviders/flagd/RunConfigCucumberTest.java
@@ -1,4 +1,4 @@
-package dev.openfeature.contrib.providers.flagd.e2e;
+package dev.openfeature.contrib.providers.flagd;
 
 import static io.cucumber.junit.platform.engine.Constants.GLUE_PROPERTY_NAME;
 import static io.cucumber.junit.platform.engine.Constants.PLUGIN_PROPERTY_NAME;

diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFileTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFileTest.java
@@ -0,0 +1,39 @@
+package dev.openfeature.contrib.providers.flagd.e2e;
+
+import dev.openfeature.contrib.providers.flagd.Config;
+import org.apache.logging.log4j.core.config.Order;
+import org.junit.platform.suite.api.BeforeSuite;
+import org.junit.platform.suite.api.ConfigurationParameter;
+import org.junit.platform.suite.api.ExcludeTags;
+import org.junit.platform.suite.api.IncludeEngines;
+import org.junit.platform.suite.api.IncludeTags;
+import org.junit.platform.suite.api.SelectDirectories;
+import org.junit.platform.suite.api.Suite;
+import org.testcontainers.junit.jupiter.Testcontainers;
+
+import static io.cucumber.junit.platform.engine.Constants.GLUE_PROPERTY_NAME;
+import static io.cucumber.junit.platform.engine.Constants.OBJECT_FACTORY_PROPERTY_NAME;
+import static io.cucumber.junit.platform.engine.Constants.PLUGIN_PROPERTY_NAME;
+
+/**
+ * Class for running the reconnection tests for the RPC provider
+ */
+@Order(value = Integer.MAX_VALUE)
+@Suite
+@IncludeEngines("cucumber")
+@SelectDirectories("test-harness/gherkin")
+// if you want to run just one feature file, use the following line instead of @SelectDirectories
+// @SelectFile("test-harness/gherkin/connection.feature")
+@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty")
+@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps")
+@ConfigurationParameter(key = OBJECT_FACTORY_PROPERTY_NAME, value = "io.cucumber.picocontainer.PicoFactory")
+@IncludeTags("file")
+@ExcludeTags({"unixsocket", "targetURI", "reconnect", "customCert", "events"})
+@Testcontainers
+public class RunFileTest {
+
+    @BeforeSuite
+    public static void before() {
+        State.resolverType = Config.Resolver.FILE;
+    }
+}
diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/State.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/State.java
@@ -23,4 +23,5 @@ public class State {
     public FlagdOptions options;
     public FlagdOptions.FlagdOptionsBuilder builder = FlagdOptions.builder();
     public static Config.Resolver resolverType;
+    public boolean hasError;
 }
diff --git a/.../flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/AbstractSteps.java b/.../flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/AbstractSteps.java
@@ -2,10 +2,10 @@
 
 import dev.openfeature.contrib.providers.flagd.e2e.State;
 
-abstract class AbstractSteps {
-    State state;
+public abstract class AbstractSteps {
+    protected State state;
 
-    public AbstractSteps(State state) {
+    protected AbstractSteps(State state) {
         this.state = state;
     }
 }
diff --git a/...rs/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ConfigSteps.java b/...rs/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ConfigSteps.java
@@ -4,6 +4,9 @@
 
 import dev.openfeature.contrib.providers.flagd.Config;
 import dev.openfeature.contrib.providers.flagd.e2e.State;
+import dev.openfeature.contrib.providers.flagd.e2e.steps.AbstractSteps;
+import dev.openfeature.contrib.providers.flagd.e2e.steps.EnvironmentVariableUtils;
+import dev.openfeature.contrib.providers.flagd.e2e.steps.Utils;
 import io.cucumber.java.en.Given;
 import io.cucumber.java.en.Then;
 import io.cucumber.java.en.When;
@@ -34,7 +37,12 @@ public ConfigSteps(State state) {
 
     @When("a config was initialized")
     public void we_initialize_a_config() {
-        state.options = state.builder.build();
+        try {
+            state.options = state.builder.build();
+        } catch (IllegalArgumentException e) {
+            state.options = null;
+            state.hasError = true;
+        }
     }
 
     @When("a config was initialized for {string}")
@@ -87,19 +95,25 @@ public void the_option_of_type_should_have_the_value(String option, String type,
         }
 
         option = mapOptionNames(option);
-
-        assertThat(state.options).hasFieldOrPropertyWithValue(option, convert);
-
-        // Resetting env vars
-        for (Map.Entry<String, String> envVar : envVarsSet.entrySet()) {
-            if (envVar.getValue() == null) {
-                EnvironmentVariableUtils.clear(envVar.getKey());
-            } else {
-                EnvironmentVariableUtils.set(envVar.getKey(), envVar.getValue());
+        try {
+            assertThat(state.options).hasFieldOrPropertyWithValue(option, convert);
+        } finally {
+            // Resetting env vars
+            for (Map.Entry<String, String> envVar : envVarsSet.entrySet()) {
+                if (envVar.getValue() == null) {
+                    EnvironmentVariableUtils.clear(envVar.getKey());
+                } else {
+                    EnvironmentVariableUtils.set(envVar.getKey(), envVar.getValue());
+                }
             }
         }
     }
 
+    @Then("we should have an error")
+    public void we_should_have_an_error() {
+        assertThat(state.hasError).isTrue();
+    }
+
     private static String mapOptionNames(String option) {
         Map<String, String> propertyMapper = new HashMap<>();
         propertyMapper.put("resolver", "resolverType");

diff --git a/...test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EnvironmentVariableUtils.java b/...test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EnvironmentVariableUtils.java
@@ -14,7 +14,7 @@
  * This class modifies the internals of the environment variables map with reflection. Warning: If
  * your {@link SecurityManager} does not allow modifications, it fails.
  */
-class EnvironmentVariableUtils {
+public class EnvironmentVariableUtils {
 
     private EnvironmentVariableUtils() {
         // private constructor to prevent instantiation of utility class

diff --git a/.../flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java b/.../flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java
@@ -134,6 +134,8 @@ public int getPort(Config.Resolver resolver, ProviderType providerType) {
                     case SSL:
                         return toxiproxy.getMappedPort(8669);
                 }
+            case FILE:
+                return 0;
             default:
                 throw new IllegalArgumentException("Unsupported resolver: " + resolver);
         }
@@ -143,10 +145,24 @@ public int getPort(Config.Resolver resolver, ProviderType providerType) {
     public void setupProvider(String providerType) throws IOException {
         state.builder.deadline(500).keepAlive(0).retryGracePeriod(3);
         boolean wait = true;
+        File flags = new File("test-harness/flags");
+        ObjectMapper objectMapper = new ObjectMapper();
+        Object merged = new Object();
+        for (File listFile : Objects.requireNonNull(flags.listFiles())) {
+            ObjectReader updater = objectMapper.readerForUpdating(merged);
+            merged = updater.readValue(listFile, Object.class);
+        }
+        Path offlinePath = Files.createTempFile("flags", ".json");
+        objectMapper.writeValue(offlinePath.toFile(), merged);
         switch (providerType) {
             case "unavailable":
                 this.state.providerType = ProviderType.SOCKET;
                 state.builder.port(UNAVAILABLE_PORT);
+                if (State.resolverType == Config.Resolver.FILE) {
+
+                    state.builder
+                            .offlineFlagSourcePath("not-existing");
+                }
                 wait = false;
                 break;
             case "socket":
@@ -167,25 +183,17 @@ public void setupProvider(String providerType) throws IOException {
                         .tls(true)
                         .certPath(absolutePath);
                 break;
-            case "offline":
-                File flags = new File("test-harness/flags");
-                ObjectMapper objectMapper = new ObjectMapper();
-                Object merged = new Object();
-                for (File listFile : Objects.requireNonNull(flags.listFiles())) {
-                    ObjectReader updater = objectMapper.readerForUpdating(merged);
-                    merged = updater.readValue(listFile, Object.class);
-                }
-                Path offlinePath = Files.createTempFile("flags", ".json");
-                objectMapper.writeValue(offlinePath.toFile(), merged);
-
-                state.builder
-                        .port(UNAVAILABLE_PORT)
-                        .offlineFlagSourcePath(offlinePath.toAbsolutePath().toString());
-                break;
 
             default:
                 this.state.providerType = ProviderType.DEFAULT;
-                state.builder.port(getPort(State.resolverType, state.providerType));
+                if (State.resolverType == Config.Resolver.FILE) {
+
+                    state.builder
+                            .port(UNAVAILABLE_PORT)
+                            .offlineFlagSourcePath(offlinePath.toAbsolutePath().toString());
+                } else {
+                    state.builder.port(getPort(State.resolverType, state.providerType));
+                }
                 break;
         }
         FeatureProvider provider =

diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java
@@ -30,6 +30,8 @@ public static Object convert(String value, String type) throws ClassNotFoundExce
                         return Config.Resolver.IN_PROCESS;
                     case "rpc":
                         return Config.Resolver.RPC;
+                    case "file":
+                        return Config.Resolver.FILE;
                     default:
                         throw new RuntimeException("Unknown resolver type: " + value);
                 }

diff --git a/providers/flagd/test-harness b/providers/flagd/test-harness
+1 −22		specification.json
+0 −3		specification/README.md
+0 −60		specification/appendix-d-observability.md
+3 −104		specification/sections/04-hooks.md
+0 −2		specification/types.md