From 0b11534ac1d846fc948f40debfb3e1b0b08722bf Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 22 Jan 2025 14:22:02 +0100 Subject: [PATCH 1/4] feat(flagd): migrate file to own provider type Signed-off-by: Simon Schrottner --- providers/flagd/spec | 2 +- .../contrib/providers/flagd/Config.java | 8 ++++ .../contrib/providers/flagd/FlagdOptions.java | 19 +++++++-- .../providers/flagd/FlagdProvider.java | 1 + .../providers/flagd/FlagdOptionsTest.java | 2 - .../{e2e => }/RunConfigCucumberTest.java | 2 +- .../providers/flagd/e2e/RunFileTest.java | 39 +++++++++++++++++++ .../contrib/providers/flagd/e2e/State.java | 1 + .../flagd/e2e/steps/AbstractSteps.java | 6 +-- .../flagd/e2e/steps/ConfigSteps.java | 31 ++++++++++----- .../e2e/steps/EnvironmentVariableUtils.java | 2 +- .../flagd/e2e/steps/ProviderSteps.java | 39 +++++++++++-------- .../providers/flagd/e2e/steps/Utils.java | 2 + providers/flagd/test-harness | 2 +- 14 files changed, 118 insertions(+), 38 deletions(-) rename providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/{e2e => }/RunConfigCucumberTest.java (94%) create mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFileTest.java diff --git a/providers/flagd/spec b/providers/flagd/spec index 6c673d771..d261f6833 160000 --- a/providers/flagd/spec +++ b/providers/flagd/spec @@ -1 +1 @@ -Subproject commit 6c673d771618d86042adbcce70ace75f2b91fe76 +Subproject commit d261f68331b94fd8ed10bc72bc0485cfc72a51a8 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 index 027d1d75d..241db670f 100644 --- 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 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 index c98effac2..2b21e02a5 100644 --- 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,20 @@ 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 index bbf7674f1..1ed6297aa 100644 --- 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; 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 index c85765223..30c08dcd8 100644 --- 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 @@ -61,7 +61,6 @@ void TestBuilderOptions() { .cacheType("lru") .maxCacheSize(100) .selector("app=weatherApp") - .offlineFlagSourcePath("some-path") .openTelemetry(openTelemetry) .customConnector(connector) .resolverType(Resolver.IN_PROCESS) @@ -76,7 +75,6 @@ void TestBuilderOptions() { 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()); diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunConfigCucumberTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/RunConfigCucumberTest.java similarity index 94% rename from providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunConfigCucumberTest.java rename to providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/RunConfigCucumberTest.java index d5dce18fe..2d124b293 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunConfigCucumberTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/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 new file mode 100644 index 000000000..099e2f4c9 --- /dev/null +++ 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 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; + +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; + +/** + * 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 index 4ecab84e5..3d98efef1 100644 --- 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/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/AbstractSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/AbstractSteps.java index 133c1fb49..4a74400c4 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/AbstractSteps.java +++ b/providers/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/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ConfigSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ConfigSteps.java index 8e8ee44d6..a6e99f8b3 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ConfigSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ConfigSteps.java @@ -34,7 +34,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 +92,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 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 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 propertyMapper = new HashMap<>(); propertyMapper.put("resolver", "resolverType"); diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EnvironmentVariableUtils.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EnvironmentVariableUtils.java index b3ef4346e..f7427cfa2 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EnvironmentVariableUtils.java +++ b/providers/flagd/src/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/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java index cf0e5ed0c..e19f21120 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java +++ b/providers/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,23 @@ 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 +182,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 index 909d4800a..de0866d94 100644 --- 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 index fc7867922..b4066419f 160000 --- a/providers/flagd/test-harness +++ b/providers/flagd/test-harness @@ -1 +1 @@ -Subproject commit fc786792273b7984911dc3bcb7b47489f261ba57 +Subproject commit b4066419f7017a6aa10ae0b34ea63d33efec4f8f From 0dd03b1e915f042f329da911a7d0db45f6a9c428 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 22 Jan 2025 14:22:02 +0100 Subject: [PATCH 2/4] feat(flagd): migrate file to own provider type Signed-off-by: Simon Schrottner --- providers/flagd/test-harness | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/flagd/test-harness b/providers/flagd/test-harness index b4066419f..212f476aa 160000 --- a/providers/flagd/test-harness +++ b/providers/flagd/test-harness @@ -1 +1 @@ -Subproject commit b4066419f7017a6aa10ae0b34ea63d33efec4f8f +Subproject commit 212f476aa2afcee0f9e85b0d9e2caf2d2357f942 From 2137928f3b48b5a5b0d27e32443429ffc2483fa6 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 6 Feb 2025 17:11:45 +0100 Subject: [PATCH 3/4] improve test and coverage Signed-off-by: Simon Schrottner --- .../contrib/providers/flagd/FlagdOptions.java | 2 +- .../flagd/resolver/grpc/GrpcResolver.java | 4 ++-- .../providers/flagd/e2e/RunFileTest.java | 2 +- .../providers/flagd/e2e/RunRpcTest.java | 3 ++- .../flagd/e2e/steps/ProviderSteps.java | 22 +++++-------------- 5 files changed, 11 insertions(+), 22 deletions(-) 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 index 2b21e02a5..62abba4ad 100644 --- 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 @@ -193,7 +193,7 @@ void prebuild() { resolverType = fromValueProvider(System::getenv); } - if (StringUtils.isEmpty(offlineFlagSourcePath)) { + if (StringUtils.isBlank(offlineFlagSourcePath)) { offlineFlagSourcePath = fallBackToEnvOrDefault(Config.OFFLINE_SOURCE_PATH, null); } diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/GrpcResolver.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/GrpcResolver.java index 2a9669ec3..275744b74 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/GrpcResolver.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/GrpcResolver.java @@ -72,8 +72,8 @@ public GrpcResolver( Evaluation.EventStreamRequest.getDefaultInstance(), new EventStreamObserver( (flags) -> { - if (cache != null) { - flags.forEach(cache::remove); + if (this.cache != null) { + flags.forEach(this.cache::remove); } onProviderEvent.accept(new FlagdProviderEvent( ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, flags)); 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 index 099e2f4c9..7d499390c 100644 --- 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 @@ -28,7 +28,7 @@ @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"}) +@ExcludeTags({"unixsocket", "targetURI", "reconnect", "customCert"}) @Testcontainers public class RunFileTest { diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java index bc649ddeb..4ef7b9a87 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java @@ -12,6 +12,7 @@ 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.SelectFile; import org.junit.platform.suite.api.Suite; import org.testcontainers.junit.jupiter.Testcontainers; @@ -23,7 +24,7 @@ @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/rpc-caching.feature") +//@SelectFile("test-harness/gherkin/rpc-caching.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") diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java index ee57d6a99..16c5a371d 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java @@ -2,8 +2,6 @@ import static io.restassured.RestAssured.when; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.ObjectReader; import dev.openfeature.contrib.providers.flagd.Config; import dev.openfeature.contrib.providers.flagd.FlagdProvider; import dev.openfeature.contrib.providers.flagd.e2e.FlagdContainer; @@ -21,7 +19,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Objects; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.parallel.Isolated; @@ -47,7 +44,7 @@ public static void beforeAll() throws IOException { sharedTempDir = Files.createDirectories( Paths.get("tmp/" + RandomStringUtils.randomAlphanumeric(8).toLowerCase() + "/")); container = new FlagdContainer() - .withFileSystemBind(sharedTempDir.toAbsolutePath().toString(), "/tmp", BindMode.READ_WRITE); + .withFileSystemBind(sharedTempDir.toAbsolutePath().toString(), "/flags", BindMode.READ_WRITE); } @AfterAll @@ -78,15 +75,7 @@ public void setupProvider(String providerType) throws IOException, InterruptedEx String flagdConfig = "default"; state.builder.deadline(1000).keepAlive(0).retryGracePeriod(2); 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; @@ -123,7 +112,7 @@ public void setupProvider(String providerType) throws IOException, InterruptedEx state.builder .port(UNAVAILABLE_PORT) - .offlineFlagSourcePath(offlinePath.toAbsolutePath().toString()); + .offlineFlagSourcePath(sharedTempDir.resolve("allFlags.json").toAbsolutePath().toString()); } else { state.builder.port(container.getPort(State.resolverType)); } @@ -134,7 +123,7 @@ public void setupProvider(String providerType) throws IOException, InterruptedEx .statusCode(200); // giving flagd a little time to start - Thread.sleep(100); + Thread.sleep(30); FeatureProvider provider = new FlagdProvider(state.builder.resolverType(State.resolverType).build()); @@ -163,10 +152,9 @@ public void the_connection_is_lost_for(int seconds) throws InterruptedException @When("the flag was modified") public void the_flag_was_modded() throws InterruptedException { - when().post("http://" + container.getLaunchpadUrl() + "/change").then().statusCode(200); // we might be too fast in the execution - Thread.sleep(100); + Thread.sleep(1000); } } From 97f48ae84fd36887a6ca4bf2f956cb0b5e397cd2 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Fri, 7 Feb 2025 13:02:01 +0100 Subject: [PATCH 4/4] fixup: add readme and missing option Signed-off-by: Simon Schrottner --- providers/flagd/.gitignore | 1 + providers/flagd/README.md | 41 ++++++++++--------- providers/flagd/schemas | 2 +- providers/flagd/spec | 2 +- .../contrib/providers/flagd/Config.java | 2 + .../contrib/providers/flagd/FlagdOptions.java | 7 ++++ .../resolver/process/InProcessResolver.java | 2 +- .../storage/connector/file/FileConnector.java | 7 ++-- .../providers/flagd/e2e/RunRpcTest.java | 3 +- .../flagd/e2e/steps/ProviderSteps.java | 5 ++- .../flagd/e2e/steps/config/ConfigSteps.java | 1 - .../connector/file/FileConnectorTest.java | 6 +-- 12 files changed, 46 insertions(+), 33 deletions(-) create mode 100644 providers/flagd/.gitignore diff --git a/providers/flagd/.gitignore b/providers/flagd/.gitignore new file mode 100644 index 000000000..3fec32c84 --- /dev/null +++ b/providers/flagd/.gitignore @@ -0,0 +1 @@ +tmp/ diff --git a/providers/flagd/README.md b/providers/flagd/README.md index cddd144bd..d57aac682 100644 --- a/providers/flagd/README.md +++ b/providers/flagd/README.md @@ -54,7 +54,7 @@ The value is updated with every (re)connection to the sync implementation. This can be used to enrich evaluations with such data. If the `in-process` mode is not used, and before the provider is ready, the `getSyncMetadata` returns an empty map. -#### Offline mode +### Offline mode (File resolver) In-process resolvers can also work in an offline mode. To enable this mode, you should provide a valid flag configuration file with the option `offlineFlagSourcePath`. @@ -62,7 +62,7 @@ To enable this mode, you should provide a valid flag configuration file with the ```java FlagdProvider flagdProvider = new FlagdProvider( FlagdOptions.builder() - .resolverType(Config.Resolver.IN_PROCESS) + .resolverType(Config.Resolver.FILE) .offlineFlagSourcePath("PATH") .build()); ``` @@ -103,24 +103,25 @@ variables. Given below are the supported configurations: -| Option name | Environment variable name | Type & Values | Default | Compatible resolver | -| --------------------- | ------------------------------ | ------------------------ | --------- | ------------------- | -| resolver | FLAGD_RESOLVER | String - rpc, in-process | rpc | | -| host | FLAGD_HOST | String | localhost | rpc & in-process | -| port | FLAGD_PORT | int | 8013 | rpc & in-process | -| targetUri | FLAGD_TARGET_URI | string | null | rpc & in-process | -| tls | FLAGD_TLS | boolean | false | rpc & in-process | -| socketPath | FLAGD_SOCKET_PATH | String | null | rpc & in-process | -| certPath | FLAGD_SERVER_CERT_PATH | String | null | rpc & in-process | -| deadline | FLAGD_DEADLINE_MS | int | 500 | rpc & in-process | -| streamDeadlineMs | FLAGD_STREAM_DEADLINE_MS | int | 600000 | rpc & in-process | -| keepAliveTime | FLAGD_KEEP_ALIVE_TIME_MS | long | 0 | rpc & in-process | -| selector | FLAGD_SOURCE_SELECTOR | String | null | in-process | -| cache | FLAGD_CACHE | String - lru, disabled | lru | rpc | -| maxCacheSize | FLAGD_MAX_CACHE_SIZE | int | 1000 | rpc | -| maxEventStreamRetries | FLAGD_MAX_EVENT_STREAM_RETRIES | int | 5 | rpc | -| retryBackoffMs | FLAGD_RETRY_BACKOFF_MS | int | 1000 | rpc | -| offlineFlagSourcePath | FLAGD_OFFLINE_FLAG_SOURCE_PATH | String | null | in-process | +| Option name | Environment variable name | Type & Values | Default | Compatible resolver | +|-----------------------|--------------------------------|--------------------------|-----------|-------------------------| +| resolver | FLAGD_RESOLVER | String - rpc, in-process | rpc | | +| host | FLAGD_HOST | String | localhost | rpc & in-process | +| port | FLAGD_PORT | int | 8013 | rpc & in-process | +| targetUri | FLAGD_TARGET_URI | string | null | rpc & in-process | +| tls | FLAGD_TLS | boolean | false | rpc & in-process | +| socketPath | FLAGD_SOCKET_PATH | String | null | rpc & in-process | +| certPath | FLAGD_SERVER_CERT_PATH | String | null | rpc & in-process | +| deadline | FLAGD_DEADLINE_MS | int | 500 | rpc & in-process & file | +| streamDeadlineMs | FLAGD_STREAM_DEADLINE_MS | int | 600000 | rpc & in-process | +| keepAliveTime | FLAGD_KEEP_ALIVE_TIME_MS | long | 0 | rpc & in-process | +| selector | FLAGD_SOURCE_SELECTOR | String | null | in-process | +| cache | FLAGD_CACHE | String - lru, disabled | lru | rpc | +| maxCacheSize | FLAGD_MAX_CACHE_SIZE | int | 1000 | rpc | +| maxEventStreamRetries | FLAGD_MAX_EVENT_STREAM_RETRIES | int | 5 | rpc | +| retryBackoffMs | FLAGD_RETRY_BACKOFF_MS | int | 1000 | rpc | +| offlineFlagSourcePath | FLAGD_OFFLINE_FLAG_SOURCE_PATH | String | null | file | +| offlinePollIntervalMs | FLAGD_OFFLINE_POLL_MS | int | 5000 | file | > [!NOTE] > Some configurations are only applicable for RPC resolver. diff --git a/providers/flagd/schemas b/providers/flagd/schemas index bb763438a..37baa2cde 160000 --- a/providers/flagd/schemas +++ b/providers/flagd/schemas @@ -1 +1 @@ -Subproject commit bb763438abc5b0e97dc26a795bc72b7a9f3e4020 +Subproject commit 37baa2cdea48a5ac614ba3e718b7d02ad4120611 diff --git a/providers/flagd/spec b/providers/flagd/spec index 8d6eeb324..5b0706598 160000 --- a/providers/flagd/spec +++ b/providers/flagd/spec @@ -1 +1 @@ -Subproject commit 8d6eeb3247600f6f66ffc92afa50ebde75b4d3ce +Subproject commit 5b070659853062262e618c74da5b640555f9ae18 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 index 241db670f..8c6da726c 100644 --- 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 @@ -17,6 +17,7 @@ public final class Config { static final int DEFAULT_STREAM_DEADLINE_MS = 10 * 60 * 1000; static final int DEFAULT_STREAM_RETRY_GRACE_PERIOD = 5; static final int DEFAULT_MAX_CACHE_SIZE = 1000; + static final int DEFAULT_OFFLINE_POLL_MS = 5000; static final long DEFAULT_KEEP_ALIVE = 0; static final String RESOLVER_ENV_VAR = "FLAGD_RESOLVER"; @@ -33,6 +34,7 @@ public final class Config { static final String STREAM_DEADLINE_MS_ENV_VAR_NAME = "FLAGD_STREAM_DEADLINE_MS"; static final String SOURCE_SELECTOR_ENV_VAR_NAME = "FLAGD_SOURCE_SELECTOR"; static final String OFFLINE_SOURCE_PATH = "FLAGD_OFFLINE_FLAG_SOURCE_PATH"; + static final String OFFLINE_POLL_MS = "FLAGD_OFFLINE_POLL_MS"; static final String KEEP_ALIVE_MS_ENV_VAR_NAME_OLD = "FLAGD_KEEP_ALIVE_TIME"; static final String KEEP_ALIVE_MS_ENV_VAR_NAME = "FLAGD_KEEP_ALIVE_TIME_MS"; static final String TARGET_URI_ENV_VAR_NAME = "FLAGD_TARGET_URI"; 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 index 62abba4ad..3d75d1063 100644 --- 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 @@ -122,6 +122,13 @@ public class FlagdOptions { */ private String offlineFlagSourcePath; + /** + * File polling interval. + * Defaults to 0 (disabled). + **/ + @Builder.Default + private int offlinePollIntervalMs = fallBackToEnvOrDefault(Config.OFFLINE_POLL_MS, Config.DEFAULT_OFFLINE_POLL_MS); + /** * gRPC custom target string. * diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolver.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolver.java index b3385781e..e28597d90 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolver.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolver.java @@ -151,7 +151,7 @@ static Connector getConnector(final FlagdOptions options, Consumer queue = new LinkedBlockingQueue<>(1); private boolean shutdown = false; - public FileConnector(final String flagSourcePath) { + public FileConnector(final String flagSourcePath, int pollInterval) { this.flagSourcePath = flagSourcePath; + this.pollInterval = pollInterval; } /** @@ -64,7 +65,7 @@ public void init() throws IOException { } } - Thread.sleep(POLL_INTERVAL_MS); + Thread.sleep(pollInterval); } log.info("Shutting down file connector."); diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java index 4ef7b9a87..bc649ddeb 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java @@ -12,7 +12,6 @@ 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.SelectFile; import org.junit.platform.suite.api.Suite; import org.testcontainers.junit.jupiter.Testcontainers; @@ -24,7 +23,7 @@ @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/rpc-caching.feature") +// @SelectFile("test-harness/gherkin/rpc-caching.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") diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java index 16c5a371d..ce0fc4234 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java @@ -112,7 +112,10 @@ public void setupProvider(String providerType) throws IOException, InterruptedEx state.builder .port(UNAVAILABLE_PORT) - .offlineFlagSourcePath(sharedTempDir.resolve("allFlags.json").toAbsolutePath().toString()); + .offlineFlagSourcePath(sharedTempDir + .resolve("allFlags.json") + .toAbsolutePath() + .toString()); } else { state.builder.port(container.getPort(State.resolverType)); } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/config/ConfigSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/config/ConfigSteps.java index c4f04a1d8..c33960f31 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/config/ConfigSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/config/ConfigSteps.java @@ -26,7 +26,6 @@ public class ConfigSteps extends AbstractSteps { */ public static final List IGNORED_FOR_NOW = new ArrayList() { { - add("offlinePollIntervalMs"); add("retryBackoffMaxMs"); } }; diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/file/FileConnectorTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/file/FileConnectorTest.java index 50759c319..c4c766475 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/file/FileConnectorTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/file/FileConnectorTest.java @@ -24,7 +24,7 @@ class FileConnectorTest { @Test void readAndExposeFeatureFlagsFromSource() throws IOException { // given - final FileConnector connector = new FileConnector(getResourcePath(VALID_LONG)); + final FileConnector connector = new FileConnector(getResourcePath(VALID_LONG), 5000); // when connector.init(); @@ -45,7 +45,7 @@ void readAndExposeFeatureFlagsFromSource() throws IOException { @Test void emitErrorStateForInvalidPath() throws IOException { // given - final FileConnector connector = new FileConnector("INVALID_PATH"); + final FileConnector connector = new FileConnector("INVALID_PATH", 5000); // when connector.init(); @@ -75,7 +75,7 @@ void watchForFileUpdatesAndEmitThem() throws IOException { final Path updPath = Paths.get(getResourcePath(UPDATABLE_FILE)); Files.write(updPath, initial.getBytes(), StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING); - final FileConnector connector = new FileConnector(updPath.toString()); + final FileConnector connector = new FileConnector(updPath.toString(), 5000); // when connector.init();