diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java index e300270d8..3fc61cbec 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java @@ -545,6 +545,80 @@ public void validateMergeReceivedResponse() throws Exception { Assert.assertEquals(0, ((JSONObject) vals.get("t")).length()); } + /** + * Concurrency test: simulate rapid merge operations while concurrently reading values + * to ensure no ConcurrentModificationException or data corruption occurs after introducing + * fine-grained remoteConfigLock and CopyOnWriteArrayList for callbacks. + */ + @Test + public void concurrentMergeAndReads() throws Exception { + CountlyConfig cc = new CountlyConfig(TestUtils.getContext(), "appkey", "http://test.count.ly").setDeviceId("dev1").setLoggingEnabled(true); + cc.immediateRequestGenerator = new ImmediateRequestGenerator() { + @Override public ImmediateRequestI CreateImmediateRequestMaker() { return (a,b,c,d,e,f,g) -> { if (f!=null) f.callback(null); }; } + @Override public ImmediateRequestI CreatePreflightRequestMaker() { return (a,b,c,d,e,f,g) -> { if (f!=null) f.callback(null); }; } + }; + Countly countly = new Countly().init(cc); + + RemoteConfigValueStore rcvsA = RemoteConfigValueStore.dataFromString("{\"k1\":123,\"k2\":\"v2\"}", false); + RemoteConfigValueStore rcvsB = RemoteConfigValueStore.dataFromString("{\"k2\":777,\"k3\":{}}", false); + RemoteConfigValueStore rcvsC = RemoteConfigValueStore.dataFromString("{\"k4\":true,\"k5\":55.5}", false); + + RemoteConfigValueStore[] arr = new RemoteConfigValueStore[] { rcvsA, rcvsB, rcvsC }; + + final int MERGE_THREADS = 3; + final int READ_THREADS = 4; + final int MERGE_OPS = 150; + final int READ_OPS = 600; + + Thread[] mergers = new Thread[MERGE_THREADS]; + Thread[] readers = new Thread[READ_THREADS]; + final Exception[] failure = { null }; + + for (int i = 0; i < MERGE_THREADS; i++) { + final int idx = i; + mergers[i] = new Thread(() -> { + try { + for (int j = 0; j < MERGE_OPS; j++) { + RemoteConfigValueStore pick = arr[(idx + j) % arr.length]; + boolean clear = (j % 10) == 0; // occasionally force a clear path + countly.moduleRemoteConfig.mergeCheckResponseIntoCurrentValues(clear, RemoteConfigHelper.DownloadedValuesIntoMap(pick.values)); + } + } catch (Exception ex) { + failure[0] = ex; + } + }, "RC-Merger-" + i); + mergers[i].start(); + } + + for (int i = 0; i < READ_THREADS; i++) { + readers[i] = new Thread(() -> { + try { + for (int j = 0; j < READ_OPS; j++) { + countly.remoteConfig().getValue("k1"); + countly.remoteConfig().getValues(); + countly.remoteConfig().getAllValuesAndEnroll(); + } + } catch (Exception ex) { + failure[0] = ex; + } + }, "RC-Reader-" + i); + readers[i].start(); + } + + for (Thread t : mergers) { t.join(); } + for (Thread t : readers) { t.join(); } + + if (failure[0] != null) { + Assert.fail("Concurrency test failure: " + failure[0]); + } + + // resulting map should not contain partially written entries + Map finalVals = countly.remoteConfig().getValues(); + for (Map.Entry e : finalVals.entrySet()) { + Assert.assertNotNull("Null RCData encountered for key=" + e.getKey(), e.getValue()); + } + } + static void assertCValueCachedState(Map rcValues, boolean valuesAreCached) { for (Map.Entry entry : rcValues.entrySet()) { if (valuesAreCached) { diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigAnrTest.java b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigAnrTest.java new file mode 100644 index 000000000..431ecaaf2 --- /dev/null +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigAnrTest.java @@ -0,0 +1,165 @@ +package ly.count.android.sdk; + +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; + +import android.content.Context; +import android.os.Looper; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.platform.app.InstrumentationRegistry; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicLong; + +/** + * Test measuring Remote Config access latency under artificial contention on the global Countly lock. + * After narrowing synchronization in RemoteConfig getters, these calls should NOT block for the full + * duration that another thread holds Countly's global monitor. + */ +@RunWith(AndroidJUnit4.class) +public class RemoteConfigAnrTest { + + private Context context; + + @Before + public void setUp() { + context = InstrumentationRegistry.getInstrumentation().getTargetContext(); + CountlyConfig cc = new CountlyConfig(context, "app", "https://server.url") + .setLoggingEnabled(true); + Countly.sharedInstance().init(cc); + } + + @After + public void tearDown() { + Countly.sharedInstance().halt(); + } + + @Test + public void testRemoteConfigAccessUnderContention() throws InterruptedException { + // Baseline latencies + long baselineGetValue = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValue("test_key")); + long baselineGetValues = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValues()); + long baselineGetValueAndEnroll = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValueAndEnroll("test_key_2")); + long baselineGetAllValuesAndEnroll = measureLatency(() -> Countly.sharedInstance().remoteConfig().getAllValuesAndEnroll()); + + // Create contention by holding global Countly lock for 4 seconds + CountDownLatch lockAcquired = new CountDownLatch(1); + CountDownLatch releaseLock = new CountDownLatch(1); + Thread locker = new Thread(() -> { + synchronized (Countly.sharedInstance()) { + lockAcquired.countDown(); + try { Thread.sleep(4000); } catch (InterruptedException ignored) {} + releaseLock.countDown(); + } + }, "CountlyGlobalLockHolder"); + locker.start(); + assertTrue("Background thread failed to acquire Countly lock", lockAcquired.await(1, TimeUnit.SECONDS)); + + long contestedGetValue = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValue("test_key")); + long contestedGetValues = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValues()); + long contestedGetValueAndEnroll = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValueAndEnroll("test_key_2")); + long contestedGetAllValuesAndEnroll = measureLatency(() -> Countly.sharedInstance().remoteConfig().getAllValuesAndEnroll()); + + releaseLock.await(5, TimeUnit.SECONDS); + + System.out.println("[RCContention] baseline getValue=" + baselineGetValue + "ms, contested=" + contestedGetValue + "ms"); + System.out.println("[RCContention] baseline getValues=" + baselineGetValues + "ms, contested=" + contestedGetValues + "ms"); + System.out.println("[RCContention] baseline getValueAndEnroll=" + baselineGetValueAndEnroll + "ms, contested=" + contestedGetValueAndEnroll + "ms"); + System.out.println("[RCContention] baseline getAllValuesAndEnroll=" + baselineGetAllValuesAndEnroll + "ms, contested=" + contestedGetAllValuesAndEnroll + "ms"); + + // Reasonable baseline expectations (< 500ms each) + assertTrue("Baseline getValue high", baselineGetValue < 500); + assertTrue("Baseline getValues high", baselineGetValues < 500); + assertTrue("Baseline getValueAndEnroll high", baselineGetValueAndEnroll < 500); + assertTrue("Baseline getAllValuesAndEnroll high", baselineGetAllValuesAndEnroll < 500); + + // Contested calls should not block near 4s. Ensure < 1500ms and not >=3000ms. + assertTrue("Contested getValue too high: " + contestedGetValue, contestedGetValue < 1500); + assertTrue("Contested getValues too high: " + contestedGetValues, contestedGetValues < 1500); + assertTrue("Contested getValueAndEnroll too high: " + contestedGetValueAndEnroll, contestedGetValueAndEnroll < 1500); + assertTrue("Contested getAllValuesAndEnroll too high: " + contestedGetAllValuesAndEnroll, contestedGetAllValuesAndEnroll < 1500); + + assertFalse("getValue appears blocked, ANR adjacent", contestedGetValue >= 3000); + assertFalse("getValues appears blocked, ANR adjacent", contestedGetValues >= 3000); + assertFalse("getValueAndEnroll appears blocked, ANR adjacent", contestedGetValueAndEnroll >= 3000); + assertFalse("getAllValuesAndEnroll appears blocked, ANR adjacent", contestedGetAllValuesAndEnroll >= 3000); + + // Relative inflation guard: contested no more than 10x baseline (allows some noise) + assertTrue("getValue inflation too large", contestedGetValue <= Math.max(50, baselineGetValue * 10)); + assertTrue("getValues inflation too large", contestedGetValues <= Math.max(50, baselineGetValues * 10)); + assertTrue("getValueAndEnroll inflation too large", contestedGetValueAndEnroll <= Math.max(50, baselineGetValueAndEnroll * 10)); + assertTrue("getAllValuesAndEnroll inflation too large", contestedGetAllValuesAndEnroll <= Math.max(50, baselineGetAllValuesAndEnroll * 10)); + } + + private long measureLatency(Runnable r) { + AtomicLong start = new AtomicLong(); + AtomicLong end = new AtomicLong(); + InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { + start.set(System.currentTimeMillis()); + r.run(); + end.set(System.currentTimeMillis()); + }); + return end.get() - start.get(); + } + + @Test + public void stressTestRemoteConfigAccessUnderContention() throws InterruptedException { + final int ITERS = 100; + + // Warm-up baseline (single call per method) to initialize any lazy structures + measureLatency(() -> Countly.sharedInstance().remoteConfig().getValue("stress_key")); + measureLatency(() -> Countly.sharedInstance().remoteConfig().getValues()); + + long maxGetValue = 0, maxGetValues = 0; + long sumGetValue = 0, sumGetValues = 0; + + // Start contention thread holding global lock intermittently + CountDownLatch startSignal = new CountDownLatch(1); + CountDownLatch doneSignal = new CountDownLatch(1); + Thread contender = new Thread(() -> { + startSignal.countDown(); + long endTime = System.currentTimeMillis() + 4000; // run for ~4s + while (System.currentTimeMillis() < endTime) { + synchronized (Countly.sharedInstance()) { + try { Thread.sleep(80); } catch (InterruptedException ignored) {} + } + try { Thread.sleep(20); } catch (InterruptedException ignored) {} + } + doneSignal.countDown(); + }, "CountlyIntermittentLocker"); + contender.start(); + assertTrue("Failed to start contention thread", startSignal.await(1, TimeUnit.SECONDS)); + + for (int i = 0; i < ITERS; i++) { + long gv = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValue("stress_key")); + long gvs = measureLatency(() -> Countly.sharedInstance().remoteConfig().getValues()); + + sumGetValue += gv; + sumGetValues += gvs; + if (gv > maxGetValue) maxGetValue = gv; + if (gvs > maxGetValues) maxGetValues = gvs; + } + + doneSignal.await(5, TimeUnit.SECONDS); + + double avgGetValue = sumGetValue / (double) ITERS; + double avgGetValues = sumGetValues / (double) ITERS; + + System.out.println("[RCStress] iterations=" + ITERS + + " getValue avg=" + avgGetValue + "ms max=" + maxGetValue + + " | getValues avg=" + avgGetValues + "ms max=" + maxGetValues); + + // average should be under 200ms, max should not exceed 1000ms under intermittent contention. + assertTrue("getValue average too high: " + avgGetValue, avgGetValue < 200); + assertTrue("getValues average too high: " + avgGetValues, avgGetValues < 200); + assertTrue("getValue max spike too large: " + maxGetValue, maxGetValue < 1000); + assertTrue("getValues max spike too large: " + maxGetValues, maxGetValues < 1000); + } +} diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 2957c87f6..c0c1d8d0b 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -3,12 +3,12 @@ import android.text.TextUtils; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.locks.ReentrantReadWriteLock; import ly.count.android.sdk.internal.RemoteConfigHelper; import ly.count.android.sdk.internal.RemoteConfigValueStore; import org.json.JSONException; @@ -22,6 +22,8 @@ public class ModuleRemoteConfig extends ModuleBase { Map variantContainer = new HashMap<>(); // Stores the fetched A/B test variants Map experimentContainer = new HashMap<>(); // Stores the fetched A/B test information (includes exp ID, description etc.) RemoteConfig remoteConfigInterface = null; + // dedicated lock for remote config + private final Object remoteConfigLock = new Object(); //if set to true, it will automatically download remote configs on module startup boolean automaticDownloadTriggersEnabled; @@ -31,15 +33,13 @@ public class ModuleRemoteConfig extends ModuleBase { boolean remoteConfigValuesShouldBeCached = false; - CopyOnWriteArrayList downloadCallbacks = new CopyOnWriteArrayList<>(); + List downloadCallbacks = new CopyOnWriteArrayList<>(); public final static String variantObjectNameKey = "name"; @Nullable Map metricOverride = null; - private final ReentrantReadWriteLock remoteConfigStoreLock = new ReentrantReadWriteLock(); - ModuleRemoteConfig(Countly cly, final CountlyConfig config) { super(cly, config); L.v("[ModuleRemoteConfig] Initialising"); @@ -267,21 +267,13 @@ void testingEnrollIntoVariantInternal(@NonNull final String key, @NonNull final * @throws Exception it throws an exception so that it is escalated upwards */ void mergeCheckResponseIntoCurrentValues(boolean clearOldValues, @NonNull Map newRC) { - remoteConfigStoreLock.writeLock().lock(); - try { - //todo iterate over all response values and print a summary of the returned keys + ideally a summary of their payload. - - //merge the new values into the current ones + //todo iterate over all response values and print a summary of the returned keys + ideally a summary of their payload. + synchronized (remoteConfigLock) { RemoteConfigValueStore rcvs = loadConfig(); rcvs.mergeValues(newRC, clearOldValues); - L.d("[ModuleRemoteConfig] Finished remote config processing, starting saving"); - saveConfig(rcvs); - L.d("[ModuleRemoteConfig] Finished remote config saving"); - } finally { - remoteConfigStoreLock.writeLock().unlock(); } } @@ -307,28 +299,22 @@ boolean isResponseValid(@NonNull JSONObject responseJson) { } RCData getRCValue(@NonNull String key) { - remoteConfigStoreLock.readLock().lock(); try { RemoteConfigValueStore rcvs = loadConfig(); return rcvs.getValue(key); } catch (Exception ex) { L.e("[ModuleRemoteConfig] getValue, Call failed:[" + ex.toString() + "]"); return new RCData(null, true); - } finally { - remoteConfigStoreLock.readLock().unlock(); } } Object getRCValueLegacy(@NonNull String key) { - remoteConfigStoreLock.readLock().lock(); try { RemoteConfigValueStore rcvs = loadConfig(); return rcvs.getValueLegacy(key); } catch (Exception ex) { L.e("[ModuleRemoteConfig] getValueLegacy, Call failed:[" + ex.toString() + "]"); return null; - } finally { - remoteConfigStoreLock.readLock().unlock(); } } @@ -348,37 +334,32 @@ void saveConfig(@NonNull RemoteConfigValueStore rcvs) { } void clearValueStoreInternal() { - remoteConfigStoreLock.writeLock().lock(); - try { + synchronized (remoteConfigLock) { storageProvider.setRemoteConfigValues(""); - } finally { - remoteConfigStoreLock.writeLock().unlock(); } } @NonNull Map getAllRemoteConfigValuesInternalLegacy() { - remoteConfigStoreLock.readLock().lock(); - try { - RemoteConfigValueStore rcvs = loadConfig(); - return rcvs.getAllValuesLegacy(); - } catch (Exception ex) { - Countly.sharedInstance().L.e("[ModuleRemoteConfig] getAllRemoteConfigValuesInternal, Call failed:[" + ex.toString() + "]"); - return new HashMap<>(); - } finally { - remoteConfigStoreLock.readLock().unlock(); + synchronized (remoteConfigLock) { + try { + RemoteConfigValueStore rcvs = loadConfig(); + return rcvs.getAllValuesLegacy(); + } catch (Exception ex) { + Countly.sharedInstance().L.e("[ModuleRemoteConfig] getAllRemoteConfigValuesInternal, Call failed:[" + ex.toString() + "]"); + return new HashMap<>(); + } } } @NonNull Map getAllRemoteConfigValuesInternal() { - remoteConfigStoreLock.readLock().lock(); - try { - RemoteConfigValueStore rcvs = loadConfig(); - return rcvs.getAllValues(); - } catch (Exception ex) { - Countly.sharedInstance().L.e("[ModuleRemoteConfig] getAllRemoteConfigValuesInternal, Call failed:[" + ex.toString() + "]"); - return new HashMap<>(); - } finally { - remoteConfigStoreLock.readLock().unlock(); + synchronized (remoteConfigLock) { + try { + RemoteConfigValueStore rcvs = loadConfig(); + return rcvs.getAllValues(); + } catch (Exception ex) { + Countly.sharedInstance().L.e("[ModuleRemoteConfig] getAllRemoteConfigValuesInternal, Call failed:[" + ex.toString() + "]"); + return new HashMap<>(); + } } } @@ -388,7 +369,9 @@ void clearValueStoreInternal() { * @return */ @NonNull Map testingGetAllVariantsInternal() { - return variantContainer; + synchronized (remoteConfigLock) { + return new HashMap<>(variantContainer); + } } /** @@ -398,12 +381,13 @@ void clearValueStoreInternal() { * @return */ @Nullable String[] testingGetVariantsForKeyInternal(@NonNull String key) { - String[] variantResponse = null; - if (variantContainer.containsKey(key)) { - variantResponse = variantContainer.get(key); + synchronized (remoteConfigLock) { + if (variantContainer.containsKey(key)) { + String[] arr = variantContainer.get(key); + return arr == null ? null : arr.clone(); + } + return null; } - - return variantResponse; } void clearAndDownloadAfterIdChange() { @@ -418,21 +402,16 @@ void clearAndDownloadAfterIdChange() { void CacheOrClearRCValuesIfNeeded() { L.v("[RemoteConfig] CacheOrClearRCValuesIfNeeded, cacheclearing values"); - remoteConfigStoreLock.writeLock().lock(); - try { + synchronized (remoteConfigLock) { RemoteConfigValueStore rc = loadConfig(); rc.cacheClearValues(); saveConfig(rc); - } finally { - remoteConfigStoreLock.writeLock().unlock(); } } void NotifyDownloadCallbacks(RCDownloadCallback devProvidedCallback, RequestResult requestResult, String message, boolean fullUpdate, Map downloadedValues) { for (RCDownloadCallback callback : downloadCallbacks) { - if (callback != null) { - callback.callback(requestResult, message, fullUpdate, downloadedValues); - } + callback.callback(requestResult, message, fullUpdate, downloadedValues); } if (devProvidedCallback != null) { @@ -500,9 +479,11 @@ public class RemoteConfig { * @deprecated Use "clearAll" */ public void clearStoredValues() { - L.i("[RemoteConfig] clearStoredValues"); + synchronized (_cly) { + L.i("[RemoteConfig] clearStoredValues"); - clearValueStoreInternal(); + clearValueStoreInternal(); + } } /** @@ -510,9 +491,11 @@ public void clearStoredValues() { * @deprecated You should use "getValues" */ public Map getAllValues() { - L.i("[RemoteConfig] getAllValues"); + synchronized (_cly) { + L.i("[RemoteConfig] getAllValues"); - return getAllRemoteConfigValuesInternalLegacy(); + return getAllRemoteConfigValuesInternalLegacy(); + } } /** @@ -523,9 +506,11 @@ public Map getAllValues() { * @deprecated You should use "getValue" */ public Object getValueForKey(String key) { - L.i("[RemoteConfig] remoteConfigValueForKey, " + key); + synchronized (_cly) { + L.i("[RemoteConfig] remoteConfigValueForKey, " + key); - return getRCValueLegacy(key); + return getRCValueLegacy(key); + } } /** @@ -536,25 +521,27 @@ public Object getValueForKey(String key) { * @deprecated You should use "downloadOmittingKeys" */ public void updateExceptKeys(String[] keysToExclude, RemoteConfigCallback callback) { - L.i("[RemoteConfig] updateExceptKeys"); + synchronized (_cly) { + L.i("[RemoteConfig] updateExceptKeys"); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - if (callback != null) { - callback.callback("No consent given"); + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (callback != null) { + callback.callback("No consent given"); + } + return; } - return; - } - if (keysToExclude == null) { - L.w("[RemoteConfig] updateExceptKeys passed 'keys to ignore' array is null"); - } - - RCDownloadCallback innerCall = (downloadResult, error, fullValueUpdate, downloadedValues) -> { - if (callback != null) { - callback.callback(error); + if (keysToExclude == null) { + L.w("[RemoteConfig] updateExceptKeys passed 'keys to ignore' array is null"); } - }; - updateRemoteConfigValues(null, keysToExclude, true, innerCall); + RCDownloadCallback innerCall = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + if (callback != null) { + callback.callback(error); + } + }; + + updateRemoteConfigValues(null, keysToExclude, true, innerCall); + } } /** @@ -565,24 +552,26 @@ public void updateExceptKeys(String[] keysToExclude, RemoteConfigCallback callba * @deprecated You should use "downloadSpecificKeys" */ public void updateForKeysOnly(String[] keysToInclude, RemoteConfigCallback callback) { - L.i("[RemoteConfig] updateForKeysOnly"); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - if (callback != null) { - callback.callback("No consent given"); + synchronized (_cly) { + L.i("[RemoteConfig] updateForKeysOnly"); + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (callback != null) { + callback.callback("No consent given"); + } + return; } - return; - } - if (keysToInclude == null) { - L.w("[RemoteConfig] updateForKeysOnly passed 'keys to include' array is null"); - } - - RCDownloadCallback innerCall = (downloadResult, error, fullValueUpdate, downloadedValues) -> { - if (callback != null) { - callback.callback(error); + if (keysToInclude == null) { + L.w("[RemoteConfig] updateForKeysOnly passed 'keys to include' array is null"); } - }; - updateRemoteConfigValues(keysToInclude, null, true, innerCall); + RCDownloadCallback innerCall = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + if (callback != null) { + callback.callback(error); + } + }; + + updateRemoteConfigValues(keysToInclude, null, true, innerCall); + } } /** @@ -592,22 +581,24 @@ public void updateForKeysOnly(String[] keysToInclude, RemoteConfigCallback callb * @deprecated You should use "downloadAllKeys" */ public void update(RemoteConfigCallback callback) { - L.i("[RemoteConfig] update"); + synchronized (_cly) { + L.i("[RemoteConfig] update"); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - if (callback != null) { - callback.callback("No consent given"); + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (callback != null) { + callback.callback("No consent given"); + } + return; } - return; - } - RCDownloadCallback innerCall = (downloadResult, error, fullValueUpdate, downloadedValues) -> { - if (callback != null) { - callback.callback(error); - } - }; + RCDownloadCallback innerCall = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + if (callback != null) { + callback.callback(error); + } + }; - updateRemoteConfigValues(null, null, true, innerCall); + updateRemoteConfigValues(null, null, true, innerCall); + } } /** @@ -618,24 +609,26 @@ public void update(RemoteConfigCallback callback) { * @param callback This is called when the operation concludes */ public void downloadOmittingKeys(@Nullable String[] keysToOmit, @Nullable RCDownloadCallback callback) { - L.i("[RemoteConfig] downloadOmittingKeys"); + synchronized (_cly) { + L.i("[RemoteConfig] downloadOmittingKeys"); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - if (callback != null) { - callback.callback(RequestResult.Error, null, false, null); + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (callback != null) { + callback.callback(RequestResult.Error, null, false, null); + } + return; + } + if (keysToOmit == null) { + L.w("[RemoteConfig] downloadOmittingKeys passed 'keys to ignore' array is null"); } - return; - } - if (keysToOmit == null) { - L.w("[RemoteConfig] downloadOmittingKeys passed 'keys to ignore' array is null"); - } - if (callback == null) { - callback = (downloadResult, error, fullValueUpdate, downloadedValues) -> { - }; - } + if (callback == null) { + callback = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + }; + } - updateRemoteConfigValues(null, keysToOmit, false, callback); + updateRemoteConfigValues(null, keysToOmit, false, callback); + } } /** @@ -646,23 +639,25 @@ public void downloadOmittingKeys(@Nullable String[] keysToOmit, @Nullable RCDown * @param callback This is called when the operation concludes */ public void downloadSpecificKeys(@Nullable String[] keysToInclude, @Nullable RCDownloadCallback callback) { - L.i("[RemoteConfig] downloadSpecificKeys"); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - if (callback != null) { - callback.callback(RequestResult.Error, null, false, null); + synchronized (_cly) { + L.i("[RemoteConfig] downloadSpecificKeys"); + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (callback != null) { + callback.callback(RequestResult.Error, null, false, null); + } + return; + } + if (keysToInclude == null) { + L.w("[RemoteConfig] downloadSpecificKeys passed 'keys to include' array is null"); } - return; - } - if (keysToInclude == null) { - L.w("[RemoteConfig] downloadSpecificKeys passed 'keys to include' array is null"); - } - if (callback == null) { - callback = (downloadResult, error, fullValueUpdate, downloadedValues) -> { - }; - } + if (callback == null) { + callback = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + }; + } - updateRemoteConfigValues(keysToInclude, null, false, callback); + updateRemoteConfigValues(keysToInclude, null, false, callback); + } } /** @@ -671,21 +666,23 @@ public void downloadSpecificKeys(@Nullable String[] keysToInclude, @Nullable RCD * @param callback This is called when the operation concludes */ public void downloadAllKeys(@Nullable RCDownloadCallback callback) { - L.i("[RemoteConfig] downloadAllKeys"); + synchronized (_cly) { + L.i("[RemoteConfig] downloadAllKeys"); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - if (callback != null) { - callback.callback(RequestResult.Error, null, true, null); + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (callback != null) { + callback.callback(RequestResult.Error, null, true, null); + } + return; } - return; - } - if (callback == null) { - callback = (downloadResult, error, fullValueUpdate, downloadedValues) -> { - }; - } + if (callback == null) { + callback = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + }; + } - updateRemoteConfigValues(null, null, false, callback); + updateRemoteConfigValues(null, null, false, callback); + } } /** @@ -695,8 +692,9 @@ public void downloadAllKeys(@Nullable RCDownloadCallback callback) { */ public @NonNull Map getValues() { L.i("[RemoteConfig] getValues"); - - return getAllRemoteConfigValuesInternal(); + synchronized (remoteConfigLock) { + return getAllRemoteConfigValuesInternal(); + } } /** @@ -706,22 +704,22 @@ public void downloadAllKeys(@Nullable RCDownloadCallback callback) { */ public @NonNull Map getAllValuesAndEnroll() { L.i("[RemoteConfig] getAllValuesAndEnroll"); - Map values = getAllRemoteConfigValuesInternal(); + Map values; + synchronized (remoteConfigLock) { + values = getAllRemoteConfigValuesInternal(); + } - if (values.isEmpty()) { - L.i("[RemoteConfig] getAllValuesAndEnroll, No value to enroll"); - } else { + if (!values.isEmpty()) { Set setOfKeys = values.keySet(); String[] arrayOfKeys = new String[setOfKeys.size()]; - int i = 0; for (String key : setOfKeys) { arrayOfKeys[i++] = key; } - enrollIntoABTestsForKeys(arrayOfKeys); + } else { + L.i("[RemoteConfig] getAllValuesAndEnroll, No value to enroll"); } - return values; } @@ -733,13 +731,13 @@ public void downloadAllKeys(@Nullable RCDownloadCallback callback) { */ public @NonNull RCData getValue(final @Nullable String key) { L.i("[RemoteConfig] getValue, key:[" + key + "]"); - if (key == null || key.equals("")) { L.i("[RemoteConfig] getValue, A valid key should be provided to get its value."); return new RCData(null, true); } - - return getRCValue(key); + synchronized (remoteConfigLock) { + return getRCValue(key); + } } /** @@ -750,21 +748,20 @@ public void downloadAllKeys(@Nullable RCDownloadCallback callback) { */ public @NonNull RCData getValueAndEnroll(@Nullable String key) { L.i("[RemoteConfig] getValueAndEnroll, key:[" + key + "]"); - if (key == null || key.equals("")) { L.i("[RemoteConfig] getValueAndEnroll, A valid key should be provided to get its value."); return new RCData(null, true); } - - RCData data = getRCValue(key); - - if (data.value == null) { - L.i("[RemoteConfig] getValueAndEnroll, No value to enroll"); - } else { + RCData data; + synchronized (remoteConfigLock) { + data = getRCValue(key); + } + if (data.value != null) { String[] arrayOfKeys = { key }; enrollIntoABTestsForKeys(arrayOfKeys); + } else { + L.i("[RemoteConfig] getValueAndEnroll, No value to enroll"); } - return data; } @@ -774,18 +771,20 @@ public void downloadAllKeys(@Nullable RCDownloadCallback callback) { * @param keys - String array of keys (parameters) */ public void enrollIntoABTestsForKeys(@Nullable String[] keys) { - L.i("[RemoteConfig] enrollIntoABTestsForKeys"); + synchronized (_cly) { + L.i("[RemoteConfig] enrollIntoABTestsForKeys"); - if (keys == null || keys.length == 0) { - L.w("[RemoteConfig] enrollIntoABTestsForKeys, A key should be provided to enroll the user."); - return; - } + if (keys == null || keys.length == 0) { + L.w("[RemoteConfig] enrollIntoABTestsForKeys, A key should be provided to enroll the user."); + return; + } - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return; - } + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + return; + } - enrollIntoABTestsForKeysInternal(keys); + enrollIntoABTestsForKeysInternal(keys); + } } /** @@ -794,17 +793,19 @@ public void enrollIntoABTestsForKeys(@Nullable String[] keys) { * @param keys - String array of keys (parameters) */ public void exitABTestsForKeys(@Nullable String[] keys) { - L.i("[RemoteConfig] exitABTestsForKeys"); + synchronized (_cly) { + L.i("[RemoteConfig] exitABTestsForKeys"); - if (keys == null) { - keys = new String[0]; - } + if (keys == null) { + keys = new String[0]; + } - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return; - } + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + return; + } - exitABTestsForKeysInternal(keys); + exitABTestsForKeysInternal(keys); + } } /** @@ -813,8 +814,8 @@ public void exitABTestsForKeys(@Nullable String[] keys) { * @param callback The callback that should be added */ public void registerDownloadCallback(@Nullable RCDownloadCallback callback) { - L.i("[RemoteConfig] registerDownloadCallback"); - if (callback != null) { + synchronized (_cly) { + L.i("[RemoteConfig] registerDownloadCallback"); downloadCallbacks.add(callback); } } @@ -825,16 +826,20 @@ public void registerDownloadCallback(@Nullable RCDownloadCallback callback) { * @param callback The callback that should be removed */ public void removeDownloadCallback(@Nullable RCDownloadCallback callback) { - L.i("[RemoteConfig] removeDownloadCallback"); - downloadCallbacks.remove(callback); + synchronized (_cly) { + L.i("[RemoteConfig] removeDownloadCallback"); + downloadCallbacks.remove(callback); + } } /** * Clear all stored remote config values. */ public void clearAll() { - L.i("[RemoteConfig] clearAll"); - clearStoredValues(); + synchronized (_cly) { + L.i("[RemoteConfig] clearAll"); + clearStoredValues(); + } } /** @@ -845,9 +850,11 @@ public void clearAll() { * @return Return the information of all available variants */ public @NonNull Map testingGetAllVariants() { - L.i("[RemoteConfig] testingGetAllVariants"); + synchronized (_cly) { + L.i("[RemoteConfig] testingGetAllVariants"); - return testingGetAllVariantsInternal(); + return testingGetAllVariantsInternal(); + } } /** @@ -858,9 +865,11 @@ public void clearAll() { * @return Return the information of all available variants */ public @NonNull Map testingGetAllExperimentInfo() { - L.i("[RemoteConfig] testingGetAllExperimentInfo"); + synchronized (_cly) { + L.i("[RemoteConfig] testingGetAllExperimentInfo"); - return experimentContainer; + return experimentContainer; + } } /** @@ -872,14 +881,16 @@ public void clearAll() { * @return If returns the stored variants for the given key. Returns "null" if there are no variants for that key. */ public @Nullable String[] testingGetVariantsForKey(@Nullable String key) { - L.i("[RemoteConfig] testingGetVariantsForKey"); + synchronized (_cly) { + L.i("[RemoteConfig] testingGetVariantsForKey"); - if (key == null) { - L.i("[RemoteConfig] testingGetVariantsForKey, provided variant key can not be null"); - return null; - } + if (key == null) { + L.i("[RemoteConfig] testingGetVariantsForKey, provided variant key can not be null"); + return null; + } - return testingGetVariantsForKeyInternal(key); + return testingGetVariantsForKeyInternal(key); + } } /** @@ -890,18 +901,20 @@ public void clearAll() { * @param completionCallback this callback will be called when the network request finished */ public void testingDownloadVariantInformation(@Nullable RCVariantCallback completionCallback) { - L.i("[RemoteConfig] testingFetchVariantInformation"); + synchronized (_cly) { + L.i("[RemoteConfig] testingFetchVariantInformation"); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return; - } + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + return; + } - if (completionCallback == null) { - completionCallback = (result, error) -> { - }; - } + if (completionCallback == null) { + completionCallback = (result, error) -> { + }; + } - testingFetchVariantInformationInternal(completionCallback, false); + testingFetchVariantInformationInternal(completionCallback, false); + } } /** @@ -912,18 +925,20 @@ public void testingDownloadVariantInformation(@Nullable RCVariantCallback comple * @param completionCallback this callback will be called when the network request finished */ public void testingDownloadExperimentInformation(@Nullable RCVariantCallback completionCallback) { - L.i("[RemoteConfig] testingDownloadExperimentInformation"); + synchronized (_cly) { + L.i("[RemoteConfig] testingDownloadExperimentInformation"); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return; - } + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + return; + } - if (completionCallback == null) { - completionCallback = (result, error) -> { - }; - } + if (completionCallback == null) { + completionCallback = (result, error) -> { + }; + } - testingFetchVariantInformationInternal(completionCallback, true); + testingFetchVariantInformationInternal(completionCallback, true); + } } /** @@ -936,23 +951,25 @@ public void testingDownloadExperimentInformation(@Nullable RCVariantCallback com * @param completionCallback */ public void testingEnrollIntoVariant(@Nullable String keyName, String variantName, @Nullable RCVariantCallback completionCallback) { - L.i("[RemoteConfig] testingEnrollIntoVariant"); + synchronized (_cly) { + L.i("[RemoteConfig] testingEnrollIntoVariant"); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return; - } + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + return; + } - if (keyName == null || variantName == null) { - L.w("[RemoteConfig] testEnrollIntoVariant, passed key or variant is null. Aborting."); - return; - } + if (keyName == null || variantName == null) { + L.w("[RemoteConfig] testEnrollIntoVariant, passed key or variant is null. Aborting."); + return; + } - if (completionCallback == null) { - completionCallback = (result, error) -> { - }; - } + if (completionCallback == null) { + completionCallback = (result, error) -> { + }; + } - testingEnrollIntoVariantInternal(keyName, variantName, completionCallback); + testingEnrollIntoVariantInternal(keyName, variantName, completionCallback); + } } } }