diff --git a/src/main/java/io/split/android/client/fallback/FallbackTreatment.java b/src/main/java/io/split/android/client/fallback/FallbackTreatment.java index f75f9f9c0..b596bd37b 100644 --- a/src/main/java/io/split/android/client/fallback/FallbackTreatment.java +++ b/src/main/java/io/split/android/client/fallback/FallbackTreatment.java @@ -5,32 +5,30 @@ import java.util.Objects; -import io.split.android.grammar.Treatments; - /** * Represents the fallback treatment, with an optional config and a fixed label. */ public final class FallbackTreatment { - private static final String LABEL_PREFIX = "fallback - "; - - /** - * Default fallback representing "control" treatment with no config. - */ - public static final FallbackTreatment CONTROL = new FallbackTreatment(Treatments.CONTROL); - @NonNull private final String mTreatment; @Nullable private final String mConfig; + @Nullable + private final String mLabel; public FallbackTreatment(@NonNull String treatment) { this(treatment, null); } public FallbackTreatment(@NonNull String treatment, @Nullable String config) { + this(treatment, config, null); + } + + FallbackTreatment(@NonNull String treatment, @Nullable String config, @Nullable String label) { mTreatment = treatment; mConfig = config; + mLabel = label; } public String getTreatment() { @@ -42,8 +40,13 @@ public String getConfig() { return mConfig; } - public String getLabelPrefix() { - return LABEL_PREFIX; + @Nullable + public String getLabel() { + return mLabel; + } + + FallbackTreatment copyWithLabel(String label) { + return new FallbackTreatment(mTreatment, mConfig, label); } @Override @@ -52,11 +55,12 @@ public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; FallbackTreatment that = (FallbackTreatment) o; return Objects.equals(mTreatment, that.mTreatment) && - Objects.equals(mConfig, that.mConfig); + Objects.equals(mConfig, that.mConfig) && + Objects.equals(mLabel, that.mLabel); } @Override public int hashCode() { - return Objects.hash(mTreatment, mConfig); + return Objects.hash(mTreatment, mConfig, mLabel); } } diff --git a/src/main/java/io/split/android/client/fallback/FallbackTreatmentsCalculator.java b/src/main/java/io/split/android/client/fallback/FallbackTreatmentsCalculator.java index cbcb18cac..5f9d097c5 100644 --- a/src/main/java/io/split/android/client/fallback/FallbackTreatmentsCalculator.java +++ b/src/main/java/io/split/android/client/fallback/FallbackTreatmentsCalculator.java @@ -1,6 +1,7 @@ package io.split.android.client.fallback; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; /** * Resolves a fallback treatment for a given flag name. @@ -10,10 +11,19 @@ public interface FallbackTreatmentsCalculator { /** * Resolve a fallback for a given flag name. - * * @param flagName non-null flag name - * @return a fallback treatment if configured, otherwise "control" + * @return a fallback treatment with a null label, if configured; otherwise "control" */ @NonNull FallbackTreatment resolve(@NonNull String flagName); + + /** + * Resolve a fallback for a given flag name and label. + * + * @param flagName non-null flag name + * @param label nullable label + * @return a fallback treatment if configured, with a prefixed label if provided; otherwise "control" + */ + @NonNull + FallbackTreatment resolve(@NonNull String flagName, @Nullable String label); } diff --git a/src/main/java/io/split/android/client/fallback/FallbackTreatmentsCalculatorImpl.java b/src/main/java/io/split/android/client/fallback/FallbackTreatmentsCalculatorImpl.java index 08d2ee493..a7d30a79c 100644 --- a/src/main/java/io/split/android/client/fallback/FallbackTreatmentsCalculatorImpl.java +++ b/src/main/java/io/split/android/client/fallback/FallbackTreatmentsCalculatorImpl.java @@ -1,11 +1,16 @@ package io.split.android.client.fallback; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import java.util.Map; +import io.split.android.grammar.Treatments; + public final class FallbackTreatmentsCalculatorImpl implements FallbackTreatmentsCalculator { + private static final String LABEL_PREFIX = "fallback - "; + @NonNull private final FallbackConfiguration mConfig; @@ -16,17 +21,32 @@ public FallbackTreatmentsCalculatorImpl(@NonNull FallbackConfiguration config) { @NonNull @Override public FallbackTreatment resolve(@NonNull String flagName) { + return resolve(flagName, null); + } + + @NonNull + @Override + public FallbackTreatment resolve(@NonNull String flagName, @Nullable String label) { Map byFlag = mConfig.getByFlag(); if (byFlag != null) { FallbackTreatment flagTreatment = byFlag.get(flagName); if (flagTreatment != null) { - return flagTreatment; + return flagTreatment.copyWithLabel(resolveLabel(label)); } } FallbackTreatment global = mConfig.getGlobal(); if (global != null) { - return global; + return global.copyWithLabel(resolveLabel(label)); } - return FallbackTreatment.CONTROL; + return new FallbackTreatment(Treatments.CONTROL, null, label); + } + + @Nullable + private static String resolveLabel(@Nullable String label) { + if (label == null) { + return null; + } + + return LABEL_PREFIX + label; } } diff --git a/src/main/java/io/split/android/client/fallback/FallbacksSanitizerImpl.java b/src/main/java/io/split/android/client/fallback/FallbacksSanitizerImpl.java index e9e1658e9..b7c543326 100644 --- a/src/main/java/io/split/android/client/fallback/FallbacksSanitizerImpl.java +++ b/src/main/java/io/split/android/client/fallback/FallbacksSanitizerImpl.java @@ -4,6 +4,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.regex.Pattern; import io.split.android.client.utils.logger.Logger; @@ -15,6 +16,8 @@ class FallbacksSanitizerImpl implements FallbacksSanitizer { private static final int MAX_FLAG_NAME_LENGTH = 100; private static final int MAX_TREATMENT_LENGTH = 100; + private static final String TREATMENT_REGEXP = "^[0-9]+[.a-zA-Z0-9_-]*$|^[a-zA-Z]+[a-zA-Z0-9_-]*$"; + private static final Pattern TREATMENT_PATTERN = Pattern.compile(TREATMENT_REGEXP); /** * Sanitizes the provided fallback configuration by applying validation rules. @@ -93,6 +96,7 @@ private static boolean isValidTreatment(FallbackTreatment treatment) { if (treatment == null || treatment.getTreatment() == null) { return false; } - return treatment.getTreatment().length() <= MAX_TREATMENT_LENGTH; + String value = treatment.getTreatment(); + return value.length() <= MAX_TREATMENT_LENGTH && TREATMENT_PATTERN.matcher(value).matches(); } } diff --git a/src/test/java/io/split/android/client/fallback/FallbackTreatmentTest.java b/src/test/java/io/split/android/client/fallback/FallbackTreatmentTest.java index a01dbb46a..3914e2c58 100644 --- a/src/test/java/io/split/android/client/fallback/FallbackTreatmentTest.java +++ b/src/test/java/io/split/android/client/fallback/FallbackTreatmentTest.java @@ -8,30 +8,36 @@ public class FallbackTreatmentTest { - private static final String FALLBACK_TREATMENT = "fallback - "; - @Test public void constructorSetsFields() { - FallbackTreatment ft = new FallbackTreatment("off", "{\"k\":true}"); + FallbackTreatment ft = new FallbackTreatment("off", "{\"k\":true}", "my label"); assertEquals("off", ft.getTreatment()); assertEquals("{\"k\":true}", ft.getConfig()); - assertEquals(FALLBACK_TREATMENT, ft.getLabelPrefix()); + assertEquals("my label", ft.getLabel()); } @Test public void configCanBeNull() { - FallbackTreatment ft = new FallbackTreatment("off", null); + FallbackTreatment ft = new FallbackTreatment("off", null, "my label"); + assertEquals("off", ft.getTreatment()); + assertNull(ft.getConfig()); + assertEquals("my label", ft.getLabel()); + } + + @Test + public void labelCanBeNull() { + FallbackTreatment ft = new FallbackTreatment("off", null, null); assertEquals("off", ft.getTreatment()); assertNull(ft.getConfig()); - assertEquals(FALLBACK_TREATMENT, ft.getLabelPrefix()); + assertNull(ft.getLabel()); } @Test - public void convenienceConstructorSetsNullConfig() { + public void convenienceConstructorSetsNullConfigAndLabel() { FallbackTreatment ft = new FallbackTreatment("off"); assertEquals("off", ft.getTreatment()); assertNull(ft.getConfig()); - assertEquals(FALLBACK_TREATMENT, ft.getLabelPrefix()); + assertNull(ft.getLabel()); } @Test diff --git a/src/test/java/io/split/android/client/fallback/FallbackTreatmentsCalculatorTest.java b/src/test/java/io/split/android/client/fallback/FallbackTreatmentsCalculatorTest.java index 19a6b1918..00083a0fa 100644 --- a/src/test/java/io/split/android/client/fallback/FallbackTreatmentsCalculatorTest.java +++ b/src/test/java/io/split/android/client/fallback/FallbackTreatmentsCalculatorTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import org.junit.Test; @@ -9,6 +10,8 @@ import java.util.HashMap; import java.util.Map; +import io.split.android.client.TreatmentLabels; + public class FallbackTreatmentsCalculatorTest { @Test @@ -72,7 +75,7 @@ public void returnsControlWhenNoFallbackConfigured() { FallbackTreatment resolved = calculator.resolve("nope"); assertNotNull(resolved); - assertEquals(FallbackTreatment.CONTROL, resolved); + assertEquals(new FallbackTreatment("control", null, null), resolved); } @Test @@ -91,4 +94,32 @@ public void nonexistentFlagFallsBackToGlobal() { assertNotNull(resolved); assertEquals(global, resolved); } + + @Test + public void labelIsPrefixed() { + FallbackTreatment global = new FallbackTreatment("off"); + FallbackConfiguration config = FallbackConfiguration.builder() + .global(global) + .build(); + + FallbackTreatmentsCalculator calculator = new FallbackTreatmentsCalculatorImpl(config); + FallbackTreatment resolved = calculator.resolve("flagA", TreatmentLabels.EXCEPTION); + + assertNotNull(resolved); + assertEquals("fallback - exception", resolved.getLabel()); + } + + @Test + public void noLabelReturnsNull() { + FallbackTreatment global = new FallbackTreatment("off"); + FallbackConfiguration config = FallbackConfiguration.builder() + .global(global) + .build(); + + FallbackTreatmentsCalculator calculator = new FallbackTreatmentsCalculatorImpl(config); + FallbackTreatment resolved = calculator.resolve("flagA", null); + + assertNotNull(resolved); + assertNull(resolved.getLabel()); + } } diff --git a/src/test/java/io/split/android/client/fallback/FallbacksSanitizerImplTest.java b/src/test/java/io/split/android/client/fallback/FallbacksSanitizerImplTest.java index 807a590d5..58dfceeaf 100644 --- a/src/test/java/io/split/android/client/fallback/FallbacksSanitizerImplTest.java +++ b/src/test/java/io/split/android/client/fallback/FallbacksSanitizerImplTest.java @@ -2,6 +2,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import org.junit.Before; import org.junit.Test; @@ -59,4 +60,63 @@ public void dropsInvalidGlobalTreatment() { assertNull(sanitized.getGlobal()); assertEquals(0, sanitized.getByFlag().size()); } + + @Test + public void byFlagTreatmentIsDroppedWhenInvalidFormat() { + Map byFlag = new HashMap<>(); + byFlag.put(VALID_FLAG, new FallbackTreatment("on.off")); + byFlag.put("valid_num_dot", new FallbackTreatment("123.on")); + byFlag.put("null_treatment", new FallbackTreatment(null)); + + FallbackConfiguration config = FallbackConfiguration.builder() + .global(null) + .byFlag(byFlag) + .build(); + + FallbackConfiguration sanitized = mSanitizer.sanitize(config); + + // Only the valid one should remain + assertEquals(1, sanitized.getByFlag().size()); + assertEquals("123.on", sanitized.getByFlag().get("valid_num_dot").getTreatment()); + } + + @Test + public void globalTreatmentIsDroppedWhenInvalidFormat() { + Map byFlag = new HashMap<>(); + byFlag.put(VALID_FLAG, new FallbackTreatment("on_1-2")); + byFlag.put("null_treatment", new FallbackTreatment(null)); + + FallbackConfiguration config = FallbackConfiguration.builder() + // Global invalid due to regex (letters cannot be followed by '.') + .global(new FallbackTreatment("on.off")) + .byFlag(byFlag) + .build(); + + FallbackConfiguration sanitized = mSanitizer.sanitize(config); + + assertNull(sanitized.getGlobal()); + // Ensure only the valid by-flag entry is preserved + assertEquals(1, sanitized.getByFlag().size()); + assertEquals("on_1-2", sanitized.getByFlag().get(VALID_FLAG).getTreatment()); + } + + @Test + public void validFormatTreatmentIsNotDropped() { + Map byFlag = new HashMap<>(); + byFlag.put("numWithDot", new FallbackTreatment("123.on")); + byFlag.put(VALID_FLAG, new FallbackTreatment("on_1-2")); + + FallbackConfiguration config = FallbackConfiguration.builder() + .global(new FallbackTreatment("on")) + .byFlag(byFlag) + .build(); + + FallbackConfiguration sanitized = mSanitizer.sanitize(config); + + assertEquals(2, sanitized.getByFlag().size()); + assertTrue(sanitized.getByFlag().containsKey("numWithDot")); + assertEquals("123.on", sanitized.getByFlag().get("numWithDot").getTreatment()); + assertEquals("on_1-2", sanitized.getByFlag().get(VALID_FLAG).getTreatment()); + assertEquals("on", sanitized.getGlobal().getTreatment()); + } }