Merge pull request #561 from splitio/fix-rbs-excluded

fixing excluded rbs

Author: sanzmauro

client/src/main/java/io/split/client/dtos/ExcludedSegments.java

@@ -1,6 +1,9 @@
 package io.split.client.dtos;
 
 public class ExcludedSegments {
+    static final String STANDARD_TYPE = "standard";
+    static final String RULE_BASED_TYPE = "rule-based";
+
     public ExcludedSegments() {}
     public ExcludedSegments(String type, String name) {
         this.type = type;
@@ -9,4 +12,12 @@ public ExcludedSegments(String type, String name) {
 
     public String type;
     public String name;
+
+    public boolean isStandard() {
+        return STANDARD_TYPE.equals(type);
+    }
+
+    public boolean isRuleBased() {
+        return RULE_BASED_TYPE.equals(type);
+    }
 }

client/src/main/java/io/split/engine/matchers/RuleBasedSegmentMatcher.java

@@ -18,9 +18,6 @@
  * @author adil
  */
 public class RuleBasedSegmentMatcher implements Matcher {
-    private final String standardType = "standard";
-    private final String ruleBasedType = "rule-based";
-
     private final String _segmentName;
 
     public RuleBasedSegmentMatcher(String segmentName) {
@@ -41,20 +38,29 @@ public boolean match(Object matchValue, String bucketingKey, Map<String, Object>
             return false;
         }
 
-        for (ExcludedSegments segment: parsedRuleBasedSegment.excludedSegments()) {
-            if (segment.type.equals(standardType) && evaluationContext.getSegmentCache().isInSegment(segment.name, (String) matchValue)) {
-                return false;
+        if (matchExcludedSegments(parsedRuleBasedSegment.excludedSegments(), matchValue, bucketingKey, attributes, evaluationContext)) {
+            return false;
+        }
+
+        return matchConditions(parsedRuleBasedSegment.parsedConditions(), matchValue, bucketingKey, attributes, evaluationContext);
+    }
+
+    private boolean matchExcludedSegments(List<ExcludedSegments> excludedSegments, Object matchValue, String bucketingKey,
+                                          Map<String, Object> attributes, EvaluationContext evaluationContext) {
+        for (ExcludedSegments excludedSegment: excludedSegments) {
+            if (excludedSegment.isStandard() && evaluationContext.getSegmentCache().isInSegment(excludedSegment.name, (String) matchValue)) {
+                return true;
             }
 
-            if (segment.type.equals(ruleBasedType)) {
-                List<ParsedCondition> conditions = evaluationContext.getRuleBasedSegmentCache().get(segment.name).parsedConditions();
-                if (matchConditions(conditions, matchValue, bucketingKey, attributes, evaluationContext)) {
+            if (excludedSegment.isRuleBased()) {
+                RuleBasedSegmentMatcher excludedRbsMatcher = new RuleBasedSegmentMatcher(excludedSegment.name);
+                if (excludedRbsMatcher.match(matchValue, bucketingKey, attributes, evaluationContext)) {
                     return true;
                 }
             }
         }
 
-        return matchConditions(parsedRuleBasedSegment.parsedConditions(), matchValue, bucketingKey, attributes, evaluationContext);
+        return  false;
     }
 
     private boolean matchConditions(List<ParsedCondition> conditions, Object matchValue, String bucketingKey,

client/src/test/java/io/split/engine/matchers/RuleBasedSegmentMatcherTest.java

@@ -1,7 +1,6 @@
 package io.split.engine.matchers;
 
 import com.google.common.collect.Lists;
-import com.google.common.collect.Sets;
 import io.split.client.dtos.ConditionType;
 import io.split.client.dtos.MatcherCombiner;
 import io.split.client.dtos.SplitChange;
@@ -14,11 +13,9 @@
 import io.split.engine.experiments.RuleBasedSegmentParser;
 import io.split.engine.matchers.strings.WhitelistMatcher;
 import io.split.storages.RuleBasedSegmentCache;
-import io.split.storages.RuleBasedSegmentCacheConsumer;
 import io.split.storages.SegmentCache;
 import io.split.storages.memory.RuleBasedSegmentCacheInMemoryImp;
 import io.split.storages.memory.SegmentCacheInMemoryImpl;
-import okhttp3.mockwebserver.MockResponse;
 import org.junit.Test;
 import org.mockito.Mockito;
 
@@ -29,7 +26,6 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
-import java.util.Set;
 
 import static io.split.client.utils.RuleBasedSegmentProcessor.processRuleBasedSegmentChanges;
 import static org.hamcrest.Matchers.is;
@@ -128,14 +124,14 @@ public void usingRbsInExcludedTest() throws IOException {
         RuleBasedSegmentsToUpdate ruleBasedSegmentsToUpdate = processRuleBasedSegmentChanges(ruleBasedSegmentParser,
                 change.ruleBasedSegments.d);
         ruleBasedSegmentCache.update(ruleBasedSegmentsToUpdate.getToAdd(), null, 123);
-        RuleBasedSegmentMatcher matcher = new RuleBasedSegmentMatcher("no_excludes");
+        RuleBasedSegmentMatcher matcher = new RuleBasedSegmentMatcher("sample_rule_based_segment");
         HashMap<String, Object> attrib1 =  new HashMap<String, Object>() {{
             put("email", "mauro@split.io");
         }};
         HashMap<String, Object> attrib2 =  new HashMap<String, Object>() {{
-            put("email", "bilal@split.io");
+            put("email", "bilal@harness.io");
         }};
-        assertThat(matcher.match("mauro@split.io", null, attrib1, evaluationContext), is(true));
-        assertThat(matcher.match("bilal@split.io", null, attrib2, evaluationContext), is(true));
+        assertThat(matcher.match("mauro", null, attrib1, evaluationContext), is(false));
+        assertThat(matcher.match("bilal", null, attrib2, evaluationContext), is(true));
     }
 }

client/src/test/resources/rule_base_segments2.json

@@ -23,7 +23,7 @@
               "negate": false,
               "whitelistMatcherData": {
                 "whitelist": [
-                  "@split.io"
+                  "@harness.io"
                 ]
               }
             }