Merge pull request #435 from splitio/SDKS-7516

[SDKS-7516] Create FlagSetSanitizer to validate flag sets

client/src/main/java/io/split/client/SplitClientConfig.java

@@ -10,12 +10,14 @@
 import pluggable.CustomStorageWrapper;
 
 import java.io.IOException;
-import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.io.InputStream;
 import java.util.Properties;
 import java.util.concurrent.ThreadFactory;
 
+import static io.split.inputValidation.FlagSetsValidator.cleanup;
+
 /**
  * Configurations for the SplitClient.
  *
@@ -86,7 +88,7 @@ public class SplitClientConfig {
     // To be set during startup
     public static String splitSdkVersion;
     private final long _lastSeenCacheSize;
-    private final List<String> _flagSetsFilter;
+    private final HashSet<String> _flagSetsFilter;
 
     public static Builder builder() {
         return new Builder();
@@ -142,7 +144,7 @@ private SplitClientConfig(String endpoint,
                               int filterUniqueKeysRefreshRate,
                               long lastSeenCacheSize,
                               ThreadFactory threadFactory,
-                              List<String> flagSetsFilter) {
+                              HashSet<String> flagSetsFilter) {
         _endpoint = endpoint;
         _eventsEndpoint = eventsEndpoint;
         _featuresRefreshRate = pollForFeatureChangesEveryNSeconds;
@@ -195,7 +197,6 @@ private SplitClientConfig(String endpoint,
         _threadFactory = threadFactory;
         _flagSetsFilter = flagSetsFilter;
 
-
         Properties props = new Properties();
         try {
             props.load(this.getClass().getClassLoader().getResourceAsStream("splitversion.properties"));
@@ -383,7 +384,7 @@ public long getLastSeenCacheSize() {
     public ThreadFactory getThreadFactory() {
         return _threadFactory;
     }
-    public List<String> getSetsFilter() {
+    public HashSet<String> getSetsFilter() {
         return _flagSetsFilter;
     }
 
@@ -442,7 +443,7 @@ public static final class Builder {
         private StorageMode _storageMode = StorageMode.MEMORY;
         private final long _lastSeenCacheSize = 500000;
         private ThreadFactory _threadFactory;
-        private List<String> _flagSetsFilter = Collections.emptyList();
+        private HashSet<String> _flagSetsFilter = new HashSet<>();
 
         public Builder() {
         }
@@ -921,8 +922,7 @@ public Builder customStorageWrapper(CustomStorageWrapper customStorageWrapper) {
          * @return this builder
          */
         public Builder flagSetsFilter(List<String> flagSetsFilter) {
-            _flagSetsFilter = flagSetsFilter;
-            //TODO Apply validations
+            _flagSetsFilter = cleanup(flagSetsFilter);
             return this;
         }
 

client/src/main/java/io/split/inputValidation/FlagSetsValidResult.java

@@ -0,0 +1,21 @@
+package io.split.inputValidation;
+
+import java.util.HashSet;
+
+public class FlagSetsValidResult {
+    private final Boolean _valid;
+    private final HashSet<String> _flagSets;
+
+    public FlagSetsValidResult(Boolean valid, HashSet<String> flagSets) {
+        _valid = valid;
+        _flagSets = flagSets;
+    }
+
+    public Boolean getValid() {
+        return _valid;
+    }
+
+    public HashSet<String> getFlagSets() {
+        return _flagSets;
+    }
+}

client/src/main/java/io/split/inputValidation/FlagSetsValidator.java

@@ -0,0 +1,49 @@
+package io.split.inputValidation;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.HashSet;
+import java.util.List;
+import java.util.regex.Pattern;
+
+public final class FlagSetsValidator {
+
+    private static final String FLAG_SET_REGEX = "^[a-z0-9][_a-z0-9]{0,49}$";
+    private static final Logger _log = LoggerFactory.getLogger(FlagSetsValidator.class);
+
+    private FlagSetsValidator() {
+        throw new IllegalStateException("Utility class");
+    }
+
+    public static HashSet<String> cleanup(List<String> flagSets) {
+        if (flagSets == null || flagSets.isEmpty()) {
+            return new HashSet<>();
+        }
+        HashSet<String> cleanFlagSets = new HashSet<>();
+        for (String flagSet: flagSets) {
+            if(flagSet != flagSet.toLowerCase()) {
+                _log.warn(String.format("Flag Set name %s should be all lowercase - converting string to lowercase", flagSet));
+                flagSet = flagSet.toLowerCase();
+            }
+            if (flagSet.trim() != flagSet) {
+                _log.warn(String.format("Flag Set name %s has extra whitespace, trimming", flagSet));
+                flagSet = flagSet.trim();
+            }
+            if (!Pattern.matches(FLAG_SET_REGEX, flagSet)) {
+                _log.warn(String.format("you passed %s, Flag Set must adhere to the regular expressions %s. This means an Flag Set must be " +
+                        "start with a letter, be in lowercase, alphanumeric and have a max length of 50 characters. %s was discarded.",
+                        flagSet, FLAG_SET_REGEX, flagSet));
+                continue;
+            }
+            cleanFlagSets.add(flagSet);
+        }
+        return cleanFlagSets;
+    }
+
+    public static FlagSetsValidResult areValid(List<String> flagSets) {
+        HashSet<String> cleanFlagSets = cleanup(flagSets);
+
+        return new FlagSetsValidResult(cleanFlagSets != null && cleanFlagSets.size() != 0, cleanFlagSets);
+    }
+}

