polishing

chillaq · chillaq · commit 8d67468d0138 · 2025-05-27T15:17:15.000-07:00
diff --git a/client/src/main/java/io/split/Spec.java b/client/src/main/java/io/split/Spec.java
@@ -6,7 +6,6 @@ private Spec() {
         // restrict instantiation
     }
 
-    // TODO: Change the schema to 1.3 when updating splitclient
     public static final String SPEC_1_3 = "1.3";
     public static final String SPEC_1_1 = "1.1";
 }
diff --git a/client/src/main/java/io/split/client/HttpSplitChangeFetcher.java b/client/src/main/java/io/split/client/HttpSplitChangeFetcher.java
@@ -33,7 +33,6 @@
 public final class HttpSplitChangeFetcher implements SplitChangeFetcher {
     private static final Logger _log = LoggerFactory.getLogger(HttpSplitChangeFetcher.class);
 
-    private final Object _lock = new Object();
     private static final String SINCE = "since";
     private static final String RB_SINCE = "rbSince";
     private static final String TILL = "till";
diff --git a/client/src/main/java/io/split/client/JsonLocalhostSplitChangeFetcher.java b/client/src/main/java/io/split/client/JsonLocalhostSplitChangeFetcher.java
@@ -80,10 +80,10 @@ private SplitChange processSplitChange(SplitChange splitChange, long changeNumbe
         return splitChangeToProcess;
     }
 
-    private byte[] getStringDigest(String Json) throws NoSuchAlgorithmException {
+    private byte[] getStringDigest(String json) throws NoSuchAlgorithmException {
         MessageDigest digest = MessageDigest.getInstance("SHA-1");
         digest.reset();
-        digest.update(Json.getBytes());
+        digest.update(json.getBytes());
         // calculate the json sha
         return digest.digest();
     }
diff --git a/client/src/main/java/io/split/client/LegacyLocalhostSplitChangeFetcher.java b/client/src/main/java/io/split/client/LegacyLocalhostSplitChangeFetcher.java
@@ -19,6 +19,7 @@
 import java.io.FileReader;
 import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Optional;
 
 public class LegacyLocalhostSplitChangeFetcher implements SplitChangeFetcher {
@@ -70,12 +71,7 @@ public SplitChange fetch(long since, long sinceRBS, FetchOptions options) {
                 split.trafficAllocation = LocalhostConstants.SIZE_100;
                 split.trafficAllocationSeed = LocalhostConstants.SIZE_1;
 
-                Condition condition;
-                if (featureTreatment.length == 2) {
-                    condition = LocalhostSanitizer.createCondition(null, featureTreatment[1]);
-                } else {
-                    condition = LocalhostSanitizer.createCondition(featureTreatment[2], featureTreatment[1]);
-                }
+                Condition condition = checkCondition(featureTreatment);
                 if(condition.conditionType != ConditionType.ROLLOUT){
                     split.conditions.add(0, condition);
                 } else {
@@ -103,4 +99,14 @@ public SplitChange fetch(long since, long sinceRBS, FetchOptions options) {
             throw new IllegalStateException("Problem fetching splitChanges: " + e.getMessage(), e);
         }
     }
+
+    private Condition checkCondition(String[] featureTreatment) {
+        Condition condition;
+        if (featureTreatment.length == 2) {
+            condition = LocalhostSanitizer.createCondition(null, featureTreatment[1]);
+        } else {
+            condition = LocalhostSanitizer.createCondition(featureTreatment[2], featureTreatment[1]);
+        }
+        return condition;
+    }
 }
diff --git a/client/src/main/java/io/split/client/SplitFactoryImpl.java b/client/src/main/java/io/split/client/SplitFactoryImpl.java
@@ -403,7 +403,7 @@ protected SplitFactoryImpl(SplitClientConfig config) {
         SegmentCache segmentCache = new SegmentCacheInMemoryImpl();
         FlagSetsFilter flagSetsFilter = new FlagSetsFilterImpl(config.getSetsFilter());
         SplitCache splitCache = new InMemoryCacheImp(flagSetsFilter);
-        RuleBasedSegmentCache _ruleBasedSegmentCache = new RuleBasedSegmentCacheInMemoryImp();
+        RuleBasedSegmentCache ruleBasedSegmentCache = new RuleBasedSegmentCacheInMemoryImp();
         _splitCache = splitCache;
         _gates = new SDKReadinessGates();
         _segmentCache = segmentCache;
@@ -422,15 +422,15 @@ protected SplitFactoryImpl(SplitClientConfig config) {
                 _telemetryStorageProducer,
                 _splitCache,
                 config.getThreadFactory(),
-                _ruleBasedSegmentCache);
+                ruleBasedSegmentCache);
 
         // SplitFetcher
         SplitChangeFetcher splitChangeFetcher = createSplitChangeFetcher(config);
         SplitParser splitParser = new SplitParser();
         RuleBasedSegmentParser ruleBasedSegmentParser = new RuleBasedSegmentParser();
 
         _splitFetcher = new SplitFetcherImp(splitChangeFetcher, splitParser, splitCache, _telemetryStorageProducer,
-                flagSetsFilter, ruleBasedSegmentParser, _ruleBasedSegmentCache);
+                flagSetsFilter, ruleBasedSegmentParser, ruleBasedSegmentCache);
 
         // SplitSynchronizationTask
         _splitSynchronizationTask = new SplitSynchronizationTask(_splitFetcher, splitCache,
@@ -442,7 +442,7 @@ protected SplitFactoryImpl(SplitClientConfig config) {
                 _impressionsManager, null, null, null);
 
         // Evaluator
-        _evaluator = new EvaluatorImp(splitCache, segmentCache, _ruleBasedSegmentCache);
+        _evaluator = new EvaluatorImp(splitCache, segmentCache, ruleBasedSegmentCache);
 
         EventsStorage eventsStorage = new NoopEventsStorageImp();
 
diff --git a/client/src/main/java/io/split/client/dtos/ChangeDto.java b/client/src/main/java/io/split/client/dtos/ChangeDto.java
@@ -1,6 +1,5 @@
 package io.split.client.dtos;
 
-import java.util.ArrayList;
 import java.util.List;
 
 public class ChangeDto<T> {
diff --git a/client/src/main/java/io/split/client/utils/LocalhostSanitizer.java b/client/src/main/java/io/split/client/utils/LocalhostSanitizer.java
@@ -30,13 +30,41 @@ private LocalhostSanitizer() {
     }
 
     public static SplitChange sanitization(SplitChange splitChange) {
-        SecureRandom random = new SecureRandom();
-        List<Split> splitsToRemove = new ArrayList<>();
-        List<RuleBasedSegment> ruleBasedSegmentsToRemove = new ArrayList<>();
         splitChange = sanitizeTillAndSince(splitChange);
+        splitChange.featureFlags.d = sanitizeFeatureFlags(splitChange.featureFlags.d);
+        splitChange.ruleBasedSegments.d = sanitizeRuleBasedSegments(splitChange.ruleBasedSegments.d);
+
+        return splitChange;
+    }
+
+    private static List<RuleBasedSegment> sanitizeRuleBasedSegments(List<RuleBasedSegment> ruleBasedSegments) {
+        List<RuleBasedSegment> ruleBasedSegmentsToRemove = new ArrayList<>();
+        if (ruleBasedSegments != null) {
+            for (RuleBasedSegment ruleBasedSegment : ruleBasedSegments) {
+                if (ruleBasedSegment.name == null) {
+                    ruleBasedSegmentsToRemove.add(ruleBasedSegment);
+                    continue;
+                }
+                ruleBasedSegment.trafficTypeName = sanitizeIfNullOrEmpty(ruleBasedSegment.trafficTypeName, LocalhostConstants.USER);
+                ruleBasedSegment.status = sanitizeStatus(ruleBasedSegment.status);
+                ruleBasedSegment.changeNumber = sanitizeChangeNumber(ruleBasedSegment.changeNumber, 0);
+                ruleBasedSegment.conditions = sanitizeConditions((ArrayList<Condition>) ruleBasedSegment.conditions, false,
+                        ruleBasedSegment.trafficTypeName);
+                ruleBasedSegment.excluded.segments = sanitizeExcluded((ArrayList) ruleBasedSegment.excluded.segments);
+                ruleBasedSegment.excluded.keys = sanitizeExcluded((ArrayList) ruleBasedSegment.excluded.keys);
+            }
+            ruleBasedSegments.removeAll(ruleBasedSegmentsToRemove);
+        } else {
+            ruleBasedSegments = new ArrayList<>();
+        }
+        return ruleBasedSegments;
+    }
 
-        if (splitChange.featureFlags.d != null) {
-            for (Split split : splitChange.featureFlags.d) {
+    private static List<Split> sanitizeFeatureFlags(List<Split> featureFlags) {
+        List<Split> splitsToRemove = new ArrayList<>();
+        SecureRandom random = new SecureRandom();
+        if (featureFlags != null) {
+            for (Split split : featureFlags) {
                 if (split.name == null) {
                     splitsToRemove.add(split);
                     continue;
@@ -60,31 +88,11 @@ public static SplitChange sanitization(SplitChange splitChange) {
                 }
                 split.conditions = sanitizeConditions((ArrayList<Condition>) split.conditions, false, split.trafficTypeName);
             }
-            splitChange.featureFlags.d.removeAll(splitsToRemove);
+            featureFlags.removeAll(splitsToRemove);
         } else {
-            splitChange.featureFlags.d = new ArrayList<>();
+            featureFlags = new ArrayList<>();
         }
-
-        if (splitChange.ruleBasedSegments.d != null) {
-            for (RuleBasedSegment ruleBasedSegment : splitChange.ruleBasedSegments.d) {
-                if (ruleBasedSegment.name == null) {
-                    ruleBasedSegmentsToRemove.add(ruleBasedSegment);
-                    continue;
-                }
-                ruleBasedSegment.trafficTypeName = sanitizeIfNullOrEmpty(ruleBasedSegment.trafficTypeName, LocalhostConstants.USER);
-                ruleBasedSegment.status = sanitizeStatus(ruleBasedSegment.status);
-                ruleBasedSegment.changeNumber = sanitizeChangeNumber(ruleBasedSegment.changeNumber, 0);
-                ruleBasedSegment.conditions = sanitizeConditions((ArrayList<Condition>) ruleBasedSegment.conditions, false,
-                        ruleBasedSegment.trafficTypeName);
-                ruleBasedSegment.excluded.segments = sanitizeExcluded((ArrayList) ruleBasedSegment.excluded.segments);
-                ruleBasedSegment.excluded.keys = sanitizeExcluded((ArrayList) ruleBasedSegment.excluded.keys);
-            }
-            splitChange.ruleBasedSegments.d.removeAll(ruleBasedSegmentsToRemove);
-        } else {
-            splitChange.ruleBasedSegments.d = new ArrayList<>();
-        }
-
-        return splitChange;
+        return featureFlags;
     }
 
     private static ArrayList<Condition> sanitizeConditions(ArrayList<Condition> conditions, boolean createPartition, String trafficTypeName) {
diff --git a/client/src/main/java/io/split/client/utils/RuleBasedSegmentProcessor.java b/client/src/main/java/io/split/client/utils/RuleBasedSegmentProcessor.java
@@ -16,6 +16,10 @@
 public class RuleBasedSegmentProcessor {
     private static final Logger _log = LoggerFactory.getLogger(RuleBasedSegmentProcessor.class);
 
+    private RuleBasedSegmentProcessor() {
+        throw new IllegalStateException("Utility class");
+    }
+
     public static RuleBasedSegmentsToUpdate processRuleBasedSegmentChanges(RuleBasedSegmentParser ruleBasedSegmentParser,
                                                                            List<RuleBasedSegment> ruleBasedSegments) {
         List<ParsedRuleBasedSegment> toAdd = new ArrayList<>();
@@ -31,10 +35,10 @@ public static RuleBasedSegmentsToUpdate processRuleBasedSegmentChanges(RuleBased
             ParsedRuleBasedSegment parsedRuleBasedSegment = ruleBasedSegmentParser.parse(ruleBasedSegment);
             if (parsedRuleBasedSegment == null) {
                 _log.debug(String.format("We could not parse the rule based segment definition for: %s", ruleBasedSegment.name));
-                continue;
+            } else {
+                segments.addAll(parsedRuleBasedSegment.getSegmentsNames());
+                toAdd.add(parsedRuleBasedSegment);
             }
-            segments.addAll(parsedRuleBasedSegment.getSegmentsNames());
-            toAdd.add(parsedRuleBasedSegment);
         }
         return new RuleBasedSegmentsToUpdate(toAdd, toRemove, segments);
     }
diff --git a/client/src/main/java/io/split/client/utils/RuleBasedSegmentsToUpdate.java b/client/src/main/java/io/split/client/utils/RuleBasedSegmentsToUpdate.java
@@ -1,7 +1,6 @@
 package io.split.client.utils;
 
 import io.split.engine.experiments.ParsedRuleBasedSegment;
-import io.split.engine.experiments.ParsedSplit;
 
 import java.util.List;
 import java.util.Set;
diff --git a/client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java b/client/src/main/java/io/split/engine/evaluator/EvaluatorImp.java
@@ -24,15 +24,13 @@ public class EvaluatorImp implements Evaluator {
     private static final Logger _log = LoggerFactory.getLogger(EvaluatorImp.class);
 
     private final SegmentCacheConsumer _segmentCacheConsumer;
-    private final RuleBasedSegmentCacheConsumer _ruleBasedSegmentCacheConsumer;
     private final EvaluationContext _evaluationContext;
     private final SplitCacheConsumer _splitCacheConsumer;
 
     public EvaluatorImp(SplitCacheConsumer splitCacheConsumer, SegmentCacheConsumer segmentCache,
                         RuleBasedSegmentCacheConsumer ruleBasedSegmentCacheConsumer) {
         _splitCacheConsumer = checkNotNull(splitCacheConsumer);
         _segmentCacheConsumer = checkNotNull(segmentCache);
-        _ruleBasedSegmentCacheConsumer = checkNotNull(ruleBasedSegmentCacheConsumer);
         _evaluationContext = new EvaluationContext(this, _segmentCacheConsumer, ruleBasedSegmentCacheConsumer);
     }
 
diff --git a/client/src/main/java/io/split/engine/experiments/ParserUtils.java b/client/src/main/java/io/split/engine/experiments/ParserUtils.java
@@ -32,8 +32,6 @@
 import io.split.engine.matchers.strings.EndsWithAnyOfMatcher;
 import io.split.engine.matchers.strings.ContainsAnyOfMatcher;
 import io.split.engine.matchers.strings.RegularExpressionMatcher;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import java.util.List;
 
@@ -42,9 +40,8 @@
 
 public final class ParserUtils {
 
-    private static final Logger _log = LoggerFactory.getLogger(ParserUtils.class);
-
-    public ParserUtils() {
+    private ParserUtils() {
+        throw new IllegalStateException("Utility class");
     }
 
     public static boolean checkUnsupportedMatcherExist(List<Matcher> matchers) {
@@ -58,8 +55,7 @@ public static boolean checkUnsupportedMatcherExist(List<Matcher> matchers) {
                 break;
             }
         }
-        if (typeCheck != null)  return false;
-        return true;
+        return (typeCheck == null);
     }
 
     public static ParsedCondition getTemplateCondition() {
diff --git a/client/src/main/java/io/split/engine/sse/dtos/CommonChangeNotification.java b/client/src/main/java/io/split/engine/sse/dtos/CommonChangeNotification.java
@@ -9,6 +9,7 @@
 
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
+import java.nio.charset.StandardCharsets;
 import java.util.Base64;
 import java.util.zip.DataFormatException;
 
@@ -82,6 +83,6 @@ public String toString() {
     }
 
     private void updateDefinition(byte[] decodedBytes) throws UnsupportedEncodingException {
-        definition = (Y) Json.fromJson(new String(decodedBytes, "UTF-8"), _definitionClass);
+        definition = (Y) Json.fromJson(new String(decodedBytes, StandardCharsets.UTF_8), _definitionClass);
     }
 }
diff --git a/client/src/main/java/io/split/engine/sse/workers/FeatureFlagWorkerImp.java b/client/src/main/java/io/split/engine/sse/workers/FeatureFlagWorkerImp.java
@@ -71,14 +71,14 @@ protected void executeRefresh(IncomingNotification incomingNotification) {
             changeNumber = featureFlagChangeNotification.getChangeNumber();
         } else {
             CommonChangeNotification<RuleBasedSegment> ruleBasedSegmentChangeNotification = (CommonChangeNotification) incomingNotification;
-            success = AddOrUpdateRuleBasedSegment(ruleBasedSegmentChangeNotification);
+            success = addOrUpdateRuleBasedSegment(ruleBasedSegmentChangeNotification);
             changeNumberRBS = ruleBasedSegmentChangeNotification.getChangeNumber();
         }
         if (!success)
             _synchronizer.refreshSplits(changeNumber, changeNumberRBS);
     }
 
-    private boolean AddOrUpdateRuleBasedSegment(CommonChangeNotification ruleBasedSegmentChangeNotification) {
+    private boolean addOrUpdateRuleBasedSegment(CommonChangeNotification ruleBasedSegmentChangeNotification) {
         if (ruleBasedSegmentChangeNotification.getChangeNumber() <= _ruleBasedSegmentCache.getChangeNumber()) {
             return true;
         }
diff --git a/client/src/main/java/io/split/service/SplitHttpClientImpl.java b/client/src/main/java/io/split/service/SplitHttpClientImpl.java
@@ -93,12 +93,7 @@ public SplitHttpResponse get(URI uri, FetchOptions options, Map<String, List<Str
                 _log.warn(String.format("Response status was: %s. Reason: %s", code, statusMessage));
             }
 
-            String body = "";
-            try {
-                body = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
-            } catch (Exception e) {
-                _log.warn("Error parsing Response.body", e);
-            }
+            String body = extractBodyFromResponse(response);
 
             return new SplitHttpResponse(code,
                     statusMessage,
@@ -113,6 +108,16 @@ public SplitHttpResponse get(URI uri, FetchOptions options, Map<String, List<Str
         }
     }
 
+    private String extractBodyFromResponse(CloseableHttpResponse response) {
+        String body = "";
+        try {
+            body = EntityUtils.toString(response.getEntity(), StandardCharsets.UTF_8);
+        } catch (Exception e) {
+            _log.warn("Error parsing Response.body", e);
+        }
+        return body;
+    }
+
     public SplitHttpResponse post(URI uri, String body, Map<String, List<String>> additionalHeaders)
             throws IOException {
 
diff --git a/client/src/main/java/io/split/storages/RuleBasedSegmentCacheConsumer.java b/client/src/main/java/io/split/storages/RuleBasedSegmentCacheConsumer.java
@@ -4,7 +4,6 @@
 
 import java.util.Collection;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 
 public interface RuleBasedSegmentCacheConsumer {
diff --git a/client/src/main/java/io/split/storages/memory/RuleBasedSegmentCacheInMemoryImp.java b/client/src/main/java/io/split/storages/memory/RuleBasedSegmentCacheInMemoryImp.java
@@ -14,6 +14,7 @@
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.stream.Collectors;
+import java.util.Map;
 
 public class RuleBasedSegmentCacheInMemoryImp implements RuleBasedSegmentCache {
 
@@ -65,8 +66,8 @@ public void setChangeNumber(long changeNumber) {
     @Override
     public List<String> ruleBasedSegmentNames() {
         List<String> ruleBasedSegmentNamesList = new ArrayList<>();
-        for (String key: _concurrentMap.keySet()) {
-            ruleBasedSegmentNamesList.add(_concurrentMap.get(key).ruleBasedSegment());
+        for (Map.Entry<String, ParsedRuleBasedSegment> key: _concurrentMap.entrySet()) {
+            ruleBasedSegmentNamesList.add(key.getValue().ruleBasedSegment());
         }
         return ruleBasedSegmentNamesList;
     }
@@ -103,6 +104,6 @@ public Set<String> getSegments() {
 
     @Override
     public boolean contains(Set<String> ruleBasedSegmentNames) {
-        return getSegments().contains(ruleBasedSegmentNames);
+        return getSegments().containsAll(ruleBasedSegmentNames);
     }
 }
diff --git a/client/src/main/java/io/split/storages/pluggable/adapters/UserCustomRuleBasedSegmentAdapterConsumer.java b/client/src/main/java/io/split/storages/pluggable/adapters/UserCustomRuleBasedSegmentAdapterConsumer.java
@@ -96,7 +96,7 @@ private List<ParsedRuleBasedSegment> stringsToParsedRuleBasedSegments(List<Strin
 
     @Override
     public boolean contains(Set<String> ruleBasedSegmentNames) {
-        return getSegments().contains(ruleBasedSegmentNames);
+        return getSegments().containsAll(ruleBasedSegmentNames);
     }
 
 }
diff --git a/client/src/main/java/io/split/storages/pluggable/adapters/UserCustomRuleBasedSegmentAdapterProducer.java b/client/src/main/java/io/split/storages/pluggable/adapters/UserCustomRuleBasedSegmentAdapterProducer.java
@@ -1,7 +1,5 @@
 package io.split.storages.pluggable.adapters;
 
-import io.split.client.dtos.RuleBasedSegment;
-import io.split.client.utils.Json;
 import io.split.engine.experiments.ParsedRuleBasedSegment;
 import io.split.storages.RuleBasedSegmentCacheProducer;
 import io.split.storages.pluggable.domain.PrefixAdapter;
diff --git a/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java b/client/src/test/java/io/split/client/HttpSplitChangeFetcherTest.java
diff --git a/client/src/test/java/io/split/engine/sse/NotificationParserImpTest.java b/client/src/test/java/io/split/engine/sse/NotificationParserImpTest.java

Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,6 @@ private Spec() {`
`6`	`6`	`// restrict instantiation`
`7`	`7`	`}`
`8`	`8`
`9`		`- // TODO: Change the schema to 1.3 when updating splitclient`
`10`	`9`	`public static final String SPEC_1_3 = "1.3";`
`11`	`10`	`public static final String SPEC_1_1 = "1.1";`
`12`	`11`	`}`
Original file line number	Diff line number	Diff line change
`@@ -80,10 +80,10 @@ private SplitChange processSplitChange(SplitChange splitChange, long changeNumbe`
`80`	`80`	`return splitChangeToProcess;`
`81`	`81`	`}`
`82`	`82`
`83`		`- private byte[] getStringDigest(String Json) throws NoSuchAlgorithmException {`
	`83`	`+ private byte[] getStringDigest(String json) throws NoSuchAlgorithmException {`
`84`	`84`	`MessageDigest digest = MessageDigest.getInstance("SHA-1");`
`85`	`85`	`digest.reset();`
`86`		`- digest.update(Json.getBytes());`
	`86`	`+ digest.update(json.getBytes());`
`87`	`87`	`// calculate the json sha`
`88`	`88`	`return digest.digest();`
`89`	`89`	`}`