From 9c6c31d6e5b43bace1a08b1e07c384c3478eb428 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 27 Aug 2025 08:58:37 -0300 Subject: [PATCH 1/3] Calculator enhancement --- .../client/fallback/FallbackTreatment.java | 23 ++++++++++---- .../FallbackTreatmentsCalculator.java | 14 +++++++-- .../FallbackTreatmentsCalculatorImpl.java | 22 +++++++++++-- .../fallback/FallbackTreatmentTest.java | 6 ++-- .../FallbackTreatmentsCalculatorTest.java | 31 +++++++++++++++++++ 5 files changed, 83 insertions(+), 13 deletions(-) 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..836eb517b 100644 --- a/src/main/java/io/split/android/client/fallback/FallbackTreatment.java +++ b/src/main/java/io/split/android/client/fallback/FallbackTreatment.java @@ -12,8 +12,6 @@ */ public final class FallbackTreatment { - private static final String LABEL_PREFIX = "fallback - "; - /** * Default fallback representing "control" treatment with no config. */ @@ -23,14 +21,21 @@ public final class FallbackTreatment { 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); + } + + private FallbackTreatment(@NonNull String treatment, @Nullable String config, @Nullable String label) { mTreatment = treatment; mConfig = config; + mLabel = label; } public String getTreatment() { @@ -42,8 +47,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 +62,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..21c025d19 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,14 @@ package io.split.android.client.fallback; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import java.util.Map; public final class FallbackTreatmentsCalculatorImpl implements FallbackTreatmentsCalculator { + private static final String LABEL_PREFIX = "fallback - "; + @NonNull private final FallbackConfiguration mConfig; @@ -16,17 +19,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; } + + @Nullable + private static String resolveLabel(@Nullable String label) { + if (label == null) { + return null; + } + + return LABEL_PREFIX + label; + } } 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..efd83efa8 100644 --- a/src/test/java/io/split/android/client/fallback/FallbackTreatmentTest.java +++ b/src/test/java/io/split/android/client/fallback/FallbackTreatmentTest.java @@ -15,7 +15,7 @@ public void constructorSetsFields() { FallbackTreatment ft = new FallbackTreatment("off", "{\"k\":true}"); assertEquals("off", ft.getTreatment()); assertEquals("{\"k\":true}", ft.getConfig()); - assertEquals(FALLBACK_TREATMENT, ft.getLabelPrefix()); + assertEquals(FALLBACK_TREATMENT, ft.getLabel()); } @Test @@ -23,7 +23,7 @@ public void configCanBeNull() { FallbackTreatment ft = new FallbackTreatment("off", null); assertEquals("off", ft.getTreatment()); assertNull(ft.getConfig()); - assertEquals(FALLBACK_TREATMENT, ft.getLabelPrefix()); + assertEquals(FALLBACK_TREATMENT, ft.getLabel()); } @Test @@ -31,7 +31,7 @@ public void convenienceConstructorSetsNullConfig() { FallbackTreatment ft = new FallbackTreatment("off"); assertEquals("off", ft.getTreatment()); assertNull(ft.getConfig()); - assertEquals(FALLBACK_TREATMENT, ft.getLabelPrefix()); + assertEquals(FALLBACK_TREATMENT, 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..66b74ae8b 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 @@ -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()); + } } From 14b189d261bbfdc8a8708275193b64b6c4e1d7c3 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 27 Aug 2025 09:59:15 -0300 Subject: [PATCH 2/3] Fixes --- .../client/fallback/FallbackTreatment.java | 9 +------- .../FallbackTreatmentsCalculatorImpl.java | 4 +++- .../fallback/FallbackTreatmentTest.java | 22 ++++++++++++------- .../FallbackTreatmentsCalculatorTest.java | 2 +- 4 files changed, 19 insertions(+), 18 deletions(-) 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 836eb517b..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,18 +5,11 @@ 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 { - /** - * Default fallback representing "control" treatment with no config. - */ - public static final FallbackTreatment CONTROL = new FallbackTreatment(Treatments.CONTROL); - @NonNull private final String mTreatment; @Nullable @@ -32,7 +25,7 @@ public FallbackTreatment(@NonNull String treatment, @Nullable String config) { this(treatment, config, null); } - private FallbackTreatment(@NonNull String treatment, @Nullable String config, @Nullable String label) { + FallbackTreatment(@NonNull String treatment, @Nullable String config, @Nullable String label) { mTreatment = treatment; mConfig = config; mLabel = 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 21c025d19..a7d30a79c 100644 --- a/src/main/java/io/split/android/client/fallback/FallbackTreatmentsCalculatorImpl.java +++ b/src/main/java/io/split/android/client/fallback/FallbackTreatmentsCalculatorImpl.java @@ -5,6 +5,8 @@ import java.util.Map; +import io.split.android.grammar.Treatments; + public final class FallbackTreatmentsCalculatorImpl implements FallbackTreatmentsCalculator { private static final String LABEL_PREFIX = "fallback - "; @@ -36,7 +38,7 @@ public FallbackTreatment resolve(@NonNull String flagName, @Nullable String labe if (global != null) { return global.copyWithLabel(resolveLabel(label)); } - return FallbackTreatment.CONTROL; + return new FallbackTreatment(Treatments.CONTROL, null, label); } @Nullable 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 efd83efa8..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.getLabel()); + 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.getLabel()); + 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.getLabel()); + 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 66b74ae8b..00083a0fa 100644 --- a/src/test/java/io/split/android/client/fallback/FallbackTreatmentsCalculatorTest.java +++ b/src/test/java/io/split/android/client/fallback/FallbackTreatmentsCalculatorTest.java @@ -75,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 From 03165b8bb9435b143a46ee6e2cfb312f342aef3d Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 27 Aug 2025 12:25:31 -0300 Subject: [PATCH 3/3] Fallback sanitizer update --- .../fallback/FallbacksSanitizerImpl.java | 6 +- .../fallback/FallbacksSanitizerImplTest.java | 60 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) 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/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()); + } }