-
Notifications
You must be signed in to change notification settings - Fork 7
FT Sanitizer #802
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
FT Sanitizer #802
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
103 changes: 103 additions & 0 deletions
103
src/main/java/io/split/android/client/fallback/FallbackConfiguration.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| package io.split.android.client.fallback; | ||
|
|
||
| import androidx.annotation.Nullable; | ||
|
|
||
| import java.util.Collections; | ||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
|
|
||
| public final class FallbackConfiguration { | ||
|
|
||
| @Nullable | ||
| private final FallbackTreatment mGlobal; | ||
| private final Map<String, FallbackTreatment> mByFlag; | ||
|
|
||
| private FallbackConfiguration(@Nullable FallbackTreatment global, | ||
| @Nullable Map<String, FallbackTreatment> byFlag) { | ||
| mGlobal = global; | ||
| if (byFlag == null || byFlag.isEmpty()) { | ||
| mByFlag = Collections.emptyMap(); | ||
| } else { | ||
| mByFlag = Collections.unmodifiableMap(new HashMap<>(byFlag)); | ||
| } | ||
| } | ||
|
|
||
| @Nullable | ||
| public FallbackTreatment getGlobal() { | ||
| return mGlobal; | ||
| } | ||
|
|
||
| public Map<String, FallbackTreatment> getByFlag() { | ||
| return mByFlag; | ||
| } | ||
|
|
||
| /** | ||
| * Creates a new {@link Builder} for {@link FallbackConfiguration}. | ||
| * Use this to provide an optional global fallback and flag-specific fallbacks. | ||
| */ | ||
| public static Builder builder() { | ||
| return new Builder(); | ||
| } | ||
|
|
||
| public static final class Builder { | ||
| @Nullable | ||
| private FallbackTreatment mGlobal; | ||
| @Nullable | ||
| private Map<String, FallbackTreatment> mByFlag; | ||
|
|
||
| private Builder() { | ||
| mGlobal = null; | ||
| mByFlag = null; | ||
| } | ||
|
|
||
| /** | ||
| * 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 global optional global {@link FallbackTreatment} | ||
| * @return this builder instance | ||
| */ | ||
| public Builder global(@Nullable FallbackTreatment global) { | ||
| mGlobal = global; | ||
| 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 {@link FallbackTreatment}; may be null or empty | ||
| * @return this builder instance | ||
| */ | ||
| public Builder byFlag(@Nullable Map<String, FallbackTreatment> byFlag) { | ||
| mByFlag = byFlag; | ||
| return this; | ||
| } | ||
|
|
||
| /** | ||
| * Builds an immutable {@link FallbackConfiguration} snapshot of the | ||
| * configured values. | ||
| * | ||
| * @return a new immutable {@link FallbackConfiguration} | ||
| */ | ||
| public FallbackConfiguration build() { | ||
| return new FallbackConfiguration(mGlobal, mByFlag); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (o == null || getClass() != o.getClass()) return false; | ||
| FallbackConfiguration that = (FallbackConfiguration) o; | ||
| return Objects.equals(mGlobal, that.mGlobal) && | ||
| Objects.equals(mByFlag, that.mByFlag); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(mGlobal, mByFlag); | ||
| } | ||
| } |
55 changes: 55 additions & 0 deletions
55
src/main/java/io/split/android/client/fallback/FallbackTreatment.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| package io.split.android.client.fallback; | ||
|
|
||
| import androidx.annotation.NonNull; | ||
| import androidx.annotation.Nullable; | ||
|
|
||
| import java.util.Objects; | ||
|
|
||
| /** | ||
| * Represents the fallback treatment, with an optional config and a fixed label. | ||
| */ | ||
| public final class FallbackTreatment { | ||
|
|
||
| public static final String LABEL = "fallback treatment"; | ||
|
|
||
| @NonNull | ||
| private final String mTreatment; | ||
| @Nullable | ||
| private final String mConfig; | ||
|
|
||
| public FallbackTreatment(@NonNull String treatment) { | ||
| this(treatment, null); | ||
| } | ||
|
|
||
| public FallbackTreatment(@NonNull String treatment, @Nullable String config) { | ||
| mTreatment = treatment; | ||
| mConfig = config; | ||
| } | ||
|
|
||
| public String getTreatment() { | ||
| return mTreatment; | ||
| } | ||
|
|
||
| @Nullable | ||
| public String getConfig() { | ||
| return mConfig; | ||
| } | ||
|
|
||
| public String getLabel() { | ||
| return LABEL; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (o == null || getClass() != o.getClass()) return false; | ||
| FallbackTreatment that = (FallbackTreatment) o; | ||
| return Objects.equals(mTreatment, that.mTreatment) && | ||
| Objects.equals(mConfig, that.mConfig); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(mTreatment, mConfig); | ||
| } | ||
| } |
51 changes: 51 additions & 0 deletions
51
src/main/java/io/split/android/client/fallback/FallbackTreatmentsConfiguration.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| package io.split.android.client.fallback; | ||
|
|
||
| import androidx.annotation.Nullable; | ||
| import androidx.annotation.VisibleForTesting; | ||
|
|
||
| public final class FallbackTreatmentsConfiguration { | ||
|
|
||
| @Nullable | ||
| private final FallbackConfiguration mByFactory; | ||
|
|
||
| private FallbackTreatmentsConfiguration(@Nullable FallbackConfiguration byFactory) { | ||
| mByFactory = byFactory; | ||
| } | ||
|
|
||
| @Nullable | ||
| public FallbackConfiguration getByFactory() { | ||
| return mByFactory; | ||
| } | ||
|
|
||
| public static Builder builder() { | ||
| return new Builder(); | ||
| } | ||
|
|
||
| public static final class Builder { | ||
| @Nullable | ||
| private FallbackConfiguration mByFactory; | ||
| private FallbacksSanitizer mSanitizer; | ||
|
|
||
| private Builder() { | ||
| mSanitizer = new FallbacksSanitizerImpl(); | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| Builder sanitizer(FallbacksSanitizer sanitizer) { | ||
| mSanitizer = sanitizer; | ||
| return this; | ||
| } | ||
|
|
||
| public Builder byFactory(@Nullable FallbackConfiguration byFactory) { | ||
| mByFactory = byFactory; | ||
| return this; | ||
| } | ||
|
|
||
| public FallbackTreatmentsConfiguration build() { | ||
| FallbackConfiguration sanitized = (mByFactory == null) | ||
| ? null | ||
| : mSanitizer.sanitize(mByFactory); | ||
| return new FallbackTreatmentsConfiguration(sanitized); | ||
| } | ||
| } | ||
| } |
9 changes: 9 additions & 0 deletions
9
src/main/java/io/split/android/client/fallback/FallbacksSanitizer.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| package io.split.android.client.fallback; | ||
|
|
||
| import androidx.annotation.Nullable; | ||
|
|
||
| interface FallbacksSanitizer { | ||
|
|
||
| @Nullable | ||
| FallbackConfiguration sanitize(@Nullable FallbackConfiguration config); | ||
| } |
98 changes: 98 additions & 0 deletions
98
src/main/java/io/split/android/client/fallback/FallbacksSanitizerImpl.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| package io.split.android.client.fallback; | ||
|
|
||
| import androidx.annotation.Nullable; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| import io.split.android.client.utils.logger.Logger; | ||
|
|
||
| /** | ||
| * Validates and sanitizes fallback configurations by applying validation rules. | ||
| * Invalid entries are dropped and warnings are logged. | ||
| */ | ||
| class FallbacksSanitizerImpl implements FallbacksSanitizer { | ||
|
|
||
| private static final int MAX_FLAG_NAME_LENGTH = 100; | ||
| private static final int MAX_TREATMENT_LENGTH = 100; | ||
|
|
||
| /** | ||
| * Sanitizes the provided fallback configuration by applying validation rules. | ||
| * Invalid entries are dropped and warnings are logged. | ||
| * | ||
| * @param config the configuration to sanitize; may be null | ||
| * @return a new sanitized configuration, or null if input was null | ||
| */ | ||
| @Nullable | ||
| public FallbackConfiguration sanitize(@Nullable FallbackConfiguration config) { | ||
| if (config == null) { | ||
| return null; | ||
| } | ||
|
|
||
| // Sanitize global treatment | ||
| FallbackTreatment sanitizedGlobal = sanitizeGlobalTreatment(config.getGlobal()); | ||
|
|
||
| // Sanitize by-flag treatments | ||
| Map<String, FallbackTreatment> sanitizedByFlag = sanitizeByFlagTreatments(config.getByFlag()); | ||
|
|
||
| return FallbackConfiguration.builder() | ||
| .global(sanitizedGlobal) | ||
| .byFlag(sanitizedByFlag) | ||
| .build(); | ||
| } | ||
|
|
||
| @Nullable | ||
| private FallbackTreatment sanitizeGlobalTreatment(@Nullable FallbackTreatment global) { | ||
| if (global == null) { | ||
| return null; | ||
| } | ||
|
|
||
| if (!isValidTreatment(global)) { | ||
| Logger.w("Discarded global fallback: Invalid treatment (max " + MAX_TREATMENT_LENGTH + " chars)"); | ||
| return null; | ||
| } | ||
|
|
||
| return global; | ||
| } | ||
|
|
||
| private Map<String, FallbackTreatment> sanitizeByFlagTreatments(Map<String, FallbackTreatment> byFlag) { | ||
| if (byFlag == null || byFlag.isEmpty()) { | ||
| return new HashMap<>(); | ||
| } | ||
|
|
||
| Map<String, FallbackTreatment> sanitized = new HashMap<>(); | ||
|
|
||
| for (Map.Entry<String, FallbackTreatment> entry : byFlag.entrySet()) { | ||
| String flagName = entry.getKey(); | ||
| FallbackTreatment treatment = entry.getValue(); | ||
|
|
||
| if (!isValidFlagName(flagName)) { | ||
| Logger.w("Discarded flag '" + flagName + "': Invalid flag name (max " + MAX_FLAG_NAME_LENGTH + " chars, no spaces)"); | ||
| continue; | ||
| } | ||
|
|
||
| if (!isValidTreatment(treatment)) { | ||
| Logger.w("Discarded treatment for flag '" + flagName + "': Invalid treatment (max " + MAX_TREATMENT_LENGTH + " chars)"); | ||
| continue; | ||
| } | ||
|
|
||
| sanitized.put(flagName, treatment); | ||
| } | ||
|
|
||
| return sanitized; | ||
| } | ||
|
|
||
| private static boolean isValidFlagName(String flagName) { | ||
| if (flagName == null) { | ||
| return false; | ||
| } | ||
| return flagName.length() <= MAX_FLAG_NAME_LENGTH && !flagName.contains(" "); | ||
| } | ||
|
|
||
| private static boolean isValidTreatment(FallbackTreatment treatment) { | ||
| if (treatment == null || treatment.getTreatment() == null) { | ||
| return false; | ||
| } | ||
| return treatment.getTreatment().length() <= MAX_TREATMENT_LENGTH; | ||
| } | ||
| } | ||
72 changes: 72 additions & 0 deletions
72
src/test/java/io/split/android/client/fallback/FallbackConfigurationTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| package io.split.android.client.fallback; | ||
|
|
||
| import static org.junit.Assert.assertEquals; | ||
| import static org.junit.Assert.assertNotEquals; | ||
| import static org.junit.Assert.assertSame; | ||
|
|
||
| import org.junit.Test; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| public class FallbackConfigurationTest { | ||
|
|
||
| @Test | ||
| public void constructorSetsFields() { | ||
| FallbackTreatment global = new FallbackTreatment("off"); | ||
| Map<String, FallbackTreatment> map = new HashMap<>(); | ||
| map.put("flagA", new FallbackTreatment("off")); | ||
|
|
||
| FallbackConfiguration cfg = FallbackConfiguration.builder() | ||
| .global(global) | ||
| .byFlag(map) | ||
| .build(); | ||
|
|
||
| assertSame(global, cfg.getGlobal()); | ||
| assertEquals(1, cfg.getByFlag().size()); | ||
| assertEquals("off", cfg.getByFlag().get("flagA").getTreatment()); | ||
| } | ||
|
|
||
| @Test | ||
| public void byFlagIsUnmodifiable() { | ||
| FallbackTreatment global = new FallbackTreatment("off"); | ||
| Map<String, FallbackTreatment> byFlag = new HashMap<>(); | ||
| byFlag.put("flagA", new FallbackTreatment("off")); | ||
|
|
||
| FallbackConfiguration config = FallbackConfiguration.builder() | ||
| .global(global) | ||
| .byFlag(byFlag) | ||
| .build(); | ||
|
|
||
| byFlag.put("flagB", new FallbackTreatment("on")); | ||
|
|
||
| // config map must not change | ||
| assertEquals(1, config.getByFlag().size()); | ||
|
|
||
| try { | ||
| config.getByFlag().put("x", new FallbackTreatment("on")); | ||
| throw new AssertionError("Map should be unmodifiable"); | ||
| } catch (UnsupportedOperationException expected) { | ||
|
|
||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void equalityAndHashCodeByValue() { | ||
| FallbackTreatment global = new FallbackTreatment("off"); | ||
| Map<String, FallbackTreatment> a = new HashMap<>(); | ||
| a.put("flagA", new FallbackTreatment("off")); | ||
|
|
||
| Map<String, FallbackTreatment> b = new HashMap<>(); | ||
| b.put("flagA", new FallbackTreatment("off")); | ||
|
|
||
| FallbackConfiguration configOne = FallbackConfiguration.builder().global(global).byFlag(a).build(); | ||
| FallbackConfiguration configTwo = FallbackConfiguration.builder().global(global).byFlag(b).build(); | ||
| FallbackConfiguration configThree = FallbackConfiguration.builder().global(null).byFlag(b).build(); | ||
|
|
||
| assertEquals(configOne, configTwo); | ||
| assertEquals(configOne.hashCode(), configTwo.hashCode()); | ||
| assertNotEquals(configOne, configThree); | ||
| assertNotEquals(configOne.hashCode(), configThree.hashCode()); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to match the UI for ff and treatment names, here is the message I see in UI:
Please rename your split. Split names must start with a letter and can contain "-_ a-z A-Z 0-9"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I'll have to double check how these values impact posted impressions.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use this regex:
^[a-zA-Z][a-zA-Z0-9-_$;]+$
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBD, we'll tighten the definition in the coming days.