diff --git a/CHANGELOG.md b/CHANGELOG.md index f5c72bf31..c84f7336e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,13 @@ -## 23.2.0 +## 23.6.0 +* !! Major breaking change !! Automatically downloaded remote config values will no longer be automatically enrolled in their AB tests. +* ! Minor breaking change ! Remote config will now return previously downloaded values when remote-config consent is not given + +* Introduced a new set of remote config methods +* Deprecated old remote config methods + +* Fixed bug where recording views would force send all stored events +* Fixed bug where exiting temporary ID mode would create unintended requests + * Removed the deprecated enum "DeviceId.Type" * Removed the deprecated value "ADVERTISING_ID" from the enum "DeviceIdType" * Removed the deprecated function "Countly.changeDeviceIdWithoutMerge(type, deviceID)" diff --git a/app/src/main/java/ly/count/android/demo/ActivityExampleRemoteConfig.java b/app/src/main/java/ly/count/android/demo/ActivityExampleRemoteConfig.java index 47e0e71c0..99063db98 100644 --- a/app/src/main/java/ly/count/android/demo/ActivityExampleRemoteConfig.java +++ b/app/src/main/java/ly/count/android/demo/ActivityExampleRemoteConfig.java @@ -7,6 +7,9 @@ import android.widget.Toast; import androidx.appcompat.app.AppCompatActivity; +import ly.count.android.sdk.RCData; +import ly.count.android.sdk.RCDownloadCallback; +import ly.count.android.sdk.RequestResult; import org.json.JSONArray; import org.json.JSONObject; @@ -24,9 +27,8 @@ public void onCreate(Bundle savedInstanceState) { } public void onClickRemoteConfigUpdate(View v) { - Countly.sharedInstance().remoteConfig().update(new RemoteConfigCallback() { - @Override - public void callback(String error) { + Countly.sharedInstance().remoteConfig().downloadAllKeys(new RCDownloadCallback() { + @Override public void callback(RequestResult downloadResult, String error, boolean fullValueUpdate, Map downloadedValues) { if (error == null) { Toast.makeText(getApplicationContext(), "Update finished", Toast.LENGTH_SHORT).show(); } else { diff --git a/app/src/main/java/ly/count/android/demo/ActivityExampleTests.java b/app/src/main/java/ly/count/android/demo/ActivityExampleTests.java index 8741a5f56..03a5074ca 100644 --- a/app/src/main/java/ly/count/android/demo/ActivityExampleTests.java +++ b/app/src/main/java/ly/count/android/demo/ActivityExampleTests.java @@ -9,8 +9,8 @@ import java.util.Arrays; import java.util.Map; import ly.count.android.sdk.Countly; -import ly.count.android.sdk.RequestResponse; import ly.count.android.sdk.RCVariantCallback; +import ly.count.android.sdk.RequestResult; public class ActivityExampleTests extends AppCompatActivity { @@ -22,10 +22,10 @@ public void onCreate(Bundle savedInstanceState) { // For fetching all variants with a button click public void onClickFetchAllVariants(View v) { - Countly.sharedInstance().remoteConfig().testingFetchVariantInformation(new RCVariantCallback() { + Countly.sharedInstance().remoteConfig().testingDownloadVariantInformation(new RCVariantCallback() { @Override - public void callback(RequestResponse result, String error) { - if (result == RequestResponse.SUCCESS) { + public void callback(RequestResult result, String error) { + if (result == RequestResult.Success) { Toast.makeText(getApplicationContext(), "Fetch finished", Toast.LENGTH_SHORT).show(); } else { Toast.makeText(getApplicationContext(), "Error: " + result, Toast.LENGTH_SHORT).show(); @@ -75,8 +75,8 @@ public void onClickEnrollVariant(View v) { Countly.sharedInstance().remoteConfig().testingEnrollIntoVariant(key, variant, new RCVariantCallback() { @Override - public void callback(RequestResponse result, String error) { - if (result == RequestResponse.SUCCESS) { + public void callback(RequestResult result, String error) { + if (result == RequestResult.Success) { Toast.makeText(getApplicationContext(), "Fetch finished", Toast.LENGTH_SHORT).show(); } else { Toast.makeText(getApplicationContext(), "Error: " + result, Toast.LENGTH_SHORT).show(); diff --git a/gradle.properties b/gradle.properties index 0a30af9c6..6c13d069d 100644 --- a/gradle.properties +++ b/gradle.properties @@ -22,7 +22,7 @@ org.gradle.configureondemand=true android.useAndroidX=true android.enableJetifier=true # RELEASE FIELD SECTION -VERSION_NAME=22.09.4 +VERSION_NAME=23.6.0 GROUP=ly.count.android POM_URL=https://github.com/Countly/countly-sdk-android POM_SCM_URL=https://github.com/Countly/countly-sdk-android diff --git a/sdk-native/build.gradle b/sdk-native/build.gradle index 354366768..732823c9a 100644 --- a/sdk-native/build.gradle +++ b/sdk-native/build.gradle @@ -2,49 +2,48 @@ apply plugin: 'com.android.library' apply plugin: "com.vanniktech.maven.publish" buildscript { - repositories { - mavenCentral() - } - dependencies { - classpath 'com.vanniktech:gradle-maven-publish-plugin:0.21.0' //for publishing - } + repositories { + mavenCentral() + } + dependencies { + classpath 'com.vanniktech:gradle-maven-publish-plugin:0.21.0' //for publishing + } } android { - compileSdkVersion 31 + compileSdkVersion 33 - defaultConfig { - minSdkVersion 17 - targetSdkVersion 31 + defaultConfig { + minSdkVersion 17 + targetSdkVersion 33 - testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" + testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" + } + buildTypes { + release { + minifyEnabled false + proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro' } - - buildTypes { - release { - minifyEnabled false - proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro' - } - } - buildToolsVersion '33.0.0' + } + buildToolsVersion '33.0.0' } dependencies { - implementation fileTree(dir: 'libs', include: ['*.jar']) + implementation fileTree(dir: 'libs', include: ['*.jar']) - //testImplementation 'junit:junit:4.13.2' - //androidTestImplementation 'androidx.test:runner:1.3.0' - //androidTestImplementation 'androidx.test.espresso:espresso-core:3.2.0' + //testImplementation 'junit:junit:4.13.2' + //androidTestImplementation 'androidx.test:runner:1.3.0' + //androidTestImplementation 'androidx.test.espresso:espresso-core:3.2.0' } task copyLibs(type: Copy) { - from "libs" - into "src/main/jniLibs" + from "libs" + into "src/main/jniLibs" } tasks.whenTaskAdded { task -> - if (task.name.startsWith('assemble')) { - task.dependsOn('copyLibs') - } + if (task.name.startsWith('assemble')) { + task.dependsOn('copyLibs') + } } diff --git a/sdk/build.gradle b/sdk/build.gradle index b2a45764f..554f537e9 100644 --- a/sdk/build.gradle +++ b/sdk/build.gradle @@ -1,18 +1,12 @@ apply plugin: 'com.android.library' apply plugin: "com.vanniktech.maven.publish" //for publishing -//Plugin for generating test coverage reports. -//This plugin required android build tools 3.6.4 and gradle 5.6.4 -//They are generated by calling the following from the root folder: -// ./gradlew createDebugCoverageReport -//apply plugin: 'jacoco-android' - buildscript { repositories { mavenCentral() } dependencies { - //classpath 'com.dicedmelon.gradle:jacoco-android:0.1.4' + //classpath "com.vanniktech:gradle-android-junit-jacoco-plugin:0.16.0" //test coverage reports classpath 'com.vanniktech:gradle-maven-publish-plugin:0.21.0' //for publishing } } @@ -66,3 +60,6 @@ dependencies { androidTestImplementation "org.mockito:mockito-android:${mockitoVersion}" //androidTestImplementation "com.squareup.okhttp3:mockwebserver:4.9.0" } + +//Plugin for generating test coverage reports. +//apply plugin: "com.vanniktech.android.junit.jacoco" \ No newline at end of file diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/ConnectionQueueTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ConnectionQueueTests.java index ffdae84f8..f146b7e1e 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ConnectionQueueTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ConnectionQueueTests.java @@ -502,7 +502,7 @@ public void testPrepareCommonRequest() { break; case "sdk_version": if (a == 0) { - Assert.assertTrue(pair[1].equals("22.09.4")); + Assert.assertTrue(pair[1].equals("23.6.0")); } else if (a == 1) { Assert.assertTrue(pair[1].equals("123sdf.v-213")); } diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/CountlyConfigTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/CountlyConfigTests.java index c9c56bf1e..aef1907a8 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/CountlyConfigTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/CountlyConfigTests.java @@ -174,8 +174,8 @@ public boolean filterCrash(String crash) { Assert.assertTrue(config.autoTrackingUseShortName); Assert.assertEquals(hv, config.customNetworkRequestHeaders); Assert.assertTrue(config.pushIntentAddMetadata); - Assert.assertTrue(config.enableRemoteConfigAutomaticDownload); - Assert.assertEquals(rcc2, config.remoteConfigCallbackNew); + Assert.assertTrue(config.enableRemoteConfigAutomaticDownloadTriggers); + Assert.assertEquals(rcc2, config.remoteConfigCallbackLegacy); Assert.assertTrue(config.shouldRequireConsent); Assert.assertArrayEquals(fn, config.enabledFeatureNames); Assert.assertTrue(config.httpPostForced); @@ -249,8 +249,8 @@ void assertDefaultValues(CountlyConfig config, boolean includeConstructorValues) Assert.assertFalse(config.autoTrackingUseShortName); Assert.assertNull(config.customNetworkRequestHeaders); Assert.assertFalse(config.pushIntentAddMetadata); - Assert.assertFalse(config.enableRemoteConfigAutomaticDownload); - Assert.assertNull(config.remoteConfigCallbackNew); + Assert.assertFalse(config.enableRemoteConfigAutomaticDownloadTriggers); + Assert.assertNull(config.remoteConfigCallbackLegacy); Assert.assertFalse(config.shouldRequireConsent); Assert.assertNull(config.enabledFeatureNames); Assert.assertFalse(config.httpPostForced); diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/MigrationHelperTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/MigrationHelperTests.java index ebddbbe41..9168a4308 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/MigrationHelperTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/MigrationHelperTests.java @@ -1,34 +1,28 @@ package ly.count.android.sdk; -import android.app.Activity; -import android.content.res.Configuration; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import java.util.HashMap; import java.util.Map; +import ly.count.android.sdk.internal.RemoteConfigValueStore; +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import org.mockito.ArgumentCaptor; import static androidx.test.InstrumentationRegistry.getContext; import static org.junit.Assert.assertNotNull; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyDouble; -import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.mockingDetails; -import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static androidx.test.InstrumentationRegistry.getContext; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; @@ -44,7 +38,7 @@ public class MigrationHelperTests { ModuleLog mockLog; CountlyStore cs; StorageProvider sp; - final int latestSchemaVersion = 1; + final int latestSchemaVersion = 2; @Before public void setUp() { @@ -80,7 +74,7 @@ void validateGeneratedUUID(String deviceId) { @Test public void validateDataSchemaVersion() { MigrationHelper mh = new MigrationHelper(sp, mockLog); - assertEquals(1, mh.DATA_SCHEMA_VERSIONS); + assertEquals(latestSchemaVersion, mh.DATA_SCHEMA_VERSIONS); } /** @@ -120,7 +114,6 @@ public void setInitialSchemaVersionLegacy() { */ @Test public void getCurrentSchemaVersionEmpty() { - cs.clear(); MigrationHelper mh = new MigrationHelper(cs, mockLog); assertEquals(mh.DATA_SCHEMA_VERSIONS, mh.getCurrentSchemaVersion()); @@ -135,7 +128,6 @@ public void getCurrentSchemaVersionEmpty() { */ @Test public void getCurrentSchemaVersionLegacy() { - cs.clear(); cs.addRequest("fff", false); MigrationHelper mh = new MigrationHelper(cs, mockLog); assertEquals(0, mh.getCurrentSchemaVersion()); @@ -151,7 +143,6 @@ public void getCurrentSchemaVersionLegacy() { */ @Test public void getCurrentSchemaVersionMisc() { - cs.clear(); MigrationHelper mh = new MigrationHelper(sp, mockLog); assertEquals(mh.DATA_SCHEMA_VERSIONS, mh.getCurrentSchemaVersion()); @@ -196,8 +187,6 @@ public void performMigration0to1_0_doWork_id_not_provided() { */ @Test public void performMigration0to1_0_init() { - cs.clear(); - Assert.assertFalse(cs.anythingSetInStorage()); Assert.assertNull(cs.getDeviceID()); Assert.assertNull(cs.getDeviceIDType()); @@ -300,7 +289,6 @@ public void performMigration0to1_3() { */ @Test public void performMigration0to1_4() { - cs.clear(); cs.setDeviceIDType(MigrationHelper.legacyDeviceIDTypeValue_AdvertisingID); Countly countly = new Countly().init(new CountlyConfig(ApplicationProvider.getApplicationContext(), TestUtils.commonAppKey, TestUtils.commonURL)); @@ -323,7 +311,6 @@ public void performMigration0to1_4() { */ @Test public void performMigration0to1_5() { - cs.clear(); cs.setDeviceIDType(DeviceIdType.OPEN_UDID.toString()); Countly countly = new Countly().init(new CountlyConfig(ApplicationProvider.getApplicationContext(), TestUtils.commonAppKey, TestUtils.commonURL)); @@ -345,7 +332,6 @@ public void performMigration0to1_5() { */ @Test public void performMigration0to1_6() { - cs.clear(); cs.setDeviceIDType(MigrationHelper.legacyDeviceIDTypeValue_AdvertisingID); cs.setDeviceID("ab"); @@ -363,7 +349,6 @@ public void performMigration0to1_6() { */ @Test public void performMigration0to1_7() { - cs.clear(); cs.setDeviceIDType(DeviceIdType.OPEN_UDID.toString()); cs.setDeviceID("cd"); @@ -380,7 +365,6 @@ public void performMigration0to1_7() { */ @Test public void performMigration0to1_8() { - cs.clear(); cs.setDataSchemaVersion(1); cs.setDeviceIDType(DeviceIdType.OPEN_UDID.toString()); cs.setDeviceID("cd"); @@ -400,7 +384,6 @@ public void performMigration0to1_8() { */ @Test public void performMigration0to1_9() { - cs.clear(); cs.setDeviceID("cd"); Countly countly = new Countly().init(new CountlyConfig(ApplicationProvider.getApplicationContext(), TestUtils.commonAppKey, TestUtils.commonURL)); @@ -417,7 +400,6 @@ public void performMigration0to1_9() { */ @Test public void performMigration0to1_10() { - cs.clear(); cs.addRequest("qqq", false); Countly countly = new Countly().init(new CountlyConfig(ApplicationProvider.getApplicationContext(), TestUtils.commonAppKey, TestUtils.commonURL)); @@ -435,7 +417,6 @@ public void performMigration0to1_10() { */ @Test public void performMigration0to1_11() { - cs.clear(); cs.setDeviceID("cd"); Countly countly = new Countly().init(new CountlyConfig(ApplicationProvider.getApplicationContext(), TestUtils.commonAppKey, TestUtils.commonURL).setDeviceId("asd")); @@ -444,4 +425,112 @@ public void performMigration0to1_11() { Assert.assertEquals("cd", countly.deviceId().getID()); Assert.assertEquals(DeviceIdType.DEVELOPER_SUPPLIED, countly.deviceId().getType()); } + + /** + * Transform the old structure to the new one + * A mixed object should still not throw it off + * All values should be accepted + */ + @Test + public void performMigration1To2_1() throws JSONException { + MigrationHelper mh = new MigrationHelper(cs, mockLog); + + JSONArray jsonArray = new JSONArray(); + jsonArray.put("11"); + jsonArray.put(44); + + JSONObject jsonObject = new JSONObject(); + try { + jsonObject.put("s", 3); + } catch (JSONException e) { + throw new RuntimeException(e); + } + + cs.setRemoteConfigValues("{" + rcEntryLegacy("a", 123) + "," + rcEntryLegacy("b", "fg") + "," + rcEntryLegacy("c", jsonArray) + "," + rcEntryLegacy("d", jsonObject) + "}"); + mh.performMigration1To2(new HashMap<>()); + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(cs.getRemoteConfigValues(), false); + + Assert.assertEquals(4, rcvs.values.length()); + + Assert.assertEquals(123, rcvs.getValue("a").value); + Assert.assertTrue(rcvs.getValue("a").isCurrentUsersData); + + Assert.assertEquals("fg", rcvs.getValue("b").value); + Assert.assertTrue(rcvs.getValue("b").isCurrentUsersData); + + Assert.assertEquals(jsonArray, rcvs.getValue("c").value); + Assert.assertTrue(rcvs.getValue("c").isCurrentUsersData); + + JSONObject retVal = (JSONObject) rcvs.getValue("d").value; + Assert.assertEquals(jsonObject.get("s"), retVal.get("s")); + Assert.assertTrue(rcvs.getValue("d").isCurrentUsersData); + } + + /** + * Make sure that an empty object works for migration + */ + @Test + public void performMigration1To2_2() { + MigrationHelper mh = new MigrationHelper(cs, mockLog); + + cs.setRemoteConfigValues(""); + mh.performMigration1To2(new HashMap<>()); + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(cs.getRemoteConfigValues(), false); + + Assert.assertEquals(0, rcvs.values.length()); + } + + /** + * Make sure that a null object works for migration + */ + @Test + public void performMigration1To2_3() { + MigrationHelper mh = new MigrationHelper(cs, mockLog); + + cs.setRemoteConfigValues(null); + mh.performMigration1To2(new HashMap<>()); + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(cs.getRemoteConfigValues(), false); + + Assert.assertEquals(0, rcvs.values.length()); + } + + /** + * Make sure that garbage doesn't break it + */ + @Test + public void performMigration1To2_4() { + MigrationHelper mh = new MigrationHelper(cs, mockLog); + + cs.setRemoteConfigValues("dsfsdf"); + mh.performMigration1To2(new HashMap<>()); + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(cs.getRemoteConfigValues(), false); + + Assert.assertEquals(0, rcvs.values.length()); + } + + /** + * Create a legacy entry + * + * @param key + * @param value + * @return + */ + public static String rcEntryLegacy(String key, Object value) { + StringBuilder ret = new StringBuilder(); + ret.append("\"" + key + "\":"); + + if (value instanceof String) { + ret.append("\""); + ret.append(value); + ret.append("\""); + } else if (value instanceof JSONArray) { + ret.append(value); + } else if (value instanceof JSONObject) { + ret.append(value); + } else { + ret.append(value); + } + + return ret.toString(); + } } diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleEventsTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleEventsTests.java index f6f6b60b5..81fdb2eca 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleEventsTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleEventsTests.java @@ -3,6 +3,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; import java.util.HashMap; import java.util.Map; +import ly.count.android.sdk.messaging.ModulePush; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -396,6 +397,50 @@ public void recordEventInternalProcessedTest() { mCountly.config_.eventProvider.recordEventInternal(eventKey, segm3, 123, 321.22d, 342.32d, null, null); verify(eqp).recordEventToEventQueue(eq(eventKey), eq(segm3), eq(123), eq(321.22d), eq(342.32d), any(Long.class), any(Integer.class), any(Integer.class), any(String.class), isNull(String.class), eq(""), any(String.class)); } + + /** + * Validating which event keys are triggering force sending of all queued events + */ + @Test + public void eventsForceClearingEQIntoRQ() { + Countly countly = (new Countly()).init((new CountlyConfig(getContext(), "appkey", "http://test.count.ly")).setDeviceId("1234").setLoggingEnabled(true).enableCrashReporting()); + + Assert.assertEquals(0, countly.countlyStore.getEventQueueSize()); + Assert.assertEquals(0, countly.countlyStore.getRequests().length); + + countly.events().recordEvent("rnd_key"); + Assert.assertEquals(1, countly.countlyStore.getEventQueueSize()); + Assert.assertEquals(0, countly.countlyStore.getRequests().length); + + countly.events().recordEvent(ModuleEvents.ACTION_EVENT_KEY); + Assert.assertEquals(2, countly.countlyStore.getEventQueueSize()); + Assert.assertEquals(0, countly.countlyStore.getRequests().length); + + countly.events().recordEvent(ModuleFeedback.NPS_EVENT_KEY); + Assert.assertEquals(0, countly.countlyStore.getEventQueueSize()); + Assert.assertEquals(1, countly.countlyStore.getRequests().length); + + countly.events().recordEvent(ModuleFeedback.SURVEY_EVENT_KEY); + Assert.assertEquals(0, countly.countlyStore.getEventQueueSize()); + Assert.assertEquals(2, countly.countlyStore.getRequests().length); + + countly.events().recordEvent(ModuleFeedback.RATING_EVENT_KEY); + Assert.assertEquals(1, countly.countlyStore.getEventQueueSize()); + Assert.assertEquals(2, countly.countlyStore.getRequests().length); + + countly.events().recordEvent(ModuleViews.VIEW_EVENT_KEY); + Assert.assertEquals(2, countly.countlyStore.getEventQueueSize()); + Assert.assertEquals(2, countly.countlyStore.getRequests().length); + + countly.events().recordEvent(ModuleViews.ORIENTATION_EVENT_KEY); + Assert.assertEquals(3, countly.countlyStore.getEventQueueSize()); + Assert.assertEquals(2, countly.countlyStore.getRequests().length); + + countly.events().recordEvent(ModulePush.PUSH_EVENT_ACTION); + Assert.assertEquals(0, countly.countlyStore.getEventQueueSize()); + Assert.assertEquals(3, countly.countlyStore.getRequests().length); + } + /* //todo should be reworked @Test 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 d9e8a8227..c255e7ccb 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java @@ -2,6 +2,8 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; import java.util.Map; +import ly.count.android.sdk.internal.RemoteConfigHelper; +import ly.count.android.sdk.internal.RemoteConfigValueStore; import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; @@ -11,6 +13,9 @@ import org.junit.runner.RunWith; import static androidx.test.InstrumentationRegistry.getContext; +import static java.lang.Thread.sleep; +import static ly.count.android.sdk.RemoteConfigValueStoreTests.rcArrIntoJSON; +import static ly.count.android.sdk.RemoteConfigValueStoreTests.rcEStr; import static org.mockito.Mockito.mock; @RunWith(AndroidJUnit4.class) @@ -21,133 +26,394 @@ public class ModuleRemoteConfigTests { public void setUp() { Countly.sharedInstance().setLoggingEnabled(true); countlyStore = new CountlyStore(getContext(), mock(ModuleLog.class)); + + countlyStore.clear(); + countlyStore.setDataSchemaVersion(MigrationHelper.DATA_SCHEMA_VERSIONS); } /** - * Basic serialization / deserialization test - * - * @throws JSONException + * Consent removal should clear stored remote config values */ @Test - public void rcvsSerializeDeserialize() throws JSONException { - ModuleRemoteConfig.RemoteConfigValueStore remoteConfigValueStore = ModuleRemoteConfig.RemoteConfigValueStore.dataFromString(null); + public void valuesClearedOnConsentRemoval() { + CountlyConfig config = (new CountlyConfig(getContext(), "appkey", "http://test.count.ly")).setDeviceId("1234").setLoggingEnabled(true).enableCrashReporting(); + config.setRequiresConsent(true); + config.setConsentEnabled(new String[] { Countly.CountlyFeatureNames.remoteConfig }); + config.enableRemoteConfigValueCaching(); + config.enableRemoteConfigAutomaticTriggers(); + Countly countly = (new Countly()).init(config); - remoteConfigValueStore.values.put("fd", 12); - remoteConfigValueStore.values.put("2fd", 142); - remoteConfigValueStore.values.put("f3d", 123); + //set RC + String[] rcArr = new String[] { rcEStr("a", 123), rcEStr("b", "fg") }; + countlyStore.setRemoteConfigValues(RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), false).dataToString()); - ModuleRemoteConfig.RemoteConfigValueStore.dataFromString(remoteConfigValueStore.dataToString()); + Assert.assertEquals(123, countly.remoteConfig().getValue("a").value); + Assert.assertEquals("fg", countly.remoteConfig().getValue("b").value); + + countly.consent().removeConsentAll(); + + Assert.assertEquals(0, countly.remoteConfig().getValues().size()); } /** - * Validating regressive cases for "dataFromString" + * Making sure that automatic RC is triggered on the right requests + * Some differences apply depending on if consent is required or isn't */ @Test - public void rcvsDataFromStringNullEmpty() { - ModuleRemoteConfig.RemoteConfigValueStore rcvs1 = ModuleRemoteConfig.RemoteConfigValueStore.dataFromString(null); - Assert.assertNotNull(rcvs1); - Assert.assertNotNull(rcvs1.values); - Assert.assertEquals(0, rcvs1.values.length()); - - ModuleRemoteConfig.RemoteConfigValueStore rcvs2 = ModuleRemoteConfig.RemoteConfigValueStore.dataFromString(""); - Assert.assertNotNull(rcvs2); - Assert.assertNotNull(rcvs2.values); - Assert.assertEquals(0, rcvs2.values.length()); + public void automaticRCTriggers() { + for (int a = 0; a < 2; a++) { + countlyStore.clear(); + final int[] triggerCounter = new int[] { 0 }; + int intendedCount = 0; + + CountlyConfig config = (new CountlyConfig(getContext(), "appkey", "http://test.count.ly")).setDeviceId("1234").setLoggingEnabled(true).enableCrashReporting(); + config.enableRemoteConfigAutomaticTriggers(); + if (a == 0) { + config.setRequiresConsent(true); + config.setConsentEnabled(new String[] { Countly.CountlyFeatureNames.remoteConfig }); + } + config.immediateRequestGenerator = () -> (ImmediateRequestI) (requestData, customEndpoint, cp, requestShouldBeDelayed, networkingIsEnabled, callback, log) -> { + triggerCounter[0]++; + }; + Countly countly = (new Countly()).init(config); + Assert.assertEquals(++intendedCount, triggerCounter[0]);//init should create a request + + countly.consent().removeConsentAll(); + Assert.assertEquals(intendedCount, triggerCounter[0]);//consent removal does nothing + + countly.consent().giveConsent(new String[] { Countly.CountlyFeatureNames.remoteConfig }); + if (a == 0) { + Assert.assertEquals(++intendedCount, triggerCounter[0]);//giving consent should create a request + } else { + Assert.assertEquals(intendedCount, triggerCounter[0]);//giving consent would not create a request if no consent is required + } + + countly.deviceId().changeWithMerge("dd"); + Assert.assertEquals(intendedCount, triggerCounter[0]);//changing device ID with merging should not create a request + + countly.deviceId().changeWithoutMerge("dd11"); + //todo the current behaviour is slightly out of spec as it would download RC after the RQ would have executed that request + //todo this should be updated once the RQ is reworked + Assert.assertEquals(intendedCount, triggerCounter[0]);//changing device ID without merging should create a request + + countly.deviceId().enableTemporaryIdMode(); + Assert.assertEquals(intendedCount, triggerCounter[0]);//entering tempID mode should not create a request + + countly.deviceId().changeWithMerge("dd"); + if (a == 0) { + Assert.assertEquals(intendedCount, triggerCounter[0]);//exiting temp ID mode with "withMerge" should create a request, but would not since there is no consent + } else { + Assert.assertEquals(++intendedCount, triggerCounter[0]);//exiting temp ID mode with "withMerge" should create a request since consent mode is not prohibiting it + } + + countly.deviceId().enableTemporaryIdMode(); + Assert.assertEquals(intendedCount, triggerCounter[0]);//entering tempID mode should not create a request + + countly.deviceId().changeWithoutMerge("dd"); + if (a == 0) { + Assert.assertEquals(intendedCount, triggerCounter[0]);//exiting temp ID mode with "withoutMerge" should create a request, but would not since there is no consent + } else { + Assert.assertEquals(++intendedCount, triggerCounter[0]);//exiting temp ID mode with "withoutMerge" should create a request since consent mode is not prohibiting it + } + } } /** - * A simple "dataFromString" test case + * Making sure that caching occurs on the required actions and with the required config options + * Creating a "matrix" that goes over combinations of RC config flags and consent */ @Test - public void rcvsDataFromStringSamples_1() { - ModuleRemoteConfig.RemoteConfigValueStore rcvs = ModuleRemoteConfig.RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}"); - Assert.assertNotNull(rcvs); - Assert.assertNotNull(rcvs.values); - Assert.assertEquals(2, rcvs.values.length()); - - Assert.assertEquals(123, rcvs.getValue("a")); - Assert.assertEquals("fg", rcvs.getValue("b")); + public void rcValueCaching() { + for (int a = 0; a < 4; a++) { + countlyStore.clear(); + + CountlyConfig config = (new CountlyConfig(getContext(), "appkey", "http://test.count.ly")).setDeviceId("1234").setLoggingEnabled(true).enableCrashReporting(); + config.enableRemoteConfigAutomaticTriggers(); + + if (a == 0 || a == 1) { + config.setRequiresConsent(true); + config.setConsentEnabled(new String[] { Countly.CountlyFeatureNames.remoteConfig }); + } + + if (a == 0 || a == 2) { + config.enableRemoteConfigValueCaching(); + } + + Countly countly = (new Countly()).init(config); + + Assert.assertEquals(0, countly.remoteConfig().getValues().size()); + + String[] rcArr = new String[] { rcEStr("a", 123), rcEStr("b", "fg") }; + countlyStore.setRemoteConfigValues(RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), false).dataToString()); + Assert.assertEquals(2, countly.remoteConfig().getValues().size()); + assertCValueCachedState(countly.remoteConfig().getValues(), false); + + //changing with merging should leave no impact on this + countly.deviceId().changeWithMerge("dd"); + Assert.assertEquals(2, countly.remoteConfig().getValues().size()); + + //changing without merging should trigger caching. Lack of consent should leave no impact on this + assertCValueCachedState(countly.remoteConfig().getValues(), false); + countly.deviceId().changeWithoutMerge("dd11"); + + for (int b = 0; b < 2; b++) { + if (b == 1) { + countly.consent().giveConsent(new String[] { Countly.CountlyFeatureNames.remoteConfig }); + } + if (a == 0 || a == 2) { + //we preserve + Assert.assertEquals(2, countly.remoteConfig().getValues().size()); + assertCValueCachedState(countly.remoteConfig().getValues(), true); + Assert.assertEquals(123, countly.remoteConfig().getValue("a").value); + Assert.assertEquals("fg", countly.remoteConfig().getValue("b").value); + } else { + Assert.assertEquals(0, countly.remoteConfig().getValues().size()); + } + } + + //entering temp ID mode should trigger caching. Lack of consent should leave no impact on this + countlyStore.setRemoteConfigValues(RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), false).dataToString()); + countly.deviceId().enableTemporaryIdMode(); + + for (int b = 0; b < 2; b++) { + if (b == 1) { + countly.consent().giveConsent(new String[] { Countly.CountlyFeatureNames.remoteConfig }); + } + if (a == 0 || a == 2) { + Assert.assertEquals(2, countly.remoteConfig().getValues().size()); + assertCValueCachedState(countly.remoteConfig().getValues(), true); + Assert.assertEquals(123, countly.remoteConfig().getValue("a").value); + Assert.assertEquals("fg", countly.remoteConfig().getValue("b").value); + } else { + Assert.assertEquals(0, countly.remoteConfig().getValues().size()); + } + } + } + } + + @Test + public void validateValuePersistence() { + //set RC + String[] rcArr = new String[] { rcEStr("a", 123), rcEStr("b", "fg") }; + countlyStore.setRemoteConfigValues(RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), false).dataToString()); + + CountlyConfig config = (new CountlyConfig(getContext(), "appkey", "http://test.count.ly")).setDeviceId("1234").setLoggingEnabled(true).enableCrashReporting(); + config.enableRemoteConfigValueCaching(); + Countly countly = (new Countly()).init(config); + + Assert.assertEquals(123, countly.remoteConfig().getValue("a").value); + Assert.assertEquals("fg", countly.remoteConfig().getValue("b").value); + + Countly countly2 = (new Countly()).init(config); + Assert.assertEquals(123, countly2.remoteConfig().getValue("a").value); + Assert.assertEquals("fg", countly2.remoteConfig().getValue("b").value); } /** - * A more complicated "dataFromString" test case - * It also validates serialization and getAllValues call - * - * @throws JSONException + * Validate the the new and old clears are clearing values after they are directly put into storage + */ + @Test + public void validateClear() { + CountlyConfig config = (new CountlyConfig(getContext(), "appkey", "http://test.count.ly")).setDeviceId("1234").setLoggingEnabled(true).enableCrashReporting(); + config.enableRemoteConfigValueCaching(); + Countly countly = (new Countly()).init(config); + + //set RC + String[] rcArr = new String[] { rcEStr("a", 123), rcEStr("b", "fg") }; + countlyStore.setRemoteConfigValues(RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), false).dataToString()); + + Assert.assertEquals(123, countly.remoteConfig().getValue("a").value); + Assert.assertEquals("fg", countly.remoteConfig().getValue("b").value); + + countly.remoteConfig().clearAll(); + + Assert.assertEquals(0, countly.remoteConfig().getValues().size()); + + countlyStore.setRemoteConfigValues(RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), false).dataToString()); + + Assert.assertEquals(123, countly.remoteConfig().getValue("a").value); + Assert.assertEquals("fg", countly.remoteConfig().getValue("b").value); + + countly.remoteConfig().clearStoredValues(); + + Assert.assertEquals(0, countly.remoteConfig().getValues().size()); + } + + RCDownloadCallback createCallback(int index, int[] resArray) { + RCDownloadCallback ret = new RCDownloadCallback() { + @Override public void callback(RequestResult downloadResult, String error, boolean fullValueUpdate, Map downloadedValues) { + resArray[index]++; + } + }; + + return ret; + } + + /** + * Validating that the RC callbacks are called for the appropriate actions */ @Test - public void rcvsDataFromStringSamples_2() throws JSONException { - String initialString = "{\"321\":123,\"\uD83D\uDE00\":\"\uD83D\uDE01\",\"c\":[3,\"44\",5.1,7.7],\"d\":6.5,\"e\":{\"q\":6,\"w\":\"op\"}}"; - ModuleRemoteConfig.RemoteConfigValueStore rcvs = ModuleRemoteConfig.RemoteConfigValueStore.dataFromString(initialString); - Assert.assertNotNull(rcvs); - Assert.assertNotNull(rcvs.values); - - Assert.assertEquals(initialString, rcvs.dataToString());//quickly validate deserialization - - //validate values while using "get" - Assert.assertEquals(5, rcvs.values.length()); - - Assert.assertEquals(123, rcvs.getValue("321")); - Assert.assertEquals("\uD83D\uDE01", rcvs.getValue("\uD83D\uDE00")); - Assert.assertEquals(6.5, rcvs.getValue("d")); - - Object v1 = rcvs.getValue("c"); - Object v2 = rcvs.getValue("e"); - - JSONArray jArr = (JSONArray) v1; - Assert.assertEquals(4, jArr.length()); - Assert.assertEquals(3, jArr.getInt(0)); - Assert.assertEquals("44", jArr.getString(1)); - Assert.assertEquals(5.1, jArr.getDouble(2), 0.000001); - Assert.assertEquals(7.7, jArr.get(3)); - - JSONObject jObj = (JSONObject) v2; - Assert.assertEquals(2, jObj.length()); - Assert.assertEquals(6, jObj.get("q")); - Assert.assertEquals("op", jObj.get("w")); - - //validate that all of the values are the same when returned with getAllValues - Map allVals = rcvs.getAllValues(); - - Assert.assertEquals(5, allVals.size()); - - Assert.assertEquals(123, allVals.get("321")); - Assert.assertEquals("\uD83D\uDE01", allVals.get("\uD83D\uDE00")); - Assert.assertEquals(6.5, allVals.get("d")); - - Object v3 = allVals.get("c"); - Object v4 = allVals.get("e"); - - JSONArray jArr2 = (JSONArray) v3; - Assert.assertEquals(4, jArr2.length()); - Assert.assertEquals(3, jArr2.getInt(0)); - Assert.assertEquals("44", jArr2.getString(1)); - Assert.assertEquals(5.1, jArr2.getDouble(2), 0.000001); - Assert.assertEquals(7.7, jArr2.get(3)); - - JSONObject jObj2 = (JSONObject) v4; - Assert.assertEquals(2, jObj2.length()); - Assert.assertEquals(6, jObj2.get("q")); - Assert.assertEquals("op", jObj2.get("w")); + public void rcGlobalCallback() { + int[] resArray = new int[10]; + int cIndex = 0; + + RemoteConfigCallback oldRCC = error -> resArray[0]++; + cIndex++; + + RCDownloadCallback c1 = createCallback(cIndex++, resArray); + RCDownloadCallback c2 = createCallback(cIndex++, resArray); + RCDownloadCallback c3 = createCallback(cIndex++, resArray); + RCDownloadCallback c4 = createCallback(cIndex++, resArray); + RCDownloadCallback c5 = createCallback(cIndex++, resArray); + RCDownloadCallback c6 = createCallback(cIndex, resArray); + + CountlyConfig config = (new CountlyConfig(getContext(), "appkey", "http://test.count.ly")).setDeviceId("1234").setLoggingEnabled(true).enableRemoteConfigAutomaticTriggers(); + config.RemoteConfigRegisterGlobalCallback(c1); + config.RemoteConfigRegisterGlobalCallback(c2); + config.setRemoteConfigAutomaticDownload(true, oldRCC); + config.immediateRequestGenerator = () -> (ImmediateRequestI) (requestData, customEndpoint, cp, requestShouldBeDelayed, networkingIsEnabled, callback, log) -> { + callback.callback(null); + }; + + Countly countly = (new Countly()).init(config); + + //check initial global ones + Assert.assertEquals(1, resArray[0]); + Assert.assertEquals(1, resArray[1]); + Assert.assertEquals(1, resArray[2]); + Assert.assertEquals(0, resArray[3]); + + countly.remoteConfig().removeDownloadCallback(c1); + countly.remoteConfig().registerDownloadCallback(c3); + + countly.remoteConfig().downloadAllKeys(c4); + Assert.assertEquals(2, resArray[0]); + Assert.assertEquals(1, resArray[1]); + Assert.assertEquals(2, resArray[2]); + Assert.assertEquals(1, resArray[3]); + Assert.assertEquals(1, resArray[4]); + + countly.remoteConfig().downloadSpecificKeys(new String[] {}, c5); + Assert.assertEquals(3, resArray[0]); + Assert.assertEquals(1, resArray[1]); + Assert.assertEquals(3, resArray[2]); + Assert.assertEquals(2, resArray[3]); + Assert.assertEquals(1, resArray[4]); + Assert.assertEquals(1, resArray[5]); + + countly.remoteConfig().removeDownloadCallback(c1); + countly.remoteConfig().downloadOmittingKeys(new String[] {}, c6); + Assert.assertEquals(4, resArray[0]); + Assert.assertEquals(1, resArray[1]); + Assert.assertEquals(4, resArray[2]); + Assert.assertEquals(3, resArray[3]); + Assert.assertEquals(1, resArray[4]); + Assert.assertEquals(1, resArray[5]); + Assert.assertEquals(1, resArray[6]); } /** - * Simple test for value merging + * Validate that the new and old getters return the correct values when putting them directly into storage * * @throws JSONException */ @Test - public void rcvsMergeValues_1() throws JSONException { - ModuleRemoteConfig.RemoteConfigValueStore rcvs1 = ModuleRemoteConfig.RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}"); - ModuleRemoteConfig.RemoteConfigValueStore rcvs2 = ModuleRemoteConfig.RemoteConfigValueStore.dataFromString("{\"b\": 123.3,\"c\": \"uio\"}"); + public void validateGetters() throws JSONException { + CountlyConfig config = (new CountlyConfig(getContext(), "appkey", "http://test.count.ly")).setDeviceId("1234").setLoggingEnabled(true).enableCrashReporting(); + config.enableRemoteConfigValueCaching(); + Countly countly = (new Countly()).init(config); + + //set RC + JSONArray jArrI = new JSONArray("[3,\"44\",5.1,7.7]"); + JSONObject jObjI = new JSONObject("{\"q\":6,\"w\":\"op\"}"); + String[] rcArr = new String[] { rcEStr("a", 123, false), rcEStr("b", "fg"), rcEStr("c", 222222222222L, false), rcEStr("d", 1.5d), rcEStr("e", jArrI, false), rcEStr("f", jObjI) }; + countlyStore.setRemoteConfigValues(RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), false).dataToString()); + + Assert.assertEquals(123, countly.remoteConfig().getValue("a").value); + Assert.assertEquals(123, countly.remoteConfig().getValueForKey("a")); + Assert.assertFalse(countly.remoteConfig().getValue("a").isCurrentUsersData); + + Assert.assertEquals("fg", countly.remoteConfig().getValue("b").value); + Assert.assertEquals("fg", countly.remoteConfig().getValueForKey("b")); + Assert.assertTrue(countly.remoteConfig().getValue("b").isCurrentUsersData); + + Assert.assertEquals(222222222222L, countly.remoteConfig().getValue("c").value); + Assert.assertEquals(222222222222L, countly.remoteConfig().getValueForKey("c")); + Assert.assertFalse(countly.remoteConfig().getValue("c").isCurrentUsersData); + + Assert.assertEquals(1.5d, countly.remoteConfig().getValue("d").value); + Assert.assertEquals(1.5d, countly.remoteConfig().getValueForKey("d")); + Assert.assertTrue(countly.remoteConfig().getValue("d").isCurrentUsersData); + + Assert.assertEquals(jArrI.toString(), countly.remoteConfig().getValue("e").value.toString()); + Assert.assertEquals(jArrI.toString(), countly.remoteConfig().getValueForKey("e").toString()); + Assert.assertFalse(countly.remoteConfig().getValue("e").isCurrentUsersData); + + Assert.assertEquals(jObjI.toString(), countly.remoteConfig().getValue("f").value.toString()); + Assert.assertEquals(jObjI.toString(), countly.remoteConfig().getValueForKey("f").toString()); + Assert.assertTrue(countly.remoteConfig().getValue("f").isCurrentUsersData); + + Map valsOld = countly.remoteConfig().getAllValues(); + Map valsNew = countly.remoteConfig().getValues(); + + Assert.assertEquals(valsNew.size(), valsOld.size()); + + for (Map.Entry entry : valsNew.entrySet()) { + Object valN = entry.getValue().value; + Object valO = valsOld.get(entry.getKey()); + + if (valN instanceof JSONObject || valN instanceof JSONArray) { + Assert.assertEquals(valN.toString(), valO.toString()); + } else { + Assert.assertEquals(valN, valO); + } + } + } + + /** + * Just making sure nothing explodes when passing bad values + */ + @Test + public void passingBadValues() { + CountlyConfig config = (new CountlyConfig(getContext(), "appkey", "http://test.count.ly")).setDeviceId("1234").setLoggingEnabled(true).enableCrashReporting(); + config.enableRemoteConfigValueCaching(); + config.enableRemoteConfigAutomaticTriggers(); + Countly countly = (new Countly()).init(config); + + countly.remoteConfig().getValue(null); + countly.remoteConfig().getValue(""); + countly.remoteConfig().getValueForKey(null); + countly.remoteConfig().getValueForKey(""); + + countly.remoteConfig().update(null); + countly.remoteConfig().downloadAllKeys(null); + + countly.remoteConfig().downloadOmittingKeys(null, null); + countly.remoteConfig().downloadOmittingKeys(new String[] {}, null); + countly.remoteConfig().updateExceptKeys(null, null); + countly.remoteConfig().updateExceptKeys(new String[] {}, null); - rcvs1.mergeValues(rcvs2.values); + countly.remoteConfig().downloadSpecificKeys(null, null); + countly.remoteConfig().downloadSpecificKeys(new String[] {}, null); + countly.remoteConfig().updateForKeysOnly(null, null); + countly.remoteConfig().updateForKeysOnly(new String[] {}, null); - Assert.assertEquals(3, rcvs1.values.length()); + countly.remoteConfig().enrollIntoABTestsForKeys(null); + countly.remoteConfig().enrollIntoABTestsForKeys(new String[] {}); - Assert.assertEquals(123, rcvs1.getValue("a")); - Assert.assertEquals(123.3, rcvs1.getValue("b")); - Assert.assertEquals("uio", rcvs1.getValue("c")); + countly.remoteConfig().exitABTestsForKeys(null); + countly.remoteConfig().exitABTestsForKeys(new String[] {}); + + countly.remoteConfig().testingGetVariantsForKey(null); + countly.remoteConfig().testingGetVariantsForKey(""); + + countly.remoteConfig().testingEnrollIntoVariant(null, null, null); + countly.remoteConfig().testingEnrollIntoVariant("", "", null); + + countly.remoteConfig().testingDownloadVariantInformation(null); + + countly.remoteConfig().registerDownloadCallback(null); + countly.remoteConfig().removeDownloadCallback(null); } /** @@ -155,32 +421,27 @@ public void rcvsMergeValues_1() throws JSONException { */ @Test public void validateKeyIncludeExclude() { - countlyStore.clear(); - CountlyConfig cc = new CountlyConfig(getContext(), "aaa", "http://www.aa.bb"); - Countly countly = new Countly(); - countly.init(cc); - //first a few cases with empty or null values - String[] res = countly.moduleRemoteConfig.prepareKeysIncludeExclude(null, null); + String[] res = RemoteConfigHelper.prepareKeysIncludeExclude(null, null, mock(ModuleLog.class)); Assert.assertNull(res[0]); Assert.assertNull(res[1]); - res = countly.moduleRemoteConfig.prepareKeysIncludeExclude(new String[0], new String[0]); + res = RemoteConfigHelper.prepareKeysIncludeExclude(new String[0], new String[0], mock(ModuleLog.class)); Assert.assertNull(res[0]); Assert.assertNull(res[1]); //setting first - res = countly.moduleRemoteConfig.prepareKeysIncludeExclude(new String[] { "a", "b" }, null); + res = RemoteConfigHelper.prepareKeysIncludeExclude(new String[] { "a", "b" }, null, mock(ModuleLog.class)); Assert.assertEquals("[\"a\",\"b\"]", res[0]); Assert.assertNull(res[1]); //setting second - res = countly.moduleRemoteConfig.prepareKeysIncludeExclude(null, new String[] { "c", "d" }); + res = RemoteConfigHelper.prepareKeysIncludeExclude(null, new String[] { "c", "d" }, mock(ModuleLog.class)); Assert.assertNull(res[0]); Assert.assertEquals("[\"c\",\"d\"]", res[1]); //setting both (include takes precedence) - res = countly.moduleRemoteConfig.prepareKeysIncludeExclude(new String[] { "e", "f" }, new String[] { "g", "h" }); + res = RemoteConfigHelper.prepareKeysIncludeExclude(new String[] { "e", "f" }, new String[] { "g", "h" }, mock(ModuleLog.class)); Assert.assertEquals("[\"e\",\"f\"]", res[0]); Assert.assertNull(res[1]); } @@ -191,14 +452,13 @@ public void validateKeyIncludeExclude() { */ @Test public void validateMergeReceivedResponse() throws Exception { - countlyStore.clear(); CountlyConfig cc = new CountlyConfig(getContext(), "aaa", "http://www.aa.bb"); Countly countly = new Countly(); countly.init(cc); - ModuleRemoteConfig.RemoteConfigValueStore rcvs1 = ModuleRemoteConfig.RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}"); - ModuleRemoteConfig.RemoteConfigValueStore rcvs2 = ModuleRemoteConfig.RemoteConfigValueStore.dataFromString("{\"b\": 33.44,\"c\": \"ww\"}"); - ModuleRemoteConfig.RemoteConfigValueStore rcvs3 = ModuleRemoteConfig.RemoteConfigValueStore.dataFromString("{\"t\": {},\"87\": \"yy\"}"); + RemoteConfigValueStore rcvs1 = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}", false); + RemoteConfigValueStore rcvs2 = RemoteConfigValueStore.dataFromString("{\"b\": 33.44,\"c\": \"ww\"}", false); + RemoteConfigValueStore rcvs3 = RemoteConfigValueStore.dataFromString("{\"t\": {},\"87\": \"yy\"}", false); //check initial state Map vals = countly.remoteConfig().getAllValues(); @@ -206,7 +466,7 @@ public void validateMergeReceivedResponse() throws Exception { Assert.assertEquals(0, vals.size()); //add first values without clearing - countly.moduleRemoteConfig.mergeCheckResponseIntoCurrentValues(false, rcvs1.values); + countly.moduleRemoteConfig.mergeCheckResponseIntoCurrentValues(false, RemoteConfigHelper.DownloadedValuesIntoMap(rcvs1.values)); vals = countly.remoteConfig().getAllValues(); Assert.assertEquals(2, vals.size()); @@ -214,7 +474,7 @@ public void validateMergeReceivedResponse() throws Exception { Assert.assertEquals("fg", vals.get("b")); //add second pair of values without clearing - countly.moduleRemoteConfig.mergeCheckResponseIntoCurrentValues(false, rcvs2.values); + countly.moduleRemoteConfig.mergeCheckResponseIntoCurrentValues(false, RemoteConfigHelper.DownloadedValuesIntoMap(rcvs2.values)); vals = countly.remoteConfig().getAllValues(); Assert.assertEquals(3, vals.size()); @@ -223,7 +483,7 @@ public void validateMergeReceivedResponse() throws Exception { Assert.assertEquals("ww", vals.get("c")); //add third pair with full clear - countly.moduleRemoteConfig.mergeCheckResponseIntoCurrentValues(true, rcvs3.values); + countly.moduleRemoteConfig.mergeCheckResponseIntoCurrentValues(true, RemoteConfigHelper.DownloadedValuesIntoMap(rcvs3.values)); vals = countly.remoteConfig().getAllValues(); Assert.assertEquals(2, vals.size()); @@ -231,4 +491,14 @@ public void validateMergeReceivedResponse() throws Exception { Assert.assertNotNull(vals.get("t")); Assert.assertEquals(0, ((JSONObject) vals.get("t")).length()); } + + static void assertCValueCachedState(Map rcValues, boolean valuesAreCached) { + for (Map.Entry entry : rcValues.entrySet()) { + if (valuesAreCached) { + Assert.assertFalse(entry.getValue().isCurrentUsersData); + } else { + Assert.assertTrue(entry.getValue().isCurrentUsersData); + } + } + } } diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java new file mode 100644 index 000000000..90783d437 --- /dev/null +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java @@ -0,0 +1,229 @@ +package ly.count.android.sdk; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import java.util.Map; +import ly.count.android.sdk.internal.RemoteConfigHelper; +import ly.count.android.sdk.internal.RemoteConfigValueStore; +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import static androidx.test.InstrumentationRegistry.getContext; +import static org.mockito.Mockito.mock; + +@RunWith(AndroidJUnit4.class) +public class RemoteConfigValueStoreTests { + CountlyStore countlyStore; + + @Before + public void setUp() { + Countly.sharedInstance().setLoggingEnabled(true); + countlyStore = new CountlyStore(getContext(), mock(ModuleLog.class)); + } + + /** + * Basic serialization / deserialization test + * + * @throws JSONException + */ + @Test + public void rcvsSerializeDeserialize() throws JSONException { + RemoteConfigValueStore remoteConfigValueStore = RemoteConfigValueStore.dataFromString(null, false); + + remoteConfigValueStore.values.put("fd", 12); + remoteConfigValueStore.values.put("2fd", 142); + remoteConfigValueStore.values.put("f3d", 123); + + RemoteConfigValueStore.dataFromString(remoteConfigValueStore.dataToString(), false); + } + + /** + * Validating regressive cases for "dataFromString" + */ + @Test + public void rcvsDataFromStringNullEmpty() { + RemoteConfigValueStore rcvs1 = RemoteConfigValueStore.dataFromString(null, false); + Assert.assertNotNull(rcvs1); + Assert.assertNotNull(rcvs1.values); + Assert.assertEquals(0, rcvs1.values.length()); + + RemoteConfigValueStore rcvs2 = RemoteConfigValueStore.dataFromString("", false); + Assert.assertNotNull(rcvs2); + Assert.assertNotNull(rcvs2.values); + Assert.assertEquals(0, rcvs2.values.length()); + } + + /** + * A simple "dataFromString" test case + */ + @Test + public void rcvsDataFromStringSamples_1() { + String[] rcArr = new String[] { rcEStr("a", 123, false), rcEStr("b", "fg", false) }; + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), true); + Assert.assertNotNull(rcvs); + Assert.assertNotNull(rcvs.values); + Assert.assertEquals(2, rcvs.values.length()); + + Assert.assertEquals(123, rcvs.getValueLegacy("a")); + Assert.assertEquals("fg", rcvs.getValueLegacy("b")); + Assert.assertEquals(123, rcvs.getValue("a").value); + Assert.assertEquals("fg", rcvs.getValue("b").value); + Assert.assertFalse(rcvs.getValue("a").isCurrentUsersData); + Assert.assertFalse(rcvs.getValue("b").isCurrentUsersData); + } + + /** + * A more complicated "dataFromString" test case + * It also validates serialization and getAllValues call + * + * @throws JSONException + */ + @Test + public void rcvsDataFromStringSamples_2() throws JSONException { + JSONArray jArrI = new JSONArray("[3,\"44\",5.1,7.7]"); + JSONObject jObjI = new JSONObject("{\"q\":6,\"w\":\"op\"}"); + + String[] rcArr = new String[] { rcEStr("321", 123, false), rcEStr("πŸ˜€", "😁"), rcEStr("c", jArrI), rcEStr("d", 6.5), rcEStr("e", jObjI) }; + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), true); + Assert.assertNotNull(rcvs); + Assert.assertNotNull(rcvs.values); + + //validate values while using "get" + Assert.assertEquals(5, rcvs.values.length()); + + Assert.assertEquals(123, rcvs.getValueLegacy("321")); + Assert.assertEquals("\uD83D\uDE01", rcvs.getValueLegacy("\uD83D\uDE00")); + Assert.assertEquals(6.5, rcvs.getValueLegacy("d")); + + Object v1 = rcvs.getValueLegacy("c"); + Object v2 = rcvs.getValueLegacy("e"); + + JSONArray jArr = (JSONArray) v1; + Assert.assertEquals(4, jArr.length()); + Assert.assertEquals(3, jArr.getInt(0)); + Assert.assertEquals("44", jArr.getString(1)); + Assert.assertEquals(5.1, jArr.getDouble(2), 0.000001); + Assert.assertEquals(7.7, jArr.get(3)); + + JSONObject jObj = (JSONObject) v2; + Assert.assertEquals(2, jObj.length()); + Assert.assertEquals(6, jObj.get("q")); + Assert.assertEquals("op", jObj.get("w")); + + //validate that all of the values are the same when returned with getAllValues + Map allVals = rcvs.getAllValuesLegacy(); + + Assert.assertEquals(5, allVals.size()); + + Assert.assertEquals(123, allVals.get("321")); + Assert.assertEquals("\uD83D\uDE01", allVals.get("\uD83D\uDE00")); + Assert.assertEquals(6.5, allVals.get("d")); + + Object v3 = allVals.get("c"); + Object v4 = allVals.get("e"); + + JSONArray jArr2 = (JSONArray) v3; + Assert.assertEquals(4, jArr2.length()); + Assert.assertEquals(3, jArr2.getInt(0)); + Assert.assertEquals("44", jArr2.getString(1)); + Assert.assertEquals(5.1, jArr2.getDouble(2), 0.000001); + Assert.assertEquals(7.7, jArr2.get(3)); + + JSONObject jObj2 = (JSONObject) v4; + Assert.assertEquals(2, jObj2.length()); + Assert.assertEquals(6, jObj2.get("q")); + Assert.assertEquals("op", jObj2.get("w")); + } + + @Test + public void dataFromString_CurrentStructure() { + String[] rcArr = new String[] { rcEStr("a", 123), rcEStr("b", "ccx", false) }; + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), false); + + Assert.assertEquals(123, rcvs.getValue("a").value); + Assert.assertTrue(rcvs.getValue("a").isCurrentUsersData); + + Assert.assertEquals("ccx", rcvs.getValue("b").value); + Assert.assertFalse(rcvs.getValue("b").isCurrentUsersData); + } + + /** + * Simple test for value merging + */ + @Test + public void rcvsMergeValues_1() throws JSONException { + String[] rcArr = new String[] { rcEStr("a", 123), rcEStr("b", "fg") }; + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), false); + JSONObject obj = new JSONObject("{\"b\": 123.3,\"c\": \"uio\"}"); + + Map newRC = RemoteConfigHelper.DownloadedValuesIntoMap(obj); + rcvs.mergeValues(newRC, false); + + Assert.assertEquals(3, rcvs.values.length()); + + Assert.assertEquals(123, rcvs.getValue("a").value); + Assert.assertEquals(123.3, rcvs.getValue("b").value); + Assert.assertEquals("uio", rcvs.getValue("c").value); + } + + /** + * Create a remote config entry string + * + * @param key + * @param value + * @return + */ + public static String rcEStr(String key, Object value, boolean isCurrentUser) { + StringBuilder ret = new StringBuilder(); + ret.append("\"").append(key).append("\":{\""); + ret.append(RemoteConfigValueStore.keyValue); + ret.append("\":"); + + if (value instanceof String) { + ret.append("\""); + ret.append(value); + ret.append("\""); + } else if (value instanceof JSONArray) { + ret.append(value); + } else if (value instanceof JSONObject) { + ret.append(value); + } else { + ret.append(value); + } + + ret.append(",\""); + ret.append(RemoteConfigValueStore.keyCacheFlag); + ret.append("\":"); + + ret.append(isCurrentUser ? RemoteConfigValueStore.cacheValFresh : RemoteConfigValueStore.cacheValCached); + + ret.append("}"); + + return ret.toString(); + } + + public static String rcEStr(String key, Object value) { + return rcEStr(key, value, true); + } + + public static String rcArrIntoJSON(String[] arr) { + StringBuilder sb = new StringBuilder(); + sb.append("{"); + + for (int a = 0; a < arr.length; a++) { + if (a != 0) { + sb.append(","); + } + + sb.append(arr[a]); + } + String input = "{" + rcEStr("a", 123, true) + "," + rcEStr("b", "ccx", false) + "}"; + + sb.append("}"); + return sb.toString(); + } +} diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigVariantControlTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigVariantControlTests.java index 904a3e46a..14f4428c4 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigVariantControlTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigVariantControlTests.java @@ -2,6 +2,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4; import java.util.Map; +import ly.count.android.sdk.internal.RemoteConfigHelper; import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; @@ -29,16 +30,16 @@ public void testConvertVariantsJsonToMap_ValidInput_Multi() throws JSONException // Create a sample JSON object with variants JSONObject variantsObj = new JSONObject(); JSONArray variantArray1 = new JSONArray(); - variantArray1.put(new JSONObject().put("name", "Variant 1")); - variantArray1.put(new JSONObject().put("name", "Variant 2")); + variantArray1.put(new JSONObject().put(ModuleRemoteConfig.variantObjectNameKey, "Variant 1")); + variantArray1.put(new JSONObject().put(ModuleRemoteConfig.variantObjectNameKey, "Variant 2")); JSONArray variantArray2 = new JSONArray(); - variantArray2.put(new JSONObject().put("name", "Variant 3")); - variantArray2.put(new JSONObject().put("name", "Variant 4")); + variantArray2.put(new JSONObject().put(ModuleRemoteConfig.variantObjectNameKey, "Variant 3")); + variantArray2.put(new JSONObject().put(ModuleRemoteConfig.variantObjectNameKey, "Variant 4")); variantsObj.put("key1", variantArray1); variantsObj.put("key2", variantArray2); // Call the function to convert variants JSON to a map - Map resultMap = ModuleRemoteConfig.convertVariantsJsonToMap(variantsObj); + Map resultMap = RemoteConfigHelper.convertVariantsJsonToMap(variantsObj, mock(ModuleLog.class)); // Assert the expected map values Assert.assertEquals(2, resultMap.size()); @@ -60,11 +61,11 @@ public void testConvertVariantsJsonToMap_ValidInput_Multi() throws JSONException public void testConvertVariantsJsonToMap_ValidInput_Single() throws JSONException { // Create a sample JSON object with valid variants JSONObject variantsObj = new JSONObject(); - variantsObj.put("key1", new JSONArray().put(new JSONObject().put("name", "Variant 1"))); - variantsObj.put("key2", new JSONArray().put(new JSONObject().put("name", "Variant 2"))); + variantsObj.put("key1", new JSONArray().put(new JSONObject().put(ModuleRemoteConfig.variantObjectNameKey, "Variant 1"))); + variantsObj.put("key2", new JSONArray().put(new JSONObject().put(ModuleRemoteConfig.variantObjectNameKey, "Variant 2"))); // Call the function to convert variants JSON to a map - Map resultMap = ModuleRemoteConfig.convertVariantsJsonToMap(variantsObj); + Map resultMap = RemoteConfigHelper.convertVariantsJsonToMap(variantsObj, mock(ModuleLog.class)); // Assert the expected map values Assert.assertEquals(2, resultMap.size()); @@ -87,7 +88,7 @@ public void testConvertVariantsJsonToMap_InvalidInput() throws JSONException { variantsObj.put("key1", new JSONArray().put(new JSONObject().put("invalid_key", "Invalid Value"))); // Call the function to convert variants JSON to a map - Map resultMap = ModuleRemoteConfig.convertVariantsJsonToMap(variantsObj); + Map resultMap = RemoteConfigHelper.convertVariantsJsonToMap(variantsObj, mock(ModuleLog.class)); Assert.assertEquals(1, resultMap.size()); // Assert the values for key1 @@ -102,24 +103,23 @@ public void testConvertVariantsJsonToMap_InvalidJson() throws JSONException { variantsObj.put("key1", "Invalid JSON"); // Call the function to convert variants JSON to a map (expecting JSONException) - ModuleRemoteConfig.convertVariantsJsonToMap(variantsObj); + RemoteConfigHelper.convertVariantsJsonToMap(variantsObj, mock(ModuleLog.class)); // Call the function to convert variants JSON to a map - Map resultMap = ModuleRemoteConfig.convertVariantsJsonToMap(variantsObj); - Assert.assertEquals(1, resultMap.size()); - - // Assert the values for key1 - String[] key1Variants = resultMap.get("key1"); - Assert.assertEquals(0, key1Variants.length); + Map resultMap = RemoteConfigHelper.convertVariantsJsonToMap(variantsObj, mock(ModuleLog.class)); + Assert.assertEquals(0, resultMap.size()); } + /** + * Empty JSON should produce an empty map + */ @Test - public void testConvertVariantsJsonToMap_NoValues() throws JSONException { + public void testConvertVariantsJsonToMap_NoValues() { // Create an empty JSON object JSONObject variantsObj = new JSONObject(); // Call the function to convert variants JSON to a map - Map resultMap = ModuleRemoteConfig.convertVariantsJsonToMap(variantsObj); + Map resultMap = RemoteConfigHelper.convertVariantsJsonToMap(variantsObj, mock(ModuleLog.class)); // Assert that the map is empty Assert.assertTrue(resultMap.isEmpty()); @@ -134,13 +134,13 @@ public void testConvertVariantsJsonToMap_DifferentStructures() throws JSONExcept variantsObj.put("key1", new JSONArray()); // Structure 2: Single variant as JSON object - variantsObj.put("key2", new JSONArray().put(new JSONObject().put("name", "Variant 1"))); + variantsObj.put("key2", new JSONArray().put(new JSONObject().put(ModuleRemoteConfig.variantObjectNameKey, "Variant 1"))); // Structure 3: Multiple variants as JSON objects - variantsObj.put("key3", new JSONArray().put(new JSONObject().put("name", "Variant 2")).put(new JSONObject().put("name", "Variant 3"))); + variantsObj.put("key3", new JSONArray().put(new JSONObject().put(ModuleRemoteConfig.variantObjectNameKey, "Variant 2")).put(new JSONObject().put(ModuleRemoteConfig.variantObjectNameKey, "Variant 3"))); // Call the function to convert variants JSON to a map - Map resultMap = ModuleRemoteConfig.convertVariantsJsonToMap(variantsObj); + Map resultMap = RemoteConfigHelper.convertVariantsJsonToMap(variantsObj, mock(ModuleLog.class)); // Assert the expected map values Assert.assertEquals(3, resultMap.size()); @@ -161,13 +161,18 @@ public void testConvertVariantsJsonToMap_DifferentStructures() throws JSONExcept Assert.assertEquals("Variant 3", key3Variants[1]); } + /** + * variant with a string test name "null" and a string variant name "null" should be let through + * + * @throws JSONException + */ @Test public void testConvertVariantsJsonToMap_NullJsonKey() throws JSONException { - // Test with a null JSON key + // Test with a null JSON key string String variantsObj = "{\"null\":[{\"name\":\"null\"}]}"; // Call the function to convert variants JSON to a map (expecting JSONException) - Map resultMap = ModuleRemoteConfig.convertVariantsJsonToMap(new JSONObject(variantsObj)); + Map resultMap = RemoteConfigHelper.convertVariantsJsonToMap(new JSONObject(variantsObj), mock(ModuleLog.class)); // Assert the values for key1 String[] key1Variants = resultMap.get("null"); @@ -181,7 +186,7 @@ public void testNormalFlow() { Countly countly = (new Countly()).init(config); // Developer did not provide a callback - countly.moduleRemoteConfig.remoteConfigInterface.testingFetchVariantInformation(null); + countly.moduleRemoteConfig.remoteConfigInterface.testingDownloadVariantInformation(null); Map values = countly.moduleRemoteConfig.remoteConfigInterface.testingGetAllVariants(); String[] variantArray = countly.moduleRemoteConfig.remoteConfigInterface.testingGetVariantsForKey("key"); String[] variantArrayFalse = countly.moduleRemoteConfig.remoteConfigInterface.testingGetVariantsForKey("key2"); @@ -192,31 +197,36 @@ public void testNormalFlow() { Assert.assertEquals("variant", key1Variants[0]); Assert.assertEquals(1, variantArray.length); Assert.assertEquals("variant", variantArray[0]); - Assert.assertEquals(0, variantArrayFalse.length); + Assert.assertNull(variantArrayFalse); } + /** + * Reject a variant if it's name is a null json value + */ @Test public void testNullVariant() { CountlyConfig config = TestUtils.createVariantConfig(createIRGForSpecificResponse("{\"key\":[{\"name\":null}]}")); Countly countly = (new Countly()).init(config); // Developer did not provide a callback - countly.moduleRemoteConfig.remoteConfigInterface.testingFetchVariantInformation(null); + countly.moduleRemoteConfig.remoteConfigInterface.testingDownloadVariantInformation(null); Map values = countly.moduleRemoteConfig.remoteConfigInterface.testingGetAllVariants(); // Assert the values String[] key1Variants = values.get("key"); - Assert.assertEquals(1, key1Variants.length); - Assert.assertEquals("null", key1Variants[0]); // TODO: is fine? + Assert.assertEquals(0, key1Variants.length); } + /** + * Reject variant entries where the object has no entry with the "name" key + */ @Test public void testFilteringWrongKeys() { CountlyConfig config = TestUtils.createVariantConfig(createIRGForSpecificResponse("{\"key\":[{\"noname\":\"variant1\"},{\"name\":\"variant2\"}]}")); Countly countly = (new Countly()).init(config); // Developer did not provide a callback - countly.moduleRemoteConfig.remoteConfigInterface.testingFetchVariantInformation(null); + countly.moduleRemoteConfig.remoteConfigInterface.testingDownloadVariantInformation(null); Map values = countly.moduleRemoteConfig.remoteConfigInterface.testingGetAllVariants(); //Assert the values @@ -226,27 +236,36 @@ public void testFilteringWrongKeys() { } ImmediateRequestGenerator createIRGForSpecificResponse(final String targetResponse) { - return new ImmediateRequestGenerator() { - @Override public ImmediateRequestI CreateImmediateRequestMaker() { - return new ImmediateRequestI() { - @Override public void doWork(String requestData, String customEndpoint, ConnectionProcessor cp, boolean requestShouldBeDelayed, boolean networkingIsEnabled, ImmediateRequestMaker.InternalImmediateRequestCallback callback, ModuleLog log) { - if (targetResponse == null) { - callback.callback(null); - return; - } - - JSONObject jobj = null; - - try { - jobj = new JSONObject(targetResponse); - } catch (JSONException e) { - e.printStackTrace(); - } - - callback.callback(jobj); - } - }; + return () -> (requestData, customEndpoint, cp, requestShouldBeDelayed, networkingIsEnabled, callback, log) -> { + if (targetResponse == null) { + callback.callback(null); + return; } + + JSONObject jobj = null; + + try { + jobj = new JSONObject(targetResponse); + } catch (JSONException e) { + e.printStackTrace(); + } + + callback.callback(jobj); }; } + + @Test + public void variantGetters_preDownload() { + CountlyConfig config = TestUtils.createVariantConfig(null); + Countly countly = (new Countly()).init(config); + + //should return empty map of values + Map vals = countly.remoteConfig().testingGetAllVariants(); + Assert.assertEquals(0, vals.size()); + + //single getters should also not fail on requesting values + Assert.assertNull(countly.remoteConfig().testingGetVariantsForKey("a")); + Assert.assertNull(countly.remoteConfig().testingGetVariantsForKey("")); + Assert.assertNull(countly.remoteConfig().testingGetVariantsForKey(null)); + } } diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/ScenarioRemoteConfigTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ScenarioRemoteConfigTests.java new file mode 100644 index 000000000..3e3500439 --- /dev/null +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ScenarioRemoteConfigTests.java @@ -0,0 +1,34 @@ +package ly.count.android.sdk; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import static androidx.test.InstrumentationRegistry.getContext; +import static org.mockito.Mockito.mock; + +@RunWith(AndroidJUnit4.class) +public class ScenarioRemoteConfigTests { + CountlyStore store; + StorageProvider sp; + + @Before + public void setUp() { + Countly.sharedInstance().setLoggingEnabled(true); + store = new CountlyStore(getContext(), mock(ModuleLog.class), false); + sp = store; + store.clear(); + } + + @After + public void tearDown() { + store.clear(); + } + + @Test + public void bump() { + + } +} diff --git a/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java b/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java index 76184635b..57ca461b7 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java +++ b/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java @@ -693,7 +693,7 @@ private String prepareLocationData(boolean locationDisabled, String locationCoun return data; } - public String prepareRemoteConfigRequest(@Nullable String keysInclude, @Nullable String keysExclude, @NonNull String preparedMetrics) { + public String prepareRemoteConfigRequestLegacy(@Nullable String keysInclude, @Nullable String keysExclude, @NonNull String preparedMetrics) { String data = prepareCommonRequestData() + "&method=fetch_remote_config" + "&device_id=" + UtilsNetworking.urlEncodeString(deviceIdProvider_.getDeviceId()); @@ -713,6 +713,46 @@ public String prepareRemoteConfigRequest(@Nullable String keysInclude, @Nullable return data; } + public String prepareRemoteConfigRequest(@Nullable String keysInclude, @Nullable String keysExclude, @NonNull String preparedMetrics) { + String data = prepareCommonRequestData() + + "&method=rc" + + "&device_id=" + UtilsNetworking.urlEncodeString(deviceIdProvider_.getDeviceId()); + + if (consentProvider.getConsent(Countly.CountlyFeatureNames.sessions)) { + //add session data if consent given + data += "&metrics=" + preparedMetrics; + } + + //add key filters + if (keysInclude != null) { + data += "&keys=" + UtilsNetworking.urlEncodeString(keysInclude); + } else if (keysExclude != null) { + data += "&omit_keys=" + UtilsNetworking.urlEncodeString(keysExclude); + } + + return data; + } + + public String prepareEnrollmentParameters(@NonNull String[] keys) { + String data = "method=ab" + + "&keys=" + UtilsNetworking.encodedArrayBuilder(keys) + + "&app_key=" + UtilsNetworking.urlEncodeString(baseInfoProvider.getAppKey()) + + "&device_id=" + UtilsNetworking.urlEncodeString(deviceIdProvider_.getDeviceId()); + return data; + } + + public String prepareRemovalParameters(@NonNull String[] keys) { + String data = "method=ab_opt_out" + + "&app_key=" + UtilsNetworking.urlEncodeString(baseInfoProvider.getAppKey()) + + "&device_id=" + UtilsNetworking.urlEncodeString(deviceIdProvider_.getDeviceId()); + + if (keys.length > 0) { + data += "&keys=" + UtilsNetworking.encodedArrayBuilder(keys); + } + + return data; + } + /** * To fetch all variants from the server. Something like this should be formed: method=ab_fetch_variants&app_key="APP_KEY"&device_id=DEVICE_ID * API end point for this is /i/sdk diff --git a/sdk/src/main/java/ly/count/android/sdk/Countly.java b/sdk/src/main/java/ly/count/android/sdk/Countly.java index f89358afb..aaf666f6c 100644 --- a/sdk/src/main/java/ly/count/android/sdk/Countly.java +++ b/sdk/src/main/java/ly/count/android/sdk/Countly.java @@ -44,7 +44,7 @@ of this software and associated documentation files (the "Software"), to deal */ public class Countly { - private final String DEFAULT_COUNTLY_SDK_VERSION_STRING = "22.09.4"; + private final String DEFAULT_COUNTLY_SDK_VERSION_STRING = "23.6.0"; /** * Used as request meta data on every request @@ -273,7 +273,11 @@ public synchronized Countly init(CountlyConfig config) { L.SetListener(config.providedLogCallback); - L.d("[Init] Initializing Countly [" + COUNTLY_SDK_NAME + "] SDK version [" + COUNTLY_SDK_VERSION_STRING + "]"); + if (COUNTLY_SDK_NAME.equals(DEFAULT_COUNTLY_SDK_NAME) && COUNTLY_SDK_VERSION_STRING.equals(DEFAULT_COUNTLY_SDK_VERSION_STRING)) { + L.d("[Init] Initializing Countly [" + COUNTLY_SDK_NAME + "] SDK version [" + COUNTLY_SDK_VERSION_STRING + "]"); + } else { + L.d("[Init] Initializing Countly [" + COUNTLY_SDK_NAME + "] SDK version [" + COUNTLY_SDK_VERSION_STRING + "] default name[" + DEFAULT_COUNTLY_SDK_NAME + "] default version[" + DEFAULT_COUNTLY_SDK_VERSION_STRING + "]"); + } if (config.context == null) { if (config.application != null) { diff --git a/sdk/src/main/java/ly/count/android/sdk/CountlyConfig.java b/sdk/src/main/java/ly/count/android/sdk/CountlyConfig.java index f8d7f1687..9a7314325 100644 --- a/sdk/src/main/java/ly/count/android/sdk/CountlyConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/CountlyConfig.java @@ -2,6 +2,8 @@ import android.app.Application; import android.content.Context; +import java.util.ArrayList; +import java.util.List; import java.util.Map; public class CountlyConfig { @@ -120,8 +122,12 @@ public class CountlyConfig { protected boolean pushIntentAddMetadata = false; - protected boolean enableRemoteConfigAutomaticDownload = false; - protected RemoteConfigCallback remoteConfigCallbackNew = null; + protected boolean enableRemoteConfigAutomaticDownloadTriggers = false; + + boolean enableRemoteConfigValueCaching = false; + protected RemoteConfigCallback remoteConfigCallbackLegacy = null; + + protected List remoteConfigGlobalCallbackList = new ArrayList<>(2); protected boolean shouldRequireConsent = false; protected String[] enabledFeatureNames = null; @@ -475,10 +481,26 @@ public synchronized CountlyConfig setPushIntentAddMetadata(boolean enable) { * @param enabled set true for enabling it * @param callback callback called after the update was done * @return Returns the same config object for convenient linking + * @deprecated */ public synchronized CountlyConfig setRemoteConfigAutomaticDownload(boolean enabled, RemoteConfigCallback callback) { - enableRemoteConfigAutomaticDownload = enabled; - remoteConfigCallbackNew = callback; + enableRemoteConfigAutomaticDownloadTriggers = enabled; + remoteConfigCallbackLegacy = callback; + return this; + } + + public synchronized CountlyConfig enableRemoteConfigAutomaticTriggers() { + enableRemoteConfigAutomaticDownloadTriggers = true; + return this; + } + + public synchronized CountlyConfig enableRemoteConfigValueCaching() { + enableRemoteConfigValueCaching = true; + return this; + } + + public synchronized CountlyConfig RemoteConfigRegisterGlobalCallback(RCDownloadCallback callback) { + remoteConfigGlobalCallbackList.add(callback); return this; } diff --git a/sdk/src/main/java/ly/count/android/sdk/MigrationHelper.java b/sdk/src/main/java/ly/count/android/sdk/MigrationHelper.java index 498b32686..4cf1d66c6 100644 --- a/sdk/src/main/java/ly/count/android/sdk/MigrationHelper.java +++ b/sdk/src/main/java/ly/count/android/sdk/MigrationHelper.java @@ -2,15 +2,21 @@ import androidx.annotation.NonNull; import java.util.Date; +import java.util.Iterator; import java.util.Map; import java.util.UUID; +import ly.count.android.sdk.internal.RemoteConfigValueStore; +import org.json.JSONException; +import org.json.JSONObject; class MigrationHelper { /** * 0 - legacy version. State of the SDK before the first migration was introduced - * 1 - adding device ID to all requests + * 1 - version where the device ID is guaranteed and advertising ID is deprecated/removed as a type + * 2 - transitioning old RC store to one that supports metadata + * x - adding device ID to all requests */ - final int DATA_SCHEMA_VERSIONS = 1; + static final int DATA_SCHEMA_VERSIONS = 2; static final public String key_from_0_to_1_custom_id_set = "0_1_custom_id_set"; @@ -76,6 +82,10 @@ void performMigrationStep(int currentVersion, @NonNull Map migra performMigration0To1(migrationParams); newVersion = newVersion + 1; break; + case 1: + performMigration1To2(migrationParams); + newVersion = newVersion + 1; + break; case DATA_SCHEMA_VERSIONS: L.w("[MigrationHelper] performMigrationStep, attempting to perform migration while already having the latest schema version, skipping [" + currentVersion + "]"); break; @@ -111,6 +121,8 @@ void setInitialSchemaVersion() { /** * Specific migration from schema version 0 to 1 + * This should make sure that a device ID exists and + * that the advertising_ID type is transformed to open_udid */ void performMigration0To1(@NonNull Map migrationParams) { String deviceIDType = storage.getDeviceIDType(); @@ -159,4 +171,43 @@ void performMigration0To1(@NonNull Map migrationParams) { } } } + + /** + * Tranforming the old RC structure date store to one that supports metadata + * + * @param migrationParams + */ + void performMigration1To2(@NonNull Map migrationParams) { + String currentRC = storage.getRemoteConfigValues(); + + JSONObject initialStructure; + + try { + initialStructure = new JSONObject(currentRC); + } catch (JSONException e) { + L.w("[MigrationHelper] performMigration1To2, failed at parsing old RC data. Clearing data structure and continuing. " + e); + storage.setRemoteConfigValues(""); + return; + } + + JSONObject newStructure = new JSONObject(); + Iterator iter = initialStructure.keys(); + while (iter.hasNext()) { + String key = iter.next(); + try { + Object value = initialStructure.opt(key); + if (value == null) { + continue; + } + JSONObject migratedValue = new JSONObject(); + migratedValue.put(RemoteConfigValueStore.keyValue, initialStructure.get(key)); + migratedValue.put(RemoteConfigValueStore.keyCacheFlag, RemoteConfigValueStore.cacheValFresh); + newStructure.put(key, migratedValue); + } catch (Exception e) { + Countly.sharedInstance().L.e("[MigrationHelper] performMigration1To2, transforming remote config values, " + e.toString()); + } + } + + storage.setRemoteConfigValues(newStructure.toString()); + } } diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleDeviceId.java b/sdk/src/main/java/ly/count/android/sdk/ModuleDeviceId.java index ec78ea4f6..9e24655b4 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleDeviceId.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleDeviceId.java @@ -80,16 +80,13 @@ void exitTemporaryIdMode(@NonNull String deviceId) { } //start by changing stored ID - deviceIdInstance.changeToCustomId(deviceId);//run init because not clear if types other then dev supplied can be provided + deviceIdInstance.changeToCustomId(deviceId); //update stored request for ID change to use this new ID replaceTempIDWithRealIDinRQ(deviceId); //update remote config_ values if automatic update is enabled - _cly.moduleRemoteConfig.clearValueStoreInternal(); - if (_cly.moduleRemoteConfig.remoteConfigAutomaticUpdateEnabled && consentProvider.anyConsentGiven()) { - _cly.moduleRemoteConfig.updateRemoteConfigValues(null, null, false, null); - } + _cly.moduleRemoteConfig.RCAutomaticDownloadTrigger(false); _cly.requestQueue().attemptToSendStoredRequests(); } @@ -119,6 +116,7 @@ void changeDeviceIdWithoutMergeInternal(@NonNull String deviceId) { // we just call our method for exiting it // we don't end the session, we just update the device ID and connection queue exitTemporaryIdMode(deviceId); + return; } // we are either making a simple ID change or entering temporary mode @@ -128,7 +126,7 @@ void changeDeviceIdWithoutMergeInternal(@NonNull String deviceId) { _cly.moduleRequestQueue.sendEventsIfNeeded(true); //update remote config_ values after id change if automatic update is enabled - _cly.moduleRemoteConfig.clearAndDownloadAfterIdChange(); + _cly.moduleRemoteConfig.clearAndDownloadAfterIdChange(true); _cly.moduleSessions.endSessionInternal(getDeviceId()); @@ -188,7 +186,7 @@ void changeDeviceIdWithMergeInternal(@NonNull String deviceId) { // in both cases we act the same as the temporary ID requests will be updated with the final ID later //update remote config_ values after id change if automatic update is enabled - _cly.moduleRemoteConfig.clearAndDownloadAfterIdChange(); + _cly.moduleRemoteConfig.clearAndDownloadAfterIdChange(false); requestQueueProvider.changeDeviceId(deviceId, _cly.moduleSessions.roundedSecondsSinceLastSessionDurationUpdate()); } diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleEvents.java b/sdk/src/main/java/ly/count/android/sdk/ModuleEvents.java index 052d70eb2..fa1fc5812 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleEvents.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleEvents.java @@ -131,7 +131,7 @@ public void recordEventInternal(@NonNull final String key, final Map variantContainer; // Stores the fetched A/B test variants + Map variantContainer = new HashMap<>(); // Stores the fetched A/B test variants RemoteConfig remoteConfigInterface = null; //if set to true, it will automatically download remote configs on module startup - boolean remoteConfigAutomaticUpdateEnabled = false; - RemoteConfigCallback remoteConfigInitCallback = null; + boolean automaticDownloadTriggersEnabled; + + boolean remoteConfigValuesShouldBeCached = false; + + List downloadCallbacks = new ArrayList<>(2); + + public final static String variantObjectNameKey = "name"; @Nullable Map metricOverride = null; @@ -28,16 +37,16 @@ public class ModuleRemoteConfig extends ModuleBase { L.v("[ModuleRemoteConfig] Initialising"); metricOverride = config.metricOverride; - immediateRequestGenerator = config.immediateRequestGenerator; + iRGenerator = config.immediateRequestGenerator; - if (config.enableRemoteConfigAutomaticDownload) { - L.d("[ModuleRemoteConfig] Setting if remote config Automatic download will be enabled, " + config.enableRemoteConfigAutomaticDownload); + L.d("[ModuleRemoteConfig] Setting if remote config Automatic triggers enabled, " + config.enableRemoteConfigAutomaticDownloadTriggers + ", caching enabled: " + config.enableRemoteConfigValueCaching); + automaticDownloadTriggersEnabled = config.enableRemoteConfigAutomaticDownloadTriggers; + remoteConfigValuesShouldBeCached = config.enableRemoteConfigValueCaching; - remoteConfigAutomaticUpdateEnabled = config.enableRemoteConfigAutomaticDownload; + downloadCallbacks.addAll(config.remoteConfigGlobalCallbackList); - if (config.remoteConfigCallbackNew != null) { - remoteConfigInitCallback = config.remoteConfigCallbackNew; - } + if (config.remoteConfigCallbackLegacy != null) { + downloadCallbacks.add((downloadResult, error, fullValueUpdate, downloadedValues) -> config.remoteConfigCallbackLegacy.callback(error)); } remoteConfigInterface = new RemoteConfig(); @@ -48,72 +57,142 @@ public class ModuleRemoteConfig extends ModuleBase { * * @param keysOnly set if these are the only keys to update * @param keysExcept set if these keys should be ignored from the update - * @param requestShouldBeDelayed this is set to true in case of update after a deviceId change - * @param callback called after the update is done + * @param devProvidedCallback dev provided callback that is called after the update is done */ - void updateRemoteConfigValues(@Nullable final String[] keysOnly, @Nullable final String[] keysExcept, final boolean requestShouldBeDelayed, @Nullable final RemoteConfigCallback callback) { - try { - L.d("[ModuleRemoteConfig] Updating remote config values, requestShouldBeDelayed:[" + requestShouldBeDelayed + "]"); + void updateRemoteConfigValues(@Nullable final String[] keysOnly, @Nullable final String[] keysExcept, final boolean useLegacyAPI, @Nullable final RCDownloadCallback devProvidedCallback) { + L.d("[ModuleRemoteConfig] Updating remote config values, legacyAPI:[" + useLegacyAPI + "]"); + String[] preparedKeys = RemoteConfigHelper.prepareKeysIncludeExclude(keysOnly, keysExcept, L); + boolean fullUpdate = (preparedKeys[0] == null || preparedKeys[0].length() == 0) && (preparedKeys[1] == null || preparedKeys[1].length() == 0); + + try { // checks if (deviceIdProvider.getDeviceId() == null) { //device ID is null, abort L.d("[ModuleRemoteConfig] RemoteConfig value update was aborted, deviceID is null"); - if (callback != null) { - callback.callback("Can't complete call, device ID is null"); - } + NotifyDownloadCallbacks(devProvidedCallback, RequestResult.Error, "Can't complete call, device ID is null", fullUpdate, null); return; } if (deviceIdProvider.isTemporaryIdEnabled() || requestQueueProvider.queueContainsTemporaryIdItems()) { //temporary id mode enabled, abort L.d("[ModuleRemoteConfig] RemoteConfig value update was aborted, temporary device ID mode is set"); - if (callback != null) { - callback.callback("Can't complete call, temporary device ID is set"); - } + NotifyDownloadCallbacks(devProvidedCallback, RequestResult.Error, "Can't complete call, temporary device ID is set", fullUpdate, null); return; } //prepare metrics and request data String preparedMetrics = deviceInfo.getMetrics(_cly.context_, deviceInfo, metricOverride); - String[] preparedKeys = prepareKeysIncludeExclude(keysOnly, keysExcept); - String requestData = requestQueueProvider.prepareRemoteConfigRequest(preparedKeys[0], preparedKeys[1], preparedMetrics); + String requestData; + + if (useLegacyAPI) { + requestData = requestQueueProvider.prepareRemoteConfigRequestLegacy(preparedKeys[0], preparedKeys[1], preparedMetrics); + } else { + requestData = requestQueueProvider.prepareRemoteConfigRequest(preparedKeys[0], preparedKeys[1], preparedMetrics); + } L.d("[ModuleRemoteConfig] RemoteConfig requestData:[" + requestData + "]"); ConnectionProcessor cp = requestQueueProvider.createConnectionProcessor(); final boolean networkingIsEnabled = cp.configProvider_.getNetworkingEnabled(); - (new ImmediateRequestMaker()).doWork(requestData, "/o/sdk", cp, requestShouldBeDelayed, networkingIsEnabled, new ImmediateRequestMaker.InternalImmediateRequestCallback() { - @Override - public void callback(JSONObject checkResponse) { - L.d("[ModuleRemoteConfig] Processing remote config received response, received response is null:[" + (checkResponse == null) + "]"); - if (checkResponse == null) { - if (callback != null) { - callback.callback("Encountered problem while trying to reach the server, possibly no internet connection"); - } - return; - } + iRGenerator.CreateImmediateRequestMaker().doWork(requestData, "/o/sdk", cp, false, networkingIsEnabled, checkResponse -> { + L.d("[ModuleRemoteConfig] Processing remote config received response, received response is null:[" + (checkResponse == null) + "]"); + if (checkResponse == null) { + NotifyDownloadCallbacks(devProvidedCallback, RequestResult.Error, "Encountered problem while trying to reach the server, possibly no internet connection", fullUpdate, null); + return; + } - String error = null; - try { - boolean clearOldValues = keysExcept == null && keysOnly == null; - mergeCheckResponseIntoCurrentValues(clearOldValues, checkResponse); - } catch (Exception ex) { - L.e("[ModuleRemoteConfig] updateRemoteConfigValues - execute, Encountered internal issue while trying to download remote config information from the server, [" + ex.toString() + "]"); - error = "Encountered internal issue while trying to download remote config information from the server, [" + ex.toString() + "]"; - } + String error = null; + Map newRC = RemoteConfigHelper.DownloadedValuesIntoMap(checkResponse); - if (callback != null) { - callback.callback(error); - } + try { + boolean clearOldValues = keysExcept == null && keysOnly == null; + mergeCheckResponseIntoCurrentValues(clearOldValues, newRC); + } catch (Exception ex) { + L.e("[ModuleRemoteConfig] updateRemoteConfigValues - execute, Encountered internal issue while trying to download remote config information from the server, [" + ex.toString() + "]"); + error = "Encountered internal issue while trying to download remote config information from the server, [" + ex.toString() + "]"; } + + NotifyDownloadCallbacks(devProvidedCallback, error == null ? RequestResult.Success : RequestResult.Error, error, fullUpdate, newRC); }, L); } catch (Exception ex) { L.e("[ModuleRemoteConfig] Encountered internal error while trying to perform a remote config update. " + ex.toString()); - if (callback != null) { - callback.callback("Encountered internal error while trying to perform a remote config update"); + NotifyDownloadCallbacks(devProvidedCallback, RequestResult.Error, "Encountered internal error while trying to perform a remote config update", fullUpdate, null); + } + } + + /** + * Internal function to form and send a request to enroll user for given keys + * + * @param keys + */ + void enrollIntoABTestsForKeysInternal(@NonNull String[] keys) { + L.d("[ModuleRemoteConfig] Enrolling user for the given keys:" + keys); + + if (deviceIdProvider.isTemporaryIdEnabled() || requestQueueProvider.queueContainsTemporaryIdItems() || deviceIdProvider.getDeviceId() == null) { + L.d("[ModuleRemoteConfig] Enrolling user was aborted, temporary device ID mode is set or device ID is null."); + return; + } + + String requestData = requestQueueProvider.prepareEnrollmentParameters(keys); + L.d("[ModuleRemoteConfig] Enrollment requestData:[" + requestData + "]"); + + ConnectionProcessor cp = requestQueueProvider.createConnectionProcessor(); + final boolean networkingIsEnabled = cp.configProvider_.getNetworkingEnabled(); + + iRGenerator.CreateImmediateRequestMaker().doWork(requestData, "/o/sdk", cp, false, networkingIsEnabled, checkResponse -> { + L.d("[ModuleRemoteConfig] Processing received response, received response is null:[" + (checkResponse == null) + "]"); + if (checkResponse == null) { + return; + } + + try { + if (checkResponse.has("result")) { + L.d("[ModuleRemoteConfig] Assuming that user was enrolled user for the A/B test"); + } else { + L.w("[ModuleRemoteConfig] Encountered a network error while enrolling the user for the A/B test."); + } + } catch (Exception ex) { + L.e("[ModuleRemoteConfig] Encountered an internal error while trying to enroll the user for A/B test. " + ex.toString()); } + }, L); + } + + /** + * Internal function to form and send the request to remove user from A/B testes for given keys + * + * @param keys + */ + void exitABTestsForKeysInternal(@NonNull String[] keys) { + L.d("[ModuleRemoteConfig] Removing user for the tests with given keys:" + keys); + + if (deviceIdProvider.isTemporaryIdEnabled() || requestQueueProvider.queueContainsTemporaryIdItems() || deviceIdProvider.getDeviceId() == null) { + L.d("[ModuleRemoteConfig] Removing user from tests was aborted, temporary device ID mode is set or device ID is null."); + return; } + + String requestData = requestQueueProvider.prepareRemovalParameters(keys); + L.d("[ModuleRemoteConfig] Removal requestData:[" + requestData + "]"); + + ConnectionProcessor cp = requestQueueProvider.createConnectionProcessor(); + final boolean networkingIsEnabled = cp.configProvider_.getNetworkingEnabled(); + + iRGenerator.CreateImmediateRequestMaker().doWork(requestData, "/i", cp, false, networkingIsEnabled, checkResponse -> { + L.d("[ModuleRemoteConfig] Processing received response, received response is null:[" + (checkResponse == null) + "]"); + if (checkResponse == null) { + return; + } + + try { + if (checkResponse.has("result") && checkResponse.getString("result").equals("Success")) { + L.d("[ModuleRemoteConfig] Removed user from the A/B test"); + } else { + L.d("[ModuleRemoteConfig] Encountered a network error while removing the user from A/B testing."); + } + } catch (Exception ex) { + L.e("[ModuleRemoteConfig] Encountered an internal error while trying to remove user from A/B testing. " + ex.toString()); + } + }, L); } /** @@ -127,7 +206,7 @@ void testingFetchVariantInformationInternal(@NonNull final RCVariantCallback cal if (deviceIdProvider.isTemporaryIdEnabled() || requestQueueProvider.queueContainsTemporaryIdItems() || deviceIdProvider.getDeviceId() == null) { L.d("[ModuleRemoteConfig] Fetching all A/B test variants was aborted, temporary device ID mode is set or device ID is null."); - callback.callback(RequestResponse.ERROR, "Temporary device ID mode is set or device ID is null."); + callback.callback(RequestResult.Error, "Temporary device ID mode is set or device ID is null."); return; } @@ -139,28 +218,20 @@ void testingFetchVariantInformationInternal(@NonNull final RCVariantCallback cal ConnectionProcessor cp = requestQueueProvider.createConnectionProcessor(); final boolean networkingIsEnabled = cp.configProvider_.getNetworkingEnabled(); - immediateRequestGenerator.CreateImmediateRequestMaker().doWork(requestData, "/i/sdk", cp, false, networkingIsEnabled, new ImmediateRequestMaker.InternalImmediateRequestCallback() { - @Override - public void callback(JSONObject checkResponse) { - L.d("[ModuleRemoteConfig] Processing Fetching all A/B test variants received response, received response is null:[" + (checkResponse == null) + "]"); - if (checkResponse == null) { - callback.callback(RequestResponse.NETWORK_ISSUE, "Received response is null." ); - return; - } + iRGenerator.CreateImmediateRequestMaker().doWork(requestData, "/o/sdk", cp, false, networkingIsEnabled, checkResponse -> { + L.d("[ModuleRemoteConfig] Processing Fetching all A/B test variants received response, received response is null:[" + (checkResponse == null) + "]"); + if (checkResponse == null) { + callback.callback(RequestResult.NetworkIssue, "Encountered problem while trying to reach the server, possibly no internet connection"); + return; + } - try { - Map parsedResponse = convertVariantsJsonToMap(checkResponse); - variantContainer = parsedResponse; - } catch (Exception ex) { - L.e("[ModuleRemoteConfig] testingFetchVariantInformationInternal - execute, Encountered internal issue while trying to fetch information from the server, [" + ex.toString() + "]"); - } + variantContainer = RemoteConfigHelper.convertVariantsJsonToMap(checkResponse, L); - callback.callback(RequestResponse.SUCCESS, null); - } + callback.callback(RequestResult.Success, null); }, L); } catch (Exception ex) { L.e("[ModuleRemoteConfig] Encountered internal error while trying to fetch all A/B test variants. " + ex.toString()); - callback.callback(RequestResponse.ERROR, "Encountered internal error while trying to fetch all A/B test variants."); + callback.callback(RequestResult.Error, "Encountered internal error while trying to fetch all A/B test variants."); } } @@ -170,14 +241,14 @@ void testingEnrollIntoVariantInternal(@NonNull final String key, @NonNull final if (deviceIdProvider.isTemporaryIdEnabled() || requestQueueProvider.queueContainsTemporaryIdItems() || deviceIdProvider.getDeviceId() == null) { L.d("[ModuleRemoteConfig] Enrolling A/B test variants was aborted, temporary device ID mode is set or device ID is null."); - callback.callback(RequestResponse.ERROR, "Temporary device ID mode is set or device ID is null."); + callback.callback(RequestResult.Error, "Temporary device ID mode is set or device ID is null."); return; } // check Key and Variant if (TextUtils.isEmpty(key) || TextUtils.isEmpty(variant)) { L.w("[ModuleRemoteConfig] Enrolling A/B test variants, Key/Variant pair is invalid. Aborting."); - callback.callback(RequestResponse.ERROR, "Provided key/variant pair is invalid."); + callback.callback(RequestResult.Error, "Provided key/variant pair is invalid."); return; } @@ -189,43 +260,30 @@ void testingEnrollIntoVariantInternal(@NonNull final String key, @NonNull final ConnectionProcessor cp = requestQueueProvider.createConnectionProcessor(); final boolean networkingIsEnabled = cp.configProvider_.getNetworkingEnabled(); - immediateRequestGenerator.CreateImmediateRequestMaker().doWork(requestData, "/i/sdk", cp, false, networkingIsEnabled, new ImmediateRequestMaker.InternalImmediateRequestCallback() { - @Override - public void callback(JSONObject checkResponse) { - L.d("[ModuleRemoteConfig] Processing Fetching all A/B test variants received response, received response is null:[" + (checkResponse == null) + "]"); - if (checkResponse == null) { - callback.callback(RequestResponse.NETWORK_ISSUE, "Received response is null."); + iRGenerator.CreateImmediateRequestMaker().doWork(requestData, "/i", cp, false, networkingIsEnabled, checkResponse -> { + L.d("[ModuleRemoteConfig] Processing Fetching all A/B test variants received response, received response is null:[" + (checkResponse == null) + "]"); + if (checkResponse == null) { + callback.callback(RequestResult.NetworkIssue, "Encountered problem while trying to reach the server, possibly no internet connection"); + return; + } + + try { + if (!isResponseValid(checkResponse)) { + callback.callback(RequestResult.NetworkIssue, "Bad response from the server:" + checkResponse.toString()); return; } - try { - if (!isResponseValid(checkResponse)) { - callback.callback(RequestResponse.NETWORK_ISSUE, "Bad response from the server:" + checkResponse.toString()); - return; - } - - // Update Remote Config - if (remoteConfigAutomaticUpdateEnabled) { - updateRemoteConfigValues(null, null, false, new RemoteConfigCallback() { - @Override public void callback(String error) { - if (error == null) { - L.d("[ModuleRemoteConfig] Updated remote config after enrolling to a variant"); - } else { - L.e("[ModuleRemoteConfig] Attempt to update the remote config after enrolling to a variant failed:" + error.toString()); - } - } - }); - } - - callback.callback(RequestResponse.SUCCESS, null); - } catch (Exception ex) { - L.e("[ModuleRemoteConfig] testingEnrollIntoVariantInternal - execute, Encountered internal issue while trying to enroll to the variant, [" + ex.toString() + "]"); - } + RCAutomaticDownloadTrigger(true);//todo afterwards cache only that one key + + callback.callback(RequestResult.Success, null); + } catch (Exception ex) { + L.e("[ModuleRemoteConfig] testingEnrollIntoVariantInternal - execute, Encountered internal issue while trying to enroll to the variant, [" + ex.toString() + "]"); + callback.callback(RequestResult.Error, "Encountered internal error while trying to take care of the A/B test variant enrolment."); } }, L); } catch (Exception ex) { L.e("[ModuleRemoteConfig] Encountered internal error while trying to enroll A/B test variants. " + ex.toString()); - callback.callback(RequestResponse.ERROR, "Encountered internal error while trying to enroll A/B test variants."); + callback.callback(RequestResult.Error, "Encountered internal error while trying to enroll A/B test variants."); } } @@ -235,16 +293,12 @@ public void callback(JSONObject checkResponse) { * * @throws Exception it throws an exception so that it is escalated upwards */ - void mergeCheckResponseIntoCurrentValues(boolean clearOldValues, JSONObject checkResponse) throws Exception { + void mergeCheckResponseIntoCurrentValues(boolean clearOldValues, @NonNull Map newRC) { //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 RemoteConfigValueStore rcvs = loadConfig(); - if (clearOldValues) { - //in case of full updates, clear old values - rcvs.values = new JSONObject(); - } - rcvs.mergeValues(checkResponse); + rcvs.mergeValues(newRC, clearOldValues); L.d("[ModuleRemoteConfig] Finished remote config processing, starting saving"); @@ -258,118 +312,43 @@ void mergeCheckResponseIntoCurrentValues(boolean clearOldValues, JSONObject chec * * @param responseJson - JSONObject response * @return - * @throws JSONException */ - boolean isResponseValid(@NonNull JSONObject responseJson) throws JSONException { + boolean isResponseValid(@NonNull JSONObject responseJson) { boolean result = false; - if (responseJson.get("result").equals("Success")) { - result = true; - } - - return result; - } - - /** - * Converts A/B testing variants fetched from the server (JSONObject) into a map - * - * @param variantsObj - JSON Object fetched from the server - * @return - * @throws JSONException - */ - static Map convertVariantsJsonToMap(@NonNull JSONObject variantsObj) throws JSONException { - // Initialize the map to store the results - Map resultMap = new HashMap<>(); - try { - // Get the keys of the JSON object using names() method - JSONArray keys = variantsObj.names(); - if (keys != null) { - for (int i = 0; i < keys.length(); i++) { - String key = keys.getString(i); - Object value = variantsObj.get(key); - - // Set the key and and an empty Array initially - String[] emptyArray = new String[0]; - resultMap.put(key, emptyArray); - - // Check if the value is a JSON array - if (value instanceof JSONArray) { - JSONArray jsonArray = (JSONArray) value; - - // Check if the JSON array contains objects - if (jsonArray.length() > 0 && jsonArray.get(0) instanceof JSONObject) { - // Extract the values from the JSON objects - String[] variants = new String[jsonArray.length()]; - int count = 0; - for (int j = 0; j < jsonArray.length(); j++) { - JSONObject variantObject = jsonArray.getJSONObject(j); - if (variantObject.has("name")) { - variants[count] = variantObject.getString("name"); - count++; - } - } - - // Map the key and its corresponding variants - if (count > 0) { - String[] filteredVariants = new String[count]; - System.arraycopy(variants, 0, filteredVariants, 0, count); - resultMap.put(key, filteredVariants); - } // else if the JSON object had no key 'name' we return String[0] - } // else if values of JSON array are not JSON object(all?) or no values at all we return String[0] - } // else if value is not JSON array we return String[0] - } + if (responseJson.get("result").equals("Success")) { + result = true; } - } catch (Exception ex) { - Countly.sharedInstance().L.e("[ModuleRemoteConfig] convertVariantsJsonToMap, failed parsing:[" + ex.toString() + "]"); - return new HashMap<>(); + } catch (JSONException e) { + L.e("[ModuleRemoteConfig] isResponseValid, encountered issue, " + e); + return false; } - return resultMap; + return result; } - /* - * Decide which keys to use - * Useful if both 'keysExcept' and 'keysOnly' set - * */ - @NonNull String[] prepareKeysIncludeExclude(@Nullable final String[] keysOnly, @Nullable final String[] keysExcept) { - String[] res = new String[2];//0 - include, 1 - exclude - + RCData getRCValue(@NonNull String key) { try { - if (keysOnly != null && keysOnly.length > 0) { - //include list takes precedence - //if there is at least one item, use it - JSONArray includeArray = new JSONArray(); - for (String key : keysOnly) { - includeArray.put(key); - } - res[0] = includeArray.toString(); - } else if (keysExcept != null && keysExcept.length > 0) { - //include list was not used, use the exclude list - JSONArray excludeArray = new JSONArray(); - for (String key : keysExcept) { - excludeArray.put(key); - } - res[1] = excludeArray.toString(); - } + RemoteConfigValueStore rcvs = loadConfig(); + return rcvs.getValue(key); } catch (Exception ex) { - L.e("[ModuleRemoteConfig] prepareKeysIncludeExclude, Failed at preparing keys, [" + ex.toString() + "]"); + L.e("[ModuleRemoteConfig] getValue, Call failed:[" + ex.toString() + "]"); + return new RCData(null, true); } - - return res; } - Object getValue(String key) { + Object getRCValueLegacy(@NonNull String key) { try { RemoteConfigValueStore rcvs = loadConfig(); - return rcvs.getValue(key); + return rcvs.getValueLegacy(key); } catch (Exception ex) { - L.e("[ModuleRemoteConfig] getValue, Call failed:[" + ex.toString() + "]"); + L.e("[ModuleRemoteConfig] getValueLegacy, Call failed:[" + ex.toString() + "]"); return null; } } - void saveConfig(RemoteConfigValueStore rcvs) throws Exception { + void saveConfig(@NonNull RemoteConfigValueStore rcvs) { storageProvider.setRemoteConfigValues(rcvs.dataToString()); } @@ -377,10 +356,10 @@ void saveConfig(RemoteConfigValueStore rcvs) throws Exception { * @return * @throws Exception For some reason this might be throwing an exception */ - RemoteConfigValueStore loadConfig() throws Exception { + @NonNull RemoteConfigValueStore loadConfig() { String rcvsString = storageProvider.getRemoteConfigValues(); //noinspection UnnecessaryLocalVariable - RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(rcvsString); + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(rcvsString, remoteConfigValuesShouldBeCached); return rcvs; } @@ -388,13 +367,23 @@ void clearValueStoreInternal() { storageProvider.setRemoteConfigValues(""); } - Map getAllRemoteConfigValuesInternal() { + @NonNull Map getAllRemoteConfigValuesInternalLegacy() { + 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() { try { RemoteConfigValueStore rcvs = loadConfig(); return rcvs.getAllValues(); } catch (Exception ex) { Countly.sharedInstance().L.e("[ModuleRemoteConfig] getAllRemoteConfigValuesInternal, Call failed:[" + ex.toString() + "]"); - return null; + return new HashMap<>(); } } @@ -403,7 +392,7 @@ Map getAllRemoteConfigValuesInternal() { * * @return */ - Map testingGetAllVariantsInternal() { + @NonNull Map testingGetAllVariantsInternal() { return variantContainer; } @@ -413,87 +402,67 @@ Map testingGetAllVariantsInternal() { * @param key * @return */ - String[] testingGetVariantsForKeyInternal(String key) { + @Nullable String[] testingGetVariantsForKeyInternal(@NonNull String key) { + String[] variantResponse = null; if (variantContainer.containsKey(key)) { - return variantContainer.get(key); + variantResponse = variantContainer.get(key); } - return new String[0]; + return variantResponse; } - static class RemoteConfigValueStore { - public JSONObject values = new JSONObject(); - - //add new values to the current storage - public void mergeValues(JSONObject newValues) { - if (newValues == null) { - return; - } - - Iterator iter = newValues.keys(); - while (iter.hasNext()) { - String key = iter.next(); - try { - Object value = newValues.get(key); - values.put(key, value); - } catch (Exception e) { - Countly.sharedInstance().L.e("[RemoteConfigValueStore] Failed merging new remote config values"); - } - } - } + void clearAndDownloadAfterIdChange(boolean valuesShouldBeCacheCleared) { + L.v("[RemoteConfig] Clearing remote config values and preparing to download after ID update, " + valuesShouldBeCacheCleared); - private RemoteConfigValueStore(JSONObject values) { - this.values = values; + if (valuesShouldBeCacheCleared) { + CacheOrClearRCValuesIfNeeded(); } - - public Object getValue(String key) { - return values.opt(key); + if (automaticDownloadTriggersEnabled && consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + updateRemoteConfigAfterIdChange = true; } + } - public Map getAllValues() { - Map ret = new HashMap<>(); - - Iterator keys = values.keys(); - - while (keys.hasNext()) { - String key = keys.next(); - - try { - ret.put(key, values.get(key)); - } catch (Exception ex) { - Countly.sharedInstance().L.e("[RemoteConfigValueStore] Got JSON exception while calling 'getAllValues': " + ex.toString()); - } - } + void CacheOrClearRCValuesIfNeeded() { + L.v("[RemoteConfig] CacheOrClearRCValuesIfNeeded, cacheclearing values"); + RemoteConfigValueStore rc = loadConfig(); + rc.cacheClearValues(); + saveConfig(rc); + } - return ret; + void NotifyDownloadCallbacks(RCDownloadCallback devProvidedCallback, RequestResult requestResult, String message, boolean fullUpdate, Map downloadedValues) { + for (RCDownloadCallback callback : downloadCallbacks) { + callback.callback(requestResult, message, fullUpdate, downloadedValues); } - public static RemoteConfigValueStore dataFromString(String storageString) { - if (storageString == null || storageString.isEmpty()) { - return new RemoteConfigValueStore(new JSONObject()); - } + if (devProvidedCallback != null) { + devProvidedCallback.callback(requestResult, message, fullUpdate, downloadedValues); + } + } - JSONObject values; - try { - values = new JSONObject(storageString); - } catch (JSONException e) { - Countly.sharedInstance().L.e("[RemoteConfigValueStore] Couldn't decode RemoteConfigValueStore successfully: " + e.toString()); - values = new JSONObject(); - } - return new RemoteConfigValueStore(values); + void RCAutomaticDownloadTrigger(boolean cacheClearOldValues) { + if (cacheClearOldValues) { + CacheOrClearRCValuesIfNeeded(); } - public String dataToString() { - return values.toString(); + if (automaticDownloadTriggersEnabled && consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + L.d("[RemoteConfig] Automatically updating remote config values"); + updateRemoteConfigValues(null, null, false, null); + } else { + L.v("[RemoteConfig] Automatic RC update trigger skipped"); } } - void clearAndDownloadAfterIdChange() { - L.v("[RemoteConfig] Clearing remote config values and preparing to download after ID update"); - - clearValueStoreInternal(); - if (remoteConfigAutomaticUpdateEnabled && consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - updateRemoteConfigAfterIdChange = true; + @Override + void onConsentChanged(@NonNull final List consentChangeDelta, final boolean newConsent, @NonNull final ModuleConsent.ConsentChangeSource changeSource) { + if (consentChangeDelta.contains(Countly.CountlyFeatureNames.remoteConfig) && changeSource == ChangeConsentCall) { + if (newConsent) { + //if consent was just given trigger automatic RC download if needed + RCAutomaticDownloadTrigger(false); + } else { + L.d("[RemoteConfig] removing remote-config consent. Clearing stored values"); + clearValueStoreInternal(); + // if consent is removed, we should clear remote config values + } } } @@ -503,16 +472,15 @@ void deviceIdChanged() { if (updateRemoteConfigAfterIdChange) { updateRemoteConfigAfterIdChange = false; - updateRemoteConfigValues(null, null, true, null); + RCAutomaticDownloadTrigger(true); } } @Override public void initFinished(@NonNull CountlyConfig config) { //update remote config_ values if automatic update is enabled and we are not in temporary id mode - if (remoteConfigAutomaticUpdateEnabled && consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig) && !deviceIdProvider.isTemporaryIdEnabled()) { - L.d("[RemoteConfig] Automatically updating remote config values"); - updateRemoteConfigValues(null, null, false, remoteConfigInitCallback); + if (!deviceIdProvider.isTemporaryIdEnabled()) { + RCAutomaticDownloadTrigger(false); } } @@ -530,6 +498,8 @@ public void halt() { public class RemoteConfig { /** * Clear all stored remote config_ values + * + * @deprecated Use "clearAll" */ public void clearStoredValues() { synchronized (_cly) { @@ -539,185 +509,349 @@ public void clearStoredValues() { } } + /** + * @return + * @deprecated + */ public Map getAllValues() { synchronized (_cly) { L.i("[RemoteConfig] Calling 'getAllValues'"); + return getAllRemoteConfigValuesInternalLegacy(); + } + } + + /** + * Get the stored value for the provided remote config_ key + * + * @param key + * @return + * @deprecated + */ + public Object getValueForKey(String key) { + synchronized (_cly) { + L.i("[RemoteConfig] Calling remoteConfigValueForKey, " + key); + + return getRCValueLegacy(key); + } + } + + /** + * Manual remote config update call. Will update all keys except the ones provided + * + * @param keysToExclude + * @param callback + * @deprecated + */ + public void updateExceptKeys(String[] keysToExclude, RemoteConfigCallback callback) { + synchronized (_cly) { + L.i("[RemoteConfig] Manually calling to updateRemoteConfig with exclude keys"); + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return null; + if (callback != null) { + callback.callback("No consent given"); + } + return; + } + if (keysToExclude == null) { + L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to ignore' array is null"); } - return getAllRemoteConfigValuesInternal(); + RCDownloadCallback innerCall = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + if (callback != null) { + callback.callback(error); + } + }; + + updateRemoteConfigValues(null, keysToExclude, true, innerCall); } } /** - * Returns all variant information as a Map + * Manual remote config_ update call. Will only update the keys provided. * - * @return + * @param keysToInclude + * @param callback + * @deprecated */ - public Map testingGetAllVariants() { + public void updateForKeysOnly(String[] keysToInclude, RemoteConfigCallback callback) { synchronized (_cly) { - L.i("[RemoteConfig] Calling 'testingGetAllVariants'"); - + L.i("[RemoteConfig] Manually calling to updateRemoteConfig with include keys"); if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return null; + if (callback != null) { + callback.callback("No consent given"); + } + return; + } + if (keysToInclude == null) { + L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to include' array is null"); } - return testingGetAllVariantsInternal(); + RCDownloadCallback innerCall = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + if (callback != null) { + callback.callback(error); + } + }; + + updateRemoteConfigValues(keysToInclude, null, true, innerCall); } } /** - * Returns variant information for a key as a String[] + * Manually update remote config_ values * - * @param key - key value to get variant information for - * @return + * @param callback + * @deprecated */ - public String[] testingGetVariantsForKey(String key) { + public void update(RemoteConfigCallback callback) { synchronized (_cly) { - L.i("[RemoteConfig] Calling 'testingGetVariantsForKey'"); + L.i("[RemoteConfig] Manually calling to updateRemoteConfig"); if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return null; + if (callback != null) { + callback.callback("No consent given"); + } + return; } - return testingGetVariantsForKeyInternal(key); + RCDownloadCallback innerCall = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + if (callback != null) { + callback.callback(error); + } + }; + + updateRemoteConfigValues(null, null, true, innerCall); } } /** - * Fetches all variants of A/B testing experiments + * Manual remote config update call. Will update all keys except the ones provided * + * @param keysToOmit * @param callback */ - public void testingFetchVariantInformation(RCVariantCallback callback) { + public void downloadOmittingKeys(@Nullable String[] keysToOmit, @Nullable RCDownloadCallback callback) { synchronized (_cly) { - L.i("[RemoteConfig] Calling 'testingFetchVariantInformation'"); + L.i("[RemoteConfig] Manually calling to updateRemoteConfig with exclude keys"); if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (callback != null) { + callback.callback(RequestResult.Error, null, false, null); + } return; } + if (keysToOmit == null) { + L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to ignore' array is null"); + } if (callback == null) { - callback = new RCVariantCallback() { - @Override public void callback(RequestResponse result, String error) { - } + callback = (downloadResult, error, fullValueUpdate, downloadedValues) -> { }; } - testingFetchVariantInformationInternal(callback); + updateRemoteConfigValues(null, keysToOmit, false, callback); } } /** - * Enrolls user for a specific variant of A/B testing experiment + * Manual remote config_ update call. Will only update the keys provided. * - * @param key - key value retrieved from the fetched variants - * @param variantName - name of the variant for the key to enroll + * @param keysToInclude * @param callback */ - public void testingEnrollIntoVariant(String key, String variantName, RCVariantCallback callback) { + public void downloadSpecificKeys(@Nullable String[] keysToInclude, @Nullable RCDownloadCallback callback) { synchronized (_cly) { - L.i("[RemoteConfig] Calling 'testingEnrollIntoVariant'"); - + L.i("[RemoteConfig] Manually calling to updateRemoteConfig with include keys"); if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (callback != null) { + callback.callback(RequestResult.Error, null, false, null); + } return; } + if (keysToInclude == null) { + L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to include' array is null"); + } - if (key == null || variantName == null) { - L.w("[RemoteConfig] testEnrollIntoVariant, passed key or variant is null. Aborting."); + if (callback == null) { + callback = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + }; + } + + updateRemoteConfigValues(keysToInclude, null, false, callback); + } + } + + public void downloadAllKeys(@Nullable RCDownloadCallback callback) { + synchronized (_cly) { + L.i("[RemoteConfig] Manually calling to update Remote Config v2"); + + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (callback != null) { + callback.callback(RequestResult.Error, null, true, null); + } return; } if (callback == null) { - callback = new RCVariantCallback() { - @Override public void callback(RequestResponse result, String error) { - } + callback = (downloadResult, error, fullValueUpdate, downloadedValues) -> { }; } - testingEnrollIntoVariantInternal(key, variantName, callback); + updateRemoteConfigValues(null, null, false, callback); + } + } + + public @NonNull Map getValues() { + synchronized (_cly) { + L.i("[RemoteConfig] Getting all Remote config values v2"); + + return getAllRemoteConfigValuesInternal(); + } + } + + public @NonNull RCData getValue(@Nullable String key) { + synchronized (_cly) { + L.i("[RemoteConfig] Getting Remote config values for key:[" + key + "] v2"); + + return getRCValue(key); } } /** - * Get the stored value for the provided remote config_ key + * Enrolls user to AB tests of the given keys. * - * @param key - * @return + * @param keys - String array of keys (parameters) */ - public Object getValueForKey(String key) { + public void enrollIntoABTestsForKeys(@Nullable String[] keys) { synchronized (_cly) { - L.i("[RemoteConfig] Calling remoteConfigValueForKey, " + key); + L.i("[RemoteConfig] Enrolling user into A/B tests."); + + if (keys == null || keys.length == 0) { + L.w("[RemoteConfig] A key should be provided to enroll the user."); + return; + } if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return null; + return; } - return getValue(key); + enrollIntoABTestsForKeysInternal(keys); } } /** - * Manual remote config update call. Will update all keys except the ones provided + * Removes user from A/B tests for the given keys. If no key provided would remove the user from all tests. * - * @param keysToExclude - * @param callback + * @param keys - String array of keys (parameters) */ - public void updateExceptKeys(String[] keysToExclude, RemoteConfigCallback callback) { + public void exitABTestsForKeys(@Nullable String[] keys) { synchronized (_cly) { - L.i("[RemoteConfig] Manually calling to updateRemoteConfig with exclude keys"); + L.i("[RemoteConfig] Removing user from A/B tests."); + + if (keys == null) { + keys = new String[0]; + } if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - if (callback != null) { - callback.callback("No consent given"); - } return; } - if (keysToExclude == null) { - L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to ignore' array is null"); + + exitABTestsForKeysInternal(keys); + } + } + + public void registerDownloadCallback(@Nullable RCDownloadCallback callback) { + downloadCallbacks.add(callback); + } + + public void removeDownloadCallback(@Nullable RCDownloadCallback callback) { + downloadCallbacks.remove(callback); + } + + public void clearAll() { + clearStoredValues(); + } + + /** + * Returns all variant information as a Map + * + * @return + */ + public @NonNull Map testingGetAllVariants() { + synchronized (_cly) { + L.i("[RemoteConfig] Calling 'testingGetAllVariants'"); + + return testingGetAllVariantsInternal(); + } + } + + /** + * Returns variant information for a key as a String[] + * + * @param key - key value to get variant information for + * @return + */ + public @Nullable String[] testingGetVariantsForKey(@Nullable String key) { + synchronized (_cly) { + L.i("[RemoteConfig] Calling 'testingGetVariantsForKey'"); + + if (key == null) { + L.i("[RemoteConfig] testingGetVariantsForKey, provided variant key can not be null"); + return null; } - updateRemoteConfigValues(null, keysToExclude, false, callback); + + return testingGetVariantsForKeyInternal(key); } } /** - * Manual remote config_ update call. Will only update the keys provided. + * Download all variants of A/B testing experiments * - * @param keysToInclude - * @param callback + * @param completionCallback */ - public void updateForKeysOnly(String[] keysToInclude, RemoteConfigCallback callback) { + public void testingDownloadVariantInformation(@Nullable RCVariantCallback completionCallback) { synchronized (_cly) { - L.i("[RemoteConfig] Manually calling to updateRemoteConfig with include keys"); + L.i("[RemoteConfig] Calling 'testingFetchVariantInformation'"); + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - if (callback != null) { - callback.callback("No consent given"); - } return; } - if (keysToInclude == null) { - L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to include' array is null"); + + if (completionCallback == null) { + completionCallback = (result, error) -> { + }; } - updateRemoteConfigValues(keysToInclude, null, false, callback); + + testingFetchVariantInformationInternal(completionCallback); } } /** - * Manually update remote config_ values + * Enrolls user for a specific variant of A/B testing experiment * - * @param callback + * @param keyName - key value retrieved from the fetched variants + * @param variantName - name of the variant for the key to enroll + * @param completionCallback */ - public void update(RemoteConfigCallback callback) { + public void testingEnrollIntoVariant(@Nullable String keyName, String variantName, @Nullable RCVariantCallback completionCallback) { synchronized (_cly) { - L.i("[RemoteConfig] Manually calling to updateRemoteConfig"); + L.i("[RemoteConfig] Calling 'testingEnrollIntoVariant'"); if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { return; } - updateRemoteConfigValues(null, null, false, callback); + if (keyName == null || variantName == null) { + L.w("[RemoteConfig] testEnrollIntoVariant, passed key or variant is null. Aborting."); + return; + } + + if (completionCallback == null) { + completionCallback = (result, error) -> { + }; + } + + testingEnrollIntoVariantInternal(keyName, variantName, completionCallback); } } } diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleUserProfile.java b/sdk/src/main/java/ly/count/android/sdk/ModuleUserProfile.java index 2011016b0..74936c954 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleUserProfile.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleUserProfile.java @@ -568,7 +568,7 @@ public void save() { } /** - * Clear all submitted information + * Clear queued operations / modifications */ public void clear() { synchronized (_cly) { diff --git a/sdk/src/main/java/ly/count/android/sdk/RCData.java b/sdk/src/main/java/ly/count/android/sdk/RCData.java new file mode 100644 index 000000000..f8a1a6fdf --- /dev/null +++ b/sdk/src/main/java/ly/count/android/sdk/RCData.java @@ -0,0 +1,11 @@ +package ly.count.android.sdk; + +public class RCData { + public Object value; + public boolean isCurrentUsersData; + + public RCData(Object givenValue, boolean givenUserState) { + this.value = givenValue; + this.isCurrentUsersData = givenUserState; + } +} diff --git a/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java b/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java new file mode 100644 index 000000000..271570d60 --- /dev/null +++ b/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java @@ -0,0 +1,8 @@ +package ly.count.android.sdk; + +import java.util.Map; +import org.json.JSONObject; + +public interface RCDownloadCallback { + void callback(RequestResult downloadResult, String error, boolean fullValueUpdate, Map downloadedValues); +} diff --git a/sdk/src/main/java/ly/count/android/sdk/RCVariantCallback.java b/sdk/src/main/java/ly/count/android/sdk/RCVariantCallback.java index 4f666dbf4..e158feeae 100644 --- a/sdk/src/main/java/ly/count/android/sdk/RCVariantCallback.java +++ b/sdk/src/main/java/ly/count/android/sdk/RCVariantCallback.java @@ -5,8 +5,8 @@ public interface RCVariantCallback { /** * Called after fetching A/B test variants * - * @param result provides an enum for the result of fetch request + * @param rResult provides an enum for the result of the download request * @param error provides an error string if it exists */ - void callback(RequestResponse result, String error); + void callback(RequestResult rResult, String error); } diff --git a/sdk/src/main/java/ly/count/android/sdk/RequestQueueProvider.java b/sdk/src/main/java/ly/count/android/sdk/RequestQueueProvider.java index 97745689b..124e60b92 100644 --- a/sdk/src/main/java/ly/count/android/sdk/RequestQueueProvider.java +++ b/sdk/src/main/java/ly/count/android/sdk/RequestQueueProvider.java @@ -50,8 +50,14 @@ interface RequestQueueProvider { ConnectionProcessor createConnectionProcessor(); + String prepareRemoteConfigRequestLegacy(@Nullable String keysInclude, @Nullable String keysExclude, @NonNull String preparedMetrics); + String prepareRemoteConfigRequest(@Nullable String keysInclude, @Nullable String keysExclude, @NonNull String preparedMetrics); + String prepareEnrollmentParameters(@NonNull String[] keys); + + String prepareRemovalParameters(@NonNull String[] keys); + String prepareFetchAllVariants(); // for fetching all A/B test variants String prepareEnrollVariant(String key, String Variant); // for enrolling to an A/B test variant diff --git a/sdk/src/main/java/ly/count/android/sdk/RequestResponse.java b/sdk/src/main/java/ly/count/android/sdk/RequestResponse.java deleted file mode 100644 index 40853396a..000000000 --- a/sdk/src/main/java/ly/count/android/sdk/RequestResponse.java +++ /dev/null @@ -1,12 +0,0 @@ -package ly.count.android.sdk; - -/** - * Enum used throughout Countly for relaying result of A/B test variants fetch - * TODO: Make it camel case? - */ -public enum RequestResponse { - NETWORK_ISSUE, - SUCCESS, - ERROR, -} - diff --git a/sdk/src/main/java/ly/count/android/sdk/RequestResult.java b/sdk/src/main/java/ly/count/android/sdk/RequestResult.java new file mode 100644 index 000000000..09db79d6c --- /dev/null +++ b/sdk/src/main/java/ly/count/android/sdk/RequestResult.java @@ -0,0 +1,5 @@ +package ly.count.android.sdk; + +public enum RequestResult { + Success, NetworkIssue, Error +} diff --git a/sdk/src/main/java/ly/count/android/sdk/UtilsNetworking.java b/sdk/src/main/java/ly/count/android/sdk/UtilsNetworking.java index 7e24f55bc..bce7f6d15 100644 --- a/sdk/src/main/java/ly/count/android/sdk/UtilsNetworking.java +++ b/sdk/src/main/java/ly/count/android/sdk/UtilsNetworking.java @@ -21,6 +21,23 @@ protected static String urlEncodeString(String givenValue) { return result; } + protected static String encodedArrayBuilder(String[] args) { + StringBuilder encodedUrlBuilder = new StringBuilder(); + + encodedUrlBuilder.append("["); + + for (int i = 0; i < args.length; i++) { + encodedUrlBuilder.append('"').append(args[i]).append('"'); + if (i < args.length - 1) { + encodedUrlBuilder.append(", "); + } + } + + encodedUrlBuilder.append("]"); + + return encodedUrlBuilder.toString(); + } + protected static String urlDecodeString(String givenValue) { String decodedResult = ""; diff --git a/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigHelper.java b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigHelper.java new file mode 100644 index 000000000..c71e14d5d --- /dev/null +++ b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigHelper.java @@ -0,0 +1,120 @@ +package ly.count.android.sdk.internal; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import ly.count.android.sdk.Countly; +import ly.count.android.sdk.ModuleLog; +import ly.count.android.sdk.ModuleRemoteConfig; +import ly.count.android.sdk.RCData; +import org.json.JSONArray; +import org.json.JSONObject; + +public class RemoteConfigHelper { + + public static @NonNull Map DownloadedValuesIntoMap(@Nullable JSONObject jsonObject) { + Map ret = new HashMap<>(); + + if (jsonObject == null) { + return ret; + } + + Iterator iter = jsonObject.keys(); + while (iter.hasNext()) { + String key = iter.next(); + try { + Object value = jsonObject.get(key); + ret.put(key, new RCData(value, true)); + } catch (Exception e) { + Countly.sharedInstance().L.e("[RemoteConfigValueStore] Failed merging new remote config values"); + } + } + + return ret; + } + + /* + * Decide which keys to use + * Useful if both 'keysExcept' and 'keysOnly' set + * */ + public static @NonNull String[] prepareKeysIncludeExclude(@Nullable final String[] keysOnly, @Nullable final String[] keysExcept, @NonNull ModuleLog L) { + String[] res = new String[2];//0 - include, 1 - exclude + + try { + if (keysOnly != null && keysOnly.length > 0) { + //include list takes precedence + //if there is at least one item, use it + JSONArray includeArray = new JSONArray(); + for (String key : keysOnly) { + includeArray.put(key); + } + res[0] = includeArray.toString(); + } else if (keysExcept != null && keysExcept.length > 0) { + //include list was not used, use the exclude list + JSONArray excludeArray = new JSONArray(); + for (String key : keysExcept) { + excludeArray.put(key); + } + res[1] = excludeArray.toString(); + } + } catch (Exception ex) { + L.e("[ModuleRemoteConfig] prepareKeysIncludeExclude, Failed at preparing keys, [" + ex.toString() + "]"); + } + + return res; + } + + /** + * Converts A/B testing variants fetched from the server (JSONObject) into a map + * + * @param variantsObj - JSON Object fetched from the server + * @return + */ + public static @NonNull Map convertVariantsJsonToMap(@NonNull JSONObject variantsObj, @NonNull ModuleLog L) { + Map resultMap = new HashMap<>(); + JSONArray keys = variantsObj.names(); + if (keys == null) { + return resultMap; + } + + List tempVariantColl = new ArrayList<>(5); + + try { + for (int i = 0; i < keys.length(); i++) { + String key = keys.getString(i); + Object value = variantsObj.get(key); + + if (!(value instanceof JSONArray)) { + //we only care about json arrays, all other values are skipped + continue; + } + + tempVariantColl.clear(); + JSONArray jsonArray = (JSONArray) value; + + for (int j = 0; j < jsonArray.length(); j++) { + JSONObject variantObject = jsonArray.optJSONObject(j); + + //skip for null values + if (variantObject == null || variantObject.isNull(ModuleRemoteConfig.variantObjectNameKey)) { + continue; + } + + tempVariantColl.add(variantObject.optString(ModuleRemoteConfig.variantObjectNameKey)); + } + + //write the filtered array to map + resultMap.put(key, tempVariantColl.toArray(new String[0])); + } + } catch (Exception ex) { + L.e("[ModuleRemoteConfig] convertVariantsJsonToMap, failed parsing:[" + ex.toString() + "]"); + return new HashMap<>(); + } + + return resultMap; + } +} diff --git a/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java new file mode 100644 index 000000000..0adb83a9a --- /dev/null +++ b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java @@ -0,0 +1,200 @@ +package ly.count.android.sdk.internal; + +import androidx.annotation.NonNull; +import androidx.annotation.Nullable; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import ly.count.android.sdk.Countly; +import ly.count.android.sdk.RCData; +import org.json.JSONException; +import org.json.JSONObject; + +public class RemoteConfigValueStore { + public JSONObject values; + public boolean valuesCanBeCached; + public static final String keyValue = "v"; + public static final String keyCacheFlag = "c"; + public static final int cacheValCached = 0; + public static final int cacheValFresh = 1; + + // Structure of the JSON objects we will have + // { + // β€œkey”: { + // β€œv”: β€œvalue”, + // β€œc”: 0 + // } + // } + + //======================================== + // CLEANSING + //======================================== + + public void cacheClearValues() { + if (!valuesCanBeCached) { + clearValues(); + return; + } + + Iterator iter = values.keys(); + while (iter.hasNext()) { + String key = iter.next(); + JSONObject value = values.optJSONObject(key); + + if (value == null) { + Object badVal = values.opt(key); + Countly.sharedInstance().L.w("[RemoteConfigValueStore] cacheClearValues, stored entry was not a JSON object, key:[" + key + "] value:[" + badVal + "]"); + continue; + } + + try { + value.put(keyCacheFlag, cacheValCached); + values.put(key, value); + } catch (Exception e) { + Countly.sharedInstance().L.e("[RemoteConfigValueStore] cacheClearValues, Failed caching remote config values, " + e); + } + } + } + + public void clearValues() { + values = new JSONObject(); + } + + //======================================== + // MERGING + //======================================== + + public void mergeValues(@NonNull Map newValues, boolean fullUpdate) { + //Countly.sharedInstance().L.i("[RemoteConfigValueStore] mergeValues, stored values:" + values.toString() + "provided values:" + newValues); + Countly.sharedInstance().L.v("[RemoteConfigValueStore] mergeValues, stored values C:" + values.length() + "provided values C:" + newValues.size()); + + if (fullUpdate) { + clearValues(); + } + + for (Map.Entry entry : newValues.entrySet()) { + String key = entry.getKey(); + Object newValue = entry.getValue().value; + JSONObject newObj = new JSONObject(); + try { + newObj.put(keyValue, newValue); + newObj.put(keyCacheFlag, cacheValFresh); + values.put(key, newObj); + } catch (Exception e) { + Countly.sharedInstance().L.e("[RemoteConfigValueStore] Failed merging remote config values"); + } + } + Countly.sharedInstance().L.v("[RemoteConfigValueStore] merging done:" + values.toString()); + } + + //======================================== + // CONSTRUCTION + //======================================== + + private RemoteConfigValueStore(@NonNull JSONObject values, boolean valuesShouldBeCached) { + this.values = values; + this.valuesCanBeCached = valuesShouldBeCached; + } + + //======================================== + // GET VALUES + //======================================== + + public @NonNull RCData getValue(@NonNull String key) { + RCData res = new RCData(null, true); + try { + JSONObject rcObj = values.optJSONObject(key); + if (rcObj == null) { + return res; + } + res.value = rcObj.get(keyValue); + res.isCurrentUsersData = rcObj.getInt(keyCacheFlag) != cacheValCached; + return res; + } catch (Exception ex) { + Countly.sharedInstance().L.e("[RemoteConfigValueStore] Got JSON exception while calling 'getValue': " + ex.toString()); + } + return res; + } + + public @NonNull Map getAllValues() { + Map ret = new HashMap<>(); + + Iterator keys = values.keys(); + while (keys.hasNext()) { + String key = keys.next(); + try { + JSONObject rcObj = values.optJSONObject(key); + if (rcObj == null) { + continue; + } + Object rcObjVal = rcObj.opt(keyValue); + int rcObjCache = rcObj.getInt(keyCacheFlag); + ret.put(key, new RCData(rcObjVal, (rcObjCache != cacheValCached))); + } catch (Exception ex) { + Countly.sharedInstance().L.e("[RemoteConfigValueStore] Got JSON exception while calling 'getAllValues': " + ex.toString()); + } + } + + return ret; + } + + public Object getValueLegacy(@NonNull String key) { + JSONObject rcObj = values.optJSONObject(key); + if (rcObj == null) { + return null; + } + + return rcObj.opt(keyValue); + } + + public @NonNull Map getAllValuesLegacy() { + Map ret = new HashMap<>(); + + Iterator keys = values.keys(); + + while (keys.hasNext()) { + String key = keys.next(); + + JSONObject jobj = values.optJSONObject(key); + if (jobj == null) { + Countly.sharedInstance().L.e("[RemoteConfigValueStore] getAllValuesLegacy, inner object seems to be 'null', key:[" + key + "]"); + continue; + } + + Object innerValue = jobj.opt(keyValue); + + if (innerValue == null) { + Countly.sharedInstance().L.e("[RemoteConfigValueStore] getAllValuesLegacy, inner value seems to be 'null', key:[" + key + "]"); + continue; + } + + ret.put(key, innerValue); + } + + return ret; + } + + //======================================== + // SERIALIZATION, DESERIALIZATION + //======================================== + + public static RemoteConfigValueStore dataFromString(@Nullable String storageString, boolean valuesShouldBeCached) { + if (storageString == null || storageString.isEmpty()) { + return new RemoteConfigValueStore(new JSONObject(), valuesShouldBeCached); + } + + JSONObject values; + try { + values = new JSONObject(storageString); + } catch (JSONException e) { + Countly.sharedInstance().L.e("[RemoteConfigValueStore] Couldn't decode RemoteConfigValueStore successfully: " + e.toString()); + values = new JSONObject(); + } + //Countly.sharedInstance().L.i("[RemoteConfigValueStore] serialization done, dataFromString:" + values.toString()); + return new RemoteConfigValueStore(values, valuesShouldBeCached); + } + + public String dataToString() { + return values.toString(); + } +}