Merge pull request #496 from splitio/unsupported-matcher-flow

Unsupported matcher flow

Author: chillaq

client/src/main/java/io/split/engine/evaluator/Labels.java

@@ -6,4 +6,5 @@ public class Labels {
     public static final String KILLED = "killed";
     public static final String DEFINITION_NOT_FOUND = "definition not found";
     public static final String EXCEPTION = "exception";
+    public static final String UNSUPPORTED_MATCHER = "unsupported matcher type";
 }

client/src/main/java/io/split/engine/experiments/SplitParser.java

@@ -6,6 +6,9 @@
 import io.split.client.dtos.MatcherGroup;
 import io.split.client.dtos.Partition;
 import io.split.client.dtos.Split;
+import io.split.client.dtos.ConditionType;
+import io.split.client.dtos.MatcherType;
+import io.split.engine.evaluator.Labels;
 import io.split.engine.matchers.AllKeysMatcher;
 import io.split.engine.matchers.AttributeMatcher;
 import io.split.engine.matchers.BetweenMatcher;
@@ -59,6 +62,12 @@ private ParsedSplit parseWithoutExceptionHandling(Split split) {
 
         for (Condition condition : split.conditions) {
             List<Partition> partitions = condition.partitions;
+            if (checkUnsupportedMatcherExist(condition.matcherGroup.matchers)) {
+                _log.error("Unsupported matcher type found for feature flag: " + split.name + " , will revert to default template matcher.");
+                parsedConditionList.clear();
+                parsedConditionList.add(getTemplateCondition());
+                break;
+            }
             CombiningMatcher matcher = toMatcher(condition.matcherGroup);
             parsedConditionList.add(new ParsedCondition(condition.conditionType, matcher, partitions, condition.label));
         }
@@ -67,6 +76,34 @@ private ParsedSplit parseWithoutExceptionHandling(Split split) {
                 split.changeNumber, split.trafficAllocation, split.trafficAllocationSeed, split.algo, split.configurations, split.sets);
     }
 
+    private boolean checkUnsupportedMatcherExist(List<io.split.client.dtos.Matcher> matchers) {
+        MatcherType typeCheck = null;
+        for (io.split.client.dtos.Matcher matcher : matchers) {
+            typeCheck = null;
+            try {
+                typeCheck = matcher.matcherType;
+            } catch (NullPointerException e) {
+                // If the exception is caught, it means unsupported matcher
+                break;
+            }
+        }
+        if (typeCheck != null)  return false;
+        return true;
+    }
+
+    private ParsedCondition getTemplateCondition() {
+        List<Partition> templatePartitions = Lists.newArrayList();
+        Partition partition = new Partition();
+        partition.treatment = "control";
+        partition.size = 100;
+        templatePartitions.add(partition);
+        return new ParsedCondition(
+                ConditionType.ROLLOUT,
+                CombiningMatcher.of(new AllKeysMatcher()),
+                templatePartitions,
+                Labels.UNSUPPORTED_MATCHER);
+    }
+
     private CombiningMatcher toMatcher(MatcherGroup matcherGroup) {
         List<io.split.client.dtos.Matcher> matchers = matcherGroup.matchers;
         checkArgument(!matchers.isEmpty());

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

@@ -1,9 +1,11 @@
 package io.split.engine.experiments;
 
 import com.google.common.collect.Lists;
+import io.split.client.utils.Json;
 import io.split.storages.SegmentCache;
 import io.split.storages.memory.SegmentCacheInMemoryImpl;
 import io.split.client.dtos.*;
+import io.split.engine.evaluator.Labels;
 import io.split.engine.ConditionsTestUtil;
 import io.split.engine.SDKReadinessGates;
 import io.split.engine.matchers.AttributeMatcher;
@@ -39,6 +41,7 @@
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 /**
  * Tests for ExperimentParser
@@ -529,6 +532,29 @@ public void contains_string() {
         set_matcher_test(c, m);
     }
 
+    @Test
+    public void unsupportedMatcher() {
+        SplitParser parser = new SplitParser();
+        String splitWithUndefinedMatcher = "{\"since\":-1,\"till\": 1457726098069,\"splits\": [{ \"changeNumber\": 123, \"trafficTypeName\": \"user\", \"name\": \"some_name\","
+                + "\"trafficAllocation\": 100, \"trafficAllocationSeed\": 123456, \"seed\": 321654, \"status\": \"ACTIVE\","
+                + "\"killed\": false, \"defaultTreatment\": \"off\", \"algo\": 2,\"conditions\": [{ \"partitions\": ["
+                + "{\"treatment\": \"on\", \"size\": 50}, {\"treatment\": \"off\", \"size\": 50}], \"contitionType\": \"ROLLOUT\","
+                + "\"label\": \"some_label\", \"matcherGroup\": { \"matchers\": [{ \"matcherType\": \"UNKNOWN\", \"negate\": false}],"
+                + "\"combiner\": \"AND\"}}], \"sets\": [\"set1\"]}]}";
+        SplitChange change = Json.fromJson(splitWithUndefinedMatcher, SplitChange.class);
+        for (Split split : change.splits) {
+            // should not cause exception
+            ParsedSplit parsedSplit = parser.parse(split);
+            for (ParsedCondition parsedCondition : parsedSplit.parsedConditions()) {
+                assertTrue(parsedCondition.label() == Labels.UNSUPPORTED_MATCHER);
+                for (AttributeMatcher matcher : parsedCondition.matcher().attributeMatchers()) {
+                    // Check the matcher is ALL_KEYS
+                    assertTrue(matcher.matcher().toString().equals(" in segment all"));
+                }
+            }
+        }
+    }
+
     public void set_matcher_test(Condition c, io.split.engine.matchers.Matcher m) {
 
 //        SegmentSynchronizationTask segmentFetcher = new SegmentSynchronizationTaskImp(fetcherMap);