From 3c95eea1705b2d4031d6512344d28cd447846571 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Wed, 23 Apr 2025 14:49:18 -0600 Subject: [PATCH 01/14] update callback manager to not use lambdas --- .../cloud/eppo/callback/CallbackManager.java | 19 ++++-- .../eppo/callback/CallbackManagerTest.java | 61 +++++++++++++------ 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/src/main/java/cloud/eppo/callback/CallbackManager.java b/src/main/java/cloud/eppo/callback/CallbackManager.java index a227cb27..cbe65919 100644 --- a/src/main/java/cloud/eppo/callback/CallbackManager.java +++ b/src/main/java/cloud/eppo/callback/CallbackManager.java @@ -3,7 +3,7 @@ import java.util.Map; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; -import java.util.function.Consumer; +import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -12,12 +12,19 @@ * * @param The type of data that will be passed to the callbacks */ -public class CallbackManager { +public class CallbackManager { + public interface Dispatcher { + public void dispatch(C callback, T data); + } + + private final Dispatcher dispatcher; + private static final Logger log = LoggerFactory.getLogger(CallbackManager.class); - private final Map> subscribers; + private final Map subscribers; - public CallbackManager() { + public CallbackManager(@NotNull Dispatcher dispatcher) { this.subscribers = new ConcurrentHashMap<>(); + this.dispatcher = dispatcher; } /** @@ -26,7 +33,7 @@ public CallbackManager() { * @param callback The callback function to be called with event data * @return A Runnable that can be called to unsubscribe the callback */ - public Runnable subscribe(Consumer callback) { + public Runnable subscribe(C callback) { String id = UUID.randomUUID().toString(); subscribers.put(id, callback); @@ -44,7 +51,7 @@ public void notifyCallbacks(T data) { .forEach( callback -> { try { - callback.accept(data); + dispatcher.dispatch(callback, data); } catch (Exception e) { log.error("Eppo SDK: Error thrown by callback: {}", e.getMessage()); } diff --git a/src/test/java/cloud/eppo/callback/CallbackManagerTest.java b/src/test/java/cloud/eppo/callback/CallbackManagerTest.java index 97ba0c7b..b5ae5ac7 100644 --- a/src/test/java/cloud/eppo/callback/CallbackManagerTest.java +++ b/src/test/java/cloud/eppo/callback/CallbackManagerTest.java @@ -8,12 +8,24 @@ public class CallbackManagerTest { + private CallbackManager> createCallbackManager() { + return new CallbackManager<>( + // Can't use a lambda as they were only introduced in java8 + new CallbackManager.Dispatcher>() { + @Override + public void dispatch(List callback, String data) { + callback.add(data); + } + }); + } + @Test public void testSubscribeAndNotify() { - CallbackManager CallbackManager = new CallbackManager<>(); + CallbackManager> CallbackManager = createCallbackManager(); + List received = new ArrayList<>(); - Runnable unsubscribe = CallbackManager.subscribe(received::add); + Runnable unsubscribe = CallbackManager.subscribe(received); CallbackManager.notifyCallbacks("test message"); assertEquals(1, received.size()); @@ -26,15 +38,20 @@ public void testSubscribeAndNotify() { @Test public void testThrowingCallback() { - CallbackManager manager = new CallbackManager<>(); + // The helper-created manager includes a dispatcher which pushes the data to the `add` method. + CallbackManager> manager = createCallbackManager(); List received = new ArrayList<>(); - Runnable unsubscribe1 = - manager.subscribe( - (s) -> { - throw new RuntimeException("test message"); - }); - Runnable unsubscribe2 = manager.subscribe(received::add); + List throwingList = + new ArrayList() { + @Override + public boolean add(String o) { + throw new RuntimeException("test message"); + } + }; + + Runnable unsubscribe1 = manager.subscribe(throwingList); + Runnable unsubscribe2 = manager.subscribe(received); manager.notifyCallbacks("value"); assertEquals(1, received.size()); @@ -42,12 +59,20 @@ public void testThrowingCallback() { @Test public void testMultipleSubscribers() { - CallbackManager manager = new CallbackManager<>(); + CallbackManager> manager = + new CallbackManager<>( + new CallbackManager.Dispatcher>() { + + @Override + public void dispatch(List callback, Integer data) { + callback.add(data); + } + }); List received1 = new ArrayList<>(); List received2 = new ArrayList<>(); - manager.subscribe(received1::add); - manager.subscribe(received2::add); + manager.subscribe(received1); + manager.subscribe(received2); manager.notifyCallbacks(42); @@ -59,11 +84,11 @@ public void testMultipleSubscribers() { @Test public void testUnsubscribe() { - CallbackManager manager = new CallbackManager<>(); + CallbackManager> manager = createCallbackManager(); List received = new ArrayList<>(); - Runnable unsubscribe1 = manager.subscribe(received::add); - Runnable unsubscribe2 = manager.subscribe(received::add); + Runnable unsubscribe1 = manager.subscribe(received); + Runnable unsubscribe2 = manager.subscribe(received); manager.notifyCallbacks("value"); assertEquals(2, received.size()); @@ -83,12 +108,12 @@ public void testUnsubscribe() { @Test public void testClear() { - CallbackManager manager = new CallbackManager<>(); + CallbackManager> manager = createCallbackManager(); List received = new ArrayList<>(); - manager.subscribe(received::add); - manager.subscribe(received::add); + manager.subscribe(received); + manager.subscribe(received); manager.notifyCallbacks("value"); assertEquals(2, received.size()); From 933c21d8d98cd1c16df6eb70d6cfb883ccc018c7 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Thu, 24 Apr 2025 14:08:10 -0600 Subject: [PATCH 02/14] rip out completable futures --- src/main/java/cloud/eppo/BaseEppoClient.java | 68 ++--- .../cloud/eppo/ConfigurationRequestor.java | 150 ++++------- .../java/cloud/eppo/ConfigurationStore.java | 4 +- src/main/java/cloud/eppo/EppoHttpClient.java | 56 ++-- .../java/cloud/eppo/IConfigurationStore.java | 4 +- src/main/java/cloud/eppo/IEppoHttpClient.java | 21 ++ .../java/cloud/eppo/api/Configuration.java | 5 + .../cloud/eppo/api/EppoActionCallback.java | 7 + .../cloud/eppo/BaseEppoClientBanditTest.java | 10 +- .../java/cloud/eppo/BaseEppoClientTest.java | 123 +++++---- .../eppo/ConfigurationRequestorTest.java | 254 ++++++++---------- .../java/cloud/eppo/helpers/TestUtils.java | 105 +++++--- 12 files changed, 423 insertions(+), 384 deletions(-) create mode 100644 src/main/java/cloud/eppo/IEppoHttpClient.java create mode 100644 src/main/java/cloud/eppo/api/EppoActionCallback.java diff --git a/src/main/java/cloud/eppo/BaseEppoClient.java b/src/main/java/cloud/eppo/BaseEppoClient.java index ea16d787..d61c3617 100644 --- a/src/main/java/cloud/eppo/BaseEppoClient.java +++ b/src/main/java/cloud/eppo/BaseEppoClient.java @@ -18,8 +18,6 @@ import java.util.HashMap; import java.util.Map; import java.util.Timer; -import java.util.concurrent.CompletableFuture; -import java.util.function.Consumer; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; @@ -43,16 +41,10 @@ public class BaseEppoClient { private final IAssignmentCache banditAssignmentCache; private Timer pollTimer; - @Nullable protected CompletableFuture getInitialConfigFuture() { - return initialConfigFuture; - } - - private final CompletableFuture initialConfigFuture; - // Fields useful for testing in situations where we want to mock the http client or configuration // store (accessed via reflection) /** @noinspection FieldMayBeFinal */ - private static EppoHttpClient httpClientOverride = null; + private static IEppoHttpClient httpClientOverride = null; // It is important that the bandit assignment cache expire with a short-enough TTL to last about // one user session. @@ -70,7 +62,7 @@ protected BaseEppoClient( boolean isGracefulMode, boolean expectObfuscatedConfig, boolean supportBandits, - @Nullable CompletableFuture initialConfiguration, + @Nullable Configuration initialConfiguration, @Nullable IAssignmentCache assignmentCache, @Nullable IAssignmentCache banditAssignmentCache) { @@ -81,7 +73,7 @@ protected BaseEppoClient( this.assignmentCache = assignmentCache; this.banditAssignmentCache = banditAssignmentCache; - EppoHttpClient httpClient = + IEppoHttpClient httpClient = buildHttpClient(apiBaseUrl, new SDKKey(apiKey), sdkName, sdkVersion); this.configurationStore = configurationStore != null ? configurationStore : new ConfigurationStore(); @@ -90,10 +82,10 @@ protected BaseEppoClient( requestor = new ConfigurationRequestor( this.configurationStore, httpClient, expectObfuscatedConfig, supportBandits); - initialConfigFuture = - initialConfiguration != null - ? requestor.setInitialConfiguration(initialConfiguration) - : null; + + if (initialConfiguration != null) { + requestor.setInitialConfiguration(initialConfiguration); + } this.assignmentLogger = assignmentLogger; this.banditLogger = banditLogger; @@ -103,7 +95,7 @@ protected BaseEppoClient( this.sdkVersion = sdkVersion; } - private EppoHttpClient buildHttpClient( + private IEppoHttpClient buildHttpClient( String apiBaseUrl, SDKKey sdkKey, String sdkName, String sdkVersion) { ApiEndpoints endpointHelper = new ApiEndpoints(sdkKey, apiBaseUrl); @@ -172,22 +164,25 @@ protected void startPolling(long pollingIntervalMs, long pollingJitterMs) { fetchConfigurationsTask.scheduleNext(); } - protected CompletableFuture loadConfigurationAsync() { - CompletableFuture future = new CompletableFuture<>(); + protected void loadConfigurationAsync(EppoActionCallback callback) { - requestor - .fetchAndSaveFromRemoteAsync() - .exceptionally( - ex -> { - log.error("Encountered Exception while loading configuration", ex); - if (!isGracefulMode) { - future.completeExceptionally(ex); - } - return null; - }) - .thenAccept(future::complete); + requestor.fetchAndSaveFromRemoteAsync( + new EppoActionCallback() { + @Override + public void onSuccess(Configuration data) { + callback.onSuccess(data); + } - return future; + @Override + public void onFailure(Throwable error) { + log.error("Encountered Exception while loading configuration", error); + if (isGracefulMode) { + callback.onSuccess(null); + } else { + callback.onFailure(error); + } + } + }); } protected EppoValue getTypedAssignment( @@ -577,14 +572,23 @@ public void setIsGracefulFailureMode(boolean isGracefulFailureMode) { this.isGracefulMode = isGracefulFailureMode; } + public abstract static class EppoListener implements Configuration.ConfigurationCallback { + @Override + public void accept(Configuration configuration) { + this.onConfigurationChanged(configuration); + } + + public abstract void onConfigurationChanged(Configuration newConfig); + } + /** * Subscribe to changes to the configuration. * - * @param callback A function to be executed when the configuration changes. + * @param callback A listener which is notified of configuration changes. * @return a Runnable which, when called unsubscribes the callback from configuration change * events. */ - public Runnable onConfigurationChange(Consumer callback) { + public Runnable onConfigurationChange(EppoListener callback) { return requestor.onConfigurationChange(callback); } diff --git a/src/main/java/cloud/eppo/ConfigurationRequestor.java b/src/main/java/cloud/eppo/ConfigurationRequestor.java index f20f8aa8..34876fe1 100644 --- a/src/main/java/cloud/eppo/ConfigurationRequestor.java +++ b/src/main/java/cloud/eppo/ConfigurationRequestor.java @@ -1,10 +1,8 @@ package cloud.eppo; import cloud.eppo.api.Configuration; +import cloud.eppo.api.EppoActionCallback; import cloud.eppo.callback.CallbackManager; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; -import java.util.function.Consumer; import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -12,20 +10,28 @@ public class ConfigurationRequestor { private static final Logger log = LoggerFactory.getLogger(ConfigurationRequestor.class); - private final EppoHttpClient client; + private final IEppoHttpClient client; private final IConfigurationStore configurationStore; private final boolean expectObfuscatedConfig; private final boolean supportBandits; - private CompletableFuture remoteFetchFuture = null; - private CompletableFuture configurationFuture = null; private boolean initialConfigSet = false; - private final CallbackManager configChangeManager = new CallbackManager<>(); + private final CallbackManager + configChangeManager = + new CallbackManager<>( + // no lambdas before java8 + new CallbackManager.Dispatcher() { + @Override + public void dispatch( + Configuration.ConfigurationCallback callback, Configuration data) { + callback.accept(data); + } + }); public ConfigurationRequestor( @NotNull IConfigurationStore configurationStore, - @NotNull EppoHttpClient client, + @NotNull IEppoHttpClient client, boolean expectObfuscatedConfig, boolean supportBandits) { this.configurationStore = configurationStore; @@ -36,49 +42,11 @@ public ConfigurationRequestor( // Synchronously set the initial configuration. public void setInitialConfiguration(@NotNull Configuration configuration) { - if (initialConfigSet || this.configurationFuture != null) { + if (initialConfigSet) { throw new IllegalStateException("Initial configuration has already been set"); } - initialConfigSet = saveConfigurationAndNotify(configuration).thenApply(v -> true).join(); - } - - /** - * Asynchronously sets the initial configuration. Resolves to `true` if the initial configuration - * was used, false if not (due to being empty, a fetched config taking precedence, etc.) - */ - public CompletableFuture setInitialConfiguration( - @NotNull CompletableFuture configurationFuture) { - if (initialConfigSet || this.configurationFuture != null) { - throw new IllegalStateException("Configuration future has already been set"); - } - this.configurationFuture = - configurationFuture - .thenApply( - (config) -> { - synchronized (configurationStore) { - if (config == null || config.isEmpty()) { - log.debug("Initial configuration future returned empty/null"); - return false; - } else if (remoteFetchFuture != null - && remoteFetchFuture.isDone() - && !remoteFetchFuture.isCompletedExceptionally()) { - // Don't clobber a successful fetch. - log.debug("Fetch has completed; ignoring initial config load."); - return false; - } else { - initialConfigSet = - saveConfigurationAndNotify(config).thenApply((s) -> true).join(); - return true; - } - } - }) - .exceptionally( - (e) -> { - log.error("Error setting initial config", e); - return false; - }); - return this.configurationFuture; + initialConfigSet = saveConfigurationAndNotify(configuration); } /** Loads configuration synchronously from the API server. */ @@ -98,62 +66,58 @@ void fetchAndSaveFromRemote() { configBuilder.banditParameters(banditParametersJsonBytes); } - saveConfigurationAndNotify(configBuilder.build()).join(); + saveConfigurationAndNotify(configBuilder.build()); } /** Loads configuration asynchronously from the API server, off-thread. */ - CompletableFuture fetchAndSaveFromRemoteAsync() { + public void fetchAndSaveFromRemoteAsync(EppoActionCallback callback) { log.debug("Fetching configuration from API server"); final Configuration lastConfig = configurationStore.getConfiguration(); - if (remoteFetchFuture != null && !remoteFetchFuture.isDone()) { - log.debug("Remote fetch is active. Cancelling and restarting"); - remoteFetchFuture.cancel(true); - remoteFetchFuture = null; - } - - remoteFetchFuture = - client - .getAsync(Constants.FLAG_CONFIG_ENDPOINT) - .thenCompose( - flagConfigJsonBytes -> { - synchronized (this) { - Configuration.Builder configBuilder = - Configuration.builder(flagConfigJsonBytes, expectObfuscatedConfig) - .banditParametersFromConfig( - lastConfig); // possibly reuse last bandit models loaded. - - if (supportBandits && configBuilder.requiresUpdatedBanditModels()) { - byte[] banditParametersJsonBytes; - try { - banditParametersJsonBytes = - client.getAsync(Constants.BANDIT_ENDPOINT).get(); - } catch (InterruptedException | ExecutionException e) { - log.error("Error fetching from remote: " + e.getMessage()); - throw new RuntimeException(e); - } - if (banditParametersJsonBytes != null) { - configBuilder.banditParameters(banditParametersJsonBytes); - } - } - - return saveConfigurationAndNotify(configBuilder.build()); - } - }); - return remoteFetchFuture; - } + client.getAsync( + Constants.FLAG_CONFIG_ENDPOINT, + new IEppoHttpClient.EppoHttpCallback() { + @Override + public void onSuccess(byte[] flagConfigJsonBytes) { + synchronized (this) { + Configuration.Builder configBuilder = + Configuration.builder(flagConfigJsonBytes) + .banditParametersFromConfig( + lastConfig); // possibly reuse last bandit models loaded. + + if (supportBandits && configBuilder.requiresUpdatedBanditModels()) { + byte[] banditParametersJsonBytes; + + banditParametersJsonBytes = client.get(Constants.BANDIT_ENDPOINT); + + if (banditParametersJsonBytes != null) { + configBuilder.banditParameters(banditParametersJsonBytes); + } + } + + Configuration config = configBuilder.build(); + saveConfigurationAndNotify(config); + callback.onSuccess(config); + } + } - private CompletableFuture saveConfigurationAndNotify(Configuration configuration) { - CompletableFuture saveFuture = configurationStore.saveConfiguration(configuration); - return saveFuture.thenRun( - () -> { - synchronized (configChangeManager) { - configChangeManager.notifyCallbacks(configuration); + @Override + public void onFailure(Throwable error) { + callback.onFailure(error); } }); } - public Runnable onConfigurationChange(Consumer callback) { + private boolean saveConfigurationAndNotify(Configuration configuration) { + configurationStore.saveConfiguration(configuration); + synchronized (configChangeManager) { + configChangeManager.notifyCallbacks(configuration); + } + + return true; + } + + public Runnable onConfigurationChange(Configuration.ConfigurationCallback callback) { return configChangeManager.subscribe(callback); } } diff --git a/src/main/java/cloud/eppo/ConfigurationStore.java b/src/main/java/cloud/eppo/ConfigurationStore.java index 6a01bc2c..45a2fb67 100644 --- a/src/main/java/cloud/eppo/ConfigurationStore.java +++ b/src/main/java/cloud/eppo/ConfigurationStore.java @@ -1,7 +1,6 @@ package cloud.eppo; import cloud.eppo.api.Configuration; -import java.util.concurrent.CompletableFuture; import org.jetbrains.annotations.NotNull; /** Memory-only configuration store. */ @@ -12,9 +11,8 @@ public class ConfigurationStore implements IConfigurationStore { public ConfigurationStore() {} - public CompletableFuture saveConfiguration(@NotNull final Configuration configuration) { + public void saveConfiguration(@NotNull final Configuration configuration) { this.configuration = configuration; - return CompletableFuture.completedFuture(null); } @NotNull public Configuration getConfiguration() { diff --git a/src/main/java/cloud/eppo/EppoHttpClient.java b/src/main/java/cloud/eppo/EppoHttpClient.java index 656e844f..a55ac65b 100644 --- a/src/main/java/cloud/eppo/EppoHttpClient.java +++ b/src/main/java/cloud/eppo/EppoHttpClient.java @@ -1,10 +1,8 @@ package cloud.eppo; +import cloud.eppo.exception.InvalidApiKeyException; import java.io.IOException; import java.net.HttpURLConnection; -import java.util.Arrays; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import okhttp3.Call; import okhttp3.Callback; @@ -16,7 +14,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class EppoHttpClient { +public class EppoHttpClient implements IEppoHttpClient { private static final Logger log = LoggerFactory.getLogger(EppoHttpClient.class); private final OkHttpClient client; @@ -44,18 +42,25 @@ private static OkHttpClient buildOkHttpClient() { return builder.build(); } + @Override public byte[] get(String path) { - try { - // Wait and return the async get. - return getAsync(path).get(); - } catch (InterruptedException | ExecutionException e) { - log.error("Config fetch interrupted", e); - throw new RuntimeException(e); + Request request = buildRequest(path); + try (Response response = client.newCall(request).execute()) { + if (response.isSuccessful() && response.body() != null) { + return response.body().bytes(); + } + + throw response.code() == HttpURLConnection.HTTP_FORBIDDEN + ? new InvalidApiKeyException("Invalid API key") + : new HttpException("Fetch failed with status code: " + response.code()); + + } catch (IOException e) { + throw new HttpException("Http request failure", e); } } - public CompletableFuture getAsync(String path) { - CompletableFuture future = new CompletableFuture<>(); + @Override + public void getAsync(String path, EppoHttpCallback callback) { Request request = buildRequest(path); client .newCall(request) @@ -64,38 +69,27 @@ public CompletableFuture getAsync(String path) { @Override public void onResponse(@NotNull Call call, @NotNull Response response) { if (response.isSuccessful() && response.body() != null) { - log.debug("Fetch successful"); try { - future.complete(response.body().bytes()); + callback.onSuccess(response.body().bytes()); } catch (IOException ex) { - future.completeExceptionally( - new RuntimeException( + callback.onFailure( + new HttpException( "Failed to read response from URL {}" + request.url(), ex)); } } else { - if (response.code() == HttpURLConnection.HTTP_FORBIDDEN) { - future.completeExceptionally(new RuntimeException("Invalid API key")); - } else { - log.debug("Fetch failed with status code: {}", response.code()); - future.completeExceptionally( - new RuntimeException("Bad response from URL " + request.url())); - } + callback.onFailure( + response.code() == HttpURLConnection.HTTP_FORBIDDEN + ? new InvalidApiKeyException("Invalid API key") + : new HttpException("Bad response from URL " + request.url())); } response.close(); } @Override public void onFailure(@NotNull Call call, @NotNull IOException e) { - log.error( - "Http request failure: {} {}", - e.getMessage(), - Arrays.toString(e.getStackTrace()), - e); - future.completeExceptionally( - new RuntimeException("Unable to fetch from URL " + request.url())); + callback.onFailure(e); } }); - return future; } private Request buildRequest(String path) { diff --git a/src/main/java/cloud/eppo/IConfigurationStore.java b/src/main/java/cloud/eppo/IConfigurationStore.java index a1666e9e..61339589 100644 --- a/src/main/java/cloud/eppo/IConfigurationStore.java +++ b/src/main/java/cloud/eppo/IConfigurationStore.java @@ -1,7 +1,6 @@ package cloud.eppo; import cloud.eppo.api.Configuration; -import java.util.concurrent.CompletableFuture; import org.jetbrains.annotations.NotNull; /** @@ -9,7 +8,8 @@ * persisting configuration data across sessions. */ public interface IConfigurationStore { + @NotNull Configuration getConfiguration(); - CompletableFuture saveConfiguration(Configuration configuration); + void saveConfiguration(Configuration configuration); } diff --git a/src/main/java/cloud/eppo/IEppoHttpClient.java b/src/main/java/cloud/eppo/IEppoHttpClient.java new file mode 100644 index 00000000..6ecd6116 --- /dev/null +++ b/src/main/java/cloud/eppo/IEppoHttpClient.java @@ -0,0 +1,21 @@ +package cloud.eppo; + +import cloud.eppo.api.EppoActionCallback; + +public interface IEppoHttpClient { + byte[] get(String path); + + void getAsync(String path, EppoHttpCallback callback); + + interface EppoHttpCallback extends EppoActionCallback {} + + class HttpException extends RuntimeException { + public HttpException(String message) { + super(message); + } + + public HttpException(String message, Throwable cause) { + super(message, cause); + } + } +} diff --git a/src/main/java/cloud/eppo/api/Configuration.java b/src/main/java/cloud/eppo/api/Configuration.java index 8b1e8376..b20a570e 100644 --- a/src/main/java/cloud/eppo/api/Configuration.java +++ b/src/main/java/cloud/eppo/api/Configuration.java @@ -49,6 +49,11 @@ * then check `requiresBanditModels()`. */ public class Configuration { + /** Callback for common operations involving the Configuration. */ + public interface ConfigurationCallback { + void accept(Configuration configuration); + } + private static final ObjectMapper mapper = new ObjectMapper().registerModule(EppoModule.eppoModule()); diff --git a/src/main/java/cloud/eppo/api/EppoActionCallback.java b/src/main/java/cloud/eppo/api/EppoActionCallback.java new file mode 100644 index 00000000..b5d10ecd --- /dev/null +++ b/src/main/java/cloud/eppo/api/EppoActionCallback.java @@ -0,0 +1,7 @@ +package cloud.eppo.api; + +public interface EppoActionCallback { + void onSuccess(T data); + + void onFailure(Throwable error); +} diff --git a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java index 84587ed5..e8ed253f 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java @@ -15,7 +15,6 @@ import java.io.File; import java.io.IOException; import java.util.*; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.stream.Stream; import org.apache.commons.io.FileUtils; @@ -89,11 +88,10 @@ public static void initClient() { private BaseEppoClient initClientWithData( final String initialFlagConfiguration, final String initialBanditParameters) { - CompletableFuture initialConfig = - CompletableFuture.completedFuture( - Configuration.builder(initialFlagConfiguration.getBytes(), false) - .banditParameters(initialBanditParameters) - .build()); + Configuration initialConfig = + Configuration.builder(initialFlagConfiguration.getBytes(), false) + .banditParameters(initialBanditParameters) + .build(); return new BaseEppoClient( DUMMY_BANDIT_API_KEY, diff --git a/src/test/java/cloud/eppo/BaseEppoClientTest.java b/src/test/java/cloud/eppo/BaseEppoClientTest.java index 7c95ccdb..4cee0196 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientTest.java @@ -11,6 +11,7 @@ import cloud.eppo.api.*; import cloud.eppo.cache.LRUInMemoryAssignmentCache; import cloud.eppo.helpers.AssignmentTestCase; +import cloud.eppo.helpers.TestUtils; import cloud.eppo.logging.Assignment; import cloud.eppo.logging.AssignmentLogger; import cloud.eppo.ufc.dto.FlagConfig; @@ -20,16 +21,20 @@ import java.io.File; import java.io.IOException; import java.net.URL; -import java.util.*; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; +import java.util.Date; +import java.util.HashMap; +import java.util.Map; +import java.util.Timer; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Stream; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; import okhttp3.mockwebserver.RecordedRequest; import org.apache.commons.io.FileUtils; -import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -58,14 +63,12 @@ public class BaseEppoClientTest { private final File initialFlagConfigFile = new File("src/test/resources/static/initial-flag-config.json"); - // TODO: async init client tests - private void initClient() { initClient(false, false); } private void initClientWithData( - final CompletableFuture initialFlagConfiguration, + final Configuration initialFlagConfiguration, boolean isConfigObfuscated, boolean isGracefulMode) { mockAssignmentLogger = mock(AssignmentLogger.class); @@ -112,8 +115,10 @@ private void initClient(boolean isGracefulMode, boolean isConfigObfuscated) { log.info("Test client initialized"); } - private CompletableFuture initClientAsync( - boolean isGracefulMode, boolean isConfigObfuscated) { + interface InitCallback extends EppoActionCallback {} + + private void initClientAsync( + boolean isGracefulMode, boolean isConfigObfuscated, InitCallback initCallback) { mockAssignmentLogger = mock(AssignmentLogger.class); eppoClient = @@ -133,7 +138,7 @@ private CompletableFuture initClientAsync( null, null); - return eppoClient.loadConfigurationAsync(); + eppoClient.loadConfigurationAsync(initCallback); } private void initClientWithAssignmentCache(IAssignmentCache cache) { @@ -160,7 +165,7 @@ private void initClientWithAssignmentCache(IAssignmentCache cache) { log.info("Test client initialized"); } - @BeforeEach + @AfterEach public void cleanUp() { // TODO: Clear any caches setBaseClientHttpClientOverrideField(null); @@ -344,12 +349,6 @@ public void testInvalidConfigJSON() { assertEquals("not-populated", result); } - private CompletableFuture immediateConfigFuture( - String config, boolean isObfuscated) { - return CompletableFuture.completedFuture( - Configuration.builder(config.getBytes(), isObfuscated).build()); - } - @Test public void testGracefulInitializationFailure() { // Set up bad HTTP response @@ -396,30 +395,67 @@ public void testNonGracefulInitializationFailure() { } @Test - public void testGracefulAsyncInitializationFailure() { + public void testGracefulAsyncInitializationFailure() throws InterruptedException { // Set up bad HTTP response mockHttpError(); + CountDownLatch initLatch = new CountDownLatch(1); + AtomicBoolean initialized = new AtomicBoolean(false); + // Initialize - CompletableFuture init = initClientAsync(true, false); + initClientAsync( + true, + false, + new InitCallback() { + @Override + public void onSuccess(Configuration data) { + initialized.set(true); + initLatch.countDown(); + } + + @Override + public void onFailure(Throwable error) { + initLatch.countDown(); + } + }); // Wait for initialization; future should not complete exceptionally (equivalent of exception // being thrown). - init.join(); - assertFalse(init.isCompletedExceptionally()); + assertTrue(initLatch.await(1, TimeUnit.SECONDS)); + assertTrue(initialized.get()); } @Test - public void testNonGracefulAsyncInitializationFailure() { + public void testNonGracefulAsyncInitializationFailure() throws InterruptedException { // Set up bad HTTP response mockHttpError(); + CountDownLatch initLatch = new CountDownLatch(1); + AtomicBoolean initialized = new AtomicBoolean(false); + final Throwable[] failure = {null}; + // Initialize - CompletableFuture init = initClientAsync(false, false); + initClientAsync( + false, + false, + new InitCallback() { + @Override + public void onSuccess(Configuration data) { + initialized.set(true); + initLatch.countDown(); + } - // Exceptions thrown in CompletableFutures are wrapped in a CompletionException. - assertThrows(CompletionException.class, init::join); - assertTrue(init.isCompletedExceptionally()); + @Override + public void onFailure(Throwable error) { + failure[0] = error; + initLatch.countDown(); + } + }); + + assertTrue(initLatch.await(1, TimeUnit.SECONDS)); + assertNotNull(failure[0]); + assertInstanceOf(RuntimeException.class, failure[0]); + assertFalse(initialized.get()); } @Test @@ -427,7 +463,7 @@ public void testWithInitialConfiguration() { try { String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, "UTF8"); - initClientWithData(immediateConfigFuture(flagConfig, false), false, true); + initClientWithData(Configuration.builder(flagConfig.getBytes()).build(), false, true); double result = eppoClient.getDoubleAssignment("numeric_flag", "dummy subject", 0); assertEquals(5, result); @@ -441,23 +477,6 @@ public void testWithInitialConfiguration() { } } - @Test - public void testWithInitialConfigurationFuture() throws IOException { - CompletableFuture futureConfig = new CompletableFuture<>(); - byte[] flagConfig = FileUtils.readFileToByteArray(initialFlagConfigFile); - - initClientWithData(futureConfig, false, true); - - double result = eppoClient.getDoubleAssignment("numeric_flag", "dummy subject", 0); - assertEquals(0, result); - - // Now, complete the initial config future and check the value. - futureConfig.complete(Configuration.builder(flagConfig, false).build()); - - result = eppoClient.getDoubleAssignment("numeric_flag", "dummy subject", 0); - assertEquals(5, result); - } - @Test public void testAssignmentEventCorrectlyCreated() { Date testStart = new Date(); @@ -601,7 +620,10 @@ public void run() { @Test public void testPolling() { - EppoHttpClient httpClient = mockHttpResponse(BOOL_FLAG_CONFIG); + TestUtils.MockHttpClient httpClient = mockHttpResponse(BOOL_FLAG_CONFIG); + + TestUtils.MockHttpClient spyClient = spy(httpClient); + setBaseClientHttpClientOverrideField(spyClient); BaseEppoClient client = eppoClient = @@ -625,14 +647,15 @@ public void testPolling() { client.startPolling(20); // Method will be called immediately on init - verify(httpClient, times(1)).get(anyString()); + + verify(spyClient, times(1)).get(anyString()); assertTrue(eppoClient.getBooleanAssignment("bool_flag", "subject1", false)); // Sleep for 25 ms to allow another polling cycle to complete sleepUninterruptedly(25); // Now, the method should have been called twice - verify(httpClient, times(2)).get(anyString()); + verify(spyClient, times(2)).get(anyString()); eppoClient.stopPolling(); assertTrue(eppoClient.getBooleanAssignment("bool_flag", "subject1", false)); @@ -640,10 +663,10 @@ public void testPolling() { sleepUninterruptedly(25); // No more calls since stopped - verify(httpClient, times(2)).get(anyString()); + verify(spyClient, times(2)).get(anyString()); // Set up a different config to be served - when(httpClient.get(anyString())).thenReturn(DISABLED_BOOL_FLAG_CONFIG.getBytes()); + spyClient.changeResponse(DISABLED_BOOL_FLAG_CONFIG.getBytes()); client.startPolling(20); // True until the next config is fetched. @@ -682,7 +705,7 @@ public void testGetConfigurationWithInitialConfig() { String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, "UTF8"); // Initialize client with initial configuration - initClientWithData(immediateConfigFuture(flagConfig, false), false, true); + initClientWithData(Configuration.builder(flagConfig.getBytes()).build(), false, true); // Get configuration Configuration config = eppoClient.getConfiguration(); diff --git a/src/test/java/cloud/eppo/ConfigurationRequestorTest.java b/src/test/java/cloud/eppo/ConfigurationRequestorTest.java index 982153fb..c5e233ce 100644 --- a/src/test/java/cloud/eppo/ConfigurationRequestorTest.java +++ b/src/test/java/cloud/eppo/ConfigurationRequestorTest.java @@ -3,16 +3,18 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; import cloud.eppo.api.Configuration; +import cloud.eppo.api.EppoActionCallback; +import cloud.eppo.helpers.TestUtils; import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.BeforeEach; @@ -25,99 +27,49 @@ public class ConfigurationRequestorTest { private final File differentFlagConfigFile = new File("src/test/resources/static/boolean-flag.json"); - @Test - public void testInitialConfigurationFuture() throws IOException { - IConfigurationStore configStore = Mockito.spy(new ConfigurationStore()); - EppoHttpClient mockHttpClient = mock(EppoHttpClient.class); - - ConfigurationRequestor requestor = - new ConfigurationRequestor(configStore, mockHttpClient, false, true); - - CompletableFuture futureConfig = new CompletableFuture<>(); - byte[] flagConfig = FileUtils.readFileToByteArray(initialFlagConfigFile); - - requestor.setInitialConfiguration(futureConfig); - - // verify config is empty to start - assertTrue(configStore.getConfiguration().isEmpty()); - Mockito.verify(configStore, Mockito.times(0)).saveConfiguration(any()); - - futureConfig.complete(Configuration.builder(flagConfig, false).build()); + private ConfigurationStore mockConfigStore; + private IEppoHttpClient mockHttpClient; + private ConfigurationRequestor requestor; - assertFalse(configStore.getConfiguration().isEmpty()); - Mockito.verify(configStore, Mockito.times(1)).saveConfiguration(any()); - assertNotNull(configStore.getConfiguration().getFlag("numeric_flag")); + @BeforeEach + public void setup() { + mockConfigStore = mock(ConfigurationStore.class); + mockHttpClient = mock(EppoHttpClient.class); + requestor = new ConfigurationRequestor(mockConfigStore, mockHttpClient, false, true); } @Test - public void testInitialConfigurationDoesntClobberFetch() throws IOException { + public void testBrokenFetchDoesntClobberCache() throws IOException, InterruptedException { IConfigurationStore configStore = Mockito.spy(new ConfigurationStore()); - EppoHttpClient mockHttpClient = mock(EppoHttpClient.class); + TestUtils.DelayedHttpClient mockHttpClient = new TestUtils.DelayedHttpClient("".getBytes()); ConfigurationRequestor requestor = new ConfigurationRequestor(configStore, mockHttpClient, false, true); - CompletableFuture initialConfigFuture = new CompletableFuture<>(); String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); - CompletableFuture configFetchFuture = new CompletableFuture<>(); - String fetchedFlagConfig = - FileUtils.readFileToString(differentFlagConfigFile, StandardCharsets.UTF_8); - - when(mockHttpClient.getAsync("/flag-config/v1/config")).thenReturn(configFetchFuture); - - // Set initial config and verify that no config has been set yet. - requestor.setInitialConfiguration(initialConfigFuture); - - assertTrue(configStore.getConfiguration().isEmpty()); - Mockito.verify(configStore, Mockito.times(0)).saveConfiguration(any()); - - // The initial config contains only one flag keyed `numeric_flag`. The fetch response has only - // one flag keyed - // `boolean_flag`. We make sure to complete the fetch future first to verify the cache load does - // not overwrite it. - CompletableFuture handle = requestor.fetchAndSaveFromRemoteAsync(); - - // Resolve the fetch and then the initialConfig - configFetchFuture.complete(fetchedFlagConfig.getBytes(StandardCharsets.UTF_8)); - initialConfigFuture.complete(new Configuration.Builder(flagConfig, false).build()); + // Set initial config and verify that it has been set. + requestor.setInitialConfiguration(Configuration.builder(flagConfig.getBytes()).build()); assertFalse(configStore.getConfiguration().isEmpty()); Mockito.verify(configStore, Mockito.times(1)).saveConfiguration(any()); - // `numeric_flag` is only in the cache which should have been ignored. - assertNull(configStore.getConfiguration().getFlag("numeric_flag")); - - // `boolean_flag` is available only from the fetch - assertNotNull(configStore.getConfiguration().getFlag("boolean_flag")); - } - - @Test - public void testBrokenFetchDoesntClobberCache() throws IOException { - IConfigurationStore configStore = Mockito.spy(new ConfigurationStore()); - EppoHttpClient mockHttpClient = mock(EppoHttpClient.class); - - ConfigurationRequestor requestor = - new ConfigurationRequestor(configStore, mockHttpClient, false, true); - - CompletableFuture initialConfigFuture = new CompletableFuture<>(); - String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); - CompletableFuture configFetchFuture = new CompletableFuture<>(); + CountDownLatch latch = new CountDownLatch(1); + requestor.fetchAndSaveFromRemoteAsync( + new EppoActionCallback() { - when(mockHttpClient.getAsync("/flag-config/v1/config")).thenReturn(configFetchFuture); - - // Set initial config and verify that no config has been set yet. - requestor.setInitialConfiguration(initialConfigFuture); - - assertTrue(configStore.getConfiguration().isEmpty()); - Mockito.verify(configStore, Mockito.times(0)).saveConfiguration(any()); + @Override + public void onSuccess(Configuration data) { + fail("Unexpected success"); + } - requestor.fetchAndSaveFromRemoteAsync(); - - // Resolve the initial config - initialConfigFuture.complete(new Configuration.Builder(flagConfig, false).build()); + @Override + public void onFailure(Throwable error) { + latch.countDown(); + } + }); // Error out the fetch - configFetchFuture.completeExceptionally(new Exception("Intentional exception")); + mockHttpClient.fail(new Exception("Intentional exception")); assertFalse(configStore.getConfiguration().isEmpty()); Mockito.verify(configStore, Mockito.times(1)).saveConfiguration(any()); @@ -126,37 +78,44 @@ public void testBrokenFetchDoesntClobberCache() throws IOException { assertNotNull(configStore.getConfiguration().getFlag("numeric_flag")); assertNull(configStore.getConfiguration().getFlag("boolean_flag")); + + // Ensure fetch failure callback is called + assertTrue(latch.await(1, TimeUnit.SECONDS)); } @Test - public void testCacheWritesAfterBrokenFetch() throws IOException { + public void testCacheWritesAfterBrokenFetch() throws IOException, InterruptedException { IConfigurationStore configStore = Mockito.spy(new ConfigurationStore()); - EppoHttpClient mockHttpClient = mock(EppoHttpClient.class); + TestUtils.DelayedHttpClient mockHttpClient = new TestUtils.DelayedHttpClient("".getBytes()); ConfigurationRequestor requestor = new ConfigurationRequestor(configStore, mockHttpClient, false, true); - CompletableFuture initialConfigFuture = new CompletableFuture<>(); String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); - CompletableFuture configFetchFuture = new CompletableFuture<>(); + Mockito.verify(configStore, Mockito.times(0)).saveConfiguration(any()); + assertTrue(configStore.getConfiguration().isEmpty()); - when(mockHttpClient.getAsync("/flag-config/v1/config")).thenReturn(configFetchFuture); + CountDownLatch latch = new CountDownLatch(1); + requestor.fetchAndSaveFromRemoteAsync( + new EppoActionCallback() { - // Set initial config and verify that no config has been set yet. - requestor.setInitialConfiguration(initialConfigFuture); - Mockito.verify(configStore, Mockito.times(0)).saveConfiguration(any()); + @Override + public void onSuccess(Configuration data) { + fail("Unexpected success"); + } - // default configuration is empty config. - assertTrue(configStore.getConfiguration().isEmpty()); + @Override + public void onFailure(Throwable error) { + latch.countDown(); + } + }); - // Fetch from remote with an error - requestor.fetchAndSaveFromRemoteAsync(); - configFetchFuture.completeExceptionally(new Exception("Intentional exception")); + // Error out the fetch + mockHttpClient.fail(new Exception("Intentional exception")); - // Resolve the initial config after the fetch throws an error. - initialConfigFuture.complete(new Configuration.Builder(flagConfig, false).build()); + // Set initial config and verify that it has been set. + requestor.setInitialConfiguration(Configuration.builder(flagConfig.getBytes()).build()); - // Verify that a configuration was saved by the requestor Mockito.verify(configStore, Mockito.times(1)).saveConfiguration(any()); assertFalse(configStore.getConfiguration().isEmpty()); @@ -164,17 +123,18 @@ public void testCacheWritesAfterBrokenFetch() throws IOException { assertNotNull(configStore.getConfiguration().getFlag("numeric_flag")); assertNull(configStore.getConfiguration().getFlag("boolean_flag")); + + // Ensure fetch failure callback is called + assertTrue(latch.await(1, TimeUnit.SECONDS)); } - private ConfigurationStore mockConfigStore; - private EppoHttpClient mockHttpClient; - private ConfigurationRequestor requestor; + static class ListAddingConfigCallback implements Configuration.ConfigurationCallback { + public final List results = new ArrayList<>(); - @BeforeEach - public void setup() { - mockConfigStore = mock(ConfigurationStore.class); - mockHttpClient = mock(EppoHttpClient.class); - requestor = new ConfigurationRequestor(mockConfigStore, mockHttpClient, false, true); + @Override + public void accept(Configuration configuration) { + results.add(configuration); + } } @Test @@ -182,41 +142,51 @@ public void testConfigurationChangeListener() throws IOException { // Setup mock response String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); when(mockHttpClient.get(anyString())).thenReturn(flagConfig.getBytes()); - when(mockConfigStore.saveConfiguration(any())) - .thenReturn(CompletableFuture.completedFuture(null)); - List receivedConfigs = new ArrayList<>(); + ListAddingConfigCallback receivedConfigs = new ListAddingConfigCallback(); // Subscribe to configuration changes - Runnable unsubscribe = requestor.onConfigurationChange(receivedConfigs::add); + Runnable unsubscribe = requestor.onConfigurationChange(receivedConfigs); // Initial fetch should trigger the callback requestor.fetchAndSaveFromRemote(); - assertEquals(1, receivedConfigs.size()); + assertEquals(1, receivedConfigs.results.size()); // Another fetch should trigger the callback again (fetches aren't optimized with eTag yet). requestor.fetchAndSaveFromRemote(); - assertEquals(2, receivedConfigs.size()); + assertEquals(2, receivedConfigs.results.size()); // Unsubscribe should prevent further callbacks unsubscribe.run(); requestor.fetchAndSaveFromRemote(); - assertEquals(2, receivedConfigs.size()); // Count should remain the same + assertEquals(2, receivedConfigs.results.size()); // Count should remain the same } @Test public void testMultipleConfigurationChangeListeners() { // Setup mock response when(mockHttpClient.get(anyString())).thenReturn("{}".getBytes()); - when(mockConfigStore.saveConfiguration(any())) - .thenReturn(CompletableFuture.completedFuture(null)); AtomicInteger callCount1 = new AtomicInteger(0); AtomicInteger callCount2 = new AtomicInteger(0); // Subscribe multiple listeners - Runnable unsubscribe1 = requestor.onConfigurationChange(v -> callCount1.incrementAndGet()); - Runnable unsubscribe2 = requestor.onConfigurationChange(v -> callCount2.incrementAndGet()); + Runnable unsubscribe1 = + requestor.onConfigurationChange( + new Configuration.ConfigurationCallback() { + @Override + public void accept(Configuration configuration) { + callCount1.incrementAndGet(); + } + }); + Runnable unsubscribe2 = + requestor.onConfigurationChange( + new Configuration.ConfigurationCallback() { + @Override + public void accept(Configuration configuration) { + callCount2.incrementAndGet(); + } + }); // Fetch should trigger both callbacks requestor.fetchAndSaveFromRemote(); @@ -238,31 +208,28 @@ public void testMultipleConfigurationChangeListeners() { @Test public void testConfigurationChangeListenerIgnoresFailedFetch() { - // Setup mock response to simulate failure - when(mockHttpClient.get(anyString())).thenThrow(new RuntimeException("Fetch failed")); + // throw on get + when(mockHttpClient.get(anyString())).thenThrow(new RuntimeException("fetch failed")); AtomicInteger callCount = new AtomicInteger(0); requestor.onConfigurationChange(v -> callCount.incrementAndGet()); - // Failed fetch should not trigger the callback + // Failed save should not trigger the callback try { requestor.fetchAndSaveFromRemote(); - } catch (Exception e) { - // Expected + } catch (RuntimeException e) { + // Pass } assertEquals(0, callCount.get()); } @Test public void testConfigurationChangeListenerIgnoresFailedSave() { - // Setup mock responses + // throw on get when(mockHttpClient.get(anyString())).thenReturn("{}".getBytes()); - when(mockConfigStore.saveConfiguration(any())) - .thenReturn( - CompletableFuture.supplyAsync( - () -> { - throw new RuntimeException("Save failed"); - })); + doThrow(new RuntimeException("Save failed")) + .when(mockConfigStore) + .saveConfiguration(any(Configuration.class)); AtomicInteger callCount = new AtomicInteger(0); requestor.onConfigurationChange(v -> callCount.incrementAndGet()); @@ -277,24 +244,37 @@ public void testConfigurationChangeListenerIgnoresFailedSave() { } @Test - public void testConfigurationChangeListenerAsyncSave() { + public void testConfigurationChangeListenerAsyncSave() throws InterruptedException { // Setup mock responses - when(mockHttpClient.getAsync(anyString())) - .thenReturn(CompletableFuture.completedFuture("{\"flags\":{}}".getBytes())); + TestUtils.DelayedHttpClient mockHttpClient = + new TestUtils.DelayedHttpClient("{\"flags\":{}}".getBytes()); + requestor = new ConfigurationRequestor(mockConfigStore, mockHttpClient, false, true); - CompletableFuture saveFuture = new CompletableFuture<>(); - when(mockConfigStore.saveConfiguration(any())).thenReturn(saveFuture); + CountDownLatch countDownLatch = new CountDownLatch(1); - AtomicInteger callCount = new AtomicInteger(0); - requestor.onConfigurationChange(v -> callCount.incrementAndGet()); + requestor.onConfigurationChange(v -> countDownLatch.countDown()); // Start fetch - CompletableFuture fetch = requestor.fetchAndSaveFromRemoteAsync(); - assertEquals(0, callCount.get()); // Callback should not be called yet + requestor.fetchAndSaveFromRemoteAsync( + new EppoActionCallback() { + @Override + public void onSuccess(Configuration data) { + // This callback should be notified after the registered listeners have been notified. + assertEquals(0, countDownLatch.getCount()); + } + + @Override + public void onFailure(Throwable error) { + fail("unexpected failure"); + } + }); + + assertEquals(1, countDownLatch.getCount()); // Fetch not yet completed // Complete the save - saveFuture.complete(null); - fetch.join(); - assertEquals(1, callCount.get()); // Callback should be called after save completes + mockHttpClient.flush(); + + assertTrue(countDownLatch.await(1, TimeUnit.SECONDS)); + assertEquals(0, countDownLatch.getCount()); // Callback should be called after save completes } } diff --git a/src/test/java/cloud/eppo/helpers/TestUtils.java b/src/test/java/cloud/eppo/helpers/TestUtils.java index 4b9a5822..699053a3 100644 --- a/src/test/java/cloud/eppo/helpers/TestUtils.java +++ b/src/test/java/cloud/eppo/helpers/TestUtils.java @@ -1,49 +1,24 @@ package cloud.eppo.helpers; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.Mockito.*; - import cloud.eppo.BaseEppoClient; -import cloud.eppo.EppoHttpClient; +import cloud.eppo.IEppoHttpClient; import java.lang.reflect.Field; -import java.util.concurrent.CompletableFuture; -import okhttp3.*; public class TestUtils { @SuppressWarnings("SameParameterValue") - public static EppoHttpClient mockHttpResponse(String responseBody) { - // Create a mock instance of EppoHttpClient - EppoHttpClient mockHttpClient = mock(EppoHttpClient.class); - - // Mock sync get - when(mockHttpClient.get(anyString())).thenReturn(responseBody.getBytes()); - - // Mock async get - CompletableFuture mockAsyncResponse = new CompletableFuture<>(); - when(mockHttpClient.getAsync(anyString())).thenReturn(mockAsyncResponse); - mockAsyncResponse.complete(responseBody.getBytes()); + public static MockHttpClient mockHttpResponse(String responseBody) { + MockHttpClient mockHttpClient = new MockHttpClient(responseBody.getBytes()); setBaseClientHttpClientOverrideField(mockHttpClient); return mockHttpClient; } public static void mockHttpError() { - // Create a mock instance of EppoHttpClient - EppoHttpClient mockHttpClient = mock(EppoHttpClient.class); - - // Mock sync get - when(mockHttpClient.get(anyString())).thenThrow(new RuntimeException("Intentional Error")); - - // Mock async get - CompletableFuture mockAsyncResponse = new CompletableFuture<>(); - when(mockHttpClient.getAsync(anyString())).thenReturn(mockAsyncResponse); - mockAsyncResponse.completeExceptionally(new RuntimeException("Intentional Error")); - - setBaseClientHttpClientOverrideField(mockHttpClient); + setBaseClientHttpClientOverrideField(new ThrowingHttpClient()); } - public static void setBaseClientHttpClientOverrideField(EppoHttpClient httpClient) { + public static void setBaseClientHttpClientOverrideField(IEppoHttpClient httpClient) { setBaseClientOverrideField("httpClientOverride", httpClient); } @@ -59,4 +34,74 @@ public static void setBaseClientOverrideField(String fieldName, T override) throw new RuntimeException(e); } } + + public static class MockHttpClient extends DelayedHttpClient { + public MockHttpClient(byte[] responseBody) { + super(responseBody); + flush(); + } + + public void changeResponse(byte[] responseBody) { + this.responseBody = responseBody; + } + } + + public static class ThrowingHttpClient implements IEppoHttpClient { + + @Override + public byte[] get(String path) { + throw new RuntimeException("Intentional Error"); + } + + @Override + public void getAsync(String path, EppoHttpCallback callback) { + callback.onFailure(new RuntimeException("Intentional Error")); + } + } + + public static class DelayedHttpClient implements IEppoHttpClient { + protected byte[] responseBody; + private EppoHttpCallback callback; + private boolean flushed = false; + private Throwable error = null; + + public int getCalls = 0; + public int getAsyncCalls = 0; + + public DelayedHttpClient(byte[] responseBody) { + this.responseBody = responseBody; + } + + @Override + public byte[] get(String path) { + getCalls++; + return responseBody; + } + + @Override + public void getAsync(String path, EppoHttpCallback callback) { + getAsyncCalls++; + if (flushed) { + callback.onSuccess(responseBody); + } else if (error != null) { + callback.onFailure(error); + } else { + this.callback = callback; + } + } + + public void fail(Throwable error) { + this.error = error; + if (this.callback != null) { + this.callback.onFailure(error); + } + } + + public void flush() { + flushed = true; + if (callback != null) { + callback.onSuccess(responseBody); + } + } + } } From ac01489a8b77b269ebd6a008a0541e6f3e14497d Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Thu, 24 Apr 2025 14:24:50 -0600 Subject: [PATCH 03/14] test in java6 --- .github/workflows/lint-test-sdk.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint-test-sdk.yml b/.github/workflows/lint-test-sdk.yml index a3596e9c..0c5383d8 100644 --- a/.github/workflows/lint-test-sdk.yml +++ b/.github/workflows/lint-test-sdk.yml @@ -33,7 +33,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - java-version: ['8', '11', '17', '21'] # Define the Java versions to test against + java-version: ['6', '8', '11', '17', '21'] # Define the Java versions to test against steps: - uses: actions/checkout@v4 with: From 2e55986645f9f2f3c96d56b604184c7c3f6521fd Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Thu, 24 Apr 2025 14:34:46 -0600 Subject: [PATCH 04/14] activateConfiguration method --- src/main/java/cloud/eppo/BaseEppoClient.java | 6 +++++- src/main/java/cloud/eppo/ConfigurationRequestor.java | 12 +++--------- .../java/cloud/eppo/ConfigurationRequestorTest.java | 4 ++-- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/main/java/cloud/eppo/BaseEppoClient.java b/src/main/java/cloud/eppo/BaseEppoClient.java index d61c3617..f2944e8e 100644 --- a/src/main/java/cloud/eppo/BaseEppoClient.java +++ b/src/main/java/cloud/eppo/BaseEppoClient.java @@ -84,7 +84,7 @@ protected BaseEppoClient( this.configurationStore, httpClient, expectObfuscatedConfig, supportBandits); if (initialConfiguration != null) { - requestor.setInitialConfiguration(initialConfiguration); + requestor.activateConfiguration(initialConfiguration); } this.assignmentLogger = assignmentLogger; @@ -104,6 +104,10 @@ private IEppoHttpClient buildHttpClient( : new EppoHttpClient(endpointHelper.getBaseUrl(), sdkKey.getToken(), sdkName, sdkVersion); } + public void activateConfiguration(Configuration configuration) { + requestor.activateConfiguration(configuration); + } + protected void loadConfiguration() { try { requestor.fetchAndSaveFromRemote(); diff --git a/src/main/java/cloud/eppo/ConfigurationRequestor.java b/src/main/java/cloud/eppo/ConfigurationRequestor.java index 34876fe1..bf5ad600 100644 --- a/src/main/java/cloud/eppo/ConfigurationRequestor.java +++ b/src/main/java/cloud/eppo/ConfigurationRequestor.java @@ -41,12 +41,8 @@ public ConfigurationRequestor( } // Synchronously set the initial configuration. - public void setInitialConfiguration(@NotNull Configuration configuration) { - if (initialConfigSet) { - throw new IllegalStateException("Initial configuration has already been set"); - } - - initialConfigSet = saveConfigurationAndNotify(configuration); + public void activateConfiguration(@NotNull Configuration configuration) { + saveConfigurationAndNotify(configuration); } /** Loads configuration synchronously from the API server. */ @@ -108,13 +104,11 @@ public void onFailure(Throwable error) { }); } - private boolean saveConfigurationAndNotify(Configuration configuration) { + private void saveConfigurationAndNotify(Configuration configuration) { configurationStore.saveConfiguration(configuration); synchronized (configChangeManager) { configChangeManager.notifyCallbacks(configuration); } - - return true; } public Runnable onConfigurationChange(Configuration.ConfigurationCallback callback) { diff --git a/src/test/java/cloud/eppo/ConfigurationRequestorTest.java b/src/test/java/cloud/eppo/ConfigurationRequestorTest.java index c5e233ce..ac22f651 100644 --- a/src/test/java/cloud/eppo/ConfigurationRequestorTest.java +++ b/src/test/java/cloud/eppo/ConfigurationRequestorTest.java @@ -49,7 +49,7 @@ public void testBrokenFetchDoesntClobberCache() throws IOException, InterruptedE String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); // Set initial config and verify that it has been set. - requestor.setInitialConfiguration(Configuration.builder(flagConfig.getBytes()).build()); + requestor.activateConfiguration(Configuration.builder(flagConfig.getBytes()).build()); assertFalse(configStore.getConfiguration().isEmpty()); Mockito.verify(configStore, Mockito.times(1)).saveConfiguration(any()); @@ -114,7 +114,7 @@ public void onFailure(Throwable error) { mockHttpClient.fail(new Exception("Intentional exception")); // Set initial config and verify that it has been set. - requestor.setInitialConfiguration(Configuration.builder(flagConfig.getBytes()).build()); + requestor.activateConfiguration(Configuration.builder(flagConfig.getBytes()).build()); Mockito.verify(configStore, Mockito.times(1)).saveConfiguration(any()); assertFalse(configStore.getConfiguration().isEmpty()); From d3a792e8e0eb86fed405a129591e6b2532a061c5 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Thu, 24 Apr 2025 20:06:50 -0600 Subject: [PATCH 05/14] not java 6 --- .github/workflows/lint-test-sdk.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint-test-sdk.yml b/.github/workflows/lint-test-sdk.yml index 0c5383d8..a3596e9c 100644 --- a/.github/workflows/lint-test-sdk.yml +++ b/.github/workflows/lint-test-sdk.yml @@ -33,7 +33,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - java-version: ['6', '8', '11', '17', '21'] # Define the Java versions to test against + java-version: ['8', '11', '17', '21'] # Define the Java versions to test against steps: - uses: actions/checkout@v4 with: From 49b03ce847a60a12d300bf660f38309c27f4b30c Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Thu, 24 Apr 2025 20:30:24 -0600 Subject: [PATCH 06/14] rename apiKey --- src/main/java/cloud/eppo/BaseEppoClient.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/cloud/eppo/BaseEppoClient.java b/src/main/java/cloud/eppo/BaseEppoClient.java index f2944e8e..e904092d 100644 --- a/src/main/java/cloud/eppo/BaseEppoClient.java +++ b/src/main/java/cloud/eppo/BaseEppoClient.java @@ -51,7 +51,7 @@ public class BaseEppoClient { // The recommended is 10 minutes (per @Sven) /** @param host To be removed in v4. use `apiBaseUrl` instead. */ protected BaseEppoClient( - @NotNull String apiKey, + @NotNull String sdkKey, @NotNull String sdkName, @NotNull String sdkVersion, @Deprecated @Nullable String host, @@ -74,7 +74,7 @@ protected BaseEppoClient( this.banditAssignmentCache = banditAssignmentCache; IEppoHttpClient httpClient = - buildHttpClient(apiBaseUrl, new SDKKey(apiKey), sdkName, sdkVersion); + buildHttpClient(apiBaseUrl, new SDKKey(sdkKey), sdkName, sdkVersion); this.configurationStore = configurationStore != null ? configurationStore : new ConfigurationStore(); From 119bcb144333c5492e7270864850725c1e417c94 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Thu, 24 Apr 2025 20:38:46 -0600 Subject: [PATCH 07/14] remove deprecated host param --- src/main/java/cloud/eppo/BaseEppoClient.java | 7 +-- .../cloud/eppo/BaseEppoClientBanditTest.java | 2 - .../java/cloud/eppo/BaseEppoClientTest.java | 53 ------------------- .../cloud/eppo/ProfileBaseEppoClientTest.java | 1 - 4 files changed, 1 insertion(+), 62 deletions(-) diff --git a/src/main/java/cloud/eppo/BaseEppoClient.java b/src/main/java/cloud/eppo/BaseEppoClient.java index e904092d..c9b5d023 100644 --- a/src/main/java/cloud/eppo/BaseEppoClient.java +++ b/src/main/java/cloud/eppo/BaseEppoClient.java @@ -49,12 +49,11 @@ public class BaseEppoClient { // It is important that the bandit assignment cache expire with a short-enough TTL to last about // one user session. // The recommended is 10 minutes (per @Sven) - /** @param host To be removed in v4. use `apiBaseUrl` instead. */ + /** */ protected BaseEppoClient( @NotNull String sdkKey, @NotNull String sdkName, @NotNull String sdkVersion, - @Deprecated @Nullable String host, @Nullable String apiBaseUrl, @Nullable AssignmentLogger assignmentLogger, @Nullable BanditLogger banditLogger, @@ -66,10 +65,6 @@ protected BaseEppoClient( @Nullable IAssignmentCache assignmentCache, @Nullable IAssignmentCache banditAssignmentCache) { - if (apiBaseUrl == null) { - apiBaseUrl = host != null ? Constants.appendApiPathToHost(host) : Constants.DEFAULT_BASE_URL; - } - this.assignmentCache = assignmentCache; this.banditAssignmentCache = banditAssignmentCache; diff --git a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java index e8ed253f..f0229b7c 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java @@ -67,7 +67,6 @@ public static void initClient() { DUMMY_BANDIT_API_KEY, "java", "3.0.0", - null, TEST_HOST, mockAssignmentLogger, mockBanditLogger, @@ -97,7 +96,6 @@ private BaseEppoClient initClientWithData( DUMMY_BANDIT_API_KEY, "java", "3.0.0", - null, TEST_HOST, mockAssignmentLogger, mockBanditLogger, diff --git a/src/test/java/cloud/eppo/BaseEppoClientTest.java b/src/test/java/cloud/eppo/BaseEppoClientTest.java index 4cee0196..ee1d6c75 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientTest.java @@ -20,7 +20,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import java.io.File; import java.io.IOException; -import java.net.URL; import java.util.Date; import java.util.HashMap; import java.util.Map; @@ -30,9 +29,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Stream; -import okhttp3.mockwebserver.MockResponse; -import okhttp3.mockwebserver.MockWebServer; -import okhttp3.mockwebserver.RecordedRequest; import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -78,7 +74,6 @@ private void initClientWithData( DUMMY_FLAG_API_KEY, isConfigObfuscated ? "android" : "java", "100.1.0", - null, TEST_BASE_URL, mockAssignmentLogger, null, @@ -99,7 +94,6 @@ private void initClient(boolean isGracefulMode, boolean isConfigObfuscated) { DUMMY_FLAG_API_KEY, isConfigObfuscated ? "android" : "java", "100.1.0", - null, TEST_BASE_URL, mockAssignmentLogger, null, @@ -126,7 +120,6 @@ private void initClientAsync( DUMMY_FLAG_API_KEY, isConfigObfuscated ? "android" : "java", "100.1.0", - null, TEST_BASE_URL, mockAssignmentLogger, null, @@ -149,7 +142,6 @@ private void initClientWithAssignmentCache(IAssignmentCache cache) { DUMMY_FLAG_API_KEY, "java", "100.1.0", - null, TEST_BASE_URL, mockAssignmentLogger, null, @@ -191,49 +183,6 @@ private static Stream getAssignmentTestData() { return AssignmentTestCase.getAssignmentTestData(); } - @Test - public void testBaseUrlBackwardsCompatibility() throws IOException, InterruptedException { - // Base client must be buildable with a HOST (i.e. no `/api` postfix) - mockAssignmentLogger = mock(AssignmentLogger.class); - - MockWebServer mockWebServer = new MockWebServer(); - URL mockServerBaseUrl = mockWebServer.url("").url(); // get base url of mockwebserver - - // Remove trailing slash to mimic typical "host" parameter of "https://fscdn.eppo.cloud" - String testHost = mockServerBaseUrl.toString().replaceAll("/$", ""); - - mockWebServer.enqueue(new MockResponse().setResponseCode(200).setBody("{}")); - - eppoClient = - new BaseEppoClient( - DUMMY_FLAG_API_KEY, - "java", - "100.1.0", - testHost, - null, - mockAssignmentLogger, - null, - null, - false, - false, - true, - null, - null, - null); - - eppoClient.loadConfiguration(); - - // Test what path the call was sent to - RecordedRequest request = mockWebServer.takeRequest(); - assertNotNull(request); - assertEquals("GET", request.getMethod()); - - // The "/api" part comes from appending it on to a "host" parameter but not a base URL param. - assertEquals( - "/api/flag-config/v1/config?apiKey=dummy-flags-api-key&sdkName=java&sdkVersion=100.1.0", - request.getPath()); - } - @Test public void testErrorGracefulModeOn() throws JsonProcessingException { initClient(true, false); @@ -631,7 +580,6 @@ public void testPolling() { DUMMY_FLAG_API_KEY, "java", "100.1.0", - null, TEST_BASE_URL, mockAssignmentLogger, null, @@ -747,7 +695,6 @@ public void testGetConfigurationBeforeInitialization() { DUMMY_FLAG_API_KEY, "java", "100.1.0", - null, TEST_BASE_URL, mockAssignmentLogger, null, diff --git a/src/test/java/cloud/eppo/ProfileBaseEppoClientTest.java b/src/test/java/cloud/eppo/ProfileBaseEppoClientTest.java index 3dd24802..aabd3c49 100644 --- a/src/test/java/cloud/eppo/ProfileBaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/ProfileBaseEppoClientTest.java @@ -39,7 +39,6 @@ public static void initClient() { DUMMY_FLAG_API_KEY, "java", "3.0.0", - null, TEST_HOST, noOpAssignmentLogger, null, From cd65a31394086f3f1a3d52b4af5e4853eb5ea8f9 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Thu, 24 Apr 2025 21:00:36 -0600 Subject: [PATCH 08/14] remove deprecated isObfuscated params --- src/main/java/cloud/eppo/BaseEppoClient.java | 5 +- .../cloud/eppo/ConfigurationRequestor.java | 8 +--- .../java/cloud/eppo/api/Configuration.java | 14 ------ .../cloud/eppo/BaseEppoClientBanditTest.java | 4 +- .../java/cloud/eppo/BaseEppoClientTest.java | 47 +++++++------------ .../eppo/ConfigurationRequestorTest.java | 8 ++-- .../cloud/eppo/ProfileBaseEppoClientTest.java | 1 - .../eppo/api/ConfigurationBuilderTest.java | 15 +----- 8 files changed, 26 insertions(+), 76 deletions(-) diff --git a/src/main/java/cloud/eppo/BaseEppoClient.java b/src/main/java/cloud/eppo/BaseEppoClient.java index c9b5d023..6840b4d7 100644 --- a/src/main/java/cloud/eppo/BaseEppoClient.java +++ b/src/main/java/cloud/eppo/BaseEppoClient.java @@ -59,7 +59,6 @@ protected BaseEppoClient( @Nullable BanditLogger banditLogger, @Nullable IConfigurationStore configurationStore, boolean isGracefulMode, - boolean expectObfuscatedConfig, boolean supportBandits, @Nullable Configuration initialConfiguration, @Nullable IAssignmentCache assignmentCache, @@ -74,9 +73,7 @@ protected BaseEppoClient( configurationStore != null ? configurationStore : new ConfigurationStore(); // For now, the configuration is only obfuscated for Android clients - requestor = - new ConfigurationRequestor( - this.configurationStore, httpClient, expectObfuscatedConfig, supportBandits); + requestor = new ConfigurationRequestor(this.configurationStore, httpClient, supportBandits); if (initialConfiguration != null) { requestor.activateConfiguration(initialConfiguration); diff --git a/src/main/java/cloud/eppo/ConfigurationRequestor.java b/src/main/java/cloud/eppo/ConfigurationRequestor.java index bf5ad600..98ca532f 100644 --- a/src/main/java/cloud/eppo/ConfigurationRequestor.java +++ b/src/main/java/cloud/eppo/ConfigurationRequestor.java @@ -12,11 +12,8 @@ public class ConfigurationRequestor { private final IEppoHttpClient client; private final IConfigurationStore configurationStore; - private final boolean expectObfuscatedConfig; private final boolean supportBandits; - private boolean initialConfigSet = false; - private final CallbackManager configChangeManager = new CallbackManager<>( @@ -32,11 +29,9 @@ public void dispatch( public ConfigurationRequestor( @NotNull IConfigurationStore configurationStore, @NotNull IEppoHttpClient client, - boolean expectObfuscatedConfig, boolean supportBandits) { this.configurationStore = configurationStore; this.client = client; - this.expectObfuscatedConfig = expectObfuscatedConfig; this.supportBandits = supportBandits; } @@ -54,8 +49,7 @@ void fetchAndSaveFromRemote() { byte[] flagConfigurationJsonBytes = client.get(Constants.FLAG_CONFIG_ENDPOINT); Configuration.Builder configBuilder = - Configuration.builder(flagConfigurationJsonBytes, expectObfuscatedConfig) - .banditParametersFromConfig(lastConfig); + Configuration.builder(flagConfigurationJsonBytes).banditParametersFromConfig(lastConfig); if (supportBandits && configBuilder.requiresUpdatedBanditModels()) { byte[] banditParametersJsonBytes = client.get(Constants.BANDIT_ENDPOINT); diff --git a/src/main/java/cloud/eppo/api/Configuration.java b/src/main/java/cloud/eppo/api/Configuration.java index b20a570e..55d5785e 100644 --- a/src/main/java/cloud/eppo/api/Configuration.java +++ b/src/main/java/cloud/eppo/api/Configuration.java @@ -179,10 +179,6 @@ public static Builder builder(byte[] flagJson) { return new Builder(flagJson); } - @Deprecated // isConfigObfuscated is determined from the byte payload - public static Builder builder(byte[] flagJson, boolean isConfigObfuscated) { - return new Builder(flagJson, isConfigObfuscated); - } /** * Builder to create the immutable config object. * @@ -210,16 +206,6 @@ private static FlagConfigResponse parseFlagResponse(byte[] flagJson) { } } - @Deprecated // isConfigObfuscated is determined from the byte payload - public Builder(String flagJson, boolean isConfigObfuscated) { - this(flagJson.getBytes(), parseFlagResponse(flagJson.getBytes()), isConfigObfuscated); - } - - @Deprecated // isConfigObfuscated is determined from the byte payload - public Builder(byte[] flagJson, boolean isConfigObfuscated) { - this(flagJson, parseFlagResponse(flagJson), isConfigObfuscated); - } - public Builder(byte[] flagJson, FlagConfigResponse flagConfigResponse) { this( flagJson, diff --git a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java index f0229b7c..cc7358d4 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java @@ -72,7 +72,6 @@ public static void initClient() { mockBanditLogger, null, false, - false, true, null, new AbstractAssignmentCache(assignmentCache) {}, @@ -88,7 +87,7 @@ private BaseEppoClient initClientWithData( final String initialFlagConfiguration, final String initialBanditParameters) { Configuration initialConfig = - Configuration.builder(initialFlagConfiguration.getBytes(), false) + Configuration.builder(initialFlagConfiguration.getBytes()) .banditParameters(initialBanditParameters) .build(); @@ -101,7 +100,6 @@ private BaseEppoClient initClientWithData( mockBanditLogger, null, false, - false, true, initialConfig, null, diff --git a/src/test/java/cloud/eppo/BaseEppoClientTest.java b/src/test/java/cloud/eppo/BaseEppoClientTest.java index ee1d6c75..14db5507 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientTest.java @@ -60,46 +60,42 @@ public class BaseEppoClientTest { new File("src/test/resources/static/initial-flag-config.json"); private void initClient() { - initClient(false, false); + initClient(false, "java"); } private void initClientWithData( - final Configuration initialFlagConfiguration, - boolean isConfigObfuscated, - boolean isGracefulMode) { + final Configuration initialFlagConfiguration, boolean isGracefulMode) { mockAssignmentLogger = mock(AssignmentLogger.class); eppoClient = new BaseEppoClient( DUMMY_FLAG_API_KEY, - isConfigObfuscated ? "android" : "java", + "java", "100.1.0", TEST_BASE_URL, mockAssignmentLogger, null, null, isGracefulMode, - isConfigObfuscated, true, initialFlagConfiguration, null, null); } - private void initClient(boolean isGracefulMode, boolean isConfigObfuscated) { + private void initClient(boolean isGracefulMode, String sdkName) { mockAssignmentLogger = mock(AssignmentLogger.class); eppoClient = new BaseEppoClient( DUMMY_FLAG_API_KEY, - isConfigObfuscated ? "android" : "java", + sdkName, "100.1.0", TEST_BASE_URL, mockAssignmentLogger, null, null, isGracefulMode, - isConfigObfuscated, true, null, null, @@ -111,21 +107,19 @@ private void initClient(boolean isGracefulMode, boolean isConfigObfuscated) { interface InitCallback extends EppoActionCallback {} - private void initClientAsync( - boolean isGracefulMode, boolean isConfigObfuscated, InitCallback initCallback) { + private void initClientAsync(boolean isGracefulMode, InitCallback initCallback) { mockAssignmentLogger = mock(AssignmentLogger.class); eppoClient = new BaseEppoClient( DUMMY_FLAG_API_KEY, - isConfigObfuscated ? "android" : "java", + "java", "100.1.0", TEST_BASE_URL, mockAssignmentLogger, null, null, isGracefulMode, - isConfigObfuscated, true, null, null, @@ -147,7 +141,6 @@ private void initClientWithAssignmentCache(IAssignmentCache cache) { null, null, true, - false, true, null, cache, @@ -166,7 +159,7 @@ public void cleanUp() { @ParameterizedTest @MethodSource("getAssignmentTestData") public void testUnobfuscatedAssignments(File testFile) { - initClient(false, false); + initClient(false, "java"); AssignmentTestCase testCase = parseTestCaseFile(testFile); runTestCase(testCase, eppoClient); } @@ -174,7 +167,7 @@ public void testUnobfuscatedAssignments(File testFile) { @ParameterizedTest @MethodSource("getAssignmentTestData") public void testObfuscatedAssignments(File testFile) { - initClient(false, true); + initClient(false, "android"); AssignmentTestCase testCase = parseTestCaseFile(testFile); runTestCase(testCase, eppoClient); } @@ -185,7 +178,7 @@ private static Stream getAssignmentTestData() { @Test public void testErrorGracefulModeOn() throws JsonProcessingException { - initClient(true, false); + initClient(true, "java"); BaseEppoClient realClient = eppoClient; BaseEppoClient spyClient = spy(realClient); @@ -234,7 +227,7 @@ public void testErrorGracefulModeOn() throws JsonProcessingException { @Test public void testErrorGracefulModeOff() { - initClient(false, false); + initClient(false, "java"); BaseEppoClient realClient = eppoClient; BaseEppoClient spyClient = spy(realClient); @@ -292,7 +285,7 @@ public void testInvalidConfigJSON() { mockHttpResponse("{}"); - initClient(false, false); + initClient(false, "java"); String result = eppoClient.getStringAssignment("dummy flag", "dummy subject", "not-populated"); assertEquals("not-populated", result); @@ -304,7 +297,7 @@ public void testGracefulInitializationFailure() { mockHttpError(); // Initialize and no exception should be thrown. - assertDoesNotThrow(() -> initClient(true, false)); + assertDoesNotThrow(() -> initClient(true, "java")); } @Test @@ -313,7 +306,7 @@ public void testClientMakesDefaultAssignmentsAfterFailingToInitialize() { mockHttpError(); // Initialize and no exception should be thrown. - assertDoesNotThrow(() -> initClient(true, false)); + assertDoesNotThrow(() -> initClient(true, "java")); assertEquals("default", eppoClient.getStringAssignment("experiment1", "subject1", "default")); } @@ -325,7 +318,7 @@ public void testClientMakesDefaultAssignmentsAfterFailingToInitializeNonGraceful // Initialize and no exception should be thrown. try { - initClient(false, false); + initClient(false, "java"); } catch (RuntimeException e) { // Expected assertEquals("Intentional Error", e.getMessage()); @@ -340,7 +333,7 @@ public void testNonGracefulInitializationFailure() { mockHttpError(); // Initialize and assert exception thrown - assertThrows(Exception.class, () -> initClient(false, false)); + assertThrows(Exception.class, () -> initClient(false, "java")); } @Test @@ -354,7 +347,6 @@ public void testGracefulAsyncInitializationFailure() throws InterruptedException // Initialize initClientAsync( true, - false, new InitCallback() { @Override public void onSuccess(Configuration data) { @@ -385,7 +377,6 @@ public void testNonGracefulAsyncInitializationFailure() throws InterruptedExcept // Initialize initClientAsync( - false, false, new InitCallback() { @Override @@ -412,7 +403,7 @@ public void testWithInitialConfiguration() { try { String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, "UTF8"); - initClientWithData(Configuration.builder(flagConfig.getBytes()).build(), false, true); + initClientWithData(Configuration.builder(flagConfig.getBytes()).build(), true); double result = eppoClient.getDoubleAssignment("numeric_flag", "dummy subject", 0); assertEquals(5, result); @@ -585,7 +576,6 @@ public void testPolling() { null, null, false, - false, true, null, null, @@ -653,7 +643,7 @@ public void testGetConfigurationWithInitialConfig() { String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, "UTF8"); // Initialize client with initial configuration - initClientWithData(Configuration.builder(flagConfig.getBytes()).build(), false, true); + initClientWithData(Configuration.builder(flagConfig.getBytes()).build(), true); // Get configuration Configuration config = eppoClient.getConfiguration(); @@ -700,7 +690,6 @@ public void testGetConfigurationBeforeInitialization() { null, null, false, - false, true, null, null, diff --git a/src/test/java/cloud/eppo/ConfigurationRequestorTest.java b/src/test/java/cloud/eppo/ConfigurationRequestorTest.java index ac22f651..5c6012db 100644 --- a/src/test/java/cloud/eppo/ConfigurationRequestorTest.java +++ b/src/test/java/cloud/eppo/ConfigurationRequestorTest.java @@ -35,7 +35,7 @@ public class ConfigurationRequestorTest { public void setup() { mockConfigStore = mock(ConfigurationStore.class); mockHttpClient = mock(EppoHttpClient.class); - requestor = new ConfigurationRequestor(mockConfigStore, mockHttpClient, false, true); + requestor = new ConfigurationRequestor(mockConfigStore, mockHttpClient, true); } @Test @@ -44,7 +44,7 @@ public void testBrokenFetchDoesntClobberCache() throws IOException, InterruptedE TestUtils.DelayedHttpClient mockHttpClient = new TestUtils.DelayedHttpClient("".getBytes()); ConfigurationRequestor requestor = - new ConfigurationRequestor(configStore, mockHttpClient, false, true); + new ConfigurationRequestor(configStore, mockHttpClient, true); String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); @@ -89,7 +89,7 @@ public void testCacheWritesAfterBrokenFetch() throws IOException, InterruptedExc TestUtils.DelayedHttpClient mockHttpClient = new TestUtils.DelayedHttpClient("".getBytes()); ConfigurationRequestor requestor = - new ConfigurationRequestor(configStore, mockHttpClient, false, true); + new ConfigurationRequestor(configStore, mockHttpClient, true); String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); Mockito.verify(configStore, Mockito.times(0)).saveConfiguration(any()); @@ -248,7 +248,7 @@ public void testConfigurationChangeListenerAsyncSave() throws InterruptedExcepti // Setup mock responses TestUtils.DelayedHttpClient mockHttpClient = new TestUtils.DelayedHttpClient("{\"flags\":{}}".getBytes()); - requestor = new ConfigurationRequestor(mockConfigStore, mockHttpClient, false, true); + requestor = new ConfigurationRequestor(mockConfigStore, mockHttpClient, true); CountDownLatch countDownLatch = new CountDownLatch(1); diff --git a/src/test/java/cloud/eppo/ProfileBaseEppoClientTest.java b/src/test/java/cloud/eppo/ProfileBaseEppoClientTest.java index aabd3c49..a5945ccc 100644 --- a/src/test/java/cloud/eppo/ProfileBaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/ProfileBaseEppoClientTest.java @@ -44,7 +44,6 @@ public static void initClient() { null, null, false, - false, true, null, null, diff --git a/src/test/java/cloud/eppo/api/ConfigurationBuilderTest.java b/src/test/java/cloud/eppo/api/ConfigurationBuilderTest.java index 54360033..34d1d222 100644 --- a/src/test/java/cloud/eppo/api/ConfigurationBuilderTest.java +++ b/src/test/java/cloud/eppo/api/ConfigurationBuilderTest.java @@ -35,7 +35,7 @@ public void testHydrateConfigFromBytesForServer_false() { @Test public void testBuildConfigAddsForServer_true() throws IOException { byte[] jsonBytes = "{ \"flags\":{} }".getBytes(); - Configuration config = Configuration.builder(jsonBytes, false).build(); + Configuration config = Configuration.builder(jsonBytes).build(); assertFalse(config.isConfigObfuscated()); byte[] serializedFlags = config.serializeFlagConfigToBytes(); @@ -45,19 +45,6 @@ public void testBuildConfigAddsForServer_true() throws IOException { assertEquals(rehydratedConfig.getFormat(), FlagConfigResponse.Format.SERVER); } - @Test - public void testBuildConfigAddsForServer_false() throws IOException { - byte[] jsonBytes = "{ \"flags\":{} }".getBytes(); - Configuration config = Configuration.builder(jsonBytes, true).build(); - assertTrue(config.isConfigObfuscated()); - - byte[] serializedFlags = config.serializeFlagConfigToBytes(); - FlagConfigResponse rehydratedConfig = - mapper.readValue(serializedFlags, FlagConfigResponse.class); - - assertEquals(rehydratedConfig.getFormat(), FlagConfigResponse.Format.CLIENT); - } - @Test public void getFlagType_shouldReturnCorrectType() { // Create a flag config with a STRING variation type From 75df168d949aa68b43243ec92beedf78f7857a73 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Thu, 24 Apr 2025 21:08:34 -0600 Subject: [PATCH 09/14] tests --- .../cloud/eppo/ConfigurationRequestor.java | 2 +- .../java/cloud/eppo/BaseEppoClientTest.java | 52 ++++++++++++++----- 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/src/main/java/cloud/eppo/ConfigurationRequestor.java b/src/main/java/cloud/eppo/ConfigurationRequestor.java index 98ca532f..95bfac6e 100644 --- a/src/main/java/cloud/eppo/ConfigurationRequestor.java +++ b/src/main/java/cloud/eppo/ConfigurationRequestor.java @@ -60,7 +60,7 @@ void fetchAndSaveFromRemote() { } /** Loads configuration asynchronously from the API server, off-thread. */ - public void fetchAndSaveFromRemoteAsync(EppoActionCallback callback) { + void fetchAndSaveFromRemoteAsync(EppoActionCallback callback) { log.debug("Fetching configuration from API server"); final Configuration lastConfig = configurationStore.getConfiguration(); diff --git a/src/test/java/cloud/eppo/BaseEppoClientTest.java b/src/test/java/cloud/eppo/BaseEppoClientTest.java index 14db5507..42915a8e 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientTest.java @@ -60,7 +60,7 @@ public class BaseEppoClientTest { new File("src/test/resources/static/initial-flag-config.json"); private void initClient() { - initClient(false, "java"); + initClient(false, "java", true); } private void initClientWithData( @@ -83,7 +83,7 @@ private void initClientWithData( null); } - private void initClient(boolean isGracefulMode, String sdkName) { + private void initClient(boolean isGracefulMode, String sdkName, boolean loadConfig) { mockAssignmentLogger = mock(AssignmentLogger.class); eppoClient = @@ -101,7 +101,9 @@ private void initClient(boolean isGracefulMode, String sdkName) { null, null); - eppoClient.loadConfiguration(); + if (loadConfig) { + eppoClient.loadConfiguration(); + } log.info("Test client initialized"); } @@ -159,7 +161,7 @@ public void cleanUp() { @ParameterizedTest @MethodSource("getAssignmentTestData") public void testUnobfuscatedAssignments(File testFile) { - initClient(false, "java"); + initClient(false, "java", true); AssignmentTestCase testCase = parseTestCaseFile(testFile); runTestCase(testCase, eppoClient); } @@ -167,7 +169,7 @@ public void testUnobfuscatedAssignments(File testFile) { @ParameterizedTest @MethodSource("getAssignmentTestData") public void testObfuscatedAssignments(File testFile) { - initClient(false, "android"); + initClient(false, "android", true); AssignmentTestCase testCase = parseTestCaseFile(testFile); runTestCase(testCase, eppoClient); } @@ -178,7 +180,7 @@ private static Stream getAssignmentTestData() { @Test public void testErrorGracefulModeOn() throws JsonProcessingException { - initClient(true, "java"); + initClient(true, "java", true); BaseEppoClient realClient = eppoClient; BaseEppoClient spyClient = spy(realClient); @@ -227,7 +229,7 @@ public void testErrorGracefulModeOn() throws JsonProcessingException { @Test public void testErrorGracefulModeOff() { - initClient(false, "java"); + initClient(false, "java", true); BaseEppoClient realClient = eppoClient; BaseEppoClient spyClient = spy(realClient); @@ -285,7 +287,7 @@ public void testInvalidConfigJSON() { mockHttpResponse("{}"); - initClient(false, "java"); + initClient(false, "java", true); String result = eppoClient.getStringAssignment("dummy flag", "dummy subject", "not-populated"); assertEquals("not-populated", result); @@ -297,7 +299,7 @@ public void testGracefulInitializationFailure() { mockHttpError(); // Initialize and no exception should be thrown. - assertDoesNotThrow(() -> initClient(true, "java")); + assertDoesNotThrow(() -> initClient(true, "java", true)); } @Test @@ -306,7 +308,7 @@ public void testClientMakesDefaultAssignmentsAfterFailingToInitialize() { mockHttpError(); // Initialize and no exception should be thrown. - assertDoesNotThrow(() -> initClient(true, "java")); + assertDoesNotThrow(() -> initClient(true, "java", true)); assertEquals("default", eppoClient.getStringAssignment("experiment1", "subject1", "default")); } @@ -318,7 +320,7 @@ public void testClientMakesDefaultAssignmentsAfterFailingToInitializeNonGraceful // Initialize and no exception should be thrown. try { - initClient(false, "java"); + initClient(false, "java", true); } catch (RuntimeException e) { // Expected assertEquals("Intentional Error", e.getMessage()); @@ -333,7 +335,7 @@ public void testNonGracefulInitializationFailure() { mockHttpError(); // Initialize and assert exception thrown - assertThrows(Exception.class, () -> initClient(false, "java")); + assertThrows(Exception.class, () -> initClient(false, "java", true)); } @Test @@ -417,6 +419,32 @@ public void testWithInitialConfiguration() { } } + + @Test + public void testWithActivatedConfiguration() { + try { + String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, "UTF8"); + Configuration configToActivate = Configuration.builder(flagConfig.getBytes()).build(); + + initClient(false, "java", false); + + // Result is the default until we activate the config. + double firstResult = eppoClient.getDoubleAssignment("numeric_flag", "dummy subject", 0); + assertEquals(0, firstResult); + + eppoClient.activateConfiguration(configToActivate); + double result = eppoClient.getDoubleAssignment("numeric_flag", "dummy subject", 0); + assertEquals(5, result); + + // Demonstrate that loaded configuration is different from the initial string passed above. + eppoClient.loadConfiguration(); + double updatedResult = eppoClient.getDoubleAssignment("numeric_flag", "dummy subject", 0); + assertEquals(3.1415926, updatedResult); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + @Test public void testAssignmentEventCorrectlyCreated() { Date testStart = new Date(); From 885219842a0863848459b83a93d6920d304e2ec8 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Thu, 24 Apr 2025 21:16:43 -0600 Subject: [PATCH 10/14] relies on spies --- src/test/java/cloud/eppo/BaseEppoClientTest.java | 1 - src/test/java/cloud/eppo/helpers/TestUtils.java | 5 ----- 2 files changed, 6 deletions(-) diff --git a/src/test/java/cloud/eppo/BaseEppoClientTest.java b/src/test/java/cloud/eppo/BaseEppoClientTest.java index 42915a8e..84aeab04 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientTest.java @@ -419,7 +419,6 @@ public void testWithInitialConfiguration() { } } - @Test public void testWithActivatedConfiguration() { try { diff --git a/src/test/java/cloud/eppo/helpers/TestUtils.java b/src/test/java/cloud/eppo/helpers/TestUtils.java index 699053a3..057583d9 100644 --- a/src/test/java/cloud/eppo/helpers/TestUtils.java +++ b/src/test/java/cloud/eppo/helpers/TestUtils.java @@ -65,22 +65,17 @@ public static class DelayedHttpClient implements IEppoHttpClient { private boolean flushed = false; private Throwable error = null; - public int getCalls = 0; - public int getAsyncCalls = 0; - public DelayedHttpClient(byte[] responseBody) { this.responseBody = responseBody; } @Override public byte[] get(String path) { - getCalls++; return responseBody; } @Override public void getAsync(String path, EppoHttpCallback callback) { - getAsyncCalls++; if (flushed) { callback.onSuccess(responseBody); } else if (error != null) { From 6fcfffbcede295c62bdce5fb37f5d410d16bc3d9 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Thu, 24 Apr 2025 22:02:10 -0600 Subject: [PATCH 11/14] style fixes --- .../cloud/eppo/ConfigurationRequestor.java | 8 +- .../java/cloud/eppo/api/Configuration.java | 16 +++- .../cloud/eppo/api/EppoActionCallback.java | 22 +++++ .../cloud/eppo/callback/CallbackManager.java | 8 +- .../eppo/ConfigurationRequestorTest.java | 96 +++++++++++-------- 5 files changed, 108 insertions(+), 42 deletions(-) diff --git a/src/main/java/cloud/eppo/ConfigurationRequestor.java b/src/main/java/cloud/eppo/ConfigurationRequestor.java index 95bfac6e..63e7dc34 100644 --- a/src/main/java/cloud/eppo/ConfigurationRequestor.java +++ b/src/main/java/cloud/eppo/ConfigurationRequestor.java @@ -35,7 +35,11 @@ public ConfigurationRequestor( this.supportBandits = supportBandits; } - // Synchronously set the initial configuration. + /** + * Synchronously sets and activates the initial configuration. + * + * @param configuration The configuration to activate + */ public void activateConfiguration(@NotNull Configuration configuration) { saveConfigurationAndNotify(configuration); } @@ -93,6 +97,8 @@ public void onSuccess(byte[] flagConfigJsonBytes) { @Override public void onFailure(Throwable error) { + log.error( + "Failed to fetch configuration from API server: {}", error.getMessage(), error); callback.onFailure(error); } }); diff --git a/src/main/java/cloud/eppo/api/Configuration.java b/src/main/java/cloud/eppo/api/Configuration.java index 55d5785e..4316a3b4 100644 --- a/src/main/java/cloud/eppo/api/Configuration.java +++ b/src/main/java/cloud/eppo/api/Configuration.java @@ -49,8 +49,22 @@ * then check `requiresBanditModels()`. */ public class Configuration { - /** Callback for common operations involving the Configuration. */ + /** + * Callback for operations involving Configuration changes. + * + *

This interface is used to notify listeners when a configuration has been updated or changed. + * Implementations should handle the new configuration appropriately, such as updating cached + * values or triggering dependent operations. + * + *

Thread safety: Callbacks may be invoked on any thread, including the calling thread or a + * background thread. Implementations should be thread-safe and avoid blocking operations. + */ public interface ConfigurationCallback { + /** + * Called when a new configuration is available. + * + * @param configuration The updated configuration, may be null in some error scenarios + */ void accept(Configuration configuration); } diff --git a/src/main/java/cloud/eppo/api/EppoActionCallback.java b/src/main/java/cloud/eppo/api/EppoActionCallback.java index b5d10ecd..deead8fc 100644 --- a/src/main/java/cloud/eppo/api/EppoActionCallback.java +++ b/src/main/java/cloud/eppo/api/EppoActionCallback.java @@ -1,7 +1,29 @@ package cloud.eppo.api; +/** + * Interface for handling asynchronous operation results. + * + *

This callback interface provides methods to handle both successful completion and failure + * scenarios for asynchronous operations. Implementations should expect that exactly one of the + * callback methods (either onSuccess or onFailure) will be invoked for each operation. + * + *

Thread safety: Callbacks may be invoked on any thread, including the calling thread or a + * background thread. Implementations should be thread-safe and avoid blocking operations. + * + * @param The type of data returned on successful completion + */ public interface EppoActionCallback { + /** + * Called when an operation completes successfully. + * + * @param data The result data from the operation, may be null in some cases + */ void onSuccess(T data); + /** + * Called when an operation fails. + * + * @param error The error that caused the operation to fail + */ void onFailure(Throwable error); } diff --git a/src/main/java/cloud/eppo/callback/CallbackManager.java b/src/main/java/cloud/eppo/callback/CallbackManager.java index cbe65919..ac0f7550 100644 --- a/src/main/java/cloud/eppo/callback/CallbackManager.java +++ b/src/main/java/cloud/eppo/callback/CallbackManager.java @@ -13,8 +13,14 @@ * @param The type of data that will be passed to the callbacks */ public class CallbackManager { + /** + * Interface for dispatching data to callbacks. + * + * @param The type of data to dispatch + * @param The type of callback to dispatch to + */ public interface Dispatcher { - public void dispatch(C callback, T data); + void dispatch(C callback, T data); } private final Dispatcher dispatcher; diff --git a/src/test/java/cloud/eppo/ConfigurationRequestorTest.java b/src/test/java/cloud/eppo/ConfigurationRequestorTest.java index 5c6012db..bb63567c 100644 --- a/src/test/java/cloud/eppo/ConfigurationRequestorTest.java +++ b/src/test/java/cloud/eppo/ConfigurationRequestorTest.java @@ -21,6 +21,7 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; +/** Tests for the ConfigurationRequestor class. */ public class ConfigurationRequestorTest { private final File initialFlagConfigFile = new File("src/test/resources/static/initial-flag-config.json"); @@ -55,18 +56,7 @@ public void testBrokenFetchDoesntClobberCache() throws IOException, InterruptedE CountDownLatch latch = new CountDownLatch(1); requestor.fetchAndSaveFromRemoteAsync( - new EppoActionCallback() { - - @Override - public void onSuccess(Configuration data) { - fail("Unexpected success"); - } - - @Override - public void onFailure(Throwable error) { - latch.countDown(); - } - }); + createCallback(() -> fail("Unexpected success"), () -> latch.countDown())); // Error out the fetch mockHttpClient.fail(new Exception("Intentional exception")); @@ -97,18 +87,7 @@ public void testCacheWritesAfterBrokenFetch() throws IOException, InterruptedExc CountDownLatch latch = new CountDownLatch(1); requestor.fetchAndSaveFromRemoteAsync( - new EppoActionCallback() { - - @Override - public void onSuccess(Configuration data) { - fail("Unexpected success"); - } - - @Override - public void onFailure(Throwable error) { - latch.countDown(); - } - }); + createCallback(() -> fail("Unexpected success"), () -> latch.countDown())); // Error out the fetch mockHttpClient.fail(new Exception("Intentional exception")); @@ -128,6 +107,7 @@ public void onFailure(Throwable error) { assertTrue(latch.await(1, TimeUnit.SECONDS)); } + /** Helper class for tracking configurations received via callbacks. */ static class ListAddingConfigCallback implements Configuration.ConfigurationCallback { public final List results = new ArrayList<>(); @@ -137,6 +117,32 @@ public void accept(Configuration configuration) { } } + /** + * Creates a simple success/failure callback with the provided actions. + * + * @param onSuccessAction Action to perform on success + * @param onFailureAction Action to perform on failure + * @return A configured callback + */ + private EppoActionCallback createCallback( + Runnable onSuccessAction, Runnable onFailureAction) { + return new EppoActionCallback() { + @Override + public void onSuccess(Configuration data) { + if (onSuccessAction != null) { + onSuccessAction.run(); + } + } + + @Override + public void onFailure(Throwable error) { + if (onFailureAction != null) { + onFailureAction.run(); + } + } + }; + } + @Test public void testConfigurationChangeListener() throws IOException { // Setup mock response @@ -212,7 +218,13 @@ public void testConfigurationChangeListenerIgnoresFailedFetch() { when(mockHttpClient.get(anyString())).thenThrow(new RuntimeException("fetch failed")); AtomicInteger callCount = new AtomicInteger(0); - requestor.onConfigurationChange(v -> callCount.incrementAndGet()); + requestor.onConfigurationChange( + new Configuration.ConfigurationCallback() { + @Override + public void accept(Configuration configuration) { + callCount.incrementAndGet(); + } + }); // Failed save should not trigger the callback try { @@ -232,7 +244,13 @@ public void testConfigurationChangeListenerIgnoresFailedSave() { .saveConfiguration(any(Configuration.class)); AtomicInteger callCount = new AtomicInteger(0); - requestor.onConfigurationChange(v -> callCount.incrementAndGet()); + requestor.onConfigurationChange( + new Configuration.ConfigurationCallback() { + @Override + public void accept(Configuration configuration) { + callCount.incrementAndGet(); + } + }); // Failed save should not trigger the callback try { @@ -252,23 +270,23 @@ public void testConfigurationChangeListenerAsyncSave() throws InterruptedExcepti CountDownLatch countDownLatch = new CountDownLatch(1); - requestor.onConfigurationChange(v -> countDownLatch.countDown()); - - // Start fetch - requestor.fetchAndSaveFromRemoteAsync( - new EppoActionCallback() { - @Override - public void onSuccess(Configuration data) { - // This callback should be notified after the registered listeners have been notified. - assertEquals(0, countDownLatch.getCount()); - } - + requestor.onConfigurationChange( + new Configuration.ConfigurationCallback() { @Override - public void onFailure(Throwable error) { - fail("unexpected failure"); + public void accept(Configuration configuration) { + countDownLatch.countDown(); } }); + // Start fetch + requestor.fetchAndSaveFromRemoteAsync( + createCallback( + () -> { + // This callback should be notified after the registered listeners have been notified. + assertEquals(0, countDownLatch.getCount()); + }, + () -> fail("unexpected failure"))); + assertEquals(1, countDownLatch.getCount()); // Fetch not yet completed // Complete the save From 6a4ca59fae1aeb79990c697cfaa7145bafe5ad16 Mon Sep 17 00:00:00 2001 From: Tyler Potter Date: Wed, 21 May 2025 14:40:17 -0600 Subject: [PATCH 12/14] feat: Changes to the v4 API for integration in downstream SDKs (#115) Simplified callback interface Renamed methods to match "gen2" sdk naming Added activateConfiguration method expose isGracefulMode to subclasses moved graceful mode check top-level callsites (in poller and in assignment calls) renamed callbacks clarified init in tests --- build.gradle | 2 +- src/main/java/cloud/eppo/BaseEppoClient.java | 43 ++++-------- .../cloud/eppo/ConfigurationRequestor.java | 24 +++---- src/main/java/cloud/eppo/EppoHttpClient.java | 11 +-- .../eppo/EppoHttpClientRequestCallback.java | 7 -- src/main/java/cloud/eppo/IEppoHttpClient.java | 4 +- .../java/cloud/eppo/api/Configuration.java | 2 +- src/main/java/cloud/eppo/api/EppoValue.java | 21 +++++- .../cloud/eppo/BaseEppoClientBanditTest.java | 4 +- .../java/cloud/eppo/BaseEppoClientTest.java | 49 ++++++++++--- .../eppo/ConfigurationRequestorTest.java | 12 ++-- src/test/java/cloud/eppo/EppoValueTest.java | 14 ---- .../cloud/eppo/ProfileBaseEppoClientTest.java | 2 +- .../java/cloud/eppo/api/EppoValueTest.java | 68 +++++++++++++++++++ .../java/cloud/eppo/helpers/TestUtils.java | 6 +- 15 files changed, 174 insertions(+), 95 deletions(-) delete mode 100644 src/main/java/cloud/eppo/EppoHttpClientRequestCallback.java delete mode 100644 src/test/java/cloud/eppo/EppoValueTest.java create mode 100644 src/test/java/cloud/eppo/api/EppoValueTest.java diff --git a/build.gradle b/build.gradle index 41456454..94df8843 100644 --- a/build.gradle +++ b/build.gradle @@ -6,7 +6,7 @@ plugins { } group = 'cloud.eppo' -version = '3.10.1-SNAPSHOT' +version = '4.0.0' ext.isReleaseVersion = !version.endsWith("SNAPSHOT") java { diff --git a/src/main/java/cloud/eppo/BaseEppoClient.java b/src/main/java/cloud/eppo/BaseEppoClient.java index 6840b4d7..d3dd9fcd 100644 --- a/src/main/java/cloud/eppo/BaseEppoClient.java +++ b/src/main/java/cloud/eppo/BaseEppoClient.java @@ -36,7 +36,7 @@ public class BaseEppoClient { private final BanditLogger banditLogger; private final String sdkName; private final String sdkVersion; - private boolean isGracefulMode; + protected boolean isGracefulMode; private final IAssignmentCache assignmentCache; private final IAssignmentCache banditAssignmentCache; private Timer pollTimer; @@ -96,19 +96,12 @@ private IEppoHttpClient buildHttpClient( : new EppoHttpClient(endpointHelper.getBaseUrl(), sdkKey.getToken(), sdkName, sdkVersion); } - public void activateConfiguration(Configuration configuration) { + public void activateConfiguration(@NotNull Configuration configuration) { requestor.activateConfiguration(configuration); } - protected void loadConfiguration() { - try { - requestor.fetchAndSaveFromRemote(); - } catch (Exception ex) { - log.error("Encountered Exception while loading configuration", ex); - if (!isGracefulMode) { - throw ex; - } - } + protected void fetchAndActivateConfiguration() { + requestor.fetchAndSaveFromRemote(); } protected void stopPolling() { @@ -148,7 +141,14 @@ protected void startPolling(long pollingIntervalMs, long pollingJitterMs) { new FetchConfigurationTask( () -> { log.debug("[Eppo SDK] Polling callback"); - this.loadConfiguration(); + try { + this.fetchAndActivateConfiguration(); + } catch (Exception ex) { + log.error("Encountered Exception while loading configuration", ex); + if (!isGracefulMode) { + throw ex; + } + } }, pollTimer, pollingIntervalMs, @@ -160,7 +160,7 @@ protected void startPolling(long pollingIntervalMs, long pollingJitterMs) { fetchConfigurationsTask.scheduleNext(); } - protected void loadConfigurationAsync(EppoActionCallback callback) { + protected void fetchAndActivateConfigurationAsync(EppoActionCallback callback) { requestor.fetchAndSaveFromRemoteAsync( new EppoActionCallback() { @@ -172,11 +172,7 @@ public void onSuccess(Configuration data) { @Override public void onFailure(Throwable error) { log.error("Encountered Exception while loading configuration", error); - if (isGracefulMode) { - callback.onSuccess(null); - } else { - callback.onFailure(error); - } + callback.onFailure(error); } }); } @@ -568,15 +564,6 @@ public void setIsGracefulFailureMode(boolean isGracefulFailureMode) { this.isGracefulMode = isGracefulFailureMode; } - public abstract static class EppoListener implements Configuration.ConfigurationCallback { - @Override - public void accept(Configuration configuration) { - this.onConfigurationChanged(configuration); - } - - public abstract void onConfigurationChanged(Configuration newConfig); - } - /** * Subscribe to changes to the configuration. * @@ -584,7 +571,7 @@ public void accept(Configuration configuration) { * @return a Runnable which, when called unsubscribes the callback from configuration change * events. */ - public Runnable onConfigurationChange(EppoListener callback) { + public Runnable onConfigurationChange(Configuration.Callback callback) { return requestor.onConfigurationChange(callback); } diff --git a/src/main/java/cloud/eppo/ConfigurationRequestor.java b/src/main/java/cloud/eppo/ConfigurationRequestor.java index 63e7dc34..711ac449 100644 --- a/src/main/java/cloud/eppo/ConfigurationRequestor.java +++ b/src/main/java/cloud/eppo/ConfigurationRequestor.java @@ -14,17 +14,15 @@ public class ConfigurationRequestor { private final IConfigurationStore configurationStore; private final boolean supportBandits; - private final CallbackManager - configChangeManager = - new CallbackManager<>( - // no lambdas before java8 - new CallbackManager.Dispatcher() { - @Override - public void dispatch( - Configuration.ConfigurationCallback callback, Configuration data) { - callback.accept(data); - } - }); + private final CallbackManager configChangeManager = + new CallbackManager<>( + // no lambdas before java8 + new CallbackManager.Dispatcher() { + @Override + public void dispatch(Configuration.Callback callback, Configuration data) { + callback.accept(data); + } + }); public ConfigurationRequestor( @NotNull IConfigurationStore configurationStore, @@ -70,7 +68,7 @@ void fetchAndSaveFromRemoteAsync(EppoActionCallback callback) { client.getAsync( Constants.FLAG_CONFIG_ENDPOINT, - new IEppoHttpClient.EppoHttpCallback() { + new IEppoHttpClient.Callback() { @Override public void onSuccess(byte[] flagConfigJsonBytes) { synchronized (this) { @@ -111,7 +109,7 @@ private void saveConfigurationAndNotify(Configuration configuration) { } } - public Runnable onConfigurationChange(Configuration.ConfigurationCallback callback) { + public Runnable onConfigurationChange(Configuration.Callback callback) { return configChangeManager.subscribe(callback); } } diff --git a/src/main/java/cloud/eppo/EppoHttpClient.java b/src/main/java/cloud/eppo/EppoHttpClient.java index a55ac65b..ad79b160 100644 --- a/src/main/java/cloud/eppo/EppoHttpClient.java +++ b/src/main/java/cloud/eppo/EppoHttpClient.java @@ -4,12 +4,7 @@ import java.io.IOException; import java.net.HttpURLConnection; import java.util.concurrent.TimeUnit; -import okhttp3.Call; -import okhttp3.Callback; -import okhttp3.HttpUrl; -import okhttp3.OkHttpClient; -import okhttp3.Request; -import okhttp3.Response; +import okhttp3.*; import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -60,12 +55,12 @@ public byte[] get(String path) { } @Override - public void getAsync(String path, EppoHttpCallback callback) { + public void getAsync(String path, Callback callback) { Request request = buildRequest(path); client .newCall(request) .enqueue( - new Callback() { + new okhttp3.Callback() { @Override public void onResponse(@NotNull Call call, @NotNull Response response) { if (response.isSuccessful() && response.body() != null) { diff --git a/src/main/java/cloud/eppo/EppoHttpClientRequestCallback.java b/src/main/java/cloud/eppo/EppoHttpClientRequestCallback.java deleted file mode 100644 index 6d67a2c4..00000000 --- a/src/main/java/cloud/eppo/EppoHttpClientRequestCallback.java +++ /dev/null @@ -1,7 +0,0 @@ -package cloud.eppo; - -public interface EppoHttpClientRequestCallback { - void onSuccess(String responseBody); - - void onFailure(String errorMessage); -} diff --git a/src/main/java/cloud/eppo/IEppoHttpClient.java b/src/main/java/cloud/eppo/IEppoHttpClient.java index 6ecd6116..f5387797 100644 --- a/src/main/java/cloud/eppo/IEppoHttpClient.java +++ b/src/main/java/cloud/eppo/IEppoHttpClient.java @@ -5,9 +5,9 @@ public interface IEppoHttpClient { byte[] get(String path); - void getAsync(String path, EppoHttpCallback callback); + void getAsync(String path, Callback callback); - interface EppoHttpCallback extends EppoActionCallback {} + interface Callback extends EppoActionCallback {} class HttpException extends RuntimeException { public HttpException(String message) { diff --git a/src/main/java/cloud/eppo/api/Configuration.java b/src/main/java/cloud/eppo/api/Configuration.java index 4316a3b4..3f8ad14b 100644 --- a/src/main/java/cloud/eppo/api/Configuration.java +++ b/src/main/java/cloud/eppo/api/Configuration.java @@ -59,7 +59,7 @@ public class Configuration { *

Thread safety: Callbacks may be invoked on any thread, including the calling thread or a * background thread. Implementations should be thread-safe and avoid blocking operations. */ - public interface ConfigurationCallback { + public interface Callback { /** * Called when a new configuration is available. * diff --git a/src/main/java/cloud/eppo/api/EppoValue.java b/src/main/java/cloud/eppo/api/EppoValue.java index d9bc7ba4..a8896d30 100644 --- a/src/main/java/cloud/eppo/api/EppoValue.java +++ b/src/main/java/cloud/eppo/api/EppoValue.java @@ -1,6 +1,7 @@ package cloud.eppo.api; import cloud.eppo.ufc.dto.EppoValueType; +import java.util.Iterator; import java.util.List; import java.util.Objects; @@ -101,7 +102,8 @@ public String toString() { case STRING: return this.stringValue; case ARRAY_OF_STRING: - return String.join(" ,", this.stringArrayValue); + // Android21 back-compatability + return joinStringArray(this.stringArrayValue); case NULL: return ""; default: @@ -130,4 +132,21 @@ public boolean equals(Object otherObject) { public int hashCode() { return Objects.hash(type, boolValue, doubleValue, stringValue, stringArrayValue); } + + /** This method is to allow for Android 21 support; String.join was introduced in API 26 */ + private static String joinStringArray(List stringArray) { + if (stringArray == null || stringArray.isEmpty()) { + return ""; + } + String delimiter = ", "; + StringBuilder stringBuilder = new StringBuilder(); + Iterator iterator = stringArray.iterator(); + while (iterator.hasNext()) { + stringBuilder.append(iterator.next()); + if (iterator.hasNext()) { + stringBuilder.append(delimiter); + } + } + return stringBuilder.toString(); + } } diff --git a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java index cc7358d4..009154bf 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java @@ -78,7 +78,7 @@ public static void initClient() { new ExpiringInMemoryAssignmentCache( banditAssignmentCache, 50, TimeUnit.MILLISECONDS) {}); - eppoClient.loadConfiguration(); + eppoClient.fetchAndActivateConfiguration(); log.info("Test client initialized"); } @@ -465,7 +465,7 @@ public void testWithInitialConfiguration() { assertEquals("adidas", result.getAction()); // Demonstrate that loaded configuration is different from the initial string passed above. - client.loadConfiguration(); + client.fetchAndActivateConfiguration(); BanditResult banditResult = client.getBanditAction( "banner_bandit_flag", "subject", new Attributes(), actions, "default"); diff --git a/src/test/java/cloud/eppo/BaseEppoClientTest.java b/src/test/java/cloud/eppo/BaseEppoClientTest.java index 84aeab04..aeb1d7ed 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientTest.java @@ -102,7 +102,13 @@ private void initClient(boolean isGracefulMode, String sdkName, boolean loadConf null); if (loadConfig) { - eppoClient.loadConfiguration(); + try { + eppoClient.fetchAndActivateConfiguration(); + } catch (Exception e) { + if (!isGracefulMode) { + throw e; + } + } } log.info("Test client initialized"); } @@ -127,7 +133,34 @@ private void initClientAsync(boolean isGracefulMode, InitCallback initCallback) null, null); - eppoClient.loadConfigurationAsync(initCallback); + // The common SDK doesn't actually have an "initialization" method. This method stands in for + // "build and instance + // and activate some configuration". + // Thus, we must provide a fallback if graceful mode is true and activating config fails. + InitCallback onInit = + new InitCallback() { + + @Override + public void onSuccess(Configuration data) { + if (data == null) { + data = Configuration.emptyConfig(); + // Fetch and activate did not produce a config, so we set an empty one. + eppoClient.activateConfiguration(data); + } + initCallback.onSuccess(data); + } + + @Override + public void onFailure(Throwable error) { + if (isGracefulMode) { + initCallback.onSuccess(null); + } else { + initCallback.onFailure(error); + } + } + }; + + eppoClient.fetchAndActivateConfigurationAsync(onInit); } private void initClientWithAssignmentCache(IAssignmentCache cache) { @@ -148,7 +181,7 @@ private void initClientWithAssignmentCache(IAssignmentCache cache) { cache, null); - eppoClient.loadConfiguration(); + eppoClient.fetchAndActivateConfiguration(); log.info("Test client initialized"); } @@ -411,7 +444,7 @@ public void testWithInitialConfiguration() { assertEquals(5, result); // Demonstrate that loaded configuration is different from the initial string passed above. - eppoClient.loadConfiguration(); + eppoClient.fetchAndActivateConfiguration(); double updatedResult = eppoClient.getDoubleAssignment("numeric_flag", "dummy subject", 0); assertEquals(3.1415926, updatedResult); } catch (IOException e) { @@ -436,7 +469,7 @@ public void testWithActivatedConfiguration() { assertEquals(5, result); // Demonstrate that loaded configuration is different from the initial string passed above. - eppoClient.loadConfiguration(); + eppoClient.fetchAndActivateConfiguration(); double updatedResult = eppoClient.getDoubleAssignment("numeric_flag", "dummy subject", 0); assertEquals(3.1415926, updatedResult); } catch (IOException e) { @@ -608,7 +641,7 @@ public void testPolling() { null, null); - client.loadConfiguration(); + client.fetchAndActivateConfiguration(); client.startPolling(20); // Method will be called immediately on init @@ -687,7 +720,7 @@ public void testGetConfigurationWithInitialConfig() { assertNull(config.getFlag("no_allocations_flag")); // Load new configuration - eppoClient.loadConfiguration(); + eppoClient.fetchAndActivateConfiguration(); // Get updated configuration Configuration updatedConfig = eppoClient.getConfiguration(); @@ -729,7 +762,7 @@ public void testGetConfigurationBeforeInitialization() { assertNotNull(config); assertTrue(config.isEmpty()); - eppoClient.loadConfiguration(); + eppoClient.fetchAndActivateConfiguration(); // Get configuration again after loading Configuration nextConfig = eppoClient.getConfiguration(); diff --git a/src/test/java/cloud/eppo/ConfigurationRequestorTest.java b/src/test/java/cloud/eppo/ConfigurationRequestorTest.java index bb63567c..135ab2e9 100644 --- a/src/test/java/cloud/eppo/ConfigurationRequestorTest.java +++ b/src/test/java/cloud/eppo/ConfigurationRequestorTest.java @@ -108,7 +108,7 @@ public void testCacheWritesAfterBrokenFetch() throws IOException, InterruptedExc } /** Helper class for tracking configurations received via callbacks. */ - static class ListAddingConfigCallback implements Configuration.ConfigurationCallback { + static class ListAddingConfigCallback implements Configuration.Callback { public final List results = new ArrayList<>(); @Override @@ -179,7 +179,7 @@ public void testMultipleConfigurationChangeListeners() { // Subscribe multiple listeners Runnable unsubscribe1 = requestor.onConfigurationChange( - new Configuration.ConfigurationCallback() { + new Configuration.Callback() { @Override public void accept(Configuration configuration) { callCount1.incrementAndGet(); @@ -187,7 +187,7 @@ public void accept(Configuration configuration) { }); Runnable unsubscribe2 = requestor.onConfigurationChange( - new Configuration.ConfigurationCallback() { + new Configuration.Callback() { @Override public void accept(Configuration configuration) { callCount2.incrementAndGet(); @@ -219,7 +219,7 @@ public void testConfigurationChangeListenerIgnoresFailedFetch() { AtomicInteger callCount = new AtomicInteger(0); requestor.onConfigurationChange( - new Configuration.ConfigurationCallback() { + new Configuration.Callback() { @Override public void accept(Configuration configuration) { callCount.incrementAndGet(); @@ -245,7 +245,7 @@ public void testConfigurationChangeListenerIgnoresFailedSave() { AtomicInteger callCount = new AtomicInteger(0); requestor.onConfigurationChange( - new Configuration.ConfigurationCallback() { + new Configuration.Callback() { @Override public void accept(Configuration configuration) { callCount.incrementAndGet(); @@ -271,7 +271,7 @@ public void testConfigurationChangeListenerAsyncSave() throws InterruptedExcepti CountDownLatch countDownLatch = new CountDownLatch(1); requestor.onConfigurationChange( - new Configuration.ConfigurationCallback() { + new Configuration.Callback() { @Override public void accept(Configuration configuration) { countDownLatch.countDown(); diff --git a/src/test/java/cloud/eppo/EppoValueTest.java b/src/test/java/cloud/eppo/EppoValueTest.java deleted file mode 100644 index 5df61127..00000000 --- a/src/test/java/cloud/eppo/EppoValueTest.java +++ /dev/null @@ -1,14 +0,0 @@ -package cloud.eppo; - -import static org.junit.jupiter.api.Assertions.assertEquals; - -import cloud.eppo.api.EppoValue; -import org.junit.jupiter.api.Test; - -public class EppoValueTest { - @Test - public void testDoubleValue() { - EppoValue eppoValue = EppoValue.valueOf(123.4567); - assertEquals(123.4567, eppoValue.doubleValue(), 0.0); - } -} diff --git a/src/test/java/cloud/eppo/ProfileBaseEppoClientTest.java b/src/test/java/cloud/eppo/ProfileBaseEppoClientTest.java index a5945ccc..3e9206e7 100644 --- a/src/test/java/cloud/eppo/ProfileBaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/ProfileBaseEppoClientTest.java @@ -49,7 +49,7 @@ public static void initClient() { null, null); - eppoClient.loadConfiguration(); + eppoClient.fetchAndActivateConfiguration(); log.info("Test client initialized"); } diff --git a/src/test/java/cloud/eppo/api/EppoValueTest.java b/src/test/java/cloud/eppo/api/EppoValueTest.java new file mode 100644 index 00000000..f2830ad2 --- /dev/null +++ b/src/test/java/cloud/eppo/api/EppoValueTest.java @@ -0,0 +1,68 @@ +package cloud.eppo.api; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import org.junit.jupiter.api.Test; + +public class EppoValueTest { + @Test + public void testDoubleValue() { + EppoValue eppoValue = EppoValue.valueOf(123.4567); + assertEquals(123.4567, eppoValue.doubleValue(), 0.0); + } + + @Test + public void testToStringWithStringArray() { + // Test with multiple values + List values = Arrays.asList("one", "two", "three"); + EppoValue value = EppoValue.valueOf(values); + assertEquals("one, two, three", value.toString()); + + // Test with single value + List singleValue = Arrays.asList("solo"); + EppoValue singleValueObj = EppoValue.valueOf(singleValue); + assertEquals("solo", singleValueObj.toString()); + + // Test with empty list + List emptyList = new ArrayList<>(); + EppoValue emptyValue = EppoValue.valueOf(emptyList); + assertEquals("", emptyValue.toString()); + } + + @Test + public void testStringArrayJoining() { + // Test joining behavior with various arrays + List values = Arrays.asList("a", "b", "c"); + EppoValue value = EppoValue.valueOf(values); + assertEquals("a, b, c", value.toString()); + + // Test with values containing the delimiter + List commaValues = Arrays.asList("first,item", "second ,item"); + EppoValue commaValue = EppoValue.valueOf(commaValues); + assertEquals("first,item, second ,item", commaValue.toString()); + } + + @Test + public void testToStringConsistencyAcrossTypes() { + // Verify string representation is consistent across types + EppoValue boolValue = EppoValue.valueOf(true); + assertEquals("true", boolValue.toString()); + + EppoValue numValue = EppoValue.valueOf(42.5); + assertEquals("42.5", numValue.toString()); + + EppoValue strValue = EppoValue.valueOf("test"); + assertEquals("test", strValue.toString()); + + EppoValue nullValue = EppoValue.nullValue(); + assertEquals("", nullValue.toString()); + + // String array should now use our custom joiner + List array = Arrays.asList("test1", "test2"); + EppoValue arrayValue = EppoValue.valueOf(array); + assertEquals("test1, test2", arrayValue.toString()); + } +} diff --git a/src/test/java/cloud/eppo/helpers/TestUtils.java b/src/test/java/cloud/eppo/helpers/TestUtils.java index 057583d9..5290500f 100644 --- a/src/test/java/cloud/eppo/helpers/TestUtils.java +++ b/src/test/java/cloud/eppo/helpers/TestUtils.java @@ -54,14 +54,14 @@ public byte[] get(String path) { } @Override - public void getAsync(String path, EppoHttpCallback callback) { + public void getAsync(String path, Callback callback) { callback.onFailure(new RuntimeException("Intentional Error")); } } public static class DelayedHttpClient implements IEppoHttpClient { protected byte[] responseBody; - private EppoHttpCallback callback; + private Callback callback; private boolean flushed = false; private Throwable error = null; @@ -75,7 +75,7 @@ public byte[] get(String path) { } @Override - public void getAsync(String path, EppoHttpCallback callback) { + public void getAsync(String path, Callback callback) { if (flushed) { callback.onSuccess(responseBody); } else if (error != null) { From 348ccd087841c6e6a0b4a109dc627fad9ffe4d11 Mon Sep 17 00:00:00 2001 From: Tyler Potter Date: Wed, 21 May 2025 14:56:02 -0600 Subject: [PATCH 13/14] chore-feat: refactor out base64 ops (#117) * chore: replace use of string.join for android 21 (#111) * extract base64 codec for portation --- src/main/java/cloud/eppo/Utils.java | 37 ++++++++++++++----- .../java/cloud/eppo/ApiEndpointsTest.java | 7 ++++ .../java/cloud/eppo/BaseEppoClientTest.java | 7 ++++ .../cloud/eppo/helpers/JavaBase64Codec.java | 22 +++++++++++ 4 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 src/test/java/cloud/eppo/helpers/JavaBase64Codec.java diff --git a/src/main/java/cloud/eppo/Utils.java b/src/main/java/cloud/eppo/Utils.java index 41c9874b..d865f7af 100644 --- a/src/main/java/cloud/eppo/Utils.java +++ b/src/main/java/cloud/eppo/Utils.java @@ -1,14 +1,13 @@ package cloud.eppo; import com.fasterxml.jackson.databind.JsonNode; -import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.text.ParseException; import java.text.SimpleDateFormat; -import java.util.Base64; import java.util.Date; import java.util.Locale; +import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -16,6 +15,11 @@ public final class Utils { private static final SimpleDateFormat UTC_ISO_DATE_FORMAT = buildUtcIsoDateFormat(); private static final Logger log = LoggerFactory.getLogger(Utils.class); private static final MessageDigest md = buildMd5MessageDigest(); + private static Base64Codec base64Codec; + + public static void setBase64Codec(@NotNull Base64Codec base64Codec) { + Utils.base64Codec = base64Codec; + } private static MessageDigest buildMd5MessageDigest() { try { @@ -101,23 +105,32 @@ public static String getISODate(Date date) { return UTC_ISO_DATE_FORMAT.format(date); } + /** + * An implementation of the Base64Codec is required to be set before these methods work. ex: + * Utils.setBase64Codec(new JavaBase64Codec()); + */ public static String base64Encode(String input) { + if (base64Codec == null) { + throw new RuntimeException("Base64 codec not initialized"); + } if (input == null) { return null; } - return new String(Base64.getEncoder().encode(input.getBytes(StandardCharsets.UTF_8))); + return base64Codec.base64Encode(input); } + /** + * An implementation of the Base64Codec is required to be set before these methods work. ex: + * Utils.setBase64Codec(new JavaBase64Codec()); + */ public static String base64Decode(String input) { + if (base64Codec == null) { + throw new RuntimeException("Base64 codec not initialized"); + } if (input == null) { return null; } - byte[] decodedBytes = Base64.getDecoder().decode(input); - if (decodedBytes.length == 0 && !input.isEmpty()) { - throw new RuntimeException( - "zero byte output from Base64; if not running on Android hardware be sure to use RobolectricTestRunner"); - } - return new String(decodedBytes); + return base64Codec.base64Decode(input); } private static SimpleDateFormat buildUtcIsoDateFormat() { @@ -126,4 +139,10 @@ private static SimpleDateFormat buildUtcIsoDateFormat() { dateFormat.setTimeZone(java.util.TimeZone.getTimeZone("UTC")); return dateFormat; } + + public interface Base64Codec { + String base64Encode(String input); + + String base64Decode(String input); + } } diff --git a/src/test/java/cloud/eppo/ApiEndpointsTest.java b/src/test/java/cloud/eppo/ApiEndpointsTest.java index dd947081..75bb30e7 100644 --- a/src/test/java/cloud/eppo/ApiEndpointsTest.java +++ b/src/test/java/cloud/eppo/ApiEndpointsTest.java @@ -2,11 +2,18 @@ import static org.junit.jupiter.api.Assertions.*; +import cloud.eppo.helpers.JavaBase64Codec; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; public class ApiEndpointsTest { final SDKKey plainKey = new SDKKey("flat token"); + @BeforeAll + public static void setUp() { + Utils.setBase64Codec(new JavaBase64Codec()); + } + @Test public void testDefaultBaseUrl() { ApiEndpoints endpoints = new ApiEndpoints(plainKey, null); diff --git a/src/test/java/cloud/eppo/BaseEppoClientTest.java b/src/test/java/cloud/eppo/BaseEppoClientTest.java index aeb1d7ed..25d7fc9b 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientTest.java @@ -11,6 +11,7 @@ import cloud.eppo.api.*; import cloud.eppo.cache.LRUInMemoryAssignmentCache; import cloud.eppo.helpers.AssignmentTestCase; +import cloud.eppo.helpers.JavaBase64Codec; import cloud.eppo.helpers.TestUtils; import cloud.eppo.logging.Assignment; import cloud.eppo.logging.AssignmentLogger; @@ -31,6 +32,7 @@ import java.util.stream.Stream; import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -185,6 +187,11 @@ private void initClientWithAssignmentCache(IAssignmentCache cache) { log.info("Test client initialized"); } + @BeforeAll + public static void setUp() { + Utils.setBase64Codec(new JavaBase64Codec()); + } + @AfterEach public void cleanUp() { // TODO: Clear any caches diff --git a/src/test/java/cloud/eppo/helpers/JavaBase64Codec.java b/src/test/java/cloud/eppo/helpers/JavaBase64Codec.java new file mode 100644 index 00000000..87296265 --- /dev/null +++ b/src/test/java/cloud/eppo/helpers/JavaBase64Codec.java @@ -0,0 +1,22 @@ +package cloud.eppo.helpers; + +import cloud.eppo.Utils; +import java.nio.charset.StandardCharsets; +import java.util.Base64; + +public class JavaBase64Codec implements Utils.Base64Codec { + @Override + public String base64Encode(String input) { + return new String(Base64.getEncoder().encode(input.getBytes(StandardCharsets.UTF_8))); + } + + @Override + public String base64Decode(String input) { + byte[] decodedBytes = Base64.getDecoder().decode(input); + if (decodedBytes.length == 0 && !input.isEmpty()) { + throw new RuntimeException( + "zero byte output from Base64; if not running on Android hardware be sure to use RobolectricTestRunner"); + } + return new String(decodedBytes); + } +} From f6a9a7d1160d992892fcd7e02d019081d2b1cbfe Mon Sep 17 00:00:00 2001 From: Tyler Potter Date: Mon, 26 May 2025 09:29:17 -0600 Subject: [PATCH 14/14] chore: refactor out JSON parsing (#118) * chore: refactor out JSON parsing --- build.gradle | 2 +- src/main/java/cloud/eppo/BaseEppoClient.java | 72 ++----------- src/main/java/cloud/eppo/Utils.java | 102 +++++++++++++----- src/main/java/cloud/eppo/api/Attributes.java | 40 +------ .../java/cloud/eppo/api/Configuration.java | 37 ++----- .../cloud/eppo/callback/CallbackManager.java | 24 +++-- .../eppo/exception/JsonParsingException.java | 7 ++ .../java/cloud/eppo/model/ShardRange.java | 6 +- .../cloud/eppo/BaseEppoClientBanditTest.java | 5 + .../java/cloud/eppo/BaseEppoClientTest.java | 43 ++------ .../eppo/ConfigurationRequestorTest.java | 5 + src/test/java/cloud/eppo/UtilsTest.java | 1 + .../eppo/api/ConfigurationBuilderTest.java | 2 +- .../eppo/helpers/AssignmentTestCase.java | 18 +++- .../AssignmentTestCaseDeserializer.java | 2 +- .../helpers/BanditTestCaseDeserializer.java | 2 +- .../eppo/helpers/JacksonJsonDeserializer.java | 92 ++++++++++++++++ .../java/cloud/eppo/helpers/TestUtils.java | 5 + .../BanditParametersResponseDeserializer.java | 2 +- .../helpers}/dto/adapters/DateSerializer.java | 2 +- .../helpers}/dto/adapters/EppoModule.java | 2 +- .../dto/adapters/EppoValueDeserializer.java | 2 +- .../dto/adapters/EppoValueSerializer.java | 2 +- .../FlagConfigResponseDeserializer.java | 4 +- ...ditParametersResponseDeserializerTest.java | 2 +- .../FlagConfigResponseDeserializerTest.java | 2 +- 26 files changed, 264 insertions(+), 219 deletions(-) create mode 100644 src/main/java/cloud/eppo/exception/JsonParsingException.java create mode 100644 src/test/java/cloud/eppo/helpers/JacksonJsonDeserializer.java rename src/{main/java/cloud/eppo/ufc => test/java/cloud/eppo/helpers}/dto/adapters/BanditParametersResponseDeserializer.java (99%) rename src/{main/java/cloud/eppo/ufc => test/java/cloud/eppo/helpers}/dto/adapters/DateSerializer.java (94%) rename src/{main/java/cloud/eppo/ufc => test/java/cloud/eppo/helpers}/dto/adapters/EppoModule.java (95%) rename src/{main/java/cloud/eppo/ufc => test/java/cloud/eppo/helpers}/dto/adapters/EppoValueDeserializer.java (97%) rename src/{main/java/cloud/eppo/ufc => test/java/cloud/eppo/helpers}/dto/adapters/EppoValueSerializer.java (95%) rename src/{main/java/cloud/eppo/ufc => test/java/cloud/eppo/helpers}/dto/adapters/FlagConfigResponseDeserializer.java (98%) diff --git a/build.gradle b/build.gradle index cfb64bc9..b8cebf05 100644 --- a/build.gradle +++ b/build.gradle @@ -17,12 +17,12 @@ java { } dependencies { - implementation 'com.fasterxml.jackson.core:jackson-databind:2.18.3' implementation 'com.github.zafarkhaja:java-semver:0.10.2' implementation "com.squareup.okhttp3:okhttp:4.12.0" // For LRU and expiring maps implementation 'org.apache.commons:commons-collections4:4.5.0' implementation 'org.slf4j:slf4j-api:2.0.17' + testImplementation 'com.fasterxml.jackson.core:jackson-databind:2.18.3' testImplementation 'org.slf4j:slf4j-simple:2.0.17' testImplementation platform('org.junit:junit-bom:5.11.4') testImplementation("com.squareup.okhttp3:mockwebserver:4.12.0") diff --git a/src/main/java/cloud/eppo/BaseEppoClient.java b/src/main/java/cloud/eppo/BaseEppoClient.java index d3dd9fcd..2ecabee2 100644 --- a/src/main/java/cloud/eppo/BaseEppoClient.java +++ b/src/main/java/cloud/eppo/BaseEppoClient.java @@ -11,10 +11,6 @@ import cloud.eppo.logging.BanditAssignment; import cloud.eppo.logging.BanditLogger; import cloud.eppo.ufc.dto.*; -import cloud.eppo.ufc.dto.adapters.EppoModule; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; import java.util.HashMap; import java.util.Map; import java.util.Timer; @@ -25,9 +21,6 @@ public class BaseEppoClient { private static final Logger log = LoggerFactory.getLogger(BaseEppoClient.class); - private final ObjectMapper mapper = - new ObjectMapper() - .registerModule(EppoModule.eppoModule()); // TODO: is this the best place for this? protected final ConfigurationRequestor requestor; @@ -101,7 +94,15 @@ public void activateConfiguration(@NotNull Configuration configuration) { } protected void fetchAndActivateConfiguration() { - requestor.fetchAndSaveFromRemote(); + try { + requestor.fetchAndSaveFromRemote(); + } catch (Throwable e) { + if (isGracefulMode) { + log.error(e.getMessage(), e); + } else { + throw e; + } + } } protected void stopPolling() { @@ -293,10 +294,7 @@ private boolean valueTypeMatchesExpected(VariationType expectedType, EppoValue v typeMatch = value.isString(); break; case JSON: - typeMatch = - value.isString() - // Eppo leaves JSON as a JSON string; to verify it's valid we attempt to parse - && parseJsonString(value.stringValue()) != null; + typeMatch = value.isString() && Utils.isValidJson(value.stringValue()); break; default: throw new IllegalArgumentException("Unexpected type for type checking: " + expectedType); @@ -385,46 +383,6 @@ public String getStringAssignment( } } - /** - * Returns the assignment for the provided feature flag key and subject key as a {@link JsonNode}. - * If the flag is not found, does not match the requested type or is disabled, defaultValue is - * returned. - * - * @param flagKey the feature flag key - * @param subjectKey the subject key - * @param defaultValue the default value to return if the flag is not found - * @return the JSON string value of the assignment - */ - public JsonNode getJSONAssignment(String flagKey, String subjectKey, JsonNode defaultValue) { - return getJSONAssignment(flagKey, subjectKey, new Attributes(), defaultValue); - } - - /** - * Returns the assignment for the provided feature flag key and subject key as a {@link JsonNode}. - * If the flag is not found, does not match the requested type or is disabled, defaultValue is - * returned. - * - * @param flagKey the feature flag key - * @param subjectKey the subject key - * @param defaultValue the default value to return if the flag is not found - * @return the JSON string value of the assignment - */ - public JsonNode getJSONAssignment( - String flagKey, String subjectKey, Attributes subjectAttributes, JsonNode defaultValue) { - try { - EppoValue value = - this.getTypedAssignment( - flagKey, - subjectKey, - subjectAttributes, - EppoValue.valueOf(defaultValue.toString()), - VariationType.JSON); - return parseJsonString(value.stringValue()); - } catch (Exception e) { - return throwIfNotGraceful(e, defaultValue); - } - } - /** * Returns the assignment for the provided feature flag key, subject key and subject attributes as * a JSON string. If the flag is not found, does not match the requested type or is disabled, @@ -465,14 +423,6 @@ public String getJSONStringAssignment(String flagKey, String subjectKey, String return this.getJSONStringAssignment(flagKey, subjectKey, new Attributes(), defaultValue); } - private JsonNode parseJsonString(String jsonString) { - try { - return mapper.readTree(jsonString); - } catch (JsonProcessingException e) { - return null; - } - } - public BanditResult getBanditAction( String flagKey, String subjectKey, @@ -552,7 +502,7 @@ private Map buildLogMetaData(boolean isConfigObfuscated) { return metaData; } - private T throwIfNotGraceful(Exception e, T defaultValue) { + protected T throwIfNotGraceful(Exception e, T defaultValue) { if (this.isGracefulMode) { log.info("error getting assignment value: {}", e.getMessage()); return defaultValue; diff --git a/src/main/java/cloud/eppo/Utils.java b/src/main/java/cloud/eppo/Utils.java index d865f7af..f22de75e 100644 --- a/src/main/java/cloud/eppo/Utils.java +++ b/src/main/java/cloud/eppo/Utils.java @@ -1,13 +1,18 @@ package cloud.eppo; -import com.fasterxml.jackson.databind.JsonNode; +import cloud.eppo.api.EppoValue; +import cloud.eppo.exception.JsonParsingException; +import cloud.eppo.ufc.dto.BanditParametersResponse; +import cloud.eppo.ufc.dto.FlagConfigResponse; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.text.ParseException; import java.text.SimpleDateFormat; import java.util.Date; import java.util.Locale; +import java.util.Map; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -16,11 +21,16 @@ public final class Utils { private static final Logger log = LoggerFactory.getLogger(Utils.class); private static final MessageDigest md = buildMd5MessageDigest(); private static Base64Codec base64Codec; + private static JsonDeserializer jsonDeserializer; public static void setBase64Codec(@NotNull Base64Codec base64Codec) { Utils.base64Codec = base64Codec; } + public static void setJsonDeserializer(@NotNull JsonDeserializer jsonDeserializer) { + Utils.jsonDeserializer = jsonDeserializer; + } + private static MessageDigest buildMd5MessageDigest() { try { return MessageDigest.getInstance("MD5"); @@ -75,32 +85,6 @@ public static int getShard(String input, int maxShardValue) { return (int) (value % maxShardValue); } - public static Date parseUtcISODateNode(JsonNode isoDateStringElement) { - if (isoDateStringElement == null || isoDateStringElement.isNull()) { - return null; - } - String isoDateString = isoDateStringElement.asText(); - Date result = null; - try { - result = UTC_ISO_DATE_FORMAT.parse(isoDateString); - } catch (ParseException e) { - // We expect to fail parsing if the date is base 64 encoded - // Thus we'll leave the result null for now and try again with the decoded value - } - - if (result == null) { - // Date may be encoded - String decodedIsoDateString = base64Decode(isoDateString); - try { - result = UTC_ISO_DATE_FORMAT.parse(decodedIsoDateString); - } catch (ParseException e) { - log.warn("Date \"{}\" not in ISO date format", isoDateString); - } - } - - return result; - } - public static String getISODate(Date date) { return UTC_ISO_DATE_FORMAT.format(date); } @@ -140,9 +124,73 @@ private static SimpleDateFormat buildUtcIsoDateFormat() { return dateFormat; } + public static Date parseUtcISODateString(@Nullable String isoDateString) { + if (isoDateString == null) { + return null; + } + Date result = null; + try { + result = UTC_ISO_DATE_FORMAT.parse(isoDateString); + } catch (ParseException e) { + // We expect to fail parsing if the date is base 64 encoded + // Thus we'll leave the result null for now and try again with the decoded value + } + + if (result == null) { + // Date may be encoded + String decodedIsoDateString = base64Decode(isoDateString); + try { + result = UTC_ISO_DATE_FORMAT.parse(decodedIsoDateString); + } catch (ParseException e) { + log.warn("Date \"{}\" not in ISO date format", isoDateString); + } + } + return result; + } + + private static void verifyJsonParser() { + if (jsonDeserializer == null) { + throw new RuntimeException("JSON Parser not initialized/set on Utils"); + } + } + + public static FlagConfigResponse parseFlagConfigResponse(byte[] jsonString) + throws JsonParsingException { + verifyJsonParser(); + return jsonDeserializer.parseFlagConfigResponse(jsonString); + } + + public static BanditParametersResponse parseBanditParametersResponse(byte[] jsonString) + throws JsonParsingException { + verifyJsonParser(); + return jsonDeserializer.parseBanditParametersResponse(jsonString); + } + + public static boolean isValidJson(String json) { + verifyJsonParser(); + return jsonDeserializer.isValidJson(json); + } + + public static String serializeAttributesToJSONString( + Map map, boolean omitNulls) { + verifyJsonParser(); + return jsonDeserializer.serializeAttributesToJSONString(map, omitNulls); + } + public interface Base64Codec { String base64Encode(String input); String base64Decode(String input); } + + public interface JsonDeserializer { + FlagConfigResponse parseFlagConfigResponse(byte[] jsonString) throws JsonParsingException; + + BanditParametersResponse parseBanditParametersResponse(byte[] jsonString) + throws JsonParsingException; + + boolean isValidJson(String json); + + String serializeAttributesToJSONString(Map map, boolean omitNulls); + } } diff --git a/src/main/java/cloud/eppo/api/Attributes.java b/src/main/java/cloud/eppo/api/Attributes.java index 2406d171..8bad0dc5 100644 --- a/src/main/java/cloud/eppo/api/Attributes.java +++ b/src/main/java/cloud/eppo/api/Attributes.java @@ -1,8 +1,6 @@ package cloud.eppo.api; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ObjectNode; +import cloud.eppo.Utils; import java.util.HashMap; import java.util.Map; import java.util.stream.Collectors; @@ -67,40 +65,6 @@ public Attributes getAllAttributes() { /** Serializes the attributes to a JSON string, omitting attributes with a null value. */ public String serializeNonNullAttributesToJSONString() { - return serializeAttributesToJSONString(true); - } - - @SuppressWarnings("SameParameterValue") - private String serializeAttributesToJSONString(boolean omitNulls) { - ObjectMapper mapper = new ObjectMapper(); - ObjectNode result = mapper.createObjectNode(); - - for (Map.Entry entry : entrySet()) { - String attributeName = entry.getKey(); - EppoValue attributeValue = entry.getValue(); - - if (attributeValue == null || attributeValue.isNull()) { - if (!omitNulls) { - result.putNull(attributeName); - } - } else { - if (attributeValue.isNumeric()) { - result.put(attributeName, attributeValue.doubleValue()); - continue; - } - if (attributeValue.isBoolean()) { - result.put(attributeName, attributeValue.booleanValue()); - continue; - } - // fall back put treating any other eppo values as a string - result.put(attributeName, attributeValue.toString()); - } - } - - try { - return mapper.writeValueAsString(result); - } catch (JsonProcessingException e) { - throw new RuntimeException(e); - } + return Utils.serializeAttributesToJSONString(this, true); } } diff --git a/src/main/java/cloud/eppo/api/Configuration.java b/src/main/java/cloud/eppo/api/Configuration.java index 3f8ad14b..3b50d967 100644 --- a/src/main/java/cloud/eppo/api/Configuration.java +++ b/src/main/java/cloud/eppo/api/Configuration.java @@ -2,12 +2,9 @@ import static cloud.eppo.Utils.getMD5Hex; +import cloud.eppo.Utils; +import cloud.eppo.exception.JsonParsingException; import cloud.eppo.ufc.dto.*; -import cloud.eppo.ufc.dto.adapters.EppoModule; -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ObjectNode; -import java.io.*; import java.util.Collections; import java.util.Map; import java.util.Set; @@ -68,9 +65,6 @@ public interface Callback { void accept(Configuration configuration); } - private static final ObjectMapper mapper = - new ObjectMapper().registerModule(EppoModule.eppoModule()); - private static final byte[] emptyFlagsBytes = "{ \"flags\": {}, \"format\": \"SERVER\" }".getBytes(); @@ -98,20 +92,6 @@ public interface Callback { this.bandits = bandits; this.isConfigObfuscated = isConfigObfuscated; - // Graft the `forServer` boolean into the flagConfigJson' - if (flagConfigJson != null && flagConfigJson.length != 0) { - try { - JsonNode jNode = mapper.readTree(flagConfigJson); - FlagConfigResponse.Format format = - isConfigObfuscated - ? FlagConfigResponse.Format.CLIENT - : FlagConfigResponse.Format.SERVER; - ((ObjectNode) jNode).put("format", format.toString()); - flagConfigJson = mapper.writeValueAsBytes(jNode); - } catch (IOException e) { - log.error("Error adding `format` field to FlagConfigResponse JSON"); - } - } this.flagConfigJson = flagConfigJson; this.banditParamsJson = banditParamsJson; } @@ -189,6 +169,10 @@ public boolean isEmpty() { return flags == null || flags.isEmpty(); } + public Set getFlagKeys() { + return flags == null ? Collections.emptySet() : flags.keySet(); + } + public static Builder builder(byte[] flagJson) { return new Builder(flagJson); } @@ -212,10 +196,9 @@ private static FlagConfigResponse parseFlagResponse(byte[] flagJson) { log.warn("Null or empty configuration string. Call `Configuration.Empty()` instead"); return null; } - FlagConfigResponse config; try { - return mapper.readValue(flagJson, FlagConfigResponse.class); - } catch (IOException e) { + return Utils.parseFlagConfigResponse(flagJson); + } catch (JsonParsingException e) { throw new RuntimeException(e); } } @@ -289,8 +272,8 @@ public Builder banditParameters(byte[] banditParameterJson) { } BanditParametersResponse config; try { - config = mapper.readValue(banditParameterJson, BanditParametersResponse.class); - } catch (IOException e) { + config = Utils.parseBanditParametersResponse(banditParameterJson); + } catch (JsonParsingException e) { log.error("Unable to parse bandit parameters JSON"); throw new RuntimeException(e); } diff --git a/src/main/java/cloud/eppo/callback/CallbackManager.java b/src/main/java/cloud/eppo/callback/CallbackManager.java index ac0f7550..9934ed74 100644 --- a/src/main/java/cloud/eppo/callback/CallbackManager.java +++ b/src/main/java/cloud/eppo/callback/CallbackManager.java @@ -43,7 +43,12 @@ public Runnable subscribe(C callback) { String id = UUID.randomUUID().toString(); subscribers.put(id, callback); - return () -> subscribers.remove(id); + return new Runnable() { + @Override + public void run() { + subscribers.remove(id); + } + }; } /** @@ -52,16 +57,13 @@ public Runnable subscribe(C callback) { * @param data The data to pass to all callbacks */ public void notifyCallbacks(T data) { - subscribers - .values() - .forEach( - callback -> { - try { - dispatcher.dispatch(callback, data); - } catch (Exception e) { - log.error("Eppo SDK: Error thrown by callback: {}", e.getMessage()); - } - }); + Iterable subs = subscribers.values(); + for (C sub : subs) { + try { + dispatcher.dispatch(sub, data); + } catch (Exception ignored) { + } + } } /** Remove all subscribers. */ diff --git a/src/main/java/cloud/eppo/exception/JsonParsingException.java b/src/main/java/cloud/eppo/exception/JsonParsingException.java new file mode 100644 index 00000000..87fcb0ec --- /dev/null +++ b/src/main/java/cloud/eppo/exception/JsonParsingException.java @@ -0,0 +1,7 @@ +package cloud.eppo.exception; + +public class JsonParsingException extends Throwable { + public JsonParsingException(Throwable e) { + super(e); + } +} diff --git a/src/main/java/cloud/eppo/model/ShardRange.java b/src/main/java/cloud/eppo/model/ShardRange.java index b03e06c9..63fe40ca 100644 --- a/src/main/java/cloud/eppo/model/ShardRange.java +++ b/src/main/java/cloud/eppo/model/ShardRange.java @@ -1,15 +1,11 @@ package cloud.eppo.model; -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonProperty; - /** Shard Range Class */ public class ShardRange { private final int start; private int end; - @JsonCreator - public ShardRange(@JsonProperty("start") int start, @JsonProperty("end") int end) { + public ShardRange(int start, int end) { this.start = start; this.end = end; } diff --git a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java index 009154bf..a414a12b 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientBanditTest.java @@ -54,6 +54,11 @@ public class BaseEppoClientBanditTest { private static final Map assignmentCache = new HashMap<>(); private static final Map banditAssignmentCache = new HashMap<>(); + static { + Utils.setBase64Codec(new JavaBase64Codec()); + Utils.setJsonDeserializer(new JacksonJsonDeserializer()); + } + @BeforeEach public void resetCaches() { assignmentCache.clear(); diff --git a/src/test/java/cloud/eppo/BaseEppoClientTest.java b/src/test/java/cloud/eppo/BaseEppoClientTest.java index 25d7fc9b..44f43a85 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientTest.java @@ -11,6 +11,7 @@ import cloud.eppo.api.*; import cloud.eppo.cache.LRUInMemoryAssignmentCache; import cloud.eppo.helpers.AssignmentTestCase; +import cloud.eppo.helpers.JacksonJsonDeserializer; import cloud.eppo.helpers.JavaBase64Codec; import cloud.eppo.helpers.TestUtils; import cloud.eppo.logging.Assignment; @@ -18,13 +19,9 @@ import cloud.eppo.ufc.dto.FlagConfig; import cloud.eppo.ufc.dto.VariationType; import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; import java.io.File; import java.io.IOException; -import java.util.Date; -import java.util.HashMap; -import java.util.Map; -import java.util.Timer; +import java.util.*; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -32,7 +29,6 @@ import java.util.stream.Stream; import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -52,8 +48,10 @@ public class BaseEppoClientTest { private static final String TEST_BASE_URL = TEST_API_CLOUD_FUNCTION_URL + (TEST_BRANCH != null ? "/b/" + TEST_BRANCH : "") + "/api"; - private final ObjectMapper mapper = - new ObjectMapper().registerModule(AssignmentTestCase.assignmentTestCaseModule()); + static { + Utils.setBase64Codec(new JavaBase64Codec()); + Utils.setJsonDeserializer(new JacksonJsonDeserializer()); + } private BaseEppoClient eppoClient; private AssignmentLogger mockAssignmentLogger; @@ -187,11 +185,6 @@ private void initClientWithAssignmentCache(IAssignmentCache cache) { log.info("Test client initialized"); } - @BeforeAll - public static void setUp() { - Utils.setBase64Codec(new JavaBase64Codec()); - } - @AfterEach public void cleanUp() { // TODO: Clear any caches @@ -249,22 +242,9 @@ public void testErrorGracefulModeOn() throws JsonProcessingException { assertEquals( "", spyClient.getStringAssignment("experiment1", "subject1", new Attributes(), "")); - assertEquals( - mapper.readTree("{\"a\": 1, \"b\": false}").toString(), - spyClient - .getJSONAssignment( - "subject1", "experiment1", mapper.readTree("{\"a\": 1, \"b\": false}")) - .toString()); - assertEquals( "{\"a\": 1, \"b\": false}", spyClient.getJSONStringAssignment("subject1", "experiment1", "{\"a\": 1, \"b\": false}")); - - assertEquals( - mapper.readTree("{}").toString(), - spyClient - .getJSONAssignment("subject1", "experiment1", new Attributes(), mapper.readTree("{}")) - .toString()); } @Test @@ -313,13 +293,8 @@ public void testErrorGracefulModeOff() { assertThrows( RuntimeException.class, () -> - spyClient.getJSONAssignment( - "subject1", "experiment1", mapper.readTree("{\"a\": 1, \"b\": false}"))); - assertThrows( - RuntimeException.class, - () -> - spyClient.getJSONAssignment( - "subject1", "experiment1", new Attributes(), mapper.readTree("{}"))); + spyClient.getJSONStringAssignment( + "subject1", "experiment1", "{\"a\": 1, \"b\": false}")); } @Test @@ -768,6 +743,7 @@ public void testGetConfigurationBeforeInitialization() { // Verify we get an empty configuration assertNotNull(config); assertTrue(config.isEmpty()); + assertEquals(Collections.emptySet(), config.getFlagKeys()); eppoClient.fetchAndActivateConfiguration(); @@ -777,6 +753,7 @@ public void testGetConfigurationBeforeInitialization() { // Verify we get an empty configuration assertNotNull(nextConfig); assertFalse(nextConfig.isEmpty()); + assertFalse(nextConfig.getFlagKeys().isEmpty()); } @SuppressWarnings("SameParameterValue") diff --git a/src/test/java/cloud/eppo/ConfigurationRequestorTest.java b/src/test/java/cloud/eppo/ConfigurationRequestorTest.java index 135ab2e9..b1902a4c 100644 --- a/src/test/java/cloud/eppo/ConfigurationRequestorTest.java +++ b/src/test/java/cloud/eppo/ConfigurationRequestorTest.java @@ -12,6 +12,7 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -52,6 +53,7 @@ public void testBrokenFetchDoesntClobberCache() throws IOException, InterruptedE // Set initial config and verify that it has been set. requestor.activateConfiguration(Configuration.builder(flagConfig.getBytes()).build()); assertFalse(configStore.getConfiguration().isEmpty()); + assertFalse(configStore.getConfiguration().getFlagKeys().isEmpty()); Mockito.verify(configStore, Mockito.times(1)).saveConfiguration(any()); CountDownLatch latch = new CountDownLatch(1); @@ -62,6 +64,7 @@ public void testBrokenFetchDoesntClobberCache() throws IOException, InterruptedE mockHttpClient.fail(new Exception("Intentional exception")); assertFalse(configStore.getConfiguration().isEmpty()); + assertFalse(configStore.getConfiguration().getFlagKeys().isEmpty()); Mockito.verify(configStore, Mockito.times(1)).saveConfiguration(any()); // `numeric_flag` is only in the cache which should be available @@ -84,6 +87,7 @@ public void testCacheWritesAfterBrokenFetch() throws IOException, InterruptedExc String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); Mockito.verify(configStore, Mockito.times(0)).saveConfiguration(any()); assertTrue(configStore.getConfiguration().isEmpty()); + assertEquals(Collections.emptySet(), configStore.getConfiguration().getFlagKeys()); CountDownLatch latch = new CountDownLatch(1); requestor.fetchAndSaveFromRemoteAsync( @@ -97,6 +101,7 @@ public void testCacheWritesAfterBrokenFetch() throws IOException, InterruptedExc Mockito.verify(configStore, Mockito.times(1)).saveConfiguration(any()); assertFalse(configStore.getConfiguration().isEmpty()); + assertFalse(configStore.getConfiguration().getFlagKeys().isEmpty()); // `numeric_flag` is only in the cache which should be available assertNotNull(configStore.getConfiguration().getFlag("numeric_flag")); diff --git a/src/test/java/cloud/eppo/UtilsTest.java b/src/test/java/cloud/eppo/UtilsTest.java index 63358df0..75c0f14f 100644 --- a/src/test/java/cloud/eppo/UtilsTest.java +++ b/src/test/java/cloud/eppo/UtilsTest.java @@ -1,6 +1,7 @@ package cloud.eppo; import static cloud.eppo.Utils.*; +import static cloud.eppo.helpers.JacksonJsonDeserializer.parseUtcISODateNode; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; diff --git a/src/test/java/cloud/eppo/api/ConfigurationBuilderTest.java b/src/test/java/cloud/eppo/api/ConfigurationBuilderTest.java index 34d1d222..96d6d218 100644 --- a/src/test/java/cloud/eppo/api/ConfigurationBuilderTest.java +++ b/src/test/java/cloud/eppo/api/ConfigurationBuilderTest.java @@ -3,10 +3,10 @@ import static cloud.eppo.Utils.getMD5Hex; import static org.junit.jupiter.api.Assertions.*; +import cloud.eppo.helpers.dto.adapters.EppoModule; import cloud.eppo.ufc.dto.FlagConfig; import cloud.eppo.ufc.dto.FlagConfigResponse; import cloud.eppo.ufc.dto.VariationType; -import cloud.eppo.ufc.dto.adapters.EppoModule; import com.fasterxml.jackson.databind.ObjectMapper; import java.io.IOException; import java.util.Collections; diff --git a/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java b/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java index f7250167..57e56e07 100644 --- a/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java +++ b/src/test/java/cloud/eppo/helpers/AssignmentTestCase.java @@ -5,6 +5,7 @@ import cloud.eppo.BaseEppoClient; import cloud.eppo.api.Attributes; import cloud.eppo.ufc.dto.VariationType; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.module.SimpleModule; @@ -122,10 +123,19 @@ public static void runTestCase(AssignmentTestCase testCase, BaseEppoClient eppoC assertAssignment(flagKey, subjectAssignment, stringAssignment); break; case JSON: - JsonNode jsonAssignment = - eppoClient.getJSONAssignment( - flagKey, subjectKey, subjectAttributes, testCase.getDefaultValue().jsonValue()); - assertAssignment(flagKey, subjectAssignment, jsonAssignment); + try { + JsonNode jsonAssignment = + mapper.readTree( + eppoClient.getJSONStringAssignment( + flagKey, + subjectKey, + subjectAttributes, + testCase.getDefaultValue().stringValue())); + assertAssignment(flagKey, subjectAssignment, jsonAssignment); + + } catch (JsonProcessingException ex) { + throw new RuntimeException(ex); + } break; default: throw new UnsupportedOperationException( diff --git a/src/test/java/cloud/eppo/helpers/AssignmentTestCaseDeserializer.java b/src/test/java/cloud/eppo/helpers/AssignmentTestCaseDeserializer.java index 76ae6cc2..adfcac6d 100644 --- a/src/test/java/cloud/eppo/helpers/AssignmentTestCaseDeserializer.java +++ b/src/test/java/cloud/eppo/helpers/AssignmentTestCaseDeserializer.java @@ -2,8 +2,8 @@ import cloud.eppo.api.Attributes; import cloud.eppo.api.EppoValue; +import cloud.eppo.helpers.dto.adapters.EppoValueDeserializer; import cloud.eppo.ufc.dto.VariationType; -import cloud.eppo.ufc.dto.adapters.EppoValueDeserializer; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.JsonNode; diff --git a/src/test/java/cloud/eppo/helpers/BanditTestCaseDeserializer.java b/src/test/java/cloud/eppo/helpers/BanditTestCaseDeserializer.java index 679cb887..d20112ae 100644 --- a/src/test/java/cloud/eppo/helpers/BanditTestCaseDeserializer.java +++ b/src/test/java/cloud/eppo/helpers/BanditTestCaseDeserializer.java @@ -1,7 +1,7 @@ package cloud.eppo.helpers; import cloud.eppo.api.*; -import cloud.eppo.ufc.dto.adapters.EppoValueDeserializer; +import cloud.eppo.helpers.dto.adapters.EppoValueDeserializer; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.databind.DeserializationContext; import com.fasterxml.jackson.databind.JsonNode; diff --git a/src/test/java/cloud/eppo/helpers/JacksonJsonDeserializer.java b/src/test/java/cloud/eppo/helpers/JacksonJsonDeserializer.java new file mode 100644 index 00000000..a709ee88 --- /dev/null +++ b/src/test/java/cloud/eppo/helpers/JacksonJsonDeserializer.java @@ -0,0 +1,92 @@ +package cloud.eppo.helpers; + +import cloud.eppo.Utils; +import cloud.eppo.api.EppoValue; +import cloud.eppo.exception.JsonParsingException; +import cloud.eppo.helpers.dto.adapters.EppoModule; +import cloud.eppo.ufc.dto.BanditParametersResponse; +import cloud.eppo.ufc.dto.FlagConfigResponse; +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ObjectNode; +import java.io.IOException; +import java.util.Date; +import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class JacksonJsonDeserializer implements Utils.JsonDeserializer { + + private static final Logger log = LoggerFactory.getLogger(JacksonJsonDeserializer.class); + private final ObjectMapper mapper = new ObjectMapper().registerModule(EppoModule.eppoModule()); + + @Override + public FlagConfigResponse parseFlagConfigResponse(byte[] jsonString) throws JsonParsingException { + try { + return mapper.readValue(jsonString, FlagConfigResponse.class); + } catch (IOException e) { + throw new JsonParsingException(e); + } + } + + @Override + public BanditParametersResponse parseBanditParametersResponse(byte[] jsonString) + throws JsonParsingException { + try { + return mapper.readValue(jsonString, BanditParametersResponse.class); + } catch (IOException e) { + throw new JsonParsingException(e); + } + } + + @Override + public boolean isValidJson(String json) { + try { + return mapper.readTree(json) != null; + } catch (JsonProcessingException e) { + return false; + } + } + + @Override + public String serializeAttributesToJSONString(Map map, boolean omitNulls) { + ObjectMapper mapper = new ObjectMapper(); + ObjectNode result = mapper.createObjectNode(); + + for (Map.Entry entry : map.entrySet()) { + String attributeName = entry.getKey(); + EppoValue attributeValue = entry.getValue(); + + if (attributeValue == null || attributeValue.isNull()) { + if (!omitNulls) { + result.putNull(attributeName); + } + } else { + if (attributeValue.isNumeric()) { + result.put(attributeName, attributeValue.doubleValue()); + continue; + } + if (attributeValue.isBoolean()) { + result.put(attributeName, attributeValue.booleanValue()); + continue; + } + // fall back put treating any other eppo values as a string + result.put(attributeName, attributeValue.toString()); + } + } + try { + return mapper.writeValueAsString(result); + } catch (JsonProcessingException e) { + throw new RuntimeException(e); + } + } + + public static Date parseUtcISODateNode(JsonNode isoDateStringElement) { + if (isoDateStringElement == null || isoDateStringElement.isNull()) { + return null; + } + String isoDateString = isoDateStringElement.asText(); + return Utils.parseUtcISODateString(isoDateString); + } +} diff --git a/src/test/java/cloud/eppo/helpers/TestUtils.java b/src/test/java/cloud/eppo/helpers/TestUtils.java index 5290500f..b7f5943b 100644 --- a/src/test/java/cloud/eppo/helpers/TestUtils.java +++ b/src/test/java/cloud/eppo/helpers/TestUtils.java @@ -2,10 +2,15 @@ import cloud.eppo.BaseEppoClient; import cloud.eppo.IEppoHttpClient; +import cloud.eppo.Utils; import java.lang.reflect.Field; public class TestUtils { + static { + Utils.setJsonDeserializer(new JacksonJsonDeserializer()); + } + @SuppressWarnings("SameParameterValue") public static MockHttpClient mockHttpResponse(String responseBody) { MockHttpClient mockHttpClient = new MockHttpClient(responseBody.getBytes()); diff --git a/src/main/java/cloud/eppo/ufc/dto/adapters/BanditParametersResponseDeserializer.java b/src/test/java/cloud/eppo/helpers/dto/adapters/BanditParametersResponseDeserializer.java similarity index 99% rename from src/main/java/cloud/eppo/ufc/dto/adapters/BanditParametersResponseDeserializer.java rename to src/test/java/cloud/eppo/helpers/dto/adapters/BanditParametersResponseDeserializer.java index 6913c387..1b2be38c 100644 --- a/src/main/java/cloud/eppo/ufc/dto/adapters/BanditParametersResponseDeserializer.java +++ b/src/test/java/cloud/eppo/helpers/dto/adapters/BanditParametersResponseDeserializer.java @@ -1,4 +1,4 @@ -package cloud.eppo.ufc.dto.adapters; +package cloud.eppo.helpers.dto.adapters; import cloud.eppo.ufc.dto.*; import com.fasterxml.jackson.core.JsonParser; diff --git a/src/main/java/cloud/eppo/ufc/dto/adapters/DateSerializer.java b/src/test/java/cloud/eppo/helpers/dto/adapters/DateSerializer.java similarity index 94% rename from src/main/java/cloud/eppo/ufc/dto/adapters/DateSerializer.java rename to src/test/java/cloud/eppo/helpers/dto/adapters/DateSerializer.java index 2ea23176..1f1d52ed 100644 --- a/src/main/java/cloud/eppo/ufc/dto/adapters/DateSerializer.java +++ b/src/test/java/cloud/eppo/helpers/dto/adapters/DateSerializer.java @@ -1,4 +1,4 @@ -package cloud.eppo.ufc.dto.adapters; +package cloud.eppo.helpers.dto.adapters; import static cloud.eppo.Utils.getISODate; diff --git a/src/main/java/cloud/eppo/ufc/dto/adapters/EppoModule.java b/src/test/java/cloud/eppo/helpers/dto/adapters/EppoModule.java similarity index 95% rename from src/main/java/cloud/eppo/ufc/dto/adapters/EppoModule.java rename to src/test/java/cloud/eppo/helpers/dto/adapters/EppoModule.java index 7066377c..77f29ca8 100644 --- a/src/main/java/cloud/eppo/ufc/dto/adapters/EppoModule.java +++ b/src/test/java/cloud/eppo/helpers/dto/adapters/EppoModule.java @@ -1,4 +1,4 @@ -package cloud.eppo.ufc.dto.adapters; +package cloud.eppo.helpers.dto.adapters; import cloud.eppo.api.EppoValue; import cloud.eppo.ufc.dto.BanditParametersResponse; diff --git a/src/main/java/cloud/eppo/ufc/dto/adapters/EppoValueDeserializer.java b/src/test/java/cloud/eppo/helpers/dto/adapters/EppoValueDeserializer.java similarity index 97% rename from src/main/java/cloud/eppo/ufc/dto/adapters/EppoValueDeserializer.java rename to src/test/java/cloud/eppo/helpers/dto/adapters/EppoValueDeserializer.java index 09ec5b36..e835e72b 100644 --- a/src/main/java/cloud/eppo/ufc/dto/adapters/EppoValueDeserializer.java +++ b/src/test/java/cloud/eppo/helpers/dto/adapters/EppoValueDeserializer.java @@ -1,4 +1,4 @@ -package cloud.eppo.ufc.dto.adapters; +package cloud.eppo.helpers.dto.adapters; import cloud.eppo.api.EppoValue; import com.fasterxml.jackson.core.JsonParser; diff --git a/src/main/java/cloud/eppo/ufc/dto/adapters/EppoValueSerializer.java b/src/test/java/cloud/eppo/helpers/dto/adapters/EppoValueSerializer.java similarity index 95% rename from src/main/java/cloud/eppo/ufc/dto/adapters/EppoValueSerializer.java rename to src/test/java/cloud/eppo/helpers/dto/adapters/EppoValueSerializer.java index 8bd10bd6..6545146e 100644 --- a/src/main/java/cloud/eppo/ufc/dto/adapters/EppoValueSerializer.java +++ b/src/test/java/cloud/eppo/helpers/dto/adapters/EppoValueSerializer.java @@ -1,4 +1,4 @@ -package cloud.eppo.ufc.dto.adapters; +package cloud.eppo.helpers.dto.adapters; import cloud.eppo.api.EppoValue; import com.fasterxml.jackson.core.JsonGenerator; diff --git a/src/main/java/cloud/eppo/ufc/dto/adapters/FlagConfigResponseDeserializer.java b/src/test/java/cloud/eppo/helpers/dto/adapters/FlagConfigResponseDeserializer.java similarity index 98% rename from src/main/java/cloud/eppo/ufc/dto/adapters/FlagConfigResponseDeserializer.java rename to src/test/java/cloud/eppo/helpers/dto/adapters/FlagConfigResponseDeserializer.java index 7c48a50f..d88c2857 100644 --- a/src/main/java/cloud/eppo/ufc/dto/adapters/FlagConfigResponseDeserializer.java +++ b/src/test/java/cloud/eppo/helpers/dto/adapters/FlagConfigResponseDeserializer.java @@ -1,6 +1,6 @@ -package cloud.eppo.ufc.dto.adapters; +package cloud.eppo.helpers.dto.adapters; -import static cloud.eppo.Utils.parseUtcISODateNode; +import static cloud.eppo.helpers.JacksonJsonDeserializer.parseUtcISODateNode; import cloud.eppo.api.EppoValue; import cloud.eppo.model.ShardRange; diff --git a/src/test/java/cloud/eppo/ufc/deserializer/BanditParametersResponseDeserializerTest.java b/src/test/java/cloud/eppo/ufc/deserializer/BanditParametersResponseDeserializerTest.java index 1891923b..f30a0b49 100644 --- a/src/test/java/cloud/eppo/ufc/deserializer/BanditParametersResponseDeserializerTest.java +++ b/src/test/java/cloud/eppo/ufc/deserializer/BanditParametersResponseDeserializerTest.java @@ -2,8 +2,8 @@ import static org.junit.jupiter.api.Assertions.assertEquals; +import cloud.eppo.helpers.dto.adapters.EppoModule; import cloud.eppo.ufc.dto.*; -import cloud.eppo.ufc.dto.adapters.EppoModule; import com.fasterxml.jackson.databind.ObjectMapper; import java.io.File; import java.io.IOException; diff --git a/src/test/java/cloud/eppo/ufc/deserializer/FlagConfigResponseDeserializerTest.java b/src/test/java/cloud/eppo/ufc/deserializer/FlagConfigResponseDeserializerTest.java index 24105543..193d928f 100644 --- a/src/test/java/cloud/eppo/ufc/deserializer/FlagConfigResponseDeserializerTest.java +++ b/src/test/java/cloud/eppo/ufc/deserializer/FlagConfigResponseDeserializerTest.java @@ -2,9 +2,9 @@ import static org.junit.jupiter.api.Assertions.*; +import cloud.eppo.helpers.dto.adapters.EppoModule; import cloud.eppo.model.ShardRange; import cloud.eppo.ufc.dto.*; -import cloud.eppo.ufc.dto.adapters.EppoModule; import com.fasterxml.jackson.databind.ObjectMapper; import java.io.File; import java.io.FileReader;