diff --git a/src/androidTest/java/tests/integration/fallback/FallbackTreatmentsTest.java b/src/androidTest/java/tests/integration/fallback/FallbackTreatmentsTest.java index 92abf6439..53c853af6 100644 --- a/src/androidTest/java/tests/integration/fallback/FallbackTreatmentsTest.java +++ b/src/androidTest/java/tests/integration/fallback/FallbackTreatmentsTest.java @@ -371,7 +371,7 @@ public void onPostExecution(SplitClient client) { readyLatch.countDown(); } }); - readyLatch.await(30, TimeUnit.SECONDS); + readyLatch.await(5, TimeUnit.SECONDS); String t1_flag1 = clientKey1.getTreatment("non_existent_flag"); String t1_flag2 = clientKey1.getTreatment("non_existent_flag_2"); @@ -424,7 +424,7 @@ public void onPostExecution(SplitClient client) { readyLatch.countDown(); } }); - readyLatch.await(30, TimeUnit.SECONDS); + readyLatch.await(5, TimeUnit.SECONDS); String t1_flag1 = clientKey1.getTreatment("non_existent_flag"); String t1_flag2 = clientKey1.getTreatment("non_existent_flag_2"); @@ -478,7 +478,7 @@ public void onPostExecution(SplitClient client) { readyLatch.countDown(); } }); - readyLatch.await(30, TimeUnit.SECONDS); + readyLatch.await(5, TimeUnit.SECONDS); SplitResult rMy = client.getTreatmentWithConfig("my_flag", null); SplitResult rUnknown = client.getTreatmentWithConfig("non_existent_flag", null); diff --git a/src/main/java/io/split/android/client/fallback/FallbackTreatmentsConfiguration.java b/src/main/java/io/split/android/client/fallback/FallbackTreatmentsConfiguration.java index 4406d4af1..992ebae2e 100644 --- a/src/main/java/io/split/android/client/fallback/FallbackTreatmentsConfiguration.java +++ b/src/main/java/io/split/android/client/fallback/FallbackTreatmentsConfiguration.java @@ -1,5 +1,6 @@ package io.split.android.client.fallback; +import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; @@ -8,6 +9,8 @@ import java.util.Map; import java.util.Objects; +import io.split.android.client.utils.logger.Logger; + public final class FallbackTreatmentsConfiguration { @Nullable @@ -63,10 +66,29 @@ private Builder() { * @return this builder instance */ public Builder global(@Nullable FallbackTreatment global) { + if (mGlobal != null && global != null) { + Logger.w("Fallback treatments - You had previously set a global fallback. The new value will replace it"); + } mGlobal = global; return this; } + /** + * Sets an optional global fallback treatment to be used when no flag-specific + * fallback exists for a given flag. This value is returned only in place of + * the "control" treatment. + * + * @param treatment the treatment string to use as global + * @return this builder instance + */ + public Builder global(String treatment) { + if (mGlobal != null) { + Logger.w("Fallback treatments - You had previously set a global fallback. The new value will replace it"); + } + mGlobal = new FallbackTreatment(treatment); + return this; + } + /** * Sets optional flag-specific fallback treatments, where keys are flag names. * These take precedence over the global fallback. @@ -75,7 +97,43 @@ public Builder global(@Nullable FallbackTreatment global) { * @return this builder instance */ public Builder byFlag(@Nullable Map byFlag) { - mByFlag = byFlag; + if (byFlag == null || byFlag.isEmpty()) { + return this; + } + if (mByFlag == null) { + mByFlag = new HashMap<>(); + } + for (Map.Entry e : byFlag.entrySet()) { + String key = e.getKey(); + if (mByFlag.containsKey(key)) { + Logger.w(getDuplicateFlagMessage(key)); + } + mByFlag.put(key, e.getValue()); + } + return this; + } + + /** + * Sets optional flag-specific fallback treatments, where keys are flag names. + * These take precedence over the global fallback. + * + * @param byFlag map of flag name to treatment string; may be null or empty + * @return this builder instance + */ + public Builder byFlagStrings(@Nullable Map byFlag) { + if (byFlag == null || byFlag.isEmpty()) { + return this; + } + if (mByFlag == null) { + mByFlag = new HashMap<>(); + } + for (Map.Entry e : byFlag.entrySet()) { + String key = e.getKey(); + if (mByFlag.containsKey(key)) { + Logger.w(getDuplicateFlagMessage(key)); + } + mByFlag.put(key, new FallbackTreatment(e.getValue())); + } return this; } @@ -95,6 +153,11 @@ Builder sanitizer(FallbacksSanitizer sanitizer) { mSanitizer = sanitizer; return this; } + + @NonNull + private static String getDuplicateFlagMessage(String key) { + return "Fallback treatments - Duplicate fallback for flag '" + key + "'. Overriding existing value."; + } } @Override diff --git a/src/test/java/io/split/android/client/fallback/FallbackTreatmentsConfigurationTest.java b/src/test/java/io/split/android/client/fallback/FallbackTreatmentsConfigurationTest.java index f9d2c11a7..8160e396a 100644 --- a/src/test/java/io/split/android/client/fallback/FallbackTreatmentsConfigurationTest.java +++ b/src/test/java/io/split/android/client/fallback/FallbackTreatmentsConfigurationTest.java @@ -1,13 +1,21 @@ package io.split.android.client.fallback; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import org.junit.Test; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ConcurrentLinkedDeque; + +import io.split.android.client.utils.logger.LogPrinterStub; +import io.split.android.client.utils.logger.Logger; +import io.split.android.client.utils.logger.SplitLogLevel; public class FallbackTreatmentsConfigurationTest { @@ -62,11 +70,125 @@ public void equalityAndHashCodeByValue() { FallbackTreatmentsConfiguration configOne = FallbackTreatmentsConfiguration.builder().global(global).byFlag(a).build(); FallbackTreatmentsConfiguration configTwo = FallbackTreatmentsConfiguration.builder().global(global).byFlag(b).build(); - FallbackTreatmentsConfiguration configThree = FallbackTreatmentsConfiguration.builder().global(null).byFlag(b).build(); + FallbackTreatmentsConfiguration configThree = FallbackTreatmentsConfiguration.builder().global((String) null).byFlag(b).build(); assertEquals(configOne, configTwo); assertEquals(configOne.hashCode(), configTwo.hashCode()); assertNotEquals(configOne, configThree); assertNotEquals(configOne.hashCode(), configThree.hashCode()); } + + @Test + public void globalStringOverloadBuildsFallbackWithNullConfig() { + FallbackTreatmentsConfiguration cfg = FallbackTreatmentsConfiguration.builder() + .global("on") + .build(); + + FallbackTreatment global = cfg.getGlobal(); + assertEquals("on", global.getTreatment()); + assertNull(global.getConfig()); + } + + @Test + public void byFlagStringMapOverloadBuildsFallbacksWithNullConfig() { + Map flagTreatments = new HashMap<>(); + flagTreatments.put("flagA", "on"); + flagTreatments.put("flagB", "off"); + + FallbackTreatmentsConfiguration cfg = FallbackTreatmentsConfiguration.builder() + .byFlagStrings(flagTreatments) + .build(); + + assertEquals(2, cfg.getByFlag().size()); + assertEquals("on", cfg.getByFlag().get("flagA").getTreatment()); + assertNull(cfg.getByFlag().get("flagA").getConfig()); + assertEquals("off", cfg.getByFlag().get("flagB").getTreatment()); + assertNull(cfg.getByFlag().get("flagB").getConfig()); + } + + @Test + public void callingByFlagStringsAfterByFlagMergesResultsAndLogsWarning() { + LogPrinterStub printer = new LogPrinterStub(); + Logger.instance().setPrinter(printer); + Logger.instance().setLevel(SplitLogLevel.WARNING); + + Map first = new HashMap<>(); + first.put("flagA", new FallbackTreatment("off", "cfgA")); + first.put("flagB", new FallbackTreatment("on")); + + Map second = new HashMap<>(); + second.put("flagA", "on"); // should override flagA + second.put("flagC", "off"); + + FallbackTreatmentsConfiguration cfg = FallbackTreatmentsConfiguration.builder() + .byFlag(first) + .byFlagStrings(second) + .build(); + + assertEquals(3, cfg.getByFlag().size()); + assertEquals("on", cfg.getByFlag().get("flagA").getTreatment()); + assertNull(cfg.getByFlag().get("flagA").getConfig()); + assertEquals("on", cfg.getByFlag().get("flagB").getTreatment()); + assertEquals("off", cfg.getByFlag().get("flagC").getTreatment()); + // warning logged with expected content for overridden key + ConcurrentLinkedDeque warns = printer.getLoggedMessages().get(android.util.Log.WARN); + assertFalse("Expected at least one warning", warns.isEmpty()); + boolean containsExpected = warns.stream().anyMatch(m -> m.contains("Fallback treatments - Duplicate fallback for flag 'flagA'. Overriding existing value.")); + assertTrue("Expected warning mentioning overridden key 'flagA'", containsExpected); + } + + @Test + public void callingByFlagAfterByFlagStringsMergesResultsAndLogsWarning() { + LogPrinterStub printer = new LogPrinterStub(); + Logger.instance().setPrinter(printer); + Logger.instance().setLevel(SplitLogLevel.WARNING); + + Map first = new HashMap<>(); + first.put("flagA", "off"); + first.put("flagB", "on"); + + Map second = new HashMap<>(); + second.put("flagA", new FallbackTreatment("on", "cfgA")); // should override flagA + second.put("flagC", new FallbackTreatment("off")); + + FallbackTreatmentsConfiguration cfg = FallbackTreatmentsConfiguration.builder() + .byFlagStrings(first) + .byFlag(second) + .build(); + + assertEquals(3, cfg.getByFlag().size()); + assertEquals("on", cfg.getByFlag().get("flagA").getTreatment()); + assertEquals("cfgA", cfg.getByFlag().get("flagA").getConfig()); + assertEquals("on", cfg.getByFlag().get("flagB").getTreatment()); + assertNull(cfg.getByFlag().get("flagB").getConfig()); + assertEquals("off", cfg.getByFlag().get("flagC").getTreatment()); + + boolean warned = !printer.getLoggedMessages().get(android.util.Log.WARN).isEmpty(); + assertTrue("Expected a warning log when merging byFlag and byFlagStrings", warned); + } + + @Test + public void byFlagAndByFlagStrings_NoOverlap_NoWarning() { + LogPrinterStub printer = new LogPrinterStub(); + Logger.instance().setPrinter(printer); + Logger.instance().setLevel(SplitLogLevel.WARNING); + + Map first = new HashMap<>(); + first.put("flagA", new FallbackTreatment("off")); + + Map second = new HashMap<>(); + second.put("flagB", "on"); + + FallbackTreatmentsConfiguration cfg = FallbackTreatmentsConfiguration.builder() + .byFlag(first) + .byFlagStrings(second) + .build(); + + assertEquals(2, cfg.getByFlag().size()); + assertEquals("off", cfg.getByFlag().get("flagA").getTreatment()); + assertEquals("on", cfg.getByFlag().get("flagB").getTreatment()); + + boolean warned = !printer.getLoggedMessages().get(android.util.Log.WARN).isEmpty(); + assertFalse("Did not expect a warning", warned); + } }