From 1ad1add50d4a64bcb45e6898ec092fb9cac5dd77 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Mon, 22 May 2023 15:53:30 +0300 Subject: [PATCH 01/30] a --- .../sdk/ScenarioRemoteConfigTests.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 sdk/src/androidTest/java/ly/count/android/sdk/ScenarioRemoteConfigTests.java 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() { + + } +} From 900ceb997105c78bb2b003e786171c35dcf16907 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Tue, 23 May 2023 11:30:33 +0300 Subject: [PATCH 02/30] Initial additions for the new API --- .../ly/count/android/sdk/CountlyConfig.java | 9 +++ .../count/android/sdk/ModuleRemoteConfig.java | 68 +++++++++++++++++++ .../count/android/sdk/RCDownloadResult.java | 5 ++ .../java/ly/count/android/sdk/RCValue.java | 7 ++ .../ly/count/android/sdk/RCValueState.java | 5 ++ .../sdk/RemoteConfigDownloadCallback.java | 7 ++ 6 files changed, 101 insertions(+) create mode 100644 sdk/src/main/java/ly/count/android/sdk/RCDownloadResult.java create mode 100644 sdk/src/main/java/ly/count/android/sdk/RCValue.java create mode 100644 sdk/src/main/java/ly/count/android/sdk/RCValueState.java create mode 100644 sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java 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 2de73878f..dfcd4b780 100644 --- a/sdk/src/main/java/ly/count/android/sdk/CountlyConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/CountlyConfig.java @@ -507,6 +507,7 @@ public synchronized CountlyConfig setRemoteConfigAutomaticDownload(boolean enabl * @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; @@ -514,6 +515,14 @@ public synchronized CountlyConfig setRemoteConfigAutomaticDownload(boolean enabl return this; } + public synchronized CountlyConfig enableRemoteConfigAutomaticTriggers() { + return this; + } + + public synchronized CountlyConfig enableRemoteConfigValueCaching() { + return this; + } + /** * Set if consent should be required * diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 1e43a7377..28599ee3d 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -361,6 +361,7 @@ public Object getValueForKey(String key) { * * @param keysToExclude * @param callback + * @deprecated */ public void updateExceptKeys(String[] keysToExclude, RemoteConfigCallback callback) { synchronized (_cly) { @@ -384,6 +385,7 @@ public void updateExceptKeys(String[] keysToExclude, RemoteConfigCallback callba * * @param keysToInclude * @param callback + * @deprecated */ public void updateForKeysOnly(String[] keysToInclude, RemoteConfigCallback callback) { synchronized (_cly) { @@ -405,12 +407,78 @@ public void updateForKeysOnly(String[] keysToInclude, RemoteConfigCallback callb * Manually update remote config_ values * * @param callback + * @deprecated */ public void update(RemoteConfigCallback callback) { synchronized (_cly) { L.i("[RemoteConfig] Manually calling to updateRemoteConfig"); if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (callback != null) { + callback.callback("No consent given"); + } + return; + } + + updateRemoteConfigValues(null, null, false, callback); + } + } + + /** + * Manual remote config update call. Will update all keys except the ones provided + * + * @param keysToExclude + * @param callback + * @deprecated + */ + public void updateExceptKeys(String[] keysToExclude, RemoteConfigDownloadCallback callback) { + synchronized (_cly) { + L.i("[RemoteConfig] Manually calling to updateRemoteConfig with exclude keys"); + + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (callback != null) { + callback.callback(RCDownloadResult.NoConsent, null); + } + return; + } + if (keysToExclude == null) { + L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to ignore' array is null"); + } + updateRemoteConfigValues(null, keysToExclude, false, callback); + } + } + + /** + * Manual remote config_ update call. Will only update the keys provided. + * + * @param keysToInclude + * @param callback + * @deprecated + */ + public void updateForKeysOnly(String[] keysToInclude, RemoteConfigDownloadCallback callback) { + synchronized (_cly) { + L.i("[RemoteConfig] Manually calling to updateRemoteConfig with include keys"); + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (callback != null) { + callback.callback(RCDownloadResult.NoConsent, null); + } + return; + } + if (keysToInclude == null) { + L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to include' array is null"); + } + updateRemoteConfigValues(keysToInclude, null, false, callback); + } + } + + public void update(RemoteConfigDownloadCallback callback) { + synchronized (_cly) { + L.i("[RemoteConfig] Manually calling to update Remote Config v2"); + + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (callback != null) { + callback.callback(RCDownloadResult.NoConsent, null); + } return; } diff --git a/sdk/src/main/java/ly/count/android/sdk/RCDownloadResult.java b/sdk/src/main/java/ly/count/android/sdk/RCDownloadResult.java new file mode 100644 index 000000000..5803fc1ed --- /dev/null +++ b/sdk/src/main/java/ly/count/android/sdk/RCDownloadResult.java @@ -0,0 +1,5 @@ +package ly.count.android.sdk; + +enum RCDownloadResult { + Success, NetworkingError, TimedOut, NoConsent, OtherFailure +} diff --git a/sdk/src/main/java/ly/count/android/sdk/RCValue.java b/sdk/src/main/java/ly/count/android/sdk/RCValue.java new file mode 100644 index 000000000..5cc10f2a9 --- /dev/null +++ b/sdk/src/main/java/ly/count/android/sdk/RCValue.java @@ -0,0 +1,7 @@ +package ly.count.android.sdk; + +class RCValue { + public Object value; + Long timestamp; + RCValueState valueState; +} diff --git a/sdk/src/main/java/ly/count/android/sdk/RCValueState.java b/sdk/src/main/java/ly/count/android/sdk/RCValueState.java new file mode 100644 index 000000000..07cd6080b --- /dev/null +++ b/sdk/src/main/java/ly/count/android/sdk/RCValueState.java @@ -0,0 +1,5 @@ +package ly.count.android.sdk; + +enum RCValueState { + Cached, NoValue, CurrentUser +} diff --git a/sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java b/sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java new file mode 100644 index 000000000..11b6714a9 --- /dev/null +++ b/sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java @@ -0,0 +1,7 @@ +package ly.count.android.sdk; + +import org.json.JSONObject; + +interface RemoteConfigDownloadCallback { + void callback(RCDownloadResult downloadResult, JSONObject downloadedValues); +} From b14d7dfeb5bbd915587b29f837afc5a1f0f6699f Mon Sep 17 00:00:00 2001 From: turtledreams Date: Wed, 31 May 2023 18:13:18 +0900 Subject: [PATCH 03/30] changes --- .../ly/count/android/sdk/ConnectionQueue.java | 20 +++ .../count/android/sdk/ModuleRemoteConfig.java | 139 +++++++++++++++++- .../count/android/sdk/RCDownloadResult.java | 5 - .../sdk/RemoteConfigDownloadCallback.java | 2 +- .../android/sdk/RequestQueueProvider.java | 4 + .../ly/count/android/sdk/RequestResult.java | 5 + .../ly/count/android/sdk/UtilsNetworking.java | 23 +++ 7 files changed, 184 insertions(+), 14 deletions(-) delete mode 100644 sdk/src/main/java/ly/count/android/sdk/RCDownloadResult.java create mode 100644 sdk/src/main/java/ly/count/android/sdk/RequestResult.java 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 29cac1513..8a5d98955 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java +++ b/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java @@ -711,6 +711,26 @@ public String prepareRemoteConfigRequest(@Nullable String keysInclude, @Nullable 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); // TODO: key? keys? /this is not settled yet, redo after its settled + } + + return data; + } + public String prepareRatingWidgetRequest(String widgetId) { String data = prepareCommonRequestData() + "&widget_id=" + UtilsNetworking.urlEncodeString(widgetId) diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 28599ee3d..563588d54 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -124,6 +124,86 @@ public void callback(JSONObject checkResponse) { } } + /** + * 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(); + + (new ImmediateRequestMaker()).doWork(requestData, "/o/sdk", cp, false, networkingIsEnabled, new ImmediateRequestMaker.InternalImmediateRequestCallback() { + @Override + public void callback(JSONObject 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] 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(); + + (new ImmediateRequestMaker()).doWork(requestData, "/o/sdk", cp, false, networkingIsEnabled, new ImmediateRequestMaker.InternalImmediateRequestCallback() { + @Override + public void callback(JSONObject 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.w("[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); + } + /** * Merge the values acquired from the server into the current values. * Clear if needed. @@ -437,16 +517,16 @@ public void updateExceptKeys(String[] keysToExclude, RemoteConfigDownloadCallbac if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { if (callback != null) { - callback.callback(RCDownloadResult.NoConsent, null); + callback.callback(RequestResult.Error, null); } return; } if (keysToExclude == null) { L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to ignore' array is null"); } - updateRemoteConfigValues(null, keysToExclude, false, callback); + updateRemoteConfigValues(null, keysToExclude, false, null); // TODO: this callback was not expected } - } + } // TODO: this is a duplicate function /** * Manual remote config_ update call. Will only update the keys provided. @@ -460,16 +540,16 @@ public void updateForKeysOnly(String[] keysToInclude, RemoteConfigDownloadCallba L.i("[RemoteConfig] Manually calling to updateRemoteConfig with include keys"); if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { if (callback != null) { - callback.callback(RCDownloadResult.NoConsent, null); + callback.callback(RequestResult.Error, null); } return; } if (keysToInclude == null) { L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to include' array is null"); } - updateRemoteConfigValues(keysToInclude, null, false, callback); + updateRemoteConfigValues(keysToInclude, null, false, null); // TODO: this callback was not expected } - } + } // TODO: this is a duplicate function public void update(RemoteConfigDownloadCallback callback) { synchronized (_cly) { @@ -477,12 +557,55 @@ public void update(RemoteConfigDownloadCallback callback) { if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { if (callback != null) { - callback.callback(RCDownloadResult.NoConsent, null); + callback.callback(RequestResult.Error, null); } return; } - updateRemoteConfigValues(null, null, false, callback); + updateRemoteConfigValues(null, null, false, null); // TODO: this callback was not expected + } + } // TODO: this is a duplicate function + + /** + * Enrolls user to AB tests of the given keys. + * + * @param keys - String array of keys (parameters) + */ + public void enrollIntoABTestsForKeys(String[] keys) { + synchronized (_cly) { + 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; + } + + enrollIntoABTestsForKeysInternal(keys); + } + } + + /** + * Removes user from A/B tests for the given keys. If no key provided would remove the user from all tests. + * + * @param keys - String array of keys (parameters) + */ + public void exitABTestsForKeys(String[] keys) { + synchronized (_cly) { + L.i("[RemoteConfig] Removing user from A/B tests."); + + if (keys == null) { + keys = new String[0]; + } + + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + return; + } + + exitABTestsForKeysInternal(keys); } } } diff --git a/sdk/src/main/java/ly/count/android/sdk/RCDownloadResult.java b/sdk/src/main/java/ly/count/android/sdk/RCDownloadResult.java deleted file mode 100644 index 5803fc1ed..000000000 --- a/sdk/src/main/java/ly/count/android/sdk/RCDownloadResult.java +++ /dev/null @@ -1,5 +0,0 @@ -package ly.count.android.sdk; - -enum RCDownloadResult { - Success, NetworkingError, TimedOut, NoConsent, OtherFailure -} diff --git a/sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java b/sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java index 11b6714a9..3bd1d183a 100644 --- a/sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java +++ b/sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java @@ -3,5 +3,5 @@ import org.json.JSONObject; interface RemoteConfigDownloadCallback { - void callback(RCDownloadResult downloadResult, JSONObject downloadedValues); + void callback(RequestResult downloadResult, JSONObject downloadedValues); } 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 29eb4c14f..6ddb28bf2 100644 --- a/sdk/src/main/java/ly/count/android/sdk/RequestQueueProvider.java +++ b/sdk/src/main/java/ly/count/android/sdk/RequestQueueProvider.java @@ -52,6 +52,10 @@ interface RequestQueueProvider { String prepareRemoteConfigRequest(@Nullable String keysInclude, @Nullable String keysExclude, @NonNull String preparedMetrics); + String prepareEnrollmentParameters(@NonNull String[] keys); + + String prepareRemovalParameters(@NonNull String[] keys); + String prepareRatingWidgetRequest(String widgetId); String prepareFeedbackListRequest(); 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..d064ad00d --- /dev/null +++ b/sdk/src/main/java/ly/count/android/sdk/RequestResult.java @@ -0,0 +1,5 @@ +package ly.count.android.sdk; + +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..cb8ffc920 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,29 @@ protected static String urlEncodeString(String givenValue) { return result; } + protected static String encodedArrayBuilder(String[] args) { + StringBuilder encodedUrlBuilder = new StringBuilder(); + + encodedUrlBuilder.append("["); + + try { + + for (int i = 0; i < args.length; i++) { + String encodedElement = java.net.URLEncoder.encode(args[i], "UTF-8"); + encodedUrlBuilder.append("\"").append(encodedElement).append("\""); + if (i < args.length - 1) { + encodedUrlBuilder.append(", "); + } + } + } catch (UnsupportedEncodingException ignored) { + // should never happen because Android guarantees UTF-8 support + } + + encodedUrlBuilder.append("]"); + + return encodedUrlBuilder.toString(); + } + protected static String urlDecodeString(String givenValue) { String decodedResult = ""; From f96adc44a434e2fd4cbdfa0d789f859a094af9e7 Mon Sep 17 00:00:00 2001 From: turtledreams Date: Wed, 31 May 2023 19:01:18 +0900 Subject: [PATCH 04/30] 1 --- .../java/ly/count/android/sdk/UtilsNetworking.java | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) 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 cb8ffc920..bce7f6d15 100644 --- a/sdk/src/main/java/ly/count/android/sdk/UtilsNetworking.java +++ b/sdk/src/main/java/ly/count/android/sdk/UtilsNetworking.java @@ -26,17 +26,11 @@ protected static String encodedArrayBuilder(String[] args) { encodedUrlBuilder.append("["); - try { - - for (int i = 0; i < args.length; i++) { - String encodedElement = java.net.URLEncoder.encode(args[i], "UTF-8"); - encodedUrlBuilder.append("\"").append(encodedElement).append("\""); - if (i < args.length - 1) { - encodedUrlBuilder.append(", "); - } + for (int i = 0; i < args.length; i++) { + encodedUrlBuilder.append('"').append(args[i]).append('"'); + if (i < args.length - 1) { + encodedUrlBuilder.append(", "); } - } catch (UnsupportedEncodingException ignored) { - // should never happen because Android guarantees UTF-8 support } encodedUrlBuilder.append("]"); From 85ed257bfc3e0d43e070d529769117138c30653d Mon Sep 17 00:00:00 2001 From: ArtursK Date: Wed, 31 May 2023 21:24:41 +0300 Subject: [PATCH 05/30] Fixing merge issues --- .../java/ly/count/android/sdk/ConnectionQueue.java | 4 ++++ .../java/ly/count/android/sdk/CountlyConfig.java | 14 -------------- .../ly/count/android/sdk/ModuleRemoteConfig.java | 5 +++-- 3 files changed, 7 insertions(+), 16 deletions(-) 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 3d61cdb0c..9bbde36c5 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java +++ b/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java @@ -729,6 +729,10 @@ public String prepareRemovalParameters(@NonNull String[] keys) { if (keys.length > 0) { data += "&keys=" + UtilsNetworking.encodedArrayBuilder(keys); // TODO: key? keys? /this is not settled yet, redo after its settled } + + 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/CountlyConfig.java b/sdk/src/main/java/ly/count/android/sdk/CountlyConfig.java index 91496e025..89a09121b 100644 --- a/sdk/src/main/java/ly/count/android/sdk/CountlyConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/CountlyConfig.java @@ -469,20 +469,6 @@ public synchronized CountlyConfig setPushIntentAddMetadata(boolean enable) { return this; } - /** - * If enable, will automatically download newest remote config values. - * - * @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 use the other version of this call that uses a different callback - */ - public synchronized CountlyConfig setRemoteConfigAutomaticDownload(boolean enabled, RemoteConfig.RemoteConfigCallback callback) { - enableRemoteConfigAutomaticDownload = enabled; - remoteConfigCallbackOld = callback; - return this; - } - /** * If enable, will automatically download newest remote config values. * diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 4c7fd79ed..626f7aacd 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -194,8 +194,9 @@ public void callback(JSONObject checkResponse) { } } }, L); + } - /** + /** * Internal call for fetching all variants of A/B test experiments * * @param callback called after the fetch is done @@ -223,7 +224,7 @@ void testingFetchVariantInformationInternal(@NonNull final RCVariantCallback cal 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." ); + callback.callback(RequestResponse.NETWORK_ISSUE, "Received response is null."); return; } From 2b6ed75ed78924b3584621c6797f36e8944f73a5 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Wed, 31 May 2023 21:46:27 +0300 Subject: [PATCH 06/30] Adding missing calls --- .../sdk/RemoteConfigVariantControlTests.java | 6 ++-- .../count/android/sdk/ModuleRemoteConfig.java | 33 +++++++++++-------- .../sdk/RemoteConfigDownloadCallback.java | 2 +- 3 files changed, 23 insertions(+), 18 deletions(-) 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..d00b44bd4 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigVariantControlTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigVariantControlTests.java @@ -181,7 +181,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"); @@ -201,7 +201,7 @@ public void testNullVariant() { 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 @@ -216,7 +216,7 @@ public void testFilteringWrongKeys() { 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 diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 626f7aacd..0d2c186d2 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -667,11 +667,11 @@ public String[] testingGetVariantsForKey(String key) { } /** - * Fetches all variants of A/B testing experiments + * Download all variants of A/B testing experiments * * @param callback */ - public void testingFetchVariantInformation(RCVariantCallback callback) { + public void TestingDownloadVariantInformation(RCVariantCallback callback) { synchronized (_cly) { L.i("[RemoteConfig] Calling 'testingFetchVariantInformation'"); @@ -790,7 +790,6 @@ public void updateForKeysOnly(String[] keysToInclude, RemoteConfigCallback callb * Manually update remote config_ values * * @param callback - * @deprecated */ public void update(RemoteConfigCallback callback) { synchronized (_cly) { @@ -812,15 +811,14 @@ public void update(RemoteConfigCallback callback) { * * @param keysToExclude * @param callback - * @deprecated */ - public void updateExceptKeys(String[] keysToExclude, RemoteConfigDownloadCallback callback) { + public void DownloadOmittingValues(String[] keysToExclude, RemoteConfigDownloadCallback callback) { synchronized (_cly) { L.i("[RemoteConfig] Manually calling to updateRemoteConfig with exclude keys"); if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { if (callback != null) { - callback.callback(RequestResult.Error, null); + callback.callback(RequestResult.Error, null, false, null); } return; } @@ -829,21 +827,20 @@ public void updateExceptKeys(String[] keysToExclude, RemoteConfigDownloadCallbac } updateRemoteConfigValues(null, keysToExclude, false, null); // TODO: this callback was not expected } - } // TODO: this is a duplicate function + } /** * Manual remote config_ update call. Will only update the keys provided. * * @param keysToInclude * @param callback - * @deprecated */ - public void updateForKeysOnly(String[] keysToInclude, RemoteConfigDownloadCallback callback) { + public void DownloadSpecificValue(String[] keysToInclude, RemoteConfigDownloadCallback callback) { synchronized (_cly) { L.i("[RemoteConfig] Manually calling to updateRemoteConfig with include keys"); if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { if (callback != null) { - callback.callback(RequestResult.Error, null); + callback.callback(RequestResult.Error, null, false, null); } return; } @@ -852,22 +849,22 @@ public void updateForKeysOnly(String[] keysToInclude, RemoteConfigDownloadCallba } updateRemoteConfigValues(keysToInclude, null, false, null); // TODO: this callback was not expected } - } // TODO: this is a duplicate function + } - public void update(RemoteConfigDownloadCallback callback) { + public void DownloadValues(RemoteConfigDownloadCallback 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); + callback.callback(RequestResult.Error, null, true, null); } return; } updateRemoteConfigValues(null, null, false, null); // TODO: this callback was not expected } - } // TODO: this is a duplicate function + } /** * Enrolls user to AB tests of the given keys. @@ -911,5 +908,13 @@ public void exitABTestsForKeys(String[] keys) { exitABTestsForKeysInternal(keys); } } + + public void registerDownloadCallback(RemoteConfigDownloadCallback callback) { + + } + + public void removeDownloadCallback(RemoteConfigDownloadCallback callback) { + + } } } diff --git a/sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java b/sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java index 3bd1d183a..432b6d32e 100644 --- a/sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java +++ b/sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java @@ -3,5 +3,5 @@ import org.json.JSONObject; interface RemoteConfigDownloadCallback { - void callback(RequestResult downloadResult, JSONObject downloadedValues); + void callback(RequestResult downloadResult, String error, boolean fullValueUpdate, JSONObject downloadedValues); } From 7f034fa5f7d9c16d9001c92ba2b8f4fd0950d9d1 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Wed, 7 Jun 2023 20:14:34 +0300 Subject: [PATCH 07/30] Updating things for code coverage calculations --- sdk-native/build.gradle | 55 ++++++++++++++++++++--------------------- sdk/build.gradle | 11 +++------ 2 files changed, 31 insertions(+), 35 deletions(-) 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 From 01d1eebd03070942087b44ebc83c9b9b4da91917 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Wed, 7 Jun 2023 20:33:46 +0300 Subject: [PATCH 08/30] tweaking the RC/AB API and data structures --- .../android/demo/ActivityExampleTests.java | 12 +- .../count/android/sdk/ModuleRemoteConfig.java | 241 ++++++++++-------- .../java/ly/count/android/sdk/RCData.java | 11 + ...dCallback.java => RCDownloadCallback.java} | 2 +- .../java/ly/count/android/sdk/RCValue.java | 7 - .../ly/count/android/sdk/RCValueState.java | 5 - .../count/android/sdk/RCVariantCallback.java | 4 +- .../ly/count/android/sdk/RequestResponse.java | 12 - .../ly/count/android/sdk/RequestResult.java | 4 +- 9 files changed, 155 insertions(+), 143 deletions(-) create mode 100644 sdk/src/main/java/ly/count/android/sdk/RCData.java rename sdk/src/main/java/ly/count/android/sdk/{RemoteConfigDownloadCallback.java => RCDownloadCallback.java} (81%) delete mode 100644 sdk/src/main/java/ly/count/android/sdk/RCValue.java delete mode 100644 sdk/src/main/java/ly/count/android/sdk/RCValueState.java delete mode 100644 sdk/src/main/java/ly/count/android/sdk/RequestResponse.java 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..7125b80d3 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/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 0d2c186d2..541e1a750 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -207,7 +207,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; } @@ -224,7 +224,7 @@ void testingFetchVariantInformationInternal(@NonNull final RCVariantCallback cal 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."); + callback.callback(RequestResult.NetworkIssue, "Received response is null."); return; } @@ -235,12 +235,12 @@ public void callback(JSONObject checkResponse) { L.e("[ModuleRemoteConfig] testingFetchVariantInformationInternal - execute, Encountered internal issue while trying to fetch information from the server, [" + ex.toString() + "]"); } - 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."); } } @@ -250,14 +250,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; } @@ -274,13 +274,13 @@ void testingEnrollIntoVariantInternal(@NonNull final String key, @NonNull final 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."); + callback.callback(RequestResult.NetworkIssue, "Received response is null."); return; } try { if (!isResponseValid(checkResponse)) { - callback.callback(RequestResponse.NETWORK_ISSUE, "Bad response from the server:" + checkResponse.toString()); + callback.callback(RequestResult.NetworkIssue, "Bad response from the server:" + checkResponse.toString()); return; } @@ -297,7 +297,7 @@ public void callback(JSONObject checkResponse) { }); } - callback.callback(RequestResponse.SUCCESS, null); + 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() + "]"); } @@ -305,7 +305,7 @@ public void callback(JSONObject checkResponse) { }, 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."); } } @@ -631,96 +631,6 @@ public Map getAllValues() { } } - /** - * Returns all variant information as a Map - * - * @return - */ - public Map testingGetAllVariants() { - synchronized (_cly) { - L.i("[RemoteConfig] Calling 'testingGetAllVariants'"); - - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return null; - } - - return testingGetAllVariantsInternal(); - } - } - - /** - * Returns variant information for a key as a String[] - * - * @param key - key value to get variant information for - * @return - */ - public String[] testingGetVariantsForKey(String key) { - synchronized (_cly) { - L.i("[RemoteConfig] Calling 'testingGetVariantsForKey'"); - - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return null; - } - - return testingGetVariantsForKeyInternal(key); - } - } - - /** - * Download all variants of A/B testing experiments - * - * @param callback - */ - public void TestingDownloadVariantInformation(RCVariantCallback callback) { - synchronized (_cly) { - L.i("[RemoteConfig] Calling 'testingFetchVariantInformation'"); - - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return; - } - - if (callback == null) { - callback = new RCVariantCallback() { - @Override public void callback(RequestResponse result, String error) { - } - }; - } - - testingFetchVariantInformationInternal(callback); - } - } - - /** - * Enrolls user for a specific variant of A/B testing experiment - * - * @param key - key value retrieved from the fetched variants - * @param variantName - name of the variant for the key to enroll - * @param callback - */ - public void testingEnrollIntoVariant(String key, String variantName, RCVariantCallback callback) { - synchronized (_cly) { - L.i("[RemoteConfig] Calling 'testingEnrollIntoVariant'"); - - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return; - } - - if (key == null || variantName == null) { - L.w("[RemoteConfig] testEnrollIntoVariant, passed key or variant is null. Aborting."); - return; - } - - if (callback == null) { - callback = new RCVariantCallback() { - @Override public void callback(RequestResponse result, String error) { - } - }; - } - - testingEnrollIntoVariantInternal(key, variantName, callback); - } - } - /** * Get the stored value for the provided remote config_ key * @@ -790,6 +700,7 @@ public void updateForKeysOnly(String[] keysToInclude, RemoteConfigCallback callb * Manually update remote config_ values * * @param callback + * @deprecated */ public void update(RemoteConfigCallback callback) { synchronized (_cly) { @@ -809,10 +720,10 @@ public void update(RemoteConfigCallback callback) { /** * Manual remote config update call. Will update all keys except the ones provided * - * @param keysToExclude + * @param keysToOmit * @param callback */ - public void DownloadOmittingValues(String[] keysToExclude, RemoteConfigDownloadCallback callback) { + public void DownloadOmittingKeys(String[] keysToOmit, RCDownloadCallback callback) { synchronized (_cly) { L.i("[RemoteConfig] Manually calling to updateRemoteConfig with exclude keys"); @@ -822,10 +733,10 @@ public void DownloadOmittingValues(String[] keysToExclude, RemoteConfigDownloadC } return; } - if (keysToExclude == null) { + if (keysToOmit == null) { L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to ignore' array is null"); } - updateRemoteConfigValues(null, keysToExclude, false, null); // TODO: this callback was not expected + updateRemoteConfigValues(null, keysToOmit, false, null); // TODO: this callback was not expected } } @@ -835,7 +746,7 @@ public void DownloadOmittingValues(String[] keysToExclude, RemoteConfigDownloadC * @param keysToInclude * @param callback */ - public void DownloadSpecificValue(String[] keysToInclude, RemoteConfigDownloadCallback callback) { + public void DownloadSpecificKeys(String[] keysToInclude, RCDownloadCallback callback) { synchronized (_cly) { L.i("[RemoteConfig] Manually calling to updateRemoteConfig with include keys"); if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { @@ -851,7 +762,7 @@ public void DownloadSpecificValue(String[] keysToInclude, RemoteConfigDownloadCa } } - public void DownloadValues(RemoteConfigDownloadCallback callback) { + public void DownloadAllKeys(RCDownloadCallback callback) { synchronized (_cly) { L.i("[RemoteConfig] Manually calling to update Remote Config v2"); @@ -866,6 +777,30 @@ public void DownloadValues(RemoteConfigDownloadCallback callback) { } } + public @NonNull Map GetAllValues() { + synchronized (_cly) { + L.i("[RemoteConfig] Getting all Remote config values v2"); + + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + return new HashMap<>(); + } + + return new HashMap<>(); + } + } + + public @NonNull RCData GetValue(String key) { + synchronized (_cly) { + L.i("[RemoteConfig] Getting Remote config values for key:[" + key + "] v2"); + + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + return new RCData(null, true); + } + + return new RCData(null, true); + } + } + /** * Enrolls user to AB tests of the given keys. * @@ -909,12 +844,102 @@ public void exitABTestsForKeys(String[] keys) { } } - public void registerDownloadCallback(RemoteConfigDownloadCallback callback) { + public void registerDownloadCallback(RCDownloadCallback callback) { + + } + + public void removeDownloadCallback(RCDownloadCallback callback) { + + } + + /** + * Returns all variant information as a Map + * + * @return + */ + public @NonNull Map testingGetAllVariants() { + synchronized (_cly) { + L.i("[RemoteConfig] Calling 'testingGetAllVariants'"); + + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + return null; + } + + return testingGetAllVariantsInternal(); + } + } + + /** + * Returns variant information for a key as a String[] + * + * @param key - key value to get variant information for + * @return + */ + public @NonNull String[] testingGetVariantsForKey(String key) { + synchronized (_cly) { + L.i("[RemoteConfig] Calling 'testingGetVariantsForKey'"); + + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + return null; + } + + return testingGetVariantsForKeyInternal(key); + } + } + + /** + * Download all variants of A/B testing experiments + * + * @param completionCallback + */ + public void TestingDownloadVariantInformation(RCVariantCallback completionCallback) { + synchronized (_cly) { + L.i("[RemoteConfig] Calling 'testingFetchVariantInformation'"); + + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + return; + } + + if (completionCallback == null) { + completionCallback = new RCVariantCallback() { + @Override public void callback(RequestResult result, String error) { + } + }; + } + testingFetchVariantInformationInternal(completionCallback); + } } - public void removeDownloadCallback(RemoteConfigDownloadCallback callback) { + /** + * Enrolls user for a specific variant of A/B testing experiment + * + * @param keyName - key value retrieved from the fetched variants + * @param variantName - name of the variant for the key to enroll + * @param completionCallback + */ + public void testingEnrollIntoVariant(String keyName, String variantName, RCVariantCallback completionCallback) { + synchronized (_cly) { + L.i("[RemoteConfig] Calling 'testingEnrollIntoVariant'"); + if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + return; + } + + if (keyName == null || variantName == null) { + L.w("[RemoteConfig] testEnrollIntoVariant, passed key or variant is null. Aborting."); + return; + } + + if (completionCallback == null) { + completionCallback = new RCVariantCallback() { + @Override public void callback(RequestResult result, String error) { + } + }; + } + + testingEnrollIntoVariantInternal(keyName, variantName, completionCallback); + } } } } 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..591bd807f --- /dev/null +++ b/sdk/src/main/java/ly/count/android/sdk/RCData.java @@ -0,0 +1,11 @@ +package ly.count.android.sdk; + +class RCData { + public Object value; + boolean isCurrentUsersData; + + protected RCData(Object givenValue, boolean givenUserState) { + this.value = givenValue; + this.isCurrentUsersData = givenUserState; + } +} diff --git a/sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java b/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java similarity index 81% rename from sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java rename to sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java index 432b6d32e..6781c5b4d 100644 --- a/sdk/src/main/java/ly/count/android/sdk/RemoteConfigDownloadCallback.java +++ b/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java @@ -2,6 +2,6 @@ import org.json.JSONObject; -interface RemoteConfigDownloadCallback { +interface RCDownloadCallback { void callback(RequestResult downloadResult, String error, boolean fullValueUpdate, JSONObject downloadedValues); } diff --git a/sdk/src/main/java/ly/count/android/sdk/RCValue.java b/sdk/src/main/java/ly/count/android/sdk/RCValue.java deleted file mode 100644 index 5cc10f2a9..000000000 --- a/sdk/src/main/java/ly/count/android/sdk/RCValue.java +++ /dev/null @@ -1,7 +0,0 @@ -package ly.count.android.sdk; - -class RCValue { - public Object value; - Long timestamp; - RCValueState valueState; -} diff --git a/sdk/src/main/java/ly/count/android/sdk/RCValueState.java b/sdk/src/main/java/ly/count/android/sdk/RCValueState.java deleted file mode 100644 index 07cd6080b..000000000 --- a/sdk/src/main/java/ly/count/android/sdk/RCValueState.java +++ /dev/null @@ -1,5 +0,0 @@ -package ly.count.android.sdk; - -enum RCValueState { - Cached, NoValue, CurrentUser -} 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/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 index d064ad00d..09db79d6c 100644 --- a/sdk/src/main/java/ly/count/android/sdk/RequestResult.java +++ b/sdk/src/main/java/ly/count/android/sdk/RequestResult.java @@ -1,5 +1,5 @@ package ly.count.android.sdk; -enum RequestResult { - Success, NetworkIssue , Error +public enum RequestResult { + Success, NetworkIssue, Error } From 4108278c1813922ce6297a10d0c87423eba70208 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Wed, 7 Jun 2023 22:53:02 +0300 Subject: [PATCH 09/30] Clearing up a few varian parsing edge cases. --- .../sdk/RemoteConfigVariantControlTests.java | 77 +++++++++-------- .../count/android/sdk/ModuleRemoteConfig.java | 86 +++++++++---------- 2 files changed, 83 insertions(+), 80 deletions(-) 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 d00b44bd4..7485f94c6 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigVariantControlTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigVariantControlTests.java @@ -29,11 +29,11 @@ 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); @@ -60,8 +60,8 @@ 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); @@ -106,15 +106,14 @@ public void testConvertVariantsJsonToMap_InvalidJson() throws JSONException { // 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); + 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(); @@ -134,10 +133,10 @@ 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); @@ -161,9 +160,14 @@ 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) @@ -195,6 +199,9 @@ public void testNormalFlow() { Assert.assertEquals(0, variantArrayFalse.length); } + /** + * Reject a variant if it's name is a null json value + */ @Test public void testNullVariant() { CountlyConfig config = TestUtils.createVariantConfig(createIRGForSpecificResponse("{\"key\":[{\"name\":null}]}")); @@ -206,10 +213,12 @@ public void testNullVariant() { // 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\"}]}")); @@ -226,27 +235,21 @@ 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); }; } } diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 541e1a750..0fe597809 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -3,8 +3,10 @@ import android.text.TextUtils; 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 org.json.JSONArray; import org.json.JSONException; @@ -20,6 +22,8 @@ public class ModuleRemoteConfig extends ModuleBase { boolean remoteConfigAutomaticUpdateEnabled = false; RemoteConfigCallback remoteConfigInitCallback = null; + public final static String variantObjectNameKey = "name"; + @Nullable Map metricOverride = null; @@ -355,50 +359,46 @@ boolean isResponseValid(@NonNull JSONObject responseJson) throws JSONException { * * @param variantsObj - JSON Object fetched from the server * @return - * @throws JSONException */ - static Map convertVariantsJsonToMap(@NonNull JSONObject variantsObj) throws JSONException { + static Map convertVariantsJsonToMap(@NonNull JSONObject variantsObj) { // Initialize the map to store the results Map resultMap = new HashMap<>(); + // Get the keys of the JSON object using names() method + JSONArray keys = variantsObj.names(); + + if (keys == null) { + return resultMap; + } + + List tempVariantColl = new ArrayList<>(5); + 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++; - } - } + for (int i = 0; i < keys.length(); i++) { + String key = keys.getString(i); + Object value = variantsObj.get(key); - // 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 (!(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(variantObjectNameKey)) { + continue; + } + + tempVariantColl.add(variantObject.optString(variantObjectNameKey)); + } + + //write the filtered array to map + resultMap.put(key, tempVariantColl.toArray(new String[0])); } } catch (Exception ex) { Countly.sharedInstance().L.e("[ModuleRemoteConfig] convertVariantsJsonToMap, failed parsing:[" + ex.toString() + "]"); @@ -439,7 +439,7 @@ static Map convertVariantsJsonToMap(@NonNull JSONObject varian return res; } - Object getValue(String key) { + Object getValue(@NonNull String key) { try { RemoteConfigValueStore rcvs = loadConfig(); return rcvs.getValue(key); @@ -449,7 +449,7 @@ Object getValue(String key) { } } - void saveConfig(RemoteConfigValueStore rcvs) throws Exception { + void saveConfig(@NonNull RemoteConfigValueStore rcvs) throws Exception { storageProvider.setRemoteConfigValues(rcvs.dataToString()); } @@ -457,7 +457,7 @@ 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() throws Exception { String rcvsString = storageProvider.getRemoteConfigValues(); //noinspection UnnecessaryLocalVariable RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(rcvsString); @@ -468,7 +468,7 @@ void clearValueStoreInternal() { storageProvider.setRemoteConfigValues(""); } - Map getAllRemoteConfigValuesInternal() { + @NonNull Map getAllRemoteConfigValuesInternal() { try { RemoteConfigValueStore rcvs = loadConfig(); return rcvs.getAllValues(); @@ -483,7 +483,7 @@ Map getAllRemoteConfigValuesInternal() { * * @return */ - Map testingGetAllVariantsInternal() { + @NonNull Map testingGetAllVariantsInternal() { return variantContainer; } @@ -493,7 +493,7 @@ Map testingGetAllVariantsInternal() { * @param key * @return */ - String[] testingGetVariantsForKeyInternal(String key) { + @NonNull String[] testingGetVariantsForKeyInternal(@NonNull String key) { if (variantContainer.containsKey(key)) { return variantContainer.get(key); } From d0ede3803dea5f5ce1f819abbfbfc5cd7cd9e217 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Wed, 7 Jun 2023 23:56:43 +0300 Subject: [PATCH 10/30] Fixing issues and refactoring --- .../ly/count/android/sdk/ConnectionQueue.java | 22 +++- .../ly/count/android/sdk/ModuleDeviceId.java | 5 +- .../count/android/sdk/ModuleRemoteConfig.java | 107 +++++++++++++----- .../android/sdk/RequestQueueProvider.java | 2 + 4 files changed, 102 insertions(+), 34 deletions(-) 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 9bbde36c5..08c07b748 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,26 @@ 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) 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..b85703cf3 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleDeviceId.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleDeviceId.java @@ -86,10 +86,7 @@ void exitTemporaryIdMode(@NonNull String deviceId) { 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(true); _cly.requestQueue().attemptToSendStoredRequests(); } diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 0fe597809..d0bfc87d0 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -12,6 +12,8 @@ import org.json.JSONException; import org.json.JSONObject; +import static ly.count.android.sdk.ModuleConsent.ConsentChangeSource.ChangeConsentCall; + public class ModuleRemoteConfig extends ModuleBase { ImmediateRequestGenerator immediateRequestGenerator; boolean updateRemoteConfigAfterIdChange = false; @@ -55,9 +57,9 @@ public class ModuleRemoteConfig extends ModuleBase { * @param requestShouldBeDelayed this is set to true in case of update after a deviceId change * @param callback called after the update is done */ - void updateRemoteConfigValues(@Nullable final String[] keysOnly, @Nullable final String[] keysExcept, final boolean requestShouldBeDelayed, @Nullable final RemoteConfigCallback callback) { + void updateRemoteConfigValues(@Nullable final String[] keysOnly, @Nullable final String[] keysExcept, final boolean requestShouldBeDelayed, final boolean useLegacyAPI, @Nullable final RemoteConfigCallback callback) { try { - L.d("[ModuleRemoteConfig] Updating remote config values, requestShouldBeDelayed:[" + requestShouldBeDelayed + "]"); + L.d("[ModuleRemoteConfig] Updating remote config values, requestShouldBeDelayed:[" + requestShouldBeDelayed + "], legacyAPI:[" + useLegacyAPI + "]"); // checks if (deviceIdProvider.getDeviceId() == null) { @@ -81,7 +83,13 @@ void updateRemoteConfigValues(@Nullable final String[] keysOnly, @Nullable final //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(); @@ -228,7 +236,7 @@ void testingFetchVariantInformationInternal(@NonNull final RCVariantCallback cal 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(RequestResult.NetworkIssue, "Received response is null."); + callback.callback(RequestResult.NetworkIssue, "Encountered problem while trying to reach the server, possibly no internet connection"); return; } @@ -278,7 +286,7 @@ void testingEnrollIntoVariantInternal(@NonNull final String key, @NonNull final 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(RequestResult.NetworkIssue, "Received response is null."); + callback.callback(RequestResult.NetworkIssue, "Encountered problem while trying to reach the server, possibly no internet connection"); return; } @@ -290,7 +298,7 @@ public void callback(JSONObject checkResponse) { // Update Remote Config if (remoteConfigAutomaticUpdateEnabled) { - updateRemoteConfigValues(null, null, false, new RemoteConfigCallback() { + updateRemoteConfigValues(null, null, false, false, new RemoteConfigCallback() { @Override public void callback(String error) { if (error == null) { L.d("[ModuleRemoteConfig] Updated remote config after enrolling to a variant"); @@ -342,13 +350,17 @@ 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; + try { + if (responseJson.get("result").equals("Success")) { + result = true; + } + } catch (JSONException e) { + L.e("[ModuleRemoteConfig] isResponseValid, encountered issue, " + e); + return false; } return result; @@ -449,7 +461,7 @@ Object getValue(@NonNull String key) { } } - void saveConfig(@NonNull RemoteConfigValueStore rcvs) throws Exception { + void saveConfig(@NonNull RemoteConfigValueStore rcvs) { storageProvider.setRemoteConfigValues(rcvs.dataToString()); } @@ -457,7 +469,7 @@ void saveConfig(@NonNull RemoteConfigValueStore rcvs) throws Exception { * @return * @throws Exception For some reason this might be throwing an exception */ - @NonNull RemoteConfigValueStore loadConfig() throws Exception { + @NonNull RemoteConfigValueStore loadConfig() { String rcvsString = storageProvider.getRemoteConfigValues(); //noinspection UnnecessaryLocalVariable RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(rcvsString); @@ -571,28 +583,58 @@ public String dataToString() { void clearAndDownloadAfterIdChange() { L.v("[RemoteConfig] Clearing remote config values and preparing to download after ID update"); - clearValueStoreInternal(); + CacheOrClearRCValuesIfNeeded(); if (remoteConfigAutomaticUpdateEnabled && consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { updateRemoteConfigAfterIdChange = true; } } + void CacheOrClearRCValuesIfNeeded() { + clearValueStoreInternal(); + } + + void RCAutomaticDownloadTrigger(boolean cacheClearOldValues) { + if (cacheClearOldValues) { + clearValueStoreInternal();//todo finish + } + + if (remoteConfigAutomaticUpdateEnabled && consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + L.d("[RemoteConfig] Automatically updating remote config values"); + updateRemoteConfigValues(null, null, false, false, remoteConfigInitCallback); + } else { + L.v("[RemoteConfig] Automatically RC update trigger skipped"); + } + } + + @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 + } + } + } + @Override void deviceIdChanged() { L.v("[RemoteConfig] Device ID changed will update values: [" + updateRemoteConfigAfterIdChange + "]"); 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); } } @@ -610,6 +652,8 @@ public void halt() { public class RemoteConfig { /** * Clear all stored remote config_ values + * + * @deprecated */ public void clearStoredValues() { synchronized (_cly) { @@ -619,6 +663,10 @@ public void clearStoredValues() { } } + /** + * @return + * @deprecated + */ public Map getAllValues() { synchronized (_cly) { L.i("[RemoteConfig] Calling 'getAllValues'"); @@ -636,6 +684,7 @@ public Map getAllValues() { * * @param key * @return + * @deprecated */ public Object getValueForKey(String key) { synchronized (_cly) { @@ -669,7 +718,7 @@ public void updateExceptKeys(String[] keysToExclude, RemoteConfigCallback callba if (keysToExclude == null) { L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to ignore' array is null"); } - updateRemoteConfigValues(null, keysToExclude, false, callback); + updateRemoteConfigValues(null, keysToExclude, false, true, callback); } } @@ -692,7 +741,7 @@ public void updateForKeysOnly(String[] keysToInclude, RemoteConfigCallback callb if (keysToInclude == null) { L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to include' array is null"); } - updateRemoteConfigValues(keysToInclude, null, false, callback); + updateRemoteConfigValues(keysToInclude, null, false, true, callback); } } @@ -713,7 +762,7 @@ public void update(RemoteConfigCallback callback) { return; } - updateRemoteConfigValues(null, null, false, callback); + updateRemoteConfigValues(null, null, false, true, callback); } } @@ -736,7 +785,7 @@ public void DownloadOmittingKeys(String[] keysToOmit, RCDownloadCallback callbac if (keysToOmit == null) { L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to ignore' array is null"); } - updateRemoteConfigValues(null, keysToOmit, false, null); // TODO: this callback was not expected + updateRemoteConfigValues(null, keysToOmit, false, false, null); // TODO: this callback was not expected } } @@ -758,7 +807,7 @@ public void DownloadSpecificKeys(String[] keysToInclude, RCDownloadCallback call if (keysToInclude == null) { L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to include' array is null"); } - updateRemoteConfigValues(keysToInclude, null, false, null); // TODO: this callback was not expected + updateRemoteConfigValues(keysToInclude, null, false, false, null); // TODO: this callback was not expected } } @@ -773,7 +822,7 @@ public void DownloadAllKeys(RCDownloadCallback callback) { return; } - updateRemoteConfigValues(null, null, false, null); // TODO: this callback was not expected + updateRemoteConfigValues(null, null, false, false, null); // TODO: this callback was not expected } } @@ -852,6 +901,10 @@ public void removeDownloadCallback(RCDownloadCallback callback) { } + public void clearAll() { + + } + /** * Returns all variant information as a Map * @@ -901,9 +954,7 @@ public void TestingDownloadVariantInformation(RCVariantCallback completionCallba } if (completionCallback == null) { - completionCallback = new RCVariantCallback() { - @Override public void callback(RequestResult result, String error) { - } + completionCallback = (result, error) -> { }; } @@ -932,9 +983,7 @@ public void testingEnrollIntoVariant(String keyName, String variantName, RCVaria } if (completionCallback == null) { - completionCallback = new RCVariantCallback() { - @Override public void callback(RequestResult result, String error) { - } + completionCallback = (result, 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 81d3f6554..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,6 +50,8 @@ 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); From b72a4539c0ba369283882e57d50af2813d9fdd21 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Thu, 8 Jun 2023 00:23:47 +0300 Subject: [PATCH 11/30] Moving things out of the remote config module --- .../android/sdk/ModuleRemoteConfigTests.java | 148 ++-------------- .../sdk/RemoteConfigValueStoreTests.java | 153 ++++++++++++++++ .../sdk/RemoteConfigVariantControlTests.java | 17 +- .../count/android/sdk/ModuleRemoteConfig.java | 163 +----------------- .../sdk/internal/RemoteConfigHelper.java | 95 ++++++++++ .../sdk/internal/RemoteConfigValueStore.java | 75 ++++++++ 6 files changed, 346 insertions(+), 305 deletions(-) create mode 100644 sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java create mode 100644 sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigHelper.java create mode 100644 sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java 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..4357994f8 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; @@ -23,164 +25,34 @@ public void setUp() { countlyStore = new CountlyStore(getContext(), mock(ModuleLog.class)); } - /** - * Basic serialization / deserialization test - * - * @throws JSONException - */ - @Test - public void rcvsSerializeDeserialize() throws JSONException { - ModuleRemoteConfig.RemoteConfigValueStore remoteConfigValueStore = ModuleRemoteConfig.RemoteConfigValueStore.dataFromString(null); - - remoteConfigValueStore.values.put("fd", 12); - remoteConfigValueStore.values.put("2fd", 142); - remoteConfigValueStore.values.put("f3d", 123); - - ModuleRemoteConfig.RemoteConfigValueStore.dataFromString(remoteConfigValueStore.dataToString()); - } - - /** - * Validating regressive cases for "dataFromString" - */ - @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()); - } - - /** - * A simple "dataFromString" test case - */ - @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")); - } - - /** - * A more complicated "dataFromString" test case - * It also validates serialization and getAllValues call - * - * @throws JSONException - */ - @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")); - } - - /** - * Simple test for value merging - * - * @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\"}"); - - rcvs1.mergeValues(rcvs2.values); - - Assert.assertEquals(3, rcvs1.values.length()); - - Assert.assertEquals(123, rcvs1.getValue("a")); - Assert.assertEquals(123.3, rcvs1.getValue("b")); - Assert.assertEquals("uio", rcvs1.getValue("c")); - } - /** * validating that 'prepareKeysIncludeExclude' works as expected */ @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]); } @@ -196,9 +68,9 @@ public void validateMergeReceivedResponse() throws Exception { 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\"}"); + RemoteConfigValueStore rcvs2 = RemoteConfigValueStore.dataFromString("{\"b\": 33.44,\"c\": \"ww\"}"); + RemoteConfigValueStore rcvs3 = RemoteConfigValueStore.dataFromString("{\"t\": {},\"87\": \"yy\"}"); //check initial state Map vals = countly.remoteConfig().getAllValues(); 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..046072803 --- /dev/null +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java @@ -0,0 +1,153 @@ +package ly.count.android.sdk; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +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.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); + + remoteConfigValueStore.values.put("fd", 12); + remoteConfigValueStore.values.put("2fd", 142); + remoteConfigValueStore.values.put("f3d", 123); + + RemoteConfigValueStore.dataFromString(remoteConfigValueStore.dataToString()); + } + + /** + * Validating regressive cases for "dataFromString" + */ + @Test + public void rcvsDataFromStringNullEmpty() { + RemoteConfigValueStore rcvs1 = RemoteConfigValueStore.dataFromString(null); + Assert.assertNotNull(rcvs1); + Assert.assertNotNull(rcvs1.values); + Assert.assertEquals(0, rcvs1.values.length()); + + RemoteConfigValueStore rcvs2 = RemoteConfigValueStore.dataFromString(""); + Assert.assertNotNull(rcvs2); + Assert.assertNotNull(rcvs2.values); + Assert.assertEquals(0, rcvs2.values.length()); + } + + /** + * A simple "dataFromString" test case + */ + @Test + public void rcvsDataFromStringSamples_1() { + RemoteConfigValueStore rcvs = 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")); + } + + /** + * A more complicated "dataFromString" test case + * It also validates serialization and getAllValues call + * + * @throws JSONException + */ + @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\"}}"; + RemoteConfigValueStore rcvs = 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")); + } + + /** + * Simple test for value merging + * + * @throws JSONException + */ + @Test + public void rcvsMergeValues_1() throws JSONException { + RemoteConfigValueStore rcvs1 = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}"); + RemoteConfigValueStore rcvs2 = RemoteConfigValueStore.dataFromString("{\"b\": 123.3,\"c\": \"uio\"}"); + + rcvs1.mergeValues(rcvs2.values); + + Assert.assertEquals(3, rcvs1.values.length()); + + Assert.assertEquals(123, rcvs1.getValue("a")); + Assert.assertEquals(123.3, rcvs1.getValue("b")); + Assert.assertEquals("uio", rcvs1.getValue("c")); + } +} 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 7485f94c6..8e5413471 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; @@ -38,7 +39,7 @@ public void testConvertVariantsJsonToMap_ValidInput_Multi() throws JSONException 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()); @@ -64,7 +65,7 @@ public void testConvertVariantsJsonToMap_ValidInput_Single() throws JSONExceptio 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,10 +103,10 @@ 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); + Map resultMap = RemoteConfigHelper.convertVariantsJsonToMap(variantsObj, mock(ModuleLog.class)); Assert.assertEquals(0, resultMap.size()); } @@ -118,7 +119,7 @@ public void testConvertVariantsJsonToMap_NoValues() { 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()); @@ -139,7 +140,7 @@ public void testConvertVariantsJsonToMap_DifferentStructures() throws JSONExcept 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()); @@ -171,7 +172,7 @@ public void testConvertVariantsJsonToMap_NullJsonKey() throws JSONException { 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"); diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index d0bfc87d0..9ecdaa402 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -8,6 +8,8 @@ import java.util.Iterator; import java.util.List; 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; @@ -82,7 +84,7 @@ void updateRemoteConfigValues(@Nullable final String[] keysOnly, @Nullable final //prepare metrics and request data String preparedMetrics = deviceInfo.getMetrics(_cly.context_, deviceInfo, metricOverride); - String[] preparedKeys = prepareKeysIncludeExclude(keysOnly, keysExcept); + String[] preparedKeys = RemoteConfigHelper.prepareKeysIncludeExclude(keysOnly, keysExcept, L); String requestData; if (useLegacyAPI) { @@ -240,12 +242,7 @@ public void callback(JSONObject checkResponse) { 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(RequestResult.Success, null); } @@ -366,91 +363,6 @@ boolean isResponseValid(@NonNull JSONObject responseJson) { return result; } - /** - * Converts A/B testing variants fetched from the server (JSONObject) into a map - * - * @param variantsObj - JSON Object fetched from the server - * @return - */ - static Map convertVariantsJsonToMap(@NonNull JSONObject variantsObj) { - // Initialize the map to store the results - Map resultMap = new HashMap<>(); - - // Get the keys of the JSON object using names() method - 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(variantObjectNameKey)) { - continue; - } - - tempVariantColl.add(variantObject.optString(variantObjectNameKey)); - } - - //write the filtered array to map - resultMap.put(key, tempVariantColl.toArray(new String[0])); - } - } catch (Exception ex) { - Countly.sharedInstance().L.e("[ModuleRemoteConfig] convertVariantsJsonToMap, failed parsing:[" + ex.toString() + "]"); - return new HashMap<>(); - } - - return resultMap; - } - - /* - * 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 - - 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; - } - Object getValue(@NonNull String key) { try { RemoteConfigValueStore rcvs = loadConfig(); @@ -513,73 +425,6 @@ void clearValueStoreInternal() { return new String[0]; } - 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"); - } - } - } - - private RemoteConfigValueStore(JSONObject values) { - this.values = values; - } - - public Object getValue(String key) { - return values.opt(key); - } - - 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()); - } - } - - return ret; - } - - public static RemoteConfigValueStore dataFromString(String storageString) { - if (storageString == null || storageString.isEmpty()) { - return new RemoteConfigValueStore(new JSONObject()); - } - - 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); - } - - public String dataToString() { - return values.toString(); - } - } - void clearAndDownloadAfterIdChange() { L.v("[RemoteConfig] Clearing remote config values and preparing to download after ID update"); 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..47d3318b8 --- /dev/null +++ b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigHelper.java @@ -0,0 +1,95 @@ +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.List; +import java.util.Map; +import ly.count.android.sdk.ModuleLog; +import ly.count.android.sdk.ModuleRemoteConfig; +import org.json.JSONArray; +import org.json.JSONObject; + +public class RemoteConfigHelper { + /* + * 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..20f69d8d7 --- /dev/null +++ b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java @@ -0,0 +1,75 @@ +package ly.count.android.sdk.internal; + +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; +import ly.count.android.sdk.Countly; +import org.json.JSONException; +import org.json.JSONObject; + +public 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"); + } + } + } + + private RemoteConfigValueStore(JSONObject values) { + this.values = values; + } + + public Object getValue(String key) { + return values.opt(key); + } + + 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()); + } + } + + return ret; + } + + public static RemoteConfigValueStore dataFromString(String storageString) { + if (storageString == null || storageString.isEmpty()) { + return new RemoteConfigValueStore(new JSONObject()); + } + + 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); + } + + public String dataToString() { + return values.toString(); + } +} From 9272a250454dcffd22faba2c85389a1b148b9f64 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Thu, 8 Jun 2023 09:12:32 +0300 Subject: [PATCH 12/30] Setup remote config value store for expansion --- .../android/sdk/ModuleRemoteConfigTests.java | 6 +-- .../sdk/RemoteConfigValueStoreTests.java | 44 ++++++++-------- .../count/android/sdk/ModuleRemoteConfig.java | 31 +++-------- .../java/ly/count/android/sdk/RCData.java | 2 +- .../sdk/internal/RemoteConfigValueStore.java | 51 ++++++++++++++++--- 5 files changed, 76 insertions(+), 58 deletions(-) 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 4357994f8..396d9c620 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java @@ -68,9 +68,9 @@ public void validateMergeReceivedResponse() throws Exception { Countly countly = new Countly(); countly.init(cc); - RemoteConfigValueStore rcvs1 = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}"); - RemoteConfigValueStore rcvs2 = RemoteConfigValueStore.dataFromString("{\"b\": 33.44,\"c\": \"ww\"}"); - RemoteConfigValueStore rcvs3 = 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(); diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java index 046072803..480387564 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java @@ -31,13 +31,13 @@ public void setUp() { */ @Test public void rcvsSerializeDeserialize() throws JSONException { - RemoteConfigValueStore remoteConfigValueStore = RemoteConfigValueStore.dataFromString(null); + 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()); + RemoteConfigValueStore.dataFromString(remoteConfigValueStore.dataToString(), false); } /** @@ -45,12 +45,12 @@ public void rcvsSerializeDeserialize() throws JSONException { */ @Test public void rcvsDataFromStringNullEmpty() { - RemoteConfigValueStore rcvs1 = RemoteConfigValueStore.dataFromString(null); + RemoteConfigValueStore rcvs1 = RemoteConfigValueStore.dataFromString(null, false); Assert.assertNotNull(rcvs1); Assert.assertNotNull(rcvs1.values); Assert.assertEquals(0, rcvs1.values.length()); - RemoteConfigValueStore rcvs2 = RemoteConfigValueStore.dataFromString(""); + RemoteConfigValueStore rcvs2 = RemoteConfigValueStore.dataFromString("", false); Assert.assertNotNull(rcvs2); Assert.assertNotNull(rcvs2.values); Assert.assertEquals(0, rcvs2.values.length()); @@ -61,13 +61,13 @@ public void rcvsDataFromStringNullEmpty() { */ @Test public void rcvsDataFromStringSamples_1() { - RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}"); + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}", false); 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")); + Assert.assertEquals(123, rcvs.getValueLegacy("a")); + Assert.assertEquals("fg", rcvs.getValueLegacy("b")); } /** @@ -79,7 +79,7 @@ public void rcvsDataFromStringSamples_1() { @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\"}}"; - RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(initialString); + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(initialString, false); Assert.assertNotNull(rcvs); Assert.assertNotNull(rcvs.values); @@ -88,12 +88,12 @@ public void rcvsDataFromStringSamples_2() throws JSONException { //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")); + Assert.assertEquals(123, rcvs.getValueLegacy("321")); + Assert.assertEquals("\uD83D\uDE01", rcvs.getValueLegacy("\uD83D\uDE00")); + Assert.assertEquals(6.5, rcvs.getValueLegacy("d")); - Object v1 = rcvs.getValue("c"); - Object v2 = rcvs.getValue("e"); + Object v1 = rcvs.getValueLegacy("c"); + Object v2 = rcvs.getValueLegacy("e"); JSONArray jArr = (JSONArray) v1; Assert.assertEquals(4, jArr.length()); @@ -108,7 +108,7 @@ public void rcvsDataFromStringSamples_2() throws JSONException { Assert.assertEquals("op", jObj.get("w")); //validate that all of the values are the same when returned with getAllValues - Map allVals = rcvs.getAllValues(); + Map allVals = rcvs.getAllValuesLegacy(); Assert.assertEquals(5, allVals.size()); @@ -134,20 +134,18 @@ public void rcvsDataFromStringSamples_2() throws JSONException { /** * Simple test for value merging - * - * @throws JSONException */ @Test - public void rcvsMergeValues_1() throws JSONException { - RemoteConfigValueStore rcvs1 = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}"); - RemoteConfigValueStore rcvs2 = RemoteConfigValueStore.dataFromString("{\"b\": 123.3,\"c\": \"uio\"}"); + public void rcvsMergeValues_1() { + RemoteConfigValueStore rcvs1 = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}", false); + RemoteConfigValueStore rcvs2 = RemoteConfigValueStore.dataFromString("{\"b\": 123.3,\"c\": \"uio\"}", false); - rcvs1.mergeValues(rcvs2.values); + rcvs1.mergeValues(rcvs2.values, false); Assert.assertEquals(3, rcvs1.values.length()); - Assert.assertEquals(123, rcvs1.getValue("a")); - Assert.assertEquals(123.3, rcvs1.getValue("b")); - Assert.assertEquals("uio", rcvs1.getValue("c")); + Assert.assertEquals(123, rcvs1.getValueLegacy("a")); + Assert.assertEquals(123.3, rcvs1.getValueLegacy("b")); + Assert.assertEquals("uio", rcvs1.getValueLegacy("c")); } } diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 9ecdaa402..fa54eb20c 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -3,14 +3,11 @@ import android.text.TextUtils; 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.internal.RemoteConfigHelper; import ly.count.android.sdk.internal.RemoteConfigValueStore; -import org.json.JSONArray; import org.json.JSONException; import org.json.JSONObject; @@ -24,6 +21,8 @@ public class ModuleRemoteConfig extends ModuleBase { //if set to true, it will automatically download remote configs on module startup boolean remoteConfigAutomaticUpdateEnabled = false; + + boolean remoteConfigValuesShouldBeCached = false; RemoteConfigCallback remoteConfigInitCallback = null; public final static String variantObjectNameKey = "name"; @@ -293,22 +292,12 @@ public void callback(JSONObject checkResponse) { return; } - // Update Remote Config - if (remoteConfigAutomaticUpdateEnabled) { - updateRemoteConfigValues(null, null, false, 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()); - } - } - }); - } + RCAutomaticDownloadTrigger(true); 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); @@ -329,11 +318,7 @@ void mergeCheckResponseIntoCurrentValues(boolean clearOldValues, JSONObject chec //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(checkResponse, clearOldValues); L.d("[ModuleRemoteConfig] Finished remote config processing, starting saving"); @@ -366,7 +351,7 @@ boolean isResponseValid(@NonNull JSONObject responseJson) { Object getValue(@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() + "]"); return null; @@ -384,7 +369,7 @@ void saveConfig(@NonNull RemoteConfigValueStore rcvs) { @NonNull RemoteConfigValueStore loadConfig() { String rcvsString = storageProvider.getRemoteConfigValues(); //noinspection UnnecessaryLocalVariable - RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(rcvsString); + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(rcvsString, remoteConfigValuesShouldBeCached); return rcvs; } @@ -395,7 +380,7 @@ void clearValueStoreInternal() { @NonNull Map getAllRemoteConfigValuesInternal() { try { RemoteConfigValueStore rcvs = loadConfig(); - return rcvs.getAllValues(); + return rcvs.getAllValuesLegacy(); } catch (Exception ex) { Countly.sharedInstance().L.e("[ModuleRemoteConfig] getAllRemoteConfigValuesInternal, Call failed:[" + ex.toString() + "]"); return null; diff --git a/sdk/src/main/java/ly/count/android/sdk/RCData.java b/sdk/src/main/java/ly/count/android/sdk/RCData.java index 591bd807f..3097dfcd8 100644 --- a/sdk/src/main/java/ly/count/android/sdk/RCData.java +++ b/sdk/src/main/java/ly/count/android/sdk/RCData.java @@ -1,6 +1,6 @@ package ly.count.android.sdk; -class RCData { +public class RCData { public Object value; boolean isCurrentUsersData; 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 index 20f69d8d7..7cdcab9f8 100644 --- a/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java +++ b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java @@ -1,17 +1,43 @@ package ly.count.android.sdk.internal; +import androidx.annotation.NonNull; 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 = new JSONObject(); - //add new values to the current storage - public void mergeValues(JSONObject newValues) { + public boolean valuesCanBeCached = false; + + public void cacheClearValues() { + if (!valuesCanBeCached) { + clearValues(); + return; + } + + // values can be cached, do that + } + + public void clearValues() { + values = new JSONObject(); + } + + /** + * add new values to the current storage + * + * @param newValues + * @param fullUpdate clear all previous values in case of full update + */ + public void mergeValues(JSONObject newValues, boolean fullUpdate) { + if (fullUpdate) { + clearValues(); + } + if (newValues == null) { return; } @@ -28,15 +54,24 @@ public void mergeValues(JSONObject newValues) { } } - private RemoteConfigValueStore(JSONObject values) { + private RemoteConfigValueStore(JSONObject values, boolean valuesShouldBeCached) { this.values = values; + this.valuesCanBeCached = valuesShouldBeCached; } - public Object getValue(String key) { + public Object getValueLegacy(String key) { return values.opt(key); } - public Map getAllValues() { + public @NonNull RCData getValue(String key) { + return null; + } + + public @NonNull Map getAllValues() { + return null; + } + + public Map getAllValuesLegacy() { Map ret = new HashMap<>(); Iterator keys = values.keys(); @@ -54,9 +89,9 @@ public Map getAllValues() { return ret; } - public static RemoteConfigValueStore dataFromString(String storageString) { + public static RemoteConfigValueStore dataFromString(String storageString, boolean valuesShouldBeCached) { if (storageString == null || storageString.isEmpty()) { - return new RemoteConfigValueStore(new JSONObject()); + return new RemoteConfigValueStore(new JSONObject(), valuesShouldBeCached); } JSONObject values; @@ -66,7 +101,7 @@ public static RemoteConfigValueStore dataFromString(String storageString) { Countly.sharedInstance().L.e("[RemoteConfigValueStore] Couldn't decode RemoteConfigValueStore successfully: " + e.toString()); values = new JSONObject(); } - return new RemoteConfigValueStore(values); + return new RemoteConfigValueStore(values, valuesShouldBeCached); } public String dataToString() { From c0b524b0f9e291f4bd3b73adfec4ba220118398a Mon Sep 17 00:00:00 2001 From: ArtursK Date: Thu, 8 Jun 2023 09:15:43 +0300 Subject: [PATCH 13/30] fixing a few issues --- .../ly/count/android/sdk/ModuleRemoteConfig.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index fa54eb20c..90615d043 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -377,13 +377,23 @@ void clearValueStoreInternal() { storageProvider.setRemoteConfigValues(""); } - @NonNull 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 null; + 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 new HashMap<>(); } } @@ -505,7 +515,7 @@ public Map getAllValues() { return null; } - return getAllRemoteConfigValuesInternal(); + return getAllRemoteConfigValuesInternalLegacy(); } } From fe4d8e103af1b03651b66250b1ab833fbec9682f Mon Sep 17 00:00:00 2001 From: ArtursK Date: Thu, 8 Jun 2023 09:20:34 +0300 Subject: [PATCH 14/30] selecting correct interface for requesting variants --- sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 90615d043..fba5a9dfa 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -232,7 +232,7 @@ 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() { + immediateRequestGenerator.CreateImmediateRequestMaker().doWork(requestData, "/o/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) + "]"); From cfb02af34e766501c065df4e009d1443a603dd46 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Thu, 8 Jun 2023 10:52:30 +0300 Subject: [PATCH 15/30] Adding global callbacks Refactoring things. --- .../count/android/sdk/CountlyConfigTests.java | 8 +- .../android/sdk/ModuleRemoteConfigTests.java | 6 +- .../sdk/RemoteConfigValueStoreTests.java | 2 +- .../ly/count/android/sdk/CountlyConfig.java | 21 ++- .../count/android/sdk/ModuleRemoteConfig.java | 134 ++++++++++++------ .../count/android/sdk/RCDownloadCallback.java | 3 +- .../sdk/internal/RemoteConfigHelper.java | 24 ++++ .../sdk/internal/RemoteConfigValueStore.java | 6 +- 8 files changed, 146 insertions(+), 58 deletions(-) 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/ModuleRemoteConfigTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java index 396d9c620..a312a3980 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java @@ -78,7 +78,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()); @@ -86,7 +86,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()); @@ -95,7 +95,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()); diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java index 480387564..0e1a18116 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java @@ -140,7 +140,7 @@ public void rcvsMergeValues_1() { RemoteConfigValueStore rcvs1 = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}", false); RemoteConfigValueStore rcvs2 = RemoteConfigValueStore.dataFromString("{\"b\": 123.3,\"c\": \"uio\"}", false); - rcvs1.mergeValues(rcvs2.values, false); + rcvs1.mergeValuesToBeRemoved(rcvs2.values, false); Assert.assertEquals(3, rcvs1.values.length()); 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 89a09121b..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; @@ -478,16 +484,23 @@ public synchronized CountlyConfig setPushIntentAddMetadata(boolean enable) { * @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/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index fba5a9dfa..ad7b54ae6 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -3,6 +3,7 @@ import android.text.TextUtils; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -20,10 +21,11 @@ public class ModuleRemoteConfig extends ModuleBase { RemoteConfig remoteConfigInterface = null; //if set to true, it will automatically download remote configs on module startup - boolean remoteConfigAutomaticUpdateEnabled = false; + boolean automaticDownloadTriggersEnabled = false; boolean remoteConfigValuesShouldBeCached = false; - RemoteConfigCallback remoteConfigInitCallback = null; + + List downloadCallbacks = new ArrayList<>(2); public final static String variantObjectNameKey = "name"; @@ -37,14 +39,13 @@ public class ModuleRemoteConfig extends ModuleBase { metricOverride = config.metricOverride; immediateRequestGenerator = 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 download will be enabled, " + config.enableRemoteConfigAutomaticDownloadTriggers); + automaticDownloadTriggersEnabled = config.enableRemoteConfigAutomaticDownloadTriggers; - 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(); @@ -55,35 +56,32 @@ 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, final boolean useLegacyAPI, @Nullable final RemoteConfigCallback callback) { - try { - L.d("[ModuleRemoteConfig] Updating remote config values, requestShouldBeDelayed:[" + requestShouldBeDelayed + "], legacyAPI:[" + useLegacyAPI + "]"); + 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].length() == 0 && 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 = RemoteConfigHelper.prepareKeysIncludeExclude(keysOnly, keysExcept, L); String requestData; if (useLegacyAPI) { @@ -96,36 +94,32 @@ void updateRemoteConfigValues(@Nullable final String[] keysOnly, @Nullable final ConnectionProcessor cp = requestQueueProvider.createConnectionProcessor(); final boolean networkingIsEnabled = cp.configProvider_.getNetworkingEnabled(); - (new ImmediateRequestMaker()).doWork(requestData, "/o/sdk", cp, requestShouldBeDelayed, networkingIsEnabled, new ImmediateRequestMaker.InternalImmediateRequestCallback() { + (new ImmediateRequestMaker()).doWork(requestData, "/o/sdk", cp, false, 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"); - } + NotifyDownloadCallbacks(devProvidedCallback, RequestResult.Error, "Encountered problem while trying to reach the server, possibly no internet connection", fullUpdate, null); return; } String error = null; + Map newRC = RemoteConfigHelper.DownloadedValuesIntoMap(checkResponse); + try { boolean clearOldValues = keysExcept == null && keysOnly == null; - mergeCheckResponseIntoCurrentValues(clearOldValues, checkResponse); + 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() + "]"; } - if (callback != null) { - callback.callback(error); - } + 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); } } @@ -313,12 +307,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, 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(); - rcvs.mergeValues(checkResponse, clearOldValues); + rcvs.mergeValues(newRC, clearOldValues); L.d("[ModuleRemoteConfig] Finished remote config processing, starting saving"); @@ -413,18 +407,23 @@ void clearValueStoreInternal() { * @return */ @NonNull String[] testingGetVariantsForKeyInternal(@NonNull String key) { + String[] variantResponse = null; if (variantContainer.containsKey(key)) { - return variantContainer.get(key); + variantResponse = variantContainer.get(key); + } + + if (variantResponse == null) { + variantResponse = new String[0]; } - return new String[0]; + return variantResponse; } void clearAndDownloadAfterIdChange() { L.v("[RemoteConfig] Clearing remote config values and preparing to download after ID update"); CacheOrClearRCValuesIfNeeded(); - if (remoteConfigAutomaticUpdateEnabled && consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (automaticDownloadTriggersEnabled && consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { updateRemoteConfigAfterIdChange = true; } } @@ -433,14 +432,24 @@ void CacheOrClearRCValuesIfNeeded() { clearValueStoreInternal(); } + void NotifyDownloadCallbacks(RCDownloadCallback devProvidedCallback, RequestResult requestResult, String message, boolean fullUpdate, Map downloadedValues) { + for (RCDownloadCallback callback : downloadCallbacks) { + callback.callback(requestResult, message, fullUpdate, downloadedValues); + } + + if (devProvidedCallback != null) { + devProvidedCallback.callback(requestResult, message, fullUpdate, downloadedValues); + } + } + void RCAutomaticDownloadTrigger(boolean cacheClearOldValues) { if (cacheClearOldValues) { clearValueStoreInternal();//todo finish } - if (remoteConfigAutomaticUpdateEnabled && consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { + if (automaticDownloadTriggersEnabled && consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { L.d("[RemoteConfig] Automatically updating remote config values"); - updateRemoteConfigValues(null, null, false, false, remoteConfigInitCallback); + updateRemoteConfigValues(null, null, false, null); } else { L.v("[RemoteConfig] Automatically RC update trigger skipped"); } @@ -558,7 +567,14 @@ public void updateExceptKeys(String[] keysToExclude, RemoteConfigCallback callba if (keysToExclude == null) { L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to ignore' array is null"); } - updateRemoteConfigValues(null, keysToExclude, false, true, callback); + + RCDownloadCallback innerCall = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + if (callback != null) { + callback.callback(error); + } + }; + + updateRemoteConfigValues(null, keysToExclude, true, innerCall); } } @@ -581,7 +597,14 @@ public void updateForKeysOnly(String[] keysToInclude, RemoteConfigCallback callb if (keysToInclude == null) { L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to include' array is null"); } - updateRemoteConfigValues(keysToInclude, null, false, true, callback); + + RCDownloadCallback innerCall = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + if (callback != null) { + callback.callback(error); + } + }; + + updateRemoteConfigValues(keysToInclude, null, true, innerCall); } } @@ -602,7 +625,13 @@ public void update(RemoteConfigCallback callback) { return; } - updateRemoteConfigValues(null, null, false, true, callback); + RCDownloadCallback innerCall = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + if (callback != null) { + callback.callback(error); + } + }; + + updateRemoteConfigValues(null, null, true, innerCall); } } @@ -625,7 +654,13 @@ public void DownloadOmittingKeys(String[] keysToOmit, RCDownloadCallback callbac if (keysToOmit == null) { L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to ignore' array is null"); } - updateRemoteConfigValues(null, keysToOmit, false, false, null); // TODO: this callback was not expected + + if (callback == null) { + callback = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + }; + } + + updateRemoteConfigValues(null, keysToOmit, false, callback); } } @@ -647,7 +682,13 @@ public void DownloadSpecificKeys(String[] keysToInclude, RCDownloadCallback call if (keysToInclude == null) { L.w("[RemoteConfig] updateRemoteConfigExceptKeys passed 'keys to include' array is null"); } - updateRemoteConfigValues(keysToInclude, null, false, false, null); // TODO: this callback was not expected + + if (callback == null) { + callback = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + }; + } + + updateRemoteConfigValues(keysToInclude, null, false, callback); } } @@ -662,7 +703,12 @@ public void DownloadAllKeys(RCDownloadCallback callback) { return; } - updateRemoteConfigValues(null, null, false, false, null); // TODO: this callback was not expected + if (callback == null) { + callback = (downloadResult, error, fullValueUpdate, downloadedValues) -> { + }; + } + + updateRemoteConfigValues(null, null, false, null); } } diff --git a/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java b/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java index 6781c5b4d..b9d2cc3b3 100644 --- a/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java +++ b/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java @@ -1,7 +1,8 @@ package ly.count.android.sdk; +import java.util.Map; import org.json.JSONObject; interface RCDownloadCallback { - void callback(RequestResult downloadResult, String error, boolean fullValueUpdate, JSONObject downloadedValues); + void callback(RequestResult downloadResult, String error, boolean fullValueUpdate, Map downloadedValues); } 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 index 47d3318b8..9d276e74d 100644 --- a/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigHelper.java +++ b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigHelper.java @@ -4,14 +4,38 @@ 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 org.json.JSONArray; import org.json.JSONObject; public class RemoteConfigHelper { + + public static @NonNull Map DownloadedValuesIntoMap(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, value); + } 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 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 index 7cdcab9f8..533516180 100644 --- a/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java +++ b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java @@ -27,13 +27,17 @@ public void clearValues() { values = new JSONObject(); } + public void mergeValues(Map newValues, boolean fullUpdate) { + //todo must be finished + } + /** * add new values to the current storage * * @param newValues * @param fullUpdate clear all previous values in case of full update */ - public void mergeValues(JSONObject newValues, boolean fullUpdate) { + public void mergeValuesToBeRemoved(JSONObject newValues, boolean fullUpdate) { if (fullUpdate) { clearValues(); } From 125aacd8c457c67539240b06aee8d6e9c9a02f66 Mon Sep 17 00:00:00 2001 From: turtledreams Date: Fri, 9 Jun 2023 22:51:34 +0900 Subject: [PATCH 16/30] 1 test --- .../sdk/RemoteConfigValueStoreTests.java | 23 ++- .../count/android/sdk/ModuleRemoteConfig.java | 8 +- .../java/ly/count/android/sdk/RCData.java | 4 +- .../sdk/internal/RemoteConfigValueStore.java | 132 ++++++++++++++++-- 4 files changed, 152 insertions(+), 15 deletions(-) diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java index 0e1a18116..c5181bfd9 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java @@ -2,6 +2,7 @@ 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; @@ -133,10 +134,10 @@ public void rcvsDataFromStringSamples_2() throws JSONException { } /** - * Simple test for value merging + * Simple test for value merging (legacy) */ @Test - public void rcvsMergeValues_1() { + public void rcvsMergeValues_1_legacy() { RemoteConfigValueStore rcvs1 = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}", false); RemoteConfigValueStore rcvs2 = RemoteConfigValueStore.dataFromString("{\"b\": 123.3,\"c\": \"uio\"}", false); @@ -148,4 +149,22 @@ public void rcvsMergeValues_1() { Assert.assertEquals(123.3, rcvs1.getValueLegacy("b")); Assert.assertEquals("uio", rcvs1.getValueLegacy("c")); } + + /** + * Simple test for value merging + */ + @Test + public void rcvsMergeValues_1() throws JSONException { + RemoteConfigValueStore rcvs1 = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}", false); + JSONObject obj = new JSONObject("{\"b\": 123.3,\"c\": \"uio\"}"); + + Map newRC = RemoteConfigHelper.DownloadedValuesIntoMap(obj); + rcvs1.mergeValues(newRC, false); + + Assert.assertEquals(3, rcvs1.values.length()); + + Assert.assertEquals(123, rcvs1.getValue("a").value); + Assert.assertEquals(123.3, rcvs1.getValue("b").value); + Assert.assertEquals("uio", rcvs1.getValue("c").value); + } } diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index ad7b54ae6..43d2a5f83 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -443,8 +443,12 @@ void NotifyDownloadCallbacks(RCDownloadCallback devProvidedCallback, RequestResu } void RCAutomaticDownloadTrigger(boolean cacheClearOldValues) { - if (cacheClearOldValues) { + if (!cacheClearOldValues) { clearValueStoreInternal();//todo finish + } else { + RemoteConfigValueStore rc = loadConfig(); + rc.cacheClearValues(); + saveConfig(rc); } if (automaticDownloadTriggersEnabled && consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { @@ -708,7 +712,7 @@ public void DownloadAllKeys(RCDownloadCallback callback) { }; } - updateRemoteConfigValues(null, null, false, null); + updateRemoteConfigValues(null, null, false, callback); } } diff --git a/sdk/src/main/java/ly/count/android/sdk/RCData.java b/sdk/src/main/java/ly/count/android/sdk/RCData.java index 3097dfcd8..f8a1a6fdf 100644 --- a/sdk/src/main/java/ly/count/android/sdk/RCData.java +++ b/sdk/src/main/java/ly/count/android/sdk/RCData.java @@ -2,9 +2,9 @@ public class RCData { public Object value; - boolean isCurrentUsersData; + public boolean isCurrentUsersData; - protected RCData(Object givenValue, boolean givenUserState) { + public RCData(Object givenValue, boolean givenUserState) { this.value = givenValue; this.isCurrentUsersData = givenUserState; } 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 index 533516180..161327e8a 100644 --- a/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java +++ b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java @@ -11,8 +11,24 @@ public class RemoteConfigValueStore { public JSONObject values = new JSONObject(); - public boolean valuesCanBeCached = false; + static final String keyValue = "v"; + static final String keyCacheFlag = "c"; + static final int cacheValCached = 0; + static final int cacheValFresh = 1; + public boolean dirty = false; + + // Structure of the JSON objects we will have + // { + // “key”: { + // “v”: “value”, + // “c”: 0 + // } + // } + + //======================================== + // CLEANSING + //======================================== public void cacheClearValues() { if (!valuesCanBeCached) { @@ -20,15 +36,54 @@ public void cacheClearValues() { return; } - // values can be cached, do that + Iterator iter = values.keys(); + while (iter.hasNext()) { + String key = iter.next(); + try { + JSONObject value = values.getJSONObject(key); + if (value != null) { + value.put(keyCacheFlag, cacheValCached); + values.put(key, value); + } + } catch (Exception e) { + Countly.sharedInstance().L.e("[RemoteConfigValueStore] Failed caching remote config values"); + } + } + dirty = true; } public void clearValues() { values = new JSONObject(); + dirty = true; } + //======================================== + // MERGING + //======================================== + public void mergeValues(Map newValues, boolean fullUpdate) { - //todo must be finished + Countly.sharedInstance().L.i("[RemoteConfigValueStore] mergeValues, stored values:" + values.toString()); + if (newValues != null) { + Countly.sharedInstance().L.i("[RemoteConfigValueStore] mergeValues, provided values:" + newValues.toString()); + } + if (fullUpdate) { + clearValues(); + } + + for (Map.Entry entry : newValues.entrySet()) { + String key = entry.getKey(); + Object newValue = entry.getValue(); + 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"); + } + } + dirty = true; + Countly.sharedInstance().L.i("[RemoteConfigValueStore] merging done:" + values.toString()); } /** @@ -58,21 +113,59 @@ public void mergeValuesToBeRemoved(JSONObject newValues, boolean fullUpdate) { } } + //======================================== + // CONSTRUCTION + //======================================== + private RemoteConfigValueStore(JSONObject values, boolean valuesShouldBeCached) { this.values = values; this.valuesCanBeCached = valuesShouldBeCached; } - public Object getValueLegacy(String key) { - return values.opt(key); - } + //======================================== + // GET VALUES + //======================================== public @NonNull RCData getValue(String key) { - return null; + 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() { - return null; + 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); + Integer 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(String key) { + return values.opt(key); } public Map getAllValuesLegacy() { @@ -86,13 +179,17 @@ public Map getAllValuesLegacy() { try { ret.put(key, values.get(key)); } catch (Exception ex) { - Countly.sharedInstance().L.e("[RemoteConfigValueStore] Got JSON exception while calling 'getAllValues': " + ex.toString()); + Countly.sharedInstance().L.e("[RemoteConfigValueStore] Got JSON exception while calling 'getAllValuesLegacy': " + ex.toString()); } } return ret; } + //======================================== + // SERIALIZATION + //======================================== + public static RemoteConfigValueStore dataFromString(String storageString, boolean valuesShouldBeCached) { if (storageString == null || storageString.isEmpty()) { return new RemoteConfigValueStore(new JSONObject(), valuesShouldBeCached); @@ -101,10 +198,27 @@ public static RemoteConfigValueStore dataFromString(String storageString, boolea JSONObject values; try { values = new JSONObject(storageString); + //iterate through all values and check if each value an instance of json object. and if it isnt convert to new data + Iterator iter = values.keys(); + while (iter.hasNext()) { + String key = iter.next(); + try { + JSONObject value = values.optJSONObject(key); + if (value == null) { + value = new JSONObject(); + value.put(keyValue, values.get(key)); + value.put(keyCacheFlag, cacheValFresh); + values.put(key, value); + } + } catch (Exception e) { + Countly.sharedInstance().L.e("[RemoteConfigValueStore] Failed caching remote config values, dataFromString:" + e.toString()); + } + } } 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); } From 84b38eeb3ffcf8464dad0e99e4c51d41ec67329a Mon Sep 17 00:00:00 2001 From: ArtursK Date: Mon, 12 Jun 2023 11:50:13 +0300 Subject: [PATCH 17/30] massaging RCVS --- .../sdk/RemoteConfigValueStoreTests.java | 19 +---- .../count/android/sdk/ModuleRemoteConfig.java | 4 +- .../sdk/internal/RemoteConfigHelper.java | 2 +- .../sdk/internal/RemoteConfigValueStore.java | 70 +++++++------------ 4 files changed, 29 insertions(+), 66 deletions(-) diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java index c5181bfd9..10e373fef 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java @@ -133,23 +133,6 @@ public void rcvsDataFromStringSamples_2() throws JSONException { Assert.assertEquals("op", jObj2.get("w")); } - /** - * Simple test for value merging (legacy) - */ - @Test - public void rcvsMergeValues_1_legacy() { - RemoteConfigValueStore rcvs1 = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}", false); - RemoteConfigValueStore rcvs2 = RemoteConfigValueStore.dataFromString("{\"b\": 123.3,\"c\": \"uio\"}", false); - - rcvs1.mergeValuesToBeRemoved(rcvs2.values, false); - - Assert.assertEquals(3, rcvs1.values.length()); - - Assert.assertEquals(123, rcvs1.getValueLegacy("a")); - Assert.assertEquals(123.3, rcvs1.getValueLegacy("b")); - Assert.assertEquals("uio", rcvs1.getValueLegacy("c")); - } - /** * Simple test for value merging */ @@ -167,4 +150,6 @@ public void rcvsMergeValues_1() throws JSONException { Assert.assertEquals(123.3, rcvs1.getValue("b").value); Assert.assertEquals("uio", rcvs1.getValue("c").value); } + + //todo: test for explicit data migration from the old thing to the new thing } diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 43d2a5f83..6ce2f0ff8 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -96,7 +96,7 @@ void updateRemoteConfigValues(@Nullable final String[] keysOnly, @Nullable final (new ImmediateRequestMaker()).doWork(requestData, "/o/sdk", cp, false, networkingIsEnabled, new ImmediateRequestMaker.InternalImmediateRequestCallback() { @Override - public void callback(JSONObject checkResponse) { + public void callback(@Nullable JSONObject 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); @@ -307,7 +307,7 @@ public void callback(JSONObject checkResponse) { * * @throws Exception it throws an exception so that it is escalated upwards */ - void mergeCheckResponseIntoCurrentValues(boolean clearOldValues, Map newRC) { + 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 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 index 9d276e74d..1dd8a7bac 100644 --- a/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigHelper.java +++ b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigHelper.java @@ -15,7 +15,7 @@ public class RemoteConfigHelper { - public static @NonNull Map DownloadedValuesIntoMap(JSONObject jsonObject) { + public static @NonNull Map DownloadedValuesIntoMap(@Nullable JSONObject jsonObject) { Map ret = new HashMap<>(); if (jsonObject == null) { 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 index 161327e8a..59dc6ab45 100644 --- a/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java +++ b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java @@ -39,14 +39,19 @@ public void cacheClearValues() { 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 { - JSONObject value = values.getJSONObject(key); - if (value != null) { - value.put(keyCacheFlag, cacheValCached); - values.put(key, value); - } + value.put(keyCacheFlag, cacheValCached); + values.put(key, value); } catch (Exception e) { - Countly.sharedInstance().L.e("[RemoteConfigValueStore] Failed caching remote config values"); + Countly.sharedInstance().L.e("[RemoteConfigValueStore] cacheClearValues, Failed caching remote config values, " + e); } } dirty = true; @@ -61,11 +66,10 @@ public void clearValues() { // MERGING //======================================== - public void mergeValues(Map newValues, boolean fullUpdate) { - Countly.sharedInstance().L.i("[RemoteConfigValueStore] mergeValues, stored values:" + values.toString()); - if (newValues != null) { - Countly.sharedInstance().L.i("[RemoteConfigValueStore] mergeValues, provided values:" + newValues.toString()); - } + 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(); } @@ -83,34 +87,7 @@ public void mergeValues(Map newValues, boolean fullUpdate) { } } dirty = true; - Countly.sharedInstance().L.i("[RemoteConfigValueStore] merging done:" + values.toString()); - } - - /** - * add new values to the current storage - * - * @param newValues - * @param fullUpdate clear all previous values in case of full update - */ - public void mergeValuesToBeRemoved(JSONObject newValues, boolean fullUpdate) { - if (fullUpdate) { - clearValues(); - } - - 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"); - } - } + Countly.sharedInstance().L.v("[RemoteConfigValueStore] merging done:" + values.toString()); } //======================================== @@ -154,7 +131,7 @@ private RemoteConfigValueStore(JSONObject values, boolean valuesShouldBeCached) continue; } Object rcObjVal = rcObj.opt(keyValue); - Integer rcObjCache = rcObj.getInt(keyCacheFlag); + 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()); @@ -204,12 +181,13 @@ public static RemoteConfigValueStore dataFromString(String storageString, boolea String key = iter.next(); try { JSONObject value = values.optJSONObject(key); - if (value == null) { - value = new JSONObject(); - value.put(keyValue, values.get(key)); - value.put(keyCacheFlag, cacheValFresh); - values.put(key, value); + if (value != null) { + continue; } + value = new JSONObject(); + value.put(keyValue, values.get(key)); + value.put(keyCacheFlag, cacheValFresh); + values.put(key, value); } catch (Exception e) { Countly.sharedInstance().L.e("[RemoteConfigValueStore] Failed caching remote config values, dataFromString:" + e.toString()); } @@ -218,7 +196,7 @@ public static RemoteConfigValueStore dataFromString(String storageString, boolea 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()); + //Countly.sharedInstance().L.i("[RemoteConfigValueStore] serialization done, dataFromString:" + values.toString()); return new RemoteConfigValueStore(values, valuesShouldBeCached); } From 1d504ad26855c16f8a860c7555c168c862d58155 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Mon, 12 Jun 2023 15:38:35 +0300 Subject: [PATCH 18/30] Adding remote config value store tests. Fixing issues with rcvs. --- .../sdk/RemoteConfigValueStoreTests.java | 100 ++++++++++++++++-- .../count/android/sdk/ModuleRemoteConfig.java | 8 +- .../sdk/internal/RemoteConfigValueStore.java | 47 +++++--- 3 files changed, 129 insertions(+), 26 deletions(-) diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java index 10e373fef..be04dcf31 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java @@ -138,17 +138,105 @@ public void rcvsDataFromStringSamples_2() throws JSONException { */ @Test public void rcvsMergeValues_1() throws JSONException { - RemoteConfigValueStore rcvs1 = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}", false); + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}", false); JSONObject obj = new JSONObject("{\"b\": 123.3,\"c\": \"uio\"}"); Map newRC = RemoteConfigHelper.DownloadedValuesIntoMap(obj); - rcvs1.mergeValues(newRC, false); + 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); + } + + @Test + public void dataFromString_LegacyStructure() { + String input = "{" + entryL("a", 123) + "," + entryL("b", "fg") + "}"; + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(input, false); + + 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); + } + + @Test + public void dataFromString_CurrentStructure() { + String input = "{" + entryC("a", 123, true) + "," + entryC("b", "ccx", false) + "}"; + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(input, 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); + } + + @Test + public void dataFromString_MixedStructure() { + String input = "{" + entryL("a", 123) + "," + entryC("b", "ccx", false) + "}"; + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(input, 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); + } + + /** + * Create a legacy entry + * + * @param key + * @param value + * @return + */ + String entryL(String key, Object value) { + StringBuilder ret = new StringBuilder(); + ret.append("\"" + key + "\":"); + + if (value instanceof String) { + ret.append("\""); + ret.append(value); + ret.append("\""); + } else { + ret.append(value); + } + + return ret.toString(); + } + + /** + * Create a current data format entry + * + * @param key + * @param value + * @return + */ + String entryC(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("\"").append(value).append("\""); + } else { + ret.append(value); + } + + ret.append(",\""); + ret.append(RemoteConfigValueStore.keyCacheFlag); + ret.append("\":"); + + ret.append(isCurrentUser ? RemoteConfigValueStore.cacheValFresh : RemoteConfigValueStore.cacheValCached); - Assert.assertEquals(3, rcvs1.values.length()); + ret.append("}"); - Assert.assertEquals(123, rcvs1.getValue("a").value); - Assert.assertEquals(123.3, rcvs1.getValue("b").value); - Assert.assertEquals("uio", rcvs1.getValue("c").value); + return ret.toString(); } //todo: test for explicit data migration from the old thing to the new thing diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 6ce2f0ff8..32f891222 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -506,7 +506,7 @@ public class RemoteConfig { /** * Clear all stored remote config_ values * - * @deprecated + * @deprecated Use "clearAll" */ public void clearStoredValues() { synchronized (_cly) { @@ -784,15 +784,15 @@ public void exitABTestsForKeys(String[] keys) { } public void registerDownloadCallback(RCDownloadCallback callback) { - + downloadCallbacks.add(callback); } public void removeDownloadCallback(RCDownloadCallback callback) { - + downloadCallbacks.remove(callback); } public void clearAll() { - + clearStoredValues(); } /** 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 index 59dc6ab45..20a4ea3d2 100644 --- a/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java +++ b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java @@ -1,6 +1,7 @@ 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; @@ -10,12 +11,12 @@ import org.json.JSONObject; public class RemoteConfigValueStore { - public JSONObject values = new JSONObject(); - public boolean valuesCanBeCached = false; - static final String keyValue = "v"; - static final String keyCacheFlag = "c"; - static final int cacheValCached = 0; - static final int cacheValFresh = 1; + 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; public boolean dirty = false; // Structure of the JSON objects we will have @@ -94,7 +95,7 @@ public void mergeValues(@NonNull Map newValues, boolean fullUpda // CONSTRUCTION //======================================== - private RemoteConfigValueStore(JSONObject values, boolean valuesShouldBeCached) { + private RemoteConfigValueStore(@NonNull JSONObject values, boolean valuesShouldBeCached) { this.values = values; this.valuesCanBeCached = valuesShouldBeCached; } @@ -103,7 +104,7 @@ private RemoteConfigValueStore(JSONObject values, boolean valuesShouldBeCached) // GET VALUES //======================================== - public @NonNull RCData getValue(String key) { + public @NonNull RCData getValue(@NonNull String key) { RCData res = new RCData(null, true); try { JSONObject rcObj = values.optJSONObject(key); @@ -141,11 +142,16 @@ private RemoteConfigValueStore(JSONObject values, boolean valuesShouldBeCached) return ret; } - public Object getValueLegacy(String key) { - return values.opt(key); + public Object getValueLegacy(@NonNull String key) { + JSONObject rcObj = values.optJSONObject(key); + if (rcObj == null) { + return null; + } + + return rcObj.opt(keyValue); } - public Map getAllValuesLegacy() { + public @NonNull Map getAllValuesLegacy() { Map ret = new HashMap<>(); Iterator keys = values.keys(); @@ -153,11 +159,20 @@ public Map getAllValuesLegacy() { 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 'getAllValuesLegacy': " + ex.toString()); + 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; @@ -167,7 +182,7 @@ public Map getAllValuesLegacy() { // SERIALIZATION //======================================== - public static RemoteConfigValueStore dataFromString(String storageString, boolean valuesShouldBeCached) { + public static RemoteConfigValueStore dataFromString(@Nullable String storageString, boolean valuesShouldBeCached) { if (storageString == null || storageString.isEmpty()) { return new RemoteConfigValueStore(new JSONObject(), valuesShouldBeCached); } From 590cbdcaa14b42312b18caf4d2188c352ee09b5f Mon Sep 17 00:00:00 2001 From: ArtursK Date: Mon, 12 Jun 2023 23:00:23 +0300 Subject: [PATCH 19/30] Refactoring RC tests. Moving RC migartion to migration class --- gradle.properties | 2 +- .../android/sdk/ConnectionQueueTests.java | 2 +- .../android/sdk/MigrationHelperTests.java | 139 ++++++++++++++---- .../sdk/RemoteConfigValueStoreTests.java | 120 +++++++-------- .../java/ly/count/android/sdk/Countly.java | 2 +- .../ly/count/android/sdk/MigrationHelper.java | 55 ++++++- .../sdk/internal/RemoteConfigValueStore.java | 19 +-- 7 files changed, 224 insertions(+), 115 deletions(-) diff --git a/gradle.properties b/gradle.properties index 0a30af9c6..2b3b2e119 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.02.0-alpha-2 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/src/androidTest/java/ly/count/android/sdk/ConnectionQueueTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ConnectionQueueTests.java index ffdae84f8..7bfd4ca90 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.02.0-alpha-2")); } else if (a == 1) { Assert.assertTrue(pair[1].equals("123sdf.v-213")); } 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/RemoteConfigValueStoreTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java index be04dcf31..de0d71b58 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java @@ -62,13 +62,18 @@ public void rcvsDataFromStringNullEmpty() { */ @Test public void rcvsDataFromStringSamples_1() { - RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}", false); + 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); } /** @@ -79,13 +84,14 @@ public void rcvsDataFromStringSamples_1() { */ @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\"}}"; - RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(initialString, false); + 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); - Assert.assertEquals(initialString, rcvs.dataToString());//quickly validate deserialization - //validate values while using "get" Assert.assertEquals(5, rcvs.values.length()); @@ -133,12 +139,25 @@ public void rcvsDataFromStringSamples_2() throws JSONException { 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 { - RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString("{\"a\": 123,\"b\": \"fg\"}", false); + 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); @@ -151,79 +170,27 @@ public void rcvsMergeValues_1() throws JSONException { Assert.assertEquals("uio", rcvs.getValue("c").value); } - @Test - public void dataFromString_LegacyStructure() { - String input = "{" + entryL("a", 123) + "," + entryL("b", "fg") + "}"; - RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(input, false); - - 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); - } - - @Test - public void dataFromString_CurrentStructure() { - String input = "{" + entryC("a", 123, true) + "," + entryC("b", "ccx", false) + "}"; - RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(input, 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); - } - - @Test - public void dataFromString_MixedStructure() { - String input = "{" + entryL("a", 123) + "," + entryC("b", "ccx", false) + "}"; - RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(input, 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); - } - /** - * Create a legacy entry + * Create a remote config entry string * * @param key * @param value * @return */ - String entryL(String key, Object value) { + public static String rcEStr(String key, Object value, boolean isCurrentUser) { StringBuilder ret = new StringBuilder(); - ret.append("\"" + key + "\":"); + ret.append("\"").append(key).append("\":{\""); + ret.append(RemoteConfigValueStore.keyValue); + ret.append("\":"); if (value instanceof String) { ret.append("\""); ret.append(value); ret.append("\""); - } else { + } else if (value instanceof JSONArray) { + ret.append(value); + } else if (value instanceof JSONObject) { ret.append(value); - } - - return ret.toString(); - } - - /** - * Create a current data format entry - * - * @param key - * @param value - * @return - */ - String entryC(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("\"").append(value).append("\""); } else { ret.append(value); } @@ -239,5 +206,24 @@ String entryC(String key, Object value, boolean isCurrentUser) { return ret.toString(); } - //todo: test for explicit data migration from the old thing to the new thing + public static String rcEStr(String key, Object value) { + return rcEStr(key, value, true); + } + + 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/main/java/ly/count/android/sdk/Countly.java b/sdk/src/main/java/ly/count/android/sdk/Countly.java index f89358afb..1d757a913 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.02.0-alpha-2"; /** * Used as request meta data on every request 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..1fa0edd8d 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; + 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/internal/RemoteConfigValueStore.java b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java index 20a4ea3d2..96d679885 100644 --- a/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java +++ b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java @@ -179,7 +179,7 @@ public Object getValueLegacy(@NonNull String key) { } //======================================== - // SERIALIZATION + // SERIALIZATION, DESERIALIZATION //======================================== public static RemoteConfigValueStore dataFromString(@Nullable String storageString, boolean valuesShouldBeCached) { @@ -190,23 +190,6 @@ public static RemoteConfigValueStore dataFromString(@Nullable String storageStri JSONObject values; try { values = new JSONObject(storageString); - //iterate through all values and check if each value an instance of json object. and if it isnt convert to new data - Iterator iter = values.keys(); - while (iter.hasNext()) { - String key = iter.next(); - try { - JSONObject value = values.optJSONObject(key); - if (value != null) { - continue; - } - value = new JSONObject(); - value.put(keyValue, values.get(key)); - value.put(keyCacheFlag, cacheValFresh); - values.put(key, value); - } catch (Exception e) { - Countly.sharedInstance().L.e("[RemoteConfigValueStore] Failed caching remote config values, dataFromString:" + e.toString()); - } - } } catch (JSONException e) { Countly.sharedInstance().L.e("[RemoteConfigValueStore] Couldn't decode RemoteConfigValueStore successfully: " + e.toString()); values = new JSONObject(); From dc64b6b5be2f2b1fba756390a05614d067995fd2 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Tue, 13 Jun 2023 09:12:10 +0300 Subject: [PATCH 20/30] Making callback public --- .../ly/count/android/demo/ActivityExampleRemoteConfig.java | 7 ++++--- .../main/java/ly/count/android/sdk/RCDownloadCallback.java | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) 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..794fc005d 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,8 @@ import android.widget.Toast; import androidx.appcompat.app.AppCompatActivity; +import ly.count.android.sdk.RCDownloadCallback; +import ly.count.android.sdk.RequestResult; import org.json.JSONArray; import org.json.JSONObject; @@ -24,9 +26,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/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java b/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java index b9d2cc3b3..99cd485dc 100644 --- a/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java +++ b/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java @@ -3,6 +3,6 @@ import java.util.Map; import org.json.JSONObject; -interface RCDownloadCallback { +public interface RCDownloadCallback { void callback(RequestResult downloadResult, String error, boolean fullValueUpdate, Map downloadedValues); } From c65a13bd70c42d1b34e19d2c7c606255f17631a3 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Tue, 13 Jun 2023 09:15:23 +0300 Subject: [PATCH 21/30] fixing check --- sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 32f891222..a0873f17e 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -62,7 +62,7 @@ void updateRemoteConfigValues(@Nullable final String[] keysOnly, @Nullable final L.d("[ModuleRemoteConfig] Updating remote config values, legacyAPI:[" + useLegacyAPI + "]"); String[] preparedKeys = RemoteConfigHelper.prepareKeysIncludeExclude(keysOnly, keysExcept, L); - boolean fullUpdate = preparedKeys[0].length() == 0 && preparedKeys[1].length() == 0; + boolean fullUpdate = (preparedKeys[0] == null || preparedKeys[0].length() == 0) && (preparedKeys[1] == null || preparedKeys[1].length() == 0); try { // checks From e2634fceb8940acc3454fc3b3d4ebaf34664bc19 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Tue, 13 Jun 2023 15:17:01 +0300 Subject: [PATCH 22/30] Changing the response callback to include RCData --- .../demo/ActivityExampleRemoteConfig.java | 3 ++- gradle.properties | 2 +- .../android/sdk/ConnectionQueueTests.java | 2 +- .../sdk/RemoteConfigValueStoreTests.java | 2 +- .../java/ly/count/android/sdk/Countly.java | 2 +- .../count/android/sdk/ModuleRemoteConfig.java | 26 +++++++++++++------ .../count/android/sdk/RCDownloadCallback.java | 2 +- .../sdk/internal/RemoteConfigHelper.java | 7 ++--- .../sdk/internal/RemoteConfigValueStore.java | 6 ++--- 9 files changed, 32 insertions(+), 20 deletions(-) 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 794fc005d..898c747e1 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,7 @@ 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; @@ -27,7 +28,7 @@ public void onCreate(Bundle savedInstanceState) { public void onClickRemoteConfigUpdate(View v) { Countly.sharedInstance().remoteConfig().DownloadAllKeys(new RCDownloadCallback() { - @Override public void callback(RequestResult downloadResult, String error, boolean fullValueUpdate, Map downloadedValues) { + @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/gradle.properties b/gradle.properties index 2b3b2e119..2f8abd3d4 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=23.02.0-alpha-2 +VERSION_NAME=23.02.0-alpha-4 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/src/androidTest/java/ly/count/android/sdk/ConnectionQueueTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ConnectionQueueTests.java index 7bfd4ca90..7f1ca0694 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("23.02.0-alpha-2")); + Assert.assertTrue(pair[1].equals("23.02.0-alpha-4")); } else if (a == 1) { Assert.assertTrue(pair[1].equals("123sdf.v-213")); } diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java index de0d71b58..9570d34d9 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java @@ -160,7 +160,7 @@ public void rcvsMergeValues_1() throws JSONException { RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), false); JSONObject obj = new JSONObject("{\"b\": 123.3,\"c\": \"uio\"}"); - Map newRC = RemoteConfigHelper.DownloadedValuesIntoMap(obj); + Map newRC = RemoteConfigHelper.DownloadedValuesIntoMap(obj); rcvs.mergeValues(newRC, false); Assert.assertEquals(3, rcvs.values.length()); 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 1d757a913..558348552 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 = "23.02.0-alpha-2"; + private final String DEFAULT_COUNTLY_SDK_VERSION_STRING = "23.02.0-alpha-4"; /** * Used as request meta data on every request diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index a0873f17e..12fea011c 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -104,7 +104,7 @@ public void callback(@Nullable JSONObject checkResponse) { } String error = null; - Map newRC = RemoteConfigHelper.DownloadedValuesIntoMap(checkResponse); + Map newRC = RemoteConfigHelper.DownloadedValuesIntoMap(checkResponse); try { boolean clearOldValues = keysExcept == null && keysOnly == null; @@ -307,7 +307,7 @@ public void callback(JSONObject checkResponse) { * * @throws Exception it throws an exception so that it is escalated upwards */ - void mergeCheckResponseIntoCurrentValues(boolean clearOldValues, @NonNull Map newRC) { + 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 @@ -342,12 +342,22 @@ boolean isResponseValid(@NonNull JSONObject responseJson) { return result; } - Object getValue(@NonNull String key) { + RCData getRCValue(@NonNull String key) { try { RemoteConfigValueStore rcvs = loadConfig(); - return rcvs.getValueLegacy(key); + return rcvs.getValue(key); } catch (Exception ex) { L.e("[ModuleRemoteConfig] getValue, Call failed:[" + ex.toString() + "]"); + return new RCData(null, true); + } + } + + Object getRCValueLegacy(@NonNull String key) { + try { + RemoteConfigValueStore rcvs = loadConfig(); + return rcvs.getValueLegacy(key); + } catch (Exception ex) { + L.e("[ModuleRemoteConfig] getValueLegacy, Call failed:[" + ex.toString() + "]"); return null; } } @@ -432,7 +442,7 @@ void CacheOrClearRCValuesIfNeeded() { clearValueStoreInternal(); } - void NotifyDownloadCallbacks(RCDownloadCallback devProvidedCallback, RequestResult requestResult, String message, boolean fullUpdate, Map downloadedValues) { + void NotifyDownloadCallbacks(RCDownloadCallback devProvidedCallback, RequestResult requestResult, String message, boolean fullUpdate, Map downloadedValues) { for (RCDownloadCallback callback : downloadCallbacks) { callback.callback(requestResult, message, fullUpdate, downloadedValues); } @@ -547,7 +557,7 @@ public Object getValueForKey(String key) { return null; } - return getValue(key); + return getRCValueLegacy(key); } } @@ -724,7 +734,7 @@ public void DownloadAllKeys(RCDownloadCallback callback) { return new HashMap<>(); } - return new HashMap<>(); + return getAllRemoteConfigValuesInternal(); } } @@ -736,7 +746,7 @@ public void DownloadAllKeys(RCDownloadCallback callback) { return new RCData(null, true); } - return new RCData(null, true); + return getRCValue(key); } } diff --git a/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java b/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java index 99cd485dc..271570d60 100644 --- a/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java +++ b/sdk/src/main/java/ly/count/android/sdk/RCDownloadCallback.java @@ -4,5 +4,5 @@ import org.json.JSONObject; public interface RCDownloadCallback { - void callback(RequestResult downloadResult, String error, boolean fullValueUpdate, Map downloadedValues); + void callback(RequestResult downloadResult, String error, boolean fullValueUpdate, Map downloadedValues); } 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 index 1dd8a7bac..c71e14d5d 100644 --- a/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigHelper.java +++ b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigHelper.java @@ -10,13 +10,14 @@ 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<>(); + public static @NonNull Map DownloadedValuesIntoMap(@Nullable JSONObject jsonObject) { + Map ret = new HashMap<>(); if (jsonObject == null) { return ret; @@ -27,7 +28,7 @@ public class RemoteConfigHelper { String key = iter.next(); try { Object value = jsonObject.get(key); - ret.put(key, value); + ret.put(key, new RCData(value, true)); } catch (Exception e) { Countly.sharedInstance().L.e("[RemoteConfigValueStore] Failed merging new remote config values"); } 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 index 96d679885..a94ca3144 100644 --- a/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java +++ b/sdk/src/main/java/ly/count/android/sdk/internal/RemoteConfigValueStore.java @@ -67,7 +67,7 @@ public void clearValues() { // MERGING //======================================== - public void mergeValues(@NonNull Map newValues, boolean fullUpdate) { + 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()); @@ -75,9 +75,9 @@ public void mergeValues(@NonNull Map newValues, boolean fullUpda clearValues(); } - for (Map.Entry entry : newValues.entrySet()) { + for (Map.Entry entry : newValues.entrySet()) { String key = entry.getKey(); - Object newValue = entry.getValue(); + Object newValue = entry.getValue().value; JSONObject newObj = new JSONObject(); try { newObj.put(keyValue, newValue); From 05dc74b1d48eaa1b40a6d9a2d56544ebd4b62af0 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Tue, 20 Jun 2023 17:27:36 +0300 Subject: [PATCH 23/30] Renaming functions to the correct style. Making sure the initial RC variant state is not null. --- .../android/sdk/ConnectionQueueTests.java | 2 +- .../sdk/RemoteConfigVariantControlTests.java | 23 +++++++++-- .../java/ly/count/android/sdk/Countly.java | 2 +- .../count/android/sdk/ModuleRemoteConfig.java | 39 ++++++++++--------- .../count/android/sdk/ModuleUserProfile.java | 2 +- 5 files changed, 42 insertions(+), 26 deletions(-) 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 7f1ca0694..541179668 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("23.02.0-alpha-4")); + Assert.assertTrue(pair[1].equals("23.02.0-alpha-6")); } else if (a == 1) { Assert.assertTrue(pair[1].equals("123sdf.v-213")); } 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 8e5413471..14f4428c4 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigVariantControlTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigVariantControlTests.java @@ -186,7 +186,7 @@ public void testNormalFlow() { Countly countly = (new Countly()).init(config); // Developer did not provide a callback - countly.moduleRemoteConfig.remoteConfigInterface.TestingDownloadVariantInformation(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"); @@ -197,7 +197,7 @@ 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); } /** @@ -209,7 +209,7 @@ public void testNullVariant() { Countly countly = (new Countly()).init(config); // Developer did not provide a callback - countly.moduleRemoteConfig.remoteConfigInterface.TestingDownloadVariantInformation(null); + countly.moduleRemoteConfig.remoteConfigInterface.testingDownloadVariantInformation(null); Map values = countly.moduleRemoteConfig.remoteConfigInterface.testingGetAllVariants(); // Assert the values @@ -226,7 +226,7 @@ public void testFilteringWrongKeys() { Countly countly = (new Countly()).init(config); // Developer did not provide a callback - countly.moduleRemoteConfig.remoteConfigInterface.TestingDownloadVariantInformation(null); + countly.moduleRemoteConfig.remoteConfigInterface.testingDownloadVariantInformation(null); Map values = countly.moduleRemoteConfig.remoteConfigInterface.testingGetAllVariants(); //Assert the values @@ -253,4 +253,19 @@ ImmediateRequestGenerator createIRGForSpecificResponse(final String targetRespon 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/main/java/ly/count/android/sdk/Countly.java b/sdk/src/main/java/ly/count/android/sdk/Countly.java index 558348552..d4f28bc98 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 = "23.02.0-alpha-4"; + private final String DEFAULT_COUNTLY_SDK_VERSION_STRING = "23.02.0-alpha-6"; /** * Used as request meta data on every request diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 12fea011c..51a163421 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -17,7 +17,7 @@ public class ModuleRemoteConfig extends ModuleBase { ImmediateRequestGenerator immediateRequestGenerator; boolean updateRemoteConfigAfterIdChange = false; - 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 @@ -416,16 +416,12 @@ void clearValueStoreInternal() { * @param key * @return */ - @NonNull String[] testingGetVariantsForKeyInternal(@NonNull String key) { + @Nullable String[] testingGetVariantsForKeyInternal(@NonNull String key) { String[] variantResponse = null; if (variantContainer.containsKey(key)) { variantResponse = variantContainer.get(key); } - if (variantResponse == null) { - variantResponse = new String[0]; - } - return variantResponse; } @@ -655,7 +651,7 @@ public void update(RemoteConfigCallback callback) { * @param keysToOmit * @param callback */ - public void DownloadOmittingKeys(String[] keysToOmit, RCDownloadCallback callback) { + public void downloadOmittingKeys(@Nullable String[] keysToOmit, @Nullable RCDownloadCallback callback) { synchronized (_cly) { L.i("[RemoteConfig] Manually calling to updateRemoteConfig with exclude keys"); @@ -684,7 +680,7 @@ public void DownloadOmittingKeys(String[] keysToOmit, RCDownloadCallback callbac * @param keysToInclude * @param callback */ - public void DownloadSpecificKeys(String[] keysToInclude, RCDownloadCallback callback) { + public void downloadSpecificKeys(@Nullable String[] keysToInclude, @Nullable RCDownloadCallback callback) { synchronized (_cly) { L.i("[RemoteConfig] Manually calling to updateRemoteConfig with include keys"); if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { @@ -706,7 +702,7 @@ public void DownloadSpecificKeys(String[] keysToInclude, RCDownloadCallback call } } - public void DownloadAllKeys(RCDownloadCallback callback) { + public void downloadAllKeys(@Nullable RCDownloadCallback callback) { synchronized (_cly) { L.i("[RemoteConfig] Manually calling to update Remote Config v2"); @@ -726,7 +722,7 @@ public void DownloadAllKeys(RCDownloadCallback callback) { } } - public @NonNull Map GetAllValues() { + public @NonNull Map getValues() { synchronized (_cly) { L.i("[RemoteConfig] Getting all Remote config values v2"); @@ -738,7 +734,7 @@ public void DownloadAllKeys(RCDownloadCallback callback) { } } - public @NonNull RCData GetValue(String key) { + public @NonNull RCData getValue(@Nullable String key) { synchronized (_cly) { L.i("[RemoteConfig] Getting Remote config values for key:[" + key + "] v2"); @@ -755,7 +751,7 @@ public void DownloadAllKeys(RCDownloadCallback callback) { * * @param keys - String array of keys (parameters) */ - public void enrollIntoABTestsForKeys(String[] keys) { + public void enrollIntoABTestsForKeys(@Nullable String[] keys) { synchronized (_cly) { L.i("[RemoteConfig] Enrolling user into A/B tests."); @@ -777,7 +773,7 @@ public void enrollIntoABTestsForKeys(String[] keys) { * * @param keys - String array of keys (parameters) */ - public void exitABTestsForKeys(String[] keys) { + public void exitABTestsForKeys(@Nullable String[] keys) { synchronized (_cly) { L.i("[RemoteConfig] Removing user from A/B tests."); @@ -793,11 +789,11 @@ public void exitABTestsForKeys(String[] keys) { } } - public void registerDownloadCallback(RCDownloadCallback callback) { + public void registerDownloadCallback(@Nullable RCDownloadCallback callback) { downloadCallbacks.add(callback); } - public void removeDownloadCallback(RCDownloadCallback callback) { + public void removeDownloadCallback(@Nullable RCDownloadCallback callback) { downloadCallbacks.remove(callback); } @@ -815,7 +811,7 @@ public void clearAll() { L.i("[RemoteConfig] Calling 'testingGetAllVariants'"); if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return null; + return new HashMap<>(); } return testingGetAllVariantsInternal(); @@ -828,7 +824,7 @@ public void clearAll() { * @param key - key value to get variant information for * @return */ - public @NonNull String[] testingGetVariantsForKey(String key) { + public @Nullable String[] testingGetVariantsForKey(@Nullable String key) { synchronized (_cly) { L.i("[RemoteConfig] Calling 'testingGetVariantsForKey'"); @@ -836,6 +832,11 @@ public void clearAll() { return null; } + if (key == null) { + L.i("[RemoteConfig] testingGetVariantsForKey, provided variant key can not be null"); + return null; + } + return testingGetVariantsForKeyInternal(key); } } @@ -845,7 +846,7 @@ public void clearAll() { * * @param completionCallback */ - public void TestingDownloadVariantInformation(RCVariantCallback completionCallback) { + public void testingDownloadVariantInformation(@Nullable RCVariantCallback completionCallback) { synchronized (_cly) { L.i("[RemoteConfig] Calling 'testingFetchVariantInformation'"); @@ -869,7 +870,7 @@ public void TestingDownloadVariantInformation(RCVariantCallback completionCallba * @param variantName - name of the variant for the key to enroll * @param completionCallback */ - public void testingEnrollIntoVariant(String keyName, String variantName, RCVariantCallback completionCallback) { + public void testingEnrollIntoVariant(@Nullable String keyName, String variantName, @Nullable RCVariantCallback completionCallback) { synchronized (_cly) { L.i("[RemoteConfig] Calling 'testingEnrollIntoVariant'"); 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) { From b351292774fb4bfa9fdfdbc546809df25e7d125a Mon Sep 17 00:00:00 2001 From: ArtursK Date: Tue, 20 Jun 2023 17:31:40 +0300 Subject: [PATCH 24/30] . --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 2f8abd3d4..e4a1dd18e 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=23.02.0-alpha-4 +VERSION_NAME=23.02.0-alpha-6 GROUP=ly.count.android POM_URL=https://github.com/Countly/countly-sdk-android POM_SCM_URL=https://github.com/Countly/countly-sdk-android From bd5968f8cbbe401dbf11b96bc2400b4dd704ba0a Mon Sep 17 00:00:00 2001 From: ArtursK Date: Wed, 21 Jun 2023 21:59:16 +0300 Subject: [PATCH 25/30] Refactoring RC callback code. Adding RC tests. --- .../demo/ActivityExampleRemoteConfig.java | 2 +- .../android/demo/ActivityExampleTests.java | 2 +- .../android/sdk/ModuleRemoteConfigTests.java | 97 ++++++++++- .../sdk/RemoteConfigValueStoreTests.java | 2 +- .../count/android/sdk/ModuleRemoteConfig.java | 151 ++++++++---------- 5 files changed, 164 insertions(+), 90 deletions(-) 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 898c747e1..99063db98 100644 --- a/app/src/main/java/ly/count/android/demo/ActivityExampleRemoteConfig.java +++ b/app/src/main/java/ly/count/android/demo/ActivityExampleRemoteConfig.java @@ -27,7 +27,7 @@ public void onCreate(Bundle savedInstanceState) { } public void onClickRemoteConfigUpdate(View v) { - Countly.sharedInstance().remoteConfig().DownloadAllKeys(new RCDownloadCallback() { + 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(); 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 7125b80d3..03a5074ca 100644 --- a/app/src/main/java/ly/count/android/demo/ActivityExampleTests.java +++ b/app/src/main/java/ly/count/android/demo/ActivityExampleTests.java @@ -22,7 +22,7 @@ public void onCreate(Bundle savedInstanceState) { // For fetching all variants with a button click public void onClickFetchAllVariants(View v) { - Countly.sharedInstance().remoteConfig().TestingDownloadVariantInformation(new RCVariantCallback() { + Countly.sharedInstance().remoteConfig().testingDownloadVariantInformation(new RCVariantCallback() { @Override public void callback(RequestResult result, String error) { if (result == RequestResult.Success) { 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 a312a3980..91d5e0f5d 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java @@ -13,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) @@ -23,6 +26,97 @@ public class ModuleRemoteConfigTests { public void setUp() { Countly.sharedInstance().setLoggingEnabled(true); countlyStore = new CountlyStore(getContext(), mock(ModuleLog.class)); + + countlyStore.clear(); + } + + /** + * Consent removal should clear stored remote config values + */ + @Test + 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); + + //set RC + String[] rcArr = new String[] { rcEStr("a", 123), rcEStr("b", "fg") }; + RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), false); + countlyStore.setRemoteConfigValues(rcvs.dataToString()); + + Assert.assertEquals(123, countly.remoteConfig().getValue("a").value); + Assert.assertEquals("fg", countly.remoteConfig().getValue("b").value); + + countly.consent().removeConsentAll(); + + Assert.assertNull(countly.remoteConfig().getValue("a").value); + Assert.assertNull(countly.remoteConfig().getValue("b").value); + } + + /** + * 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 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 + } + } } /** @@ -30,8 +124,6 @@ public void setUp() { */ @Test public void validateKeyIncludeExclude() { - countlyStore.clear(); - //first a few cases with empty or null values String[] res = RemoteConfigHelper.prepareKeysIncludeExclude(null, null, mock(ModuleLog.class)); Assert.assertNull(res[0]); @@ -63,7 +155,6 @@ 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); diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java index 9570d34d9..90783d437 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/RemoteConfigValueStoreTests.java @@ -210,7 +210,7 @@ public static String rcEStr(String key, Object value) { return rcEStr(key, value, true); } - String rcArrIntoJSON(String[] arr) { + public static String rcArrIntoJSON(String[] arr) { StringBuilder sb = new StringBuilder(); sb.append("{"); diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 51a163421..95cc70af3 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -15,7 +15,7 @@ import static ly.count.android.sdk.ModuleConsent.ConsentChangeSource.ChangeConsentCall; public class ModuleRemoteConfig extends ModuleBase { - ImmediateRequestGenerator immediateRequestGenerator; + ImmediateRequestGenerator iRGenerator; boolean updateRemoteConfigAfterIdChange = false; Map variantContainer = new HashMap<>(); // Stores the fetched A/B test variants RemoteConfig remoteConfigInterface = null; @@ -37,7 +37,7 @@ public class ModuleRemoteConfig extends ModuleBase { L.v("[ModuleRemoteConfig] Initialising"); metricOverride = config.metricOverride; - immediateRequestGenerator = config.immediateRequestGenerator; + iRGenerator = config.immediateRequestGenerator; L.d("[ModuleRemoteConfig] Setting if remote config Automatic download will be enabled, " + config.enableRemoteConfigAutomaticDownloadTriggers); automaticDownloadTriggersEnabled = config.enableRemoteConfigAutomaticDownloadTriggers; @@ -94,28 +94,25 @@ void updateRemoteConfigValues(@Nullable final String[] keysOnly, @Nullable final ConnectionProcessor cp = requestQueueProvider.createConnectionProcessor(); final boolean networkingIsEnabled = cp.configProvider_.getNetworkingEnabled(); - (new ImmediateRequestMaker()).doWork(requestData, "/o/sdk", cp, false, networkingIsEnabled, new ImmediateRequestMaker.InternalImmediateRequestCallback() { - @Override - public void callback(@Nullable JSONObject 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; - } + 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; - Map newRC = RemoteConfigHelper.DownloadedValuesIntoMap(checkResponse); + String error = null; + Map newRC = RemoteConfigHelper.DownloadedValuesIntoMap(checkResponse); - 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); + 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()); @@ -142,23 +139,20 @@ void enrollIntoABTestsForKeysInternal(@NonNull String[] keys) { ConnectionProcessor cp = requestQueueProvider.createConnectionProcessor(); final boolean networkingIsEnabled = cp.configProvider_.getNetworkingEnabled(); - (new ImmediateRequestMaker()).doWork(requestData, "/o/sdk", cp, false, networkingIsEnabled, new ImmediateRequestMaker.InternalImmediateRequestCallback() { - @Override - public void callback(JSONObject checkResponse) { - L.d("[ModuleRemoteConfig] Processing received response, received response is null:[" + (checkResponse == null) + "]"); - if (checkResponse == null) { - return; - } + 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") && checkResponse.getString("result").equals("Success")) { - L.d("[ModuleRemoteConfig] 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()); + try { + if (checkResponse.has("result") && checkResponse.getString("result").equals("Success")) { + L.d("[ModuleRemoteConfig] 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); } @@ -182,23 +176,20 @@ void exitABTestsForKeysInternal(@NonNull String[] keys) { ConnectionProcessor cp = requestQueueProvider.createConnectionProcessor(); final boolean networkingIsEnabled = cp.configProvider_.getNetworkingEnabled(); - (new ImmediateRequestMaker()).doWork(requestData, "/o/sdk", cp, false, networkingIsEnabled, new ImmediateRequestMaker.InternalImmediateRequestCallback() { - @Override - public void callback(JSONObject checkResponse) { - L.d("[ModuleRemoteConfig] Processing received response, received response is null:[" + (checkResponse == null) + "]"); - if (checkResponse == null) { - return; - } + 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") && checkResponse.getString("result").equals("Success")) { - L.d("[ModuleRemoteConfig] Removed user from the A/B test"); - } else { - L.w("[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()); + try { + if (checkResponse.has("result") && checkResponse.getString("result").equals("Success")) { + L.d("[ModuleRemoteConfig] Removed user from the A/B test"); + } else { + L.w("[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); } @@ -226,19 +217,16 @@ void testingFetchVariantInformationInternal(@NonNull final RCVariantCallback cal ConnectionProcessor cp = requestQueueProvider.createConnectionProcessor(); final boolean networkingIsEnabled = cp.configProvider_.getNetworkingEnabled(); - immediateRequestGenerator.CreateImmediateRequestMaker().doWork(requestData, "/o/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(RequestResult.NetworkIssue, "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 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; + } - variantContainer = RemoteConfigHelper.convertVariantsJsonToMap(checkResponse, L); + variantContainer = RemoteConfigHelper.convertVariantsJsonToMap(checkResponse, L); - callback.callback(RequestResult.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()); @@ -271,28 +259,25 @@ 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(RequestResult.NetworkIssue, "Encountered problem while trying to reach the server, possibly no internet connection"); + iRGenerator.CreateImmediateRequestMaker().doWork(requestData, "/i/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 { + if (!isResponseValid(checkResponse)) { + callback.callback(RequestResult.NetworkIssue, "Bad response from the server:" + checkResponse.toString()); return; } - try { - if (!isResponseValid(checkResponse)) { - callback.callback(RequestResult.NetworkIssue, "Bad response from the server:" + checkResponse.toString()); - return; - } + RCAutomaticDownloadTrigger(true); - RCAutomaticDownloadTrigger(true); - - 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."); - } + 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) { @@ -449,9 +434,7 @@ void NotifyDownloadCallbacks(RCDownloadCallback devProvidedCallback, RequestResu } void RCAutomaticDownloadTrigger(boolean cacheClearOldValues) { - if (!cacheClearOldValues) { - clearValueStoreInternal();//todo finish - } else { + if (cacheClearOldValues) { RemoteConfigValueStore rc = loadConfig(); rc.cacheClearValues(); saveConfig(rc); @@ -461,7 +444,7 @@ void RCAutomaticDownloadTrigger(boolean cacheClearOldValues) { L.d("[RemoteConfig] Automatically updating remote config values"); updateRemoteConfigValues(null, null, false, null); } else { - L.v("[RemoteConfig] Automatically RC update trigger skipped"); + L.v("[RemoteConfig] Automatic RC update trigger skipped"); } } From fa0615518f146f1eddf42a8226da09935db510ca Mon Sep 17 00:00:00 2001 From: ArtursK Date: Wed, 21 Jun 2023 23:32:22 +0300 Subject: [PATCH 26/30] Adding more RC tests --- .../android/sdk/ModuleRemoteConfigTests.java | 124 +++++++++++++++++- 1 file changed, 120 insertions(+), 4 deletions(-) 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 91d5e0f5d..675861753 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java @@ -44,16 +44,14 @@ public void valuesClearedOnConsentRemoval() { //set RC String[] rcArr = new String[] { rcEStr("a", 123), rcEStr("b", "fg") }; - RemoteConfigValueStore rcvs = RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), false); - countlyStore.setRemoteConfigValues(rcvs.dataToString()); + 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.consent().removeConsentAll(); - Assert.assertNull(countly.remoteConfig().getValue("a").value); - Assert.assertNull(countly.remoteConfig().getValue("b").value); + Assert.assertEquals(0, countly.remoteConfig().getValues().size()); } /** @@ -119,6 +117,114 @@ public void automaticRCTriggers() { } } + @Test + public void xx() { + for (int a = 0; a < 2; a++) { + countlyStore.clear(); + + CountlyConfig config = (new CountlyConfig(getContext(), "appkey", "http://test.count.ly")).setDeviceId("1234").setLoggingEnabled(true).enableCrashReporting(); + config.enableRemoteConfigAutomaticTriggers(); + config.setRequiresConsent(true); + config.setConsentEnabled(new String[] { Countly.CountlyFeatureNames.remoteConfig }); + + if (a == 0) { + config.enableRemoteConfigValueCaching(); + } + + Countly countly = (new Countly()).init(config); + + Assert.assertEquals(0, countly.remoteConfig().getValues().size()); + } + } + + /** + * 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()); + } + + /** + * Validate that the new and old getters return the correct values when putting them directly into storage + * + * @throws JSONException + */ + @Test + 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); + } + } + } + /** * validating that 'prepareKeysIncludeExclude' works as expected */ @@ -194,4 +300,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.assertTrue(entry.getValue().isCurrentUsersData); + } else { + Assert.assertFalse(entry.getValue().isCurrentUsersData); + } + } + } } From b07231056d21a1015a6058712f6cc347bd0a1c9b Mon Sep 17 00:00:00 2001 From: ArtursK Date: Thu, 22 Jun 2023 01:38:40 +0300 Subject: [PATCH 27/30] Adding caching flow RC tests. Fixing caching bugs --- .../android/sdk/ModuleRemoteConfigTests.java | 100 ++++++++++++++++-- .../ly/count/android/sdk/ModuleDeviceId.java | 4 +- .../count/android/sdk/ModuleRemoteConfig.java | 19 ++-- 3 files changed, 106 insertions(+), 17 deletions(-) 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 675861753..f0299c89f 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java @@ -117,23 +117,63 @@ public void automaticRCTriggers() { } } + /** + * Making sure that caching occurs on the required actions and with the required config options + */ @Test - public void xx() { - for (int a = 0; a < 2; a++) { + 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(); - config.setRequiresConsent(true); - config.setConsentEnabled(new String[] { Countly.CountlyFeatureNames.remoteConfig }); - if (a == 0) { + 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); + + countly.deviceId().changeWithMerge("dd"); + Assert.assertEquals(2, countly.remoteConfig().getValues().size()); + assertCValueCachedState(countly.remoteConfig().getValues(), false); + + countly.deviceId().changeWithoutMerge("dd11"); + 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()); + } + + countlyStore.setRemoteConfigValues(RemoteConfigValueStore.dataFromString(rcArrIntoJSON(rcArr), false).dataToString()); + + countly.deviceId().enableTemporaryIdMode(); + 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()); + } } } @@ -225,6 +265,52 @@ public void validateGetters() throws JSONException { } } + /** + * 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); + + countly.remoteConfig().downloadSpecificKeys(null, null); + countly.remoteConfig().downloadSpecificKeys(new String[] {}, null); + countly.remoteConfig().updateForKeysOnly(null, null); + countly.remoteConfig().updateForKeysOnly(new String[] {}, null); + + countly.remoteConfig().enrollIntoABTestsForKeys(null); + countly.remoteConfig().enrollIntoABTestsForKeys(new String[] {}); + + 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); + } + /** * validating that 'prepareKeysIncludeExclude' works as expected */ @@ -304,9 +390,9 @@ public void validateMergeReceivedResponse() throws Exception { static void assertCValueCachedState(Map rcValues, boolean valuesAreCached) { for (Map.Entry entry : rcValues.entrySet()) { if (valuesAreCached) { - Assert.assertTrue(entry.getValue().isCurrentUsersData); - } else { Assert.assertFalse(entry.getValue().isCurrentUsersData); + } else { + Assert.assertTrue(entry.getValue().isCurrentUsersData); } } } 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 b85703cf3..1eb649d50 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleDeviceId.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleDeviceId.java @@ -125,7 +125,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()); @@ -185,7 +185,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/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 95cc70af3..52fc76b90 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -21,7 +21,7 @@ public class ModuleRemoteConfig extends ModuleBase { RemoteConfig remoteConfigInterface = null; //if set to true, it will automatically download remote configs on module startup - boolean automaticDownloadTriggersEnabled = false; + boolean automaticDownloadTriggersEnabled; boolean remoteConfigValuesShouldBeCached = false; @@ -39,8 +39,9 @@ public class ModuleRemoteConfig extends ModuleBase { metricOverride = config.metricOverride; iRGenerator = config.immediateRequestGenerator; - L.d("[ModuleRemoteConfig] Setting if remote config Automatic download will be enabled, " + config.enableRemoteConfigAutomaticDownloadTriggers); + L.d("[ModuleRemoteConfig] Setting if remote config Automatic triggers enabled, " + config.enableRemoteConfigAutomaticDownloadTriggers + ", caching enabled: " + config.enableRemoteConfigValueCaching); automaticDownloadTriggersEnabled = config.enableRemoteConfigAutomaticDownloadTriggers; + remoteConfigValuesShouldBeCached = config.enableRemoteConfigValueCaching; downloadCallbacks.addAll(config.remoteConfigGlobalCallbackList); @@ -410,17 +411,21 @@ void clearValueStoreInternal() { return variantResponse; } - void clearAndDownloadAfterIdChange() { + void clearAndDownloadAfterIdChange(boolean valuesShouldBeCacheCleared) { L.v("[RemoteConfig] Clearing remote config values and preparing to download after ID update"); - CacheOrClearRCValuesIfNeeded(); + if (valuesShouldBeCacheCleared) { + CacheOrClearRCValuesIfNeeded(); + } if (automaticDownloadTriggersEnabled && consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { updateRemoteConfigAfterIdChange = true; } } void CacheOrClearRCValuesIfNeeded() { - clearValueStoreInternal(); + RemoteConfigValueStore rc = loadConfig(); + rc.cacheClearValues(); + saveConfig(rc); } void NotifyDownloadCallbacks(RCDownloadCallback devProvidedCallback, RequestResult requestResult, String message, boolean fullUpdate, Map downloadedValues) { @@ -435,9 +440,7 @@ void NotifyDownloadCallbacks(RCDownloadCallback devProvidedCallback, RequestResu void RCAutomaticDownloadTrigger(boolean cacheClearOldValues) { if (cacheClearOldValues) { - RemoteConfigValueStore rc = loadConfig(); - rc.cacheClearValues(); - saveConfig(rc); + CacheOrClearRCValuesIfNeeded(); } if (automaticDownloadTriggersEnabled && consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { From 2b136a88932790276a67cd58aea414da0f914121 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Thu, 22 Jun 2023 14:22:48 +0300 Subject: [PATCH 28/30] Using the correct endpoint. Removing consent checks --- CHANGELOG.md | 8 ++- gradle.properties | 2 +- .../android/sdk/ConnectionQueueTests.java | 2 +- .../android/sdk/ModuleRemoteConfigTests.java | 70 ++++++++++++++----- .../java/ly/count/android/sdk/Countly.java | 2 +- .../ly/count/android/sdk/MigrationHelper.java | 2 +- .../count/android/sdk/ModuleRemoteConfig.java | 36 ++-------- 7 files changed, 68 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5c72bf31..e5586df4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,10 @@ -## 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 + * 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/gradle.properties b/gradle.properties index e4a1dd18e..10e2c8d80 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=23.02.0-alpha-6 +VERSION_NAME=23.02.0-alpha-8 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/src/androidTest/java/ly/count/android/sdk/ConnectionQueueTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ConnectionQueueTests.java index 541179668..42c51eb36 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("23.02.0-alpha-6")); + Assert.assertTrue(pair[1].equals("23.02.0-alpha-8")); } else if (a == 1) { Assert.assertTrue(pair[1].equals("123sdf.v-213")); } 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 f0299c89f..c33902062 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java @@ -28,6 +28,7 @@ public void setUp() { countlyStore = new CountlyStore(getContext(), mock(ModuleLog.class)); countlyStore.clear(); + countlyStore.setDataSchemaVersion(MigrationHelper.DATA_SCHEMA_VERSIONS); } /** @@ -119,6 +120,7 @@ public void automaticRCTriggers() { /** * 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 rcValueCaching() { @@ -146,37 +148,67 @@ public void rcValueCaching() { 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()); - assertCValueCachedState(countly.remoteConfig().getValues(), false); + //changing without merging should trigger caching. Lack of consent should leave no impact on this + assertCValueCachedState(countly.remoteConfig().getValues(), false); countly.deviceId().changeWithoutMerge("dd11"); - 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()); + + 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(); - 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()); + + 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); + } + /** * Validate the the new and old clears are clearing values after they are directly put into storage */ 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 d4f28bc98..403ac8617 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 = "23.02.0-alpha-6"; + private final String DEFAULT_COUNTLY_SDK_VERSION_STRING = "23.02.0-alpha-8"; /** * Used as request meta data on every request 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 1fa0edd8d..4cf1d66c6 100644 --- a/sdk/src/main/java/ly/count/android/sdk/MigrationHelper.java +++ b/sdk/src/main/java/ly/count/android/sdk/MigrationHelper.java @@ -16,7 +16,7 @@ class MigrationHelper { * 2 - transitioning old RC store to one that supports metadata * x - adding device ID to all requests */ - final int DATA_SCHEMA_VERSIONS = 2; + static final int DATA_SCHEMA_VERSIONS = 2; static final public String key_from_0_to_1_custom_id_set = "0_1_custom_id_set"; diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java index 52fc76b90..6e8617be1 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java +++ b/sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java @@ -147,10 +147,10 @@ void enrollIntoABTestsForKeysInternal(@NonNull String[] keys) { } try { - if (checkResponse.has("result") && checkResponse.getString("result").equals("Success")) { - L.d("[ModuleRemoteConfig] Enrolled user for the A/B test"); + 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."); + 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()); @@ -177,7 +177,7 @@ void exitABTestsForKeysInternal(@NonNull String[] keys) { ConnectionProcessor cp = requestQueueProvider.createConnectionProcessor(); final boolean networkingIsEnabled = cp.configProvider_.getNetworkingEnabled(); - iRGenerator.CreateImmediateRequestMaker().doWork(requestData, "/o/sdk", cp, false, networkingIsEnabled, checkResponse -> { + 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; @@ -187,7 +187,7 @@ void exitABTestsForKeysInternal(@NonNull String[] keys) { if (checkResponse.has("result") && checkResponse.getString("result").equals("Success")) { L.d("[ModuleRemoteConfig] Removed user from the A/B test"); } else { - L.w("[ModuleRemoteConfig] Encountered a network error while removing the user from A/B testing."); + 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()); @@ -260,7 +260,7 @@ void testingEnrollIntoVariantInternal(@NonNull final String key, @NonNull final ConnectionProcessor cp = requestQueueProvider.createConnectionProcessor(); final boolean networkingIsEnabled = cp.configProvider_.getNetworkingEnabled(); - iRGenerator.CreateImmediateRequestMaker().doWork(requestData, "/i/sdk", cp, false, networkingIsEnabled, checkResponse -> { + 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"); @@ -516,10 +516,6 @@ public Map getAllValues() { synchronized (_cly) { L.i("[RemoteConfig] Calling 'getAllValues'"); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return null; - } - return getAllRemoteConfigValuesInternalLegacy(); } } @@ -535,10 +531,6 @@ public Object getValueForKey(String key) { synchronized (_cly) { L.i("[RemoteConfig] Calling remoteConfigValueForKey, " + key); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return null; - } - return getRCValueLegacy(key); } } @@ -712,10 +704,6 @@ public void downloadAllKeys(@Nullable RCDownloadCallback callback) { synchronized (_cly) { L.i("[RemoteConfig] Getting all Remote config values v2"); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return new HashMap<>(); - } - return getAllRemoteConfigValuesInternal(); } } @@ -724,10 +712,6 @@ public void downloadAllKeys(@Nullable RCDownloadCallback callback) { synchronized (_cly) { L.i("[RemoteConfig] Getting Remote config values for key:[" + key + "] v2"); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return new RCData(null, true); - } - return getRCValue(key); } } @@ -796,10 +780,6 @@ public void clearAll() { synchronized (_cly) { L.i("[RemoteConfig] Calling 'testingGetAllVariants'"); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return new HashMap<>(); - } - return testingGetAllVariantsInternal(); } } @@ -814,10 +794,6 @@ public void clearAll() { synchronized (_cly) { L.i("[RemoteConfig] Calling 'testingGetVariantsForKey'"); - if (!consentProvider.getConsent(Countly.CountlyFeatureNames.remoteConfig)) { - return null; - } - if (key == null) { L.i("[RemoteConfig] testingGetVariantsForKey, provided variant key can not be null"); return null; From 3b8b159bdb0804367c58270946f1fb1d4f884733 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Mon, 26 Jun 2023 22:23:47 +0300 Subject: [PATCH 29/30] Fixing a few RC/AB inconsistencies. Fixing temp id bug. --- CHANGELOG.md | 2 + .../count/android/sdk/ModuleEventsTests.java | 45 +++++++++++++++++++ .../java/ly/count/android/sdk/Countly.java | 6 ++- .../ly/count/android/sdk/ModuleDeviceId.java | 5 ++- .../ly/count/android/sdk/ModuleEvents.java | 2 +- .../count/android/sdk/ModuleRemoteConfig.java | 5 ++- .../sdk/internal/RemoteConfigValueStore.java | 4 -- 7 files changed, 59 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5586df4d..fd91f07ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ * Introduced a new set of remote config methods * Deprecated old remote config methods +* Fixed bug where recording views would force send all stored events + * 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/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/main/java/ly/count/android/sdk/Countly.java b/sdk/src/main/java/ly/count/android/sdk/Countly.java index 403ac8617..e27f8f02a 100644 --- a/sdk/src/main/java/ly/count/android/sdk/Countly.java +++ b/sdk/src/main/java/ly/count/android/sdk/Countly.java @@ -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/ModuleDeviceId.java b/sdk/src/main/java/ly/count/android/sdk/ModuleDeviceId.java index 1eb649d50..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,13 +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.RCAutomaticDownloadTrigger(true); + _cly.moduleRemoteConfig.RCAutomaticDownloadTrigger(false); _cly.requestQueue().attemptToSendStoredRequests(); } @@ -116,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 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 newValues, boolean fullUpda Countly.sharedInstance().L.e("[RemoteConfigValueStore] Failed merging remote config values"); } } - dirty = true; Countly.sharedInstance().L.v("[RemoteConfigValueStore] merging done:" + values.toString()); } From a9f0aa17bbc9f7154f32f16e75e29132b577b3e2 Mon Sep 17 00:00:00 2001 From: ArtursK Date: Tue, 27 Jun 2023 10:11:49 +0300 Subject: [PATCH 30/30] Setting the release version --- CHANGELOG.md | 1 + gradle.properties | 2 +- .../android/sdk/ConnectionQueueTests.java | 2 +- .../android/sdk/ModuleRemoteConfigTests.java | 73 +++++++++++++++++++ .../ly/count/android/sdk/ConnectionQueue.java | 2 +- .../java/ly/count/android/sdk/Countly.java | 2 +- 6 files changed, 78 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd91f07ef..c84f7336e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ * 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" diff --git a/gradle.properties b/gradle.properties index 10e2c8d80..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=23.02.0-alpha-8 +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/src/androidTest/java/ly/count/android/sdk/ConnectionQueueTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ConnectionQueueTests.java index 42c51eb36..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("23.02.0-alpha-8")); + 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/ModuleRemoteConfigTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java index c33902062..c255e7ccb 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleRemoteConfigTests.java @@ -239,6 +239,79 @@ public void validateClear() { 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 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]); + } + /** * Validate that the new and old getters return the correct values when putting them directly into storage * 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 08c07b748..57ca461b7 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java +++ b/sdk/src/main/java/ly/count/android/sdk/ConnectionQueue.java @@ -747,7 +747,7 @@ public String prepareRemovalParameters(@NonNull String[] keys) { + "&device_id=" + UtilsNetworking.urlEncodeString(deviceIdProvider_.getDeviceId()); if (keys.length > 0) { - data += "&keys=" + UtilsNetworking.encodedArrayBuilder(keys); // TODO: key? keys? /this is not settled yet, redo after its settled + data += "&keys=" + UtilsNetworking.encodedArrayBuilder(keys); } return data; 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 e27f8f02a..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 = "23.02.0-alpha-8"; + private final String DEFAULT_COUNTLY_SDK_VERSION_STRING = "23.6.0"; /** * Used as request meta data on every request