diff --git a/build.gradle b/build.gradle index 6cfcecb4..bf11c01b 100644 --- a/build.gradle +++ b/build.gradle @@ -6,7 +6,7 @@ plugins { } group = 'cloud.eppo' -version = '3.13.2-SNAPSHOT' +version = '3.14.0' ext.isReleaseVersion = !version.endsWith("SNAPSHOT") java { diff --git a/src/main/java/cloud/eppo/ConfigurationRequestor.java b/src/main/java/cloud/eppo/ConfigurationRequestor.java index f20f8aa8..002efe0c 100644 --- a/src/main/java/cloud/eppo/ConfigurationRequestor.java +++ b/src/main/java/cloud/eppo/ConfigurationRequestor.java @@ -88,14 +88,24 @@ void fetchAndSaveFromRemote() { // Reuse the `lastConfig` as its bandits may be useful Configuration lastConfig = configurationStore.getConfiguration(); - byte[] flagConfigurationJsonBytes = client.get(Constants.FLAG_CONFIG_ENDPOINT); + EppoHttpResponse flagsResponse = client.get(Constants.FLAG_CONFIG_ENDPOINT); + + // Handle 304 Not Modified + if (flagsResponse.isNotModified()) { + // No update needed - existing config is still valid + return; + } + Configuration.Builder configBuilder = - Configuration.builder(flagConfigurationJsonBytes, expectObfuscatedConfig) - .banditParametersFromConfig(lastConfig); + Configuration.builder(flagsResponse.getBody()) + .banditParametersFromConfig(lastConfig) + .flagsETag(flagsResponse.getETag()); if (supportBandits && configBuilder.requiresUpdatedBanditModels()) { - byte[] banditParametersJsonBytes = client.get(Constants.BANDIT_ENDPOINT); - configBuilder.banditParameters(banditParametersJsonBytes); + EppoHttpResponse banditsResponse = client.get(Constants.BANDIT_ENDPOINT); + if (banditsResponse.isSuccessful()) { + configBuilder.banditParameters(banditsResponse.getBody()); + } } saveConfigurationAndNotify(configBuilder.build()).join(); @@ -105,6 +115,7 @@ void fetchAndSaveFromRemote() { CompletableFuture fetchAndSaveFromRemoteAsync() { log.debug("Fetching configuration from API server"); final Configuration lastConfig = configurationStore.getConfiguration(); + final String existingETag = lastConfig.getFlagsETag(); if (remoteFetchFuture != null && !remoteFetchFuture.isDone()) { log.debug("Remote fetch is active. Cancelling and restarting"); @@ -114,26 +125,33 @@ CompletableFuture fetchAndSaveFromRemoteAsync() { remoteFetchFuture = client - .getAsync(Constants.FLAG_CONFIG_ENDPOINT) + .getAsync(Constants.FLAG_CONFIG_ENDPOINT, existingETag) .thenCompose( - flagConfigJsonBytes -> { + flagsResponse -> { + // Handle 304 Not Modified + if (flagsResponse.isNotModified()) { + // No update needed - existing config is still valid + return CompletableFuture.completedFuture(null); + } + synchronized (this) { Configuration.Builder configBuilder = - Configuration.builder(flagConfigJsonBytes, expectObfuscatedConfig) + Configuration.builder(flagsResponse.getBody()) .banditParametersFromConfig( - lastConfig); // possibly reuse last bandit models loaded. + lastConfig) // possibly reuse last bandit models loaded. + .flagsETag(flagsResponse.getETag()); if (supportBandits && configBuilder.requiresUpdatedBanditModels()) { - byte[] banditParametersJsonBytes; + EppoHttpResponse banditParametersResponse; try { - banditParametersJsonBytes = - client.getAsync(Constants.BANDIT_ENDPOINT).get(); + banditParametersResponse = 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); + if (banditParametersResponse != null + && banditParametersResponse.isSuccessful()) { + configBuilder.banditParameters(banditParametersResponse.getBody()); } } diff --git a/src/main/java/cloud/eppo/EppoHttpClient.java b/src/main/java/cloud/eppo/EppoHttpClient.java index 656e844f..fb0ca3ef 100644 --- a/src/main/java/cloud/eppo/EppoHttpClient.java +++ b/src/main/java/cloud/eppo/EppoHttpClient.java @@ -13,6 +13,7 @@ import okhttp3.Request; import okhttp3.Response; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -44,44 +45,61 @@ private static OkHttpClient buildOkHttpClient() { return builder.build(); } - public byte[] get(String path) { + public EppoHttpResponse get(String path) { try { // Wait and return the async get. - return getAsync(path).get(); + return getAsync(path, null).get(); } catch (InterruptedException | ExecutionException e) { log.error("Config fetch interrupted", e); throw new RuntimeException(e); } } - public CompletableFuture getAsync(String path) { - CompletableFuture future = new CompletableFuture<>(); - Request request = buildRequest(path); + public CompletableFuture getAsync(String path) { + return getAsync(path, null); + } + + public CompletableFuture getAsync(String path, @Nullable String ifNoneMatch) { + CompletableFuture future = new CompletableFuture<>(); + Request request = buildRequest(path, ifNoneMatch); + client .newCall(request) .enqueue( new Callback() { @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()); - } catch (IOException ex) { - future.completeExceptionally( - new RuntimeException( - "Failed to read response from URL {}" + request.url(), ex)); + try { + int statusCode = response.code(); + String eTag = response.header("ETag"); + + // Handle 304 Not Modified + if (statusCode == HttpURLConnection.HTTP_NOT_MODIFIED) { + future.complete(new EppoHttpResponse(new byte[0], statusCode, eTag)); + return; } - } else { - if (response.code() == HttpURLConnection.HTTP_FORBIDDEN) { + + // Handle 2xx success + if (response.isSuccessful() && response.body() != null) { + log.debug("Fetch successful"); + try { + byte[] bytes = response.body().bytes(); + future.complete(new EppoHttpResponse(bytes, statusCode, eTag)); + } catch (IOException ex) { + future.completeExceptionally( + new RuntimeException( + "Failed to read response from URL " + request.url(), ex)); + } + } else if (statusCode == HttpURLConnection.HTTP_FORBIDDEN) { future.completeExceptionally(new RuntimeException("Invalid API key")); } else { - log.debug("Fetch failed with status code: {}", response.code()); + log.debug("Fetch failed with status code: {}", statusCode); future.completeExceptionally( new RuntimeException("Bad response from URL " + request.url())); } + } finally { + response.close(); } - response.close(); } @Override @@ -98,7 +116,7 @@ public void onFailure(@NotNull Call call, @NotNull IOException e) { return future; } - private Request buildRequest(String path) { + private Request buildRequest(String path, @Nullable String ifNoneMatch) { HttpUrl httpUrl = HttpUrl.parse(baseUrl + path) .newBuilder() @@ -107,6 +125,13 @@ private Request buildRequest(String path) { .addQueryParameter("sdkVersion", sdkVersion) .build(); - return new Request.Builder().url(httpUrl).build(); + Request.Builder requestBuilder = new Request.Builder().url(httpUrl); + + // Add If-None-Match header if eTag provided + if (ifNoneMatch != null && !ifNoneMatch.isEmpty()) { + requestBuilder.header("If-None-Match", ifNoneMatch); + } + + return requestBuilder.build(); } } diff --git a/src/main/java/cloud/eppo/EppoHttpResponse.java b/src/main/java/cloud/eppo/EppoHttpResponse.java new file mode 100644 index 00000000..4c217d71 --- /dev/null +++ b/src/main/java/cloud/eppo/EppoHttpResponse.java @@ -0,0 +1,44 @@ +package cloud.eppo; + +import org.jetbrains.annotations.Nullable; + +/** + * HTTP response wrapper containing status code, body, and optional ETag header. Used to support + * conditional requests via If-None-Match/ETag headers. + */ +public class EppoHttpResponse { + private final byte[] body; + private final int statusCode; + @Nullable private final String eTag; + + public EppoHttpResponse(byte[] body, int statusCode, @Nullable String eTag) { + this.body = body; + this.statusCode = statusCode; + this.eTag = eTag; + } + + /** Get response body bytes. Empty for 304 responses. */ + public byte[] getBody() { + return body; + } + + /** Get HTTP status code. */ + public int getStatusCode() { + return statusCode; + } + + /** Get ETag header value if present. */ + @Nullable public String getETag() { + return eTag; + } + + /** Returns true if status code is 304 Not Modified. */ + public boolean isNotModified() { + return statusCode == 304; + } + + /** Returns true if status code is 2xx success. */ + public boolean isSuccessful() { + return statusCode >= 200 && statusCode < 300; + } +} diff --git a/src/main/java/cloud/eppo/api/Configuration.java b/src/main/java/cloud/eppo/api/Configuration.java index 8634a3ac..e12a1267 100644 --- a/src/main/java/cloud/eppo/api/Configuration.java +++ b/src/main/java/cloud/eppo/api/Configuration.java @@ -68,6 +68,8 @@ public class Configuration { private final byte[] banditParamsJson; + @Nullable private final String flagsETag; + /** Default visibility for tests. */ Configuration( Map flags, @@ -75,7 +77,8 @@ public class Configuration { Map bandits, boolean isConfigObfuscated, byte[] flagConfigJson, - byte[] banditParamsJson) { + byte[] banditParamsJson, + @Nullable String flagsETag) { this.flags = flags; this.banditReferences = banditReferences; this.bandits = bandits; @@ -97,6 +100,7 @@ public class Configuration { } this.flagConfigJson = flagConfigJson; this.banditParamsJson = banditParamsJson; + this.flagsETag = flagsETag; } public static Configuration emptyConfig() { @@ -106,19 +110,29 @@ public static Configuration emptyConfig() { Collections.emptyMap(), false, emptyFlagsBytes, + null, null); } @Override public String toString() { - return "Configuration{" + - "banditReferences=" + banditReferences + - ", flags=" + flags + - ", bandits=" + bandits + - ", isConfigObfuscated=" + isConfigObfuscated + - ", flagConfigJson=" + Arrays.toString(flagConfigJson) + - ", banditParamsJson=" + Arrays.toString(banditParamsJson) + - '}'; + return "Configuration{" + + "banditReferences=" + + banditReferences + + ", flags=" + + flags + + ", bandits=" + + bandits + + ", isConfigObfuscated=" + + isConfigObfuscated + + ", flagConfigJson=" + + Arrays.toString(flagConfigJson) + + ", banditParamsJson=" + + Arrays.toString(banditParamsJson) + + ", flagsETag='" + + flagsETag + + '\'' + + '}'; } @Override @@ -126,16 +140,24 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; Configuration that = (Configuration) o; return isConfigObfuscated == that.isConfigObfuscated - && Objects.equals(banditReferences, that.banditReferences) - && Objects.equals(flags, that.flags) - && Objects.equals(bandits, that.bandits) - && Objects.deepEquals(flagConfigJson, that.flagConfigJson) - && Objects.deepEquals(banditParamsJson, that.banditParamsJson); + && Objects.equals(banditReferences, that.banditReferences) + && Objects.equals(flags, that.flags) + && Objects.equals(bandits, that.bandits) + && Objects.deepEquals(flagConfigJson, that.flagConfigJson) + && Objects.deepEquals(banditParamsJson, that.banditParamsJson) + && Objects.equals(flagsETag, that.flagsETag); } @Override public int hashCode() { - return Objects.hash(banditReferences, flags, bandits, isConfigObfuscated, Arrays.hashCode(flagConfigJson), Arrays.hashCode(banditParamsJson)); + return Objects.hash( + banditReferences, + flags, + bandits, + isConfigObfuscated, + Arrays.hashCode(flagConfigJson), + Arrays.hashCode(banditParamsJson), + flagsETag); } public FlagConfig getFlag(String flagKey) { @@ -197,6 +219,10 @@ public byte[] serializeBanditParamsToBytes() { return banditParamsJson; } + @Nullable public String getFlagsETag() { + return flagsETag; + } + public boolean isEmpty() { return flags == null || flags.isEmpty(); } @@ -226,6 +252,7 @@ public static class Builder { private Map bandits = Collections.emptyMap(); private final byte[] flagJson; private byte[] banditParamsJson; + @Nullable private String flagsETag; private static FlagConfigResponse parseFlagResponse(byte[] flagJson) { if (flagJson == null || flagJson.length == 0) { @@ -336,9 +363,20 @@ public Builder banditParameters(byte[] banditParameterJson) { return this; } + public Builder flagsETag(@Nullable String eTag) { + this.flagsETag = eTag; + return this; + } + public Configuration build() { return new Configuration( - flags, banditReferences, bandits, isConfigObfuscated, flagJson, banditParamsJson); + flags, + banditReferences, + bandits, + isConfigObfuscated, + flagJson, + banditParamsJson, + flagsETag); } } } diff --git a/src/test/java/cloud/eppo/BaseEppoClientTest.java b/src/test/java/cloud/eppo/BaseEppoClientTest.java index 4c805440..b40ee517 100644 --- a/src/test/java/cloud/eppo/BaseEppoClientTest.java +++ b/src/test/java/cloud/eppo/BaseEppoClientTest.java @@ -734,7 +734,8 @@ public void testPolling() { verify(httpClient, times(2)).get(anyString()); // Set up a different config to be served - when(httpClient.get(anyString())).thenReturn(DISABLED_BOOL_FLAG_CONFIG.getBytes()); + when(httpClient.get(anyString())) + .thenReturn(new EppoHttpResponse(DISABLED_BOOL_FLAG_CONFIG.getBytes(), 200, null)); client.startPolling(20); // True until the next config is fetched. diff --git a/src/test/java/cloud/eppo/ConfigurationRequestorTest.java b/src/test/java/cloud/eppo/ConfigurationRequestorTest.java index 11a2448b..9884350b 100644 --- a/src/test/java/cloud/eppo/ConfigurationRequestorTest.java +++ b/src/test/java/cloud/eppo/ConfigurationRequestorTest.java @@ -3,6 +3,8 @@ import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -62,11 +64,11 @@ public void testInitialConfigurationDoesntClobberFetch() throws IOException { CompletableFuture initialConfigFuture = new CompletableFuture<>(); String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); - CompletableFuture configFetchFuture = new CompletableFuture<>(); + CompletableFuture configFetchFuture = new CompletableFuture<>(); String fetchedFlagConfig = FileUtils.readFileToString(differentFlagConfigFile, StandardCharsets.UTF_8); - when(mockHttpClient.getAsync("/flag-config/v1/config")).thenReturn(configFetchFuture); + when(mockHttpClient.getAsync(anyString(), any())).thenReturn(configFetchFuture); // Set initial config and verify that no config has been set yet. requestor.setInitialConfiguration(initialConfigFuture); @@ -82,7 +84,8 @@ public void testInitialConfigurationDoesntClobberFetch() throws IOException { CompletableFuture handle = requestor.fetchAndSaveFromRemoteAsync(); // Resolve the fetch and then the initialConfig - configFetchFuture.complete(fetchedFlagConfig.getBytes(StandardCharsets.UTF_8)); + configFetchFuture.complete( + new EppoHttpResponse(fetchedFlagConfig.getBytes(StandardCharsets.UTF_8), 200, null)); initialConfigFuture.complete(new Configuration.Builder(flagConfig, false).build()); assertFalse(configStore.getConfiguration().isEmpty()); @@ -106,9 +109,9 @@ public void testBrokenFetchDoesntClobberCache() throws IOException { CompletableFuture initialConfigFuture = new CompletableFuture<>(); String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); - CompletableFuture configFetchFuture = new CompletableFuture<>(); + CompletableFuture configFetchFuture = new CompletableFuture<>(); - when(mockHttpClient.getAsync("/flag-config/v1/config")).thenReturn(configFetchFuture); + when(mockHttpClient.getAsync(anyString(), any())).thenReturn(configFetchFuture); // Set initial config and verify that no config has been set yet. requestor.setInitialConfiguration(initialConfigFuture); @@ -145,9 +148,9 @@ public void testCacheWritesAfterBrokenFetch() throws IOException { CompletableFuture initialConfigFuture = new CompletableFuture<>(); String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); - CompletableFuture configFetchFuture = new CompletableFuture<>(); + CompletableFuture configFetchFuture = new CompletableFuture<>(); - when(mockHttpClient.getAsync("/flag-config/v1/config")).thenReturn(configFetchFuture); + when(mockHttpClient.getAsync(anyString(), any())).thenReturn(configFetchFuture); // Set initial config and verify that no config has been set yet. requestor.setInitialConfiguration(initialConfigFuture); @@ -190,7 +193,8 @@ public void setup() { public void testConfigurationChangeListener() throws IOException { // Setup mock response String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); - when(mockHttpClient.get(anyString())).thenReturn(flagConfig.getBytes()); + when(mockHttpClient.get(anyString())) + .thenReturn(new EppoHttpResponse(flagConfig.getBytes(), 200, null)); when(mockConfigStore.saveConfiguration(any())) .thenReturn(CompletableFuture.completedFuture(null)); @@ -216,7 +220,8 @@ public void testConfigurationChangeListener() throws IOException { @Test public void testMultipleConfigurationChangeListeners() { // Setup mock response - when(mockHttpClient.get(anyString())).thenReturn("{}".getBytes()); + when(mockHttpClient.get(anyString())) + .thenReturn(new EppoHttpResponse("{}".getBytes(), 200, null)); when(mockConfigStore.saveConfiguration(any())) .thenReturn(CompletableFuture.completedFuture(null)); @@ -265,7 +270,8 @@ public void testConfigurationChangeListenerIgnoresFailedFetch() { @Test public void testConfigurationChangeListenerIgnoresFailedSave() { // Setup mock responses - when(mockHttpClient.get(anyString())).thenReturn("{}".getBytes()); + when(mockHttpClient.get(anyString())) + .thenReturn(new EppoHttpResponse("{}".getBytes(), 200, null)); when(mockConfigStore.saveConfiguration(any())) .thenReturn( CompletableFuture.supplyAsync( @@ -288,8 +294,11 @@ public void testConfigurationChangeListenerIgnoresFailedSave() { @Test public void testConfigurationChangeListenerAsyncSave() { // Setup mock responses - when(mockHttpClient.getAsync(anyString())) - .thenReturn(CompletableFuture.completedFuture("{\"flags\":{}}".getBytes())); + when(mockHttpClient.getAsync(anyString(), any())) + .thenReturn( + CompletableFuture.completedFuture( + new EppoHttpResponse("{\"flags\":{}}".getBytes(), 200, null))); + when(mockConfigStore.getConfiguration()).thenReturn(Configuration.emptyConfig()); CompletableFuture saveFuture = new CompletableFuture<>(); when(mockConfigStore.saveConfiguration(any())).thenReturn(saveFuture); @@ -306,4 +315,200 @@ public void testConfigurationChangeListenerAsyncSave() { fetch.join(); assertEquals(1, callCount.get()); // Callback should be called after save completes } + + @Test + public void testETagStoredFromSuccessfulFetch() throws IOException { + // Setup + ConfigurationStore store = new ConfigurationStore(); + EppoHttpClient mockClient = mock(EppoHttpClient.class); + ConfigurationRequestor requestor = new ConfigurationRequestor(store, mockClient, false, false); + + String testETag = "test-etag-12345"; + String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); + EppoHttpResponse response = new EppoHttpResponse(flagConfig.getBytes(), 200, testETag); + + when(mockClient.get(anyString())).thenReturn(response); + + // Execute + requestor.fetchAndSaveFromRemote(); + + // Verify eTag was stored + Configuration config = store.getConfiguration(); + assertEquals(testETag, config.getFlagsETag()); + } + + @Test + public void test304SkipsConfigurationUpdate() throws IOException { + // Setup with existing config that has an eTag + ConfigurationStore store = Mockito.spy(new ConfigurationStore()); + String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); + Configuration existingConfig = + Configuration.builder(flagConfig.getBytes()).flagsETag("old-etag").build(); + store.saveConfiguration(existingConfig).join(); + + EppoHttpClient mockClient = mock(EppoHttpClient.class); + ConfigurationRequestor requestor = new ConfigurationRequestor(store, mockClient, false, false); + + // Mock 304 response + EppoHttpResponse response304 = new EppoHttpResponse(new byte[0], 304, "old-etag"); + when(mockClient.get(anyString())).thenReturn(response304); + + // Execute + requestor.fetchAndSaveFromRemote(); + + // Verify: Configuration NOT updated (saveConfiguration called only once - the initial setup) + Mockito.verify(store, Mockito.times(1)).saveConfiguration(any()); + + // Verify: Same config instance still in store + assertEquals("old-etag", store.getConfiguration().getFlagsETag()); + } + + @Test + public void test304DoesNotFireCallbacks() throws IOException { + // Setup with existing config + ConfigurationStore store = new ConfigurationStore(); + String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); + Configuration existingConfig = + Configuration.builder(flagConfig.getBytes()).flagsETag("test-etag").build(); + store.saveConfiguration(existingConfig).join(); + + EppoHttpClient mockClient = mock(EppoHttpClient.class); + ConfigurationRequestor requestor = new ConfigurationRequestor(store, mockClient, false, false); + + // Setup callback counter + AtomicInteger callbackCount = new AtomicInteger(0); + requestor.onConfigurationChange(config -> callbackCount.incrementAndGet()); + + // Mock 304 response + EppoHttpResponse response304 = new EppoHttpResponse(new byte[0], 304, "test-etag"); + when(mockClient.get(anyString())).thenReturn(response304); + + // Execute + requestor.fetchAndSaveFromRemote(); + + // Verify: Callback was NOT called + assertEquals(0, callbackCount.get()); + } + + @Test + public void testIfNoneMatchHeaderSentAsync() throws Exception { + // Setup with existing config that has eTag + ConfigurationStore store = new ConfigurationStore(); + String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); + Configuration existingConfig = + Configuration.builder(flagConfig.getBytes()).flagsETag("existing-etag").build(); + store.saveConfiguration(existingConfig).join(); + + EppoHttpClient mockClient = mock(EppoHttpClient.class); + ConfigurationRequestor requestor = new ConfigurationRequestor(store, mockClient, false, false); + + // Mock response + EppoHttpResponse response = new EppoHttpResponse(flagConfig.getBytes(), 200, "new-etag"); + when(mockClient.getAsync(anyString(), any())) + .thenReturn(CompletableFuture.completedFuture(response)); + + // Execute async fetch + requestor.fetchAndSaveFromRemoteAsync().join(); + + // Verify: getAsync was called with the existing eTag + Mockito.verify(mockClient).getAsync(anyString(), eq("existing-etag")); + } + + @Test + public void testNoIfNoneMatchHeaderWhenNoETag() throws Exception { + // Setup with empty config (no eTag) + ConfigurationStore store = new ConfigurationStore(); + + EppoHttpClient mockClient = mock(EppoHttpClient.class); + ConfigurationRequestor requestor = new ConfigurationRequestor(store, mockClient, false, false); + + // Mock response + String flagConfig = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); + EppoHttpResponse response = new EppoHttpResponse(flagConfig.getBytes(), 200, "new-etag"); + when(mockClient.getAsync(anyString(), any())) + .thenReturn(CompletableFuture.completedFuture(response)); + + // Execute + requestor.fetchAndSaveFromRemoteAsync().join(); + + // Verify: getAsync was called with null (no If-None-Match header) + Mockito.verify(mockClient).getAsync(anyString(), isNull()); + } + + @Test + public void testEmptyConfigHasNullETag() { + Configuration emptyConfig = Configuration.emptyConfig(); + assertNull(emptyConfig.getFlagsETag()); + } + + @Test + public void testETagRoundTripScenario() throws Exception { + ConfigurationStore store = new ConfigurationStore(); + EppoHttpClient mockClient = mock(EppoHttpClient.class); + ConfigurationRequestor requestor = new ConfigurationRequestor(store, mockClient, false, false); + + AtomicInteger callbackCount = new AtomicInteger(0); + requestor.onConfigurationChange(config -> callbackCount.incrementAndGet()); + + String flagConfig1 = FileUtils.readFileToString(initialFlagConfigFile, StandardCharsets.UTF_8); + String flagConfig2 = + FileUtils.readFileToString(differentFlagConfigFile, StandardCharsets.UTF_8); + + // First fetch: 200 OK with eTag + EppoHttpResponse response1 = new EppoHttpResponse(flagConfig1.getBytes(), 200, "etag-v1"); + when(mockClient.getAsync(anyString(), isNull())) + .thenReturn(CompletableFuture.completedFuture(response1)); + + requestor.fetchAndSaveFromRemoteAsync().join(); + + assertEquals("etag-v1", store.getConfiguration().getFlagsETag()); + assertEquals(1, callbackCount.get()); + + // Second fetch: 304 Not Modified + EppoHttpResponse response2 = new EppoHttpResponse(new byte[0], 304, "etag-v1"); + when(mockClient.getAsync(anyString(), eq("etag-v1"))) + .thenReturn(CompletableFuture.completedFuture(response2)); + + requestor.fetchAndSaveFromRemoteAsync().join(); + + // ETag unchanged, callback not fired + assertEquals("etag-v1", store.getConfiguration().getFlagsETag()); + assertEquals(1, callbackCount.get()); // Still 1, not 2 + + // Third fetch: 200 OK with new eTag (data changed) + EppoHttpResponse response3 = new EppoHttpResponse(flagConfig2.getBytes(), 200, "etag-v2"); + when(mockClient.getAsync(anyString(), eq("etag-v1"))) + .thenReturn(CompletableFuture.completedFuture(response3)); + + requestor.fetchAndSaveFromRemoteAsync().join(); + + // New eTag stored, callback fired + assertEquals("etag-v2", store.getConfiguration().getFlagsETag()); + assertEquals(2, callbackCount.get()); // Now 2 + } + + @Test + public void testBanditsNotFetchedOn304() throws IOException { + // Setup with config that has bandits + ConfigurationStore store = new ConfigurationStore(); + String flagConfigWithBandits = + "{\"flags\":{},\"banditReferences\":{\"test-bandit\":{\"modelVersion\":\"v1\",\"flagVariations\":[]}}}"; + Configuration existingConfig = + Configuration.builder(flagConfigWithBandits.getBytes()).flagsETag("test-etag").build(); + store.saveConfiguration(existingConfig).join(); + + EppoHttpClient mockClient = mock(EppoHttpClient.class); + ConfigurationRequestor requestor = + new ConfigurationRequestor(store, mockClient, false, true); // bandits enabled + + // Mock 304 for flags + EppoHttpResponse response304 = new EppoHttpResponse(new byte[0], 304, "test-etag"); + when(mockClient.get(Constants.FLAG_CONFIG_ENDPOINT)).thenReturn(response304); + + // Execute + requestor.fetchAndSaveFromRemote(); + + // Verify: Bandits endpoint was NOT called + Mockito.verify(mockClient, Mockito.never()).get(Constants.BANDIT_ENDPOINT); + } } diff --git a/src/test/java/cloud/eppo/api/ConfigurationBuilderTest.java b/src/test/java/cloud/eppo/api/ConfigurationBuilderTest.java index 54360033..f148fed5 100644 --- a/src/test/java/cloud/eppo/api/ConfigurationBuilderTest.java +++ b/src/test/java/cloud/eppo/api/ConfigurationBuilderTest.java @@ -73,7 +73,8 @@ public void getFlagType_shouldReturnCorrectType() { // Create configuration with this flag Map flags = Collections.singletonMap("test-flag", flagConfig); Configuration config = - new Configuration(flags, Collections.emptyMap(), Collections.emptyMap(), false, null, null); + new Configuration( + flags, Collections.emptyMap(), Collections.emptyMap(), false, null, null, null); // Test successful case assertEquals(VariationType.STRING, config.getFlagType("test-flag")); @@ -104,6 +105,7 @@ public void getFlagType_withObfuscatedConfig_shouldReturnCorrectType() { Collections.emptyMap(), true, // obfuscated null, + null, null); // Test successful case with obfuscated config diff --git a/src/test/java/cloud/eppo/helpers/TestUtils.java b/src/test/java/cloud/eppo/helpers/TestUtils.java index 4b9a5822..cd5a5cbc 100644 --- a/src/test/java/cloud/eppo/helpers/TestUtils.java +++ b/src/test/java/cloud/eppo/helpers/TestUtils.java @@ -5,6 +5,7 @@ import cloud.eppo.BaseEppoClient; import cloud.eppo.EppoHttpClient; +import cloud.eppo.EppoHttpResponse; import java.lang.reflect.Field; import java.util.concurrent.CompletableFuture; import okhttp3.*; @@ -16,13 +17,17 @@ public static EppoHttpClient mockHttpResponse(String responseBody) { // Create a mock instance of EppoHttpClient EppoHttpClient mockHttpClient = mock(EppoHttpClient.class); + // Create EppoHttpResponse with 200 status and no eTag + EppoHttpResponse response = new EppoHttpResponse(responseBody.getBytes(), 200, null); + // Mock sync get - when(mockHttpClient.get(anyString())).thenReturn(responseBody.getBytes()); + when(mockHttpClient.get(anyString())).thenReturn(response); - // Mock async get - CompletableFuture mockAsyncResponse = new CompletableFuture<>(); + // Mock async get - both signatures + CompletableFuture mockAsyncResponse = new CompletableFuture<>(); when(mockHttpClient.getAsync(anyString())).thenReturn(mockAsyncResponse); - mockAsyncResponse.complete(responseBody.getBytes()); + when(mockHttpClient.getAsync(anyString(), any())).thenReturn(mockAsyncResponse); + mockAsyncResponse.complete(response); setBaseClientHttpClientOverrideField(mockHttpClient); return mockHttpClient; @@ -35,9 +40,10 @@ public static void mockHttpError() { // Mock sync get when(mockHttpClient.get(anyString())).thenThrow(new RuntimeException("Intentional Error")); - // Mock async get - CompletableFuture mockAsyncResponse = new CompletableFuture<>(); + // Mock async get - both signatures + CompletableFuture mockAsyncResponse = new CompletableFuture<>(); when(mockHttpClient.getAsync(anyString())).thenReturn(mockAsyncResponse); + when(mockHttpClient.getAsync(anyString(), any())).thenReturn(mockAsyncResponse); mockAsyncResponse.completeExceptionally(new RuntimeException("Intentional Error")); setBaseClientHttpClientOverrideField(mockHttpClient);