From 7a4b3fc7cbcdf36c1b352274f16a9b0451a73f7f Mon Sep 17 00:00:00 2001 From: Daniel Zehetner Date: Fri, 7 Jun 2024 11:00:59 +0200 Subject: [PATCH 1/2] feat: [flagd] Default port to 8015 if in-process resolver is used. closes: #809 Signed-off-by: Daniel Zehetner Co-authored-by: Alexandra Oberaigner Signed-off-by: Daniel Zehetner --- .../contrib/providers/flagd/Config.java | 8 +- .../contrib/providers/flagd/FlagdOptions.java | 56 ++++++----- .../providers/flagd/FlagdOptionsTest.java | 99 ++++++++++++++----- 3 files changed, 115 insertions(+), 48 deletions(-) diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/Config.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/Config.java index 5ca2c2e48..5d5cee709 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/Config.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/Config.java @@ -11,7 +11,13 @@ @Slf4j public final class Config { static final Resolver DEFAULT_RESOLVER_TYPE = Resolver.RPC; - static final String DEFAULT_PORT = "8013"; + static final String DEFAULT_RPC_PORT = "8013"; + /** + * @deprecated Use {@link Config#DEFAULT_RPC_PORT} or {@link Config#DEFAULT_IN_PROCESS_PORT} + */ + @Deprecated + static final String DEFAULT_PORT = DEFAULT_RPC_PORT; + static final String DEFAULT_IN_PROCESS_PORT = "8015"; static final String DEFAULT_TLS = "false"; static final String DEFAULT_HOST = "localhost"; diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdOptions.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdOptions.java index 53b5fcd9a..81362dcf2 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdOptions.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdOptions.java @@ -5,28 +5,8 @@ import lombok.Builder; import lombok.Getter; -import static dev.openfeature.contrib.providers.flagd.Config.BASE_EVENT_STREAM_RETRY_BACKOFF_MS; -import static dev.openfeature.contrib.providers.flagd.Config.BASE_EVENT_STREAM_RETRY_BACKOFF_MS_ENV_VAR_NAME; -import static dev.openfeature.contrib.providers.flagd.Config.CACHE_ENV_VAR_NAME; -import static dev.openfeature.contrib.providers.flagd.Config.DEADLINE_MS_ENV_VAR_NAME; -import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_CACHE; -import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_DEADLINE; -import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_HOST; -import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_MAX_CACHE_SIZE; -import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_MAX_EVENT_STREAM_RETRIES; -import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_PORT; -import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_TLS; -import static dev.openfeature.contrib.providers.flagd.Config.HOST_ENV_VAR_NAME; -import static dev.openfeature.contrib.providers.flagd.Config.MAX_CACHE_SIZE_ENV_VAR_NAME; -import static dev.openfeature.contrib.providers.flagd.Config.MAX_EVENT_STREAM_RETRIES_ENV_VAR_NAME; -import static dev.openfeature.contrib.providers.flagd.Config.OFFLINE_SOURCE_PATH; -import static dev.openfeature.contrib.providers.flagd.Config.PORT_ENV_VAR_NAME; -import static dev.openfeature.contrib.providers.flagd.Config.SERVER_CERT_PATH_ENV_VAR_NAME; -import static dev.openfeature.contrib.providers.flagd.Config.SOCKET_PATH_ENV_VAR_NAME; -import static dev.openfeature.contrib.providers.flagd.Config.SOURCE_SELECTOR_ENV_VAR_NAME; -import static dev.openfeature.contrib.providers.flagd.Config.TLS_ENV_VAR_NAME; +import static dev.openfeature.contrib.providers.flagd.Config.*; import static dev.openfeature.contrib.providers.flagd.Config.fallBackToEnvOrDefault; -import static dev.openfeature.contrib.providers.flagd.Config.fromValueProvider; /** * FlagdOptions is a builder to build flagd provider options. @@ -39,8 +19,7 @@ public class FlagdOptions { /** * flagd resolving type. */ - @Builder.Default - private Config.EvaluatorType resolverType = fromValueProvider(System::getenv); + private Config.EvaluatorType resolverType; /** * flagd connection host. @@ -51,8 +30,7 @@ public class FlagdOptions { /** * flagd connection port. */ - @Builder.Default - private int port = Integer.parseInt(fallBackToEnvOrDefault(PORT_ENV_VAR_NAME, DEFAULT_PORT)); + private int port; /** * Use TLS connectivity. @@ -126,6 +104,17 @@ public class FlagdOptions { */ private OpenTelemetry openTelemetry; + + public static FlagdOptionsBuilder builder() { + return new FlagdOptionsBuilder() { + @Override + public FlagdOptions build() { + prebuild(); + return super.build(); + } + }; + } + /** * Overload default lombok builder. */ @@ -140,5 +129,22 @@ public FlagdOptionsBuilder withGlobalTelemetry(final boolean b) { } return this; } + + void prebuild() { + if (resolverType == null) { + resolverType = fromValueProvider(System::getenv); + } + + if (port == 0) { + port = Integer.parseInt(fallBackToEnvOrDefault(PORT_ENV_VAR_NAME, determineDefaultPortForResolver())); + } + } + + private String determineDefaultPortForResolver() { + if (resolverType.equals(Config.Resolver.RPC)) { + return Config.DEFAULT_RPC_PORT; + } + return Config.DEFAULT_IN_PROCESS_PORT; + } } } diff --git a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdOptionsTest.java b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdOptionsTest.java index a2e626a73..c7d99d981 100644 --- a/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdOptionsTest.java +++ b/providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/FlagdOptionsTest.java @@ -1,29 +1,24 @@ package dev.openfeature.contrib.providers.flagd; import io.opentelemetry.api.OpenTelemetry; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.mockito.Mockito; import java.util.function.Function; -import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_CACHE; -import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_HOST; -import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_MAX_CACHE_SIZE; -import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_MAX_EVENT_STREAM_RETRIES; -import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_PORT; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static dev.openfeature.contrib.providers.flagd.Config.*; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.*; -public class FlagdOptionsTest { +class FlagdOptionsTest { @Test - public void TestDefaults() { + void TestDefaults() { final FlagdOptions builder = FlagdOptions.builder().build(); assertEquals(DEFAULT_HOST, builder.getHost()); - assertEquals(DEFAULT_PORT, Integer.toString(builder.getPort())); + assertEquals(DEFAULT_RPC_PORT, Integer.toString(builder.getPort())); assertFalse(builder.isTls()); assertNull(builder.getCertPath()); assertNull(builder.getSocketPath()); @@ -33,11 +28,11 @@ public void TestDefaults() { assertNull(builder.getSelector()); assertNull(builder.getOpenTelemetry()); assertNull(builder.getOfflineFlagSourcePath()); - assertEquals(Config.Resolver.RPC, builder.getResolverType()); + assertEquals(Resolver.RPC, builder.getResolverType()); } @Test - public void TestBuilderOptions() { + void TestBuilderOptions() { OpenTelemetry openTelemetry = Mockito.mock(OpenTelemetry.class); FlagdOptions flagdOptions = FlagdOptions.builder() @@ -51,7 +46,7 @@ public void TestBuilderOptions() { .selector("app=weatherApp") .offlineFlagSourcePath("some-path") .openTelemetry(openTelemetry) - .resolverType(Config.Resolver.IN_PROCESS) + .resolverType(Resolver.IN_PROCESS) .build(); assertEquals("https://hosted-flagd", flagdOptions.getHost()); @@ -64,27 +59,27 @@ public void TestBuilderOptions() { assertEquals("app=weatherApp", flagdOptions.getSelector()); assertEquals("some-path", flagdOptions.getOfflineFlagSourcePath()); assertEquals(openTelemetry, flagdOptions.getOpenTelemetry()); - assertEquals(Config.Resolver.IN_PROCESS, flagdOptions.getResolverType()); + assertEquals(Resolver.IN_PROCESS, flagdOptions.getResolverType()); } @Test - public void testValueProviderForEdgeCase_valid() { + void testValueProviderForEdgeCase_valid() { Function valueProvider = s -> "in-process"; - assertEquals(Config.Resolver.IN_PROCESS, Config.fromValueProvider(valueProvider)); + assertEquals(Resolver.IN_PROCESS, Config.fromValueProvider(valueProvider)); valueProvider = s -> "IN-PROCESS"; - assertEquals(Config.Resolver.IN_PROCESS, Config.fromValueProvider(valueProvider)); + assertEquals(Resolver.IN_PROCESS, Config.fromValueProvider(valueProvider)); valueProvider = s -> "rpc"; - assertEquals(Config.Resolver.RPC, Config.fromValueProvider(valueProvider)); + assertEquals(Resolver.RPC, Config.fromValueProvider(valueProvider)); valueProvider = s -> "RPC"; - assertEquals(Config.Resolver.RPC, Config.fromValueProvider(valueProvider)); + assertEquals(Resolver.RPC, Config.fromValueProvider(valueProvider)); } @Test - public void testValueProviderForEdgeCase_invalid() { + void testValueProviderForEdgeCase_invalid() { Function dummy = s -> "some-other"; assertEquals(Config.DEFAULT_RESOLVER_TYPE, Config.fromValueProvider(dummy)); @@ -92,4 +87,64 @@ public void testValueProviderForEdgeCase_invalid() { assertEquals(Config.DEFAULT_RESOLVER_TYPE, Config.fromValueProvider(dummy)); } + @Test + @Disabled("Currently there is no defined way on how to set environment variables for tests") + void testInProcessProviderFromEnv_noPortConfigured_defaultsToCorrectPort() { + FlagdOptions flagdOptions = FlagdOptions.builder().build(); + + assertThat(flagdOptions.getResolverType()).isEqualTo(Resolver.IN_PROCESS); + assertThat(flagdOptions.getPort()).isEqualTo(Integer.parseInt(DEFAULT_IN_PROCESS_PORT)); + } + + @Test + void testInProcessProvider_noPortConfigured_defaultsToCorrectPort() { + FlagdOptions flagdOptions = FlagdOptions.builder() + .resolverType(Resolver.IN_PROCESS) + .build(); + + assertThat(flagdOptions.getResolverType()).isEqualTo(Resolver.IN_PROCESS); + assertThat(flagdOptions.getPort()).isEqualTo(Integer.parseInt(DEFAULT_IN_PROCESS_PORT)); + } + + @Test + @Disabled("Currently there is no defined way on how to set environment variables for tests") + void testInProcessProviderFromEnv_portConfigured_usesConfiguredPort() { + FlagdOptions flagdOptions = FlagdOptions.builder() + .port(1000) + .build(); + + assertThat(flagdOptions.getResolverType()).isEqualTo(Resolver.IN_PROCESS); + assertThat(flagdOptions.getPort()).isEqualTo(1000); + } + + @Test + @Disabled("Currently there is no defined way on how to set environment variables for tests") + void testRpcProviderFromEnv_noPortConfigured_defaultsToCorrectPort() { + FlagdOptions flagdOptions = FlagdOptions.builder().build(); + + assertThat(flagdOptions.getResolverType()).isEqualTo(Resolver.RPC); + assertThat(flagdOptions.getPort()).isEqualTo(Integer.parseInt(DEFAULT_RPC_PORT)); + } + + @Test + void testRpcProvider_noPortConfigured_defaultsToCorrectPort() { + FlagdOptions flagdOptions = FlagdOptions.builder() + .resolverType(Resolver.RPC) + .build(); + + assertThat(flagdOptions.getResolverType()).isEqualTo(Resolver.RPC); + assertThat(flagdOptions.getPort()).isEqualTo(Integer.parseInt(DEFAULT_RPC_PORT)); + } + + @Test + @Disabled("Currently there is no defined way on how to set environment variables for tests") + void testRpcProviderFromEnv_portConfigured_usesConfiguredPort() { + FlagdOptions flagdOptions = FlagdOptions.builder() + .port(1534) + .build(); + + assertThat(flagdOptions.getResolverType()).isEqualTo(Resolver.RPC); + assertThat(flagdOptions.getPort()).isEqualTo(1534); + + } } From 72e9bf646b8800762e1e0f0dc5a08b9a5e6aed7f Mon Sep 17 00:00:00 2001 From: Daniel Zehetner Date: Mon, 10 Jun 2024 17:06:05 +0200 Subject: [PATCH 2/2] Fixed checkstyle violations and some minor improvements out of PR comments Signed-off-by: Daniel Zehetner --- .../contrib/providers/flagd/Config.java | 5 ---- .../contrib/providers/flagd/FlagdOptions.java | 26 ++++++++++++++++++- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/Config.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/Config.java index 5d5cee709..9f7a51dad 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/Config.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/Config.java @@ -12,11 +12,6 @@ public final class Config { static final Resolver DEFAULT_RESOLVER_TYPE = Resolver.RPC; static final String DEFAULT_RPC_PORT = "8013"; - /** - * @deprecated Use {@link Config#DEFAULT_RPC_PORT} or {@link Config#DEFAULT_IN_PROCESS_PORT} - */ - @Deprecated - static final String DEFAULT_PORT = DEFAULT_RPC_PORT; static final String DEFAULT_IN_PROCESS_PORT = "8015"; static final String DEFAULT_TLS = "false"; static final String DEFAULT_HOST = "localhost"; diff --git a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdOptions.java b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdOptions.java index 81362dcf2..2f4e20b9d 100644 --- a/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdOptions.java +++ b/providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdOptions.java @@ -5,8 +5,27 @@ import lombok.Builder; import lombok.Getter; -import static dev.openfeature.contrib.providers.flagd.Config.*; +import static dev.openfeature.contrib.providers.flagd.Config.BASE_EVENT_STREAM_RETRY_BACKOFF_MS; +import static dev.openfeature.contrib.providers.flagd.Config.BASE_EVENT_STREAM_RETRY_BACKOFF_MS_ENV_VAR_NAME; +import static dev.openfeature.contrib.providers.flagd.Config.CACHE_ENV_VAR_NAME; +import static dev.openfeature.contrib.providers.flagd.Config.DEADLINE_MS_ENV_VAR_NAME; +import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_CACHE; +import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_DEADLINE; +import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_HOST; +import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_MAX_CACHE_SIZE; +import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_MAX_EVENT_STREAM_RETRIES; +import static dev.openfeature.contrib.providers.flagd.Config.DEFAULT_TLS; +import static dev.openfeature.contrib.providers.flagd.Config.HOST_ENV_VAR_NAME; +import static dev.openfeature.contrib.providers.flagd.Config.MAX_CACHE_SIZE_ENV_VAR_NAME; +import static dev.openfeature.contrib.providers.flagd.Config.MAX_EVENT_STREAM_RETRIES_ENV_VAR_NAME; +import static dev.openfeature.contrib.providers.flagd.Config.OFFLINE_SOURCE_PATH; +import static dev.openfeature.contrib.providers.flagd.Config.PORT_ENV_VAR_NAME; +import static dev.openfeature.contrib.providers.flagd.Config.SERVER_CERT_PATH_ENV_VAR_NAME; +import static dev.openfeature.contrib.providers.flagd.Config.SOCKET_PATH_ENV_VAR_NAME; +import static dev.openfeature.contrib.providers.flagd.Config.SOURCE_SELECTOR_ENV_VAR_NAME; +import static dev.openfeature.contrib.providers.flagd.Config.TLS_ENV_VAR_NAME; import static dev.openfeature.contrib.providers.flagd.Config.fallBackToEnvOrDefault; +import static dev.openfeature.contrib.providers.flagd.Config.fromValueProvider; /** * FlagdOptions is a builder to build flagd provider options. @@ -105,6 +124,11 @@ public class FlagdOptions { private OpenTelemetry openTelemetry; + /** + * Builder overwrite in order to customize the "build" method. + * + * @return the flagd options builder + */ public static FlagdOptionsBuilder builder() { return new FlagdOptionsBuilder() { @Override