Merge pull request #558 from splitio/rbs-fix-exlcuded-arrays

fix empty arrays in excluded

Author: chillaq

client/src/main/java/io/split/client/utils/RuleBasedSegmentProcessor.java

@@ -22,10 +22,7 @@ public static RuleBasedSegmentsToUpdate processRuleBasedSegmentChanges(RuleBased
         List<String> toRemove = new ArrayList<>();
         Set<String> segments = new HashSet<>();
         for (RuleBasedSegment ruleBasedSegment : ruleBasedSegments) {
-            if (ruleBasedSegment.excluded == null)
-            {
-                ruleBasedSegment.excluded = createEmptyExcluded();
-            }
+            ruleBasedSegment.excluded = checkExcluded(ruleBasedSegment.excluded);
             if (ruleBasedSegment.status != Status.ACTIVE) {
                 // archive.
                 toRemove.add(ruleBasedSegment.name);
@@ -49,4 +46,16 @@ private static Excluded createEmptyExcluded() {
         return excluded;
     }
 
+    private static Excluded checkExcluded(Excluded excluded) {
+        if (excluded == null) {
+            excluded = createEmptyExcluded();
+        }
+        if (excluded.segments == null) {
+            excluded.segments = new ArrayList<>();
+        }
+        if (excluded.keys == null) {
+            excluded.keys = new ArrayList<>();
+        }
+        return excluded;
+    }
 }

client/src/test/java/io/split/engine/experiments/ParsedRuleBasedSegmentTest.java

@@ -46,5 +46,37 @@ public void worksWithoutExcluded() {
         RuleBasedSegmentsToUpdate toUpdate = processRuleBasedSegmentChanges(parser, change.ruleBasedSegments.d);
         Assert.assertTrue(toUpdate.getToAdd().get(0).excludedKeys().isEmpty());
         Assert.assertTrue(toUpdate.getToAdd().get(0).excludedSegments().isEmpty());
+
+        load = "{\"ff\":{\"s\":-1,\"t\":-1,\"d\":[]},\"rbs\":{\"s\":-1,\"t\":1457726098069,\"d\":[{ \"changeNumber\": 123, \"trafficTypeName\": \"user\", \"name\": \"some_name\","
+                + "\"status\": \"ACTIVE\",\"excluded\":{\"segments\":[\"segment1\"]},\"conditions\": [{\"contitionType\": \"ROLLOUT\","
+                + "\"label\": \"some_label\", \"matcherGroup\": { \"matchers\": [{ \"matcherType\": \"ALL_KEYS\", \"negate\": false}],"
+                + "\"combiner\": \"AND\"}}]}]}}";
+        change = Json.fromJson(load, SplitChange.class);
+        toUpdate = processRuleBasedSegmentChanges(parser, change.ruleBasedSegments.d);
+        Assert.assertTrue(toUpdate.getToAdd().get(0).excludedKeys().isEmpty());
+
+        load = "{\"ff\":{\"s\":-1,\"t\":-1,\"d\":[]},\"rbs\":{\"s\":-1,\"t\":1457726098069,\"d\":[{ \"changeNumber\": 123, \"trafficTypeName\": \"user\", \"name\": \"some_name\","
+                + "\"status\": \"ACTIVE\",\"excluded\":{\"segments\":[\"segment1\"], \"keys\":null},\"conditions\": [{\"contitionType\": \"ROLLOUT\","
+                + "\"label\": \"some_label\", \"matcherGroup\": { \"matchers\": [{ \"matcherType\": \"ALL_KEYS\", \"negate\": false}],"
+                + "\"combiner\": \"AND\"}}]}]}}";
+        change = Json.fromJson(load, SplitChange.class);
+        toUpdate = processRuleBasedSegmentChanges(parser, change.ruleBasedSegments.d);
+        Assert.assertTrue(toUpdate.getToAdd().get(0).excludedKeys().isEmpty());
+
+        load = "{\"ff\":{\"s\":-1,\"t\":-1,\"d\":[]},\"rbs\":{\"s\":-1,\"t\":1457726098069,\"d\":[{ \"changeNumber\": 123, \"trafficTypeName\": \"user\", \"name\": \"some_name\","
+                + "\"status\": \"ACTIVE\",\"excluded\":{\"keys\":[\"key1\"]},\"conditions\": [{\"contitionType\": \"ROLLOUT\","
+                + "\"label\": \"some_label\", \"matcherGroup\": { \"matchers\": [{ \"matcherType\": \"ALL_KEYS\", \"negate\": false}],"
+                + "\"combiner\": \"AND\"}}]}]}}";
+        change = Json.fromJson(load, SplitChange.class);
+        toUpdate = processRuleBasedSegmentChanges(parser, change.ruleBasedSegments.d);
+        Assert.assertTrue(toUpdate.getToAdd().get(0).excludedSegments().isEmpty());
+
+        load = "{\"ff\":{\"s\":-1,\"t\":-1,\"d\":[]},\"rbs\":{\"s\":-1,\"t\":1457726098069,\"d\":[{ \"changeNumber\": 123, \"trafficTypeName\": \"user\", \"name\": \"some_name\","
+                + "\"status\": \"ACTIVE\",\"excluded\":{\"segments\":null, \"keys\":[\"key1\"]},\"conditions\": [{\"contitionType\": \"ROLLOUT\","
+                + "\"label\": \"some_label\", \"matcherGroup\": { \"matchers\": [{ \"matcherType\": \"ALL_KEYS\", \"negate\": false}],"
+                + "\"combiner\": \"AND\"}}]}]}}";
+        change = Json.fromJson(load, SplitChange.class);
+        toUpdate = processRuleBasedSegmentChanges(parser, change.ruleBasedSegments.d);
+        Assert.assertTrue(toUpdate.getToAdd().get(0).excludedSegments().isEmpty());
     }
 }