client/src/test/java/io/split/client/SplitClientConfigTest.java

@@ -195,9 +195,9 @@ public void checkDefaultRateForFeatureAndSegment() {
 
     @Test
     public void checkSetFlagSetsFilter() {
-        List<String> sets = Stream.of("test1", "test2").collect(Collectors.toList());
+        List<String> sets = Stream.of("test1", "test2", "TEST3", "test-4").collect(Collectors.toList());
         SplitClientConfig config = SplitClientConfig.builder().flagSetsFilter(sets).build();
         Assert.assertNotNull(config.getSetsFilter());
-        Assert.assertEquals(2, config.getSetsFilter().size());
+        Assert.assertEquals(3, config.getSetsFilter().size());
     }
 }

client/src/test/java/io/split/client/utils/FlagSetsValidatorTest.java

@@ -0,0 +1,76 @@
+package io.split.client.utils;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+
+import static io.split.inputValidation.FlagSetsValidator.cleanup;
+import static io.split.inputValidation.FlagSetsValidator.areValid;
+
+public class FlagSetsValidatorTest {
+
+    @Test
+    public void testEmptyFlagSets() {
+        List<String> flagSets = new ArrayList<>();
+        HashSet<String> cleanFlagSets = cleanup(flagSets);
+        Assert.assertTrue(cleanFlagSets.isEmpty());
+    }
+
+    @Test
+    public void testUpperFlagSets() {
+        List<String> flagSets = new ArrayList<>();
+        flagSets.add("Test1");
+        flagSets.add("TEST2");
+        HashSet<String> cleanFlagSets = cleanup(flagSets);
+        Assert.assertTrue(cleanFlagSets.contains("test1"));
+        Assert.assertTrue(cleanFlagSets.contains("test2"));
+    }
+
+    @Test
+    public void testTrimFlagSets() {
+        List<String> flagSets = new ArrayList<>();
+        flagSets.add(" test1");
+        flagSets.add(" test2 ");
+        HashSet<String> cleanFlagSets = cleanup(flagSets);
+        Assert.assertTrue(cleanFlagSets.contains("test1"));
+        Assert.assertTrue(cleanFlagSets.contains("test2"));
+    }
+
+    @Test
+    public void testRegexFlagSets() {
+        List<String> flagSets = new ArrayList<>();
+        flagSets.add(" test1");
+        flagSets.add(" test-2 ");
+        HashSet<String> cleanFlagSets = cleanup(flagSets);
+        Assert.assertEquals(1, cleanFlagSets.size());
+        Assert.assertTrue(cleanFlagSets.contains("test1"));
+        Assert.assertFalse(cleanFlagSets.contains("test-2"));
+    }
+
+    @Test
+    public void testDuplicateFlagSets() {
+        List<String> flagSets = new ArrayList<>();
+        flagSets.add(" test1");
+        flagSets.add(" test1 ");
+        HashSet<String> cleanFlagSets = cleanup(flagSets);
+        Assert.assertEquals(1, cleanFlagSets.size());
+        Assert.assertTrue(cleanFlagSets.contains("test1"));
+    }
+
+    @Test
+    public void testIsValid(){
+        Assert.assertTrue(areValid(Arrays.asList(" test1 ")).getValid());
+        Assert.assertTrue(areValid(Arrays.asList("Test1 ")).getValid());
+    }
+
+    @Test
+    public void testIsNotValid(){
+        Assert.assertFalse(areValid(Arrays.asList(" test 1 ")).getValid());
+        Assert.assertFalse(areValid(Arrays.asList("")).getValid());
+        Assert.assertFalse(areValid(null).getValid());
+    }
+}