From 5d34a4425653807abf2d1e0a5bf5342a1596aeb5 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Tue, 26 Nov 2024 11:23:49 -0700 Subject: [PATCH 1/7] chore:update to the latest common package --- Makefile | 2 +- build.gradle | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index f5041c8..531cee6 100644 --- a/Makefile +++ b/Makefile @@ -40,7 +40,7 @@ test-data: mkdir -p ${tempDir} git clone -b ${branchName} --depth 1 --single-branch ${githubRepoLink} ${gitDataDir} cp -r ${gitDataDir}/ufc ${testDataDir} - rm ${testDataDir}/ufc/bandit-tests/*.dynamic-typing.json + rm -f ${testDataDir}/ufc/bandit-tests/*.dynamic-typing.json rm -rf ${tempDir} .PHONY: test diff --git a/build.gradle b/build.gradle index 0f1a40a..7db117b 100644 --- a/build.gradle +++ b/build.gradle @@ -11,7 +11,7 @@ java { } group = 'cloud.eppo' -version = '4.0.0-SNAPSHOT' +version = '4.0.1' ext.isReleaseVersion = !version.endsWith("SNAPSHOT") import org.apache.tools.ant.filters.ReplaceTokens @@ -30,7 +30,7 @@ repositories { } dependencies { - api 'cloud.eppo:sdk-common-jvm:3.5.0' + api 'cloud.eppo:sdk-common-jvm:3.5.4' implementation 'com.github.zafarkhaja:java-semver:0.10.2' implementation 'com.fasterxml.jackson.core:jackson-databind:2.17.1' From feb8bc0f8b79951c74a177d76945e229a723132b Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Tue, 26 Nov 2024 11:30:50 -0700 Subject: [PATCH 2/7] fix: respect graceful mode on init --- src/main/java/cloud/eppo/EppoClient.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main/java/cloud/eppo/EppoClient.java b/src/main/java/cloud/eppo/EppoClient.java index f0a3b26..68edc2f 100644 --- a/src/main/java/cloud/eppo/EppoClient.java +++ b/src/main/java/cloud/eppo/EppoClient.java @@ -197,8 +197,15 @@ public EppoClient buildAndInit() { pollingIntervalMs, pollingIntervalMs / DEFAULT_JITTER_INTERVAL_RATIO); - // Kick off the first fetch - fetchConfigurationsTask.run(); + // Kick off the first fetch, respecting graceful mode. + try { + fetchConfigurationsTask.run(); + } catch (RuntimeException e) { + log.error("Encountered Exception while loading configuration", e); + if (!isGracefulMode) { + throw e; + } + } return instance; } From 794bb44b534d106c7a8a202cb6fd2731fe0aedb3 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Tue, 26 Nov 2024 11:53:21 -0700 Subject: [PATCH 3/7] tests for respecting graceful mode --- src/test/java/cloud/eppo/EppoClientTest.java | 93 ++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/src/test/java/cloud/eppo/EppoClientTest.java b/src/test/java/cloud/eppo/EppoClientTest.java index 582f7fa..2211f46 100644 --- a/src/test/java/cloud/eppo/EppoClientTest.java +++ b/src/test/java/cloud/eppo/EppoClientTest.java @@ -4,6 +4,7 @@ import static cloud.eppo.helpers.AssignmentTestCase.runTestCase; import static cloud.eppo.helpers.BanditTestCase.parseBanditTestCaseFile; import static cloud.eppo.helpers.BanditTestCase.runBanditTestCase; +import static cloud.eppo.helpers.TestUtils.mockHttpError; import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.*; @@ -23,6 +24,7 @@ import com.github.tomakehurst.wiremock.junit5.WireMockExtension; import java.io.File; import java.lang.reflect.Field; +import java.util.concurrent.CompletableFuture; import java.util.stream.Stream; import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.AfterAll; @@ -238,6 +240,71 @@ public void testPolling() { verify(httpClientSpy, times(2)).get(anyString()); } + @Test + public void testGracefulInitializationFailure() { + // Set up bad HTTP response + mockHttpError(); + + // Initialize and no exception should be thrown. + assertDoesNotThrow(() -> initFailingGracefulClient(true)); + } + + @Test + public void testClientMakesDefaultAssignmentsAfterFailingToInitialize() { + // Set up bad HTTP response + mockHttpError(); + + // Initialize and no exception should be thrown. + try { + EppoClient eppoClient = initFailingGracefulClient(true); + assertEquals("default", eppoClient.getStringAssignment("experiment1", "subject1", "default")); + } catch (Exception e) { + fail("Unexpected exception: " + e); + } + } + + @Test + public void testClientMakesDefaultAssignmentsAfterFailingToInitializeNonGracefulMode() { + // Set up bad HTTP response + mockHttpError(); + + // Initialize and the exception should be thrown. + try { + initFailingGracefulClient(false); + } catch (RuntimeException e) { + // Expected + assertEquals("Intentional Error", e.getMessage()); + } finally { + assertEquals( + "default", + EppoClient.getInstance().getStringAssignment("experiment1", "subject1", "default")); + } + } + + @Test + public void testNonGracefulInitializationFailure() { + // Set up bad HTTP response + mockHttpError(); + + // Initialize and assert exception thrown + assertThrows(Exception.class, () -> initFailingGracefulClient(false)); + } + + 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); + } + @SuppressWarnings("SameParameterValue") private void sleepUninterruptedly(long sleepMs) { try { @@ -261,6 +328,20 @@ private EppoClient initClient(String apiKey) { .buildAndInit(); } + private EppoClient initFailingGracefulClient(boolean isGracefulMode) { + mockAssignmentLogger = mock(AssignmentLogger.class); + mockBanditLogger = mock(BanditLogger.class); + + return new EppoClient.Builder() + .apiKey(DUMMY_FLAG_API_KEY) + .host("blag") + .assignmentLogger(mockAssignmentLogger) + .banditLogger(mockBanditLogger) + .isGracefulMode(isGracefulMode) + .forceReinitialize(true) // Useful for tests + .buildAndInit(); + } + private void uninitClient() { try { Field httpClientOverrideField = EppoClient.class.getDeclaredField("instance"); @@ -281,4 +362,16 @@ private void initBuggyClient() { throw new RuntimeException(e); } } + + public static void setBaseClientHttpClientOverrideField(EppoHttpClient httpClient) { + // Uses reflection to set a static override field used for tests (e.g., httpClientOverride) + try { + Field httpClientOverrideField = BaseEppoClient.class.getDeclaredField("httpClientOverride"); + httpClientOverrideField.setAccessible(true); + httpClientOverrideField.set(null, httpClient); + httpClientOverrideField.setAccessible(false); + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new RuntimeException(e); + } + } } From a5fea3835481cc001f61a17ca63ebad6bd29ae46 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Tue, 26 Nov 2024 11:55:09 -0700 Subject: [PATCH 4/7] make init blocking on load config --- src/main/java/cloud/eppo/EppoClient.java | 4 +++- src/main/java/cloud/eppo/FetchConfigurationsTask.java | 4 ++++ src/test/java/cloud/eppo/EppoClientTest.java | 1 - 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/main/java/cloud/eppo/EppoClient.java b/src/main/java/cloud/eppo/EppoClient.java index 68edc2f..1443558 100644 --- a/src/main/java/cloud/eppo/EppoClient.java +++ b/src/main/java/cloud/eppo/EppoClient.java @@ -197,9 +197,11 @@ public EppoClient buildAndInit() { pollingIntervalMs, pollingIntervalMs / DEFAULT_JITTER_INTERVAL_RATIO); + fetchConfigurationsTask.scheduleNext(); + // Kick off the first fetch, respecting graceful mode. try { - fetchConfigurationsTask.run(); + instance.loadConfiguration(); } catch (RuntimeException e) { log.error("Encountered Exception while loading configuration", e); if (!isGracefulMode) { diff --git a/src/main/java/cloud/eppo/FetchConfigurationsTask.java b/src/main/java/cloud/eppo/FetchConfigurationsTask.java index c84bcfc..0f5ad6a 100644 --- a/src/main/java/cloud/eppo/FetchConfigurationsTask.java +++ b/src/main/java/cloud/eppo/FetchConfigurationsTask.java @@ -28,6 +28,10 @@ public void run() { } catch (Exception e) { log.error("[Eppo SDK] Error fetching experiment configuration", e); } + scheduleNext(); + } + + public void scheduleNext() { long delay = this.intervalInMillis - (long) (Math.random() * this.jitterInMillis); FetchConfigurationsTask nextTask = diff --git a/src/test/java/cloud/eppo/EppoClientTest.java b/src/test/java/cloud/eppo/EppoClientTest.java index 2211f46..0f2b470 100644 --- a/src/test/java/cloud/eppo/EppoClientTest.java +++ b/src/test/java/cloud/eppo/EppoClientTest.java @@ -4,7 +4,6 @@ import static cloud.eppo.helpers.AssignmentTestCase.runTestCase; import static cloud.eppo.helpers.BanditTestCase.parseBanditTestCaseFile; import static cloud.eppo.helpers.BanditTestCase.runBanditTestCase; -import static cloud.eppo.helpers.TestUtils.mockHttpError; import static org.junit.jupiter.api.Assertions.*; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.*; From 5ec2dec827b977933c47ae78dcadd2955af94974 Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Tue, 26 Nov 2024 12:03:33 -0700 Subject: [PATCH 5/7] bump version in readme --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f175728..8cccf6a 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ ```groovy dependencies { - implementation 'cloud.eppo:eppo-server-sdk:3.1.0' + implementation 'cloud.eppo:eppo-server-sdk:4.0.1' } ``` From e1560d69f1adfd233c96a04375bf117c8a6decbc Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Tue, 26 Nov 2024 12:06:24 -0700 Subject: [PATCH 6/7] additional test assertion --- src/test/java/cloud/eppo/EppoClientTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/java/cloud/eppo/EppoClientTest.java b/src/test/java/cloud/eppo/EppoClientTest.java index 0f2b470..3b75b97 100644 --- a/src/test/java/cloud/eppo/EppoClientTest.java +++ b/src/test/java/cloud/eppo/EppoClientTest.java @@ -270,6 +270,7 @@ public void testClientMakesDefaultAssignmentsAfterFailingToInitializeNonGraceful // Initialize and the exception should be thrown. try { initFailingGracefulClient(false); + fail("Exception should have been thrown"); } catch (RuntimeException e) { // Expected assertEquals("Intentional Error", e.getMessage()); From 6004c70186a3375ee0c7c8a872efcf7b0cc7d7ac Mon Sep 17 00:00:00 2001 From: Ty Potter Date: Thu, 28 Nov 2024 09:57:45 -0700 Subject: [PATCH 7/7] don't block on init --- src/main/java/cloud/eppo/EppoClient.java | 15 ++----- .../cloud/eppo/FetchConfigurationsTask.java | 4 -- src/test/java/cloud/eppo/EppoClientTest.java | 39 ++----------------- 3 files changed, 7 insertions(+), 51 deletions(-) diff --git a/src/main/java/cloud/eppo/EppoClient.java b/src/main/java/cloud/eppo/EppoClient.java index 1443558..e26cc25 100644 --- a/src/main/java/cloud/eppo/EppoClient.java +++ b/src/main/java/cloud/eppo/EppoClient.java @@ -197,17 +197,10 @@ public EppoClient buildAndInit() { pollingIntervalMs, pollingIntervalMs / DEFAULT_JITTER_INTERVAL_RATIO); - fetchConfigurationsTask.scheduleNext(); - - // Kick off the first fetch, respecting graceful mode. - try { - instance.loadConfiguration(); - } catch (RuntimeException e) { - log.error("Encountered Exception while loading configuration", e); - if (!isGracefulMode) { - throw e; - } - } + // Kick off the first fetch + // Graceful mode is implicit here because `FetchConfigurationsTask` catches and logs errors + // without rethrowing. + fetchConfigurationsTask.run(); return instance; } diff --git a/src/main/java/cloud/eppo/FetchConfigurationsTask.java b/src/main/java/cloud/eppo/FetchConfigurationsTask.java index 0f5ad6a..c84bcfc 100644 --- a/src/main/java/cloud/eppo/FetchConfigurationsTask.java +++ b/src/main/java/cloud/eppo/FetchConfigurationsTask.java @@ -28,10 +28,6 @@ public void run() { } catch (Exception e) { log.error("[Eppo SDK] Error fetching experiment configuration", e); } - scheduleNext(); - } - - public void scheduleNext() { long delay = this.intervalInMillis - (long) (Math.random() * this.jitterInMillis); FetchConfigurationsTask nextTask = diff --git a/src/test/java/cloud/eppo/EppoClientTest.java b/src/test/java/cloud/eppo/EppoClientTest.java index 3b75b97..924bd86 100644 --- a/src/test/java/cloud/eppo/EppoClientTest.java +++ b/src/test/java/cloud/eppo/EppoClientTest.java @@ -239,14 +239,8 @@ public void testPolling() { verify(httpClientSpy, times(2)).get(anyString()); } - @Test - public void testGracefulInitializationFailure() { - // Set up bad HTTP response - mockHttpError(); - - // Initialize and no exception should be thrown. - assertDoesNotThrow(() -> initFailingGracefulClient(true)); - } + // NOTE: Graceful mode during init is intrinsically true since the call is non-blocking and + // exceptions are caught without rethrowing in `FetchConfigurationsTask` @Test public void testClientMakesDefaultAssignmentsAfterFailingToInitialize() { @@ -256,40 +250,13 @@ public void testClientMakesDefaultAssignmentsAfterFailingToInitialize() { // Initialize and no exception should be thrown. try { EppoClient eppoClient = initFailingGracefulClient(true); + Thread.sleep(25); // Sleep to allow the async config fetch call to happen (and fail) assertEquals("default", eppoClient.getStringAssignment("experiment1", "subject1", "default")); } catch (Exception e) { fail("Unexpected exception: " + e); } } - @Test - public void testClientMakesDefaultAssignmentsAfterFailingToInitializeNonGracefulMode() { - // Set up bad HTTP response - mockHttpError(); - - // Initialize and the exception should be thrown. - try { - initFailingGracefulClient(false); - fail("Exception should have been thrown"); - } catch (RuntimeException e) { - // Expected - assertEquals("Intentional Error", e.getMessage()); - } finally { - assertEquals( - "default", - EppoClient.getInstance().getStringAssignment("experiment1", "subject1", "default")); - } - } - - @Test - public void testNonGracefulInitializationFailure() { - // Set up bad HTTP response - mockHttpError(); - - // Initialize and assert exception thrown - assertThrows(Exception.class, () -> initFailingGracefulClient(false)); - } - public static void mockHttpError() { // Create a mock instance of EppoHttpClient EppoHttpClient mockHttpClient = mock(EppoHttpClient.class);