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/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/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/executor/SplitTaskFactoryImpl.java b/src/main/java/io/split/android/client/service/executor/SplitTaskFactoryImpl.java index 5e81d0844..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,8 +110,10 @@ public SplitTaskFactoryImpl(@NonNull SplitClientConfig splitClientConfig, mSplitChangeProcessor, mRuleBasedSegmentChangeProcessor, ruleBasedSegmentStorageProducer, + mSplitsStorageContainer.getGeneralInfoStorage(), 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/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..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 @@ -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(HttpStatus status) { + return status == HttpStatus.INTERNAL_PROXY_OUTDATED; + } } 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 new file mode 100644 index 000000000..47b61c4dc --- /dev/null +++ b/src/main/java/io/split/android/client/service/splits/OutdatedSplitProxyHandler.java @@ -0,0 +1,167 @@ +package io.split.android.client.service.splits; + +import static io.split.android.client.utils.Utils.checkNotNull; + +import androidx.annotation.VisibleForTesting; + +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. + * + *

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.

+ * + */ +public class OutdatedSplitProxyHandler { + + private static final String PREVIOUS_SPEC = "1.2"; + + private final String mLatestSpec; + private final String mPreviousSpec; + private final boolean mForBackgroundSync; + private final long mProxyCheckIntervalMillis; + + private final AtomicLong mLastProxyCheckTimestamp = new AtomicLong(0L); + 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. + * + * @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; + mPreviousSpec = previousSpec; + mForBackgroundSync = forBackgroundSync; + mProxyCheckIntervalMillis = proxyCheckIntervalMillis; + 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"); + updateHandlingType(ProxyHandlingType.NONE); + } else { + updateLastProxyCheckTimestamp(System.currentTimeMillis()); + updateHandlingType(ProxyHandlingType.FALLBACK); + } + } + + /** + * Performs a periodic proxy check to attempt recovery. + */ + void performProxyCheck() { + if (mForBackgroundSync) { + updateHandlingType(ProxyHandlingType.NONE); + } + + long lastProxyCheckTimestamp = getLastProxyCheckTimestamp(); + + if (lastProxyCheckTimestamp == 0L) { + updateHandlingType(ProxyHandlingType.NONE); + } else if (System.currentTimeMillis() - lastProxyCheckTimestamp > mProxyCheckIntervalMillis) { + Logger.i("Time since last check elapsed. Attempting recovery with latest spec: " + mLatestSpec); + updateHandlingType(ProxyHandlingType.RECOVERY); + } else { + Logger.v("Have used proxy fallback mode; time since last check has not elapsed. Using previous spec"); + updateHandlingType(ProxyHandlingType.FALLBACK); + } + } + + 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; + } + + 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(); + } + + private void updateLastProxyCheckTimestamp(long newTimestamp) { + mLastProxyCheckTimestamp.set(newTimestamp); + mGeneralInfoStorage.setLastProxyUpdateTimestamp(newTimestamp); + } + + /** + * Enum representing the proxy handling types. + */ + private 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 7c8e802b2..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 @@ -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; @@ -38,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; @@ -46,34 +48,64 @@ 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, @NonNull SplitChangeProcessor splitChangeProcessor, @NonNull RuleBasedSegmentChangeProcessor ruleBasedSegmentChangeProcessor, @NonNull RuleBasedSegmentStorageProducer ruleBasedSegmentStorage, + @NonNull GeneralInfoStorage generalInfoStorage, @NonNull TelemetryRuntimeProducer telemetryRuntimeProducer, - @Nullable String flagsSpec) { + @Nullable String flagsSpec, + boolean forBackgroundSync) { this(splitFetcher, splitsStorage, splitChangeProcessor, ruleBasedSegmentChangeProcessor, ruleBasedSegmentStorage, + generalInfoStorage, telemetryRuntimeProducer, new ReconnectBackoffCounter(1, ON_DEMAND_FETCH_BACKOFF_MAX_WAIT), - flagsSpec); + flagsSpec, + forBackgroundSync, + DEFAULT_PROXY_CHECK_INTERVAL_MILLIS); } - @VisibleForTesting public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, @NonNull SplitsStorage splitsStorage, @NonNull SplitChangeProcessor splitChangeProcessor, @NonNull RuleBasedSegmentChangeProcessor ruleBasedSegmentChangeProcessor, @NonNull RuleBasedSegmentStorageProducer ruleBasedSegmentStorage, + @NonNull GeneralInfoStorage generalInfoStorage, @NonNull TelemetryRuntimeProducer telemetryRuntimeProducer, @NonNull BackoffCounter backoffCounter, @Nullable String flagsSpec) { + this(splitFetcher, + splitsStorage, + splitChangeProcessor, + ruleBasedSegmentChangeProcessor, + ruleBasedSegmentStorage, + generalInfoStorage, + telemetryRuntimeProducer, + backoffCounter, + flagsSpec, + false, + DEFAULT_PROXY_CHECK_INTERVAL_MILLIS); + } + + @VisibleForTesting + public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, + @NonNull SplitsStorage splitsStorage, + @NonNull SplitChangeProcessor splitChangeProcessor, + @NonNull RuleBasedSegmentChangeProcessor ruleBasedSegmentChangeProcessor, + @NonNull RuleBasedSegmentStorageProducer ruleBasedSegmentStorage, + @NonNull GeneralInfoStorage generalInfoStorage, + @NonNull TelemetryRuntimeProducer telemetryRuntimeProducer, + @NonNull BackoffCounter backoffCounter, + @Nullable String flagsSpec, + boolean forBackgroundSync, + long proxyCheckIntervalMillis) { mSplitFetcher = checkNotNull(splitFetcher); mSplitsStorage = checkNotNull(splitsStorage); mSplitChangeProcessor = checkNotNull(splitChangeProcessor); @@ -81,7 +113,7 @@ public SplitsSyncHelper(@NonNull HttpFetcher splitFetcher, mRuleBasedSegmentStorage = checkNotNull(ruleBasedSegmentStorage); mTelemetryRuntimeProducer = checkNotNull(telemetryRuntimeProducer); mBackoffCounter = checkNotNull(backoffCounter); - mFlagsSpec = flagsSpec; + mOutdatedSplitProxyHandler = new OutdatedSplitProxyHandler(flagsSpec, forBackgroundSync, generalInfoStorage, proxyCheckIntervalMillis); } public SplitTaskExecutionInfo sync(SinceChangeNumbers till, int onDemandFetchBackoffMaxRetries) { @@ -94,13 +126,19 @@ public SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBeforeU private SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBeforeUpdate, boolean avoidCache, boolean resetChangeNumber, int onDemandFetchBackoffMaxRetries) { try { + mOutdatedSplitProxyHandler.performProxyCheck(); + if (mOutdatedSplitProxyHandler.isRecoveryMode()) { + clearBeforeUpdate = true; + resetChangeNumber = true; + } + CdnByPassType cdnByPassType = attemptSplitSync(till, clearBeforeUpdate, avoidCache, CdnByPassType.NONE, resetChangeNumber, onDemandFetchBackoffMaxRetries); if (cdnByPassType != CdnByPassType.NONE) { 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()); @@ -113,6 +151,14 @@ private SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBefore Collections.singletonMap(SplitTaskExecutionInfo.DO_NOT_RETRY, true)); } + if (HttpStatus.isProxyOutdated(httpStatus)) { + try { + mOutdatedSplitProxyHandler.trackProxyError(); + } 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()); @@ -120,14 +166,25 @@ private SplitTaskExecutionInfo sync(SinceChangeNumbers till, boolean clearBefore } Logger.d("Feature flags have been updated"); + + if (mOutdatedSplitProxyHandler.isRecoveryMode()) { + Logger.i("Resetting proxy check timestamp due to successful recovery"); + mOutdatedSplitProxyHandler.resetProxyCheckTimestamp(); + } + return SplitTaskExecutionInfo.success(SplitTaskType.SPLITS_SYNC); + } + + private SplitTaskExecutionInfo handleOutdatedProxy(SinceChangeNumbers till, boolean ignoredAvoidCache, boolean resetChangeNumber, int onDemandFetchBackoffMaxRetries) throws Exception { + + 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 */ @@ -140,7 +197,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 +228,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 +247,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 (!mOutdatedSplitProxyHandler.isFallbackMode() && till.getRbsSince() != null) { + params.put(RBS_SINCE_PARAM, till.getRbsSince()); + } if (cdnByPassType == CdnByPassType.RBS) { params.put(TILL_PARAM, till.getRbsSince()); @@ -231,9 +292,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 +304,8 @@ public long getFlagsSince() { return mFlagsSince; } - public long getRbsSince() { + @Nullable + public Long getRbsSince() { return mRbsSince; } @@ -250,7 +313,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/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 640949301..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,7 +25,9 @@ SplitsSyncHelper provideSplitsSyncHelper(HttpFetcher split splitChangeProcessor, ruleBasedSegmentChangeProcessor, ruleBasedSegmentStorage, + generalInfoStorage, telemetryStorage, - mFlagsSpec); + mFlagsSpec, + true); } } 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 8cb17df13..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 @@ -88,7 +88,7 @@ private void updateRuleBasedSegment(RuleBasedSegmentDao ruleBasedSegmentDao) { if (toName != null && toBody != null) { ruleBasedSegmentDao.update(name, toName, toBody); - } else{ + } else { Logger.e("Error applying cipher to rule based segment storage"); } } @@ -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/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; + } } 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/service/SplitsSyncHelperTest.java b/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java index 48477e321..b6e0793ec 100644 --- a/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java +++ b/src/test/java/io/split/android/client/service/SplitsSyncHelperTest.java @@ -2,11 +2,13 @@ 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.never; import static org.mockito.Mockito.times; @@ -17,6 +19,7 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; import org.mockito.Mock; import org.mockito.MockitoAnnotations; @@ -28,6 +31,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,10 +42,12 @@ 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; 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 +73,8 @@ public class SplitsSyncHelperTest { private BackoffCounter mBackoffCounter; @Mock private RuleBasedSegmentStorageProducer mRuleBasedSegmentStorageProducer; + @Mock + private GeneralInfoStorage mGeneralInfoStorage; private SplitsSyncHelper mSplitsSyncHelper; @@ -81,7 +89,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, 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(); } @@ -105,7 +114,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()); @@ -130,7 +139,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 +154,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 +169,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 +187,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 +203,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 +223,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 +241,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 +252,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 +261,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 +286,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 +322,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 +350,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,10 +358,10 @@ 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"); + params.put("s", "1.3"); params.put("since", -1L); params.put("rbSince", -1L); verify(mSplitsFetcher).execute(eq(params), eq(null)); @@ -365,7 +374,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 +385,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 +396,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 +407,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 @@ -417,6 +426,109 @@ 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 + 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(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 + mSplitsSyncHelper.sync(getSinceChangeNumbers(-1, -1L), false, false, ServiceConstants.ON_DEMAND_FETCH_BACKOFF_MAX_RETRIES); + verify(mSplitsFetcher, times(1)).execute(argThat(params -> + "1.2".equals(params.get("s")) && + !params.containsKey("rbSince") + ), 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(); @@ -426,7 +538,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); 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..7d8792a84 --- /dev/null +++ b/src/test/java/io/split/android/client/service/splits/OutdatedSplitProxyHandlerTest.java @@ -0,0 +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()); + } +} 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"); 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