From 74f6bbc5217d0965caa899b356166fb313a520ca Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Mon, 28 Apr 2025 15:45:49 -0300 Subject: [PATCH 01/17] Handle legacy SplitChange DTO --- .../client/dtos/TargetingRulesChange.java | 4 - .../rules/TargetingRulesResponseParser.java | 58 +++++++- .../TargetingRulesResponseParserTest.java | 49 +++++++ src/test/resources/split_changes_legacy.json | 121 +++++++++++++++++ src/test/resources/split_changes_small.json | 128 ++++++++++++++++++ 5 files changed, 350 insertions(+), 10 deletions(-) create mode 100644 src/test/resources/split_changes_legacy.json create mode 100644 src/test/resources/split_changes_small.json diff --git a/src/main/java/io/split/android/client/dtos/TargetingRulesChange.java b/src/main/java/io/split/android/client/dtos/TargetingRulesChange.java index 0341879c2..23cfe911c 100644 --- a/src/main/java/io/split/android/client/dtos/TargetingRulesChange.java +++ b/src/main/java/io/split/android/client/dtos/TargetingRulesChange.java @@ -1,7 +1,5 @@ package io.split.android.client.dtos; -import androidx.annotation.VisibleForTesting; - import com.google.gson.annotations.SerializedName; public class TargetingRulesChange { @@ -19,12 +17,10 @@ public RuleBasedSegmentChange getRuleBasedSegmentsChange() { return rbs; } - @VisibleForTesting public static TargetingRulesChange create(SplitChange splitChange) { return create(splitChange, RuleBasedSegmentChange.createEmpty()); } - @VisibleForTesting public static TargetingRulesChange create(SplitChange splitChange, RuleBasedSegmentChange ruleBasedSegmentChange) { TargetingRulesChange targetingRulesChange = new TargetingRulesChange(); targetingRulesChange.ff = splitChange; diff --git a/src/main/java/io/split/android/client/service/rules/TargetingRulesResponseParser.java b/src/main/java/io/split/android/client/service/rules/TargetingRulesResponseParser.java index 855019fbd..8b330fd39 100644 --- a/src/main/java/io/split/android/client/service/rules/TargetingRulesResponseParser.java +++ b/src/main/java/io/split/android/client/service/rules/TargetingRulesResponseParser.java @@ -1,7 +1,11 @@ package io.split.android.client.service.rules; -import com.google.gson.JsonSyntaxException; +import com.google.gson.stream.JsonReader; +import com.google.gson.stream.JsonToken; +import java.io.StringReader; + +import io.split.android.client.dtos.SplitChange; import io.split.android.client.dtos.TargetingRulesChange; import io.split.android.client.service.http.HttpResponseParser; import io.split.android.client.service.http.HttpResponseParserException; @@ -12,12 +16,54 @@ public class TargetingRulesResponseParser implements HttpResponseParser Date: Mon, 28 Apr 2025 15:51:33 -0300 Subject: [PATCH 02/17] Update tests --- .../TargetingRulesResponseParserTest.java | 11 +++ src/test/resources/split_changes_small.json | 78 +++++++++++++++++-- 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java b/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java index 81832cd10..4037a908f 100644 --- a/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java +++ b/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java @@ -3,6 +3,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import org.junit.Before; import org.junit.Test; @@ -29,6 +30,11 @@ public void parsesNewTargetingRulesChangeJson() throws Exception { assertNotNull(result); assertNotNull(result.getFeatureFlagsChange()); assertEquals("FACUNDO_TEST", result.getFeatureFlagsChange().splits.get(0).name); + assertEquals(1506703262916L, result.getFeatureFlagsChange().till); + assertEquals(-1, result.getFeatureFlagsChange().since); + assertEquals(1506703262920L, result.getRuleBasedSegmentsChange().getSince()); + assertEquals(1506703263000L, result.getRuleBasedSegmentsChange().getTill()); + assertEquals("mauro_rule_based_segment", result.getRuleBasedSegmentsChange().getSegments().get(0).getName()); } @Test @@ -38,6 +44,11 @@ public void parsesLegacySplitChangeJson() throws Exception { assertNotNull(result); assertNotNull(result.getFeatureFlagsChange()); assertEquals("FACUNDO_TEST", result.getFeatureFlagsChange().splits.get(0).name); + assertEquals(1506703262916L, result.getFeatureFlagsChange().till); + assertEquals(-1, result.getFeatureFlagsChange().since); + assertEquals(-1, result.getRuleBasedSegmentsChange().getSince()); + assertEquals(-1, result.getRuleBasedSegmentsChange().getTill()); + assertTrue(result.getRuleBasedSegmentsChange().getSegments().isEmpty()); } @Test diff --git a/src/test/resources/split_changes_small.json b/src/test/resources/split_changes_small.json index b99de52a9..1e2e4ff94 100644 --- a/src/test/resources/split_changes_small.json +++ b/src/test/resources/split_changes_small.json @@ -117,12 +117,80 @@ ] } ], - "since": -1, - "till": 1506703262916 + "s": -1, + "t": 1506703262916 }, "rbs": { - "d": [], - "s": -1, - "t": 1506703261916 + "d": [ + { + "changeNumber": 5, + "name": "mauro_rule_based_segment", + "status": "ACTIVE", + "trafficTypeName": "user", + "excluded": { + "keys": [ + "mauro@split.io" + ], + "segments": [ + "segment_test" + ] + }, + "conditions": [ + { + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "user" + }, + "matcherType": "WHITELIST", + "negate": false, + "whitelistMatcherData": { + "whitelist": [ + "mdp", + "tandil", + "bsas" + ] + } + }, + { + "keySelector": { + "trafficType": "user", + "attribute": "email" + }, + "matcherType": "ENDS_WITH", + "negate": false, + "whitelistMatcherData": { + "whitelist": [ + "@split.io" + ] + } + } + ] + } + }, + { + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "user" + }, + "matcherType": "IN_SEGMENT", + "negate": false, + "userDefinedSegmentMatcherData": { + "segmentName": "regular_segment" + } + } + ] + } + } + ] + } + ], + "s": 1506703262920, + "t": 1506703263000 } } \ No newline at end of file From 3495e0b77a9c0883d1d95c0bdbcd68fdf2e43d28 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Fri, 2 May 2025 12:17:35 -0300 Subject: [PATCH 03/17] Excluded segment types --- src/androidTest/assets/split_changes_rbs.json | 5 +- .../split/android/client/dtos/Excluded.java | 4 +- .../android/client/dtos/ExcludedSegment.java | 79 +++++++++++++++++++ .../experiments/ParsedRuleBasedSegment.java | 13 +-- .../engine/experiments/ParserCommons.java | 1 + .../matchers/InRuleBasedSegmentMatcher.java | 45 +++++++++-- .../TargetingRulesResponseParserTest.java | 12 +++ .../sseclient/SplitUpdateWorkerTest.java | 2 +- .../storage/cipher/ApplyCipherTaskTest.kt | 6 +- .../RuleBasedSegmentParserTest.java | 3 +- .../InRuleBasedSegmentMatcherTest.java | 58 +++++++++++++- src/test/resources/split_changes_small.json | 17 +++- 12 files changed, 221 insertions(+), 24 deletions(-) create mode 100644 src/main/java/io/split/android/client/dtos/ExcludedSegment.java diff --git a/src/androidTest/assets/split_changes_rbs.json b/src/androidTest/assets/split_changes_rbs.json index 6c1709108..253374da7 100644 --- a/src/androidTest/assets/split_changes_rbs.json +++ b/src/androidTest/assets/split_changes_rbs.json @@ -91,7 +91,10 @@ "key22" ], "segments": [ - "segment_test" + { + "name": "segment_test", + "type": "standard" + } ] }, "conditions": [ diff --git a/src/main/java/io/split/android/client/dtos/Excluded.java b/src/main/java/io/split/android/client/dtos/Excluded.java index c7929954d..caf436d12 100644 --- a/src/main/java/io/split/android/client/dtos/Excluded.java +++ b/src/main/java/io/split/android/client/dtos/Excluded.java @@ -11,9 +11,9 @@ public class Excluded { private Set mKeys; @SerializedName("segments") - private Set mSegments; + private Set mSegments; - public Set getSegments() { + public Set getSegments() { return mSegments; } diff --git a/src/main/java/io/split/android/client/dtos/ExcludedSegment.java b/src/main/java/io/split/android/client/dtos/ExcludedSegment.java new file mode 100644 index 000000000..89c76f1e5 --- /dev/null +++ b/src/main/java/io/split/android/client/dtos/ExcludedSegment.java @@ -0,0 +1,79 @@ +package io.split.android.client.dtos; + +import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; + +import com.google.gson.annotations.SerializedName; + +public class ExcludedSegment { + + private static final String TYPE_LARGE = "large"; + private static final String TYPE_STANDARD = "standard"; + private static final String TYPE_RULE_BASED = "rule-based"; + + @SerializedName("type") + private String mType; + + @SerializedName("name") + private String mName; + + public ExcludedSegment() {} + + private ExcludedSegment(String name, String type) { + mName = name; + mType = type; + } + + @VisibleForTesting + public static ExcludedSegment standard(String name) { + return new ExcludedSegment(name, TYPE_STANDARD); + } + + @VisibleForTesting + public static ExcludedSegment large(String name) { + return new ExcludedSegment(name, TYPE_LARGE); + } + + public static ExcludedSegment ruleBased(String name) { + return new ExcludedSegment(name, TYPE_RULE_BASED); + } + + public String getName() { + return mName; + } + + public boolean isStandard() { + return TYPE_STANDARD.equals(mType); + } + + public boolean isLarge() { + return TYPE_LARGE.equals(mType); + } + + public boolean isRuleBased() { + return TYPE_RULE_BASED.equals(mType); + } + + @Override + public boolean equals(@Nullable Object obj) { + if (obj == null) { + return false; + } + if (obj == this) { + return true; + } + if (!(obj instanceof ExcludedSegment)) { + return false; + } + ExcludedSegment other = (ExcludedSegment) obj; + return mName.equals(other.mName) && mType.equals(other.mType); + } + + @Override + public int hashCode() { + int result = 17; + result = 31 * result + mName.hashCode(); + result = 31 * result + mType.hashCode(); + return result; + } +} diff --git a/src/main/java/io/split/android/engine/experiments/ParsedRuleBasedSegment.java b/src/main/java/io/split/android/engine/experiments/ParsedRuleBasedSegment.java index b77388836..0ebf0d6a3 100644 --- a/src/main/java/io/split/android/engine/experiments/ParsedRuleBasedSegment.java +++ b/src/main/java/io/split/android/engine/experiments/ParsedRuleBasedSegment.java @@ -1,20 +1,23 @@ package io.split.android.engine.experiments; +import java.util.HashSet; import java.util.List; import java.util.Set; +import io.split.android.client.dtos.ExcludedSegment; + public class ParsedRuleBasedSegment { private final String mName; private final Set mExcludedKeys; - private final Set mExcludedSegments; + private final Set mExcludedSegments; private final List mParsedConditions; private final String mTrafficTypeName; private final long mChangeNumber; - public ParsedRuleBasedSegment(String name, Set excludedKeys, Set excludedSegments, List parsedConditions, String trafficTypeName, long changeNumber) { + public ParsedRuleBasedSegment(String name, Set excludedKeys, Set excludedSegments, List parsedConditions, String trafficTypeName, long changeNumber) { mName = name; - mExcludedKeys = excludedKeys; - mExcludedSegments = excludedSegments; + mExcludedKeys = excludedKeys == null ? new HashSet<>() : excludedKeys; + mExcludedSegments = excludedSegments == null ? new HashSet<>() : excludedSegments; mParsedConditions = parsedConditions; mTrafficTypeName = trafficTypeName; mChangeNumber = changeNumber; @@ -28,7 +31,7 @@ public Set getExcludedKeys() { return mExcludedKeys; } - public Set getExcludedSegments() { + public Set getExcludedSegments() { return mExcludedSegments; } diff --git a/src/main/java/io/split/android/engine/experiments/ParserCommons.java b/src/main/java/io/split/android/engine/experiments/ParserCommons.java index 8d82a0746..449f2cc20 100644 --- a/src/main/java/io/split/android/engine/experiments/ParserCommons.java +++ b/src/main/java/io/split/android/engine/experiments/ParserCommons.java @@ -214,6 +214,7 @@ private AttributeMatcher toMatcher(Matcher matcher, String matchingKey) throws U if (mRuleBasedSegmentStorage != null) { delegate = new InRuleBasedSegmentMatcher(mRuleBasedSegmentStorage, (matchingKey != null) ? mMySegmentsStorageContainer.getStorageForKey(matchingKey) : getEmptyMySegmentsStorage(), + (matchingKey != null) ? mMyLargeSegmentsStorageContainer.getStorageForKey(matchingKey) : getEmptyMySegmentsStorage(), matcher.userDefinedSegmentMatcherData.segmentName); } else { // shouldn't happen diff --git a/src/main/java/io/split/android/engine/matchers/InRuleBasedSegmentMatcher.java b/src/main/java/io/split/android/engine/matchers/InRuleBasedSegmentMatcher.java index bde3291a2..54d4b1f57 100644 --- a/src/main/java/io/split/android/engine/matchers/InRuleBasedSegmentMatcher.java +++ b/src/main/java/io/split/android/engine/matchers/InRuleBasedSegmentMatcher.java @@ -2,9 +2,12 @@ import static io.split.android.client.utils.Utils.checkNotNull; +import androidx.annotation.NonNull; + import java.util.Map; import io.split.android.client.Evaluator; +import io.split.android.client.dtos.ExcludedSegment; import io.split.android.client.storage.mysegments.MySegmentsStorage; import io.split.android.client.storage.rbs.RuleBasedSegmentStorageConsumer; import io.split.android.engine.experiments.ParsedCondition; @@ -14,11 +17,16 @@ public class InRuleBasedSegmentMatcher implements Matcher { private final RuleBasedSegmentStorageConsumer mRuleBasedSegmentStorage; private final MySegmentsStorage mMySegmentsStorage; + private final MySegmentsStorage mMyLargeSegmentsStorage; private final String mSegmentName; - public InRuleBasedSegmentMatcher(RuleBasedSegmentStorageConsumer ruleBasedSegmentStorage, MySegmentsStorage mySegmentsStorage, String segmentName) { + public InRuleBasedSegmentMatcher(@NonNull RuleBasedSegmentStorageConsumer ruleBasedSegmentStorage, + @NonNull MySegmentsStorage mySegmentsStorage, + @NonNull MySegmentsStorage myLargeSegmentsStorage, + @NonNull String segmentName) { mRuleBasedSegmentStorage = checkNotNull(ruleBasedSegmentStorage); mMySegmentsStorage = checkNotNull(mySegmentsStorage); + mMyLargeSegmentsStorage = checkNotNull(myLargeSegmentsStorage); mSegmentName = checkNotNull(segmentName); } @@ -35,22 +43,47 @@ public boolean match(Object matchValue, String bucketingKey, Map return false; } - if (parsedRuleBasedSegment.getExcludedKeys().contains(matchingKey)) { + if (isKeyExcluded(parsedRuleBasedSegment, matchingKey)) { return false; } - for (String segmentName : parsedRuleBasedSegment.getExcludedSegments()) { - if (mMySegmentsStorage.getAll().contains(segmentName)) { - return false; + if (inExcludedSegment(parsedRuleBasedSegment, matchingKey, bucketingKey, attributes, evaluator)) { + return false; + } + + return matchesConditions(bucketingKey, attributes, evaluator, parsedRuleBasedSegment, matchingKey); + } + + private static boolean isKeyExcluded(ParsedRuleBasedSegment parsedRuleBasedSegment, String matchingKey) { + return parsedRuleBasedSegment.getExcludedKeys().contains(matchingKey); + } + + private boolean inExcludedSegment(ParsedRuleBasedSegment parsedRuleBasedSegment, Object matchingKey, String bucketingKey, Map attributes, Evaluator evaluator) { + for (ExcludedSegment segment : parsedRuleBasedSegment.getExcludedSegments()) { + if (segment.isStandard() && mMySegmentsStorage.getAll().contains(segment.getName())) { + return true; + } + + if (segment.isRuleBased()) { + InRuleBasedSegmentMatcher inRuleBasedSegmentMatcher = new InRuleBasedSegmentMatcher(mRuleBasedSegmentStorage, mMySegmentsStorage, mMyLargeSegmentsStorage, segment.getName()); + if (inRuleBasedSegmentMatcher.match(matchingKey, bucketingKey, attributes, evaluator)) { + return true; + } + } + + if (segment.isLarge() && mMyLargeSegmentsStorage.getAll().contains(segment.getName())) { + return true; } } + return false; + } + private static boolean matchesConditions(String bucketingKey, Map attributes, Evaluator evaluator, ParsedRuleBasedSegment parsedRuleBasedSegment, String matchingKey) { for (ParsedCondition condition : parsedRuleBasedSegment.getParsedConditions()) { if (condition.matcher().match(matchingKey, bucketingKey, attributes, evaluator)) { return true; } } - return false; } } diff --git a/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java b/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java index 4037a908f..2ce2ded65 100644 --- a/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java +++ b/src/test/java/io/split/android/client/service/rules/TargetingRulesResponseParserTest.java @@ -8,6 +8,10 @@ import org.junit.Before; import org.junit.Test; +import java.util.Set; + +import io.split.android.client.dtos.Excluded; +import io.split.android.client.dtos.ExcludedSegment; import io.split.android.client.dtos.TargetingRulesChange; import io.split.android.client.service.http.HttpResponseParserException; import io.split.android.helpers.FileHelper; @@ -35,6 +39,14 @@ public void parsesNewTargetingRulesChangeJson() throws Exception { assertEquals(1506703262920L, result.getRuleBasedSegmentsChange().getSince()); assertEquals(1506703263000L, result.getRuleBasedSegmentsChange().getTill()); assertEquals("mauro_rule_based_segment", result.getRuleBasedSegmentsChange().getSegments().get(0).getName()); + Excluded excluded = result.getRuleBasedSegmentsChange().getSegments().get(0).getExcluded(); + assertEquals(1, excluded.getKeys().size()); + Set excludedSegments = excluded.getSegments(); + assertEquals(4, excludedSegments.size()); + // check that it contains 4 excluded segments: standard, large, rule-based and unsupported + assertTrue(excludedSegments.contains(ExcludedSegment.standard("segment_test"))); + assertTrue(excludedSegments.contains(ExcludedSegment.large("segment_test2"))); + assertTrue(excludedSegments.contains(ExcludedSegment.ruleBased("segment_test3"))); } @Test diff --git a/src/test/java/io/split/android/client/service/sseclient/SplitUpdateWorkerTest.java b/src/test/java/io/split/android/client/service/sseclient/SplitUpdateWorkerTest.java index 657862e4d..b946113db 100644 --- a/src/test/java/io/split/android/client/service/sseclient/SplitUpdateWorkerTest.java +++ b/src/test/java/io/split/android/client/service/sseclient/SplitUpdateWorkerTest.java @@ -63,7 +63,7 @@ public class SplitUpdateWorkerTest { private RuleBasedSegmentStorage mRuleBasedSegmentStorage; private static final String TEST_SPLIT = "{\"trafficTypeName\":\"account\",\"name\":\"android_test_2\",\"trafficAllocation\":100,\"trafficAllocationSeed\":-1955610140,\"seed\":-633015570,\"status\":\"ACTIVE\",\"killed\":false,\"defaultTreatment\":\"off\",\"changeNumber\":1648733409158,\"algo\":2,\"configurations\":{},\"conditions\":[{\"conditionType\":\"ROLLOUT\",\"matcherGroup\":{\"combiner\":\"AND\",\"matchers\":[{\"keySelector\":{\"trafficType\":\"account\",\"attribute\":null},\"matcherType\":\"IN_SPLIT_TREATMENT\",\"negate\":false,\"userDefinedSegmentMatcherData\":null,\"whitelistMatcherData\":null,\"unaryNumericMatcherData\":null,\"betweenMatcherData\":null,\"booleanMatcherData\":null,\"dependencyMatcherData\":{\"split\":\"android_test_3\",\"treatments\":[\"on\"]},\"stringMatcherData\":null}]},\"partitions\":[{\"treatment\":\"on\",\"size\":100},{\"treatment\":\"off\",\"size\":0}],\"label\":\"in split android_test_3 treatment [on]\"},{\"conditionType\":\"ROLLOUT\",\"matcherGroup\":{\"combiner\":\"AND\",\"matchers\":[{\"keySelector\":{\"trafficType\":\"account\",\"attribute\":null},\"matcherType\":\"ALL_KEYS\",\"negate\":false,\"userDefinedSegmentMatcherData\":null,\"whitelistMatcherData\":null,\"unaryNumericMatcherData\":null,\"betweenMatcherData\":null,\"booleanMatcherData\":null,\"dependencyMatcherData\":null,\"stringMatcherData\":null}]},\"partitions\":[{\"treatment\":\"on\",\"size\":0},{\"treatment\":\"off\",\"size\":100}],\"label\":\"default rule\"}]}"; - private static final String TEST_RULE_BASED_SEGMENT = "{\"changeNumber\":5,\"name\":\"mauro_rule_based_segment\",\"status\":\"ACTIVE\",\"trafficTypeName\":\"user\",\"excluded\":{\"keys\":[\"mauro@split.io\",\"gaston@split.io\"],\"segments\":[\"segment_test\"]},\"conditions\":[{\"matcherGroup\":{\"combiner\":\"AND\",\"matchers\":[{\"keySelector\":{\"trafficType\":\"user\"},\"matcherType\":\"WHITELIST\",\"negate\":false,\"whitelistMatcherData\":{\"whitelist\":[\"mdp\",\"tandil\",\"bsas\"]}},{\"keySelector\":{\"trafficType\":\"user\",\"attribute\":\"email\"},\"matcherType\":\"ENDS_WITH\",\"negate\":false,\"whitelistMatcherData\":{\"whitelist\":[\"@split.io\"]}}]}},{\"matcherGroup\":{\"combiner\":\"AND\",\"matchers\":[{\"keySelector\":{\"trafficType\":\"user\"},\"matcherType\":\"IN_SEGMENT\",\"negate\":false,\"userDefinedSegmentMatcherData\":{\"segmentName\":\"regular_segment\"}}]}}]}"; + private static final String TEST_RULE_BASED_SEGMENT = "{\"changeNumber\":5,\"name\":\"mauro_rule_based_segment\",\"status\":\"ACTIVE\",\"trafficTypeName\":\"user\",\"excluded\":{\"keys\":[\"mauro@split.io\",\"gaston@split.io\"],\"segments\":[{\"name\":\"segment_test\",\"type\":\"standard\"}]},\"conditions\":[{\"matcherGroup\":{\"combiner\":\"AND\",\"matchers\":[{\"keySelector\":{\"trafficType\":\"user\"},\"matcherType\":\"WHITELIST\",\"negate\":false,\"whitelistMatcherData\":{\"whitelist\":[\"mdp\",\"tandil\",\"bsas\"]}},{\"keySelector\":{\"trafficType\":\"user\",\"attribute\":\"email\"},\"matcherType\":\"ENDS_WITH\",\"negate\":false,\"whitelistMatcherData\":{\"whitelist\":[\"@split.io\"]}}]}},{\"matcherGroup\":{\"combiner\":\"AND\",\"matchers\":[{\"keySelector\":{\"trafficType\":\"user\"},\"matcherType\":\"IN_SEGMENT\",\"negate\":false,\"userDefinedSegmentMatcherData\":{\"segmentName\":\"regular_segment\"}}]}}]}"; @Before public void setup() { diff --git a/src/test/java/io/split/android/client/storage/cipher/ApplyCipherTaskTest.kt b/src/test/java/io/split/android/client/storage/cipher/ApplyCipherTaskTest.kt index cb7417bb1..e68b61c4d 100644 --- a/src/test/java/io/split/android/client/storage/cipher/ApplyCipherTaskTest.kt +++ b/src/test/java/io/split/android/client/storage/cipher/ApplyCipherTaskTest.kt @@ -390,18 +390,18 @@ class ApplyCipherTaskTest { verify(fromCipher).decrypt("body1") verify(toCipher).encrypt("decrypted_name1") verify(toCipher).encrypt("decrypted_body1") - verify(ruleBasedSegmentDao).update("decrypted_name1", "encrypted_decrypted_name1", "encrypted_decrypted_body1") + verify(ruleBasedSegmentDao).update("name1", "encrypted_decrypted_name1", "encrypted_decrypted_body1") verify(fromCipher).decrypt("name2") verify(fromCipher).decrypt("body2") verify(toCipher).encrypt("decrypted_name2") verify(toCipher).encrypt("decrypted_body2") - verify(ruleBasedSegmentDao).update("decrypted_name2", "encrypted_decrypted_name2", "encrypted_decrypted_body2") + verify(ruleBasedSegmentDao).update("name2", "encrypted_decrypted_name2", "encrypted_decrypted_body2") verify(fromCipher).decrypt("name3") verify(fromCipher).decrypt("body3") verify(toCipher).encrypt("decrypted_name3") verify(toCipher).encrypt("decrypted_body3") - verify(ruleBasedSegmentDao).update("decrypted_name3", "encrypted_decrypted_name3", "encrypted_decrypted_body3") + verify(ruleBasedSegmentDao).update("name3", "encrypted_decrypted_name3", "encrypted_decrypted_body3") } } diff --git a/src/test/java/io/split/android/engine/experiments/RuleBasedSegmentParserTest.java b/src/test/java/io/split/android/engine/experiments/RuleBasedSegmentParserTest.java index cfcefdc75..cd0391292 100644 --- a/src/test/java/io/split/android/engine/experiments/RuleBasedSegmentParserTest.java +++ b/src/test/java/io/split/android/engine/experiments/RuleBasedSegmentParserTest.java @@ -20,6 +20,7 @@ import io.split.android.client.dtos.Condition; import io.split.android.client.dtos.Excluded; +import io.split.android.client.dtos.ExcludedSegment; import io.split.android.client.dtos.RuleBasedSegment; import io.split.android.client.dtos.Status; @@ -46,7 +47,7 @@ public void validParsing() { parsedConditions.add(mock(ParsedCondition.class)); Set excludedKeys = new HashSet<>(Arrays.asList("excluded1", "excluded2")); - Set excludedSegments = new HashSet<>(Arrays.asList("segment1", "segment2")); + Set excludedSegments = new HashSet<>(Arrays.asList(ExcludedSegment.standard("segment1"), ExcludedSegment.standard("segment2"))); Excluded excluded = mock(Excluded.class); when(excluded.getKeys()).thenReturn(excludedKeys); diff --git a/src/test/java/io/split/android/engine/matchers/InRuleBasedSegmentMatcherTest.java b/src/test/java/io/split/android/engine/matchers/InRuleBasedSegmentMatcherTest.java index e2a6d1cc5..7c5883148 100644 --- a/src/test/java/io/split/android/engine/matchers/InRuleBasedSegmentMatcherTest.java +++ b/src/test/java/io/split/android/engine/matchers/InRuleBasedSegmentMatcherTest.java @@ -20,6 +20,7 @@ import java.util.Set; import io.split.android.client.Evaluator; +import io.split.android.client.dtos.ExcludedSegment; import io.split.android.client.storage.mysegments.MySegmentsStorage; import io.split.android.client.storage.rbs.RuleBasedSegmentStorage; import io.split.android.engine.experiments.ParsedCondition; @@ -33,6 +34,7 @@ public class InRuleBasedSegmentMatcherTest { private RuleBasedSegmentStorage mRuleBasedSegmentStorage; private MySegmentsStorage mMySegmentsStorage; + private MySegmentsStorage mMyLargeSegmentsStorage; private InRuleBasedSegmentMatcher mMatcher; private Evaluator mEvaluator; @@ -40,8 +42,9 @@ public class InRuleBasedSegmentMatcherTest { public void setUp() { mRuleBasedSegmentStorage = mock(RuleBasedSegmentStorage.class); mMySegmentsStorage = mock(MySegmentsStorage.class); + mMyLargeSegmentsStorage = mock(MySegmentsStorage.class); mEvaluator = mock(Evaluator.class); - mMatcher = new InRuleBasedSegmentMatcher(mRuleBasedSegmentStorage, mMySegmentsStorage, SEGMENT_NAME); + mMatcher = new InRuleBasedSegmentMatcher(mRuleBasedSegmentStorage, mMySegmentsStorage, mMyLargeSegmentsStorage, SEGMENT_NAME); } @Test @@ -66,8 +69,8 @@ public void matchReturnsFalseWhenKeyIsExcluded() { @Test public void matchReturnsFalseWhenInExcludedSegment() { - String excludedSegment = "excluded-segment"; - Set mySegments = new HashSet<>(Collections.singletonList(excludedSegment)); + ExcludedSegment excludedSegment = ExcludedSegment.standard("excluded-segment"); + Set mySegments = new HashSet<>(Collections.singletonList("excluded-segment")); ParsedRuleBasedSegment segment = createSegment( Collections.emptySet(), @@ -81,6 +84,53 @@ public void matchReturnsFalseWhenInExcludedSegment() { assertFalse(mMatcher.match(MATCHING_KEY, BUCKETING_KEY, Collections.emptyMap(), mEvaluator)); } + @Test + public void matchReturnsFalseWhenInExcludedLargeSegment() { + ExcludedSegment excludedSegment = ExcludedSegment.large("excluded-segment"); + Set mySegments = new HashSet<>(Collections.singletonList("excluded-segment")); + + ParsedRuleBasedSegment segment = createSegment( + Collections.emptySet(), + new HashSet<>(Collections.singletonList(excludedSegment)), + Collections.emptyList() + ); + + when(mRuleBasedSegmentStorage.get(eq(SEGMENT_NAME), eq(MATCHING_KEY))).thenReturn(segment); + when(mMyLargeSegmentsStorage.getAll()).thenReturn(mySegments); + + assertFalse(mMatcher.match(MATCHING_KEY, BUCKETING_KEY, Collections.emptyMap(), mEvaluator)); + } + + @Test + public void matchReturnsFalseWhenInExcludedRuleBasedSegment() { + ExcludedSegment excludedSegment = ExcludedSegment.ruleBased("excluded-segment"); + + ParsedRuleBasedSegment segment = createSegment( + Collections.emptySet(), + new HashSet<>(Collections.singletonList(excludedSegment)), + Collections.emptyList() + ); + CombiningMatcher conditionMatcher = mock(CombiningMatcher.class); + Map attributes = new HashMap<>(); + attributes.put("age", 30); + attributes.put("country", "US"); + + when(conditionMatcher.match(eq(MATCHING_KEY), eq(BUCKETING_KEY), eq(attributes), eq(mEvaluator))).thenReturn(true); + + ParsedCondition condition = mock(ParsedCondition.class); + when(condition.matcher()).thenReturn(conditionMatcher); + + List conditions = Collections.singletonList(condition); + + ParsedRuleBasedSegment storedExcludedRbs + = createSegment(Collections.emptySet(), Collections.emptySet(), conditions); + + when(mRuleBasedSegmentStorage.get(eq(SEGMENT_NAME), eq(MATCHING_KEY))).thenReturn(segment); + when(mRuleBasedSegmentStorage.get(eq("excluded-segment"), eq(MATCHING_KEY))).thenReturn(storedExcludedRbs); + + assertFalse(mMatcher.match(MATCHING_KEY, BUCKETING_KEY, attributes, mEvaluator)); + } + @Test public void matchReturnsTrueWhenConditionMatches() { CombiningMatcher conditionMatcher = mock(CombiningMatcher.class); @@ -183,7 +233,7 @@ public void matchWhenStorageReturnsNull() { assertFalse(result); } - private ParsedRuleBasedSegment createSegment(Set excludedKeys, Set excludedSegments, List conditions) { + private ParsedRuleBasedSegment createSegment(Set excludedKeys, Set excludedSegments, List conditions) { ParsedRuleBasedSegment segment = mock(ParsedRuleBasedSegment.class); when(segment.getExcludedKeys()).thenReturn(excludedKeys); when(segment.getExcludedSegments()).thenReturn(excludedSegments); diff --git a/src/test/resources/split_changes_small.json b/src/test/resources/split_changes_small.json index 1e2e4ff94..a5ed67f1d 100644 --- a/src/test/resources/split_changes_small.json +++ b/src/test/resources/split_changes_small.json @@ -132,7 +132,22 @@ "mauro@split.io" ], "segments": [ - "segment_test" + { + "name": "segment_test", + "type": "standard" + }, + { + "name": "segment_test2", + "type": "large" + }, + { + "name": "segment_test3", + "type": "rule-based" + }, + { + "name": "segment_test4", + "type": "unsupported" + } ] }, "conditions": [ From f5a0afe322108bb2e96caef6fe875383af36c9a0 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Fri, 2 May 2025 16:19:20 -0300 Subject: [PATCH 04/17] Fix encryption migration for rbs --- .../android/client/storage/cipher/ApplyCipherTask.java | 5 +++-- .../storage/splits/SqLitePersistentSplitsStorage.java | 9 ++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/io/split/android/client/storage/cipher/ApplyCipherTask.java b/src/main/java/io/split/android/client/storage/cipher/ApplyCipherTask.java index b55299797..0325cc861 100644 --- a/src/main/java/io/split/android/client/storage/cipher/ApplyCipherTask.java +++ b/src/main/java/io/split/android/client/storage/cipher/ApplyCipherTask.java @@ -79,14 +79,15 @@ private void updateRuleBasedSegment(RuleBasedSegmentDao ruleBasedSegmentDao) { } for (RuleBasedSegmentEntity item : items) { - String fromName = mFromCipher.decrypt(item.getName()); + String name = item.getName(); + String fromName = mFromCipher.decrypt(name); String fromBody = mFromCipher.decrypt(item.getBody()); String toName = mToCipher.encrypt(fromName); String toBody = mToCipher.encrypt(fromBody); if (toName != null && toBody != null) { - ruleBasedSegmentDao.update(fromName, toName, toBody); + ruleBasedSegmentDao.update(name, toName, toBody); } } } diff --git a/src/main/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorage.java b/src/main/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorage.java index 5bd250a14..8de3f2325 100644 --- a/src/main/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorage.java +++ b/src/main/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorage.java @@ -11,12 +11,15 @@ import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; +import com.google.gson.reflect.TypeToken; + +import java.lang.reflect.Type; import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; -import io.split.android.client.SplitFactoryImpl; import io.split.android.client.dtos.Split; import io.split.android.client.dtos.Status; import io.split.android.client.service.executor.parallel.SplitParallelTaskExecutorFactory; @@ -28,10 +31,6 @@ import io.split.android.client.utils.Json; import io.split.android.client.utils.logger.Logger; -import com.google.gson.reflect.TypeToken; -import java.lang.reflect.Type; -import java.util.concurrent.ConcurrentHashMap; - public class SqLitePersistentSplitsStorage implements PersistentSplitsStorage { private static final int SQL_PARAM_BIND_SIZE = 20; From d058c944375022b5a5bb805744780d8f468b3294 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Fri, 2 May 2025 16:17:16 -0300 Subject: [PATCH 05/17] Encryption migration fixes --- .../rbs/RuleBasedSegmentsIntegrationTest.java | 2 +- .../storage/cipher/ApplyCipherTask.java | 11 +++++---- .../client/storage/cipher/CBCCipher.java | 2 +- .../client/storage/db/SplitQueryDao.java | 2 ++ .../client/storage/db/SplitQueryDaoImpl.java | 24 +++++++++++++++---- .../splits/SqLitePersistentSplitsStorage.java | 24 ++++++++++--------- 6 files changed, 43 insertions(+), 22 deletions(-) diff --git a/src/androidTest/java/tests/integration/rbs/RuleBasedSegmentsIntegrationTest.java b/src/androidTest/java/tests/integration/rbs/RuleBasedSegmentsIntegrationTest.java index 85154d0d9..164405fd4 100644 --- a/src/androidTest/java/tests/integration/rbs/RuleBasedSegmentsIntegrationTest.java +++ b/src/androidTest/java/tests/integration/rbs/RuleBasedSegmentsIntegrationTest.java @@ -163,7 +163,7 @@ private void referencedRbsTest(String notification) throws IOException, Interrup List names = entities.stream().map(RuleBasedSegmentEntity::getName).collect(Collectors.toList()); assertEquals(2, names.size()); assertTrue(names.contains("rbs_test") && names.contains("new_rbs_test")); - assertEquals(3, mSplitChangesHits.get()); + assertEquals(2, mSplitChangesHits.get()); } private void successfulInstantUpdateTest(String rbsChange0, String expectedContents) throws IOException, InterruptedException { diff --git a/src/main/java/io/split/android/client/storage/cipher/ApplyCipherTask.java b/src/main/java/io/split/android/client/storage/cipher/ApplyCipherTask.java index 0325cc861..3b04c44f5 100644 --- a/src/main/java/io/split/android/client/storage/cipher/ApplyCipherTask.java +++ b/src/main/java/io/split/android/client/storage/cipher/ApplyCipherTask.java @@ -54,7 +54,7 @@ public SplitTaskExecutionInfo execute() { @Override public void run() { updateAttributes(mSplitDatabase.attributesDao()); - updateSplits(mSplitDatabase.splitDao(), mSplitDatabase.generalInfoDao()); + updateSplits(mSplitDatabase, mSplitDatabase.generalInfoDao()); updateSegments(mSplitDatabase.mySegmentDao()); updateLargeSegments(mSplitDatabase.myLargeSegmentDao()); updateImpressions(mSplitDatabase.impressionDao()); @@ -88,6 +88,8 @@ private void updateRuleBasedSegment(RuleBasedSegmentDao ruleBasedSegmentDao) { if (toName != null && toBody != null) { ruleBasedSegmentDao.update(name, toName, toBody); + } else { + Logger.e("Error applying cipher to rule based segment storage"); } } } @@ -120,7 +122,7 @@ private void updateUniqueKeys(UniqueKeysDao uniqueKeysDao) { String toUserKey = mToCipher.encrypt(fromUserKey); String toFeatureList = mToCipher.encrypt(fromFeatureList); - if (toFeatureList != null) { + if (toUserKey != null && toFeatureList != null) { item.setUserKey(toUserKey); item.setFeatureList(toFeatureList); uniqueKeysDao.insert(item); @@ -213,9 +215,10 @@ private void updateEvents(EventDao eventDao) { } } - private void updateSplits(SplitDao dao, GeneralInfoDao generalInfoDao) { + private void updateSplits(SplitRoomDatabase splitDatabase, GeneralInfoDao generalInfoDao) { + SplitDao dao = splitDatabase.splitDao(); List items = dao.getAll(); - + splitDatabase.getSplitQueryDao().invalidate(); for (SplitEntity item : items) { String name = item.getName(); String fromName = mFromCipher.decrypt(name); diff --git a/src/main/java/io/split/android/client/storage/cipher/CBCCipher.java b/src/main/java/io/split/android/client/storage/cipher/CBCCipher.java index ca4f1d7b5..d296a1045 100644 --- a/src/main/java/io/split/android/client/storage/cipher/CBCCipher.java +++ b/src/main/java/io/split/android/client/storage/cipher/CBCCipher.java @@ -60,7 +60,7 @@ public String decrypt(String data) { return new String(bytes, CHARSET); } catch (Exception e) { - Logger.e("Error decrypting data: " + e.getMessage()); + Logger.e("Error decrypting data for source: " + data + " - " + e.getMessage()); return null; } finally { mCipherProvider.release(cipher); diff --git a/src/main/java/io/split/android/client/storage/db/SplitQueryDao.java b/src/main/java/io/split/android/client/storage/db/SplitQueryDao.java index 784e6f863..82ddcc086 100644 --- a/src/main/java/io/split/android/client/storage/db/SplitQueryDao.java +++ b/src/main/java/io/split/android/client/storage/db/SplitQueryDao.java @@ -4,4 +4,6 @@ public interface SplitQueryDao { Map getAllAsMap(); + + void invalidate(); } \ No newline at end of file diff --git a/src/main/java/io/split/android/client/storage/db/SplitQueryDaoImpl.java b/src/main/java/io/split/android/client/storage/db/SplitQueryDaoImpl.java index 42458f16e..2c126feb2 100644 --- a/src/main/java/io/split/android/client/storage/db/SplitQueryDaoImpl.java +++ b/src/main/java/io/split/android/client/storage/db/SplitQueryDaoImpl.java @@ -15,8 +15,9 @@ public class SplitQueryDaoImpl implements SplitQueryDao { private final SplitRoomDatabase mDatabase; private volatile Map mCachedSplitsMap; private final Object mLock = new Object(); - private boolean mIsInitialized = false; private final Thread mInitializationThread; + private boolean mIsInitialized = false; + private boolean mIsInvalidated = false; public SplitQueryDaoImpl(SplitRoomDatabase mDatabase) { this.mDatabase = mDatabase; @@ -27,7 +28,6 @@ public SplitQueryDaoImpl(SplitRoomDatabase mDatabase) { } catch (Exception ignore) { // Ignore } - long startTime = System.currentTimeMillis(); Map result = loadSplitsMap(); @@ -51,13 +51,13 @@ int getColumnIndexOrThrow(@NonNull Cursor c, @NonNull String name) { public Map getAllAsMap() { // Fast path - if the map is already initialized, return it immediately - if (mIsInitialized && !mCachedSplitsMap.isEmpty()) { + if (isValid() && !mCachedSplitsMap.isEmpty()) { return new HashMap<>(mCachedSplitsMap); } // Wait for initialization to complete if it's in progress synchronized (mLock) { - if (mIsInitialized && !mCachedSplitsMap.isEmpty()) { + if (isValid() && !mCachedSplitsMap.isEmpty()) { return new HashMap<>(mCachedSplitsMap); } @@ -66,7 +66,7 @@ public Map getAllAsMap() { try { mLock.wait(5000); // Wait up to 5 seconds - if (mIsInitialized) { + if (isValid()) { return new HashMap<>(mCachedSplitsMap); } } catch (InterruptedException e) { @@ -85,6 +85,20 @@ public Map getAllAsMap() { return new HashMap<>(result); } } + + private boolean isValid() { + return mIsInitialized && !mIsInvalidated; + } + + @Override + public void invalidate() { + synchronized (mLock) { + mCachedSplitsMap.clear(); + mIsInvalidated = true; + mLock.notifyAll(); + Logger.i("Invalidated preloaded flags"); + } + } /** * Internal method to load the splits map from the database. diff --git a/src/main/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorage.java b/src/main/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorage.java index 8de3f2325..9f328e455 100644 --- a/src/main/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorage.java +++ b/src/main/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorage.java @@ -85,12 +85,14 @@ public void run() { mDatabase.splitDao().delete(removedSplits); } if (!mTrafficTypes.isEmpty()) { + String encryptedTrafficTypes = mCipher.encrypt(Json.toJson(mTrafficTypes)); mDatabase.generalInfoDao().update(new GeneralInfoEntity(GeneralInfoEntity.TRAFFIC_TYPES_MAP, - Json.toJson(mTrafficTypes))); + encryptedTrafficTypes)); } if (!mFlagSets.isEmpty()) { + String encryptedFlagSets = mCipher.encrypt(Json.toJson(mFlagSets)); mDatabase.generalInfoDao().update(new GeneralInfoEntity(GeneralInfoEntity.FLAG_SETS_MAP, - Json.toJson(mFlagSets))); + encryptedFlagSets)); } mDatabase.generalInfoDao().update( new GeneralInfoEntity(GeneralInfoEntity.SPLITS_UPDATE_TIMESTAMP, splitChange.getUpdateTimestamp())); @@ -178,10 +180,8 @@ public String getFilterQueryString() { private List loadSplits() { Map allNamesAndBodies = mDatabase.getSplitQueryDao().getAllAsMap(); - long transformStartTime = System.currentTimeMillis(); - List splits = mEntityToSplitTransformer.transform(allNamesAndBodies); - return splits; + return mEntityToSplitTransformer.transform(allNamesAndBodies); } private List convertSplitListToEntities(List splits) { @@ -278,12 +278,14 @@ private void migrateTrafficTypesAndSetsFromStoredData() { try { for (Split split : mSplits) { Split parsedSplit = Json.fromJson(split.json, Split.class); - if (parsedSplit != null && parsedSplit.status == Status.ACTIVE) { - increaseTrafficTypeCount(parsedSplit.trafficTypeName, mTrafficTypes); - addOrUpdateFlagSets(parsedSplit, mFlagSets); - } else { - decreaseTrafficTypeCount(parsedSplit.trafficTypeName, mTrafficTypes); - deleteFromFlagSetsIfNecessary(parsedSplit, mFlagSets); + if (parsedSplit != null) { + if (parsedSplit.status == Status.ACTIVE) { + increaseTrafficTypeCount(parsedSplit.trafficTypeName, mTrafficTypes); + addOrUpdateFlagSets(parsedSplit, mFlagSets); + } else { + decreaseTrafficTypeCount(parsedSplit.trafficTypeName, mTrafficTypes); + deleteFromFlagSetsIfNecessary(parsedSplit, mFlagSets); + } } } From 03c6a4c55b1d8a32859ac06282861b1e7d12c3fb Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Fri, 2 May 2025 16:31:37 -0300 Subject: [PATCH 06/17] Add query dao invalidation to test --- .../android/client/storage/cipher/ApplyCipherTaskTest.kt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/java/io/split/android/client/storage/cipher/ApplyCipherTaskTest.kt b/src/test/java/io/split/android/client/storage/cipher/ApplyCipherTaskTest.kt index e68b61c4d..0c5b4ae45 100644 --- a/src/test/java/io/split/android/client/storage/cipher/ApplyCipherTaskTest.kt +++ b/src/test/java/io/split/android/client/storage/cipher/ApplyCipherTaskTest.kt @@ -16,6 +16,7 @@ import io.split.android.client.storage.db.MySegmentDao import io.split.android.client.storage.db.MySegmentEntity import io.split.android.client.storage.db.SplitDao import io.split.android.client.storage.db.SplitEntity +import io.split.android.client.storage.db.SplitQueryDao import io.split.android.client.storage.db.SplitRoomDatabase import io.split.android.client.storage.db.attributes.AttributesDao import io.split.android.client.storage.db.attributes.AttributesEntity @@ -50,6 +51,9 @@ class ApplyCipherTaskTest { @Mock private lateinit var splitDao: SplitDao + @Mock + private lateinit var splitQueryDao: SplitQueryDao + @Mock private lateinit var mySegmentDao: MySegmentDao @@ -83,6 +87,7 @@ class ApplyCipherTaskTest { fun setup() { MockitoAnnotations.openMocks(this) `when`(splitDatabase.splitDao()).thenReturn(splitDao) + `when`(splitDatabase.splitQueryDao).thenReturn(splitQueryDao) `when`(splitDatabase.mySegmentDao()).thenReturn(mySegmentDao) `when`(splitDatabase.myLargeSegmentDao()).thenReturn(myLargeSegmentDao) `when`(splitDatabase.impressionDao()).thenReturn(impressionDao) @@ -131,6 +136,8 @@ class ApplyCipherTaskTest { verify(toCipher).encrypt("decrypted_name2") verify(toCipher).encrypt("decrypted_body2") verify(splitDao).update("name2", "encrypted_decrypted_name2", "encrypted_decrypted_body2") + + verify(splitQueryDao).invalidate() } @Test From af4f9a13e17f32b1c88cbd5abac7f07f3f8734c7 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 29 Apr 2025 11:51:25 -0300 Subject: [PATCH 07/17] Proxy outdated error code --- .../client/network/HttpStreamRequestImpl.java | 26 ++++++++----------- .../client/service/http/HttpFetcherImpl.java | 17 ++++++++++++ .../client/service/http/HttpStatus.java | 8 +++++- .../service/splits/SplitsSyncHelper.java | 2 +- 4 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/main/java/io/split/android/client/network/HttpStreamRequestImpl.java b/src/main/java/io/split/android/client/network/HttpStreamRequestImpl.java index ac9a86105..60453013b 100644 --- a/src/main/java/io/split/android/client/network/HttpStreamRequestImpl.java +++ b/src/main/java/io/split/android/client/network/HttpStreamRequestImpl.java @@ -88,9 +88,7 @@ public void addHeader(String name, String value) { public void close() { try { Logger.d("Closing streaming connection"); - if (mConnection != null) { - mConnection.disconnect(); - } + disconnect(); } catch (Exception e) { Logger.d("Unknown error closing connection: " + e.getLocalizedMessage()); } finally { @@ -119,24 +117,16 @@ private HttpStreamResponse getRequest() throws HttpException { response = handleAuthentication(response); } } catch (MalformedURLException e) { - if (mConnection != null) { - mConnection.disconnect(); - } + disconnect(); throw new HttpException("URL is malformed: " + e.getLocalizedMessage()); } catch (ProtocolException e) { - if (mConnection != null) { - mConnection.disconnect(); - } + disconnect(); throw new HttpException("Http method not allowed: " + e.getLocalizedMessage()); } catch (SSLPeerUnverifiedException e) { - if (mConnection != null) { - mConnection.disconnect(); - } + disconnect(); throw new HttpException("SSL peer not verified: " + e.getLocalizedMessage(), HttpStatus.INTERNAL_NON_RETRYABLE.getCode()); } catch (IOException e) { - if (mConnection != null) { - mConnection.disconnect(); - } + disconnect(); throw new HttpException("Something happened while retrieving data: " + e.getLocalizedMessage()); } @@ -187,4 +177,10 @@ private HttpStreamResponse buildResponse(HttpURLConnection connection) throws IO return new HttpStreamResponseImpl(responseCode); } + + private void disconnect() { + if (mConnection != null) { + mConnection.disconnect(); + } + } } diff --git a/src/main/java/io/split/android/client/service/http/HttpFetcherImpl.java b/src/main/java/io/split/android/client/service/http/HttpFetcherImpl.java index 38c445713..d45e36f6f 100644 --- a/src/main/java/io/split/android/client/service/http/HttpFetcherImpl.java +++ b/src/main/java/io/split/android/client/service/http/HttpFetcherImpl.java @@ -8,6 +8,8 @@ import java.net.URI; import java.util.Map; +import io.split.android.android_client.BuildConfig; +import io.split.android.client.ServiceEndpoints; import io.split.android.client.network.HttpClient; import io.split.android.client.network.HttpException; import io.split.android.client.network.HttpMethod; @@ -56,6 +58,9 @@ public T execute(@NonNull Map params, } if (!response.isSuccess()) { int httpStatus = response.getHttpStatus(); + + checkOutdatedProxyError(httpStatus, builtUri, params); + throw new HttpFetcherException(mTarget.toString(), "http return code " + httpStatus, httpStatus); } @@ -73,4 +78,16 @@ public T execute(@NonNull Map params, } return responseData; } + + private void checkOutdatedProxyError(int httpStatus, @Nullable URI builtUri, Map params) throws HttpFetcherException { + int proxyErrorStatus = HttpStatus.BAD_REQUEST.getCode(); + boolean sdkEndpointOverridden = builtUri != null && + builtUri.getHost() != null && + ServiceEndpoints.EndpointValidator.sdkEndpointIsOverridden(builtUri.getHost()); + boolean isLatestSpec = params != null && BuildConfig.FLAGS_SPEC.equals(params.get("s")); + + if (httpStatus == proxyErrorStatus && sdkEndpointOverridden && isLatestSpec) { + throw new HttpFetcherException(mTarget.toString(), "Proxy is outdated", HttpStatus.INTERNAL_PROXY_OUTDATED.getCode()); + } + } } diff --git a/src/main/java/io/split/android/client/service/http/HttpStatus.java b/src/main/java/io/split/android/client/service/http/HttpStatus.java index 2ae1c9a86..a9e2ca139 100644 --- a/src/main/java/io/split/android/client/service/http/HttpStatus.java +++ b/src/main/java/io/split/android/client/service/http/HttpStatus.java @@ -6,8 +6,10 @@ public enum HttpStatus { URI_TOO_LONG(414, "URI Too Long"), FORBIDDEN(403, "Forbidden"), + BAD_REQUEST(400, "Bad request"), - INTERNAL_NON_RETRYABLE(9009, "Non retryable"); + INTERNAL_NON_RETRYABLE(9009, "Non retryable"), + INTERNAL_PROXY_OUTDATED(9010, "Split Proxy outdated"); private final int mCode; private final String mDescription; @@ -49,4 +51,8 @@ public static boolean isNotRetryable(HttpStatus httpStatus) { public static boolean isNotRetryable(Integer code) { return isNotRetryable(fromCode(code)); } + + public static boolean isProxyOutdated(Integer code) { + return fromCode(code) == HttpStatus.INTERNAL_PROXY_OUTDATED; + } } diff --git a/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java b/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java index 7c8e802b2..a9f010903 100644 --- a/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java +++ b/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java @@ -100,7 +100,7 @@ private SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBefore attemptSplitSync(till, clearBeforeUpdate, avoidCache, cdnByPassType, resetChangeNumber, onDemandFetchBackoffMaxRetries); } } catch (HttpFetcherException e) { - logError("Network error while fetching feature flags" + e.getLocalizedMessage()); + logError("Network error while fetching feature flags - " + e.getLocalizedMessage()); mTelemetryRuntimeProducer.recordSyncError(OperationType.SPLITS, e.getHttpStatus()); HttpStatus httpStatus = HttpStatus.fromCode(e.getHttpStatus()); From 1d87ea33510091fb1fdfce3ee0f5693e6ef38a53 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 29 Apr 2025 13:11:25 -0300 Subject: [PATCH 08/17] WIP proxy handling --- .../executor/SplitTaskFactoryImpl.java | 3 +- .../client/service/http/HttpStatus.java | 4 +- .../splits/OutdatedSplitProxyHandler.java | 61 ++++++++++++++ .../service/splits/SplitsSyncHelper.java | 84 +++++++++++++++---- .../splits/SyncHelperProvider.java | 3 +- .../client/service/SplitsSyncHelperTest.java | 42 +++++----- 6 files changed, 160 insertions(+), 37 deletions(-) create mode 100644 src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java diff --git a/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java b/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java index 5e81d0844..0af56f047 100644 --- a/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java +++ b/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java @@ -110,7 +110,8 @@ public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig, mRuleBasedSegmentChangeProcessor, ruleBasedSegmentStorageProducer, mTelemetryRuntimeProducer, - flagsSpecFromConfig); + flagsSpecFromConfig, + false); } mFilters = (filters == null) ? new ArrayList<>() : new ArrayList<>(filters.values()); diff --git a/src/main/java/io/split/android/client/service/http/HttpStatus.java b/src/main/java/io/split/android/client/service/http/HttpStatus.java index a9e2ca139..33dede76e 100644 --- a/src/main/java/io/split/android/client/service/http/HttpStatus.java +++ b/src/main/java/io/split/android/client/service/http/HttpStatus.java @@ -52,7 +52,7 @@ public static boolean isNotRetryable(Integer code) { return isNotRetryable(fromCode(code)); } - public static boolean isProxyOutdated(Integer code) { - return fromCode(code) == HttpStatus.INTERNAL_PROXY_OUTDATED; + public static boolean isProxyOutdated(HttpStatus status) { + return status == HttpStatus.INTERNAL_PROXY_OUTDATED; } } diff --git a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java new file mode 100644 index 000000000..3ddb4dddc --- /dev/null +++ b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java @@ -0,0 +1,61 @@ +package io.split.android.client.service.splits; + +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; + +import io.split.android.client.utils.logger.Logger; + +public class OutdatedSplitProxyHandler { + + private final String mLatestSpec; + private final String mPreviousSpec; + private static final long PROXY_CHECK_INTERNAL_MILLIS = TimeUnit.HOURS.toMillis(1); + private final AtomicReference mCurrentSpec; + private final AtomicLong mLastProxyCheckTimestamp = new AtomicLong(0); // TODO, persist + private final boolean mForBackgroundSync; + + OutdatedSplitProxyHandler(String flagSpec, String previousSpec, boolean forBackgroundSync) { + mLatestSpec = flagSpec; + mPreviousSpec = previousSpec; + mCurrentSpec = new AtomicReference<>(flagSpec); + mForBackgroundSync = forBackgroundSync; + } + + ProxyHandlingType handle() { + if (mForBackgroundSync) { + Logger.i("Background sync fetch; skipping proxy handling"); + return ProxyHandlingType.NONE; + } + + if (mCurrentSpec.get().equals(mLatestSpec)) { + Logger.i("Switching to previous spec: " + mPreviousSpec); + + mLastProxyCheckTimestamp.set(System.currentTimeMillis()); + + mCurrentSpec.set(mPreviousSpec); + + return ProxyHandlingType.FALLBACK; + } else if (mCurrentSpec.get().equals(mPreviousSpec)) { + if (System.currentTimeMillis() - mLastProxyCheckTimestamp.get() > PROXY_CHECK_INTERNAL_MILLIS) { + Logger.i("Attempting recovery with latest spec: " + mLatestSpec); + return ProxyHandlingType.RECOVERY; + } + } + + return ProxyHandlingType.NONE; + } + + String getCurrentSpec() { + return mCurrentSpec.get(); + } + + enum ProxyHandlingType { + // no action + NONE, + // switch to previous spec + FALLBACK, + // attempt recovery + RECOVERY, + } +} diff --git a/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java b/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java index a9f010903..c527a4fe4 100644 --- a/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java +++ b/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java @@ -46,7 +46,7 @@ public class SplitsSyncHelper { private final RuleBasedSegmentStorageProducer mRuleBasedSegmentStorage; private final TelemetryRuntimeProducer mTelemetryRuntimeProducer; private final BackoffCounter mBackoffCounter; - private final String mFlagsSpec; + private final OutdatedSplitProxyHandler mOutdatedSplitProxyHandler; public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, @NonNull SplitsStorage splitsStorage, @@ -54,7 +54,8 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, @NonNull RuleBasedSegmentChangeProcessor ruleBasedSegmentChangeProcessor, @NonNull RuleBasedSegmentStorageProducer ruleBasedSegmentStorage, @NonNull TelemetryRuntimeProducer telemetryRuntimeProducer, - @Nullable String flagsSpec) { + @Nullable String flagsSpec, + boolean forBackgroundSync) { this(splitFetcher, splitsStorage, splitChangeProcessor, @@ -62,10 +63,10 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, ruleBasedSegmentStorage, telemetryRuntimeProducer, new ReconnectBackoffCounter(1, ON_DEMAND_FETCH_BACKOFF_MAX_WAIT), - flagsSpec); + flagsSpec, + forBackgroundSync); } - @VisibleForTesting public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, @NonNull SplitsStorage splitsStorage, @NonNull SplitChangeProcessor splitChangeProcessor, @@ -74,6 +75,27 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, @NonNull TelemetryRuntimeProducer telemetryRuntimeProducer, @NonNull BackoffCounter backoffCounter, @Nullable String flagsSpec) { + this(splitFetcher, + splitsStorage, + splitChangeProcessor, + ruleBasedSegmentChangeProcessor, + ruleBasedSegmentStorage, + telemetryRuntimeProducer, + backoffCounter, + flagsSpec, + false); + } + + @VisibleForTesting + public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, + @NonNull SplitsStorage splitsStorage, + @NonNull SplitChangeProcessor splitChangeProcessor, + @NonNull RuleBasedSegmentChangeProcessor ruleBasedSegmentChangeProcessor, + @NonNull RuleBasedSegmentStorageProducer ruleBasedSegmentStorage, + @NonNull TelemetryRuntimeProducer telemetryRuntimeProducer, + @NonNull BackoffCounter backoffCounter, + @Nullable String flagsSpec, + boolean forBackgroundSync) { mSplitFetcher = checkNotNull(splitFetcher); mSplitsStorage = checkNotNull(splitsStorage); mSplitChangeProcessor = checkNotNull(splitChangeProcessor); @@ -81,7 +103,8 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, mRuleBasedSegmentStorage = checkNotNull(ruleBasedSegmentStorage); mTelemetryRuntimeProducer = checkNotNull(telemetryRuntimeProducer); mBackoffCounter = checkNotNull(backoffCounter); - mFlagsSpec = flagsSpec; + String mPreviousSpec = "1.2"; + mOutdatedSplitProxyHandler = new OutdatedSplitProxyHandler(flagsSpec, mPreviousSpec, forBackgroundSync); } public SplitTaskExecutionInfo sync(SinceChangeNumbers till, int onDemandFetchBackoffMaxRetries) { @@ -113,6 +136,14 @@ private SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBefore Collections.singletonMap(SplitTaskExecutionInfo.DO_NOT_RETRY, true)); } + if (HttpStatus.isProxyOutdated(httpStatus)) { + try { + return handleOutdatedProxy(till, avoidCache, resetChangeNumber, onDemandFetchBackoffMaxRetries); + } catch (Exception e1) { + logError("Unexpected while handling outdated proxy" + e1.getLocalizedMessage()); + } + } + return SplitTaskExecutionInfo.error(SplitTaskType.SPLITS_SYNC); } catch (Exception e) { logError("Unexpected while fetching feature flags" + e.getLocalizedMessage()); @@ -123,6 +154,25 @@ private SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBefore return SplitTaskExecutionInfo.success(SplitTaskType.SPLITS_SYNC); } + private SplitTaskExecutionInfo handleOutdatedProxy(SinceChangeNumbers till, boolean avoidCache, boolean resetChangeNumber, int onDemandFetchBackoffMaxRetries) throws Exception { + OutdatedSplitProxyHandler.ProxyHandlingType handle = mOutdatedSplitProxyHandler.handle(); + switch (handle) { + case FALLBACK: { + long flagsSince = till.getFlagsSince(); + SinceChangeNumbers newTill = new SinceChangeNumbers(flagsSince, null); + attemptSplitSync(newTill, false, false, CdnByPassType.NONE, false, onDemandFetchBackoffMaxRetries); + break; + } + case RECOVERY: { + SinceChangeNumbers newTill = new SinceChangeNumbers(-1, -1L); + attemptSplitSync(newTill, true, false, CdnByPassType.NONE, true, onDemandFetchBackoffMaxRetries); + break; + } + } + + return SplitTaskExecutionInfo.success(SplitTaskType.SPLITS_SYNC); + } + /** * @param targetChangeNumber target changeNumber * @param clearBeforeUpdate whether to clear splits storage before updating it @@ -140,7 +190,8 @@ private CdnByPassType attemptSplitSync(SinceChangeNumbers targetChangeNumber, bo SinceChangeNumbers retrievedChangeNumber = fetchUntil(targetChangeNumber, clearBeforeUpdate, avoidCache, withCdnBypass, resetChangeNumber); resetChangeNumber = false; - if (targetChangeNumber.getFlagsSince() <= retrievedChangeNumber.getFlagsSince() && targetChangeNumber.getRbsSince() <= retrievedChangeNumber.getRbsSince()) { + if (targetChangeNumber.getFlagsSince() <= retrievedChangeNumber.getFlagsSince() && + targetChangeNumber.getRbsSince() != null && retrievedChangeNumber.getRbsSince() != null && targetChangeNumber.getRbsSince() <= retrievedChangeNumber.getRbsSince()) { return CdnByPassType.NONE; } @@ -170,7 +221,7 @@ private SinceChangeNumbers fetchUntil(SinceChangeNumbers till, boolean clearBefo long changeNumber = (resetChangeNumber) ? -1 : mSplitsStorage.getTill(); long rbsChangeNumber = (resetChangeNumber) ? -1 : mRuleBasedSegmentStorage.getChangeNumber(); resetChangeNumber = false; - if (newTill.getFlagsSince() < changeNumber && newTill.getRbsSince() < rbsChangeNumber) { + if ((newTill.getFlagsSince() < changeNumber) && ((newTill.getRbsSince() == null) || (newTill.getRbsSince() < rbsChangeNumber))) { return new SinceChangeNumbers(changeNumber, rbsChangeNumber); } @@ -189,11 +240,14 @@ private SinceChangeNumbers fetchUntil(SinceChangeNumbers till, boolean clearBefo private TargetingRulesChange fetchSplits(SinceChangeNumbers till, boolean avoidCache, CdnByPassType cdnByPassType) throws HttpFetcherException { Map params = new LinkedHashMap<>(); - if (mFlagsSpec != null && !mFlagsSpec.trim().isEmpty()) { - params.put(FLAGS_SPEC_PARAM, mFlagsSpec); + String flagsSpec = mOutdatedSplitProxyHandler.getCurrentSpec(); + if (flagsSpec != null && !flagsSpec.trim().isEmpty()) { + params.put(FLAGS_SPEC_PARAM, flagsSpec); } params.put(SINCE_PARAM, till.getFlagsSince()); - params.put(RBS_SINCE_PARAM, till.getRbsSince()); + if (till.getRbsSince() != null) { + params.put(RBS_SINCE_PARAM, till.getRbsSince()); + } if (cdnByPassType == CdnByPassType.RBS) { params.put(TILL_PARAM, till.getRbsSince()); @@ -231,9 +285,10 @@ private void logError(String message) { public static class SinceChangeNumbers { private final long mFlagsSince; - private final long mRbsSince; + @Nullable + private final Long mRbsSince; - public SinceChangeNumbers(long flagsSince, long rbsSince) { + public SinceChangeNumbers(long flagsSince, @Nullable Long rbsSince) { mFlagsSince = flagsSince; mRbsSince = rbsSince; } @@ -242,7 +297,8 @@ public long getFlagsSince() { return mFlagsSince; } - public long getRbsSince() { + @Nullable + public Long getRbsSince() { return mRbsSince; } @@ -250,7 +306,7 @@ public long getRbsSince() { public boolean equals(@Nullable Object obj) { return obj instanceof SinceChangeNumbers && mFlagsSince == ((SinceChangeNumbers) obj).mFlagsSince && - mRbsSince == ((SinceChangeNumbers) obj).mRbsSince; + (mRbsSince == null && ((SinceChangeNumbers) obj).mRbsSince == null); } @NonNull diff --git a/src/main/java/io/split/android/client/service/workmanager/splits/SyncHelperProvider.java b/src/main/java/io/split/android/client/service/workmanager/splits/SyncHelperProvider.java index 640949301..1b2965398 100644 --- a/src/main/java/io/split/android/client/service/workmanager/splits/SyncHelperProvider.java +++ b/src/main/java/io/split/android/client/service/workmanager/splits/SyncHelperProvider.java @@ -24,6 +24,7 @@ SplitsSyncHelper provideSplitsSyncHelper(HttpFetcher split ruleBasedSegmentChangeProcessor, ruleBasedSegmentStorage, telemetryStorage, - mFlagsSpec); + mFlagsSpec, + true); } } diff --git a/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java b/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java index 48477e321..8cb872360 100644 --- a/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java +++ b/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java @@ -105,7 +105,7 @@ public void correctSyncExecution() throws HttpFetcherException { when(mSplitsStorage.getTill()).thenReturn(-1L); when(mRuleBasedSegmentStorageProducer.getChangeNumber()).thenReturn(-1L).thenReturn(262325L); - SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(-1, -1), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); verify(mSplitsFetcher, times(1)).execute(mDefaultParams, null); verify(mSplitsStorage, times(1)).update(any()); @@ -117,6 +117,10 @@ public void correctSyncExecution() throws HttpFetcherException { assertEquals(SplitTaskExecutionStatus.SUCCESS, result.getStatus()); } + private static SplitsSyncHelper.SinceChangeNumbers getSinceChangeNumbers(int flagsSince, long rbsSince) { + return new SplitsSyncHelper.SinceChangeNumbers(flagsSince, rbsSince); + } + @Test public void correctSyncExecutionNoCache() throws HttpFetcherException { // On correct execution without having clear param @@ -130,7 +134,7 @@ public void correctSyncExecutionNoCache() throws HttpFetcherException { .thenReturn(TargetingRulesChange.create(secondSplitChange, RuleBasedSegmentChange.create(262325L, 262325L, Collections.emptyList()))); when(mSplitsStorage.getTill()).thenReturn(-1L); - SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(-1, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); verify(mSplitsFetcher, times(1)).execute(mDefaultParams, headers); verify(mSplitsStorage, times(1)).update(any()); @@ -145,7 +149,7 @@ public void fetcherSyncException() throws HttpFetcherException { .thenThrow(HttpFetcherException.class); when(mSplitsStorage.getTill()).thenReturn(-1L); - SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(-1, -1), true, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), true, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); verify(mSplitsFetcher, times(1)).execute(mDefaultParams, null); verify(mSplitsStorage, never()).update(any()); @@ -160,7 +164,7 @@ public void storageException() throws HttpFetcherException { doThrow(NullPointerException.class).when(mSplitsStorage).update(any(ProcessedSplitChange.class)); when(mSplitsStorage.getTill()).thenReturn(-1L); - SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(-1, -1), true, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), true, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); verify(mSplitsFetcher, times(1)).execute(mDefaultParams, null); verify(mSplitsStorage, times(1)).update(any()); @@ -178,7 +182,7 @@ public void shouldClearStorageAfterFetch() throws HttpFetcherException { .thenReturn(TargetingRulesChange.create(secondSplitChange, RuleBasedSegmentChange.create(262325L, 262325L, Collections.emptyList()))); when(mSplitsStorage.getTill()).thenReturn(-1L); - SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(-1, -1), true, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), true, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); verify(mSplitsFetcher, times(1)).execute(mDefaultParams, null); verify(mSplitsStorage, times(1)).update(any()); @@ -194,7 +198,7 @@ public void errorIsRecordedInTelemetry() throws HttpFetcherException { .thenThrow(new HttpFetcherException("error", "error", 500)); when(mSplitsStorage.getTill()).thenReturn(-1L); - mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(-1, -1), true, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), true, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); verify(mTelemetryRuntimeProducer).recordSyncError(OperationType.SPLITS, 500); } @@ -214,7 +218,7 @@ public void performSplitsFetchUntilSinceEqualsTill() throws HttpFetcherException when(mSplitsFetcher.execute(eq(secondParams), any())).thenReturn(secondSplitChange); when(mSplitsFetcher.execute(eq(thirdParams), any())).thenReturn(thirdSplitChange); - mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(3, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + mSplitsSyncHelper.sync(getSinceChangeNumbers(3, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); verify(mSplitsStorage, times(3)).getTill(); verify(mSplitsFetcher).execute(eq(firstParams), any()); @@ -232,7 +236,7 @@ public void performSplitFetchUntilStoredChangeNumberIsGreaterThanRequested() thr when(mSplitsFetcher.execute(any(), any())).thenReturn(firstSplitChange).thenReturn(secondSplitChange).thenReturn(thirdSplitChange); - mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(3, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + mSplitsSyncHelper.sync(getSinceChangeNumbers(3, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); verify(mSplitsStorage, times(3)).getTill(); verify(mSplitsFetcher, times(3)).execute(any(), any()); @@ -243,7 +247,7 @@ public void syncWithClearBeforeUpdateOnlyClearsStorageOnce() throws HttpFetcherE when(mSplitsFetcher.execute(mDefaultParams, null)).thenReturn(mTargetingRulesChange); when(mSplitsStorage.getTill()).thenReturn(-1L, 2L, 4L); - mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(3, -1), true, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + mSplitsSyncHelper.sync(getSinceChangeNumbers(3, -1), true, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); verify(mSplitsStorage).clear(); } @@ -252,7 +256,7 @@ public void syncWithClearBeforeUpdateOnlyClearsStorageOnce() throws HttpFetcherE public void syncWithoutClearBeforeUpdateDoesNotClearStorage() { when(mSplitsStorage.getTill()).thenReturn(-1L, 2L, 4L); - mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(3, -1), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + mSplitsSyncHelper.sync(getSinceChangeNumbers(3, -1), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); verify(mSplitsStorage, never()).clear(); } @@ -277,7 +281,7 @@ public void cdnIsBypassedWhenNeeded() throws HttpFetcherException { getSplitChange(4, 4) ); - mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(4, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + mSplitsSyncHelper.sync(getSinceChangeNumbers(4, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); Map headers = new HashMap<>(); headers.put("Cache-Control", "no-cache"); @@ -313,7 +317,7 @@ public void cdnIsBypassedWhenNeededWithRuleBasedSegments() throws HttpFetcherExc getRuleBasedSegmentChange(4, 4) ); - mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(-1, 4), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, 4), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); Map headers = new HashMap<>(); headers.put("Cache-Control", "no-cache"); @@ -341,7 +345,7 @@ public void backoffIsAppliedWhenRetryingSplits() throws HttpFetcherException { getSplitChange(4, 4) ); - mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(4, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + mSplitsSyncHelper.sync(getSinceChangeNumbers(4, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); verify(mBackoffCounter).resetCounter(); verify(mBackoffCounter, times(2)).getNextRetryTime(); @@ -349,7 +353,7 @@ public void backoffIsAppliedWhenRetryingSplits() throws HttpFetcherException { @Test public void replaceTillWhenFilterHasChanged() throws HttpFetcherException { - mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(14829471, -1), true, true, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + mSplitsSyncHelper.sync(getSinceChangeNumbers(14829471, -1), true, true, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); Map params = new HashMap<>(); params.put("s", "1.1"); @@ -365,7 +369,7 @@ public void returnTaskInfoToDoNotRetryWhenHttpFetcherExceptionStatusCodeIs414() .thenThrow(new HttpFetcherException("error", "error", 414)); when(mSplitsStorage.getTill()).thenReturn(-1L); - SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(-1, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); assertEquals(true, result.getBoolValue(SplitTaskExecutionInfo.DO_NOT_RETRY)); } @@ -376,7 +380,7 @@ public void doNotRetryFlagIsNullWhenFetcherExceptionStatusCodeIsNot414() throws .thenThrow(new HttpFetcherException("error", "error", 500)); when(mSplitsStorage.getTill()).thenReturn(-1L); - SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(-1, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); assertNull(result.getBoolValue(SplitTaskExecutionInfo.DO_NOT_RETRY)); } @@ -387,7 +391,7 @@ public void returnTaskInfoToDoNotRetryWhenHttpFetcherExceptionStatusCodeIs9009() .thenThrow(new HttpFetcherException("error", "error", 9009)); when(mSplitsStorage.getTill()).thenReturn(-1L); - SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(-1, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); assertEquals(true, result.getBoolValue(SplitTaskExecutionInfo.DO_NOT_RETRY)); } @@ -398,14 +402,14 @@ public void doNotRetryFlagIsNullWhenFetcherExceptionStatusCodeIsNot9009() throws .thenThrow(new HttpFetcherException("error", "error", 500)); when(mSplitsStorage.getTill()).thenReturn(-1L); - SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(-1, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + SplitTaskExecutionInfo result = mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); assertNull(result.getBoolValue(SplitTaskExecutionInfo.DO_NOT_RETRY)); } @Test public void defaultQueryParamOrderIsCorrect() throws HttpFetcherException { - mSplitsSyncHelper.sync(new SplitsSyncHelper.SinceChangeNumbers(100, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + mSplitsSyncHelper.sync(getSinceChangeNumbers(100, -1), ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); verify(mSplitsFetcher).execute(argThat(new ArgumentMatcher>() { @Override From a5c7b59cb5f0e51febd8a4ac3d6de6d54ffa793a Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 29 Apr 2025 16:15:16 -0300 Subject: [PATCH 09/17] WIP --- .../executor/SplitTaskFactoryImpl.java | 2 + .../splits/OutdatedSplitProxyHandler.java | 82 ++++++++++++++++--- .../service/splits/SplitsSyncHelper.java | 42 +++++----- .../splits/SplitsSyncWorkerTaskBuilder.java | 3 + .../workmanager/splits/StorageProvider.java | 5 ++ .../splits/SyncHelperProvider.java | 3 + .../storage/general/GeneralInfoStorage.java | 4 + .../general/GeneralInfoStorageImpl.java | 14 +++- 8 files changed, 122 insertions(+), 33 deletions(-) diff --git a/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java b/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java index 0af56f047..fe770bc20 100644 --- a/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java +++ b/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java @@ -100,6 +100,7 @@ public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig, mSplitChangeProcessor, mRuleBasedSegmentChangeProcessor, ruleBasedSegmentStorageProducer, + mSplitsStorageContainer.getGeneralInfoStorage(), mTelemetryRuntimeProducer, new ReconnectBackoffCounter(1, testingConfig.getCdnBackoffTime()), flagsSpecFromConfig); @@ -109,6 +110,7 @@ public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig, mSplitChangeProcessor, mRuleBasedSegmentChangeProcessor, ruleBasedSegmentStorageProducer, + mSplitsStorageContainer.getGeneralInfoStorage(), mTelemetryRuntimeProducer, flagsSpecFromConfig, false); diff --git a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java index 3ddb4dddc..e442749db 100644 --- a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java +++ b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java @@ -1,25 +1,42 @@ package io.split.android.client.service.splits; +import static io.split.android.client.utils.Utils.checkNotNull; + +import androidx.annotation.NonNull; +import androidx.annotation.VisibleForTesting; + import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; +import io.split.android.client.storage.general.GeneralInfoStorage; import io.split.android.client.utils.logger.Logger; public class OutdatedSplitProxyHandler { + private static final long PROXY_CHECK_INTERVAL_MILLIS = TimeUnit.SECONDS.toMillis(1); + private final String mLatestSpec; private final String mPreviousSpec; - private static final long PROXY_CHECK_INTERNAL_MILLIS = TimeUnit.HOURS.toMillis(1); private final AtomicReference mCurrentSpec; - private final AtomicLong mLastProxyCheckTimestamp = new AtomicLong(0); // TODO, persist private final boolean mForBackgroundSync; + private final long mProxyCheckIntervalMillis; + + private final AtomicLong mLastProxyCheckTimestamp = new AtomicLong(0L); + private final GeneralInfoStorage mGeneralInfoStorage; - OutdatedSplitProxyHandler(String flagSpec, String previousSpec, boolean forBackgroundSync) { + OutdatedSplitProxyHandler(String flagSpec, String previousSpec, boolean forBackgroundSync, GeneralInfoStorage generalInfoStorage) { + this(flagSpec, previousSpec, forBackgroundSync, generalInfoStorage, PROXY_CHECK_INTERVAL_MILLIS); + } + + @VisibleForTesting + OutdatedSplitProxyHandler(String flagSpec, String previousSpec, boolean forBackgroundSync, GeneralInfoStorage generalInfoStorage, long proxyCheckIntervalMillis) { mLatestSpec = flagSpec; mPreviousSpec = previousSpec; mCurrentSpec = new AtomicReference<>(flagSpec); mForBackgroundSync = forBackgroundSync; + mProxyCheckIntervalMillis = proxyCheckIntervalMillis; + mGeneralInfoStorage = checkNotNull(generalInfoStorage); } ProxyHandlingType handle() { @@ -29,27 +46,68 @@ ProxyHandlingType handle() { } if (mCurrentSpec.get().equals(mLatestSpec)) { - Logger.i("Switching to previous spec: " + mPreviousSpec); + return fallback(); + } - mLastProxyCheckTimestamp.set(System.currentTimeMillis()); + return ProxyHandlingType.NONE; + } + + @NonNull + private ProxyHandlingType fallback() { + Logger.i("Switching to previous spec: " + mPreviousSpec); - mCurrentSpec.set(mPreviousSpec); + updateLastProxyCheckTimestamp(System.currentTimeMillis()); + + mCurrentSpec.set(mPreviousSpec); + + return ProxyHandlingType.FALLBACK; + } - return ProxyHandlingType.FALLBACK; - } else if (mCurrentSpec.get().equals(mPreviousSpec)) { - if (System.currentTimeMillis() - mLastProxyCheckTimestamp.get() > PROXY_CHECK_INTERNAL_MILLIS) { - Logger.i("Attempting recovery with latest spec: " + mLatestSpec); - return ProxyHandlingType.RECOVERY; + ProxyHandlingType proxyCheck() { + if (mCurrentSpec.get().equals(mLatestSpec)) { + long lastProxyCheckTimestamp = getLastProxyCheckTimestamp(); + if (lastProxyCheckTimestamp != 0L) { + // we may need to recover + if (System.currentTimeMillis() - lastProxyCheckTimestamp > mProxyCheckIntervalMillis) { + Logger.i("Attempting recovery with latest spec: " + mLatestSpec); + mCurrentSpec.set(mLatestSpec); + updateLastProxyCheckTimestamp(System.currentTimeMillis()); + return ProxyHandlingType.RECOVERY; + } + } else { + return fallback(); } } - + Logger.v("No need to handle outdated proxy"); return ProxyHandlingType.NONE; } + void resetProxyCheckTimestamp() { + updateLastProxyCheckTimestamp(0L); + } + + private long getLastProxyCheckTimestamp() { + mLastProxyCheckTimestamp.compareAndSet(0L, mGeneralInfoStorage.getLastProxyUpdateTimestamp()); + return mLastProxyCheckTimestamp.get(); + } + + private void updateLastProxyCheckTimestamp(long newTimestamp) { + mLastProxyCheckTimestamp.set(newTimestamp); + mGeneralInfoStorage.setLastProxyUpdateTimestamp(newTimestamp); + } + String getCurrentSpec() { return mCurrentSpec.get(); } + boolean isFallbackMode() { + return mCurrentSpec.get().equals(mPreviousSpec); + } + + boolean isNormalMode() { + return mCurrentSpec.get().equals(mLatestSpec); + } + enum ProxyHandlingType { // no action NONE, diff --git a/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java b/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java index c527a4fe4..0c15dc024 100644 --- a/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java +++ b/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java @@ -26,6 +26,7 @@ import io.split.android.client.service.rules.RuleBasedSegmentChangeProcessor; import io.split.android.client.service.sseclient.BackoffCounter; import io.split.android.client.service.sseclient.ReconnectBackoffCounter; +import io.split.android.client.storage.general.GeneralInfoStorage; import io.split.android.client.storage.rbs.RuleBasedSegmentStorageProducer; import io.split.android.client.storage.splits.SplitsStorage; import io.split.android.client.telemetry.model.OperationType; @@ -53,6 +54,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, @NonNull SplitChangeProcessor splitChangeProcessor, @NonNull RuleBasedSegmentChangeProcessor ruleBasedSegmentChangeProcessor, @NonNull RuleBasedSegmentStorageProducer ruleBasedSegmentStorage, + @NonNull GeneralInfoStorage generalInfoStorage, @NonNull TelemetryRuntimeProducer telemetryRuntimeProducer, @Nullable String flagsSpec, boolean forBackgroundSync) { @@ -61,6 +63,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, splitChangeProcessor, ruleBasedSegmentChangeProcessor, ruleBasedSegmentStorage, + generalInfoStorage, telemetryRuntimeProducer, new ReconnectBackoffCounter(1, ON_DEMAND_FETCH_BACKOFF_MAX_WAIT), flagsSpec, @@ -72,6 +75,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, @NonNull SplitChangeProcessor splitChangeProcessor, @NonNull RuleBasedSegmentChangeProcessor ruleBasedSegmentChangeProcessor, @NonNull RuleBasedSegmentStorageProducer ruleBasedSegmentStorage, + @NonNull GeneralInfoStorage generalInfoStorage, @NonNull TelemetryRuntimeProducer telemetryRuntimeProducer, @NonNull BackoffCounter backoffCounter, @Nullable String flagsSpec) { @@ -80,6 +84,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, splitChangeProcessor, ruleBasedSegmentChangeProcessor, ruleBasedSegmentStorage, + generalInfoStorage, telemetryRuntimeProducer, backoffCounter, flagsSpec, @@ -92,6 +97,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, @NonNull SplitChangeProcessor splitChangeProcessor, @NonNull RuleBasedSegmentChangeProcessor ruleBasedSegmentChangeProcessor, @NonNull RuleBasedSegmentStorageProducer ruleBasedSegmentStorage, + @NonNull GeneralInfoStorage generalInfoStorage, @NonNull TelemetryRuntimeProducer telemetryRuntimeProducer, @NonNull BackoffCounter backoffCounter, @Nullable String flagsSpec, @@ -104,7 +110,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, mTelemetryRuntimeProducer = checkNotNull(telemetryRuntimeProducer); mBackoffCounter = checkNotNull(backoffCounter); String mPreviousSpec = "1.2"; - mOutdatedSplitProxyHandler = new OutdatedSplitProxyHandler(flagsSpec, mPreviousSpec, forBackgroundSync); + mOutdatedSplitProxyHandler = new OutdatedSplitProxyHandler(flagsSpec, mPreviousSpec, forBackgroundSync, generalInfoStorage); } public SplitTaskExecutionInfo sync(SinceChangeNumbers till, int onDemandFetchBackoffMaxRetries) { @@ -117,6 +123,12 @@ public SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBeforeU private SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBeforeUpdate, boolean avoidCache, boolean resetChangeNumber, int onDemandFetchBackoffMaxRetries) { try { + OutdatedSplitProxyHandler.ProxyHandlingType proxyHandlingType = mOutdatedSplitProxyHandler.proxyCheck(); + if (proxyHandlingType == OutdatedSplitProxyHandler.ProxyHandlingType.RECOVERY) { + clearBeforeUpdate = true; + resetChangeNumber = true; + } + CdnByPassType cdnByPassType = attemptSplitSync(till, clearBeforeUpdate, avoidCache, CdnByPassType.NONE, resetChangeNumber, onDemandFetchBackoffMaxRetries); if (cdnByPassType != CdnByPassType.NONE) { @@ -154,30 +166,20 @@ private SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBefore return SplitTaskExecutionInfo.success(SplitTaskType.SPLITS_SYNC); } - private SplitTaskExecutionInfo handleOutdatedProxy(SinceChangeNumbers till, boolean avoidCache, boolean resetChangeNumber, int onDemandFetchBackoffMaxRetries) throws Exception { + private SplitTaskExecutionInfo handleOutdatedProxy(SinceChangeNumbers till, boolean ignoredAvoidCache, boolean resetChangeNumber, int onDemandFetchBackoffMaxRetries) throws Exception { OutdatedSplitProxyHandler.ProxyHandlingType handle = mOutdatedSplitProxyHandler.handle(); - switch (handle) { - case FALLBACK: { - long flagsSince = till.getFlagsSince(); - SinceChangeNumbers newTill = new SinceChangeNumbers(flagsSince, null); - attemptSplitSync(newTill, false, false, CdnByPassType.NONE, false, onDemandFetchBackoffMaxRetries); - break; - } - case RECOVERY: { - SinceChangeNumbers newTill = new SinceChangeNumbers(-1, -1L); - attemptSplitSync(newTill, true, false, CdnByPassType.NONE, true, onDemandFetchBackoffMaxRetries); - break; - } - } + long flagsSince = till.getFlagsSince(); + SinceChangeNumbers newTill = new SinceChangeNumbers(flagsSince, null); + attemptSplitSync(newTill, false, false, CdnByPassType.NONE, false, onDemandFetchBackoffMaxRetries); return SplitTaskExecutionInfo.success(SplitTaskType.SPLITS_SYNC); } /** - * @param targetChangeNumber target changeNumber - * @param clearBeforeUpdate whether to clear splits storage before updating it - * @param avoidCache whether to send no-cache header to api - * @param withCdnBypass whether to add additional query param to bypass CDN + * @param targetChangeNumber target changeNumber + * @param clearBeforeUpdate whether to clear splits storage before updating it + * @param avoidCache whether to send no-cache header to api + * @param withCdnBypass whether to add additional query param to bypass CDN * @param onDemandFetchBackoffMaxRetries max backoff retries for CDN bypass * @return whether sync finished successfully */ @@ -245,7 +247,7 @@ private TargetingRulesChange fetchSplits(SinceChangeNumbers till, boolean avoidC params.put(FLAGS_SPEC_PARAM, flagsSpec); } params.put(SINCE_PARAM, till.getFlagsSince()); - if (till.getRbsSince() != null) { + if (!mOutdatedSplitProxyHandler.isFallbackMode() && till.getRbsSince() != null) { params.put(RBS_SINCE_PARAM, till.getRbsSince()); } diff --git a/src/main/java/io/split/android/client/service/workmanager/splits/SplitsSyncWorkerTaskBuilder.java b/src/main/java/io/split/android/client/service/workmanager/splits/SplitsSyncWorkerTaskBuilder.java index 4c76c5f01..7ef5daaf4 100644 --- a/src/main/java/io/split/android/client/service/workmanager/splits/SplitsSyncWorkerTaskBuilder.java +++ b/src/main/java/io/split/android/client/service/workmanager/splits/SplitsSyncWorkerTaskBuilder.java @@ -7,6 +7,7 @@ import io.split.android.client.service.splits.SplitChangeProcessor; import io.split.android.client.service.splits.SplitsSyncHelper; import io.split.android.client.service.splits.SplitsSyncTask; +import io.split.android.client.storage.general.GeneralInfoStorage; import io.split.android.client.storage.rbs.RuleBasedSegmentStorageProducer; import io.split.android.client.storage.splits.SplitsStorage; import io.split.android.client.telemetry.storage.TelemetryStorage; @@ -43,6 +44,7 @@ SplitTask getTask() { SplitsStorage splitsStorage = mStorageProvider.provideSplitsStorage(); TelemetryStorage telemetryStorage = mStorageProvider.provideTelemetryStorage(); RuleBasedSegmentStorageProducer ruleBasedSegmentStorageProducer = mStorageProvider.provideRuleBasedSegmentStorage(); + GeneralInfoStorage generalInfoStorage = mStorageProvider.provideGeneralInfoStorage(); String splitsFilterQueryString = splitsStorage.getSplitsFilterQueryString(); SplitsSyncHelper splitsSyncHelper = mSplitsSyncHelperProvider.provideSplitsSyncHelper( @@ -51,6 +53,7 @@ SplitTask getTask() { mSplitChangeProcessor, mRuleBasedSegmentChangeProcessor, ruleBasedSegmentStorageProducer, + generalInfoStorage, telemetryStorage, mFlagsSpec); diff --git a/src/main/java/io/split/android/client/service/workmanager/splits/StorageProvider.java b/src/main/java/io/split/android/client/service/workmanager/splits/StorageProvider.java index 335bcd01b..d60e81967 100644 --- a/src/main/java/io/split/android/client/service/workmanager/splits/StorageProvider.java +++ b/src/main/java/io/split/android/client/service/workmanager/splits/StorageProvider.java @@ -4,6 +4,7 @@ import io.split.android.client.storage.cipher.SplitCipherFactory; import io.split.android.client.storage.db.SplitRoomDatabase; import io.split.android.client.storage.db.StorageFactory; +import io.split.android.client.storage.general.GeneralInfoStorage; import io.split.android.client.storage.rbs.RuleBasedSegmentStorageProducer; import io.split.android.client.storage.splits.SplitsStorage; import io.split.android.client.telemetry.storage.TelemetryStorage; @@ -37,4 +38,8 @@ RuleBasedSegmentStorageProducer provideRuleBasedSegmentStorage() { return ruleBasedSegmentStorageForWorker; } + + GeneralInfoStorage provideGeneralInfoStorage() { + return StorageFactory.getGeneralInfoStorage(mDatabase); + } } diff --git a/src/main/java/io/split/android/client/service/workmanager/splits/SyncHelperProvider.java b/src/main/java/io/split/android/client/service/workmanager/splits/SyncHelperProvider.java index 1b2965398..2fd411dbd 100644 --- a/src/main/java/io/split/android/client/service/workmanager/splits/SyncHelperProvider.java +++ b/src/main/java/io/split/android/client/service/workmanager/splits/SyncHelperProvider.java @@ -5,6 +5,7 @@ import io.split.android.client.service.rules.RuleBasedSegmentChangeProcessor; import io.split.android.client.service.splits.SplitChangeProcessor; import io.split.android.client.service.splits.SplitsSyncHelper; +import io.split.android.client.storage.general.GeneralInfoStorage; import io.split.android.client.storage.rbs.RuleBasedSegmentStorageProducer; import io.split.android.client.storage.splits.SplitsStorage; import io.split.android.client.telemetry.storage.TelemetryStorage; @@ -16,6 +17,7 @@ SplitsSyncHelper provideSplitsSyncHelper(HttpFetcher split SplitChangeProcessor splitChangeProcessor, RuleBasedSegmentChangeProcessor ruleBasedSegmentChangeProcessor, RuleBasedSegmentStorageProducer ruleBasedSegmentStorage, + GeneralInfoStorage generalInfoStorage, TelemetryStorage telemetryStorage, String mFlagsSpec) { return new SplitsSyncHelper(splitsFetcher, @@ -23,6 +25,7 @@ SplitsSyncHelper provideSplitsSyncHelper(HttpFetcher split splitChangeProcessor, ruleBasedSegmentChangeProcessor, ruleBasedSegmentStorage, + generalInfoStorage, telemetryStorage, mFlagsSpec, true); diff --git a/src/main/java/io/split/android/client/storage/general/GeneralInfoStorage.java b/src/main/java/io/split/android/client/storage/general/GeneralInfoStorage.java index f7d121cb3..87a6a55ec 100644 --- a/src/main/java/io/split/android/client/storage/general/GeneralInfoStorage.java +++ b/src/main/java/io/split/android/client/storage/general/GeneralInfoStorage.java @@ -34,4 +34,8 @@ public interface GeneralInfoStorage { long getRolloutCacheLastClearTimestamp(); void setRolloutCacheLastClearTimestamp(long timestamp); + + void setLastProxyUpdateTimestamp(long timestamp); + + long getLastProxyUpdateTimestamp(); } diff --git a/src/main/java/io/split/android/client/storage/general/GeneralInfoStorageImpl.java b/src/main/java/io/split/android/client/storage/general/GeneralInfoStorageImpl.java index adb2759b4..c351d9a48 100644 --- a/src/main/java/io/split/android/client/storage/general/GeneralInfoStorageImpl.java +++ b/src/main/java/io/split/android/client/storage/general/GeneralInfoStorageImpl.java @@ -8,10 +8,11 @@ import io.split.android.client.storage.db.GeneralInfoDao; import io.split.android.client.storage.db.GeneralInfoEntity; -public class GeneralInfoStorageImpl implements GeneralInfoStorage{ +public class GeneralInfoStorageImpl implements GeneralInfoStorage { private static final String ROLLOUT_CACHE_LAST_CLEAR_TIMESTAMP = "rolloutCacheLastClearTimestamp"; private static final String RBS_CHANGE_NUMBER = "rbsChangeNumber"; + private static final String LAST_PROXY_CHECK_TIMESTAMP = "lastProxyCheckTimestamp"; private final GeneralInfoDao mGeneralInfoDao; @@ -97,4 +98,15 @@ public long getRolloutCacheLastClearTimestamp() { public void setRolloutCacheLastClearTimestamp(long timestamp) { mGeneralInfoDao.update(new GeneralInfoEntity(ROLLOUT_CACHE_LAST_CLEAR_TIMESTAMP, timestamp)); } + + @Override + public void setLastProxyUpdateTimestamp(long timestamp) { + mGeneralInfoDao.update(new GeneralInfoEntity(LAST_PROXY_CHECK_TIMESTAMP, timestamp)); + } + + @Override + public long getLastProxyUpdateTimestamp() { + GeneralInfoEntity entity = mGeneralInfoDao.getByName(LAST_PROXY_CHECK_TIMESTAMP); + return entity != null ? entity.getLongValue() : 0L; + } } From ae417cd0243e27af67a8f2ed4ac9e62626c33c7a Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 29 Apr 2025 17:22:11 -0300 Subject: [PATCH 10/17] WIP --- .../splits/OutdatedSplitProxyHandler.java | 33 +++++++++++-------- .../service/splits/SplitsSyncHelper.java | 13 ++++---- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java index e442749db..5bf14bd84 100644 --- a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java +++ b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java @@ -14,7 +14,7 @@ public class OutdatedSplitProxyHandler { - private static final long PROXY_CHECK_INTERVAL_MILLIS = TimeUnit.SECONDS.toMillis(1); + private static final long PROXY_CHECK_INTERVAL_MILLIS = TimeUnit.HOURS.toMillis(1); private final String mLatestSpec; private final String mPreviousSpec; @@ -46,24 +46,18 @@ ProxyHandlingType handle() { } if (mCurrentSpec.get().equals(mLatestSpec)) { + updateLastProxyCheckTimestamp(System.currentTimeMillis()); return fallback(); } return ProxyHandlingType.NONE; } - @NonNull - private ProxyHandlingType fallback() { - Logger.i("Switching to previous spec: " + mPreviousSpec); - - updateLastProxyCheckTimestamp(System.currentTimeMillis()); - - mCurrentSpec.set(mPreviousSpec); - - return ProxyHandlingType.FALLBACK; - } - ProxyHandlingType proxyCheck() { + if (mForBackgroundSync) { + return ProxyHandlingType.NONE; + } + if (mCurrentSpec.get().equals(mLatestSpec)) { long lastProxyCheckTimestamp = getLastProxyCheckTimestamp(); if (lastProxyCheckTimestamp != 0L) { @@ -73,9 +67,10 @@ ProxyHandlingType proxyCheck() { mCurrentSpec.set(mLatestSpec); updateLastProxyCheckTimestamp(System.currentTimeMillis()); return ProxyHandlingType.RECOVERY; + } else { + Logger.i("No time passed since last proxy check"); + return fallback(); } - } else { - return fallback(); } } Logger.v("No need to handle outdated proxy"); @@ -83,9 +78,19 @@ ProxyHandlingType proxyCheck() { } void resetProxyCheckTimestamp() { + Logger.i("Resetting proxy check timestamp due to successful recovery"); updateLastProxyCheckTimestamp(0L); } + @NonNull + private ProxyHandlingType fallback() { + Logger.i("Switching to previous spec: " + mPreviousSpec); + + mCurrentSpec.set(mPreviousSpec); + + return ProxyHandlingType.FALLBACK; + } + private long getLastProxyCheckTimestamp() { mLastProxyCheckTimestamp.compareAndSet(0L, mGeneralInfoStorage.getLastProxyUpdateTimestamp()); return mLastProxyCheckTimestamp.get(); diff --git a/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java b/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java index 0c15dc024..d42e45dba 100644 --- a/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java +++ b/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java @@ -150,9 +150,9 @@ private SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBefore if (HttpStatus.isProxyOutdated(httpStatus)) { try { - return handleOutdatedProxy(till, avoidCache, resetChangeNumber, onDemandFetchBackoffMaxRetries); + mOutdatedSplitProxyHandler.handle(); } catch (Exception e1) { - logError("Unexpected while handling outdated proxy" + e1.getLocalizedMessage()); + logError("Unexpected while handling outdated proxy " + e1.getLocalizedMessage()); } } @@ -163,14 +163,15 @@ private SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBefore } Logger.d("Feature flags have been updated"); + +// if (mOutdatedSplitProxyHandler.isNormalMode()) { +// mOutdatedSplitProxyHandler.resetProxyCheckTimestamp(); +// } return SplitTaskExecutionInfo.success(SplitTaskType.SPLITS_SYNC); } private SplitTaskExecutionInfo handleOutdatedProxy(SinceChangeNumbers till, boolean ignoredAvoidCache, boolean resetChangeNumber, int onDemandFetchBackoffMaxRetries) throws Exception { - OutdatedSplitProxyHandler.ProxyHandlingType handle = mOutdatedSplitProxyHandler.handle(); - long flagsSince = till.getFlagsSince(); - SinceChangeNumbers newTill = new SinceChangeNumbers(flagsSince, null); - attemptSplitSync(newTill, false, false, CdnByPassType.NONE, false, onDemandFetchBackoffMaxRetries); + return SplitTaskExecutionInfo.success(SplitTaskType.SPLITS_SYNC); } From c1638e6aba8a617efd69dffde80009f58135958e Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 29 Apr 2025 22:00:06 -0300 Subject: [PATCH 11/17] Working version --- .../splits/OutdatedSplitProxyHandler.java | 78 +++++++++---------- .../service/splits/SplitsSyncHelper.java | 13 ++-- .../client/service/SplitsSyncHelperTest.java | 5 +- .../SplitsSyncWorkerTaskBuilderTest.java | 9 ++- 4 files changed, 53 insertions(+), 52 deletions(-) diff --git a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java index 5bf14bd84..07ce59374 100644 --- a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java +++ b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java @@ -2,7 +2,6 @@ import static io.split.android.client.utils.Utils.checkNotNull; -import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; import java.util.concurrent.TimeUnit; @@ -24,6 +23,7 @@ public class OutdatedSplitProxyHandler { private final AtomicLong mLastProxyCheckTimestamp = new AtomicLong(0L); private final GeneralInfoStorage mGeneralInfoStorage; + private final AtomicReference mCurrentProxyHandlingType = new AtomicReference<>(ProxyHandlingType.NONE); OutdatedSplitProxyHandler(String flagSpec, String previousSpec, boolean forBackgroundSync, GeneralInfoStorage generalInfoStorage) { this(flagSpec, previousSpec, forBackgroundSync, generalInfoStorage, PROXY_CHECK_INTERVAL_MILLIS); @@ -39,56 +39,60 @@ public class OutdatedSplitProxyHandler { mGeneralInfoStorage = checkNotNull(generalInfoStorage); } - ProxyHandlingType handle() { + void trackProxyError() { if (mForBackgroundSync) { Logger.i("Background sync fetch; skipping proxy handling"); - return ProxyHandlingType.NONE; - } - - if (mCurrentSpec.get().equals(mLatestSpec)) { + } else { updateLastProxyCheckTimestamp(System.currentTimeMillis()); - return fallback(); } - return ProxyHandlingType.NONE; + updateHandlingType(ProxyHandlingType.NONE); } - ProxyHandlingType proxyCheck() { + void performProxyCheck() { if (mForBackgroundSync) { - return ProxyHandlingType.NONE; + updateHandlingType(ProxyHandlingType.NONE); } - if (mCurrentSpec.get().equals(mLatestSpec)) { - long lastProxyCheckTimestamp = getLastProxyCheckTimestamp(); - if (lastProxyCheckTimestamp != 0L) { - // we may need to recover - if (System.currentTimeMillis() - lastProxyCheckTimestamp > mProxyCheckIntervalMillis) { - Logger.i("Attempting recovery with latest spec: " + mLatestSpec); - mCurrentSpec.set(mLatestSpec); - updateLastProxyCheckTimestamp(System.currentTimeMillis()); - return ProxyHandlingType.RECOVERY; - } else { - Logger.i("No time passed since last proxy check"); - return fallback(); - } + long lastProxyCheckTimestamp = getLastProxyCheckTimestamp(); + + if (lastProxyCheckTimestamp == 0L) { + Logger.v("Never checked proxy; continuing with latest spec"); + mCurrentSpec.set(mLatestSpec); + updateHandlingType(ProxyHandlingType.NONE); + } else if (System.currentTimeMillis() - lastProxyCheckTimestamp > mProxyCheckIntervalMillis) { + Logger.i("Time since last check elapsed. Attempting recovery with latest spec: " + mLatestSpec); + mCurrentSpec.set(mLatestSpec); + updateHandlingType(ProxyHandlingType.RECOVERY); + } else { + Logger.v("Have used proxy fallback mode; time since last check has not elapsed"); + if (mCurrentSpec.compareAndSet(mLatestSpec, mPreviousSpec)) { + Logger.i("Switching to previous spec: " + mPreviousSpec); + } else { + Logger.v("Still in proxy fallback mode"); } + updateHandlingType(ProxyHandlingType.FALLBACK); } - Logger.v("No need to handle outdated proxy"); - return ProxyHandlingType.NONE; + } + + private void updateHandlingType(ProxyHandlingType proxyHandlingType) { + mCurrentProxyHandlingType.set(proxyHandlingType); } void resetProxyCheckTimestamp() { - Logger.i("Resetting proxy check timestamp due to successful recovery"); updateLastProxyCheckTimestamp(0L); } - @NonNull - private ProxyHandlingType fallback() { - Logger.i("Switching to previous spec: " + mPreviousSpec); + String getCurrentSpec() { + return mCurrentSpec.get(); + } - mCurrentSpec.set(mPreviousSpec); + boolean isFallbackMode() { + return mCurrentProxyHandlingType.get() == ProxyHandlingType.FALLBACK; + } - return ProxyHandlingType.FALLBACK; + boolean isRecoveryMode() { + return mCurrentProxyHandlingType.get() == ProxyHandlingType.RECOVERY; } private long getLastProxyCheckTimestamp() { @@ -101,18 +105,6 @@ private void updateLastProxyCheckTimestamp(long newTimestamp) { mGeneralInfoStorage.setLastProxyUpdateTimestamp(newTimestamp); } - String getCurrentSpec() { - return mCurrentSpec.get(); - } - - boolean isFallbackMode() { - return mCurrentSpec.get().equals(mPreviousSpec); - } - - boolean isNormalMode() { - return mCurrentSpec.get().equals(mLatestSpec); - } - enum ProxyHandlingType { // no action NONE, diff --git a/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java b/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java index d42e45dba..ccacf3ebf 100644 --- a/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java +++ b/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java @@ -123,8 +123,8 @@ public SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBeforeU private SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBeforeUpdate, boolean avoidCache, boolean resetChangeNumber, int onDemandFetchBackoffMaxRetries) { try { - OutdatedSplitProxyHandler.ProxyHandlingType proxyHandlingType = mOutdatedSplitProxyHandler.proxyCheck(); - if (proxyHandlingType == OutdatedSplitProxyHandler.ProxyHandlingType.RECOVERY) { + mOutdatedSplitProxyHandler.performProxyCheck(); + if (mOutdatedSplitProxyHandler.isRecoveryMode()) { clearBeforeUpdate = true; resetChangeNumber = true; } @@ -150,7 +150,7 @@ private SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBefore if (HttpStatus.isProxyOutdated(httpStatus)) { try { - mOutdatedSplitProxyHandler.handle(); + mOutdatedSplitProxyHandler.trackProxyError(); } catch (Exception e1) { logError("Unexpected while handling outdated proxy " + e1.getLocalizedMessage()); } @@ -164,9 +164,10 @@ private SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBefore Logger.d("Feature flags have been updated"); -// if (mOutdatedSplitProxyHandler.isNormalMode()) { -// mOutdatedSplitProxyHandler.resetProxyCheckTimestamp(); -// } + if (mOutdatedSplitProxyHandler.isRecoveryMode()) { + Logger.i("Resetting proxy check timestamp due to successful recovery"); + mOutdatedSplitProxyHandler.resetProxyCheckTimestamp(); + } return SplitTaskExecutionInfo.success(SplitTaskType.SPLITS_SYNC); } diff --git a/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java b/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java index 8cb872360..fcd5f6672 100644 --- a/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java +++ b/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java @@ -42,6 +42,7 @@ import io.split.android.client.service.splits.SplitChangeProcessor; import io.split.android.client.service.splits.SplitsSyncHelper; import io.split.android.client.service.sseclient.BackoffCounter; +import io.split.android.client.storage.general.GeneralInfoStorage; import io.split.android.client.storage.rbs.RuleBasedSegmentStorageImplTest; import io.split.android.client.storage.rbs.RuleBasedSegmentStorageProducer; import io.split.android.client.storage.splits.ProcessedSplitChange; @@ -67,6 +68,8 @@ public class SplitsSyncHelperTest { private BackoffCounter mBackoffCounter; @Mock private RuleBasedSegmentStorageProducer mRuleBasedSegmentStorageProducer; + @Mock + private GeneralInfoStorage mGeneralInfoStorage; private SplitsSyncHelper mSplitsSyncHelper; @@ -81,7 +84,7 @@ public void setup() { mDefaultParams = getSinceParams(-1, -1); mSecondFetchParams = getSinceParams(1506703262916L, 262325L); when(mRuleBasedSegmentStorageProducer.getChangeNumber()).thenReturn(-1L).thenReturn(262325L); - mSplitsSyncHelper = new SplitsSyncHelper(mSplitsFetcher, mSplitsStorage, mSplitChangeProcessor, mRuleBasedSegmentChangeProcessor, mRuleBasedSegmentStorageProducer, mTelemetryRuntimeProducer, mBackoffCounter, "1.1"); + mSplitsSyncHelper = new SplitsSyncHelper(mSplitsFetcher, mSplitsStorage, mSplitChangeProcessor, mRuleBasedSegmentChangeProcessor, mRuleBasedSegmentStorageProducer, mGeneralInfoStorage, mTelemetryRuntimeProducer, mBackoffCounter, "1.1"); loadSplitChanges(); } diff --git a/src/test/java/io/split/android/client/service/workmanager/splits/SplitsSyncWorkerTaskBuilderTest.java b/src/test/java/io/split/android/client/service/workmanager/splits/SplitsSyncWorkerTaskBuilderTest.java index 9220e74b0..b5f2ade6a 100644 --- a/src/test/java/io/split/android/client/service/workmanager/splits/SplitsSyncWorkerTaskBuilderTest.java +++ b/src/test/java/io/split/android/client/service/workmanager/splits/SplitsSyncWorkerTaskBuilderTest.java @@ -22,6 +22,7 @@ import io.split.android.client.service.splits.SplitChangeProcessor; import io.split.android.client.service.splits.SplitsSyncHelper; import io.split.android.client.service.splits.SplitsSyncTask; +import io.split.android.client.storage.general.GeneralInfoStorage; import io.split.android.client.storage.rbs.RuleBasedSegmentStorageProducer; import io.split.android.client.storage.splits.SplitsStorage; import io.split.android.client.telemetry.storage.TelemetryStorage; @@ -37,6 +38,7 @@ public class SplitsSyncWorkerTaskBuilderTest { private HttpFetcher mSplitsFetcher; private TelemetryStorage mTelemetryStorage; private RuleBasedSegmentStorageProducer mRuleBasedSegmentStorageProducer; + private GeneralInfoStorage mGeneralinfoStorage; @Before public void setUp() throws URISyntaxException { @@ -49,13 +51,14 @@ public void setUp() throws URISyntaxException { mSplitsSyncHelperProvider = mock(SyncHelperProvider.class); mRuleBasedSegmentStorageProducer = mock(RuleBasedSegmentStorageProducer.class); mRuleBasedSegmentChangeProcessor = mock(RuleBasedSegmentChangeProcessor.class); + mGeneralinfoStorage = mock(GeneralInfoStorage.class); when(mSplitsStorage.getSplitsFilterQueryString()).thenReturn("filterQueryString"); when(mStorageProvider.provideSplitsStorage()).thenReturn(mSplitsStorage); when(mStorageProvider.provideRuleBasedSegmentStorage()).thenReturn(mRuleBasedSegmentStorageProducer); when(mStorageProvider.provideTelemetryStorage()).thenReturn(mTelemetryStorage); when(mFetcherProvider.provideFetcher("filterQueryString")).thenReturn(mSplitsFetcher); - when(mSplitsSyncHelperProvider.provideSplitsSyncHelper(any(), any(), any(), any(), any(), any(), any())).thenReturn(mock(SplitsSyncHelper.class)); + when(mSplitsSyncHelperProvider.provideSplitsSyncHelper(any(), any(), any(), any(), any(), any(), any(), any())).thenReturn(mock(SplitsSyncHelper.class)); } @Test @@ -96,6 +99,7 @@ public void getTaskUsesSplitsSyncHelperProviderForSplitsSyncHelper() throws URIS when(mSplitsStorage.getSplitsFilterQueryString()).thenReturn("string"); when(mStorageProvider.provideRuleBasedSegmentStorage()).thenReturn(mRuleBasedSegmentStorageProducer); when(mFetcherProvider.provideFetcher("string")).thenReturn(mSplitsFetcher); + when(mStorageProvider.provideGeneralInfoStorage()).thenReturn(mGeneralinfoStorage); SplitsSyncWorkerTaskBuilder builder = new SplitsSyncWorkerTaskBuilder( mStorageProvider, @@ -114,6 +118,7 @@ public void getTaskUsesSplitsSyncHelperProviderForSplitsSyncHelper() throws URIS mSplitChangeProcessor, mRuleBasedSegmentChangeProcessor, mRuleBasedSegmentStorageProducer, + mGeneralinfoStorage, mTelemetryStorage, "1.5"); } @@ -133,7 +138,7 @@ public void getTaskReturnsNullWhenURISyntaxExceptionIsThrown() throws URISyntaxE public void getTaskUsesSplitSyncTaskStaticMethod() { try (MockedStatic mockedStatic = mockStatic(SplitsSyncTask.class)) { SplitsSyncHelper splitsSyncHelper = mock(SplitsSyncHelper.class); - when(mSplitsSyncHelperProvider.provideSplitsSyncHelper(any(), any(), any(), any(), any(), any(), any())).thenReturn(splitsSyncHelper); + when(mSplitsSyncHelperProvider.provideSplitsSyncHelper(any(), any(), any(), any(), any(), any(), any(), any())).thenReturn(splitsSyncHelper); when(mStorageProvider.provideRuleBasedSegmentStorage()).thenReturn(mRuleBasedSegmentStorageProducer); SplitsSyncWorkerTaskBuilder builder = getSplitsSyncWorkerTaskBuilder("2.5"); From 5a02522e94c110f3e5c2b3f266329d5d0e4921f2 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Tue, 29 Apr 2025 22:26:23 -0300 Subject: [PATCH 12/17] Improvements --- .../splits/OutdatedSplitProxyHandler.java | 23 ++++++++----------- .../splits/OutdatedSplitProxyHandlerTest.java | 4 ++++ 2 files changed, 13 insertions(+), 14 deletions(-) create mode 100644 src/test/java/io/split/android/client/service/splits/OutdatedSplitProxyHandlerTest.java diff --git a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java index 07ce59374..0927b918d 100644 --- a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java +++ b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java @@ -17,7 +17,6 @@ public class OutdatedSplitProxyHandler { private final String mLatestSpec; private final String mPreviousSpec; - private final AtomicReference mCurrentSpec; private final boolean mForBackgroundSync; private final long mProxyCheckIntervalMillis; @@ -33,7 +32,6 @@ public class OutdatedSplitProxyHandler { OutdatedSplitProxyHandler(String flagSpec, String previousSpec, boolean forBackgroundSync, GeneralInfoStorage generalInfoStorage, long proxyCheckIntervalMillis) { mLatestSpec = flagSpec; mPreviousSpec = previousSpec; - mCurrentSpec = new AtomicReference<>(flagSpec); mForBackgroundSync = forBackgroundSync; mProxyCheckIntervalMillis = proxyCheckIntervalMillis; mGeneralInfoStorage = checkNotNull(generalInfoStorage); @@ -42,11 +40,11 @@ public class OutdatedSplitProxyHandler { void trackProxyError() { if (mForBackgroundSync) { Logger.i("Background sync fetch; skipping proxy handling"); + updateHandlingType(ProxyHandlingType.NONE); } else { updateLastProxyCheckTimestamp(System.currentTimeMillis()); + updateHandlingType(ProxyHandlingType.FALLBACK); } - - updateHandlingType(ProxyHandlingType.NONE); } void performProxyCheck() { @@ -58,19 +56,12 @@ void performProxyCheck() { if (lastProxyCheckTimestamp == 0L) { Logger.v("Never checked proxy; continuing with latest spec"); - mCurrentSpec.set(mLatestSpec); updateHandlingType(ProxyHandlingType.NONE); } else if (System.currentTimeMillis() - lastProxyCheckTimestamp > mProxyCheckIntervalMillis) { Logger.i("Time since last check elapsed. Attempting recovery with latest spec: " + mLatestSpec); - mCurrentSpec.set(mLatestSpec); updateHandlingType(ProxyHandlingType.RECOVERY); } else { - Logger.v("Have used proxy fallback mode; time since last check has not elapsed"); - if (mCurrentSpec.compareAndSet(mLatestSpec, mPreviousSpec)) { - Logger.i("Switching to previous spec: " + mPreviousSpec); - } else { - Logger.v("Still in proxy fallback mode"); - } + Logger.v("Have used proxy fallback mode; time since last check has not elapsed. Using previous spec"); updateHandlingType(ProxyHandlingType.FALLBACK); } } @@ -84,7 +75,11 @@ void resetProxyCheckTimestamp() { } String getCurrentSpec() { - return mCurrentSpec.get(); + if (mCurrentProxyHandlingType.get() == ProxyHandlingType.FALLBACK) { + return mPreviousSpec; + } + + return mLatestSpec; } boolean isFallbackMode() { @@ -105,7 +100,7 @@ private void updateLastProxyCheckTimestamp(long newTimestamp) { mGeneralInfoStorage.setLastProxyUpdateTimestamp(newTimestamp); } - enum ProxyHandlingType { + private enum ProxyHandlingType { // no action NONE, // switch to previous spec diff --git a/src/test/java/io/split/android/client/service/splits/OutdatedSplitProxyHandlerTest.java b/src/test/java/io/split/android/client/service/splits/OutdatedSplitProxyHandlerTest.java new file mode 100644 index 000000000..a95f95b87 --- /dev/null +++ b/src/test/java/io/split/android/client/service/splits/OutdatedSplitProxyHandlerTest.java @@ -0,0 +1,4 @@ +package io.split.android.client.service.splits; + +public class OutdatedSplitProxyHandlerTest { +} From 27ed53a5aee8db53beeab4f7de718148d3f877c8 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 30 Apr 2025 11:54:04 -0300 Subject: [PATCH 13/17] Tests for proxy handler --- .../splits/OutdatedSplitProxyHandlerTest.java | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/src/test/java/io/split/android/client/service/splits/OutdatedSplitProxyHandlerTest.java b/src/test/java/io/split/android/client/service/splits/OutdatedSplitProxyHandlerTest.java index a95f95b87..7d8792a84 100644 --- a/src/test/java/io/split/android/client/service/splits/OutdatedSplitProxyHandlerTest.java +++ b/src/test/java/io/split/android/client/service/splits/OutdatedSplitProxyHandlerTest.java @@ -1,4 +1,90 @@ package io.split.android.client.service.splits; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.junit.Before; +import org.junit.Test; + +import io.split.android.client.storage.general.GeneralInfoStorage; + public class OutdatedSplitProxyHandlerTest { + private static final String LATEST_SPEC = "1.3"; + private static final String PREVIOUS_SPEC = "1.2"; + private GeneralInfoStorage mockStorage; + private OutdatedSplitProxyHandler handler; + + @Before + public void setUp() { + mockStorage = mock(GeneralInfoStorage.class); + when(mockStorage.getLastProxyUpdateTimestamp()).thenReturn(0L); + handler = new OutdatedSplitProxyHandler(LATEST_SPEC, PREVIOUS_SPEC, false, mockStorage, 1000L); // 1s interval for tests + } + + @Test + public void initialStateIsNoneAndUsesLatestSpec() { + assertFalse(handler.isFallbackMode()); + assertFalse(handler.isRecoveryMode()); + assertEquals(LATEST_SPEC, handler.getCurrentSpec()); + } + + @Test + public void proxyErrorTriggersFallbackModeAndUsesPreviousSpec() { + handler.trackProxyError(); + assertTrue(handler.isFallbackMode()); + assertEquals(PREVIOUS_SPEC, handler.getCurrentSpec()); + } + + @Test + public void fallbackModePersistsUntilIntervalElapses() { + handler.trackProxyError(); + assertTrue(handler.isFallbackMode()); + // simulate a call to performProxyCheck within interval + handler.performProxyCheck(); + assertTrue(handler.isFallbackMode()); + assertEquals(PREVIOUS_SPEC, handler.getCurrentSpec()); + } + + @Test + public void intervalElapsedEntersRecoveryModeAndUsesLatestSpec() { + handler.trackProxyError(); + // simulate time passing (10 seconds ago) + long now = System.currentTimeMillis(); + when(mockStorage.getLastProxyUpdateTimestamp()).thenReturn(now - 10000L); + // Re-create handler to force atomic long to re-read from storage + handler = new OutdatedSplitProxyHandler(LATEST_SPEC, PREVIOUS_SPEC, false, mockStorage, 1000L); + handler.performProxyCheck(); + assertTrue(handler.isRecoveryMode()); + assertEquals(LATEST_SPEC, handler.getCurrentSpec()); + } + + @Test + public void recoveryModeResetsToNoneAfterResetProxyCheckTimestamp() { + handler.trackProxyError(); + long now = System.currentTimeMillis(); + when(mockStorage.getLastProxyUpdateTimestamp()).thenReturn(now - 10000L); + handler = new OutdatedSplitProxyHandler(LATEST_SPEC, PREVIOUS_SPEC, false, mockStorage, 1000L); + handler.performProxyCheck(); + assertTrue(handler.isRecoveryMode()); + handler.resetProxyCheckTimestamp(); + // Simulate storage now returns 0L after reset + when(mockStorage.getLastProxyUpdateTimestamp()).thenReturn(0L); + handler = new OutdatedSplitProxyHandler(LATEST_SPEC, PREVIOUS_SPEC, false, mockStorage, 1000L); + handler.performProxyCheck(); + assertFalse(handler.isFallbackMode()); + assertFalse(handler.isRecoveryMode()); + assertEquals(LATEST_SPEC, handler.getCurrentSpec()); + } + + @Test + public void settingUpForBackgroundSyncIsAlwaysInNoneMode() { + handler = new OutdatedSplitProxyHandler(LATEST_SPEC, PREVIOUS_SPEC, true, mockStorage, 1000L); + handler.performProxyCheck(); + assertFalse(handler.isFallbackMode()); + assertFalse(handler.isRecoveryMode()); + assertEquals(LATEST_SPEC, handler.getCurrentSpec()); + } } From 375c45e1bb8660563800fee097939fe4d02681ed Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 30 Apr 2025 15:29:24 -0300 Subject: [PATCH 14/17] Inject check interval --- .../splits/OutdatedSplitProxyHandler.java | 86 ++++++++++++++++--- .../service/splits/SplitsSyncHelper.java | 12 ++- 2 files changed, 80 insertions(+), 18 deletions(-) diff --git a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java index 0927b918d..545a21c44 100644 --- a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java +++ b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java @@ -4,17 +4,46 @@ import androidx.annotation.VisibleForTesting; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import io.split.android.client.storage.general.GeneralInfoStorage; import io.split.android.client.utils.logger.Logger; +/** + * Handles proxy spec fallback and recovery for Split SDK. + * + *

This class manages the state machine that determines which spec version (latest or legacy) should be used + * to communicate with the Split Proxy, based on observed proxy compatibility errors. + * It ensures that the SDK can automatically fall back to a legacy spec when the proxy is outdated, periodically + * attempt recovery, and return to normal operation if the proxy is upgraded.

+ * + *

State Machine:

+ *
    + *
  • NONE: Normal operation, using latest spec. Default state.
    • + *
  • FALLBACK: Entered when a proxy error is detected with the latest spec. SDK uses legacy spec and omits RB_SINCE param.
    • + *
  • RECOVERY: Entered after fallback interval elapses. SDK attempts to use latest spec again. If successful, returns to NONE.
    • + *
+ *

Transitions:

+ *
    + *
  • NONE --(proxy error w/ latest spec)--> FALLBACK
    • + *
  • FALLBACK --(interval elapsed)--> RECOVERY
    • + *
  • RECOVERY --(success w/ latest spec)--> NONE
    • + *
  • RECOVERY --(proxy error)--> FALLBACK
    • + *
  • FALLBACK --(generic 400)--> FALLBACK (error surfaced, no state change)
    • + *
+ *

Only an explicit proxy outdated error triggers fallback. Generic 400s do not.

+ * + *

This class provides the following functionality:

+ *
    + *
  • Tracks proxy errors and updates the state machine accordingly.
    • + *
  • Performs periodic proxy checks to attempt recovery.
    • + *
  • Provides the current spec version based on the state machine.
    • + *
  • Indicates whether the SDK is in fallback or recovery mode.
    • + *
+ */ public class OutdatedSplitProxyHandler { - private static final long PROXY_CHECK_INTERVAL_MILLIS = TimeUnit.HOURS.toMillis(1); - private final String mLatestSpec; private final String mPreviousSpec; private final boolean mForBackgroundSync; @@ -24,10 +53,15 @@ public class OutdatedSplitProxyHandler { private final GeneralInfoStorage mGeneralInfoStorage; private final AtomicReference mCurrentProxyHandlingType = new AtomicReference<>(ProxyHandlingType.NONE); - OutdatedSplitProxyHandler(String flagSpec, String previousSpec, boolean forBackgroundSync, GeneralInfoStorage generalInfoStorage) { - this(flagSpec, previousSpec, forBackgroundSync, generalInfoStorage, PROXY_CHECK_INTERVAL_MILLIS); - } - + /** + * Constructs an OutdatedSplitProxyHandler instance with a custom proxy check interval. + * + * @param flagSpec the latest spec version + * @param previousSpec the previous spec version + * @param forBackgroundSync whether this instance is for background sync + * @param generalInfoStorage the general info storage + * @param proxyCheckIntervalMillis the custom proxy check interval + */ @VisibleForTesting OutdatedSplitProxyHandler(String flagSpec, String previousSpec, boolean forBackgroundSync, GeneralInfoStorage generalInfoStorage, long proxyCheckIntervalMillis) { mLatestSpec = flagSpec; @@ -37,6 +71,9 @@ public class OutdatedSplitProxyHandler { mGeneralInfoStorage = checkNotNull(generalInfoStorage); } + /** + * Tracks a proxy error and updates the state machine accordingly. + */ void trackProxyError() { if (mForBackgroundSync) { Logger.i("Background sync fetch; skipping proxy handling"); @@ -47,6 +84,9 @@ void trackProxyError() { } } + /** + * Performs a periodic proxy check to attempt recovery. + */ void performProxyCheck() { if (mForBackgroundSync) { updateHandlingType(ProxyHandlingType.NONE); @@ -66,14 +106,15 @@ void performProxyCheck() { } } - private void updateHandlingType(ProxyHandlingType proxyHandlingType) { - mCurrentProxyHandlingType.set(proxyHandlingType); - } - void resetProxyCheckTimestamp() { updateLastProxyCheckTimestamp(0L); } + /** + * Returns the current spec version based on the state machine. + * + * @return the current spec version + */ String getCurrentSpec() { if (mCurrentProxyHandlingType.get() == ProxyHandlingType.FALLBACK) { return mPreviousSpec; @@ -82,14 +123,28 @@ String getCurrentSpec() { return mLatestSpec; } + /** + * Indicates whether the SDK is in fallback mode. + * + * @return true if in fallback mode, false otherwise + */ boolean isFallbackMode() { return mCurrentProxyHandlingType.get() == ProxyHandlingType.FALLBACK; } + /** + * Indicates whether the SDK is in recovery mode. + * + * @return true if in recovery mode, false otherwise + */ boolean isRecoveryMode() { return mCurrentProxyHandlingType.get() == ProxyHandlingType.RECOVERY; } + private void updateHandlingType(ProxyHandlingType proxyHandlingType) { + mCurrentProxyHandlingType.set(proxyHandlingType); + } + private long getLastProxyCheckTimestamp() { mLastProxyCheckTimestamp.compareAndSet(0L, mGeneralInfoStorage.getLastProxyUpdateTimestamp()); return mLastProxyCheckTimestamp.get(); @@ -100,12 +155,15 @@ private void updateLastProxyCheckTimestamp(long newTimestamp) { mGeneralInfoStorage.setLastProxyUpdateTimestamp(newTimestamp); } + /** + * Enum representing the proxy handling types. + */ private enum ProxyHandlingType { - // no action + // No action NONE, - // switch to previous spec + // Switch to previous spec FALLBACK, - // attempt recovery + // Attempt recovery RECOVERY, } } diff --git a/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java b/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java index ccacf3ebf..7409c773a 100644 --- a/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java +++ b/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java @@ -39,6 +39,7 @@ public class SplitsSyncHelper { private static final String TILL_PARAM = "till"; private static final String RBS_SINCE_PARAM = "rbSince"; private static final int ON_DEMAND_FETCH_BACKOFF_MAX_WAIT = ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_WAIT; + private static final long DEFAULT_PROXY_CHECK_INTERVAL_MILLIS = TimeUnit.HOURS.toMillis(1); private final HttpFetcher mSplitFetcher; private final SplitsStorage mSplitsStorage; @@ -67,7 +68,8 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, telemetryRuntimeProducer, new ReconnectBackoffCounter(1, ON_DEMAND_FETCH_BACKOFF_MAX_WAIT), flagsSpec, - forBackgroundSync); + forBackgroundSync, + DEFAULT_PROXY_CHECK_INTERVAL_MILLIS); } public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, @@ -88,7 +90,8 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, telemetryRuntimeProducer, backoffCounter, flagsSpec, - false); + false, + DEFAULT_PROXY_CHECK_INTERVAL_MILLIS); } @VisibleForTesting @@ -101,7 +104,8 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, @NonNull TelemetryRuntimeProducer telemetryRuntimeProducer, @NonNull BackoffCounter backoffCounter, @Nullable String flagsSpec, - boolean forBackgroundSync) { + boolean forBackgroundSync, + long proxyCheckIntervalMillis) { mSplitFetcher = checkNotNull(splitFetcher); mSplitsStorage = checkNotNull(splitsStorage); mSplitChangeProcessor = checkNotNull(splitChangeProcessor); @@ -110,7 +114,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, mTelemetryRuntimeProducer = checkNotNull(telemetryRuntimeProducer); mBackoffCounter = checkNotNull(backoffCounter); String mPreviousSpec = "1.2"; - mOutdatedSplitProxyHandler = new OutdatedSplitProxyHandler(flagsSpec, mPreviousSpec, forBackgroundSync, generalInfoStorage); + mOutdatedSplitProxyHandler = new OutdatedSplitProxyHandler(flagsSpec, mPreviousSpec, forBackgroundSync, generalInfoStorage, proxyCheckIntervalMillis); } public SplitTaskExecutionInfo sync(SinceChangeNumbers till, int onDemandFetchBackoffMaxRetries) { From 8f55b66365936731225781c2c3c474adf33a6677 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Wed, 30 Apr 2025 15:29:34 -0300 Subject: [PATCH 15/17] WIP new sync helper tests --- .../client/service/SplitsSyncHelperTest.java | 146 +++++++++++++++++- 1 file changed, 139 insertions(+), 7 deletions(-) diff --git a/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java b/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java index fcd5f6672..5e3169da2 100644 --- a/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java +++ b/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java @@ -2,12 +2,15 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -17,7 +20,9 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; +import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.mockito.Spy; @@ -28,6 +33,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import io.split.android.client.dtos.RuleBasedSegment; import io.split.android.client.dtos.RuleBasedSegmentChange; @@ -38,6 +44,7 @@ import io.split.android.client.service.executor.SplitTaskExecutionStatus; import io.split.android.client.service.http.HttpFetcher; import io.split.android.client.service.http.HttpFetcherException; +import io.split.android.client.service.http.HttpStatus; import io.split.android.client.service.rules.RuleBasedSegmentChangeProcessor; import io.split.android.client.service.splits.SplitChangeProcessor; import io.split.android.client.service.splits.SplitsSyncHelper; @@ -84,7 +91,8 @@ public void setup() { mDefaultParams = getSinceParams(-1, -1); mSecondFetchParams = getSinceParams(1506703262916L, 262325L); when(mRuleBasedSegmentStorageProducer.getChangeNumber()).thenReturn(-1L).thenReturn(262325L); - mSplitsSyncHelper = new SplitsSyncHelper(mSplitsFetcher, mSplitsStorage, mSplitChangeProcessor, mRuleBasedSegmentChangeProcessor, mRuleBasedSegmentStorageProducer, mGeneralInfoStorage, mTelemetryRuntimeProducer, mBackoffCounter, "1.1"); + // Use a short proxy check interval for all tests + mSplitsSyncHelper = new SplitsSyncHelper(mSplitsFetcher, mSplitsStorage, mSplitChangeProcessor, mRuleBasedSegmentChangeProcessor, mRuleBasedSegmentStorageProducer, mGeneralInfoStorage, mTelemetryRuntimeProducer, mBackoffCounter, "1.3", false, 1L); loadSplitChanges(); } @@ -120,10 +128,6 @@ public void correctSyncExecution() throws HttpFetcherException { assertEquals(SplitTaskExecutionStatus.SUCCESS, result.getStatus()); } - private static SplitsSyncHelper.SinceChangeNumbers getSinceChangeNumbers(int flagsSince, long rbsSince) { - return new SplitsSyncHelper.SinceChangeNumbers(flagsSince, rbsSince); - } - @Test public void correctSyncExecutionNoCache() throws HttpFetcherException { // On correct execution without having clear param @@ -359,7 +363,7 @@ public void replaceTillWhenFilterHasChanged() throws HttpFetcherException { mSplitsSyncHelper.sync(getSinceChangeNumbers(14829471, -1), true, true, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); Map params = new HashMap<>(); - params.put("s", "1.1"); + params.put("s", "1.3"); params.put("since", -1L); params.put("rbSince", -1L); verify(mSplitsFetcher).execute(eq(params), eq(null)); @@ -424,6 +428,134 @@ public boolean matches(Map argument) { }), any()); } + @Test + public void proxyErrorTriggersFallbackAndOmitsRbSince() throws Exception { + // Before first sync (no fallback) + when(mGeneralInfoStorage.getLastProxyUpdateTimestamp()).thenReturn(0L); + + // Simulate proxy outdated error (400 with latest spec) + when(mSplitsFetcher.execute(any(), any())) + .thenThrow(new HttpFetcherException("Proxy outdated", "Proxy outdated", HttpStatus.INTERNAL_PROXY_OUTDATED.getCode())) // Use real status code + .thenReturn(TargetingRulesChange.create(SplitChange.create(-1, 2, Collections.emptyList()), RuleBasedSegmentChange.create(-1, 2, Collections.emptyList()))) + .thenReturn(TargetingRulesChange.create(SplitChange.create(2, 2, Collections.emptyList()), RuleBasedSegmentChange.create(2, 2, Collections.emptyList()))); + when(mSplitsStorage.getTill()).thenReturn(-1L, -1L); + + // First sync triggers the proxy error and sets fallback mode + try { + mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + } catch (Exception ignored) {} + + // Now simulate fallback state persisted + when(mGeneralInfoStorage.getLastProxyUpdateTimestamp()).thenReturn(System.currentTimeMillis()); + + // Second sync, now in fallback mode, should use fallback spec and omit rbSince + mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + + // Capture and verify the params + ArgumentCaptor paramsCaptor = ArgumentCaptor.forClass(Map.class); + verify(mSplitsFetcher, atLeastOnce()).execute(paramsCaptor.capture(), any()); + boolean foundFallback = false; + for (Map params : paramsCaptor.getAllValues()) { + System.out.println("Captured params: " + params); + + if ("1.2".equals(params.get("s")) && params.get("since") != null && !params.containsKey("rbSince")) { + foundFallback = true; + break; + } + } + assertTrue("Expected a fallback call with s=1.2 and no rbSince", foundFallback); + } + + @Test + public void fallbackPersistsUntilIntervalElapses() throws Exception { + // Simulate proxy outdated error + when(mSplitsFetcher.execute(any(), any())) + .thenThrow(new HttpFetcherException("Proxy outdated", "Proxy outdated", HttpStatus.INTERNAL_PROXY_OUTDATED.getCode())) + // First fallback fetch returns till=2, second fallback fetch returns till=2 (still not caught up), + // third fallback fetch returns till=3 (caught up, loop can exit) + .thenReturn(TargetingRulesChange.create(SplitChange.create(-1, 2, Collections.emptyList()), RuleBasedSegmentChange.create(-1, 2, Collections.emptyList()))) + .thenReturn(TargetingRulesChange.create(SplitChange.create(2, 3, Collections.emptyList()), RuleBasedSegmentChange.create(2, 3, Collections.emptyList()))); + // Simulate advancing change numbers for storage + when(mSplitsStorage.getTill()).thenReturn(-1L, 2L, 3L); + // Trigger fallback + try { mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); } catch (Exception ignored) {} + // Simulate time NOT elapsed + when(mGeneralInfoStorage.getLastProxyUpdateTimestamp()).thenReturn(System.currentTimeMillis()); + mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + verify(mSplitsFetcher, times(2)).execute(argThat(params -> + "1.2".equals(params.get("s")) && + !params.containsKey("rbSince") + ), any()); + } + + @Test + public void recoveryAttemptsWithLatestSpecAfterInterval() throws Exception { + // Simulate proxy outdated error + when(mSplitsFetcher.execute(any(), any())) + .thenThrow(new HttpFetcherException("Proxy outdated", "Proxy outdated", HttpStatus.INTERNAL_PROXY_OUTDATED.getCode())) + .thenReturn(TargetingRulesChange.create(SplitChange.create(-1, 2, Collections.emptyList()), RuleBasedSegmentChange.create(-1, 2, Collections.emptyList()))) + .thenReturn(TargetingRulesChange.create(SplitChange.create(2, 2, Collections.emptyList()), RuleBasedSegmentChange.create(2, 2, Collections.emptyList()))); + when(mSplitsStorage.getTill()).thenReturn(-1L, -1L, 2L); + // Trigger fallback + try { mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); } catch (Exception ignored) {} + // Simulate interval elapsed + when(mGeneralInfoStorage.getLastProxyUpdateTimestamp()).thenReturn(System.currentTimeMillis() - TimeUnit.HOURS.toMillis(1)); + mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + // Should now attempt with latest spec + verify(mSplitsFetcher, times(1)).execute(argThat(params -> + "1.3".equals(params.get("s")) && + params.containsKey("rbSince") + ), any()); + // Assert that storage is cleared before update during recovery + InOrder inOrder = inOrder(mSplitsStorage, mRuleBasedSegmentStorageProducer); + inOrder.verify(mSplitsStorage).clear(); + inOrder.verify(mRuleBasedSegmentStorageProducer).clear(); + inOrder.verify(mSplitsStorage).update(any()); + inOrder.verify(mRuleBasedSegmentStorageProducer).update(any(), any(), any()); + } + + @Test + public void generic400InFallbackDoesNotResetToNone() throws Exception { + // Simulate proxy outdated error + when(mSplitsFetcher.execute(any(), any())) + .thenThrow(new HttpFetcherException("Proxy outdated", "Proxy outdated", HttpStatus.INTERNAL_PROXY_OUTDATED.getCode())) + .thenThrow(new HttpFetcherException("Bad Request", "Bad Request", 400)); + when(mSplitsStorage.getTill()).thenReturn(-1L); + // Trigger fallback + mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + // Next call gets a generic 400, should remain in fallback + mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + verify(mSplitsFetcher, times(2)).execute(argThat(params -> + "1.2".equals(params.get("s")) && + !params.containsKey("rbSince") + ), any()); + } + + @Test + public void successfulRecoveryReturnsToNormalSpec() throws Exception { + // Simulate proxy outdated error, then recovery + when(mSplitsFetcher.execute(any(), any())) + .thenThrow(new HttpFetcherException("Proxy outdated", "Proxy outdated", HttpStatus.INTERNAL_PROXY_OUTDATED.getCode())) + .thenReturn(TargetingRulesChange.create(SplitChange.create(-1, 2, Collections.emptyList()), RuleBasedSegmentChange.create(-1, 2, Collections.emptyList()))) + .thenReturn(TargetingRulesChange.create(SplitChange.create(2, 2, Collections.emptyList()), RuleBasedSegmentChange.create(2, 2, Collections.emptyList()))); + when(mSplitsStorage.getTill()).thenReturn(-1L); + // Trigger fallback + try { mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); } catch (Exception ignored) {} + // Simulate interval elapsed + when(mGeneralInfoStorage.getLastProxyUpdateTimestamp()).thenReturn(System.currentTimeMillis() - TimeUnit.HOURS.toMillis(1)); + mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + // Next call should be with latest spec + verify(mSplitsFetcher, times(1)).execute(argThat(params -> + "1.3".equals(params.get("s")) && + params.containsKey("rbSince") + ), any()); + } + + private static SplitsSyncHelper.SinceChangeNumbers getSinceChangeNumbers(int flagsSince, long rbsSince) { + return new SplitsSyncHelper.SinceChangeNumbers(flagsSince, rbsSince); + } + private void loadSplitChanges() { if (mTargetingRulesChange == null) { FileHelper fileHelper = new FileHelper(); @@ -433,7 +565,7 @@ private void loadSplitChanges() { private Map getSinceParams(long since, long rbSince) { Map params = new LinkedHashMap<>(); - params.put("s", "1.1"); + params.put("s", "1.3"); params.put("since", since); params.put("rbSince", rbSince); From 84b2b4904bd55b928105a0d1f3ad9144cae13389 Mon Sep 17 00:00:00 2001 From: Gaston Thea Date: Fri, 2 May 2025 18:25:39 -0300 Subject: [PATCH 16/17] Fixes --- build.gradle | 2 +- .../splits/OutdatedSplitProxyHandler.java | 15 ++++---- .../service/splits/SplitsSyncHelper.java | 3 +- .../client/service/SplitsSyncHelperTest.java | 35 +++---------------- 4 files changed, 13 insertions(+), 42 deletions(-) diff --git a/build.gradle b/build.gradle index c6bc7c13e..74a039591 100644 --- a/build.gradle +++ b/build.gradle @@ -17,7 +17,7 @@ apply plugin: 'kotlin-android' apply from: 'spec.gradle' ext { - splitVersion = '5.2.0-alpha.2' + splitVersion = '5.3.0-alpha.1' } android { diff --git a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java index 545a21c44..9e028254c 100644 --- a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java +++ b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java @@ -11,7 +11,7 @@ import io.split.android.client.utils.logger.Logger; /** - * Handles proxy spec fallback and recovery for Split SDK. + * Handles proxy spec fallback and recovery. * *

This class manages the state machine that determines which spec version (latest or legacy) should be used * to communicate with the Split Proxy, based on observed proxy compatibility errors. @@ -34,16 +34,11 @@ * *

Only an explicit proxy outdated error triggers fallback. Generic 400s do not.

* - *

This class provides the following functionality:

- *
    - *
  • Tracks proxy errors and updates the state machine accordingly.
    • - *
  • Performs periodic proxy checks to attempt recovery.
    • - *
  • Provides the current spec version based on the state machine.
    • - *
  • Indicates whether the SDK is in fallback or recovery mode.
    • - *
*/ public class OutdatedSplitProxyHandler { + private static final String PREVIOUS_SPEC = "1.2"; + private final String mLatestSpec; private final String mPreviousSpec; private final boolean mForBackgroundSync; @@ -53,6 +48,10 @@ public class OutdatedSplitProxyHandler { private final GeneralInfoStorage mGeneralInfoStorage; private final AtomicReference mCurrentProxyHandlingType = new AtomicReference<>(ProxyHandlingType.NONE); + OutdatedSplitProxyHandler(String flagSpec, boolean forBackgroundSync, GeneralInfoStorage generalInfoStorage, long proxyCheckIntervalMillis) { + this(flagSpec, PREVIOUS_SPEC, forBackgroundSync, generalInfoStorage, proxyCheckIntervalMillis); + } + /** * Constructs an OutdatedSplitProxyHandler instance with a custom proxy check interval. * diff --git a/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java b/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java index 7409c773a..e13f5a85b 100644 --- a/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java +++ b/src/main/java/io/split/android/client/service/splits/SplitsSyncHelper.java @@ -113,8 +113,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, mRuleBasedSegmentStorage = checkNotNull(ruleBasedSegmentStorage); mTelemetryRuntimeProducer = checkNotNull(telemetryRuntimeProducer); mBackoffCounter = checkNotNull(backoffCounter); - String mPreviousSpec = "1.2"; - mOutdatedSplitProxyHandler = new OutdatedSplitProxyHandler(flagsSpec, mPreviousSpec, forBackgroundSync, generalInfoStorage, proxyCheckIntervalMillis); + mOutdatedSplitProxyHandler = new OutdatedSplitProxyHandler(flagsSpec, forBackgroundSync, generalInfoStorage, proxyCheckIntervalMillis); } public SplitTaskExecutionInfo sync(SinceChangeNumbers till, int onDemandFetchBackoffMaxRetries) { diff --git a/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java b/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java index 5e3169da2..b6e0793ec 100644 --- a/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java +++ b/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java @@ -10,7 +10,6 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -22,7 +21,6 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; -import org.mockito.InOrder; import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.mockito.Spy; @@ -469,51 +467,26 @@ public void proxyErrorTriggersFallbackAndOmitsRbSince() throws Exception { @Test public void fallbackPersistsUntilIntervalElapses() throws Exception { // Simulate proxy outdated error + long timestamp = System.currentTimeMillis(); + when(mGeneralInfoStorage.getLastProxyUpdateTimestamp()).thenReturn(timestamp); when(mSplitsFetcher.execute(any(), any())) .thenThrow(new HttpFetcherException("Proxy outdated", "Proxy outdated", HttpStatus.INTERNAL_PROXY_OUTDATED.getCode())) // First fallback fetch returns till=2, second fallback fetch returns till=2 (still not caught up), // third fallback fetch returns till=3 (caught up, loop can exit) .thenReturn(TargetingRulesChange.create(SplitChange.create(-1, 2, Collections.emptyList()), RuleBasedSegmentChange.create(-1, 2, Collections.emptyList()))) - .thenReturn(TargetingRulesChange.create(SplitChange.create(2, 3, Collections.emptyList()), RuleBasedSegmentChange.create(2, 3, Collections.emptyList()))); + .thenReturn(TargetingRulesChange.create(SplitChange.create(3, 3, Collections.emptyList()), RuleBasedSegmentChange.create(3, 3, Collections.emptyList()))); // Simulate advancing change numbers for storage when(mSplitsStorage.getTill()).thenReturn(-1L, 2L, 3L); // Trigger fallback try { mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); } catch (Exception ignored) {} // Simulate time NOT elapsed - when(mGeneralInfoStorage.getLastProxyUpdateTimestamp()).thenReturn(System.currentTimeMillis()); mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); - verify(mSplitsFetcher, times(2)).execute(argThat(params -> + verify(mSplitsFetcher, times(1)).execute(argThat(params -> "1.2".equals(params.get("s")) && !params.containsKey("rbSince") ), any()); } - @Test - public void recoveryAttemptsWithLatestSpecAfterInterval() throws Exception { - // Simulate proxy outdated error - when(mSplitsFetcher.execute(any(), any())) - .thenThrow(new HttpFetcherException("Proxy outdated", "Proxy outdated", HttpStatus.INTERNAL_PROXY_OUTDATED.getCode())) - .thenReturn(TargetingRulesChange.create(SplitChange.create(-1, 2, Collections.emptyList()), RuleBasedSegmentChange.create(-1, 2, Collections.emptyList()))) - .thenReturn(TargetingRulesChange.create(SplitChange.create(2, 2, Collections.emptyList()), RuleBasedSegmentChange.create(2, 2, Collections.emptyList()))); - when(mSplitsStorage.getTill()).thenReturn(-1L, -1L, 2L); - // Trigger fallback - try { mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); } catch (Exception ignored) {} - // Simulate interval elapsed - when(mGeneralInfoStorage.getLastProxyUpdateTimestamp()).thenReturn(System.currentTimeMillis() - TimeUnit.HOURS.toMillis(1)); - mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); - // Should now attempt with latest spec - verify(mSplitsFetcher, times(1)).execute(argThat(params -> - "1.3".equals(params.get("s")) && - params.containsKey("rbSince") - ), any()); - // Assert that storage is cleared before update during recovery - InOrder inOrder = inOrder(mSplitsStorage, mRuleBasedSegmentStorageProducer); - inOrder.verify(mSplitsStorage).clear(); - inOrder.verify(mRuleBasedSegmentStorageProducer).clear(); - inOrder.verify(mSplitsStorage).update(any()); - inOrder.verify(mRuleBasedSegmentStorageProducer).update(any(), any(), any()); - } - @Test public void generic400InFallbackDoesNotResetToNone() throws Exception { // Simulate proxy outdated error From a5e1a1cfcacba834bf6b4494b8ee9759621ebaee Mon Sep 17 00:00:00 2001 From: gthea Date: Mon, 12 May 2025 09:54:16 -0300 Subject: [PATCH 17/17] Outdated proxy handling integration tests (#765) --- .../assets/split_changes_legacy.json | 121 +++++++++++ .../java/helper/IntegrationHelper.java | 20 +- .../largesegments/LargeSegmentTestHelper.java | 6 +- .../rbs/OutdatedProxyIntegrationTest.java | 194 ++++++++++++++++++ .../rbs/RuleBasedSegmentsIntegrationTest.java | 8 +- .../android/client/SplitClientConfig.java | 2 +- .../client/service/splits/LoadSplitsTask.java | 4 +- .../splits/OutdatedSplitProxyHandler.java | 1 - .../storage/cipher/ApplyCipherTask.java | 4 +- .../splits/SqLitePersistentSplitsStorage.java | 23 ++- .../SqLitePersistentSplitsStorageTest.java | 4 + 11 files changed, 360 insertions(+), 27 deletions(-) create mode 100644 src/androidTest/assets/split_changes_legacy.json create mode 100644 src/androidTest/java/tests/integration/rbs/OutdatedProxyIntegrationTest.java diff --git a/src/androidTest/assets/split_changes_legacy.json b/src/androidTest/assets/split_changes_legacy.json new file mode 100644 index 000000000..f52d7fa54 --- /dev/null +++ b/src/androidTest/assets/split_changes_legacy.json @@ -0,0 +1,121 @@ +{ + "splits": [ + { + "trafficTypeName": "account", + "name": "FACUNDO_TEST", + "trafficAllocation": 59, + "trafficAllocationSeed": -2108186082, + "seed": -1947050785, + "status": "ACTIVE", + "killed": false, + "defaultTreatment": "off", + "changeNumber": 1506703262916, + "algo": 2, + "conditions": [ + { + "conditionType": "WHITELIST", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": null, + "matcherType": "WHITELIST", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": { + "whitelist": [ + "nico_test", + "othertest" + ] + }, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 100 + } + ], + "label": "whitelisted" + }, + { + "conditionType": "WHITELIST", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": null, + "matcherType": "WHITELIST", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": { + "whitelist": [ + "bla" + ] + }, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "off", + "size": 100 + } + ], + "label": "whitelisted" + }, + { + "conditionType": "ROLLOUT", + "matcherGroup": { + "combiner": "AND", + "matchers": [ + { + "keySelector": { + "trafficType": "account", + "attribute": null + }, + "matcherType": "ALL_KEYS", + "negate": false, + "userDefinedSegmentMatcherData": null, + "whitelistMatcherData": null, + "unaryNumericMatcherData": null, + "betweenMatcherData": null, + "booleanMatcherData": null, + "dependencyMatcherData": null, + "stringMatcherData": null + } + ] + }, + "partitions": [ + { + "treatment": "on", + "size": 0 + }, + { + "treatment": "off", + "size": 100 + }, + { + "treatment": "visa", + "size": 0 + } + ], + "label": "in segment all" + } + ] + } + ], + "since": -1, + "till": 1506703262916 +} \ No newline at end of file diff --git a/src/androidTest/java/helper/IntegrationHelper.java b/src/androidTest/java/helper/IntegrationHelper.java index 1e53c36ef..df13116fb 100644 --- a/src/androidTest/java/helper/IntegrationHelper.java +++ b/src/androidTest/java/helper/IntegrationHelper.java @@ -344,14 +344,22 @@ public static String rbsChange(String changeNumber, String previousChangeNumber, } public static String loadSplitChanges(Context context, String fileName) { - FileHelper fileHelper = new FileHelper(); - String change = fileHelper.loadFileContent(context, fileName); + String change = getFileContentsAsString(context, fileName); TargetingRulesChange targetingRulesChange = Json.fromJson(change, TargetingRulesChange.class); SplitChange parsedChange = targetingRulesChange.getFeatureFlagsChange(); parsedChange.since = parsedChange.till; return Json.toJson(TargetingRulesChange.create(parsedChange, targetingRulesChange.getRuleBasedSegmentsChange())); } + public static String loadLegacySplitChanges(Context context, String fileName) { + return getFileContentsAsString(context, fileName); + } + + private static String getFileContentsAsString(Context context, String fileName) { + FileHelper fileHelper = new FileHelper(); + return fileHelper.loadFileContent(context, fileName); + } + /** * Builds a dispatcher with the given responses. * @@ -442,6 +450,14 @@ public static String getRbSinceFromUri(URI uri) { } } + public static String getSpecFromUri(URI uri) { + try { + return parse(uri.getQuery()).get("s"); + } catch (UnsupportedEncodingException e) { + throw new RuntimeException(e); + } + } + static Map parse(String query) throws UnsupportedEncodingException { Map queryPairs = new HashMap<>(); String[] pairs = query.split("&"); diff --git a/src/androidTest/java/tests/integration/largesegments/LargeSegmentTestHelper.java b/src/androidTest/java/tests/integration/largesegments/LargeSegmentTestHelper.java index 3e30292f5..c19195a2b 100644 --- a/src/androidTest/java/tests/integration/largesegments/LargeSegmentTestHelper.java +++ b/src/androidTest/java/tests/integration/largesegments/LargeSegmentTestHelper.java @@ -65,8 +65,8 @@ public MockResponse dispatch(RecordedRequest request) throws InterruptedExceptio return new MockResponse().setResponseCode(500); } - if (request.getRequestUrl().encodedPathSegments().contains("splitChanges")) { - updateEndpointHit("splitChanges"); + if (request.getRequestUrl().encodedPathSegments().contains(IntegrationHelper.ServicePath.SPLIT_CHANGES)) { + updateEndpointHit(IntegrationHelper.ServicePath.SPLIT_CHANGES); return new MockResponse().setResponseCode(200).setBody(splitChangesLargeSegments(1602796638344L, 1602796638344L)); } else if (request.getRequestUrl().encodedPathSegments().contains(IntegrationHelper.ServicePath.MEMBERSHIPS)) { Thread.sleep(mMySegmentsDelay.get()); @@ -93,7 +93,7 @@ public void tearDown() throws IOException { private void initializeLatches() { mLatches = new ConcurrentHashMap<>(); - mLatches.put("splitChanges", new CountDownLatch(1)); + mLatches.put(IntegrationHelper.ServicePath.SPLIT_CHANGES, new CountDownLatch(1)); mLatches.put(IntegrationHelper.ServicePath.MEMBERSHIPS, new CountDownLatch(1)); } diff --git a/src/androidTest/java/tests/integration/rbs/OutdatedProxyIntegrationTest.java b/src/androidTest/java/tests/integration/rbs/OutdatedProxyIntegrationTest.java new file mode 100644 index 000000000..09bcab6c7 --- /dev/null +++ b/src/androidTest/java/tests/integration/rbs/OutdatedProxyIntegrationTest.java @@ -0,0 +1,194 @@ +package tests.integration.rbs; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static helper.IntegrationHelper.emptyTargetingRulesChanges; +import static helper.IntegrationHelper.getSinceFromUri; +import static helper.IntegrationHelper.getSpecFromUri; + +import android.content.Context; + +import androidx.annotation.NonNull; +import androidx.test.platform.app.InstrumentationRegistry; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; + +import helper.DatabaseHelper; +import helper.IntegrationHelper; +import helper.TestableSplitConfigBuilder; +import io.split.android.client.ServiceEndpoints; +import io.split.android.client.SplitClient; +import io.split.android.client.SplitFactory; +import io.split.android.client.TestingConfig; +import io.split.android.client.events.SplitEvent; +import io.split.android.client.storage.db.GeneralInfoEntity; +import io.split.android.client.storage.db.SplitRoomDatabase; +import okhttp3.mockwebserver.Dispatcher; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; +import okhttp3.mockwebserver.RecordedRequest; +import tests.integration.shared.TestingHelper; + +public class OutdatedProxyIntegrationTest { + + private final Context mContext = InstrumentationRegistry.getInstrumentation().getContext(); + + private MockWebServer mWebServer; + private Map mEndpointHits; + private Map mLatches; + private final AtomicBoolean mOutdatedProxy = new AtomicBoolean(false); + private final AtomicBoolean mSimulatedProxyError = new AtomicBoolean(false); + private final AtomicBoolean mRecoveryHit = new AtomicBoolean(false); + + @Before + public void setUp() throws IOException { + mEndpointHits = new ConcurrentHashMap<>(); + mOutdatedProxy.set(false); + initializeLatches(); + + mWebServer = new MockWebServer(); + mWebServer.setDispatcher(new Dispatcher() { + @NonNull + @Override + public MockResponse dispatch(@NonNull RecordedRequest request) { + if (request.getRequestUrl().encodedPathSegments().contains(IntegrationHelper.ServicePath.SPLIT_CHANGES)) { + updateEndpointHit(IntegrationHelper.ServicePath.SPLIT_CHANGES); + float specFromUri = Float.parseFloat(getSpecFromUri(request.getRequestUrl().uri())); + if (mOutdatedProxy.get() && specFromUri > 1.2f) { + mSimulatedProxyError.set(true); + return new MockResponse().setResponseCode(400); + } else if (mOutdatedProxy.get()) { + String body = (getSinceFromUri(request.getRequestUrl().uri()).equals("-1")) ? + IntegrationHelper.loadLegacySplitChanges(mContext, "split_changes_legacy.json") : + emptyTargetingRulesChanges(1506703262916L, -1L); + return new MockResponse().setResponseCode(200) + .setBody(body); + } + + if (!mOutdatedProxy.get()) { + if (request.getRequestUrl().uri().toString().contains("?s=1.3&since=-1&rbSince=-1")) { + mRecoveryHit.set(true); + } + } + + return new MockResponse().setResponseCode(200).setBody(IntegrationHelper.loadSplitChanges(mContext, "split_changes_rbs.json")); + } else if (request.getRequestUrl().encodedPathSegments().contains(IntegrationHelper.ServicePath.MEMBERSHIPS)) { + updateEndpointHit(IntegrationHelper.ServicePath.MEMBERSHIPS); + + return new MockResponse().setResponseCode(200).setBody(IntegrationHelper.emptyAllSegments()); + } else { + return new MockResponse().setResponseCode(404); + } + } + }); + mWebServer.start(); + } + + @After + public void tearDown() throws IOException { + mWebServer.shutdown(); + } + + @Test + public void clientIsReadyEvenWhenUsingOutdatedProxy() { + mOutdatedProxy.set(true); + SplitClient readyClient = getReadyClient(IntegrationHelper.dummyUserKey().matchingKey(), getFactory()); + + assertNotNull(readyClient); + assertFalse(mRecoveryHit.get()); + assertTrue(mSimulatedProxyError.get()); + } + + @Test + public void clientIsReadyWithLatestProxy() { + mOutdatedProxy.set(false); + SplitClient readyClient = getReadyClient(IntegrationHelper.dummyUserKey().matchingKey(), getFactory()); + + assertNotNull(readyClient); + assertFalse(mRecoveryHit.get() && mOutdatedProxy.get()); + assertFalse(mSimulatedProxyError.get()); + } + + @Test + public void clientRecoversFromOutdatedProxy() { + mOutdatedProxy.set(false); + SplitRoomDatabase database = DatabaseHelper.getTestDatabase(mContext); + database.generalInfoDao().update(new GeneralInfoEntity("lastProxyCheckTimestamp", System.currentTimeMillis() - TimeUnit.MINUTES.toMillis(62))); + SplitClient readyClient = getReadyClient(IntegrationHelper.dummyUserKey().matchingKey(), getFactory(database)); + + assertNotNull(readyClient); + assertTrue(mRecoveryHit.get() && !mOutdatedProxy.get()); + assertFalse(mSimulatedProxyError.get()); + } + + private void initializeLatches() { + mLatches = new ConcurrentHashMap<>(); + mLatches.put(IntegrationHelper.ServicePath.SPLIT_CHANGES, new CountDownLatch(1)); + mLatches.put(IntegrationHelper.ServicePath.MEMBERSHIPS, new CountDownLatch(1)); + } + + private void updateEndpointHit(String splitChanges) { + if (mEndpointHits.containsKey(splitChanges)) { + mEndpointHits.get(splitChanges).getAndIncrement(); + } else { + mEndpointHits.put(splitChanges, new AtomicInteger(1)); + } + + if (mLatches.containsKey(splitChanges)) { + mLatches.get(splitChanges).countDown(); + } + } + + protected SplitFactory getFactory() { + return getFactory(null); + } + + protected SplitFactory getFactory(SplitRoomDatabase database) { + TestableSplitConfigBuilder configBuilder = new TestableSplitConfigBuilder() + .enableDebug() + .serviceEndpoints(ServiceEndpoints.builder() + .apiEndpoint("http://" + mWebServer.getHostName() + ":" + mWebServer.getPort()) + .build()); + + configBuilder.streamingEnabled(false); + configBuilder.ready(10000); + TestingConfig testingConfig = new TestingConfig(); + testingConfig.setFlagsSpec("1.3"); + return IntegrationHelper.buildFactory( + IntegrationHelper.dummyApiKey(), + IntegrationHelper.dummyUserKey(), + configBuilder.build(), + mContext, + null, database, null, testingConfig, null); + } + + protected SplitClient getReadyClient(String matchingKey, SplitFactory factory) { + CountDownLatch countDownLatch = new CountDownLatch(1); + + SplitClient client = factory.client(matchingKey); + boolean await; + client.on(SplitEvent.SDK_READY, TestingHelper.testTask(countDownLatch)); + try { + await = countDownLatch.await(10, TimeUnit.SECONDS); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + if (!await) { + fail("Client is not ready"); + } + + return client; + } +} diff --git a/src/androidTest/java/tests/integration/rbs/RuleBasedSegmentsIntegrationTest.java b/src/androidTest/java/tests/integration/rbs/RuleBasedSegmentsIntegrationTest.java index 164405fd4..25e663f86 100644 --- a/src/androidTest/java/tests/integration/rbs/RuleBasedSegmentsIntegrationTest.java +++ b/src/androidTest/java/tests/integration/rbs/RuleBasedSegmentsIntegrationTest.java @@ -226,12 +226,11 @@ public void onPostExecutionView(SplitClient client) { private Map getStringResponseClosureMap(CountDownLatch authLatch) { Map responses = new HashMap<>(); responses.put(IntegrationHelper.ServicePath.SPLIT_CHANGES, (uri, httpMethod, body) -> { - int currentHit = mSplitChangesHits.incrementAndGet(); if (mCustomSplitChangesResponse.get() != null) { return new HttpResponseMock(200, mCustomSplitChangesResponse.get()); } - return new HttpResponseMock(200, IntegrationHelper.emptySplitChanges(1, 1)); + return new HttpResponseMock(200, IntegrationHelper.emptyTargetingRulesChanges(1, 1)); }); responses.put(IntegrationHelper.ServicePath.MEMBERSHIPS + "/" + "/CUSTOMER_ID", (uri, httpMethod, body) -> new HttpResponseMock(200, IntegrationHelper.emptyAllSegments())); responses.put("v2/auth", (uri, httpMethod, body) -> { @@ -241,11 +240,6 @@ private Map getStringResponseClosureM return responses; } - @NonNull - private static String missingSegmentFetch(long flagSince, long segmentSince) { - return "{\"ff\":{\"s\":" + flagSince + ",\"t\":" + flagSince + ",\"d\":[]},\"rbs\":{\"s\":" + segmentSince + ",\"t\":" + segmentSince + ",\"d\":[{\"name\":\"new_rbs_test\",\"status\":\"ACTIVE\",\"trafficTypeName\":\"user\",\"excluded\":{\"keys\":[],\"segments\":[]},\"conditions\":[{\"matcherGroup\":{\"combiner\":\"AND\",\"matchers\":[{\"keySelector\":{\"trafficType\":\"user\"},\"matcherType\":\"WHITELIST\",\"negate\":false,\"whitelistMatcherData\":{\"whitelist\":[\"mdp\",\"tandil\",\"bsas\"]}},{\"keySelector\":{\"trafficType\":\"user\",\"attribute\":\"email\"},\"matcherType\":\"ENDS_WITH\",\"negate\":false,\"whitelistMatcherData\":{\"whitelist\":[\"@split.io\"]}}]}}]},{\"name\":\"rbs_test\",\"status\":\"ACTIVE\",\"trafficTypeName\":\"user\",\"excluded\":{\"keys\":[],\"segments\":[]},\"conditions\":[{\"conditionType\":\"ROLLOUT\",\"matcherGroup\":{\"combiner\":\"AND\",\"matchers\":[{\"keySelector\":{\"trafficType\":\"user\"},\"matcherType\":\"IN_RULE_BASED_SEGMENT\",\"negate\":false,\"userDefinedSegmentMatcherData\":{\"segmentName\":\"new_rbs_test\"}}]}}]}]}}"; - } - private boolean processUpdate(SplitClient client, LinkedBlockingDeque streamingData, String change, String... expectedContents) throws InterruptedException { CountDownLatch updateLatch = new CountDownLatch(1); client.on(SplitEvent.SDK_UPDATE, TestingHelper.testTask(updateLatch)); diff --git a/src/main/java/io/split/android/client/SplitClientConfig.java b/src/main/java/io/split/android/client/SplitClientConfig.java index d9f55df7c..00af53115 100644 --- a/src/main/java/io/split/android/client/SplitClientConfig.java +++ b/src/main/java/io/split/android/client/SplitClientConfig.java @@ -1118,7 +1118,7 @@ public Builder impressionsDedupeTimeInterval(long impressionsDedupeTimeInterval) * @param rolloutCacheConfiguration Configuration object * @return This builder */ - Builder rolloutCacheConfiguration(@NonNull RolloutCacheConfiguration rolloutCacheConfiguration) { + public Builder rolloutCacheConfiguration(@NonNull RolloutCacheConfiguration rolloutCacheConfiguration) { if (rolloutCacheConfiguration == null) { Logger.w("Rollout cache configuration is null. Setting to default value."); mRolloutCacheConfiguration = RolloutCacheConfiguration.builder().build(); diff --git a/src/main/java/io/split/android/client/service/splits/LoadSplitsTask.java b/src/main/java/io/split/android/client/service/splits/LoadSplitsTask.java index f5546470e..49ca1d512 100644 --- a/src/main/java/io/split/android/client/service/splits/LoadSplitsTask.java +++ b/src/main/java/io/split/android/client/service/splits/LoadSplitsTask.java @@ -1,5 +1,7 @@ package io.split.android.client.service.splits; +import static io.split.android.client.utils.Utils.checkNotNull; + import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -9,8 +11,6 @@ import io.split.android.client.storage.splits.SplitsStorage; import io.split.android.client.utils.logger.Logger; -import static io.split.android.client.utils.Utils.checkNotNull; - /** * This task is responsible for loading the feature flags, saved filter & saved flags spec values * from the persistent storage into the in-memory storage. diff --git a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java index 9e028254c..47b61c4dc 100644 --- a/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java +++ b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java @@ -94,7 +94,6 @@ void performProxyCheck() { long lastProxyCheckTimestamp = getLastProxyCheckTimestamp(); if (lastProxyCheckTimestamp == 0L) { - Logger.v("Never checked proxy; continuing with latest spec"); updateHandlingType(ProxyHandlingType.NONE); } else if (System.currentTimeMillis() - lastProxyCheckTimestamp > mProxyCheckIntervalMillis) { Logger.i("Time since last check elapsed. Attempting recovery with latest spec: " + mLatestSpec); diff --git a/src/main/java/io/split/android/client/storage/cipher/ApplyCipherTask.java b/src/main/java/io/split/android/client/storage/cipher/ApplyCipherTask.java index 3b04c44f5..8c756db7e 100644 --- a/src/main/java/io/split/android/client/storage/cipher/ApplyCipherTask.java +++ b/src/main/java/io/split/android/client/storage/cipher/ApplyCipherTask.java @@ -235,7 +235,7 @@ private void updateSplits(SplitRoomDatabase splitDatabase, GeneralInfoDao genera } GeneralInfoEntity trafficTypesEntity = generalInfoDao.getByName(GeneralInfoEntity.TRAFFIC_TYPES_MAP); - if (trafficTypesEntity != null) { + if (trafficTypesEntity != null && !trafficTypesEntity.getStringValue().isEmpty()) { String fromTrafficTypes = mFromCipher.decrypt(trafficTypesEntity.getStringValue()); String toTrafficTypes = mToCipher.encrypt(fromTrafficTypes); if (toTrafficTypes != null) { @@ -246,7 +246,7 @@ private void updateSplits(SplitRoomDatabase splitDatabase, GeneralInfoDao genera } GeneralInfoEntity flagSetsEntity = generalInfoDao.getByName(GeneralInfoEntity.FLAG_SETS_MAP); - if (flagSetsEntity != null) { + if (flagSetsEntity != null && !flagSetsEntity.getStringValue().isEmpty()) { String fromFlagSets = mFromCipher.decrypt(flagSetsEntity.getStringValue()); String toFlagSets = mToCipher.encrypt(fromFlagSets); if (toFlagSets != null) { diff --git a/src/main/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorage.java b/src/main/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorage.java index 9f328e455..ecffa0670 100644 --- a/src/main/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorage.java +++ b/src/main/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorage.java @@ -161,6 +161,7 @@ public void run() { mDatabase.generalInfoDao().update(new GeneralInfoEntity(GeneralInfoEntity.CHANGE_NUMBER_INFO, -1)); mDatabase.generalInfoDao().update(new GeneralInfoEntity(GeneralInfoEntity.FLAG_SETS_MAP, "")); mDatabase.generalInfoDao().update(new GeneralInfoEntity(GeneralInfoEntity.TRAFFIC_TYPES_MAP, "")); + mDatabase.getSplitQueryDao().invalidate(); mDatabase.splitDao().deleteAll(); } }); @@ -258,14 +259,14 @@ public void run() { private synchronized void parseTrafficTypesAndSets(@Nullable GeneralInfoEntity trafficTypesEntity, @Nullable GeneralInfoEntity flagSetsEntity) { Logger.v("Parsing traffic types and sets"); - if (trafficTypesEntity != null) { + if (trafficTypesEntity != null && !trafficTypesEntity.getStringValue().isEmpty()) { Type mapType = new TypeToken>(){}.getType(); String encryptedTrafficTypes = trafficTypesEntity.getStringValue(); String decryptedTrafficTypes = mCipher.decrypt(encryptedTrafficTypes); mTrafficTypes = Json.fromJson(decryptedTrafficTypes, mapType); } - if (flagSetsEntity != null) { + if (flagSetsEntity != null && !flagSetsEntity.getStringValue().isEmpty()) { Type flagsMapType = new TypeToken>>(){}.getType(); String encryptedFlagSets = flagSetsEntity.getStringValue(); String decryptedFlagSets = mCipher.decrypt(encryptedFlagSets); @@ -290,15 +291,19 @@ private void migrateTrafficTypesAndSetsFromStoredData() { } // persist TTs - String decryptedTrafficTypes = Json.toJson(mTrafficTypes); - String encryptedTrafficTypes = mCipher.encrypt(decryptedTrafficTypes); + if (mTrafficTypes != null && !mTrafficTypes.isEmpty()) { + String decryptedTrafficTypes = Json.toJson(mTrafficTypes); + String encryptedTrafficTypes = mCipher.encrypt(decryptedTrafficTypes); + mDatabase.generalInfoDao().update(new GeneralInfoEntity(GeneralInfoEntity.TRAFFIC_TYPES_MAP, encryptedTrafficTypes)); + } - // persist flag sets - String decryptedFlagSets = Json.toJson(mFlagSets); - String encryptedFlagSets = mCipher.encrypt(decryptedFlagSets); + if (mFlagSets != null && !mFlagSets.isEmpty()) { + // persist flag sets + String decryptedFlagSets = Json.toJson(mFlagSets); + String encryptedFlagSets = mCipher.encrypt(decryptedFlagSets); - mDatabase.generalInfoDao().update(new GeneralInfoEntity(GeneralInfoEntity.TRAFFIC_TYPES_MAP, encryptedTrafficTypes)); - mDatabase.generalInfoDao().update(new GeneralInfoEntity(GeneralInfoEntity.FLAG_SETS_MAP, encryptedFlagSets)); + mDatabase.generalInfoDao().update(new GeneralInfoEntity(GeneralInfoEntity.FLAG_SETS_MAP, encryptedFlagSets)); + } } catch (Exception e) { Logger.e("Failed to migrate traffic types and flag sets", e); } diff --git a/src/test/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorageTest.java b/src/test/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorageTest.java index 38c07f3c0..7145fd775 100644 --- a/src/test/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorageTest.java +++ b/src/test/java/io/split/android/client/storage/splits/SqLitePersistentSplitsStorageTest.java @@ -45,6 +45,8 @@ public class SqLitePersistentSplitsStorageTest { @Mock private SplitDao mSplitDao; @Mock + private SplitQueryDao mSplitQueryDao; + @Mock private SplitCipher mCipher; private SqLitePersistentSplitsStorage mStorage; private AutoCloseable mAutoCloseable; @@ -56,6 +58,7 @@ public void setUp() { mAutoCloseable = MockitoAnnotations.openMocks(this); when(mDatabase.generalInfoDao()).thenReturn(mock(GeneralInfoDao.class)); when(mDatabase.splitDao()).thenReturn(mSplitDao); + when(mDatabase.getSplitQueryDao()).thenReturn(mSplitQueryDao); doAnswer((Answer) invocation -> { ((Runnable) invocation.getArgument(0)).run(); return null; @@ -201,6 +204,7 @@ public boolean matches(GeneralInfoEntity argument) { } })); verify(mDatabase.splitDao()).deleteAll(); + verify(mDatabase.getSplitQueryDao()).invalidate(); } @Test