From 4a37f8ea2d149091683082ff5146eab2381600df Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Sat, 28 Dec 2024 17:27:19 +0100 Subject: [PATCH 01/13] test(flagd): rework e2e tests to new format Signed-off-by: Simon Schrottner --- pom.xml | 7 + .../providers/flagd/e2e/ContainerConfig.java | 100 ---- .../providers/flagd/e2e/FlagdContainer.java | 89 +++ .../e2e/RunFlagdInProcessCucumberTest.java | 29 - .../RunFlagdInProcessEnvoyCucumberTest.java | 29 - ...unFlagdInProcessReconnectCucumberTest.java | 24 - .../e2e/RunFlagdInProcessSSLCucumberTest.java | 23 - .../flagd/e2e/RunFlagdRpcCucumberTest.java | 29 - .../e2e/RunFlagdRpcReconnectCucumberTest.java | 24 - .../flagd/e2e/RunFlagdRpcSSLCucumberTest.java | 23 - .../providers/flagd/e2e/RunInProcessTest.java | 39 ++ .../providers/flagd/e2e/RunRpcTest.java | 39 ++ .../contrib/providers/flagd/e2e/State.java | 29 + .../e2e/process/core/FlagdInProcessSetup.java | 44 -- .../envoy/FlagdInProcessEnvoySetup.java | 45 -- .../process/FlagdInProcessSetup.java | 49 -- .../e2e/reconnect/rpc/FlagdRpcSetup.java | 53 -- .../e2e/reconnect/steps/StepDefinitions.java | 121 ---- .../flagd/e2e/rpc/FlagdRpcSetup.java | 42 -- .../e2e/ssl/process/FlagdInProcessSetup.java | 51 -- .../flagd/e2e/ssl/rpc/FlagdRpcSetup.java | 51 -- .../flagd/e2e/steps/AbstractSteps.java | 10 + .../e2e/steps/{config => }/ConfigSteps.java | 56 +- .../flagd/e2e/steps/ContextSteps.java | 43 ++ .../EnvironmentVariableUtils.java | 2 +- .../providers/flagd/e2e/steps/Event.java | 13 + .../providers/flagd/e2e/steps/EventSteps.java | 69 +++ .../providers/flagd/e2e/steps/FlagSteps.java | 86 +++ .../flagd/e2e/steps/ProviderSteps.java | 159 +++++ .../flagd/e2e/steps/ProviderType.java | 7 + .../flagd/e2e/steps/StepDefinitions.java | 555 ------------------ .../providers/flagd/e2e/steps/Utils.java | 39 ++ providers/flagd/test-harness | 2 +- 33 files changed, 648 insertions(+), 1333 deletions(-) delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ContainerConfig.java create mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessCucumberTest.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessEnvoyCucumberTest.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessReconnectCucumberTest.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessSSLCucumberTest.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdRpcCucumberTest.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdRpcReconnectCucumberTest.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdRpcSSLCucumberTest.java create mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java create mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java create mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/State.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/process/core/FlagdInProcessSetup.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/process/envoy/FlagdInProcessEnvoySetup.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/reconnect/process/FlagdInProcessSetup.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/reconnect/rpc/FlagdRpcSetup.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/reconnect/steps/StepDefinitions.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/rpc/FlagdRpcSetup.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ssl/process/FlagdInProcessSetup.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ssl/rpc/FlagdRpcSetup.java create mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/AbstractSteps.java rename providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/{config => }/ConfigSteps.java (66%) create mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ContextSteps.java rename providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/{config => }/EnvironmentVariableUtils.java (98%) create mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Event.java create mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java create mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java create mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java create mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderType.java delete mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/StepDefinitions.java create mode 100644 providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java diff --git a/pom.xml b/pom.xml index cafc1767c..8f161285c 100644 --- a/pom.xml +++ b/pom.xml @@ -200,6 +200,13 @@ test + + io.cucumber + cucumber-picocontainer + 7.20.1 + test + + org.awaitility awaitility diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ContainerConfig.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ContainerConfig.java deleted file mode 100644 index e5231b1bc..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ContainerConfig.java +++ /dev/null @@ -1,100 +0,0 @@ -package dev.openfeature.contrib.providers.flagd.e2e; - -import java.io.File; -import java.nio.file.Files; -import java.util.List; -import org.apache.logging.log4j.util.Strings; -import org.jetbrains.annotations.NotNull; -import org.testcontainers.containers.GenericContainer; -import org.testcontainers.containers.Network; -import org.testcontainers.utility.DockerImageName; -import org.testcontainers.utility.MountableFile; - -public class ContainerConfig { - private static final String version; - private static final Network network = Network.newNetwork(); - - static { - String path = "test-harness/version.txt"; - File file = new File(path); - try { - List lines = Files.readAllLines(file.toPath()); - version = lines.get(0); - } catch (Exception e) { - throw new RuntimeException(e); - } - } - - /** - * @return a {@link org.testcontainers.containers.GenericContainer} instance of a stable sync - * flagd server with the port 9090 exposed - */ - public static GenericContainer sync() { - return sync(false, false); - } - - /** - * @param unstable if an unstable version of the container, which terminates the connection - * regularly should be used. - * @param addNetwork if set to true a custom network is attached for cross container access e.g. - * envoy --> sync:8015 - * @return a {@link org.testcontainers.containers.GenericContainer} instance of a sync flagd - * server with the port 8015 exposed - */ - public static GenericContainer sync(boolean unstable, boolean addNetwork) { - String container = generateContainerName("flagd", unstable ? "unstable" : ""); - GenericContainer genericContainer = - new GenericContainer(DockerImageName.parse(container)).withExposedPorts(8015); - - if (addNetwork) { - genericContainer.withNetwork(network); - genericContainer.withNetworkAliases("sync-service"); - } - - return genericContainer; - } - - /** - * @return a {@link org.testcontainers.containers.GenericContainer} instance of a stable flagd - * server with the port 8013 exposed - */ - public static GenericContainer flagd() { - return flagd(false); - } - - /** - * @param unstable if an unstable version of the container, which terminates the connection - * regularly should be used. - * @return a {@link org.testcontainers.containers.GenericContainer} instance of a flagd server - * with the port 8013 exposed - */ - public static GenericContainer flagd(boolean unstable) { - String container = generateContainerName("flagd", unstable ? "unstable" : ""); - return new GenericContainer(DockerImageName.parse(container)).withExposedPorts(8013); - } - - /** - * @return a {@link org.testcontainers.containers.GenericContainer} instance of envoy container - * using flagd sync service as backend expose on port 9211 - */ - public static GenericContainer envoy() { - final String container = "envoyproxy/envoy:v1.31.0"; - return new GenericContainer(DockerImageName.parse(container)) - .withCopyFileToContainer( - MountableFile.forClasspathResource("/envoy-config/envoy-custom.yaml"), "/etc/envoy/envoy.yaml") - .withExposedPorts(9211) - .withNetwork(network) - .withNetworkAliases("envoy"); - } - - public static @NotNull String generateContainerName(String type, String addition) { - String container = "ghcr.io/open-feature/"; - container += type; - container += "-testbed"; - if (!Strings.isBlank(addition)) { - container += "-" + addition; - } - container += ":v" + version; - return container; - } -} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java new file mode 100644 index 000000000..53bd91160 --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java @@ -0,0 +1,89 @@ +package dev.openfeature.contrib.providers.flagd.e2e; + +import dev.openfeature.contrib.providers.flagd.Config; +import org.apache.logging.log4j.util.Strings; +import org.jetbrains.annotations.NotNull; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.Network; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.containers.wait.strategy.WaitStrategy; +import org.testcontainers.utility.DockerImageName; +import org.testcontainers.utility.MountableFile; + +import java.io.File; +import java.nio.file.Files; +import java.util.List; + +public class FlagdContainer extends GenericContainer { + private static final String version; + private static final Network network = Network.newNetwork(); + + static { + String path = "test-harness/version.txt"; + File file = new File(path); + try { + List lines = Files.readAllLines(file.toPath()); + version = lines.get(0); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + + private String feature; + + public FlagdContainer() { + this(""); + } + + public FlagdContainer(String feature) { + super(generateContainerName(feature)); + this.withReuse(true); + this.feature = feature; + if (!"socket".equals(this.feature)) + this.addExposedPorts(8013, 8014, 8015, 8016); + } + + @Override + public void start() { + if (!"socket".equals(this.feature)) + this.addExposedPorts(8013, 8014, 8015, 8016); + super.start(); + waitUntilContainerStarted(); + } + + public int getPort(Config.Resolver resolver) { + waitUntilContainerStarted(); + switch (resolver) { + case RPC: + return this.getMappedPort(8013); + case IN_PROCESS: + return this.getMappedPort(8015); + default: + throw new IllegalArgumentException("Unsupported resolver: " + resolver); + } + } + + + /** + * @return a {@link org.testcontainers.containers.GenericContainer} instance of envoy container using + * flagd sync service as backend expose on port 9211 + */ + public static GenericContainer envoy() { + final String container = "envoyproxy/envoy:v1.31.0"; + return new GenericContainer(DockerImageName.parse(container)) + .withCopyFileToContainer(MountableFile.forClasspathResource("/envoy-config/envoy-custom.yaml"), + "/etc/envoy/envoy.yaml") + .withExposedPorts(9211) + .withNetwork(network) + .withNetworkAliases("envoy"); + } + + public static @NotNull String generateContainerName(String feature) { + String container = "ghcr.io/open-feature/flagd-testbed"; + if (!Strings.isBlank(feature)) { + container += "-" + feature; + } + container += ":v" + version; + return container; + } +} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessCucumberTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessCucumberTest.java deleted file mode 100644 index 39ff92993..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessCucumberTest.java +++ /dev/null @@ -1,29 +0,0 @@ -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.PLUGIN_PROPERTY_NAME; - -import org.junit.jupiter.api.Order; -import org.junit.platform.suite.api.ConfigurationParameter; -import org.junit.platform.suite.api.IncludeEngines; -import org.junit.platform.suite.api.SelectFile; -import org.junit.platform.suite.api.Suite; -import org.testcontainers.junit.jupiter.Testcontainers; - -/** - * Class for running the tests associated with "stable" e2e tests (no fake disconnection) for the - * in-process provider - */ -@Order(value = Integer.MAX_VALUE) -@Suite -@IncludeEngines("cucumber") -@SelectFile("spec/specification/assets/gherkin/evaluation.feature") -@SelectFile("test-harness/gherkin/flagd-json-evaluator.feature") -@SelectFile("test-harness/gherkin/flagd.feature") -@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty") -@ConfigurationParameter( - key = GLUE_PROPERTY_NAME, - value = - "dev.openfeature.contrib.providers.flagd.e2e.process.core,dev.openfeature.contrib.providers.flagd.e2e.steps") -@Testcontainers -public class RunFlagdInProcessCucumberTest {} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessEnvoyCucumberTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessEnvoyCucumberTest.java deleted file mode 100644 index a2bb60793..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessEnvoyCucumberTest.java +++ /dev/null @@ -1,29 +0,0 @@ -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.PLUGIN_PROPERTY_NAME; - -import org.junit.jupiter.api.Order; -import org.junit.platform.suite.api.ConfigurationParameter; -import org.junit.platform.suite.api.IncludeEngines; -import org.junit.platform.suite.api.SelectFile; -import org.junit.platform.suite.api.Suite; -import org.testcontainers.junit.jupiter.Testcontainers; - -/** - * Class for running the tests associated with "stable" e2e tests (no fake disconnection) for the - * in-process provider - */ -@Order(value = Integer.MAX_VALUE) -@Suite -@IncludeEngines("cucumber") -@SelectFile("spec/specification/assets/gherkin/evaluation.feature") -@SelectFile("test-harness/gherkin/flagd-json-evaluator.feature") -@SelectFile("test-harness/gherkin/flagd.feature") -@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty") -@ConfigurationParameter( - key = GLUE_PROPERTY_NAME, - value = - "dev.openfeature.contrib.providers.flagd.e2e.process.envoy,dev.openfeature.contrib.providers.flagd.e2e.steps") -@Testcontainers -public class RunFlagdInProcessEnvoyCucumberTest {} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessReconnectCucumberTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessReconnectCucumberTest.java deleted file mode 100644 index 346d9b5a4..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessReconnectCucumberTest.java +++ /dev/null @@ -1,24 +0,0 @@ -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.PLUGIN_PROPERTY_NAME; - -import org.junit.jupiter.api.Order; -import org.junit.platform.suite.api.ConfigurationParameter; -import org.junit.platform.suite.api.IncludeEngines; -import org.junit.platform.suite.api.SelectFile; -import org.junit.platform.suite.api.Suite; -import org.testcontainers.junit.jupiter.Testcontainers; - -/** Class for running the reconnection tests for the in-process provider */ -@Order(value = Integer.MAX_VALUE) -@Suite -@IncludeEngines("cucumber") -@SelectFile("test-harness/gherkin/flagd-reconnect.feature") -@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty") -@ConfigurationParameter( - key = GLUE_PROPERTY_NAME, - value = - "dev.openfeature.contrib.providers.flagd.e2e.reconnect.process,dev.openfeature.contrib.providers.flagd.e2e.reconnect.steps") -@Testcontainers -public class RunFlagdInProcessReconnectCucumberTest {} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessSSLCucumberTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessSSLCucumberTest.java deleted file mode 100644 index ae7ca415b..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdInProcessSSLCucumberTest.java +++ /dev/null @@ -1,23 +0,0 @@ -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.PLUGIN_PROPERTY_NAME; - -import org.apache.logging.log4j.core.config.Order; -import org.junit.platform.suite.api.ConfigurationParameter; -import org.junit.platform.suite.api.IncludeEngines; -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(failIfNoTests = false) -@IncludeEngines("cucumber") -// @SelectFile("spec/specification/assets/gherkin/evaluation.feature") -@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty") -@ConfigurationParameter( - key = GLUE_PROPERTY_NAME, - value = - "dev.openfeature.contrib.providers.flagd.e2e.ssl.process,dev.openfeature.contrib.providers.flagd.e2e.steps") -@Testcontainers -public class RunFlagdInProcessSSLCucumberTest {} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdRpcCucumberTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdRpcCucumberTest.java deleted file mode 100644 index e0d872b9f..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdRpcCucumberTest.java +++ /dev/null @@ -1,29 +0,0 @@ -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.PLUGIN_PROPERTY_NAME; - -import org.apache.logging.log4j.core.config.Order; -import org.junit.platform.suite.api.ConfigurationParameter; -import org.junit.platform.suite.api.IncludeEngines; -import org.junit.platform.suite.api.SelectFile; -import org.junit.platform.suite.api.Suite; -import org.testcontainers.junit.jupiter.Testcontainers; - -/** - * Class for running the tests associated with "stable" e2e tests (no fake disconnection) for the - * RPC provider - */ -@Order(value = Integer.MAX_VALUE) -@Suite -@IncludeEngines("cucumber") -@SelectFile("spec/specification/assets/gherkin/evaluation.feature") -@SelectFile("test-harness/gherkin/flagd-json-evaluator.feature") -@SelectFile("test-harness/gherkin/flagd.feature") -@SelectFile("test-harness/gherkin/flagd-rpc-caching.feature") -@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty") -@ConfigurationParameter( - key = GLUE_PROPERTY_NAME, - value = "dev.openfeature.contrib.providers.flagd.e2e.rpc,dev.openfeature.contrib.providers.flagd.e2e.steps") -@Testcontainers -public class RunFlagdRpcCucumberTest {} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdRpcReconnectCucumberTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdRpcReconnectCucumberTest.java deleted file mode 100644 index 436ebf00f..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdRpcReconnectCucumberTest.java +++ /dev/null @@ -1,24 +0,0 @@ -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.PLUGIN_PROPERTY_NAME; - -import org.apache.logging.log4j.core.config.Order; -import org.junit.platform.suite.api.ConfigurationParameter; -import org.junit.platform.suite.api.IncludeEngines; -import org.junit.platform.suite.api.SelectFile; -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") -@SelectFile("test-harness/gherkin/flagd-reconnect.feature") -@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty") -@ConfigurationParameter( - key = GLUE_PROPERTY_NAME, - value = - "dev.openfeature.contrib.providers.flagd.e2e.reconnect.rpc,dev.openfeature.contrib.providers.flagd.e2e.reconnect.steps") -@Testcontainers -public class RunFlagdRpcReconnectCucumberTest {} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdRpcSSLCucumberTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdRpcSSLCucumberTest.java deleted file mode 100644 index f09846d41..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunFlagdRpcSSLCucumberTest.java +++ /dev/null @@ -1,23 +0,0 @@ -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.PLUGIN_PROPERTY_NAME; - -import org.apache.logging.log4j.core.config.Order; -import org.junit.platform.suite.api.ConfigurationParameter; -import org.junit.platform.suite.api.IncludeEngines; -import org.junit.platform.suite.api.SelectFile; -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") -@SelectFile("spec/specification/assets/gherkin/evaluation.feature") -@ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty") -@ConfigurationParameter( - key = GLUE_PROPERTY_NAME, - value = "dev.openfeature.contrib.providers.flagd.e2e.ssl.rpc,dev.openfeature.contrib.providers.flagd.e2e.steps") -@Testcontainers -public class RunFlagdRpcSSLCucumberTest {} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java new file mode 100644 index 000000000..12c4a66d4 --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java @@ -0,0 +1,39 @@ +package dev.openfeature.contrib.providers.flagd.e2e; + +import dev.openfeature.contrib.providers.flagd.Config; +import io.cucumber.junit.platform.engine.Constants; +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") +@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("in-process") +@ExcludeTags({"unixsocket", "customCert"}) +@Testcontainers +public class RunInProcessTest { + + @BeforeSuite + public static void before() { + State.resolverType = Config.Resolver.IN_PROCESS; + } +} + 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 new file mode 100644 index 000000000..20dc320bc --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunRpcTest.java @@ -0,0 +1,39 @@ +package dev.openfeature.contrib.providers.flagd.e2e; + +import dev.openfeature.contrib.providers.flagd.Config; +import io.cucumber.junit.platform.engine.Constants; +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") +@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("rpc") +@Testcontainers +public class RunRpcTest { + + @BeforeSuite + public static void before() { + State.resolverType = Config.Resolver.RPC; + } + +} + 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 new file mode 100644 index 000000000..99126a583 --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/State.java @@ -0,0 +1,29 @@ +package dev.openfeature.contrib.providers.flagd.e2e; + +import dev.openfeature.contrib.providers.flagd.Config; +import dev.openfeature.contrib.providers.flagd.FlagdOptions; +import dev.openfeature.contrib.providers.flagd.e2e.steps.Event; +import dev.openfeature.contrib.providers.flagd.e2e.steps.FlagSteps; +import dev.openfeature.contrib.providers.flagd.e2e.steps.ProviderType; +import dev.openfeature.sdk.Client; +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.MutableContext; + +import java.util.ArrayList; +import java.util.Optional; + +public class State { + public ProviderType providerType; + public Client client; + public ArrayList events = new ArrayList<>(); + public Optional lastEvent; + public FlagSteps.Flag flag; + public MutableContext context + = new MutableContext(); + public FlagEvaluationDetails evaluation; + public FlagdOptions options; + public FlagdOptions.FlagdOptionsBuilder builder = FlagdOptions.builder(); + public static Config.Resolver resolverType; + + +} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/process/core/FlagdInProcessSetup.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/process/core/FlagdInProcessSetup.java deleted file mode 100644 index e23a95965..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/process/core/FlagdInProcessSetup.java +++ /dev/null @@ -1,44 +0,0 @@ -package dev.openfeature.contrib.providers.flagd.e2e.process.core; - -import dev.openfeature.contrib.providers.flagd.Config; -import dev.openfeature.contrib.providers.flagd.FlagdOptions; -import dev.openfeature.contrib.providers.flagd.FlagdProvider; -import dev.openfeature.contrib.providers.flagd.e2e.ContainerConfig; -import dev.openfeature.contrib.providers.flagd.e2e.steps.StepDefinitions; -import dev.openfeature.sdk.FeatureProvider; -import io.cucumber.java.AfterAll; -import io.cucumber.java.Before; -import io.cucumber.java.BeforeAll; -import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.parallel.Isolated; -import org.testcontainers.containers.GenericContainer; - -@Isolated() -@Order(value = Integer.MAX_VALUE) -public class FlagdInProcessSetup { - - private static FeatureProvider provider; - - private static final GenericContainer flagdContainer = ContainerConfig.sync(); - - @BeforeAll() - public static void setup() throws InterruptedException { - flagdContainer.start(); - } - - @Before() - public static void setupTest() throws InterruptedException { - FlagdInProcessSetup.provider = new FlagdProvider(FlagdOptions.builder() - .resolverType(Config.Resolver.IN_PROCESS) - .deadline(1000) - .streamDeadlineMs(0) // this makes reconnect tests more predictable - .port(flagdContainer.getFirstMappedPort()) - .build()); - StepDefinitions.setProvider(provider); - } - - @AfterAll - public static void tearDown() { - flagdContainer.stop(); - } -} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/process/envoy/FlagdInProcessEnvoySetup.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/process/envoy/FlagdInProcessEnvoySetup.java deleted file mode 100644 index 37381a682..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/process/envoy/FlagdInProcessEnvoySetup.java +++ /dev/null @@ -1,45 +0,0 @@ -package dev.openfeature.contrib.providers.flagd.e2e.process.envoy; - -import dev.openfeature.contrib.providers.flagd.Config; -import dev.openfeature.contrib.providers.flagd.FlagdOptions; -import dev.openfeature.contrib.providers.flagd.FlagdProvider; -import dev.openfeature.contrib.providers.flagd.e2e.ContainerConfig; -import dev.openfeature.contrib.providers.flagd.e2e.steps.StepDefinitions; -import dev.openfeature.sdk.FeatureProvider; -import io.cucumber.java.AfterAll; -import io.cucumber.java.BeforeAll; -import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.parallel.Isolated; -import org.testcontainers.containers.GenericContainer; - -@Isolated() -@Order(value = Integer.MAX_VALUE) -public class FlagdInProcessEnvoySetup { - - private static FeatureProvider provider; - - private static final GenericContainer flagdContainer = ContainerConfig.sync(false, true); - private static final GenericContainer envoyContainer = ContainerConfig.envoy(); - - @BeforeAll() - public static void setup() throws InterruptedException { - flagdContainer.start(); - envoyContainer.start(); - final String targetUri = - String.format("envoy://localhost:%s/flagd-sync.service", envoyContainer.getFirstMappedPort()); - - FlagdInProcessEnvoySetup.provider = new FlagdProvider(FlagdOptions.builder() - .resolverType(Config.Resolver.IN_PROCESS) - .deadline(1000) - .streamDeadlineMs(0) // this makes reconnect tests more predictabl - .targetUri(targetUri) - .build()); - StepDefinitions.setProvider(provider); - } - - @AfterAll - public static void tearDown() { - flagdContainer.stop(); - envoyContainer.stop(); - } -} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/reconnect/process/FlagdInProcessSetup.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/reconnect/process/FlagdInProcessSetup.java deleted file mode 100644 index 4ab76dd40..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/reconnect/process/FlagdInProcessSetup.java +++ /dev/null @@ -1,49 +0,0 @@ -package dev.openfeature.contrib.providers.flagd.e2e.reconnect.process; - -import dev.openfeature.contrib.providers.flagd.Config; -import dev.openfeature.contrib.providers.flagd.FlagdOptions; -import dev.openfeature.contrib.providers.flagd.FlagdProvider; -import dev.openfeature.contrib.providers.flagd.e2e.ContainerConfig; -import dev.openfeature.contrib.providers.flagd.e2e.reconnect.steps.StepDefinitions; -import dev.openfeature.sdk.FeatureProvider; -import io.cucumber.java.AfterAll; -import io.cucumber.java.Before; -import io.cucumber.java.BeforeAll; -import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.parallel.Isolated; -import org.testcontainers.containers.GenericContainer; - -@Isolated() -@Order(value = Integer.MAX_VALUE) -public class FlagdInProcessSetup { - - private static final GenericContainer flagdContainer = ContainerConfig.sync(true, false); - - @BeforeAll() - public static void setup() throws InterruptedException { - flagdContainer.start(); - } - - @Before() - public static void setupTest() throws InterruptedException { - FeatureProvider workingProvider = new FlagdProvider(FlagdOptions.builder() - .resolverType(Config.Resolver.IN_PROCESS) - .port(flagdContainer.getFirstMappedPort()) - // set a generous deadline, to prevent timeouts in actions - .deadline(3000) - .build()); - StepDefinitions.setUnstableProvider(workingProvider); - - FeatureProvider unavailableProvider = new FlagdProvider(FlagdOptions.builder() - .resolverType(Config.Resolver.IN_PROCESS) - .deadline(100) - .port(9092) // this port isn't serving anything, error expected - .build()); - StepDefinitions.setUnavailableProvider(unavailableProvider); - } - - @AfterAll - public static void tearDown() { - flagdContainer.stop(); - } -} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/reconnect/rpc/FlagdRpcSetup.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/reconnect/rpc/FlagdRpcSetup.java deleted file mode 100644 index e913265d5..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/reconnect/rpc/FlagdRpcSetup.java +++ /dev/null @@ -1,53 +0,0 @@ -package dev.openfeature.contrib.providers.flagd.e2e.reconnect.rpc; - -import dev.openfeature.contrib.providers.flagd.Config; -import dev.openfeature.contrib.providers.flagd.FlagdOptions; -import dev.openfeature.contrib.providers.flagd.FlagdProvider; -import dev.openfeature.contrib.providers.flagd.e2e.ContainerConfig; -import dev.openfeature.contrib.providers.flagd.e2e.reconnect.steps.StepDefinitions; -import dev.openfeature.contrib.providers.flagd.resolver.grpc.cache.CacheType; -import dev.openfeature.sdk.FeatureProvider; -import io.cucumber.java.AfterAll; -import io.cucumber.java.Before; -import io.cucumber.java.BeforeAll; -import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.parallel.Isolated; -import org.testcontainers.containers.GenericContainer; - -@Isolated() -@Order(value = Integer.MAX_VALUE) -public class FlagdRpcSetup { - private static final GenericContainer flagdContainer = ContainerConfig.flagd(true); - - @BeforeAll() - public static void setups() throws InterruptedException { - flagdContainer.start(); - } - - @Before() - public static void setupTest() throws InterruptedException { - - FeatureProvider workingProvider = new FlagdProvider(FlagdOptions.builder() - .resolverType(Config.Resolver.RPC) - .port(flagdContainer.getFirstMappedPort()) - .deadline(1000) - .retryGracePeriod(1) - .streamDeadlineMs(0) // this makes reconnect tests more predictable - .cacheType(CacheType.DISABLED.getValue()) - .build()); - StepDefinitions.setUnstableProvider(workingProvider); - - FeatureProvider unavailableProvider = new FlagdProvider(FlagdOptions.builder() - .resolverType(Config.Resolver.RPC) - .port(8015) // this port isn't serving anything, error expected - .deadline(100) - .cacheType(CacheType.DISABLED.getValue()) - .build()); - StepDefinitions.setUnavailableProvider(unavailableProvider); - } - - @AfterAll - public static void tearDown() { - flagdContainer.stop(); - } -} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/reconnect/steps/StepDefinitions.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/reconnect/steps/StepDefinitions.java deleted file mode 100644 index d438e7d0f..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/reconnect/steps/StepDefinitions.java +++ /dev/null @@ -1,121 +0,0 @@ -package dev.openfeature.contrib.providers.flagd.e2e.reconnect.steps; - -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertThrows; - -import dev.openfeature.sdk.Client; -import dev.openfeature.sdk.EventDetails; -import dev.openfeature.sdk.FeatureProvider; -import dev.openfeature.sdk.OpenFeatureAPI; -import io.cucumber.java.AfterAll; -import io.cucumber.java.en.Given; -import io.cucumber.java.en.Then; -import io.cucumber.java.en.When; -import java.time.Duration; -import java.util.function.Consumer; -import org.awaitility.Awaitility; -import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.parallel.Isolated; - -/** - * Test suite for testing flagd provider reconnect functionality. The associated container run a - * flagd instance which restarts every 5s. - */ -@Isolated() -@Order(value = Integer.MAX_VALUE) -public class StepDefinitions { - - private static Client client; - private static FeatureProvider unstableProvider; - private static FeatureProvider unavailableProvider; - - private int readyHandlerRunCount = 0; - private int errorHandlerRunCount = 0; - - private Consumer readyHandler = (EventDetails) -> { - readyHandlerRunCount++; - }; - private Consumer errorHandler = (EventDetails) -> { - errorHandlerRunCount++; - }; - - /** - * Injects the client to use for this test. Tests run one at a time, but just in case, a lock is - * used to make sure the client is not updated mid-test. - * - * @param provider client to inject into test. - */ - public static void setUnstableProvider(FeatureProvider provider) { - StepDefinitions.unstableProvider = provider; - } - - public static void setUnavailableProvider(FeatureProvider provider) { - StepDefinitions.unavailableProvider = provider; - } - - public StepDefinitions() { - StepDefinitions.client = OpenFeatureAPI.getInstance().getClient("unstable"); - OpenFeatureAPI.getInstance().setProviderAndWait("unstable", unstableProvider); - } - - @Given("a flagd provider is set") - public static void setup() { - // done in constructor - } - - @AfterAll() - public static void cleanUp() throws InterruptedException { - StepDefinitions.unstableProvider.shutdown(); - StepDefinitions.unstableProvider = null; - StepDefinitions.client = null; - } - - @When("a PROVIDER_READY handler and a PROVIDER_ERROR handler are added") - public void a_provider_ready_handler_and_a_provider_error_handler_are_added() { - client.onProviderReady(this.readyHandler); - client.onProviderError(this.errorHandler); - } - - @Then("the PROVIDER_READY handler must run when the provider connects") - public void the_provider_ready_handler_must_run_when_the_provider_connects() { - // no errors expected yet - assertEquals(0, errorHandlerRunCount); - // wait up to 240 seconds for a connect (PROVIDER_READY event) - Awaitility.await().atMost(Duration.ofSeconds(240)).until(() -> { - return this.readyHandlerRunCount == 1; - }); - } - - @Then("the PROVIDER_ERROR handler must run when the provider's connection is lost") - public void the_provider_error_handler_must_run_when_the_provider_s_connection_is_lost() { - // wait up to 240 seconds for a disconnect (PROVIDER_ERROR event) - Awaitility.await().atMost(Duration.ofSeconds(240)).until(() -> { - return this.errorHandlerRunCount > 0; - }); - } - - @Then("when the connection is reestablished the PROVIDER_READY handler must run again") - public void when_the_connection_is_reestablished_the_provider_ready_handler_must_run_again() { - // wait up to 240 seconds for a reconnect (PROVIDER_READY event) - Awaitility.await().atMost(Duration.ofSeconds(240)).until(() -> { - return this.readyHandlerRunCount > 1; - }); - } - - @Given("flagd is unavailable") - public void flagd_is_unavailable() { - // there is no flag available on the port used by StepDefinitions.unavailableProvider - } - - @When("a flagd provider is set and initialization is awaited") - public void a_flagd_provider_is_set_and_initialization_is_awaited() { - // handled below - } - - @Then("an error should be indicated within the configured deadline") - public void an_error_should_be_indicated_within_the_configured_deadline() { - assertThrows(Exception.class, () -> { - OpenFeatureAPI.getInstance().setProviderAndWait("unavailable", StepDefinitions.unavailableProvider); - }); - } -} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/rpc/FlagdRpcSetup.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/rpc/FlagdRpcSetup.java deleted file mode 100644 index 3d61dc034..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/rpc/FlagdRpcSetup.java +++ /dev/null @@ -1,42 +0,0 @@ -package dev.openfeature.contrib.providers.flagd.e2e.rpc; - -import dev.openfeature.contrib.providers.flagd.Config; -import dev.openfeature.contrib.providers.flagd.FlagdOptions; -import dev.openfeature.contrib.providers.flagd.FlagdProvider; -import dev.openfeature.contrib.providers.flagd.e2e.ContainerConfig; -import dev.openfeature.contrib.providers.flagd.e2e.steps.StepDefinitions; -import dev.openfeature.sdk.FeatureProvider; -import io.cucumber.java.AfterAll; -import io.cucumber.java.Before; -import io.cucumber.java.BeforeAll; -import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.parallel.Isolated; -import org.testcontainers.containers.GenericContainer; - -@Isolated() -@Order(value = Integer.MAX_VALUE) -public class FlagdRpcSetup { - - private static FeatureProvider provider; - private static final GenericContainer flagdContainer = ContainerConfig.flagd(); - - @BeforeAll() - public static void setup() { - flagdContainer.start(); - } - - @Before() - public static void test_setup() { - FlagdRpcSetup.provider = new FlagdProvider(FlagdOptions.builder() - .resolverType(Config.Resolver.RPC) - .port(flagdContainer.getFirstMappedPort()) - .deadline(500) - .build()); - StepDefinitions.setProvider(provider); - } - - @AfterAll - public static void tearDown() { - flagdContainer.stop(); - } -} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ssl/process/FlagdInProcessSetup.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ssl/process/FlagdInProcessSetup.java deleted file mode 100644 index e75c92394..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ssl/process/FlagdInProcessSetup.java +++ /dev/null @@ -1,51 +0,0 @@ -package dev.openfeature.contrib.providers.flagd.e2e.ssl.process; - -import dev.openfeature.contrib.providers.flagd.Config; -import dev.openfeature.contrib.providers.flagd.FlagdOptions; -import dev.openfeature.contrib.providers.flagd.FlagdProvider; -import dev.openfeature.contrib.providers.flagd.e2e.ContainerConfig; -import dev.openfeature.contrib.providers.flagd.e2e.steps.StepDefinitions; -import dev.openfeature.sdk.FeatureProvider; -import io.cucumber.java.AfterAll; -import io.cucumber.java.Before; -import io.cucumber.java.BeforeAll; -import java.io.File; -import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.parallel.Isolated; -import org.testcontainers.containers.GenericContainer; -import org.testcontainers.utility.DockerImageName; - -@Isolated() -@Order(value = Integer.MAX_VALUE) -public class FlagdInProcessSetup { - private static final GenericContainer flagdContainer = new GenericContainer( - DockerImageName.parse(ContainerConfig.generateContainerName("flagd", "ssl"))) - .withExposedPorts(8015); - - @BeforeAll() - public static void setups() throws InterruptedException { - flagdContainer.start(); - } - - @Before() - public static void setupTest() throws InterruptedException { - String path = "test-harness/ssl/custom-root-cert.crt"; - - File file = new File(path); - String absolutePath = file.getAbsolutePath(); - FeatureProvider workingProvider = new FlagdProvider(FlagdOptions.builder() - .resolverType(Config.Resolver.IN_PROCESS) - .port(flagdContainer.getFirstMappedPort()) - .deadline(10000) - .streamDeadlineMs(0) // this makes reconnect tests more predictable - .tls(true) - .certPath(absolutePath) - .build()); - StepDefinitions.setProvider(workingProvider); - } - - @AfterAll - public static void tearDown() { - flagdContainer.stop(); - } -} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ssl/rpc/FlagdRpcSetup.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ssl/rpc/FlagdRpcSetup.java deleted file mode 100644 index 6b318ae9e..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/ssl/rpc/FlagdRpcSetup.java +++ /dev/null @@ -1,51 +0,0 @@ -package dev.openfeature.contrib.providers.flagd.e2e.ssl.rpc; - -import dev.openfeature.contrib.providers.flagd.Config; -import dev.openfeature.contrib.providers.flagd.FlagdOptions; -import dev.openfeature.contrib.providers.flagd.FlagdProvider; -import dev.openfeature.contrib.providers.flagd.e2e.ContainerConfig; -import dev.openfeature.contrib.providers.flagd.e2e.steps.StepDefinitions; -import dev.openfeature.sdk.FeatureProvider; -import io.cucumber.java.AfterAll; -import io.cucumber.java.Before; -import io.cucumber.java.BeforeAll; -import java.io.File; -import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.parallel.Isolated; -import org.testcontainers.containers.GenericContainer; -import org.testcontainers.utility.DockerImageName; - -@Isolated() -@Order(value = Integer.MAX_VALUE) -public class FlagdRpcSetup { - private static final GenericContainer flagdContainer = new GenericContainer( - DockerImageName.parse(ContainerConfig.generateContainerName("flagd", "ssl"))) - .withExposedPorts(8013); - - @BeforeAll() - public static void setups() throws InterruptedException { - flagdContainer.start(); - } - - @Before() - public static void setupTest() throws InterruptedException { - String path = "test-harness/ssl/custom-root-cert.crt"; - - File file = new File(path); - String absolutePath = file.getAbsolutePath(); - FeatureProvider workingProvider = new FlagdProvider(FlagdOptions.builder() - .resolverType(Config.Resolver.RPC) - .port(flagdContainer.getFirstMappedPort()) - .deadline(10000) - .streamDeadlineMs(0) // this makes reconnect tests more predictable - .tls(true) - .certPath(absolutePath) - .build()); - StepDefinitions.setProvider(workingProvider); - } - - @AfterAll - public static void tearDown() { - flagdContainer.stop(); - } -} 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 new file mode 100644 index 000000000..c712bf242 --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/AbstractSteps.java @@ -0,0 +1,10 @@ +package dev.openfeature.contrib.providers.flagd.e2e.steps; + +import dev.openfeature.contrib.providers.flagd.e2e.State; + +abstract class AbstractSteps { + State state; + public AbstractSteps(State state) { + this.state = state; + } +} 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/ConfigSteps.java similarity index 66% rename from providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/config/ConfigSteps.java rename to providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ConfigSteps.java index a680a9947..a319ec543 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/ConfigSteps.java @@ -1,10 +1,9 @@ -package dev.openfeature.contrib.providers.flagd.e2e.steps.config; +package dev.openfeature.contrib.providers.flagd.e2e.steps; import static org.assertj.core.api.Assertions.assertThat; import dev.openfeature.contrib.providers.flagd.Config; -import dev.openfeature.contrib.providers.flagd.FlagdOptions; -import dev.openfeature.contrib.providers.flagd.resolver.grpc.cache.CacheType; +import dev.openfeature.contrib.providers.flagd.e2e.State; import io.cucumber.java.en.Given; import io.cucumber.java.en.Then; import io.cucumber.java.en.When; @@ -19,7 +18,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class ConfigSteps { +import static org.assertj.core.api.Assertions.assertThat; + +public class ConfigSteps extends AbstractSteps { /** * Not all properties are correctly implemented, hence that we need to ignore them till this is * fixed @@ -33,22 +34,24 @@ public class ConfigSteps { private static final Logger LOG = LoggerFactory.getLogger(ConfigSteps.class); - FlagdOptions.FlagdOptionsBuilder builder = FlagdOptions.builder(); - FlagdOptions options; + + public ConfigSteps(State state) { + super(state); + } @When("a config was initialized") public void we_initialize_a_config() { - options = builder.build(); + state.options = state.builder.build(); } @When("a config was initialized for {string}") public void we_initialize_a_config_for(String string) { switch (string.toLowerCase()) { case "in-process": - options = builder.resolverType(Config.Resolver.IN_PROCESS).build(); + state.options = state.builder.resolverType(Config.Resolver.IN_PROCESS).build(); break; case "rpc": - options = builder.resolverType(Config.Resolver.RPC).build(); + state.options = state.builder.resolverType(Config.Resolver.RPC).build(); break; default: throw new RuntimeException("Unknown resolver type: " + string); @@ -63,12 +66,12 @@ public void we_have_an_option_of_type_with_value(String option, String type, Str return; } - Object converted = convert(value, type); - Method method = Arrays.stream(builder.getClass().getMethods()) + Object converted = Utils.convert(value, type); + Method method = Arrays.stream(state.builder.getClass().getMethods()) .filter(method1 -> method1.getName().equals(mapOptionNames(option))) .findFirst() .orElseThrow(RuntimeException::new); - method.invoke(builder, converted); + method.invoke(state.builder, converted); } Map envVarsSet = new HashMap<>(); @@ -81,35 +84,10 @@ public void we_have_an_environment_variable_with_value(String varName, String va EnvironmentVariableUtils.set(varName, value); } - private Object convert(String value, String type) throws ClassNotFoundException { - if (Objects.equals(value, "null")) return null; - switch (type) { - case "Boolean": - return Boolean.parseBoolean(value); - case "String": - return value; - case "Integer": - return Integer.parseInt(value); - case "Long": - return Long.parseLong(value); - case "ResolverType": - switch (value.toLowerCase()) { - case "in-process": - return Config.Resolver.IN_PROCESS; - case "rpc": - return Config.Resolver.RPC; - default: - throw new RuntimeException("Unknown resolver type: " + value); - } - case "CacheType": - return CacheType.valueOf(value.toUpperCase()).getValue(); - } - throw new RuntimeException("Unknown config type: " + type); - } @Then("the option {string} of type {string} should have the value {string}") public void the_option_of_type_should_have_the_value(String option, String type, String value) throws Throwable { - Object convert = convert(value, type); + Object convert = Utils.convert(value, type); if (IGNORED_FOR_NOW.contains(option)) { LOG.error("option '{}' is not supported", option); @@ -118,7 +96,7 @@ public void the_option_of_type_should_have_the_value(String option, String type, option = mapOptionNames(option); - assertThat(options).hasFieldOrPropertyWithValue(option, convert); + assertThat(state.options).hasFieldOrPropertyWithValue(option, convert); // Resetting env vars for (Map.Entry envVar : envVarsSet.entrySet()) { diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ContextSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ContextSteps.java new file mode 100644 index 000000000..c8309a3b4 --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ContextSteps.java @@ -0,0 +1,43 @@ +package dev.openfeature.contrib.providers.flagd.e2e.steps; + +import dev.openfeature.contrib.providers.flagd.e2e.State; +import dev.openfeature.sdk.ImmutableStructure; +import dev.openfeature.sdk.MutableContext; +import dev.openfeature.sdk.Value; +import io.cucumber.java.en.Given; +import org.junit.jupiter.api.parallel.Isolated; + +import java.util.HashMap; +import java.util.Map; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +@Isolated() +public class ContextSteps extends AbstractSteps { + + + public ContextSteps(State state) { + super(state); + } + + + @Given("a context containing a key {string}, with type {string} and with value {string}") + public void a_context_containing_a_key_with_type_and_with_value(String key, String type, String value) throws ClassNotFoundException, InstantiationException { + Map map = state.context.asMap(); + map.put(key, new Value(value)); + state.context = new MutableContext(state.context.getTargetingKey(), map); + } + + @Given("a context containing a targeting key with value {string}") + public void a_context_containing_a_targeting_key_with_value(String string) { + state.context.setTargetingKey(string); + } + + @Given("a context containing a nested property with outer key {string} and inner key {string}, with value {string}") + public void a_context_containing_a_nested_property_with_outer_key_and_inner_key_with_value(String outer, String inner, String value) { + Map innerMap = new HashMap<>(); + innerMap.put(inner, new Value(value)); + state.context.add(outer, new ImmutableStructure(innerMap)); + } + +} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/config/EnvironmentVariableUtils.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EnvironmentVariableUtils.java similarity index 98% rename from providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/config/EnvironmentVariableUtils.java rename to providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EnvironmentVariableUtils.java index 6d1946134..b3ef4346e 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/config/EnvironmentVariableUtils.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EnvironmentVariableUtils.java @@ -1,4 +1,4 @@ -package dev.openfeature.contrib.providers.flagd.e2e.steps.config; +package dev.openfeature.contrib.providers.flagd.e2e.steps; /* * Copy of JUnit Pioneer's EnvironmentVariable Utils diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Event.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Event.java new file mode 100644 index 000000000..f93c327e6 --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Event.java @@ -0,0 +1,13 @@ +package dev.openfeature.contrib.providers.flagd.e2e.steps; + +import dev.openfeature.sdk.EventDetails; + +public class Event { + public String type; + public EventDetails details; + + public Event(String eventType, EventDetails eventDetails) { + this.type = eventType; + this.details = eventDetails; + } +} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java new file mode 100644 index 000000000..67b24b155 --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java @@ -0,0 +1,69 @@ +package dev.openfeature.contrib.providers.flagd.e2e.steps; + +import dev.openfeature.contrib.providers.flagd.e2e.State; +import dev.openfeature.sdk.ProviderEvent; +import io.cucumber.java.en.Given; +import io.cucumber.java.en.Then; +import io.cucumber.java.en.When; +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.parallel.Isolated; + +import java.util.ArrayList; + +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static org.awaitility.Awaitility.await; + +@Isolated() +public class EventSteps extends AbstractSteps { + + + public EventSteps(State state) { + super(state); + state.events = new ArrayList<>(); + } + + @Given("a {} event handler") + public void a_stale_event_handler(String eventType) { + state.client.on(mapEventType(eventType), eventDetails -> { + System.out.println(eventType); + state.events.add(new Event(eventType, eventDetails)); + }); + } + + private static @NotNull ProviderEvent mapEventType(String eventType) { + switch (eventType) { + case "stale": + return ProviderEvent.PROVIDER_STALE; + case "ready": + return ProviderEvent.PROVIDER_READY; + case "error": + return ProviderEvent.PROVIDER_ERROR; + case "change": + return ProviderEvent.PROVIDER_CONFIGURATION_CHANGED; + default: + throw new IllegalArgumentException("Unknown event type: " + eventType); + } + } + + @When("a {} event was fired") + public void eventWasFired(String eventType) { + eventHandlerShouldBeExecutedWithin(eventType, 10000); + } + + @Then("the {} event handler should have been executed") + public void eventHandlerShouldBeExecuted(String eventType) { + eventHandlerShouldBeExecutedWithin(eventType, 10000); + } + + @Then("the {} event handler should have been executed within {int}ms") + public void eventHandlerShouldBeExecutedWithin(String eventType, int ms) { + await() + .atMost(ms, MILLISECONDS) + .until(() -> { + state.events.forEach((e) -> System.out.println(e.type)); + return state.events.stream().anyMatch(event -> event.type.equals(eventType)); + }); + state.lastEvent = state.events.stream().filter(event -> event.type.equals(eventType)).findFirst(); + state.events.removeIf(event -> event.type.equals(eventType)); + } +} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java new file mode 100644 index 000000000..4b193af3e --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java @@ -0,0 +1,86 @@ +package dev.openfeature.contrib.providers.flagd.e2e.steps; + +import dev.openfeature.contrib.providers.flagd.e2e.State; +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.Value; +import io.cucumber.java.en.Given; +import io.cucumber.java.en.Then; +import io.cucumber.java.en.When; +import org.junit.jupiter.api.parallel.Isolated; + +import static org.assertj.core.api.Assertions.assertThat; + + +@Isolated() +public class FlagSteps extends AbstractSteps { + + + public FlagSteps(State state) { + super(state); + } + + @Given("a {}-flag with key {string} and a default value {string}") + public void givenAFlag(String type, String name, String defaultValue) throws ClassNotFoundException { + state.flag = new Flag(type, name, Utils.convert(defaultValue, type)); + + } + + @When("the flag was evaluated with details") + public void the_flag_was_evaluated_with_details() { + FlagEvaluationDetails details; + switch (state.flag.type) { + case "String": + details = state.client.getStringDetails(state.flag.name, (String) state.flag.defaultValue, state.context); + break; + case "Boolean": + details = state.client.getBooleanDetails(state.flag.name, (Boolean) state.flag.defaultValue, state.context); + break; + case "Float": + details = state.client.getDoubleDetails(state.flag.name, (Double) state.flag.defaultValue, state.context); + break; + case "Integer": + details = state.client.getIntegerDetails(state.flag.name, (Integer) state.flag.defaultValue, state.context); + break; + case "Object": + details = state.client.getObjectDetails(state.flag.name, (Value) state.flag.defaultValue, state.context); + break; + default: + throw new AssertionError(); + } + state.evaluation = details; + } + + @Then("the resolved details value should be {string}") + public void the_resolved_details_value_should_be(String value) throws ClassNotFoundException { + assertThat(state.evaluation.getValue()).isEqualTo(Utils.convert(value, state.flag.type)); + } + + @Then("the reason should be {string}") + public void the_reason_should_be(String reason) { + assertThat(state.evaluation.getReason()).isEqualTo(reason); + } + @Then("the variant should be {string}") + public void the_variant_should_be(String variant) { + assertThat(state.evaluation.getVariant()).isEqualTo(variant); + } + @Then("the flag was modified") + public void the_flag_was_modified() { + assertThat(state.lastEvent).isPresent().hasValueSatisfying((event) -> { + assertThat(event.type).isEqualTo("change"); + assertThat(event.details.getFlagsChanged()).contains(state.flag.name); + }); + } + + + public class Flag { + String name; + Object defaultValue; + String type; + + public Flag(String type, String name, Object defaultValue) { + this.name = name; + this.defaultValue = defaultValue; + this.type = type; + } + } +} 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 new file mode 100644 index 000000000..17ae53025 --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderSteps.java @@ -0,0 +1,159 @@ +package dev.openfeature.contrib.providers.flagd.e2e.steps; + +import dev.openfeature.contrib.providers.flagd.FlagdProvider; +import dev.openfeature.contrib.providers.flagd.e2e.FlagdContainer; +import dev.openfeature.contrib.providers.flagd.e2e.State; +import dev.openfeature.sdk.Client; +import dev.openfeature.sdk.FeatureProvider; +import dev.openfeature.sdk.OpenFeatureAPI; +import io.cucumber.java.After; +import io.cucumber.java.AfterAll; +import io.cucumber.java.Before; +import io.cucumber.java.BeforeAll; +import io.cucumber.java.en.Given; +import io.cucumber.java.en.When; +import org.apache.commons.lang3.RandomStringUtils; +import org.junit.jupiter.api.parallel.Isolated; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testcontainers.containers.BindMode; +import org.testcontainers.shaded.org.apache.commons.io.FileUtils; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.HashMap; +import java.util.Map; +import java.util.Timer; +import java.util.TimerTask; + +@Isolated() +public class ProviderSteps extends AbstractSteps { + + private static final Logger LOG = LoggerFactory.getLogger(ProviderSteps.class); + + public static final int UNAVAILABLE_PORT = 9999; + static Map containers = new HashMap<>(); + + static Path sharedTempDir; + + static { + try { + sharedTempDir = Files.createDirectories( + Paths.get("tmp/" + RandomStringUtils.randomAlphanumeric(8).toLowerCase() + "/")); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + public ProviderSteps(State state) { + super(state); + } + + @BeforeAll + public static void beforeAll() throws IOException { + containers.put(ProviderType.DEFAULT, new FlagdContainer()); + containers.put(ProviderType.SSL, new FlagdContainer("ssl")); + containers.put(ProviderType.SOCKET, new FlagdContainer("socket") + .withFileSystemBind(sharedTempDir.toAbsolutePath().toString(), "/tmp", BindMode.READ_WRITE)); + + } + + @AfterAll + public static void afterAll() throws IOException { + + containers.forEach((name, container) -> container.stop()); + FileUtils.deleteDirectory(sharedTempDir.toFile()); + } + + @Before + public void before() { + containers.values().stream().filter(containers -> !containers.isRunning()) + .forEach(FlagdContainer::start); + } + @After + public void tearDown() { + OpenFeatureAPI.getInstance().shutdown(); + } + + + @Given("a {} flagd provider") + public void setupProvider(String providerType) { + state.builder + .deadline(500) + .keepAlive(0) + .retryGracePeriod(1); + boolean wait = true; + switch (providerType) { + case "unavailable": + this.state.providerType = ProviderType.SOCKET; + state.builder.port(UNAVAILABLE_PORT); + wait = false; + break; + case "socket": + this.state.providerType = ProviderType.SOCKET; + String socketPath = sharedTempDir.resolve("socket.sock").toAbsolutePath().toString(); + state.builder.socketPath(socketPath); + state.builder.port(UNAVAILABLE_PORT); + break; + case "ssl": + String path = "test-harness/ssl/custom-root-cert.crt"; + + File file = new File(path); + String absolutePath = file.getAbsolutePath(); + this.state.providerType = ProviderType.SSL; + state + .builder + .port(getContainer().getPort(State.resolverType)) + .tls(true) + .certPath(absolutePath); + break; + + default: + this.state.providerType = ProviderType.DEFAULT; + state.builder.port(getContainer().getPort(State.resolverType)); + break; + } + FeatureProvider provider = new FlagdProvider(state.builder + .resolverType(State.resolverType) + .build()); + + OpenFeatureAPI api = OpenFeatureAPI.getInstance(); + if (wait) { + api.setProviderAndWait(providerType, provider); + } else { + api.setProvider(providerType, provider); + } + this.state.client = api.getClient(providerType); + } + + @When("the connection is lost for {int}s") + public void the_connection_is_lost_for(int seconds) throws InterruptedException { + FlagdContainer container = getContainer(); + +/* TimerTask task = new TimerTask() { + public void run() { + container.start(); + int port = container.getPort(State.resolverType); + } + }; + Timer timer = new Timer("Timer");*/ + + LOG.info("stopping container for {}", state.providerType); + container.stop(); + + //timer.schedule(task, seconds * 1000L); + Thread.sleep(seconds * 1000L); + + LOG.info("starting container for {}", state.providerType); + container.start(); + } + + private FlagdContainer getContainer() { + LOG.info("getting container for {}", state.providerType); + System.out.println("getting container for " + state.providerType); + return containers.getOrDefault(state.providerType, containers.get(ProviderType.DEFAULT)); + } +} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderType.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderType.java new file mode 100644 index 000000000..eac0ba967 --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ProviderType.java @@ -0,0 +1,7 @@ +package dev.openfeature.contrib.providers.flagd.e2e.steps; + +public enum ProviderType { + DEFAULT, + SSL, + SOCKET +} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/StepDefinitions.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/StepDefinitions.java deleted file mode 100644 index c3f894a08..000000000 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/StepDefinitions.java +++ /dev/null @@ -1,555 +0,0 @@ -package dev.openfeature.contrib.providers.flagd.e2e.steps; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -import dev.openfeature.sdk.Client; -import dev.openfeature.sdk.EvaluationContext; -import dev.openfeature.sdk.EventDetails; -import dev.openfeature.sdk.FeatureProvider; -import dev.openfeature.sdk.FlagEvaluationDetails; -import dev.openfeature.sdk.ImmutableContext; -import dev.openfeature.sdk.ImmutableStructure; -import dev.openfeature.sdk.OpenFeatureAPI; -import dev.openfeature.sdk.Reason; -import dev.openfeature.sdk.Structure; -import dev.openfeature.sdk.Value; -import io.cucumber.java.AfterAll; -import io.cucumber.java.en.And; -import io.cucumber.java.en.Given; -import io.cucumber.java.en.Then; -import io.cucumber.java.en.When; -import java.time.Duration; -import java.util.HashMap; -import java.util.Map; -import java.util.function.Consumer; -import org.awaitility.Awaitility; -import org.junit.jupiter.api.Order; -import org.junit.jupiter.api.parallel.Isolated; - -/** Common test suite used by both RPC and in-process flagd providers. */ -@Isolated() -@Order(value = Integer.MAX_VALUE) -public class StepDefinitions { - - private static Client client; - private static FeatureProvider provider; - - private String booleanFlagKey; - private String stringFlagKey; - private String intFlagKey; - private String doubleFlagKey; - private String objectFlagKey; - - private boolean booleanFlagDefaultValue; - private String stringFlagDefaultValue; - private int intFlagDefaultValue; - private double doubleFlagDefaultValue; - private Value objectFlagDefaultValue; - - private FlagEvaluationDetails objectFlagDetails; - - private String contextAwareFlagKey; - private String contextAwareDefaultValue; - private EvaluationContext context; - private String contextAwareValue; - - private String notFoundFlagKey; - private String notFoundDefaultValue; - private FlagEvaluationDetails notFoundDetails; - private String typeErrorFlagKey; - private int typeErrorDefaultValue; - private FlagEvaluationDetails typeErrorDetails; - - private boolean isChangeHandlerRun = false; - private String changedFlag; - private boolean isReadyHandlerRun = false; - - private Consumer changeHandler; - private Consumer readyHandler; - - /** - * Injects the client to use for this test. Tests run one at a time, but just in case, a lock is - * used to make sure the client is not updated mid-test. - * - * @param client client to inject into test. - */ - public static void setProvider(FeatureProvider provider) { - StepDefinitions.provider = provider; - } - - public StepDefinitions() { - OpenFeatureAPI.getInstance().setProviderAndWait("e2e", provider); - StepDefinitions.client = OpenFeatureAPI.getInstance().getClient("e2e"); - } - - @Given("a provider is registered") - @Given("a flagd provider is set") - public static void setup() { - // done in constructor - } - - @AfterAll() - public static void cleanUp() throws InterruptedException { - StepDefinitions.provider.shutdown(); - StepDefinitions.provider = null; - StepDefinitions.client = null; - } - - /* - * Basic evaluation - */ - - // boolean value - @When("a boolean flag with key {string} is evaluated with default value {string}") - public void a_boolean_flag_with_key_boolean_flag_is_evaluated_with_default_value_false( - String flagKey, String defaultValue) { - this.booleanFlagKey = flagKey; - this.booleanFlagDefaultValue = Boolean.valueOf(defaultValue); - } - - @Then("the resolved boolean value should be {string}") - public void the_resolved_boolean_value_should_be_true(String expected) { - boolean value = client.getBooleanValue(this.booleanFlagKey, Boolean.valueOf(this.booleanFlagDefaultValue)); - assertEquals(Boolean.valueOf(expected), value); - } - - // string value - - @When("a string flag with key {string} is evaluated with default value {string}") - public void a_string_flag_with_key_is_evaluated_with_default_value(String flagKey, String defaultValue) { - this.stringFlagKey = flagKey; - this.stringFlagDefaultValue = defaultValue; - } - - @Then("the resolved string value should be {string}") - public void the_resolved_string_value_should_be(String expected) { - String value = client.getStringValue(this.stringFlagKey, this.stringFlagDefaultValue); - assertEquals(expected, value); - } - - // integer value - @When("an integer flag with key {string} is evaluated with default value {int}") - public void an_integer_flag_with_key_is_evaluated_with_default_value(String flagKey, Integer defaultValue) { - this.intFlagKey = flagKey; - this.intFlagDefaultValue = defaultValue; - } - - @Then("the resolved integer value should be {int}") - public void the_resolved_integer_value_should_be(int expected) { - int value = client.getIntegerValue(this.intFlagKey, this.intFlagDefaultValue); - assertEquals(expected, value); - } - - // float/double value - @When("a float flag with key {string} is evaluated with default value {double}") - public void a_float_flag_with_key_is_evaluated_with_default_value(String flagKey, double defaultValue) { - this.doubleFlagKey = flagKey; - this.doubleFlagDefaultValue = defaultValue; - } - - @Then("the resolved float value should be {double}") - public void the_resolved_float_value_should_be(double expected) { - double value = client.getDoubleValue(this.doubleFlagKey, this.doubleFlagDefaultValue); - assertEquals(expected, value); - } - - // object value - @When("an object flag with key {string} is evaluated with a null default value") - public void an_object_flag_with_key_is_evaluated_with_a_null_default_value(String flagKey) { - this.objectFlagKey = flagKey; - this.objectFlagDefaultValue = new Value(); // empty value is equivalent to null - } - - @Then( - "the resolved object value should be contain fields {string}, {string}, and {string}, with values {string}, {string} and {int}, respectively") - public void the_resolved_object_value_should_be_contain_fields_and_with_values_and_respectively( - String boolField, - String stringField, - String numberField, - String boolValue, - String stringValue, - int numberValue) { - Value value = client.getObjectValue(this.objectFlagKey, this.objectFlagDefaultValue); - Structure structure = value.asStructure(); - - assertEquals( - Boolean.valueOf(boolValue), structure.asMap().get(boolField).asBoolean()); - assertEquals(stringValue, structure.asMap().get(stringField).asString()); - assertEquals(numberValue, structure.asMap().get(numberField).asInteger()); - } - - /* - * Detailed evaluation - */ - - // boolean details - @When("a boolean flag with key {string} is evaluated with details and default value {string}") - public void a_boolean_flag_with_key_is_evaluated_with_details_and_default_value( - String flagKey, String defaultValue) { - this.booleanFlagKey = flagKey; - this.booleanFlagDefaultValue = Boolean.valueOf(defaultValue); - } - - @Then( - "the resolved boolean details value should be {string}, the variant should be {string}, and the reason should be {string}") - public void the_resolved_boolean_value_should_be_the_variant_should_be_and_the_reason_should_be( - String expectedValue, String expectedVariant, String expectedReason) { - FlagEvaluationDetails details = - client.getBooleanDetails(this.booleanFlagKey, Boolean.valueOf(this.booleanFlagDefaultValue)); - - assertEquals(Boolean.valueOf(expectedValue), details.getValue()); - assertEquals(expectedVariant, details.getVariant()); - assertEquals(expectedReason, details.getReason()); - } - - // string details - @When("a string flag with key {string} is evaluated with details and default value {string}") - public void a_string_flag_with_key_is_evaluated_with_details_and_default_value( - String flagKey, String defaultValue) { - this.stringFlagKey = flagKey; - this.stringFlagDefaultValue = defaultValue; - } - - @When("a string flag with key {string} is evaluated with details") - public void a_string_flag_with_key_is_evaluated_with_details(String flagKey) { - this.stringFlagKey = flagKey; - this.stringFlagDefaultValue = ""; - } - - @Then( - "the resolved string details value should be {string}, the variant should be {string}, and the reason should be {string}") - public void the_resolved_string_value_should_be_the_variant_should_be_and_the_reason_should_be( - String expectedValue, String expectedVariant, String expectedReason) { - FlagEvaluationDetails details = - client.getStringDetails(this.stringFlagKey, this.stringFlagDefaultValue); - - assertEquals(expectedValue, details.getValue()); - assertEquals(expectedVariant, details.getVariant()); - assertEquals(expectedReason, details.getReason()); - } - - // integer details - @When("an integer flag with key {string} is evaluated with details and default value {int}") - public void an_integer_flag_with_key_is_evaluated_with_details_and_default_value(String flagKey, int defaultValue) { - this.intFlagKey = flagKey; - this.intFlagDefaultValue = defaultValue; - } - - @Then( - "the resolved integer details value should be {int}, the variant should be {string}, and the reason should be {string}") - public void the_resolved_integer_value_should_be_the_variant_should_be_and_the_reason_should_be( - int expectedValue, String expectedVariant, String expectedReason) { - FlagEvaluationDetails details = client.getIntegerDetails(this.intFlagKey, this.intFlagDefaultValue); - - assertEquals(expectedValue, details.getValue()); - assertEquals(expectedVariant, details.getVariant()); - assertEquals(expectedReason, details.getReason()); - } - - // float/double details - @When("a float flag with key {string} is evaluated with details and default value {double}") - public void a_float_flag_with_key_is_evaluated_with_details_and_default_value(String flagKey, double defaultValue) { - this.doubleFlagKey = flagKey; - this.doubleFlagDefaultValue = defaultValue; - } - - @Then( - "the resolved float details value should be {double}, the variant should be {string}, and the reason should be {string}") - public void the_resolved_float_value_should_be_the_variant_should_be_and_the_reason_should_be( - double expectedValue, String expectedVariant, String expectedReason) { - FlagEvaluationDetails details = - client.getDoubleDetails(this.doubleFlagKey, this.doubleFlagDefaultValue); - - assertEquals(expectedValue, details.getValue()); - assertEquals(expectedVariant, details.getVariant()); - assertEquals(expectedReason, details.getReason()); - } - - // object details - @When("an object flag with key {string} is evaluated with details and a null default value") - public void an_object_flag_with_key_is_evaluated_with_details_and_a_null_default_value(String flagKey) { - this.objectFlagKey = flagKey; - this.objectFlagDefaultValue = new Value(); - } - - @Then( - "the resolved object details value should be contain fields {string}, {string}, and {string}, with values {string}, {string} and {int}, respectively") - public void the_resolved_object_value_should_be_contain_fields_and_with_values_and_respectively_again( - String boolField, - String stringField, - String numberField, - String boolValue, - String stringValue, - int numberValue) { - this.objectFlagDetails = client.getObjectDetails(this.objectFlagKey, this.objectFlagDefaultValue); - Structure structure = this.objectFlagDetails.getValue().asStructure(); - - assertEquals( - Boolean.valueOf(boolValue), structure.asMap().get(boolField).asBoolean()); - assertEquals(stringValue, structure.asMap().get(stringField).asString()); - assertEquals(numberValue, structure.asMap().get(numberField).asInteger()); - } - - @Then("the variant should be {string}, and the reason should be {string}") - public void the_variant_should_be_and_the_reason_should_be(String expectedVariant, String expectedReason) { - assertEquals(expectedVariant, this.objectFlagDetails.getVariant()); - assertEquals(expectedReason, this.objectFlagDetails.getReason()); - } - - /* - * Context-aware evaluation - */ - - @When( - "context contains keys {string}, {string}, {string}, {string} with values {string}, {string}, {int}, {string}") - public void context_contains_keys_with_values( - String field1, - String field2, - String field3, - String field4, - String value1, - String value2, - Integer value3, - String value4) { - Map attributes = new HashMap<>(); - attributes.put(field1, new Value(value1)); - attributes.put(field2, new Value(value2)); - attributes.put(field3, new Value(value3)); - attributes.put(field4, new Value(Boolean.valueOf(value4))); - this.context = new ImmutableContext(attributes); - } - - @When("a flag with key {string} is evaluated with default value {string}") - public void an_a_flag_with_key_is_evaluated(String flagKey, String defaultValue) { - contextAwareFlagKey = flagKey; - contextAwareDefaultValue = defaultValue; - contextAwareValue = client.getStringValue(flagKey, contextAwareDefaultValue, context); - } - - @Then("the resolved string response should be {string}") - public void the_resolved_string_response_should_be(String expected) { - assertEquals(expected, this.contextAwareValue); - } - - @Then("the resolved flag value is {string} when the context is empty") - public void the_resolved_flag_value_is_when_the_context_is_empty(String expected) { - String emptyContextValue = - client.getStringValue(contextAwareFlagKey, contextAwareDefaultValue, new ImmutableContext()); - assertEquals(expected, emptyContextValue); - } - - /* - * Errors - */ - - // not found - @When("a non-existent string flag with key {string} is evaluated with details and a default value {string}") - public void a_non_existent_string_flag_with_key_is_evaluated_with_details_and_a_default_value( - String flagKey, String defaultValue) { - notFoundFlagKey = flagKey; - notFoundDefaultValue = defaultValue; - notFoundDetails = client.getStringDetails(notFoundFlagKey, notFoundDefaultValue); - } - - @Then("the default string value should be returned") - public void then_the_default_string_value_should_be_returned() { - assertEquals(notFoundDefaultValue, notFoundDetails.getValue()); - } - - @Then("the reason should indicate an error and the error code should indicate a missing flag with {string}") - public void the_reason_should_indicate_an_error_and_the_error_code_should_be_flag_not_found(String errorCode) { - assertEquals(Reason.ERROR.toString(), notFoundDetails.getReason()); - assertEquals(errorCode, notFoundDetails.getErrorCode().toString()); - } - - // type mismatch - @When("a string flag with key {string} is evaluated as an integer, with details and a default value {int}") - public void a_string_flag_with_key_is_evaluated_as_an_integer_with_details_and_a_default_value( - String flagKey, int defaultValue) { - typeErrorFlagKey = flagKey; - typeErrorDefaultValue = defaultValue; - typeErrorDetails = client.getIntegerDetails(typeErrorFlagKey, typeErrorDefaultValue); - } - - @Then("the default integer value should be returned") - public void then_the_default_integer_value_should_be_returned() { - assertEquals(typeErrorDefaultValue, typeErrorDetails.getValue()); - } - - @Then("the reason should indicate an error and the error code should indicate a type mismatch with {string}") - public void the_reason_should_indicate_an_error_and_the_error_code_should_be_type_mismatch(String errorCode) { - assertEquals(Reason.ERROR.toString(), typeErrorDetails.getReason()); - assertEquals(errorCode, typeErrorDetails.getErrorCode().toString()); - } - - /* - * Custom JSON evaluators (only run for flagd-in-process) - */ - - @And("a context containing a nested property with outer key {string} and inner key {string}, with value {string}") - public void a_context_containing_a_nested_property_with_outer_key_and_inner_key_with_value( - String outerKey, String innerKey, String value) throws InstantiationException { - Map innerMap = new HashMap(); - innerMap.put(innerKey, new Value(value)); - Map outerMap = new HashMap(); - outerMap.put(outerKey, new Value(new ImmutableStructure(innerMap))); - this.context = new ImmutableContext(outerMap); - } - - @And("a context containing a nested property with outer key {string} and inner key {string}, with value {int}") - public void a_context_containing_a_nested_property_with_outer_key_and_inner_key_with_value_int( - String outerKey, String innerKey, Integer value) throws InstantiationException { - Map innerMap = new HashMap(); - innerMap.put(innerKey, new Value(value)); - Map outerMap = new HashMap(); - outerMap.put(outerKey, new Value(new ImmutableStructure(innerMap))); - this.context = new ImmutableContext(outerMap); - } - - @And("a context containing a key {string}, with value {string}") - public void a_context_containing_a_key_with_value(String key, String value) { - Map attrs = new HashMap(); - attrs.put(key, new Value(value)); - this.context = new ImmutableContext(attrs); - } - - @And("a context containing a key {string}, with value {double}") - public void a_context_containing_a_key_with_value_double(String key, Double value) { - Map attrs = new HashMap(); - attrs.put(key, new Value(value)); - this.context = new ImmutableContext(attrs); - } - - @Then("the returned value should be {string}") - public void the_returned_value_should_be(String expected) { - String value = client.getStringValue(this.stringFlagKey, this.stringFlagDefaultValue, this.context); - assertEquals(expected, value); - } - - @Then("the returned value should be {int}") - public void the_returned_value_should_be(Integer expectedValue) { - Integer value = client.getIntegerValue(this.intFlagKey, this.intFlagDefaultValue, this.context); - assertEquals(expectedValue, value); - } - - /* - * Events - */ - - // Flag change event - @When("a PROVIDER_CONFIGURATION_CHANGED handler is added") - public void a_provider_configuration_changed_handler_is_added() { - this.changeHandler = (EventDetails details) -> { - if (details.getFlagsChanged().size() > 0) { - // we get multiple change events from the test container... - // we're only interested in the ones with the changed flag in question - this.changedFlag = details.getFlagsChanged().get(0); - this.isChangeHandlerRun = true; - } - }; - client.onProviderConfigurationChanged(this.changeHandler); - } - - @When("a flag with key {string} is modified") - public void a_flag_with_key_is_modified(String flagKey) { - // This happens automatically - } - - @Then("the PROVIDER_CONFIGURATION_CHANGED handler must run") - public void the_provider_configuration_changed_handler_must_run() { - Awaitility.await().atMost(Duration.ofSeconds(2)).until(() -> { - return this.isChangeHandlerRun; - }); - } - - @Then("the event details must indicate {string} was altered") - public void the_event_details_must_indicate_was_altered(String flagKey) { - assertEquals(flagKey, this.changedFlag); - } - - // Provider ready event - @When("a PROVIDER_READY handler is added") - public void a_provider_ready_handler_is_added() { - this.readyHandler = (EventDetails details) -> { - this.isReadyHandlerRun = true; - }; - client.onProviderReady(this.readyHandler); - } - - @Then("the PROVIDER_READY handler must run") - public void the_provider_ready_handler_must_run() { - Awaitility.await().atMost(Duration.ofSeconds(2)).until(() -> { - return this.isReadyHandlerRun; - }); - } - - /* - * Zero Value - */ - - // boolean value - @When("a zero-value boolean flag with key {string} is evaluated with default value {string}") - public void a_zero_value_boolean_flag_with_key_is_evaluated_with_default_value( - String flagKey, String defaultValue) { - this.booleanFlagKey = flagKey; - this.booleanFlagDefaultValue = Boolean.valueOf(defaultValue); - } - - @Then("the resolved boolean zero-value should be {string}") - public void the_resolved_boolean_zero_value_should_be(String expected) { - boolean value = client.getBooleanValue(this.booleanFlagKey, this.booleanFlagDefaultValue); - assertEquals(Boolean.valueOf(expected), value); - } - - // float/double value - @When("a zero-value float flag with key {string} is evaluated with default value {double}") - public void a_zero_value_float_flag_with_key_is_evaluated_with_default_value(String flagKey, Double defaultValue) { - this.doubleFlagKey = flagKey; - this.doubleFlagDefaultValue = defaultValue; - } - - @Then("the resolved float zero-value should be {double}") - public void the_resolved_float_zero_value_should_be(Double expected) { - FlagEvaluationDetails details = client.getDoubleDetails("float-zero-flag", this.doubleFlagDefaultValue); - assertEquals(expected, details.getValue()); - } - - // integer value - @When("a zero-value integer flag with key {string} is evaluated with default value {int}") - public void a_zero_value_integer_flag_with_key_is_evaluated_with_default_value( - String flagKey, Integer defaultValue) { - this.intFlagKey = flagKey; - this.intFlagDefaultValue = defaultValue; - } - - @Then("the resolved integer zero-value should be {int}") - public void the_resolved_integer_zero_value_should_be(Integer expected) { - int value = client.getIntegerValue(this.intFlagKey, this.intFlagDefaultValue); - assertEquals(expected, value); - } - - // string value - @When("a zero-value string flag with key {string} is evaluated with default value {string}") - public void a_zero_value_string_flag_with_key_is_evaluated_with_default_value(String flagKey, String defaultValue) { - this.stringFlagKey = flagKey; - this.stringFlagDefaultValue = defaultValue; - } - - @Then("the resolved string zero-value should be {string}") - public void the_resolved_string_zero_value_should_be(String expected) { - String value = client.getStringValue(this.stringFlagKey, this.stringFlagDefaultValue); - assertEquals(expected, value); - } - - @When("a context containing a targeting key with value {string}") - public void a_context_containing_a_targeting_key_with_value(String targetingKey) { - this.context = new ImmutableContext(targetingKey); - } - - @Then("the returned reason should be {string}") - public void the_returned_reason_should_be(String reason) { - FlagEvaluationDetails details = - client.getStringDetails(this.stringFlagKey, this.stringFlagDefaultValue, this.context); - assertEquals(reason, details.getReason()); - } -} 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 new file mode 100644 index 000000000..ea511158e --- /dev/null +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java @@ -0,0 +1,39 @@ +package dev.openfeature.contrib.providers.flagd.e2e.steps; + +import dev.openfeature.contrib.providers.flagd.Config; +import dev.openfeature.contrib.providers.flagd.resolver.grpc.cache.CacheType; + +import java.util.Objects; + +public final class Utils { + + private Utils() {} + + public static Object convert(String value, String type) throws ClassNotFoundException { + if (Objects.equals(value, "null")) return null; + switch (type) { + case "Boolean": + return Boolean.parseBoolean(value); + case "String": + return value; + case "Integer": + return Integer.parseInt(value); + case "Float": + return Double.parseDouble(value); + case "Long": + return Long.parseLong(value); + case "ResolverType": + switch (value.toLowerCase()) { + case "in-process": + return Config.Resolver.IN_PROCESS; + case "rpc": + return Config.Resolver.RPC; + default: + throw new RuntimeException("Unknown resolver type: " + value); + } + case "CacheType": + return CacheType.valueOf(value.toUpperCase()).getValue(); + } + throw new RuntimeException("Unknown config type: " + type); + } +} diff --git a/providers/flagd/test-harness b/providers/flagd/test-harness index 8931c8645..9488dfdb7 160000 --- a/providers/flagd/test-harness +++ b/providers/flagd/test-harness @@ -1 +1 @@ -Subproject commit 8931c8645b8600e251d5e3ebbad42dff8ce4c78e +Subproject commit 9488dfdb7c687538f7bba2a0c4e014d740c9033d From c506fea60f8dec151307a61764bdae4da642b7dc Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Tue, 14 Jan 2025 10:21:40 +0100 Subject: [PATCH 02/13] fixup: using toxiproxy Signed-off-by: Simon Schrottner --- providers/flagd/pom.xml | 6 ++ .../resolver/common/ConnectionEvent.java | 3 + .../providers/flagd/e2e/FlagdContainer.java | 14 ++-- .../providers/flagd/e2e/RunRpcTest.java | 7 +- .../contrib/providers/flagd/e2e/State.java | 4 +- .../providers/flagd/e2e/steps/EventSteps.java | 16 ++-- .../flagd/e2e/steps/ProviderSteps.java | 81 +++++++++++++------ .../test/resources/simplelogger.properties | 2 + 8 files changed, 90 insertions(+), 43 deletions(-) create mode 100644 providers/flagd/src/test/resources/simplelogger.properties diff --git a/providers/flagd/pom.xml b/providers/flagd/pom.xml index 935b1f83a..3fc308dd4 100644 --- a/providers/flagd/pom.xml +++ b/providers/flagd/pom.xml @@ -149,6 +149,12 @@ 1.20.4 test + + org.testcontainers + toxiproxy + 1.20.4 + test + diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionEvent.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionEvent.java index 0e8ff4c6b..38dda199f 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionEvent.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionEvent.java @@ -2,6 +2,7 @@ import dev.openfeature.sdk.ImmutableStructure; import dev.openfeature.sdk.Structure; +import lombok.Getter; import java.util.Collections; import java.util.List; @@ -16,6 +17,7 @@ public class ConnectionEvent { /** * The current state of the connection. */ + @Getter private final ConnectionState connected; /** @@ -121,4 +123,5 @@ public boolean isConnected() { public boolean isStale() { return this.connected == ConnectionState.STALE; } + } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java index 53bd91160..4da7830c6 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java @@ -1,18 +1,21 @@ package dev.openfeature.contrib.providers.flagd.e2e; +import com.github.dockerjava.api.command.PauseContainerCmd; +import com.github.dockerjava.api.command.SyncDockerCmd; +import com.github.dockerjava.api.command.UnpauseContainerCmd; import dev.openfeature.contrib.providers.flagd.Config; import org.apache.logging.log4j.util.Strings; import org.jetbrains.annotations.NotNull; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.Network; -import org.testcontainers.containers.wait.strategy.Wait; -import org.testcontainers.containers.wait.strategy.WaitStrategy; import org.testcontainers.utility.DockerImageName; import org.testcontainers.utility.MountableFile; import java.io.File; import java.nio.file.Files; import java.util.List; +import java.util.Timer; +import java.util.TimerTask; public class FlagdContainer extends GenericContainer { private static final String version; @@ -43,13 +46,6 @@ public FlagdContainer(String feature) { this.addExposedPorts(8013, 8014, 8015, 8016); } - @Override - public void start() { - if (!"socket".equals(this.feature)) - this.addExposedPorts(8013, 8014, 8015, 8016); - super.start(); - waitUntilContainerStarted(); - } public int getPort(Config.Resolver resolver) { waitUntilContainerStarted(); 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 20dc320bc..53d4f6d37 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 @@ -9,6 +9,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; @@ -22,11 +23,13 @@ @Order(value = Integer.MAX_VALUE) @Suite @IncludeEngines("cucumber") -@SelectDirectories("test-harness/gherkin") +//@SelectDirectories("test-harness/gherkin") +@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("rpc") +@IncludeTags({"rpc","reconnect"}) +@ExcludeTags({"targetURI", "customCert", "unixsocket"}) @Testcontainers public class RunRpcTest { 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 99126a583..256174a53 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 @@ -10,12 +10,14 @@ import dev.openfeature.sdk.MutableContext; import java.util.ArrayList; +import java.util.LinkedList; +import java.util.List; import java.util.Optional; public class State { public ProviderType providerType; public Client client; - public ArrayList events = new ArrayList<>(); + public List events = new LinkedList<>(); public Optional lastEvent; public FlagSteps.Flag flag; public MutableContext context diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java index 67b24b155..0288fc370 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java @@ -7,25 +7,29 @@ import io.cucumber.java.en.When; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.parallel.Isolated; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.util.ArrayList; +import java.util.LinkedList; import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.awaitility.Awaitility.await; @Isolated() public class EventSteps extends AbstractSteps { + private static final Logger LOG = LoggerFactory.getLogger(EventSteps.class); public EventSteps(State state) { super(state); - state.events = new ArrayList<>(); + state.events = new LinkedList<>(); } @Given("a {} event handler") public void a_stale_event_handler(String eventType) { state.client.on(mapEventType(eventType), eventDetails -> { - System.out.println(eventType); + LOG.info("event tracked for {} ", eventType); state.events.add(new Event(eventType, eventDetails)); }); } @@ -52,17 +56,15 @@ public void eventWasFired(String eventType) { @Then("the {} event handler should have been executed") public void eventHandlerShouldBeExecuted(String eventType) { - eventHandlerShouldBeExecutedWithin(eventType, 10000); + eventHandlerShouldBeExecutedWithin(eventType, 30000); } @Then("the {} event handler should have been executed within {int}ms") public void eventHandlerShouldBeExecutedWithin(String eventType, int ms) { + LOG.info("waiting for eventtype: {}", eventType); await() .atMost(ms, MILLISECONDS) - .until(() -> { - state.events.forEach((e) -> System.out.println(e.type)); - return state.events.stream().anyMatch(event -> event.type.equals(eventType)); - }); + .until(() -> state.events.stream().anyMatch(event -> event.type.equals(eventType))); state.lastEvent = state.events.stream().filter(event -> event.type.equals(eventType)).findFirst(); state.events.removeIf(event -> event.type.equals(eventType)); } 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 17ae53025..2b6aebb03 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 @@ -1,11 +1,15 @@ package dev.openfeature.contrib.providers.flagd.e2e.steps; +import dev.openfeature.contrib.providers.flagd.Config; import dev.openfeature.contrib.providers.flagd.FlagdProvider; import dev.openfeature.contrib.providers.flagd.e2e.FlagdContainer; import dev.openfeature.contrib.providers.flagd.e2e.State; -import dev.openfeature.sdk.Client; import dev.openfeature.sdk.FeatureProvider; import dev.openfeature.sdk.OpenFeatureAPI; +import eu.rekawek.toxiproxy.Proxy; +import eu.rekawek.toxiproxy.ToxiproxyClient; +import eu.rekawek.toxiproxy.model.ToxicDirection; +import eu.rekawek.toxiproxy.model.toxic.Timeout; import io.cucumber.java.After; import io.cucumber.java.AfterAll; import io.cucumber.java.Before; @@ -17,6 +21,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.testcontainers.containers.BindMode; +import org.testcontainers.containers.Network; +import org.testcontainers.containers.ToxiproxyContainer; import org.testcontainers.shaded.org.apache.commons.io.FileUtils; import java.io.File; @@ -36,6 +42,11 @@ public class ProviderSteps extends AbstractSteps { public static final int UNAVAILABLE_PORT = 9999; static Map containers = new HashMap<>(); + static Map> proxyports = new HashMap<>(); + public static Network network = Network.newNetwork(); + public static ToxiproxyContainer toxiproxy = new ToxiproxyContainer("ghcr.io/shopify/toxiproxy:2.5.0") + .withNetwork(network).withCreateContainerCmdModifier((cmd -> cmd.withName("toxiproxy"))); + public static ToxiproxyClient toxiproxyClient; static Path sharedTempDir; @@ -52,13 +63,32 @@ public ProviderSteps(State state) { super(state); } + static String generateProxyName(Config.Resolver resolver, ProviderType providerType) { + return providerType + "-" + resolver; + } + @BeforeAll public static void beforeAll() throws IOException { - containers.put(ProviderType.DEFAULT, new FlagdContainer()); - containers.put(ProviderType.SSL, new FlagdContainer("ssl")); + toxiproxy.start(); + toxiproxyClient = new ToxiproxyClient(toxiproxy.getHost(), toxiproxy.getControlPort()); + toxiproxyClient.createProxy( + generateProxyName(Config.Resolver.RPC, ProviderType.DEFAULT), + "0.0.0.0:8666", "default:8013"); + + toxiproxyClient.createProxy(generateProxyName(Config.Resolver.IN_PROCESS, ProviderType.DEFAULT), "0.0.0.0:8667", "default:8015"); + toxiproxyClient.createProxy(generateProxyName(Config.Resolver.RPC, ProviderType.SSL), "0.0.0.0:8668", "ssl:8013"); + toxiproxyClient.createProxy(generateProxyName(Config.Resolver.IN_PROCESS, ProviderType.SSL), "0.0.0.0:8669", "ssl:8015"); + + containers.put(ProviderType.DEFAULT, + new FlagdContainer().withNetwork(network).withNetworkAliases("default") + ); + containers.put(ProviderType.SSL, + new FlagdContainer("ssl").withNetwork(network).withNetworkAliases("ssl") + ); containers.put(ProviderType.SOCKET, new FlagdContainer("socket") .withFileSystemBind(sharedTempDir.toAbsolutePath().toString(), "/tmp", BindMode.READ_WRITE)); + } @AfterAll @@ -69,16 +99,19 @@ public static void afterAll() throws IOException { } @Before - public void before() { + public void before() throws IOException { + containers.values().stream().filter(containers -> !containers.isRunning()) .forEach(FlagdContainer::start); } + @After public void tearDown() { OpenFeatureAPI.getInstance().shutdown(); } + @Given("a {} flagd provider") public void setupProvider(String providerType) { state.builder @@ -106,14 +139,14 @@ public void setupProvider(String providerType) { this.state.providerType = ProviderType.SSL; state .builder - .port(getContainer().getPort(State.resolverType)) + .port(getContainer(state.providerType).getPort(State.resolverType)) .tls(true) .certPath(absolutePath); break; default: this.state.providerType = ProviderType.DEFAULT; - state.builder.port(getContainer().getPort(State.resolverType)); + state.builder.port(toxiproxy.getMappedPort(8666)); break; } FeatureProvider provider = new FlagdProvider(state.builder @@ -130,30 +163,30 @@ public void setupProvider(String providerType) { } @When("the connection is lost for {int}s") - public void the_connection_is_lost_for(int seconds) throws InterruptedException { - FlagdContainer container = getContainer(); - -/* TimerTask task = new TimerTask() { + public void the_connection_is_lost_for(int seconds) throws InterruptedException, IOException { + LOG.info("Timeout and wait for {} seconds", seconds); + Proxy proxy = toxiproxyClient.getProxy(generateProxyName(State.resolverType, state.providerType)); + Timeout restart = proxy + .toxics() + .timeout("restart", ToxicDirection.UPSTREAM, seconds); + + TimerTask task = new TimerTask() { public void run() { - container.start(); - int port = container.getPort(State.resolverType); + try { + proxy.toxics().get("restart").remove(); + } catch (IOException e) { + throw new RuntimeException(e); + } } }; - Timer timer = new Timer("Timer");*/ - - LOG.info("stopping container for {}", state.providerType); - container.stop(); + Timer timer = new Timer("Timer"); - //timer.schedule(task, seconds * 1000L); - Thread.sleep(seconds * 1000L); + timer.schedule(task, seconds * 1000L); - LOG.info("starting container for {}", state.providerType); - container.start(); } - private FlagdContainer getContainer() { - LOG.info("getting container for {}", state.providerType); - System.out.println("getting container for " + state.providerType); - return containers.getOrDefault(state.providerType, containers.get(ProviderType.DEFAULT)); + static FlagdContainer getContainer(ProviderType providerType) { + LOG.info("getting container for {}", providerType); + return containers.getOrDefault(providerType, containers.get(ProviderType.DEFAULT)); } } diff --git a/providers/flagd/src/test/resources/simplelogger.properties b/providers/flagd/src/test/resources/simplelogger.properties new file mode 100644 index 000000000..55fc08759 --- /dev/null +++ b/providers/flagd/src/test/resources/simplelogger.properties @@ -0,0 +1,2 @@ +org.slf4j.simpleLogger.defaultLogLevel=debug +org.slf4j.simpleLogger.logFile=System.out From 381c1b67a23949fea8a7e29f644fa62456b55478 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Tue, 14 Jan 2025 10:21:40 +0100 Subject: [PATCH 03/13] fixup: using toxiproxy Signed-off-by: Simon Schrottner --- .../providers/flagd/e2e/FlagdContainer.java | 24 ++---- .../providers/flagd/e2e/RunInProcessTest.java | 10 +-- .../providers/flagd/e2e/RunRpcTest.java | 17 ++-- .../contrib/providers/flagd/e2e/State.java | 9 +-- .../flagd/e2e/steps/AbstractSteps.java | 1 + .../flagd/e2e/steps/ConfigSteps.java | 8 +- .../flagd/e2e/steps/ContextSteps.java | 14 ++-- .../providers/flagd/e2e/steps/EventSteps.java | 12 ++- .../providers/flagd/e2e/steps/FlagSteps.java | 25 +++--- .../flagd/e2e/steps/ProviderSteps.java | 77 +++++++++++-------- .../providers/flagd/e2e/steps/Utils.java | 1 - 11 files changed, 95 insertions(+), 103 deletions(-) diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java index 4da7830c6..9fd06108c 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java @@ -4,6 +4,9 @@ import com.github.dockerjava.api.command.SyncDockerCmd; import com.github.dockerjava.api.command.UnpauseContainerCmd; import dev.openfeature.contrib.providers.flagd.Config; +import java.io.File; +import java.nio.file.Files; +import java.util.List; import org.apache.logging.log4j.util.Strings; import org.jetbrains.annotations.NotNull; import org.testcontainers.containers.GenericContainer; @@ -42,24 +45,9 @@ public FlagdContainer(String feature) { super(generateContainerName(feature)); this.withReuse(true); this.feature = feature; - if (!"socket".equals(this.feature)) - this.addExposedPorts(8013, 8014, 8015, 8016); - } - - - public int getPort(Config.Resolver resolver) { - waitUntilContainerStarted(); - switch (resolver) { - case RPC: - return this.getMappedPort(8013); - case IN_PROCESS: - return this.getMappedPort(8015); - default: - throw new IllegalArgumentException("Unsupported resolver: " + resolver); - } + if (!"socket".equals(this.feature)) this.addExposedPorts(8013, 8014, 8015, 8016); } - /** * @return a {@link org.testcontainers.containers.GenericContainer} instance of envoy container using * flagd sync service as backend expose on port 9211 @@ -67,8 +55,8 @@ public int getPort(Config.Resolver resolver) { public static GenericContainer envoy() { final String container = "envoyproxy/envoy:v1.31.0"; return new GenericContainer(DockerImageName.parse(container)) - .withCopyFileToContainer(MountableFile.forClasspathResource("/envoy-config/envoy-custom.yaml"), - "/etc/envoy/envoy.yaml") + .withCopyFileToContainer( + MountableFile.forClasspathResource("/envoy-config/envoy-custom.yaml"), "/etc/envoy/envoy.yaml") .withExposedPorts(9211) .withNetwork(network) .withNetworkAliases("envoy"); diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java index 12c4a66d4..cf89d69f9 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java @@ -1,7 +1,10 @@ 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 io.cucumber.junit.platform.engine.Constants; import org.apache.logging.log4j.core.config.Order; import org.junit.platform.suite.api.BeforeSuite; import org.junit.platform.suite.api.ConfigurationParameter; @@ -12,10 +15,6 @@ 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 */ @@ -36,4 +35,3 @@ public static void before() { State.resolverType = Config.Resolver.IN_PROCESS; } } - 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 53d4f6d37..eb7c00491 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 @@ -1,34 +1,33 @@ 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 io.cucumber.junit.platform.engine.Constants; 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.SelectFile; import org.junit.platform.suite.api.SelectFile; 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") +// @SelectDirectories("test-harness/gherkin") @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({"rpc","reconnect"}) +@IncludeTags({"rpc", "reconnect"}) @ExcludeTags({"targetURI", "customCert", "unixsocket"}) @Testcontainers public class RunRpcTest { @@ -37,6 +36,4 @@ public class RunRpcTest { public static void before() { State.resolverType = Config.Resolver.RPC; } - } - 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 256174a53..c62fe4808 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 @@ -8,8 +8,8 @@ import dev.openfeature.sdk.Client; import dev.openfeature.sdk.FlagEvaluationDetails; import dev.openfeature.sdk.MutableContext; - -import java.util.ArrayList; +import java.util.LinkedList; +import java.util.List; import java.util.LinkedList; import java.util.List; import java.util.Optional; @@ -20,12 +20,9 @@ public class State { public List events = new LinkedList<>(); public Optional lastEvent; public FlagSteps.Flag flag; - public MutableContext context - = new MutableContext(); + public MutableContext context = new MutableContext(); public FlagEvaluationDetails evaluation; public FlagdOptions options; public FlagdOptions.FlagdOptionsBuilder builder = FlagdOptions.builder(); public static Config.Resolver resolverType; - - } 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 c712bf242..133c1fb49 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 @@ -4,6 +4,7 @@ abstract class AbstractSteps { State state; + public 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 a319ec543..ab79713be 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 @@ -14,12 +14,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static org.assertj.core.api.Assertions.assertThat; - public class ConfigSteps extends AbstractSteps { /** * Not all properties are correctly implemented, hence that we need to ignore them till this is @@ -34,7 +31,6 @@ public class ConfigSteps extends AbstractSteps { private static final Logger LOG = LoggerFactory.getLogger(ConfigSteps.class); - public ConfigSteps(State state) { super(state); } @@ -48,7 +44,8 @@ public void we_initialize_a_config() { public void we_initialize_a_config_for(String string) { switch (string.toLowerCase()) { case "in-process": - state.options = state.builder.resolverType(Config.Resolver.IN_PROCESS).build(); + state.options = + state.builder.resolverType(Config.Resolver.IN_PROCESS).build(); break; case "rpc": state.options = state.builder.resolverType(Config.Resolver.RPC).build(); @@ -84,7 +81,6 @@ public void we_have_an_environment_variable_with_value(String varName, String va EnvironmentVariableUtils.set(varName, value); } - @Then("the option {string} of type {string} should have the value {string}") public void the_option_of_type_should_have_the_value(String option, String type, String value) throws Throwable { Object convert = Utils.convert(value, type); diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ContextSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ContextSteps.java index c8309a3b4..9de541e23 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ContextSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ContextSteps.java @@ -5,24 +5,20 @@ import dev.openfeature.sdk.MutableContext; import dev.openfeature.sdk.Value; import io.cucumber.java.en.Given; -import org.junit.jupiter.api.parallel.Isolated; - import java.util.HashMap; import java.util.Map; - -import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import org.junit.jupiter.api.parallel.Isolated; @Isolated() public class ContextSteps extends AbstractSteps { - public ContextSteps(State state) { super(state); } - @Given("a context containing a key {string}, with type {string} and with value {string}") - public void a_context_containing_a_key_with_type_and_with_value(String key, String type, String value) throws ClassNotFoundException, InstantiationException { + public void a_context_containing_a_key_with_type_and_with_value(String key, String type, String value) + throws ClassNotFoundException, InstantiationException { Map map = state.context.asMap(); map.put(key, new Value(value)); state.context = new MutableContext(state.context.getTargetingKey(), map); @@ -34,10 +30,10 @@ public void a_context_containing_a_targeting_key_with_value(String string) { } @Given("a context containing a nested property with outer key {string} and inner key {string}, with value {string}") - public void a_context_containing_a_nested_property_with_outer_key_and_inner_key_with_value(String outer, String inner, String value) { + public void a_context_containing_a_nested_property_with_outer_key_and_inner_key_with_value( + String outer, String inner, String value) { Map innerMap = new HashMap<>(); innerMap.put(inner, new Value(value)); state.context.add(outer, new ImmutableStructure(innerMap)); } - } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java index 0288fc370..785b0322d 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java @@ -1,10 +1,14 @@ package dev.openfeature.contrib.providers.flagd.e2e.steps; +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static org.awaitility.Awaitility.await; + import dev.openfeature.contrib.providers.flagd.e2e.State; import dev.openfeature.sdk.ProviderEvent; import io.cucumber.java.en.Given; import io.cucumber.java.en.Then; import io.cucumber.java.en.When; +import java.util.LinkedList; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.parallel.Isolated; import org.slf4j.Logger; @@ -20,7 +24,6 @@ public class EventSteps extends AbstractSteps { private static final Logger LOG = LoggerFactory.getLogger(EventSteps.class); - public EventSteps(State state) { super(state); state.events = new LinkedList<>(); @@ -62,10 +65,11 @@ public void eventHandlerShouldBeExecuted(String eventType) { @Then("the {} event handler should have been executed within {int}ms") public void eventHandlerShouldBeExecutedWithin(String eventType, int ms) { LOG.info("waiting for eventtype: {}", eventType); - await() - .atMost(ms, MILLISECONDS) + await().atMost(ms, MILLISECONDS) .until(() -> state.events.stream().anyMatch(event -> event.type.equals(eventType))); - state.lastEvent = state.events.stream().filter(event -> event.type.equals(eventType)).findFirst(); + state.lastEvent = state.events.stream() + .filter(event -> event.type.equals(eventType)) + .findFirst(); state.events.removeIf(event -> event.type.equals(eventType)); } } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java index 4b193af3e..b27cb7d55 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java @@ -1,5 +1,7 @@ package dev.openfeature.contrib.providers.flagd.e2e.steps; +import static org.assertj.core.api.Assertions.assertThat; + import dev.openfeature.contrib.providers.flagd.e2e.State; import dev.openfeature.sdk.FlagEvaluationDetails; import dev.openfeature.sdk.Value; @@ -8,13 +10,9 @@ import io.cucumber.java.en.When; import org.junit.jupiter.api.parallel.Isolated; -import static org.assertj.core.api.Assertions.assertThat; - - @Isolated() public class FlagSteps extends AbstractSteps { - public FlagSteps(State state) { super(state); } @@ -22,7 +20,6 @@ public FlagSteps(State state) { @Given("a {}-flag with key {string} and a default value {string}") public void givenAFlag(String type, String name, String defaultValue) throws ClassNotFoundException { state.flag = new Flag(type, name, Utils.convert(defaultValue, type)); - } @When("the flag was evaluated with details") @@ -30,19 +27,24 @@ public void the_flag_was_evaluated_with_details() { FlagEvaluationDetails details; switch (state.flag.type) { case "String": - details = state.client.getStringDetails(state.flag.name, (String) state.flag.defaultValue, state.context); + details = + state.client.getStringDetails(state.flag.name, (String) state.flag.defaultValue, state.context); break; case "Boolean": - details = state.client.getBooleanDetails(state.flag.name, (Boolean) state.flag.defaultValue, state.context); + details = state.client.getBooleanDetails( + state.flag.name, (Boolean) state.flag.defaultValue, state.context); break; case "Float": - details = state.client.getDoubleDetails(state.flag.name, (Double) state.flag.defaultValue, state.context); + details = + state.client.getDoubleDetails(state.flag.name, (Double) state.flag.defaultValue, state.context); break; case "Integer": - details = state.client.getIntegerDetails(state.flag.name, (Integer) state.flag.defaultValue, state.context); + details = state.client.getIntegerDetails( + state.flag.name, (Integer) state.flag.defaultValue, state.context); break; case "Object": - details = state.client.getObjectDetails(state.flag.name, (Value) state.flag.defaultValue, state.context); + details = + state.client.getObjectDetails(state.flag.name, (Value) state.flag.defaultValue, state.context); break; default: throw new AssertionError(); @@ -59,10 +61,12 @@ public void the_resolved_details_value_should_be(String value) throws ClassNotFo public void the_reason_should_be(String reason) { assertThat(state.evaluation.getReason()).isEqualTo(reason); } + @Then("the variant should be {string}") public void the_variant_should_be(String variant) { assertThat(state.evaluation.getVariant()).isEqualTo(variant); } + @Then("the flag was modified") public void the_flag_was_modified() { assertThat(state.lastEvent).isPresent().hasValueSatisfying((event) -> { @@ -71,7 +75,6 @@ public void the_flag_was_modified() { }); } - public class Flag { String name; Object defaultValue; 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 2b6aebb03..23084ff93 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 @@ -34,6 +34,14 @@ import java.util.Map; import java.util.Timer; import java.util.TimerTask; +import org.apache.commons.lang3.RandomStringUtils; +import org.junit.jupiter.api.parallel.Isolated; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.testcontainers.containers.BindMode; +import org.testcontainers.containers.Network; +import org.testcontainers.containers.ToxiproxyContainer; +import org.testcontainers.shaded.org.apache.commons.io.FileUtils; @Isolated() public class ProviderSteps extends AbstractSteps { @@ -45,7 +53,8 @@ public class ProviderSteps extends AbstractSteps { static Map> proxyports = new HashMap<>(); public static Network network = Network.newNetwork(); public static ToxiproxyContainer toxiproxy = new ToxiproxyContainer("ghcr.io/shopify/toxiproxy:2.5.0") - .withNetwork(network).withCreateContainerCmdModifier((cmd -> cmd.withName("toxiproxy"))); + .withNetwork(network) + .withCreateContainerCmdModifier((cmd -> cmd.withName("toxiproxy"))); public static ToxiproxyClient toxiproxyClient; static Path sharedTempDir; @@ -72,23 +81,23 @@ public static void beforeAll() throws IOException { toxiproxy.start(); toxiproxyClient = new ToxiproxyClient(toxiproxy.getHost(), toxiproxy.getControlPort()); toxiproxyClient.createProxy( - generateProxyName(Config.Resolver.RPC, ProviderType.DEFAULT), - "0.0.0.0:8666", "default:8013"); - - toxiproxyClient.createProxy(generateProxyName(Config.Resolver.IN_PROCESS, ProviderType.DEFAULT), "0.0.0.0:8667", "default:8015"); - toxiproxyClient.createProxy(generateProxyName(Config.Resolver.RPC, ProviderType.SSL), "0.0.0.0:8668", "ssl:8013"); - toxiproxyClient.createProxy(generateProxyName(Config.Resolver.IN_PROCESS, ProviderType.SSL), "0.0.0.0:8669", "ssl:8015"); - - containers.put(ProviderType.DEFAULT, - new FlagdContainer().withNetwork(network).withNetworkAliases("default") - ); - containers.put(ProviderType.SSL, - new FlagdContainer("ssl").withNetwork(network).withNetworkAliases("ssl") - ); - containers.put(ProviderType.SOCKET, new FlagdContainer("socket") - .withFileSystemBind(sharedTempDir.toAbsolutePath().toString(), "/tmp", BindMode.READ_WRITE)); - + generateProxyName(Config.Resolver.RPC, ProviderType.DEFAULT), "0.0.0.0:8666", "default:8013"); + toxiproxyClient.createProxy( + generateProxyName(Config.Resolver.IN_PROCESS, ProviderType.DEFAULT), "0.0.0.0:8667", "default:8015"); + toxiproxyClient.createProxy( + generateProxyName(Config.Resolver.RPC, ProviderType.SSL), "0.0.0.0:8668", "ssl:8013"); + toxiproxyClient.createProxy( + generateProxyName(Config.Resolver.IN_PROCESS, ProviderType.SSL), "0.0.0.0:8669", "ssl:8015"); + + containers.put( + ProviderType.DEFAULT, new FlagdContainer().withNetwork(network).withNetworkAliases("default")); + containers.put( + ProviderType.SSL, new FlagdContainer("ssl").withNetwork(network).withNetworkAliases("ssl")); + containers.put( + ProviderType.SOCKET, + new FlagdContainer("socket") + .withFileSystemBind(sharedTempDir.toAbsolutePath().toString(), "/tmp", BindMode.READ_WRITE)); } @AfterAll @@ -101,7 +110,8 @@ public static void afterAll() throws IOException { @Before public void before() throws IOException { - containers.values().stream().filter(containers -> !containers.isRunning()) + containers.values().stream() + .filter(containers -> !containers.isRunning()) .forEach(FlagdContainer::start); } @@ -110,14 +120,21 @@ public void tearDown() { OpenFeatureAPI.getInstance().shutdown(); } + public int getPort(Config.Resolver resolver, ProviderType providerType) { + switch (resolver) { + case RPC: + return this.getMappedPort(8013); + case IN_PROCESS: + return this.getMappedPort(8015); + default: + throw new IllegalArgumentException("Unsupported resolver: " + resolver); + } + } @Given("a {} flagd provider") public void setupProvider(String providerType) { - state.builder - .deadline(500) - .keepAlive(0) - .retryGracePeriod(1); + state.builder.deadline(500).keepAlive(0).retryGracePeriod(1); boolean wait = true; switch (providerType) { case "unavailable": @@ -127,7 +144,8 @@ public void setupProvider(String providerType) { break; case "socket": this.state.providerType = ProviderType.SOCKET; - String socketPath = sharedTempDir.resolve("socket.sock").toAbsolutePath().toString(); + String socketPath = + sharedTempDir.resolve("socket.sock").toAbsolutePath().toString(); state.builder.socketPath(socketPath); state.builder.port(UNAVAILABLE_PORT); break; @@ -137,8 +155,7 @@ public void setupProvider(String providerType) { File file = new File(path); String absolutePath = file.getAbsolutePath(); this.state.providerType = ProviderType.SSL; - state - .builder + state.builder .port(getContainer(state.providerType).getPort(State.resolverType)) .tls(true) .certPath(absolutePath); @@ -149,9 +166,8 @@ public void setupProvider(String providerType) { state.builder.port(toxiproxy.getMappedPort(8666)); break; } - FeatureProvider provider = new FlagdProvider(state.builder - .resolverType(State.resolverType) - .build()); + FeatureProvider provider = + new FlagdProvider(state.builder.resolverType(State.resolverType).build()); OpenFeatureAPI api = OpenFeatureAPI.getInstance(); if (wait) { @@ -166,9 +182,7 @@ public void setupProvider(String providerType) { public void the_connection_is_lost_for(int seconds) throws InterruptedException, IOException { LOG.info("Timeout and wait for {} seconds", seconds); Proxy proxy = toxiproxyClient.getProxy(generateProxyName(State.resolverType, state.providerType)); - Timeout restart = proxy - .toxics() - .timeout("restart", ToxicDirection.UPSTREAM, seconds); + Timeout restart = proxy.toxics().timeout("restart", ToxicDirection.UPSTREAM, seconds); TimerTask task = new TimerTask() { public void run() { @@ -182,7 +196,6 @@ public void run() { Timer timer = new Timer("Timer"); timer.schedule(task, seconds * 1000L); - } static FlagdContainer getContainer(ProviderType providerType) { 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 ea511158e..b4f8dd429 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 @@ -2,7 +2,6 @@ import dev.openfeature.contrib.providers.flagd.Config; import dev.openfeature.contrib.providers.flagd.resolver.grpc.cache.CacheType; - import java.util.Objects; public final class Utils { From 0fe3c4f0140bb942efef27811cc3bbce02cf33cf Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Tue, 14 Jan 2025 12:17:32 +0100 Subject: [PATCH 04/13] fixup: using toxiproxy Signed-off-by: Simon Schrottner --- .../contrib/providers/flagd/e2e/FlagdContainer.java | 10 ---------- .../contrib/providers/flagd/e2e/RunRpcTest.java | 1 - .../openfeature/contrib/providers/flagd/e2e/State.java | 2 -- .../contrib/providers/flagd/e2e/steps/EventSteps.java | 8 +------- .../providers/flagd/e2e/steps/ProviderSteps.java | 10 ---------- 5 files changed, 1 insertion(+), 30 deletions(-) diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java index 9fd06108c..0aa8f50d0 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/FlagdContainer.java @@ -1,9 +1,5 @@ package dev.openfeature.contrib.providers.flagd.e2e; -import com.github.dockerjava.api.command.PauseContainerCmd; -import com.github.dockerjava.api.command.SyncDockerCmd; -import com.github.dockerjava.api.command.UnpauseContainerCmd; -import dev.openfeature.contrib.providers.flagd.Config; import java.io.File; import java.nio.file.Files; import java.util.List; @@ -14,12 +10,6 @@ import org.testcontainers.utility.DockerImageName; import org.testcontainers.utility.MountableFile; -import java.io.File; -import java.nio.file.Files; -import java.util.List; -import java.util.Timer; -import java.util.TimerTask; - public class FlagdContainer extends GenericContainer { private static final String version; private static final Network network = Network.newNetwork(); 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 eb7c00491..ceb8f0934 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.SelectFile; -import org.junit.platform.suite.api.SelectFile; import org.junit.platform.suite.api.Suite; import org.testcontainers.junit.jupiter.Testcontainers; 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 c62fe4808..4ecab84e5 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 @@ -10,8 +10,6 @@ import dev.openfeature.sdk.MutableContext; import java.util.LinkedList; import java.util.List; -import java.util.LinkedList; -import java.util.List; import java.util.Optional; public class State { diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java index 785b0322d..cbf3d250e 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java @@ -14,12 +14,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; -import java.util.LinkedList; - -import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static org.awaitility.Awaitility.await; - @Isolated() public class EventSteps extends AbstractSteps { private static final Logger LOG = LoggerFactory.getLogger(EventSteps.class); @@ -32,7 +26,7 @@ public EventSteps(State state) { @Given("a {} event handler") public void a_stale_event_handler(String eventType) { state.client.on(mapEventType(eventType), eventDetails -> { - LOG.info("event tracked for {} ", eventType); + LOG.info("event tracked for {} ", eventType); state.events.add(new Event(eventType, eventDetails)); }); } 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 23084ff93..62eb92e68 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 @@ -16,15 +16,6 @@ import io.cucumber.java.BeforeAll; import io.cucumber.java.en.Given; import io.cucumber.java.en.When; -import org.apache.commons.lang3.RandomStringUtils; -import org.junit.jupiter.api.parallel.Isolated; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.testcontainers.containers.BindMode; -import org.testcontainers.containers.Network; -import org.testcontainers.containers.ToxiproxyContainer; -import org.testcontainers.shaded.org.apache.commons.io.FileUtils; - import java.io.File; import java.io.IOException; import java.nio.file.Files; @@ -131,7 +122,6 @@ public int getPort(Config.Resolver resolver, ProviderType providerType) { } } - @Given("a {} flagd provider") public void setupProvider(String providerType) { state.builder.deadline(500).keepAlive(0).retryGracePeriod(1); From 82db146058eecd5ac4f5cdb3456103ae3e7cd349 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Tue, 14 Jan 2025 20:32:58 +0100 Subject: [PATCH 05/13] fixup: fixing rpc mode - in-process still buggy on reconnect Signed-off-by: Simon Schrottner --- providers/flagd/pom.xml | 6 +- .../resolver/common/ConnectionEvent.java | 9 +++ .../flagd/resolver/common/GrpcConnector.java | 5 +- .../resolver/grpc/EventStreamObserver.java | 57 +++++-------------- .../flagd/resolver/grpc/GrpcResolver.java | 20 +++++-- .../flagd/e2e/RunConfigCucumberTest.java | 2 +- .../providers/flagd/e2e/RunInProcessTest.java | 2 +- .../providers/flagd/e2e/RunRpcTest.java | 9 +-- .../flagd/e2e/steps/ConfigSteps.java | 3 +- .../providers/flagd/e2e/steps/FlagSteps.java | 21 ++++--- .../flagd/e2e/steps/ProviderSteps.java | 55 ++++++++++++++---- .../providers/flagd/e2e/steps/Utils.java | 8 ++- .../grpc/EventStreamObserverTest.java | 4 +- .../test/resources/simplelogger.properties | 2 - 14 files changed, 118 insertions(+), 85 deletions(-) delete mode 100644 providers/flagd/src/test/resources/simplelogger.properties diff --git a/providers/flagd/pom.xml b/providers/flagd/pom.xml index 3fc308dd4..0ee1ce333 100644 --- a/providers/flagd/pom.xml +++ b/providers/flagd/pom.xml @@ -155,7 +155,11 @@ 1.20.4 test - + + org.slf4j + slf4j-simple + 2.0.16 + diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionEvent.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionEvent.java index 38dda199f..58b878d6a 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionEvent.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionEvent.java @@ -114,6 +114,15 @@ public boolean isConnected() { return this.connected == ConnectionState.CONNECTED; } + /** + * Indicates + * whether the current connection state is disconnected. + * + * @return {@code true} if disconnected, otherwise {@code false}. + */ + public boolean isDisconnected() { + return this.connected == ConnectionState.DISCONNECTED; + } /** * Indicates * whether the current connection state is stale. diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnector.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnector.java index d5ca69aff..bac93e880 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnector.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnector.java @@ -101,8 +101,8 @@ public GrpcConnector( ManagedChannel channel) { this.channel = channel; - this.serviceStub = stub.apply(channel); - this.blockingStub = blockingStub.apply(channel); + this.serviceStub = stub.apply(channel).withWaitForReady(); + this.blockingStub = blockingStub.apply(channel).withWaitForReady(); this.deadline = options.getDeadline(); this.streamDeadlineMs = options.getStreamDeadlineMs(); this.onConnectionEvent = onConnectionEvent; @@ -190,7 +190,6 @@ private synchronized void onReady() { log.debug("Reconnection task cancelled as connection became READY."); } restartStream(); - this.onConnectionEvent.accept(new ConnectionEvent(true)); } /** diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserver.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserver.java index db89931e5..705115e5e 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserver.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserver.java @@ -1,6 +1,7 @@ package dev.openfeature.contrib.providers.flagd.resolver.grpc; import com.google.protobuf.Value; +import dev.openfeature.contrib.providers.flagd.resolver.common.ConnectionEvent; import dev.openfeature.contrib.providers.flagd.resolver.grpc.cache.Cache; import dev.openfeature.flagd.grpc.evaluation.Evaluation.EventStreamResponse; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; @@ -10,6 +11,7 @@ import java.util.List; import java.util.Map; import java.util.function.BiConsumer; +import java.util.function.Consumer; import lombok.extern.slf4j.Slf4j; /** @@ -20,25 +22,18 @@ @SuppressFBWarnings(justification = "cache needs to be read and write by multiple objects") class EventStreamObserver implements StreamObserver { - /** - * A consumer to handle connection events with a flag indicating success and a list of changed flags. - */ - private final BiConsumer> onConnectionEvent; - /** - * The cache to update based on received events. - */ - private final Cache cache; + private final Consumer> onConfigurationChange; + private final Consumer onReady; /** * Constructs a new {@code EventStreamObserver} instance. * - * @param cache the cache to update based on received events * @param onConnectionEvent a consumer to handle connection events with a boolean and a list of changed flags */ - EventStreamObserver(Cache cache, BiConsumer> onConnectionEvent) { - this.cache = cache; - this.onConnectionEvent = onConnectionEvent; + EventStreamObserver(Consumer> onConfigurationChange, Consumer onReady) { + this.onConfigurationChange = onConfigurationChange; + this.onReady = onReady; } /** @@ -60,27 +55,14 @@ public void onNext(EventStreamResponse value) { } } - /** - * Called when an error occurs in the stream. - * - * @param throwable the error that occurred - */ @Override public void onError(Throwable throwable) { - if (this.cache.getEnabled().equals(Boolean.TRUE)) { - this.cache.clear(); - } + } - /** - * Called when the stream is completed. - */ @Override public void onCompleted() { - if (this.cache.getEnabled().equals(Boolean.TRUE)) { - this.cache.clear(); - } - this.onConnectionEvent.accept(false, Collections.emptyList()); + } /** @@ -90,33 +72,22 @@ public void onCompleted() { */ private void handleConfigurationChangeEvent(EventStreamResponse value) { List changedFlags = new ArrayList<>(); - boolean cachingEnabled = this.cache.getEnabled(); Map data = value.getData().getFieldsMap(); Value flagsValue = data.get(Constants.FLAGS_KEY); - if (flagsValue == null) { - if (cachingEnabled) { - this.cache.clear(); - } - } else { + if (flagsValue != null) { Map flags = flagsValue.getStructValue().getFieldsMap(); - for (String flagKey : flags.keySet()) { - changedFlags.add(flagKey); - if (cachingEnabled) { - this.cache.remove(flagKey); - } - } + changedFlags.addAll(flags.keySet()); } - this.onConnectionEvent.accept(true, changedFlags); + onConfigurationChange.accept(changedFlags); } /** * Handles provider readiness events by clearing the cache (if enabled) and notifying listeners of readiness. */ private void handleProviderReadyEvent() { - if (this.cache.getEnabled().equals(Boolean.TRUE)) { - this.cache.clear(); - } + log.info("Received provider ready event"); + onReady.accept(new ConnectionEvent(true)); } } 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 5c8ad3ea1..dfc6047cb 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 @@ -67,13 +67,23 @@ public GrpcResolver( options, ServiceGrpc::newStub, ServiceGrpc::newBlockingStub, - onConnectionEvent, + (event) -> { + if( cache != null && event.isDisconnected()) { + cache.clear(); + } + onConnectionEvent.accept(event); + }, stub -> stub.eventStream( Evaluation.EventStreamRequest.getDefaultInstance(), new EventStreamObserver( - cache, - (k, e) -> - onConnectionEvent.accept(new ConnectionEvent(ConnectionState.CONNECTED, e))))); + (flags) -> { + if( cache != null) { + flags.forEach(cache::remove); + } + onConnectionEvent.accept(new ConnectionEvent(ConnectionState.CONNECTED, flags)); + }, + onConnectionEvent + ))); } /** @@ -207,7 +217,7 @@ private Boolean isEvaluationCacheable(ProviderEvaluation evaluation) { } private Boolean cacheAvailable() { - return this.cache.getEnabled() && this.connector.isConnected(); + return this.cache.getEnabled(); } private static ImmutableMetadata metadataFromResponse(Message response) { 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/e2e/RunConfigCucumberTest.java index 0ef8b36be..d5dce18fe 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/e2e/RunConfigCucumberTest.java @@ -18,5 +18,5 @@ @IncludeEngines("cucumber") @SelectFile("test-harness/gherkin/config.feature") @ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty") -@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps.config") +@ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps") public class RunConfigCucumberTest {} diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java index cf89d69f9..fef39688f 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java @@ -26,7 +26,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("in-process") -@ExcludeTags({"unixsocket", "customCert"}) +@ExcludeTags({"unixsocket", "customCert", "targetURI"}) @Testcontainers public class RunInProcessTest { 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 ceb8f0934..562aa6c46 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 @@ -11,6 +11,7 @@ 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.SelectFile; import org.junit.platform.suite.api.Suite; import org.testcontainers.junit.jupiter.Testcontainers; @@ -21,13 +22,13 @@ @Order(value = Integer.MAX_VALUE) @Suite @IncludeEngines("cucumber") -// @SelectDirectories("test-harness/gherkin") -@SelectFile("test-harness/gherkin/connection.feature") +@SelectDirectories("test-harness/gherkin") +//@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") -@IncludeTags({"rpc", "reconnect"}) -@ExcludeTags({"targetURI", "customCert", "unixsocket"}) +@IncludeTags({"rpc"}) +@ExcludeTags({ "unixsocket", "targetURI"}) @Testcontainers public class RunRpcTest { 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 ab79713be..049cd59ca 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 @@ -7,6 +7,7 @@ import io.cucumber.java.en.Given; import io.cucumber.java.en.Then; import io.cucumber.java.en.When; +import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; @@ -57,7 +58,7 @@ public void we_initialize_a_config_for(String string) { @Given("an option {string} of type {string} with value {string}") public void we_have_an_option_of_type_with_value(String option, String type, String value) - throws ClassNotFoundException, NoSuchMethodException, InvocationTargetException, IllegalAccessException { + throws Throwable { if (IGNORED_FOR_NOW.contains(option)) { LOG.error("option '{}' is not supported", option); return; diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java index b27cb7d55..27193ccfd 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java @@ -1,6 +1,8 @@ package dev.openfeature.contrib.providers.flagd.e2e.steps; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; import dev.openfeature.contrib.providers.flagd.e2e.State; import dev.openfeature.sdk.FlagEvaluationDetails; @@ -18,12 +20,12 @@ public FlagSteps(State state) { } @Given("a {}-flag with key {string} and a default value {string}") - public void givenAFlag(String type, String name, String defaultValue) throws ClassNotFoundException { + public void givenAFlag(String type, String name, String defaultValue) throws Throwable { state.flag = new Flag(type, name, Utils.convert(defaultValue, type)); } @When("the flag was evaluated with details") - public void the_flag_was_evaluated_with_details() { + public void the_flag_was_evaluated_with_details() throws InterruptedException { FlagEvaluationDetails details; switch (state.flag.type) { case "String": @@ -52,8 +54,8 @@ public void the_flag_was_evaluated_with_details() { state.evaluation = details; } - @Then("the resolved details value should be {string}") - public void the_resolved_details_value_should_be(String value) throws ClassNotFoundException { + @Then("the resolved details value should be \"{}\"") + public void the_resolved_details_value_should_be(String value) throws Throwable { assertThat(state.evaluation.getValue()).isEqualTo(Utils.convert(value, state.flag.type)); } @@ -66,13 +68,14 @@ public void the_reason_should_be(String reason) { public void the_variant_should_be(String variant) { assertThat(state.evaluation.getVariant()).isEqualTo(variant); } - + @Then("the flag should be part of the event payload") @Then("the flag was modified") public void the_flag_was_modified() { - assertThat(state.lastEvent).isPresent().hasValueSatisfying((event) -> { - assertThat(event.type).isEqualTo("change"); - assertThat(event.details.getFlagsChanged()).contains(state.flag.name); - }); + await().atMost(5000, MILLISECONDS) + .until(() -> state.events.stream().anyMatch(event -> event.type.equals("change") && event.details.getFlagsChanged().contains(state.flag.name))); + state.lastEvent = state.events.stream() + .filter(event -> event.type.equals("change") && event.details.getFlagsChanged().contains(state.flag.name)) + .findFirst(); } public class Flag { 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 62eb92e68..4403856cf 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 @@ -44,8 +44,7 @@ public class ProviderSteps extends AbstractSteps { static Map> proxyports = new HashMap<>(); public static Network network = Network.newNetwork(); public static ToxiproxyContainer toxiproxy = new ToxiproxyContainer("ghcr.io/shopify/toxiproxy:2.5.0") - .withNetwork(network) - .withCreateContainerCmdModifier((cmd -> cmd.withName("toxiproxy"))); + .withNetwork(network); public static ToxiproxyClient toxiproxyClient; static Path sharedTempDir; @@ -59,6 +58,8 @@ public class ProviderSteps extends AbstractSteps { } } + private Timer restartTimer; + public ProviderSteps(State state) { super(state); } @@ -96,11 +97,28 @@ public static void afterAll() throws IOException { containers.forEach((name, container) -> container.stop()); FileUtils.deleteDirectory(sharedTempDir.toFile()); + toxiproxyClient.reset(); + toxiproxy.stop(); } @Before public void before() throws IOException { + toxiproxyClient.getProxies().forEach(proxy -> + { + try { + proxy.toxics().getAll().forEach(toxic -> { + try { + toxic.remove(); + } catch (IOException e) { + LOG.debug("Failed to remove timout", e); + } + }); + } catch (IOException e) { + LOG.debug("Failed to remove timout", e); + } + }); + containers.values().stream() .filter(containers -> !containers.isRunning()) .forEach(FlagdContainer::start); @@ -114,9 +132,19 @@ public void tearDown() { public int getPort(Config.Resolver resolver, ProviderType providerType) { switch (resolver) { case RPC: - return this.getMappedPort(8013); + switch (providerType) { + case DEFAULT: + return toxiproxy.getMappedPort(8666); + case SSL: + return toxiproxy.getMappedPort(8668); + } case IN_PROCESS: - return this.getMappedPort(8015); + switch (providerType) { + case DEFAULT: + return toxiproxy.getMappedPort(8667); + case SSL: + return toxiproxy.getMappedPort(8669); + } default: throw new IllegalArgumentException("Unsupported resolver: " + resolver); } @@ -124,7 +152,7 @@ public int getPort(Config.Resolver resolver, ProviderType providerType) { @Given("a {} flagd provider") public void setupProvider(String providerType) { - state.builder.deadline(500).keepAlive(0).retryGracePeriod(1); + state.builder.deadline(500).keepAlive(0).retryGracePeriod(3); boolean wait = true; switch (providerType) { case "unavailable": @@ -146,14 +174,14 @@ public void setupProvider(String providerType) { String absolutePath = file.getAbsolutePath(); this.state.providerType = ProviderType.SSL; state.builder - .port(getContainer(state.providerType).getPort(State.resolverType)) + .port(getPort(State.resolverType, state.providerType)) .tls(true) .certPath(absolutePath); break; default: this.state.providerType = ProviderType.DEFAULT; - state.builder.port(toxiproxy.getMappedPort(8666)); + state.builder.port(getPort(State.resolverType, state.providerType)); break; } FeatureProvider provider = @@ -171,23 +199,26 @@ public void setupProvider(String providerType) { @When("the connection is lost for {int}s") public void the_connection_is_lost_for(int seconds) throws InterruptedException, IOException { LOG.info("Timeout and wait for {} seconds", seconds); + String randomizer = RandomStringUtils.randomAlphanumeric(5); + String timoutName = "restart" + randomizer; Proxy proxy = toxiproxyClient.getProxy(generateProxyName(State.resolverType, state.providerType)); - Timeout restart = proxy.toxics().timeout("restart", ToxicDirection.UPSTREAM, seconds); + proxy.toxics().timeout(timoutName, ToxicDirection.UPSTREAM, seconds); TimerTask task = new TimerTask() { public void run() { try { - proxy.toxics().get("restart").remove(); + proxy.toxics().get(timoutName).remove(); } catch (IOException e) { - throw new RuntimeException(e); + LOG.debug("Failed to remove timout", e); } } }; - Timer timer = new Timer("Timer"); + restartTimer = new Timer("Timer" + randomizer); - timer.schedule(task, seconds * 1000L); + restartTimer.schedule(task, seconds * 1000L); } + static FlagdContainer getContainer(ProviderType providerType) { LOG.info("getting container for {}", providerType); return containers.getOrDefault(providerType, containers.get(ProviderType.DEFAULT)); 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 b4f8dd429..9f3adfd75 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 @@ -2,13 +2,17 @@ import dev.openfeature.contrib.providers.flagd.Config; import dev.openfeature.contrib.providers.flagd.resolver.grpc.cache.CacheType; +import dev.openfeature.sdk.Value; +import org.testcontainers.shaded.com.fasterxml.jackson.databind.ObjectMapper; +import java.io.IOException; +import java.util.Map; import java.util.Objects; public final class Utils { private Utils() {} - public static Object convert(String value, String type) throws ClassNotFoundException { + public static Object convert(String value, String type) throws ClassNotFoundException, IOException { if (Objects.equals(value, "null")) return null; switch (type) { case "Boolean": @@ -32,6 +36,8 @@ public static Object convert(String value, String type) throws ClassNotFoundExce } case "CacheType": return CacheType.valueOf(value.toUpperCase()).getValue(); + case "Object": + return Value.objectToValue(new ObjectMapper().readValue(value, Object.class)); } throw new RuntimeException("Unknown config type: " + type); } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserverTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserverTest.java index 9370f821a..68757be7b 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserverTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserverTest.java @@ -23,7 +23,7 @@ class EventStreamObserverTest { - @Nested + /* @Nested class StateChange { Cache cache; @@ -87,5 +87,5 @@ public void cacheBustingForKnownKeys() { verify(cache, times(1)).remove(eq(key1)); verify(cache, times(1)).remove(eq(key2)); } - } + }*/ } diff --git a/providers/flagd/src/test/resources/simplelogger.properties b/providers/flagd/src/test/resources/simplelogger.properties deleted file mode 100644 index 55fc08759..000000000 --- a/providers/flagd/src/test/resources/simplelogger.properties +++ /dev/null @@ -1,2 +0,0 @@ -org.slf4j.simpleLogger.defaultLogLevel=debug -org.slf4j.simpleLogger.logFile=System.out From 2b81d5ba01d57459368562845d98cc70088d4c03 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 15 Jan 2025 11:38:59 +0100 Subject: [PATCH 06/13] fixup: fixing inprocess and eventing Signed-off-by: Simon Schrottner --- providers/flagd/pom.xml | 1 + .../providers/flagd/FlagdProvider.java | 135 +++++++++++++----- .../providers/flagd/resolver/Resolver.java | 2 + .../flagd/resolver/common/ChannelMonitor.java | 2 +- .../resolver/common/ConnectionState.java | 27 ---- ...tionEvent.java => FlagdProviderEvent.java} | 70 +++------ .../flagd/resolver/common/GrpcConnector.java | 64 +-------- .../resolver/grpc/EventStreamObserver.java | 21 +-- .../flagd/resolver/grpc/GrpcResolver.java | 30 ++-- .../resolver/process/InProcessResolver.java | 45 +++--- .../connector/grpc/GrpcStreamConnector.java | 13 +- .../providers/flagd/FlagdProviderTest.java | 36 +++-- .../providers/flagd/e2e/RunInProcessTest.java | 1 + .../providers/flagd/e2e/RunRpcTest.java | 5 +- .../flagd/e2e/steps/ConfigSteps.java | 15 +- .../providers/flagd/e2e/steps/EventSteps.java | 13 +- .../providers/flagd/e2e/steps/FlagSteps.java | 9 +- .../flagd/e2e/steps/ProviderSteps.java | 25 ++-- .../providers/flagd/e2e/steps/Utils.java | 3 +- .../resolver/common/GrpcConnectorTest.java | 10 +- .../grpc/EventStreamObserverTest.java | 23 +-- .../process/InProcessResolverTest.java | 14 +- .../test/resources/simplelogger.properties | 2 + providers/flagd/test-harness | 2 +- 24 files changed, 247 insertions(+), 321 deletions(-) delete mode 100644 providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionState.java rename providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/{ConnectionEvent.java => FlagdProviderEvent.java} (54%) create mode 100644 providers/flagd/src/test/resources/simplelogger.properties diff --git a/providers/flagd/pom.xml b/providers/flagd/pom.xml index 0ee1ce333..5c7808528 100644 --- a/providers/flagd/pom.xml +++ b/providers/flagd/pom.xml @@ -159,6 +159,7 @@ org.slf4j slf4j-simple 2.0.16 + test 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 1e9c30882..b0e1defb4 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 @@ -1,7 +1,8 @@ package dev.openfeature.contrib.providers.flagd; import dev.openfeature.contrib.providers.flagd.resolver.Resolver; -import dev.openfeature.contrib.providers.flagd.resolver.common.ConnectionEvent; +import dev.openfeature.contrib.providers.flagd.resolver.common.FlagdProviderEvent; +import dev.openfeature.contrib.providers.flagd.resolver.common.Util; import dev.openfeature.contrib.providers.flagd.resolver.grpc.GrpcResolver; import dev.openfeature.contrib.providers.flagd.resolver.grpc.cache.Cache; import dev.openfeature.contrib.providers.flagd.resolver.process.InProcessResolver; @@ -12,12 +13,17 @@ import dev.openfeature.sdk.ImmutableStructure; import dev.openfeature.sdk.Metadata; import dev.openfeature.sdk.ProviderEvaluation; +import dev.openfeature.sdk.ProviderEvent; import dev.openfeature.sdk.ProviderEventDetails; import dev.openfeature.sdk.Structure; import dev.openfeature.sdk.Value; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.TimeUnit; import java.util.function.Function; import lombok.extern.slf4j.Slf4j; @@ -31,10 +37,29 @@ public class FlagdProvider extends EventProvider { private static final String FLAGD_PROVIDER = "flagd"; private final Resolver flagResolver; private volatile boolean initialized = false; - private volatile boolean connected = false; private volatile Structure syncMetadata = new ImmutableStructure(); private volatile EvaluationContext enrichedContext = new ImmutableContext(); private final List hooks = new ArrayList<>(); + private volatile ProviderEvent previousEvent = null; + + /** + * An executor service responsible for scheduling reconnection attempts. + */ + private final ScheduledExecutorService reconnectExecutor; + + /** + * A scheduled task for managing reconnection attempts. + */ + private ScheduledFuture reconnectTask; + + /** + * The grace period in milliseconds to wait for reconnection before emitting an error event. + */ + private final long gracePeriod; + /** + * The deadline in milliseconds for GRPC operations. + */ + private final long deadline; protected final void finalize() { // DO NOT REMOVE, spotbugs: CT_CONSTRUCTOR_THROW @@ -55,11 +80,11 @@ public FlagdProvider() { public FlagdProvider(final FlagdOptions options) { switch (options.getResolverType().asString()) { case Config.RESOLVER_IN_PROCESS: - this.flagResolver = new InProcessResolver(options, this::isConnected, this::onConnectionEvent); + this.flagResolver = new InProcessResolver(options, this::onProviderEvent); break; case Config.RESOLVER_RPC: this.flagResolver = new GrpcResolver( - options, new Cache(options.getCacheType(), options.getMaxCacheSize()), this::onConnectionEvent); + options, new Cache(options.getCacheType(), options.getMaxCacheSize()), this::onProviderEvent); break; default: throw new IllegalStateException( @@ -67,6 +92,9 @@ public FlagdProvider(final FlagdOptions options) { } hooks.add(new SyncMetadataHook(this::getEnrichedContext)); contextEnricher = options.getContextEnricher(); + this.reconnectExecutor = Executors.newSingleThreadScheduledExecutor(); + this.gracePeriod = options.getRetryGracePeriod(); + this.deadline = options.getDeadline(); } @Override @@ -81,7 +109,9 @@ public synchronized void initialize(EvaluationContext evaluationContext) throws } this.flagResolver.init(); - this.initialized = this.connected = true; + // block till ready - this works with deadline fine for rpc, but with in_process we also need to take parsing + // into the equation + Util.busyWaitAndCheck(this.deadline + 1000, () -> initialized); } @Override @@ -89,9 +119,12 @@ public synchronized void shutdown() { if (!this.initialized) { return; } - try { this.flagResolver.shutdown(); + if (reconnectExecutor != null) { + reconnectExecutor.shutdownNow(); + reconnectExecutor.awaitTermination(deadline, TimeUnit.MILLISECONDS); + } } catch (Exception e) { log.error("Error during shutdown {}", FLAGD_PROVIDER, e); } finally { @@ -151,47 +184,73 @@ EvaluationContext getEnrichedContext() { return enrichedContext; } - private boolean isConnected() { - return this.connected; - } + private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) { - private void onConnectionEvent(ConnectionEvent connectionEvent) { - final boolean wasConnected = connected; - final boolean isConnected = connected = connectionEvent.isConnected(); + syncMetadata = flagdProviderEvent.getSyncMetadata(); + if (flagdProviderEvent.getSyncMetadata() != null) { + enrichedContext = contextEnricher.apply(flagdProviderEvent.getSyncMetadata()); + } - syncMetadata = connectionEvent.getSyncMetadata(); - enrichedContext = contextEnricher.apply(connectionEvent.getSyncMetadata()); + switch (flagdProviderEvent.getEvent()) { + case PROVIDER_CONFIGURATION_CHANGED: + if (previousEvent == ProviderEvent.PROVIDER_READY) { + this.emitProviderConfigurationChanged(ProviderEventDetails.builder() + .flagsChanged(flagdProviderEvent.getFlagsChanged()) + .message("configuration changed") + .build()); + break; + } + case PROVIDER_READY: + onReady(); + previousEvent = ProviderEvent.PROVIDER_READY; + break; - if (!initialized) { - return; + case PROVIDER_ERROR: + if (previousEvent != ProviderEvent.PROVIDER_ERROR) { + onError(); + } + previousEvent = ProviderEvent.PROVIDER_ERROR; + break; } + } - if (!wasConnected && isConnected) { - ProviderEventDetails details = ProviderEventDetails.builder() - .flagsChanged(connectionEvent.getFlagsChanged()) - .message("connected to flagd") - .build(); - this.emitProviderReady(details); - return; + private void onReady() { + if (!initialized) { + initialized = true; + log.info("initialized FlagdProvider"); + } + if (reconnectTask != null && !reconnectTask.isCancelled()) { + reconnectTask.cancel(false); + log.debug("Reconnection task cancelled as connection became READY."); } + this.emitProviderReady( + ProviderEventDetails.builder().message("connected to flagd").build()); + } - if (wasConnected && isConnected) { - ProviderEventDetails details = ProviderEventDetails.builder() - .flagsChanged(connectionEvent.getFlagsChanged()) - .message("configuration changed") - .build(); - this.emitProviderConfigurationChanged(details); - return; + private void onError() { + log.info("Connection lost. Emit STALE event..."); + log.debug("Waiting {}s for connection to become available...", gracePeriod); + this.emitProviderStale(ProviderEventDetails.builder() + .message("there has been an error") + .build()); + + if (reconnectTask != null && !reconnectTask.isCancelled()) { + reconnectTask.cancel(false); } - if (connectionEvent.isStale()) { - this.emitProviderStale(ProviderEventDetails.builder() - .message("there has been an error") - .build()); - } else { - this.emitProviderError(ProviderEventDetails.builder() - .message("there has been an error") - .build()); + if (!reconnectExecutor.isShutdown()) { + reconnectTask = reconnectExecutor.schedule( + () -> { + log.debug( + "Provider did not reconnect successfully within {}s. Emit ERROR event...", gracePeriod); + flagResolver.onError(); + this.emitProviderError(ProviderEventDetails.builder() + .message("there has been an error") + .build()); + ; + }, + gracePeriod, + TimeUnit.SECONDS); } } } diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/Resolver.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/Resolver.java index 1f9106501..c86a175fe 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/Resolver.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/Resolver.java @@ -10,6 +10,8 @@ public interface Resolver { void shutdown() throws Exception; + default void onError() {} + ProviderEvaluation booleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx); ProviderEvaluation stringEvaluation(String key, String defaultValue, EvaluationContext ctx); diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelMonitor.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelMonitor.java index 8ccb73c15..7bac59e4e 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelMonitor.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelMonitor.java @@ -32,7 +32,7 @@ public static void monitorChannelState( Runnable onConnectionLost) { channel.notifyWhenStateChanged(expectedState, () -> { ConnectivityState currentState = channel.getState(true); - log.info("Channel state changed to: {}", currentState); + // log.info("Channel state changed to: {}", currentState); if (currentState == ConnectivityState.READY) { if (onConnectionReady != null) { onConnectionReady.run(); diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionState.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionState.java deleted file mode 100644 index 6dbd388a0..000000000 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionState.java +++ /dev/null @@ -1,27 +0,0 @@ -package dev.openfeature.contrib.providers.flagd.resolver.common; - -/** - * Represents the possible states of a connection. - */ -public enum ConnectionState { - - /** - * The connection is active and functioning as expected. - */ - CONNECTED, - - /** - * The connection is not active and has been fully disconnected. - */ - DISCONNECTED, - - /** - * The connection is inactive or degraded but may still recover. - */ - STALE, - - /** - * The connection has encountered an error and cannot function correctly. - */ - ERROR, -} diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionEvent.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/FlagdProviderEvent.java similarity index 54% rename from providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionEvent.java rename to providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/FlagdProviderEvent.java index 58b878d6a..517a23218 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ConnectionEvent.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/FlagdProviderEvent.java @@ -1,10 +1,11 @@ package dev.openfeature.contrib.providers.flagd.resolver.common; import dev.openfeature.sdk.ImmutableStructure; +import dev.openfeature.sdk.ProviderEvent; import dev.openfeature.sdk.Structure; -import lombok.Getter; import java.util.Collections; import java.util.List; +import lombok.Getter; /** * Represents an event payload for a connection state change in a @@ -12,13 +13,13 @@ * The event includes information about the connection status, any flags that have changed, * and metadata associated with the synchronization process. */ -public class ConnectionEvent { +public class FlagdProviderEvent { /** * The current state of the connection. */ @Getter - private final ConnectionState connected; + private final ProviderEvent event; /** * A list of flags that have changed due to this connection event. @@ -30,57 +31,45 @@ public class ConnectionEvent { */ private final Structure syncMetadata; - /** - * Constructs a new {@code ConnectionEvent} with the connection status only. - * - * @param connected {@code true} if the connection is established, otherwise {@code false}. - */ - public ConnectionEvent(boolean connected) { - this( - connected ? ConnectionState.CONNECTED : ConnectionState.DISCONNECTED, - Collections.emptyList(), - new ImmutableStructure()); - } - /** * Constructs a new {@code ConnectionEvent} with the specified connection state. * - * @param connected the connection state indicating if the connection is established or not. + * @param event the event indicating the provider state. */ - public ConnectionEvent(ConnectionState connected) { - this(connected, Collections.emptyList(), new ImmutableStructure()); + public FlagdProviderEvent(ProviderEvent event) { + this(event, Collections.emptyList(), new ImmutableStructure()); } /** * Constructs a new {@code ConnectionEvent} with the specified connection state and changed flags. * - * @param connected the connection state indicating if the connection is established or not. + * @param event the event indicating the provider state. * @param flagsChanged a list of flags that have changed due to this connection event. */ - public ConnectionEvent(ConnectionState connected, List flagsChanged) { - this(connected, flagsChanged, new ImmutableStructure()); + public FlagdProviderEvent(ProviderEvent event, List flagsChanged) { + this(event, flagsChanged, new ImmutableStructure()); } /** * Constructs a new {@code ConnectionEvent} with the specified connection state and synchronization metadata. * - * @param connected the connection state indicating if the connection is established or not. + * @param event the event indicating the provider state. * @param syncMetadata metadata related to the synchronization process of this event. */ - public ConnectionEvent(ConnectionState connected, Structure syncMetadata) { - this(connected, Collections.emptyList(), new ImmutableStructure(syncMetadata.asMap())); + public FlagdProviderEvent(ProviderEvent event, Structure syncMetadata) { + this(event, Collections.emptyList(), new ImmutableStructure(syncMetadata.asMap())); } /** * Constructs a new {@code ConnectionEvent} with the specified connection state, changed flags, and * synchronization metadata. * - * @param connectionState the state of the connection. + * @param event the event. * @param flagsChanged a list of flags that have changed due to this connection event. * @param syncMetadata metadata related to the synchronization process of this event. */ - public ConnectionEvent(ConnectionState connectionState, List flagsChanged, Structure syncMetadata) { - this.connected = connectionState; + public FlagdProviderEvent(ProviderEvent event, List flagsChanged, Structure syncMetadata) { + this.event = event; this.flagsChanged = flagsChanged != null ? flagsChanged : Collections.emptyList(); // Ensure non-null list this.syncMetadata = syncMetadata != null ? new ImmutableStructure(syncMetadata.asMap()) @@ -105,32 +94,7 @@ public Structure getSyncMetadata() { return new ImmutableStructure(syncMetadata.asMap()); } - /** - * Indicates whether the current connection state is connected. - * - * @return {@code true} if connected, otherwise {@code false}. - */ - public boolean isConnected() { - return this.connected == ConnectionState.CONNECTED; - } - - /** - * Indicates - * whether the current connection state is disconnected. - * - * @return {@code true} if disconnected, otherwise {@code false}. - */ public boolean isDisconnected() { - return this.connected == ConnectionState.DISCONNECTED; + return event == ProviderEvent.PROVIDER_ERROR || event == ProviderEvent.PROVIDER_STALE; } - /** - * Indicates - * whether the current connection state is stale. - * - * @return {@code true} if stale, otherwise {@code false}. - */ - public boolean isStale() { - return this.connected == ConnectionState.STALE; - } - } diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnector.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnector.java index bac93e880..b7d351e41 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnector.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnector.java @@ -2,14 +2,12 @@ import dev.openfeature.contrib.providers.flagd.FlagdOptions; import dev.openfeature.sdk.ImmutableStructure; +import dev.openfeature.sdk.ProviderEvent; import io.grpc.ConnectivityState; import io.grpc.ManagedChannel; import io.grpc.stub.AbstractBlockingStub; import io.grpc.stub.AbstractStub; import java.util.Collections; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import java.util.function.Function; @@ -54,34 +52,19 @@ public class GrpcConnector, K extends AbstractBlocking /** * A consumer that handles connection events such as connection loss or reconnection. */ - private final Consumer onConnectionEvent; + private final Consumer onConnectionEvent; /** * A consumer that handles GRPC service stubs for event stream handling. */ private final Consumer streamObserver; - /** - * An executor service responsible for scheduling reconnection attempts. - */ - private final ScheduledExecutorService reconnectExecutor; - - /** - * The grace period in milliseconds to wait for reconnection before emitting an error event. - */ - private final long gracePeriod; - /** * Indicates whether the connector is currently connected to the GRPC service. */ @Getter private boolean connected = false; - /** - * A scheduled task for managing reconnection attempts. - */ - private ScheduledFuture reconnectTask; - /** * Constructs a new {@code GrpcConnector} instance with the specified options and parameters. * @@ -96,7 +79,7 @@ public GrpcConnector( final FlagdOptions options, final Function stub, final Function blockingStub, - final Consumer onConnectionEvent, + final Consumer onConnectionEvent, final Consumer eventStreamObserver, ManagedChannel channel) { @@ -107,8 +90,6 @@ public GrpcConnector( this.streamDeadlineMs = options.getStreamDeadlineMs(); this.onConnectionEvent = onConnectionEvent; this.streamObserver = eventStreamObserver; - this.gracePeriod = options.getRetryGracePeriod(); - this.reconnectExecutor = Executors.newSingleThreadScheduledExecutor(); } /** @@ -124,7 +105,7 @@ public GrpcConnector( final FlagdOptions options, final Function stub, final Function blockingStub, - final Consumer onConnectionEvent, + final Consumer onConnectionEvent, final Consumer eventStreamObserver) { this(options, stub, blockingStub, onConnectionEvent, eventStreamObserver, ChannelBuilder.nettyChannel(options)); } @@ -136,8 +117,6 @@ public GrpcConnector( */ public void initialize() throws Exception { log.info("Initializing GRPC connection..."); - ChannelMonitor.waitForDesiredState( - ConnectivityState.READY, channel, this::onInitialConnect, deadline, TimeUnit.MILLISECONDS); ChannelMonitor.monitorChannelState(ConnectivityState.READY, channel, this::onReady, this::onConnectionLost); } @@ -157,20 +136,11 @@ public K getResolver() { */ public void shutdown() throws InterruptedException { log.info("Shutting down GRPC connection..."); - if (reconnectExecutor != null) { - reconnectExecutor.shutdownNow(); - reconnectExecutor.awaitTermination(deadline, TimeUnit.MILLISECONDS); - } if (!channel.isShutdown()) { channel.shutdownNow(); channel.awaitTermination(deadline, TimeUnit.MILLISECONDS); } - - if (connected) { - this.onConnectionEvent.accept(new ConnectionEvent(false)); - connected = false; - } } private synchronized void onInitialConnect() { @@ -184,11 +154,6 @@ private synchronized void onInitialConnect() { */ private synchronized void onReady() { connected = true; - - if (reconnectTask != null && !reconnectTask.isCancelled()) { - reconnectTask.cancel(false); - log.debug("Reconnection task cancelled as connection became READY."); - } restartStream(); } @@ -197,27 +162,10 @@ private synchronized void onReady() { * Schedules a reconnection task after a grace period and emits a stale connection event. */ private synchronized void onConnectionLost() { - log.debug("Connection lost. Emit STALE event..."); - log.debug("Waiting {}s for connection to become available...", gracePeriod); connected = false; - this.onConnectionEvent.accept( - new ConnectionEvent(ConnectionState.STALE, Collections.emptyList(), new ImmutableStructure())); - - if (reconnectTask != null && !reconnectTask.isCancelled()) { - reconnectTask.cancel(false); - } - - if (!reconnectExecutor.isShutdown()) { - reconnectTask = reconnectExecutor.schedule( - () -> { - log.debug( - "Provider did not reconnect successfully within {}s. Emit ERROR event...", gracePeriod); - this.onConnectionEvent.accept(new ConnectionEvent(false)); - }, - gracePeriod, - TimeUnit.SECONDS); - } + this.onConnectionEvent.accept(new FlagdProviderEvent( + ProviderEvent.PROVIDER_ERROR, Collections.emptyList(), new ImmutableStructure())); } /** diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserver.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserver.java index 705115e5e..8b8886bf8 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserver.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserver.java @@ -1,16 +1,14 @@ package dev.openfeature.contrib.providers.flagd.resolver.grpc; import com.google.protobuf.Value; -import dev.openfeature.contrib.providers.flagd.resolver.common.ConnectionEvent; -import dev.openfeature.contrib.providers.flagd.resolver.grpc.cache.Cache; +import dev.openfeature.contrib.providers.flagd.resolver.common.FlagdProviderEvent; import dev.openfeature.flagd.grpc.evaluation.Evaluation.EventStreamResponse; +import dev.openfeature.sdk.ProviderEvent; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import io.grpc.stub.StreamObserver; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.function.BiConsumer; import java.util.function.Consumer; import lombok.extern.slf4j.Slf4j; @@ -22,16 +20,15 @@ @SuppressFBWarnings(justification = "cache needs to be read and write by multiple objects") class EventStreamObserver implements StreamObserver { - private final Consumer> onConfigurationChange; - private final Consumer onReady; + private final Consumer onReady; /** * Constructs a new {@code EventStreamObserver} instance. * * @param onConnectionEvent a consumer to handle connection events with a boolean and a list of changed flags */ - EventStreamObserver(Consumer> onConfigurationChange, Consumer onReady) { + EventStreamObserver(Consumer> onConfigurationChange, Consumer onReady) { this.onConfigurationChange = onConfigurationChange; this.onReady = onReady; } @@ -56,14 +53,10 @@ public void onNext(EventStreamResponse value) { } @Override - public void onError(Throwable throwable) { - - } + public void onError(Throwable throwable) {} @Override - public void onCompleted() { - - } + public void onCompleted() {} /** * Handles configuration change events by updating the cache and notifying listeners about changed flags. @@ -88,6 +81,6 @@ private void handleConfigurationChangeEvent(EventStreamResponse value) { */ private void handleProviderReadyEvent() { log.info("Received provider ready event"); - onReady.accept(new ConnectionEvent(true)); + onReady.accept(new FlagdProviderEvent(ProviderEvent.PROVIDER_READY)); } } 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 dfc6047cb..2a9669ec3 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 @@ -10,8 +10,7 @@ import dev.openfeature.contrib.providers.flagd.Config; import dev.openfeature.contrib.providers.flagd.FlagdOptions; import dev.openfeature.contrib.providers.flagd.resolver.Resolver; -import dev.openfeature.contrib.providers.flagd.resolver.common.ConnectionEvent; -import dev.openfeature.contrib.providers.flagd.resolver.common.ConnectionState; +import dev.openfeature.contrib.providers.flagd.resolver.common.FlagdProviderEvent; import dev.openfeature.contrib.providers.flagd.resolver.common.GrpcConnector; import dev.openfeature.contrib.providers.flagd.resolver.grpc.cache.Cache; import dev.openfeature.contrib.providers.flagd.resolver.grpc.strategy.ResolveFactory; @@ -26,6 +25,7 @@ import dev.openfeature.sdk.EvaluationContext; import dev.openfeature.sdk.ImmutableMetadata; import dev.openfeature.sdk.ProviderEvaluation; +import dev.openfeature.sdk.ProviderEvent; import dev.openfeature.sdk.Value; import dev.openfeature.sdk.exceptions.FlagNotFoundError; import dev.openfeature.sdk.exceptions.GeneralError; @@ -57,33 +57,28 @@ public final class GrpcResolver implements Resolver { * * @param options flagd options * @param cache cache to use - * @param onConnectionEvent lambda which handles changes in the connection/stream + * @param onProviderEvent lambda which handles changes in the connection/stream */ public GrpcResolver( - final FlagdOptions options, final Cache cache, final Consumer onConnectionEvent) { + final FlagdOptions options, final Cache cache, final Consumer onProviderEvent) { this.cache = cache; this.strategy = ResolveFactory.getStrategy(options); this.connector = new GrpcConnector<>( options, ServiceGrpc::newStub, ServiceGrpc::newBlockingStub, - (event) -> { - if( cache != null && event.isDisconnected()) { - cache.clear(); - } - onConnectionEvent.accept(event); - }, + onProviderEvent, stub -> stub.eventStream( Evaluation.EventStreamRequest.getDefaultInstance(), new EventStreamObserver( (flags) -> { - if( cache != null) { + if (cache != null) { flags.forEach(cache::remove); } - onConnectionEvent.accept(new ConnectionEvent(ConnectionState.CONNECTED, flags)); + onProviderEvent.accept(new FlagdProviderEvent( + ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, flags)); }, - onConnectionEvent - ))); + onProviderEvent))); } /** @@ -100,6 +95,13 @@ public void shutdown() throws Exception { this.connector.shutdown(); } + @Override + public void onError() { + if (cache != null) { + cache.clear(); + } + } + /** * Boolean evaluation from grpc resolver. */ 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 95f2eb6d6..b3385781e 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 @@ -4,14 +4,11 @@ import dev.openfeature.contrib.providers.flagd.FlagdOptions; import dev.openfeature.contrib.providers.flagd.resolver.Resolver; -import dev.openfeature.contrib.providers.flagd.resolver.common.ConnectionEvent; -import dev.openfeature.contrib.providers.flagd.resolver.common.ConnectionState; -import dev.openfeature.contrib.providers.flagd.resolver.common.Util; +import dev.openfeature.contrib.providers.flagd.resolver.common.FlagdProviderEvent; import dev.openfeature.contrib.providers.flagd.resolver.process.model.FeatureFlag; import dev.openfeature.contrib.providers.flagd.resolver.process.storage.FlagStore; import dev.openfeature.contrib.providers.flagd.resolver.process.storage.Storage; import dev.openfeature.contrib.providers.flagd.resolver.process.storage.StorageQueryResult; -import dev.openfeature.contrib.providers.flagd.resolver.process.storage.StorageState; import dev.openfeature.contrib.providers.flagd.resolver.process.storage.StorageStateChange; import dev.openfeature.contrib.providers.flagd.resolver.process.storage.connector.Connector; import dev.openfeature.contrib.providers.flagd.resolver.process.storage.connector.file.FileConnector; @@ -22,13 +19,13 @@ import dev.openfeature.sdk.EvaluationContext; import dev.openfeature.sdk.ImmutableMetadata; import dev.openfeature.sdk.ProviderEvaluation; +import dev.openfeature.sdk.ProviderEvent; import dev.openfeature.sdk.Reason; import dev.openfeature.sdk.Value; import dev.openfeature.sdk.exceptions.ParseError; import dev.openfeature.sdk.exceptions.TypeMismatchError; import java.util.Map; import java.util.function.Consumer; -import java.util.function.Supplier; import lombok.extern.slf4j.Slf4j; /** @@ -39,10 +36,9 @@ @Slf4j public class InProcessResolver implements Resolver { private final Storage flagStore; - private final Consumer onConnectionEvent; + private final Consumer onConnectionEvent; private final Operator operator; private final long deadline; - private final Supplier connectedSupplier; private final String scope; /** @@ -51,20 +47,14 @@ public class InProcessResolver implements Resolver { * Flags are evaluated locally. * * @param options flagd options - * @param connectedSupplier lambda providing current connection status from - * caller * @param onConnectionEvent lambda which handles changes in the * connection/stream */ - public InProcessResolver( - FlagdOptions options, - final Supplier connectedSupplier, - Consumer onConnectionEvent) { + public InProcessResolver(FlagdOptions options, Consumer onConnectionEvent) { this.flagStore = new FlagStore(getConnector(options, onConnectionEvent)); this.deadline = options.getDeadline(); this.onConnectionEvent = onConnectionEvent; this.operator = new Operator(); - this.connectedSupplier = connectedSupplier; this.scope = options.getSelector(); } @@ -78,15 +68,20 @@ public void init() throws Exception { while (true) { final StorageStateChange storageStateChange = flagStore.getStateQueue().take(); - if (storageStateChange.getStorageState() != StorageState.OK) { - log.info( - String.format("Storage returned NOK status: %s", storageStateChange.getStorageState())); - continue; + switch (storageStateChange.getStorageState()) { + case OK: + onConnectionEvent.accept(new FlagdProviderEvent( + ProviderEvent.PROVIDER_CONFIGURATION_CHANGED, + storageStateChange.getChangedFlagsKeys(), + storageStateChange.getSyncMetadata())); + break; + case ERROR: + onConnectionEvent.accept(new FlagdProviderEvent(ProviderEvent.PROVIDER_ERROR)); + break; + default: + log.info(String.format( + "Storage emitted unhandled status: %s", storageStateChange.getStorageState())); } - onConnectionEvent.accept(new ConnectionEvent( - ConnectionState.CONNECTED, - storageStateChange.getChangedFlagsKeys(), - storageStateChange.getSyncMetadata())); } } catch (InterruptedException e) { log.warn("Storage state watcher interrupted", e); @@ -95,9 +90,6 @@ public void init() throws Exception { }); stateWatcher.setDaemon(true); stateWatcher.start(); - - // block till ready - Util.busyWaitAndCheck(this.deadline, this.connectedSupplier); } /** @@ -107,7 +99,6 @@ public void init() throws Exception { */ public void shutdown() throws InterruptedException { flagStore.shutdown(); - onConnectionEvent.accept(new ConnectionEvent(false)); } /** @@ -154,7 +145,7 @@ public ProviderEvaluation objectEvaluation(String key, Value defaultValue .build(); } - static Connector getConnector(final FlagdOptions options, Consumer onConnectionEvent) { + static Connector getConnector(final FlagdOptions options, Consumer onConnectionEvent) { if (options.getCustomConnector() != null) { return options.getCustomConnector(); } diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java index e48a65211..0a7ab91a0 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java @@ -1,7 +1,7 @@ package dev.openfeature.contrib.providers.flagd.resolver.process.storage.connector.grpc; import dev.openfeature.contrib.providers.flagd.FlagdOptions; -import dev.openfeature.contrib.providers.flagd.resolver.common.ConnectionEvent; +import dev.openfeature.contrib.providers.flagd.resolver.common.FlagdProviderEvent; import dev.openfeature.contrib.providers.flagd.resolver.common.GrpcConnector; import dev.openfeature.contrib.providers.flagd.resolver.process.storage.connector.Connector; import dev.openfeature.contrib.providers.flagd.resolver.process.storage.connector.QueuePayload; @@ -43,7 +43,7 @@ public class GrpcStreamConnector implements Connector { /** * Creates a new GrpcStreamConnector responsible for observing the event stream. */ - public GrpcStreamConnector(final FlagdOptions options, Consumer onConnectionEvent) { + public GrpcStreamConnector(final FlagdOptions options, Consumer onConnectionEvent) { deadline = options.getDeadline(); selector = options.getSelector(); streamReceiver = new LinkedBlockingQueue<>(QUEUE_SIZE); @@ -60,7 +60,7 @@ public void init() throws Exception { grpcConnector.initialize(); Thread listener = new Thread(() -> { try { - observeEventStream(blockingQueue, shutdown, selector, deadline); + observeEventStream(blockingQueue, shutdown, deadline); } catch (InterruptedException e) { log.warn("gRPC event stream interrupted, flag configurations are stale", e); Thread.currentThread().interrupt(); @@ -89,11 +89,7 @@ public void shutdown() throws InterruptedException { } /** Contains blocking calls, to be used concurrently. */ - void observeEventStream( - final BlockingQueue writeTo, - final AtomicBoolean shutdown, - final String selector, - final int deadline) + void observeEventStream(final BlockingQueue writeTo, final AtomicBoolean shutdown, final int deadline) throws InterruptedException { log.info("Initializing sync stream observer"); @@ -137,6 +133,7 @@ void observeEventStream( Throwable streamException = response.getError(); if (streamException != null || metadataException != null) { + log.debug("Exception in GRPC connection"); if (!writeTo.offer(new QueuePayload( QueuePayloadType.ERROR, "Error from stream or metadata", metadataResponse))) { log.error("Failed to convey ERROR status, queue is full"); diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java index c223cbc31..43c9ac2e9 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java @@ -17,8 +17,7 @@ import com.google.protobuf.Struct; import dev.openfeature.contrib.providers.flagd.resolver.Resolver; -import dev.openfeature.contrib.providers.flagd.resolver.common.ConnectionEvent; -import dev.openfeature.contrib.providers.flagd.resolver.common.ConnectionState; +import dev.openfeature.contrib.providers.flagd.resolver.common.FlagdProviderEvent; import dev.openfeature.contrib.providers.flagd.resolver.common.GrpcConnector; import dev.openfeature.contrib.providers.flagd.resolver.grpc.GrpcResolver; import dev.openfeature.contrib.providers.flagd.resolver.grpc.cache.Cache; @@ -43,11 +42,13 @@ import dev.openfeature.sdk.MutableContext; import dev.openfeature.sdk.MutableStructure; import dev.openfeature.sdk.OpenFeatureAPI; +import dev.openfeature.sdk.ProviderEvent; import dev.openfeature.sdk.Reason; import dev.openfeature.sdk.Structure; import dev.openfeature.sdk.Value; import io.cucumber.java.AfterAll; import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -60,6 +61,7 @@ import java.util.function.Consumer; import java.util.function.Function; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.mockito.MockedConstruction; @@ -319,6 +321,8 @@ void resolvers_should_not_cache_responses_if_not_static() { } @Test + @Disabled( + "This test seems to be wrong on the way, we are handling caching, as we return values as long as we are in stale mode") void resolvers_should_not_cache_responses_if_event_stream_not_alive() { do_resolvers_cache_responses(STATIC_REASON, false, false); } @@ -570,6 +574,16 @@ void initializationAndShutdown() throws Exception { flagResolver.setAccessible(true); flagResolver.set(provider, resolverMock); + Method onProviderEvent = FlagdProvider.class.getDeclaredMethod("onProviderEvent", FlagdProviderEvent.class); + onProviderEvent.setAccessible(true); + + doAnswer((i) -> { + onProviderEvent.invoke(provider, new FlagdProviderEvent(ProviderEvent.PROVIDER_READY)); + return null; + }) + .when(resolverMock) + .init(); + // when // validate multiple initialization @@ -599,16 +613,17 @@ void contextEnrichment() throws Exception { // mock a resolver try (MockedConstruction mockResolver = mockConstruction(InProcessResolver.class, (mock, context) -> { - Consumer onConnectionEvent; + Consumer onConnectionEvent; // get a reference to the onConnectionEvent callback onConnectionEvent = - (Consumer) context.arguments().get(2); + (Consumer) context.arguments().get(1); // when our mock resolver initializes, it runs the passed onConnectionEvent // callback doAnswer(invocation -> { - onConnectionEvent.accept(new ConnectionEvent(ConnectionState.CONNECTED, metadata)); + onConnectionEvent.accept( + new FlagdProviderEvent(ProviderEvent.PROVIDER_READY, metadata)); return null; }) .when(mock) @@ -639,16 +654,17 @@ void updatesSyncMetadataWithCallback() throws Exception { // mock a resolver try (MockedConstruction mockResolver = mockConstruction(InProcessResolver.class, (mock, context) -> { - Consumer onConnectionEvent; + Consumer onConnectionEvent; // get a reference to the onConnectionEvent callback onConnectionEvent = - (Consumer) context.arguments().get(2); + (Consumer) context.arguments().get(1); // when our mock resolver initializes, it runs the passed onConnectionEvent // callback doAnswer(invocation -> { - onConnectionEvent.accept(new ConnectionEvent(ConnectionState.CONNECTED, metadata)); + onConnectionEvent.accept( + new FlagdProviderEvent(ProviderEvent.PROVIDER_READY, metadata)); return null; }) .when(mock) @@ -702,6 +718,10 @@ private FlagdProvider createProvider(GrpcConnector grpc, Cache cache) { Field flagResolver = FlagdProvider.class.getDeclaredField("flagResolver"); flagResolver.setAccessible(true); flagResolver.set(provider, grpcResolver); + + Field initialized = FlagdProvider.class.getDeclaredField("initialized"); + initialized.setAccessible(true); + initialized.set(provider, true); } catch (NoSuchFieldException | IllegalAccessException e) { throw new RuntimeException(e); } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java index fef39688f..abaa2595a 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java @@ -22,6 +22,7 @@ @Suite @IncludeEngines("cucumber") @SelectDirectories("test-harness/gherkin") +// @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") 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 562aa6c46..a950aa351 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; @@ -23,12 +22,12 @@ @Suite @IncludeEngines("cucumber") @SelectDirectories("test-harness/gherkin") -//@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") @IncludeTags({"rpc"}) -@ExcludeTags({ "unixsocket", "targetURI"}) +@ExcludeTags({"targetURI"}) @Testcontainers public class RunRpcTest { 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 049cd59ca..8e8ee44d6 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 @@ -7,17 +7,15 @@ import io.cucumber.java.en.Given; import io.cucumber.java.en.Then; import io.cucumber.java.en.When; -import java.io.IOException; -import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import lombok.extern.slf4j.Slf4j; +@Slf4j public class ConfigSteps extends AbstractSteps { /** * Not all properties are correctly implemented, hence that we need to ignore them till this is @@ -30,8 +28,6 @@ public class ConfigSteps extends AbstractSteps { } }; - private static final Logger LOG = LoggerFactory.getLogger(ConfigSteps.class); - public ConfigSteps(State state) { super(state); } @@ -57,10 +53,9 @@ public void we_initialize_a_config_for(String string) { } @Given("an option {string} of type {string} with value {string}") - public void we_have_an_option_of_type_with_value(String option, String type, String value) - throws Throwable { + public void we_have_an_option_of_type_with_value(String option, String type, String value) throws Throwable { if (IGNORED_FOR_NOW.contains(option)) { - LOG.error("option '{}' is not supported", option); + log.error("option '{}' is not supported", option); return; } @@ -87,7 +82,7 @@ public void the_option_of_type_should_have_the_value(String option, String type, Object convert = Utils.convert(value, type); if (IGNORED_FOR_NOW.contains(option)) { - LOG.error("option '{}' is not supported", option); + log.error("option '{}' is not supported", option); return; } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java index cbf3d250e..6dbd0c9ca 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/EventSteps.java @@ -9,14 +9,13 @@ import io.cucumber.java.en.Then; import io.cucumber.java.en.When; import java.util.LinkedList; +import lombok.extern.slf4j.Slf4j; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.parallel.Isolated; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @Isolated() +@Slf4j public class EventSteps extends AbstractSteps { - private static final Logger LOG = LoggerFactory.getLogger(EventSteps.class); public EventSteps(State state) { super(state); @@ -26,7 +25,7 @@ public EventSteps(State state) { @Given("a {} event handler") public void a_stale_event_handler(String eventType) { state.client.on(mapEventType(eventType), eventDetails -> { - LOG.info("event tracked for {} ", eventType); + log.info("event tracked for {} ", eventType); state.events.add(new Event(eventType, eventDetails)); }); } @@ -47,8 +46,10 @@ public void a_stale_event_handler(String eventType) { } @When("a {} event was fired") - public void eventWasFired(String eventType) { + public void eventWasFired(String eventType) throws InterruptedException { eventHandlerShouldBeExecutedWithin(eventType, 10000); + // we might be too fast in the execution + Thread.sleep(500); } @Then("the {} event handler should have been executed") @@ -58,7 +59,7 @@ public void eventHandlerShouldBeExecuted(String eventType) { @Then("the {} event handler should have been executed within {int}ms") public void eventHandlerShouldBeExecutedWithin(String eventType, int ms) { - LOG.info("waiting for eventtype: {}", eventType); + log.info("waiting for eventtype: {}", eventType); await().atMost(ms, MILLISECONDS) .until(() -> state.events.stream().anyMatch(event -> event.type.equals(eventType))); state.lastEvent = state.events.stream() diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java index 27193ccfd..aab14a857 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/FlagSteps.java @@ -68,13 +68,16 @@ public void the_reason_should_be(String reason) { public void the_variant_should_be(String variant) { assertThat(state.evaluation.getVariant()).isEqualTo(variant); } + @Then("the flag should be part of the event payload") @Then("the flag was modified") public void the_flag_was_modified() { - await().atMost(5000, MILLISECONDS) - .until(() -> state.events.stream().anyMatch(event -> event.type.equals("change") && event.details.getFlagsChanged().contains(state.flag.name))); + await().atMost(5000, MILLISECONDS).until(() -> state.events.stream() + .anyMatch(event -> event.type.equals("change") + && event.details.getFlagsChanged().contains(state.flag.name))); state.lastEvent = state.events.stream() - .filter(event -> event.type.equals("change") && event.details.getFlagsChanged().contains(state.flag.name)) + .filter(event -> event.type.equals("change") + && event.details.getFlagsChanged().contains(state.flag.name)) .findFirst(); } 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 4403856cf..241f58927 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 @@ -9,7 +9,6 @@ import eu.rekawek.toxiproxy.Proxy; import eu.rekawek.toxiproxy.ToxiproxyClient; import eu.rekawek.toxiproxy.model.ToxicDirection; -import eu.rekawek.toxiproxy.model.toxic.Timeout; import io.cucumber.java.After; import io.cucumber.java.AfterAll; import io.cucumber.java.Before; @@ -25,26 +24,24 @@ import java.util.Map; import java.util.Timer; import java.util.TimerTask; +import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.parallel.Isolated; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.testcontainers.containers.BindMode; import org.testcontainers.containers.Network; import org.testcontainers.containers.ToxiproxyContainer; import org.testcontainers.shaded.org.apache.commons.io.FileUtils; @Isolated() +@Slf4j public class ProviderSteps extends AbstractSteps { - private static final Logger LOG = LoggerFactory.getLogger(ProviderSteps.class); - public static final int UNAVAILABLE_PORT = 9999; static Map containers = new HashMap<>(); static Map> proxyports = new HashMap<>(); public static Network network = Network.newNetwork(); - public static ToxiproxyContainer toxiproxy = new ToxiproxyContainer("ghcr.io/shopify/toxiproxy:2.5.0") - .withNetwork(network); + public static ToxiproxyContainer toxiproxy = + new ToxiproxyContainer("ghcr.io/shopify/toxiproxy:2.5.0").withNetwork(network); public static ToxiproxyClient toxiproxyClient; static Path sharedTempDir; @@ -104,18 +101,17 @@ public static void afterAll() throws IOException { @Before public void before() throws IOException { - toxiproxyClient.getProxies().forEach(proxy -> - { + toxiproxyClient.getProxies().forEach(proxy -> { try { proxy.toxics().getAll().forEach(toxic -> { try { toxic.remove(); } catch (IOException e) { - LOG.debug("Failed to remove timout", e); + log.debug("Failed to remove timout", e); } }); } catch (IOException e) { - LOG.debug("Failed to remove timout", e); + log.debug("Failed to remove timout", e); } }); @@ -198,7 +194,7 @@ public void setupProvider(String providerType) { @When("the connection is lost for {int}s") public void the_connection_is_lost_for(int seconds) throws InterruptedException, IOException { - LOG.info("Timeout and wait for {} seconds", seconds); + log.info("Timeout and wait for {} seconds", seconds); String randomizer = RandomStringUtils.randomAlphanumeric(5); String timoutName = "restart" + randomizer; Proxy proxy = toxiproxyClient.getProxy(generateProxyName(State.resolverType, state.providerType)); @@ -209,7 +205,7 @@ public void run() { try { proxy.toxics().get(timoutName).remove(); } catch (IOException e) { - LOG.debug("Failed to remove timout", e); + log.debug("Failed to remove timout", e); } } }; @@ -218,9 +214,8 @@ public void run() { restartTimer.schedule(task, seconds * 1000L); } - static FlagdContainer getContainer(ProviderType providerType) { - LOG.info("getting container for {}", providerType); + log.info("getting container for {}", providerType); return containers.getOrDefault(providerType, containers.get(ProviderType.DEFAULT)); } } 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 9f3adfd75..909d4800a 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 @@ -3,10 +3,9 @@ import dev.openfeature.contrib.providers.flagd.Config; import dev.openfeature.contrib.providers.flagd.resolver.grpc.cache.CacheType; import dev.openfeature.sdk.Value; -import org.testcontainers.shaded.com.fasterxml.jackson.databind.ObjectMapper; import java.io.IOException; -import java.util.Map; import java.util.Objects; +import org.testcontainers.shaded.com.fasterxml.jackson.databind.ObjectMapper; public final class Utils { diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnectorTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnectorTest.java index 4c417f957..0229b2660 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnectorTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnectorTest.java @@ -17,6 +17,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -71,11 +72,12 @@ private void tearDownGrpcServer() throws InterruptedException { } @Test + @Disabled("not sure this test makes sense in this kind of way") void whenShuttingDownAndRestartingGrpcServer_ConsumerReceivesDisconnectedAndConnectedEvent() throws Exception { CountDownLatch sync = new CountDownLatch(2); ArrayList connectionStateChanges = Lists.newArrayList(); - Consumer testConsumer = event -> { - connectionStateChanges.add(event.isConnected()); + Consumer testConsumer = event -> { + connectionStateChanges.add(!event.isDisconnected()); sync.countDown(); }; @@ -106,8 +108,8 @@ void whenShuttingDownAndRestartingGrpcServer_ConsumerReceivesDisconnectedAndConn void whenShuttingDownGrpcConnector_ConsumerReceivesDisconnectedEvent() throws Exception { CountDownLatch sync = new CountDownLatch(1); ArrayList connectionStateChanges = Lists.newArrayList(); - Consumer testConsumer = event -> { - connectionStateChanges.add(event.isConnected()); + Consumer testConsumer = event -> { + connectionStateChanges.add(!event.isDisconnected()); sync.countDown(); }; diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserverTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserverTest.java index 68757be7b..1b7a73ec2 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserverTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/grpc/EventStreamObserverTest.java @@ -1,29 +1,8 @@ package dev.openfeature.contrib.providers.flagd.resolver.grpc; -import static org.junit.Assert.assertTrue; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.atLeast; -import static org.mockito.Mockito.atMost; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import com.google.protobuf.Struct; -import com.google.protobuf.Value; -import dev.openfeature.contrib.providers.flagd.resolver.grpc.cache.Cache; -import dev.openfeature.flagd.grpc.evaluation.Evaluation.EventStreamResponse; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Nested; -import org.junit.jupiter.api.Test; - class EventStreamObserverTest { - /* @Nested + /* @Nested class StateChange { Cache cache; diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolverTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolverTest.java index 90295ef10..141f4a305 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolverTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolverTest.java @@ -20,7 +20,7 @@ import dev.openfeature.contrib.providers.flagd.Config; import dev.openfeature.contrib.providers.flagd.FlagdOptions; -import dev.openfeature.contrib.providers.flagd.resolver.common.ConnectionEvent; +import dev.openfeature.contrib.providers.flagd.resolver.common.FlagdProviderEvent; import dev.openfeature.contrib.providers.flagd.resolver.process.model.FeatureFlag; import dev.openfeature.contrib.providers.flagd.resolver.process.storage.MockConnector; import dev.openfeature.contrib.providers.flagd.resolver.process.storage.StorageState; @@ -88,7 +88,7 @@ void eventHandling() throws Throwable { InProcessResolver inProcessResolver = getInProcessResolverWith( new MockStorage(new HashMap<>(), sender), connectionEvent -> receiver.offer(new StorageStateChange( - connectionEvent.isConnected() ? StorageState.OK : StorageState.ERROR, + connectionEvent.isDisconnected() ? StorageState.ERROR : StorageState.OK, connectionEvent.getFlagsChanged(), connectionEvent.getSyncMetadata()))); @@ -517,25 +517,25 @@ void flagSetMetadataIsOverwrittenByFlagMetadataToEvaluation() throws Exception { private InProcessResolver getInProcessResolverWith(final FlagdOptions options, final MockStorage storage) throws NoSuchFieldException, IllegalAccessException { - final InProcessResolver resolver = new InProcessResolver(options, () -> true, connectionEvent -> {}); + final InProcessResolver resolver = new InProcessResolver(options, connectionEvent -> {}); return injectFlagStore(resolver, storage); } private InProcessResolver getInProcessResolverWith( - final MockStorage storage, final Consumer onConnectionEvent) + final MockStorage storage, final Consumer onConnectionEvent) throws NoSuchFieldException, IllegalAccessException { final InProcessResolver resolver = - new InProcessResolver(FlagdOptions.builder().deadline(1000).build(), () -> true, onConnectionEvent); + new InProcessResolver(FlagdOptions.builder().deadline(1000).build(), onConnectionEvent); return injectFlagStore(resolver, storage); } private InProcessResolver getInProcessResolverWith( - final MockStorage storage, final Consumer onConnectionEvent, String selector) + final MockStorage storage, final Consumer onConnectionEvent, String selector) throws NoSuchFieldException, IllegalAccessException { final InProcessResolver resolver = new InProcessResolver( - FlagdOptions.builder().selector(selector).deadline(1000).build(), () -> true, onConnectionEvent); + FlagdOptions.builder().selector(selector).deadline(1000).build(), onConnectionEvent); return injectFlagStore(resolver, storage); } diff --git a/providers/flagd/src/test/resources/simplelogger.properties b/providers/flagd/src/test/resources/simplelogger.properties new file mode 100644 index 000000000..32c1f1a08 --- /dev/null +++ b/providers/flagd/src/test/resources/simplelogger.properties @@ -0,0 +1,2 @@ +org.slf4j.simpleLogger.defaultLogLevel=info +org.slf4j.simpleLogger.logFile=System.out diff --git a/providers/flagd/test-harness b/providers/flagd/test-harness index 9488dfdb7..a4faffc71 160000 --- a/providers/flagd/test-harness +++ b/providers/flagd/test-harness @@ -1 +1 @@ -Subproject commit 9488dfdb7c687538f7bba2a0c4e014d740c9033d +Subproject commit a4faffc71e632b734699503427db998f2ef86edf From 5f5c630268c829448de4c0a091896fa58c3a2fb4 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 15 Jan 2025 15:13:31 +0100 Subject: [PATCH 07/13] fixup: javadoc, naming, and log improvements Signed-off-by: Simon Schrottner --- .../providers/flagd/FlagdProvider.java | 22 ++++++++++--------- .../flagd/resolver/common/ChannelMonitor.java | 2 +- .../flagd/e2e/steps/ProviderSteps.java | 1 - 3 files changed, 13 insertions(+), 12 deletions(-) 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 c2c4a5076..ea0ab6941 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 @@ -43,17 +43,19 @@ public class FlagdProvider extends EventProvider { private volatile ProviderEvent previousEvent = null; /** - * An executor service responsible for scheduling reconnection attempts. + * An executor service responsible for emitting {@link ProviderEvent#PROVIDER_ERROR} after the provider went + * {@link ProviderEvent#PROVIDER_STALE} for {@link #gracePeriod} seconds. */ - private final ScheduledExecutorService reconnectExecutor; + private final ScheduledExecutorService errorExecutor; /** - * A scheduled task for managing reconnection attempts. + * A scheduled task for emitting {@link ProviderEvent#PROVIDER_ERROR}. */ private ScheduledFuture reconnectTask; /** - * The grace period in milliseconds to wait for reconnection before emitting an error event. + * The grace period in milliseconds to wait after {@link ProviderEvent#PROVIDER_STALE} before emitting a + * {@link ProviderEvent#PROVIDER_ERROR}. */ private final long gracePeriod; /** @@ -92,7 +94,7 @@ public FlagdProvider(final FlagdOptions options) { } hooks.add(new SyncMetadataHook(this::getEnrichedContext)); contextEnricher = options.getContextEnricher(); - this.reconnectExecutor = Executors.newSingleThreadScheduledExecutor(); + this.errorExecutor = Executors.newSingleThreadScheduledExecutor(); this.gracePeriod = options.getRetryGracePeriod(); this.deadline = options.getDeadline(); } @@ -122,9 +124,9 @@ public synchronized void shutdown() { } try { this.flagResolver.shutdown(); - if (reconnectExecutor != null) { - reconnectExecutor.shutdownNow(); - reconnectExecutor.awaitTermination(deadline, TimeUnit.MILLISECONDS); + if (errorExecutor != null) { + errorExecutor.shutdownNow(); + errorExecutor.awaitTermination(deadline, TimeUnit.MILLISECONDS); } } catch (Exception e) { log.error("Error during shutdown {}", FLAGD_PROVIDER, e); @@ -242,8 +244,8 @@ private void onError() { reconnectTask.cancel(false); } - if (!reconnectExecutor.isShutdown()) { - reconnectTask = reconnectExecutor.schedule( + if (!errorExecutor.isShutdown()) { + reconnectTask = errorExecutor.schedule( () -> { log.debug( "Provider did not reconnect successfully within {}s. Emit ERROR event...", gracePeriod); diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelMonitor.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelMonitor.java index 7bac59e4e..1b201d640 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelMonitor.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/common/ChannelMonitor.java @@ -32,7 +32,7 @@ public static void monitorChannelState( Runnable onConnectionLost) { channel.notifyWhenStateChanged(expectedState, () -> { ConnectivityState currentState = channel.getState(true); - // log.info("Channel state changed to: {}", currentState); + log.debug("Channel state changed to: {}", currentState); if (currentState == ConnectivityState.READY) { if (onConnectionReady != null) { onConnectionReady.run(); 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 ed05ff707..d0d179b3d 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,7 +2,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectReader; -import com.google.gson.JsonObject; import dev.openfeature.contrib.providers.flagd.Config; import dev.openfeature.contrib.providers.flagd.FlagdProvider; import dev.openfeature.contrib.providers.flagd.e2e.FlagdContainer; From 92a77293c79998e21f028de0689490551c8e8da5 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Wed, 15 Jan 2025 12:20:52 -0500 Subject: [PATCH 08/13] fixup: toxic down and upstream Signed-off-by: Todd Baert --- .../providers/flagd/e2e/steps/ProviderSteps.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) 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 d0d179b3d..cf0e5ed0c 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 @@ -204,16 +204,19 @@ public void setupProvider(String providerType) throws IOException { public void the_connection_is_lost_for(int seconds) throws InterruptedException, IOException { log.info("Timeout and wait for {} seconds", seconds); String randomizer = RandomStringUtils.randomAlphanumeric(5); - String timoutName = "restart" + randomizer; + String timeoutUpName = "restart-up-" + randomizer; + String timeoutDownName = "restart-down-" + randomizer; Proxy proxy = toxiproxyClient.getProxy(generateProxyName(State.resolverType, state.providerType)); - proxy.toxics().timeout(timoutName, ToxicDirection.UPSTREAM, seconds); + proxy.toxics().timeout(timeoutDownName, ToxicDirection.DOWNSTREAM, seconds); + proxy.toxics().timeout(timeoutUpName, ToxicDirection.UPSTREAM, seconds); TimerTask task = new TimerTask() { public void run() { try { - proxy.toxics().get(timoutName).remove(); + proxy.toxics().get(timeoutUpName).remove(); + proxy.toxics().get(timeoutDownName).remove(); } catch (IOException e) { - log.debug("Failed to remove timout", e); + log.debug("Failed to remove timeout", e); } } }; From ff5bccfa0e3e4c29e32690c786f644c9fb002833 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 15 Jan 2025 21:07:24 +0100 Subject: [PATCH 09/13] fixup: fixing nit, and adding information for fallthrough Signed-off-by: Simon Schrottner --- .gitmodules | 2 +- providers/flagd/pom.xml | 3 +- .../providers/flagd/FlagdProvider.java | 21 +++++++++--- .../connector/grpc/GrpcStreamConnector.java | 2 +- .../providers/flagd/FlagdProviderTest.java | 8 ----- .../resolver/common/GrpcConnectorTest.java | 34 ------------------- providers/flagd/test-harness | 2 +- 7 files changed, 20 insertions(+), 52 deletions(-) diff --git a/.gitmodules b/.gitmodules index e439dea5d..b302279b9 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,7 +4,7 @@ [submodule "providers/flagd/test-harness"] path = providers/flagd/test-harness url = https://github.com/open-feature/test-harness.git - branch = v0.5.21 + branch = v1.1.0 [submodule "providers/flagd/spec"] path = providers/flagd/spec url = https://github.com/open-feature/spec.git diff --git a/providers/flagd/pom.xml b/providers/flagd/pom.xml index 138adc430..84410503d 100644 --- a/providers/flagd/pom.xml +++ b/providers/flagd/pom.xml @@ -156,14 +156,13 @@ test - 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 ea0ab6941..7f517a7c1 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 @@ -114,7 +114,7 @@ public synchronized void initialize(EvaluationContext evaluationContext) throws // block till ready - this works with deadline fine for rpc, but with in_process we also need to take parsing // into the equation // TODO: evaluate where we are losing time, so we can remove this magic number - follow up - Util.busyWaitAndCheck(this.deadline + 200, () -> initialized); + Util.busyWaitAndCheck(this.deadline + 500, () -> initialized); } @Override @@ -195,15 +195,19 @@ private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) { enrichedContext = contextEnricher.apply(flagdProviderEvent.getSyncMetadata()); } + /* + We only use Error and Ready as previous states. + As error will first be emitted as Stale, and only turns after a while into an emitted Error. + Ready is needed, as the InProcessResolver does not have a dedicated ready event, hence we need to + forward a configuration changed to the ready, if we are not in the ready state. + */ switch (flagdProviderEvent.getEvent()) { case PROVIDER_CONFIGURATION_CHANGED: if (previousEvent == ProviderEvent.PROVIDER_READY) { - this.emitProviderConfigurationChanged(ProviderEventDetails.builder() - .flagsChanged(flagdProviderEvent.getFlagsChanged()) - .message("configuration changed") - .build()); + onConfigurationChanged(flagdProviderEvent); break; } + // intentional fall through, a not-ready change will trigger a ready. case PROVIDER_READY: onReady(); previousEvent = ProviderEvent.PROVIDER_READY; @@ -220,6 +224,13 @@ private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) { } } + private void onConfigurationChanged(FlagdProviderEvent flagdProviderEvent) { + this.emitProviderConfigurationChanged(ProviderEventDetails.builder() + .flagsChanged(flagdProviderEvent.getFlagsChanged()) + .message("configuration changed") + .build()); + } + private void onReady() { if (!initialized) { initialized = true; diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java index 0a7ab91a0..3033d31f4 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java @@ -112,7 +112,6 @@ void observeEventStream(final BlockingQueue writeTo, final AtomicB try { metadataResponse = grpcConnector .getResolver() - .withDeadlineAfter(deadline, TimeUnit.MILLISECONDS) .getMetadata(metadataRequest.build()); } catch (Exception e) { // the chances this call fails but the syncRequest does not are slim @@ -122,6 +121,7 @@ void observeEventStream(final BlockingQueue writeTo, final AtomicB metadataException = e; } + log.info("stream"); while (!shutdown.get()) { final GrpcResponseModel response = streamReceiver.take(); if (response.isComplete()) { diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java index 43c9ac2e9..e719f4bcf 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java @@ -61,7 +61,6 @@ import java.util.function.Consumer; import java.util.function.Function; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.mockito.MockedConstruction; @@ -320,13 +319,6 @@ void resolvers_should_not_cache_responses_if_not_static() { do_resolvers_cache_responses(DEFAULT.toString(), true, false); } - @Test - @Disabled( - "This test seems to be wrong on the way, we are handling caching, as we return values as long as we are in stale mode") - void resolvers_should_not_cache_responses_if_event_stream_not_alive() { - do_resolvers_cache_responses(STATIC_REASON, false, false); - } - @Test void context_is_parsed_and_passed_to_grpc_service() { final String BOOLEAN_ATTR_KEY = "bool-attr"; diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnectorTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnectorTest.java index 0229b2660..596042bb5 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnectorTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/common/GrpcConnectorTest.java @@ -17,7 +17,6 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -71,39 +70,6 @@ private void tearDownGrpcServer() throws InterruptedException { } } - @Test - @Disabled("not sure this test makes sense in this kind of way") - void whenShuttingDownAndRestartingGrpcServer_ConsumerReceivesDisconnectedAndConnectedEvent() throws Exception { - CountDownLatch sync = new CountDownLatch(2); - ArrayList connectionStateChanges = Lists.newArrayList(); - Consumer testConsumer = event -> { - connectionStateChanges.add(!event.isDisconnected()); - sync.countDown(); - }; - - GrpcConnector instance = new GrpcConnector<>( - FlagdOptions.builder().build(), - ServiceGrpc::newStub, - ServiceGrpc::newBlockingStub, - testConsumer, - stub -> stub.eventStream(Evaluation.EventStreamRequest.getDefaultInstance(), mockEventStreamObserver), - testChannel); - - instance.initialize(); - - // when shutting down server - testServer.shutdown(); - testServer.awaitTermination(1, TimeUnit.SECONDS); - - // when restarting server - setupTestGrpcServer(); - - // then consumer received DISCONNECTED and CONNECTED event - boolean finished = sync.await(10, TimeUnit.SECONDS); - Assertions.assertTrue(finished); - Assertions.assertEquals(Lists.newArrayList(DISCONNECTED, CONNECTED), connectionStateChanges); - } - @Test void whenShuttingDownGrpcConnector_ConsumerReceivesDisconnectedEvent() throws Exception { CountDownLatch sync = new CountDownLatch(1); diff --git a/providers/flagd/test-harness b/providers/flagd/test-harness index a4faffc71..dd6b67067 160000 --- a/providers/flagd/test-harness +++ b/providers/flagd/test-harness @@ -1 +1 @@ -Subproject commit a4faffc71e632b734699503427db998f2ef86edf +Subproject commit dd6b67067a3c146f5b7dd414637905f7ec369901 From 853727fad2ee9414f515e7831b3ef757129756a5 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 16 Jan 2025 13:13:33 +0100 Subject: [PATCH 10/13] Update providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java Co-authored-by: warber <72415058+warber@users.noreply.github.com> Signed-off-by: Simon Schrottner --- .../process/storage/connector/grpc/GrpcStreamConnector.java | 1 - 1 file changed, 1 deletion(-) diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java index 3033d31f4..7e7637095 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java @@ -121,7 +121,6 @@ void observeEventStream(final BlockingQueue writeTo, final AtomicB metadataException = e; } - log.info("stream"); while (!shutdown.get()) { final GrpcResponseModel response = streamReceiver.take(); if (response.isComplete()) { From 338f36709662ebdd93bb3f9b4e73d9cc1c9856df Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Wed, 15 Jan 2025 21:07:24 +0100 Subject: [PATCH 11/13] fixup: fixing nit, and adding information for fallthrough Signed-off-by: Simon Schrottner --- .../providers/flagd/FlagdProvider.java | 31 ++++++++++--------- .../connector/grpc/GrpcStreamConnector.java | 2 +- .../providers/flagd/e2e/RunInProcessTest.java | 1 + .../providers/flagd/e2e/RunRpcTest.java | 1 + 4 files changed, 20 insertions(+), 15 deletions(-) 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 7f517a7c1..94acbac1a 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 @@ -51,7 +51,7 @@ public class FlagdProvider extends EventProvider { /** * A scheduled task for emitting {@link ProviderEvent#PROVIDER_ERROR}. */ - private ScheduledFuture reconnectTask; + private ScheduledFuture errorTask; /** * The grace period in milliseconds to wait after {@link ProviderEvent#PROVIDER_STALE} before emitting a @@ -114,7 +114,7 @@ public synchronized void initialize(EvaluationContext evaluationContext) throws // block till ready - this works with deadline fine for rpc, but with in_process we also need to take parsing // into the equation // TODO: evaluate where we are losing time, so we can remove this magic number - follow up - Util.busyWaitAndCheck(this.deadline + 500, () -> initialized); + Util.busyWaitAndCheck(this.deadline + 200, () -> initialized); } @Override @@ -188,7 +188,7 @@ EvaluationContext getEnrichedContext() { } @SuppressWarnings("checkstyle:fallthrough") - private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) { + private synchronized void onProviderEvent(FlagdProviderEvent flagdProviderEvent) { syncMetadata = flagdProviderEvent.getSyncMetadata(); if (flagdProviderEvent.getSyncMetadata() != null) { @@ -236,8 +236,8 @@ private void onReady() { initialized = true; log.info("initialized FlagdProvider"); } - if (reconnectTask != null && !reconnectTask.isCancelled()) { - reconnectTask.cancel(false); + if (errorTask != null && !errorTask.isCancelled()) { + errorTask.cancel(false); log.debug("Reconnection task cancelled as connection became READY."); } this.emitProviderReady( @@ -251,19 +251,22 @@ private void onError() { .message("there has been an error") .build()); - if (reconnectTask != null && !reconnectTask.isCancelled()) { - reconnectTask.cancel(false); + if (errorTask != null && !errorTask.isCancelled()) { + errorTask.cancel(false); } if (!errorExecutor.isShutdown()) { - reconnectTask = errorExecutor.schedule( + errorTask = errorExecutor.schedule( () -> { - log.debug( - "Provider did not reconnect successfully within {}s. Emit ERROR event...", gracePeriod); - flagResolver.onError(); - this.emitProviderError(ProviderEventDetails.builder() - .message("there has been an error") - .build()); + if(previousEvent == ProviderEvent.PROVIDER_ERROR) { + log.debug( + "Provider did not reconnect successfully within {}s. Emit ERROR event...", + gracePeriod); + flagResolver.onError(); + this.emitProviderError(ProviderEventDetails.builder() + .message("there has been an error") + .build()); + } }, gracePeriod, TimeUnit.SECONDS); diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java index 7e7637095..18a2300d9 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java @@ -16,7 +16,6 @@ import io.grpc.Context.CancellableContext; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import lombok.extern.slf4j.Slf4j; @@ -121,6 +120,7 @@ void observeEventStream(final BlockingQueue writeTo, final AtomicB metadataException = e; } + log.info("stream"); while (!shutdown.get()) { final GrpcResponseModel response = streamReceiver.take(); if (response.isComplete()) { diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java index abaa2595a..bc1367d96 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java @@ -22,6 +22,7 @@ @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") 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 bcf6d016a..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 @@ -22,6 +22,7 @@ @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/rpc-caching.feature") @ConfigurationParameter(key = PLUGIN_PROPERTY_NAME, value = "pretty") @ConfigurationParameter(key = GLUE_PROPERTY_NAME, value = "dev.openfeature.contrib.providers.flagd.e2e.steps") From a14931fd7907915face2992de3e6deeefd1092e8 Mon Sep 17 00:00:00 2001 From: Simon Schrottner Date: Thu, 16 Jan 2025 19:57:39 +0100 Subject: [PATCH 12/13] fixup: fixing synchronization in a strange way Signed-off-by: Simon Schrottner --- .gitmodules | 2 +- .../providers/flagd/FlagdProvider.java | 65 ++++++++++--------- .../connector/grpc/GrpcStreamConnector.java | 4 +- .../providers/flagd/e2e/RunInProcessTest.java | 2 +- providers/flagd/test-harness | 2 +- 5 files changed, 39 insertions(+), 36 deletions(-) diff --git a/.gitmodules b/.gitmodules index b302279b9..619d4fff1 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,7 +4,7 @@ [submodule "providers/flagd/test-harness"] path = providers/flagd/test-harness url = https://github.com/open-feature/test-harness.git - branch = v1.1.0 + branch = v1.1.1 [submodule "providers/flagd/spec"] path = providers/flagd/spec url = https://github.com/open-feature/spec.git 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 94acbac1a..c01c33e91 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 @@ -41,6 +41,7 @@ public class FlagdProvider extends EventProvider { private volatile EvaluationContext enrichedContext = new ImmutableContext(); private final List hooks = new ArrayList<>(); private volatile ProviderEvent previousEvent = null; + private final Object eventLock; /** * An executor service responsible for emitting {@link ProviderEvent#PROVIDER_ERROR} after the provider went @@ -97,6 +98,7 @@ public FlagdProvider(final FlagdOptions options) { this.errorExecutor = Executors.newSingleThreadScheduledExecutor(); this.gracePeriod = options.getRetryGracePeriod(); this.deadline = options.getDeadline(); + this.eventLock = new Object(); } @Override @@ -188,39 +190,42 @@ EvaluationContext getEnrichedContext() { } @SuppressWarnings("checkstyle:fallthrough") - private synchronized void onProviderEvent(FlagdProviderEvent flagdProviderEvent) { + private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) { - syncMetadata = flagdProviderEvent.getSyncMetadata(); - if (flagdProviderEvent.getSyncMetadata() != null) { - enrichedContext = contextEnricher.apply(flagdProviderEvent.getSyncMetadata()); - } + synchronized (eventLock) { + log.info("FlagdProviderEvent: {}", flagdProviderEvent); + syncMetadata = flagdProviderEvent.getSyncMetadata(); + if (flagdProviderEvent.getSyncMetadata() != null) { + enrichedContext = contextEnricher.apply(flagdProviderEvent.getSyncMetadata()); + } - /* - We only use Error and Ready as previous states. - As error will first be emitted as Stale, and only turns after a while into an emitted Error. - Ready is needed, as the InProcessResolver does not have a dedicated ready event, hence we need to - forward a configuration changed to the ready, if we are not in the ready state. - */ - switch (flagdProviderEvent.getEvent()) { - case PROVIDER_CONFIGURATION_CHANGED: - if (previousEvent == ProviderEvent.PROVIDER_READY) { - onConfigurationChanged(flagdProviderEvent); + /* + We only use Error and Ready as previous states. + As error will first be emitted as Stale, and only turns after a while into an emitted Error. + Ready is needed, as the InProcessResolver does not have a dedicated ready event, hence we need to + forward a configuration changed to the ready, if we are not in the ready state. + */ + switch (flagdProviderEvent.getEvent()) { + case PROVIDER_CONFIGURATION_CHANGED: + if (previousEvent == ProviderEvent.PROVIDER_READY) { + onConfigurationChanged(flagdProviderEvent); + break; + } + // intentional fall through, a not-ready change will trigger a ready. + case PROVIDER_READY: + onReady(); + previousEvent = ProviderEvent.PROVIDER_READY; break; - } - // intentional fall through, a not-ready change will trigger a ready. - case PROVIDER_READY: - onReady(); - previousEvent = ProviderEvent.PROVIDER_READY; - break; - case PROVIDER_ERROR: - if (previousEvent != ProviderEvent.PROVIDER_ERROR) { - onError(); - } - previousEvent = ProviderEvent.PROVIDER_ERROR; - break; - default: - log.info("Unknown event {}", flagdProviderEvent.getEvent()); + case PROVIDER_ERROR: + if (previousEvent != ProviderEvent.PROVIDER_ERROR) { + onError(); + } + previousEvent = ProviderEvent.PROVIDER_ERROR; + break; + default: + log.info("Unknown event {}", flagdProviderEvent.getEvent()); + } } } @@ -258,7 +263,7 @@ private void onError() { if (!errorExecutor.isShutdown()) { errorTask = errorExecutor.schedule( () -> { - if(previousEvent == ProviderEvent.PROVIDER_ERROR) { + if (previousEvent == ProviderEvent.PROVIDER_ERROR) { log.debug( "Provider did not reconnect successfully within {}s. Emit ERROR event...", gracePeriod); diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java index 18a2300d9..832c8a487 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/grpc/GrpcStreamConnector.java @@ -109,9 +109,7 @@ void observeEventStream(final BlockingQueue writeTo, final AtomicB try (CancellableContext context = Context.current().withCancellation()) { try { - metadataResponse = grpcConnector - .getResolver() - .getMetadata(metadataRequest.build()); + metadataResponse = grpcConnector.getResolver().getMetadata(metadataRequest.build()); } catch (Exception e) { // the chances this call fails but the syncRequest does not are slim // it could be that the server doesn't implement this RPC diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java index bc1367d96..e0edef240 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.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("in-process") -@ExcludeTags({"unixsocket", "customCert", "targetURI"}) +@ExcludeTags({"unixsocket", "targetURI"}) @Testcontainers public class RunInProcessTest { diff --git a/providers/flagd/test-harness b/providers/flagd/test-harness index dd6b67067..fc7867922 160000 --- a/providers/flagd/test-harness +++ b/providers/flagd/test-harness @@ -1 +1 @@ -Subproject commit dd6b67067a3c146f5b7dd414637905f7ec369901 +Subproject commit fc786792273b7984911dc3bcb7b47489f261ba57 From 3d2b826deb70f5e7d8a232e61657340545b5fa61 Mon Sep 17 00:00:00 2001 From: Todd Baert Date: Mon, 20 Jan 2025 14:39:08 -0500 Subject: [PATCH 13/13] fixup: make locking clearer Signed-off-by: Todd Baert --- .../providers/flagd/FlagdProvider.java | 138 +++++++++++------- .../providers/flagd/FlagdProviderTest.java | 12 +- 2 files changed, 85 insertions(+), 65 deletions(-) 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 c01c33e91..bbf7674f1 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 @@ -36,15 +36,12 @@ public class FlagdProvider extends EventProvider { private Function contextEnricher; private static final String FLAGD_PROVIDER = "flagd"; private final Resolver flagResolver; - private volatile boolean initialized = false; - private volatile Structure syncMetadata = new ImmutableStructure(); - private volatile EvaluationContext enrichedContext = new ImmutableContext(); private final List hooks = new ArrayList<>(); - private volatile ProviderEvent previousEvent = null; - private final Object eventLock; + private final EventsLock eventsLock = new EventsLock(); /** - * An executor service responsible for emitting {@link ProviderEvent#PROVIDER_ERROR} after the provider went + * An executor service responsible for emitting + * {@link ProviderEvent#PROVIDER_ERROR} after the provider went * {@link ProviderEvent#PROVIDER_STALE} for {@link #gracePeriod} seconds. */ private final ScheduledExecutorService errorExecutor; @@ -55,7 +52,8 @@ public class FlagdProvider extends EventProvider { private ScheduledFuture errorTask; /** - * The grace period in milliseconds to wait after {@link ProviderEvent#PROVIDER_STALE} before emitting a + * The grace period in milliseconds to wait after + * {@link ProviderEvent#PROVIDER_STALE} before emitting a * {@link ProviderEvent#PROVIDER_ERROR}. */ private final long gracePeriod; @@ -95,10 +93,22 @@ public FlagdProvider(final FlagdOptions options) { } hooks.add(new SyncMetadataHook(this::getEnrichedContext)); contextEnricher = options.getContextEnricher(); - this.errorExecutor = Executors.newSingleThreadScheduledExecutor(); - this.gracePeriod = options.getRetryGracePeriod(); - this.deadline = options.getDeadline(); - this.eventLock = new Object(); + errorExecutor = Executors.newSingleThreadScheduledExecutor(); + gracePeriod = options.getRetryGracePeriod(); + deadline = options.getDeadline(); + } + + /** + * Internal constructor for test cases. + * DO NOT MAKE PUBLIC + */ + FlagdProvider(Resolver resolver, boolean initialized) { + this.flagResolver = resolver; + deadline = Config.DEFAULT_DEADLINE; + gracePeriod = Config.DEFAULT_STREAM_RETRY_GRACE_PERIOD; + hooks.add(new SyncMetadataHook(this::getEnrichedContext)); + errorExecutor = Executors.newSingleThreadScheduledExecutor(); + this.eventsLock.initialized = initialized; } @Override @@ -107,33 +117,39 @@ public List getProviderHooks() { } @Override - public synchronized void initialize(EvaluationContext evaluationContext) throws Exception { - if (this.initialized) { - return; - } + public void initialize(EvaluationContext evaluationContext) throws Exception { + synchronized (eventsLock) { + if (eventsLock.initialized) { + return; + } - this.flagResolver.init(); - // block till ready - this works with deadline fine for rpc, but with in_process we also need to take parsing - // into the equation - // TODO: evaluate where we are losing time, so we can remove this magic number - follow up - Util.busyWaitAndCheck(this.deadline + 200, () -> initialized); + flagResolver.init(); + } + // block till ready - this works with deadline fine for rpc, but with in_process + // we also need to take parsing into the equation + // TODO: evaluate where we are losing time, so we can remove this magic number - + // follow up + // wait outside of the synchonrization or we'll deadlock + Util.busyWaitAndCheck(this.deadline * 2, () -> eventsLock.initialized); } @Override - public synchronized void shutdown() { - if (!this.initialized) { - return; - } - try { - this.flagResolver.shutdown(); - if (errorExecutor != null) { - errorExecutor.shutdownNow(); - errorExecutor.awaitTermination(deadline, TimeUnit.MILLISECONDS); + public void shutdown() { + synchronized (eventsLock) { + if (!eventsLock.initialized) { + return; + } + 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 { + eventsLock.initialized = false; } - } catch (Exception e) { - log.error("Error during shutdown {}", FLAGD_PROVIDER, e); - } finally { - this.initialized = false; } } @@ -144,27 +160,27 @@ public Metadata getMetadata() { @Override public ProviderEvaluation getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx) { - return this.flagResolver.booleanEvaluation(key, defaultValue, ctx); + return flagResolver.booleanEvaluation(key, defaultValue, ctx); } @Override public ProviderEvaluation getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) { - return this.flagResolver.stringEvaluation(key, defaultValue, ctx); + return flagResolver.stringEvaluation(key, defaultValue, ctx); } @Override public ProviderEvaluation getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) { - return this.flagResolver.doubleEvaluation(key, defaultValue, ctx); + return flagResolver.doubleEvaluation(key, defaultValue, ctx); } @Override public ProviderEvaluation getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx) { - return this.flagResolver.integerEvaluation(key, defaultValue, ctx); + return flagResolver.integerEvaluation(key, defaultValue, ctx); } @Override public ProviderEvaluation getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx) { - return this.flagResolver.objectEvaluation(key, defaultValue, ctx); + return flagResolver.objectEvaluation(key, defaultValue, ctx); } /** @@ -177,7 +193,7 @@ public ProviderEvaluation getObjectEvaluation(String key, Value defaultVa * @return Object map representing sync metadata */ protected Structure getSyncMetadata() { - return new ImmutableStructure(syncMetadata.asMap()); + return new ImmutableStructure(eventsLock.syncMetadata.asMap()); } /** @@ -186,42 +202,45 @@ protected Structure getSyncMetadata() { * @return context */ EvaluationContext getEnrichedContext() { - return enrichedContext; + return eventsLock.enrichedContext; } @SuppressWarnings("checkstyle:fallthrough") private void onProviderEvent(FlagdProviderEvent flagdProviderEvent) { - synchronized (eventLock) { + synchronized (eventsLock) { log.info("FlagdProviderEvent: {}", flagdProviderEvent); - syncMetadata = flagdProviderEvent.getSyncMetadata(); + eventsLock.syncMetadata = flagdProviderEvent.getSyncMetadata(); if (flagdProviderEvent.getSyncMetadata() != null) { - enrichedContext = contextEnricher.apply(flagdProviderEvent.getSyncMetadata()); + eventsLock.enrichedContext = contextEnricher.apply(flagdProviderEvent.getSyncMetadata()); } /* - We only use Error and Ready as previous states. - As error will first be emitted as Stale, and only turns after a while into an emitted Error. - Ready is needed, as the InProcessResolver does not have a dedicated ready event, hence we need to - forward a configuration changed to the ready, if we are not in the ready state. + * We only use Error and Ready as previous states. + * As error will first be emitted as Stale, and only turns after a while into an + * emitted Error. + * Ready is needed, as the InProcessResolver does not have a dedicated ready + * event, hence we need to + * forward a configuration changed to the ready, if we are not in the ready + * state. */ switch (flagdProviderEvent.getEvent()) { case PROVIDER_CONFIGURATION_CHANGED: - if (previousEvent == ProviderEvent.PROVIDER_READY) { + if (eventsLock.previousEvent == ProviderEvent.PROVIDER_READY) { onConfigurationChanged(flagdProviderEvent); break; } // intentional fall through, a not-ready change will trigger a ready. case PROVIDER_READY: onReady(); - previousEvent = ProviderEvent.PROVIDER_READY; + eventsLock.previousEvent = ProviderEvent.PROVIDER_READY; break; case PROVIDER_ERROR: - if (previousEvent != ProviderEvent.PROVIDER_ERROR) { + if (eventsLock.previousEvent != ProviderEvent.PROVIDER_ERROR) { onError(); } - previousEvent = ProviderEvent.PROVIDER_ERROR; + eventsLock.previousEvent = ProviderEvent.PROVIDER_ERROR; break; default: log.info("Unknown event {}", flagdProviderEvent.getEvent()); @@ -237,8 +256,8 @@ private void onConfigurationChanged(FlagdProviderEvent flagdProviderEvent) { } private void onReady() { - if (!initialized) { - initialized = true; + if (!eventsLock.initialized) { + eventsLock.initialized = true; log.info("initialized FlagdProvider"); } if (errorTask != null && !errorTask.isCancelled()) { @@ -263,7 +282,7 @@ private void onError() { if (!errorExecutor.isShutdown()) { errorTask = errorExecutor.schedule( () -> { - if (previousEvent == ProviderEvent.PROVIDER_ERROR) { + if (eventsLock.previousEvent == ProviderEvent.PROVIDER_ERROR) { log.debug( "Provider did not reconnect successfully within {}s. Emit ERROR event...", gracePeriod); @@ -277,4 +296,15 @@ private void onError() { TimeUnit.SECONDS); } } + + /** + * Contains all fields we need to worry about locking, used as intrinsic lock + * for sync blocks. + */ + static class EventsLock { + volatile ProviderEvent previousEvent = null; + volatile Structure syncMetadata = new ImmutableStructure(); + volatile boolean initialized = false; + volatile EvaluationContext enrichedContext = new ImmutableContext(); + } } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java index e719f4bcf..0aa226fcf 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdProviderTest.java @@ -700,24 +700,14 @@ private FlagdProvider createProvider(GrpcConnector grpc, Cache cache) { final FlagdOptions flagdOptions = FlagdOptions.builder().build(); final GrpcResolver grpcResolver = new GrpcResolver(flagdOptions, cache, (connectionEvent) -> {}); - final FlagdProvider provider = new FlagdProvider(); - try { Field connector = GrpcResolver.class.getDeclaredField("connector"); connector.setAccessible(true); connector.set(grpcResolver, grpc); - - Field flagResolver = FlagdProvider.class.getDeclaredField("flagResolver"); - flagResolver.setAccessible(true); - flagResolver.set(provider, grpcResolver); - - Field initialized = FlagdProvider.class.getDeclaredField("initialized"); - initialized.setAccessible(true); - initialized.set(provider, true); } catch (NoSuchFieldException | IllegalAccessException e) { throw new RuntimeException(e); } - + final FlagdProvider provider = new FlagdProvider(grpcResolver, true); return provider; }