Merge pull request #347 from Countly/fix_sessions_without_merge

Fix sessions without merge

Author: turtledreams

CHANGELOG.md

@@ -1,3 +1,8 @@
+## XX.XX.XX
+* Mitigated issues where:
+  * session was ending regardless of manual control after without merge, not anymore.
+  * session was not starting even if consent is not required and automatic sessions are enabled after without merge, not anymore.
+
 ## 24.7.0
 * ! Minor breaking change ! User properties will now be automatically saved under the following conditions:
   * When an event is recorded

sdk/src/androidTest/java/ly/count/android/sdk/DeviceIdTests.java

@@ -260,11 +260,11 @@ public void legacyFallbackType_null() {
      * ---
      * set a couple of user properties
      * sleep 1 second
-     * change device ID with merge (creates a session duration request with the duration of 1 second)
+     * change device ID with merge
      * save user profiles
      * sleep 2 seconds
-     * change device ID without merge (creates a session duration request with the duration of 2 seconds)
-     * -- at this point 4 requests are generated (firs begin, 1 merge, 1 user profile, 1 end session for without merge)
+     * change device ID without merge (creates a session duration request with the duration of 3 seconds) and will generate new begin session because consent not required
+     * -- at this point 4 requests are generated (firs begin, 1 merge, 1 user profile, 1 end session for without merge, 1 begin session for the new user)
      * sleep 1 second
      * set user property and save it
      * change device ID without merge with same device id (no session duration request)
@@ -277,7 +277,7 @@ public void legacyFallbackType_null() {
      * set user properties and save them
      * sleep 1 second
      * change device ID with merge (creates a session duration request with the duration of 4 seconds)
-     * -- at this point 8 requests are generated (firs begin, 1 merge, 1 user profile, 1 end session for without merge, 1 user profile, 1 user profile, 1 user profile, 1 merge)
+     * -- at this point 9 requests are generated (firs begin, 1 merge, 1 user profile, 1 end session for without merge, 1 begin session for the new user, 1 user profile, 1 user profile, 1 user profile, 1 merge)
      */
     @Test
     public void sessionDurationScenario_1() throws InterruptedException {
@@ -294,13 +294,13 @@ public void sessionDurationScenario_1() throws InterruptedException {
 
         Thread.sleep(1000);
 
-        countly.deviceId().changeWithMerge("ff_merge"); // this will generate a request with "session_duration" field and reset duration
+        countly.deviceId().changeWithMerge("ff_merge");
         countly.userProfile().save();
 
         Thread.sleep(2000);
 
-        countly.deviceId().changeWithoutMerge("ff"); // this will generate a request with "end_session", "session_duration" fields and reset duration
-        assertEquals(4, TestUtils.getCurrentRQ().length);
+        countly.deviceId().changeWithoutMerge("ff"); // this will generate a request with "end_session", "session_duration" fields and reset duration + begin_session
+        assertEquals(5, TestUtils.getCurrentRQ().length);
 
         TestUtils.validateRequest("ff_merge", TestUtils.map("old_device_id", "1234"), 1);
         TestUtils.validateRequest("ff_merge", TestUtils.map("user_details", "{\"custom\":{\"prop2\":\"123\",\"prop1\":\"string\",\"prop3\":\"false\"}}"), 2);
@@ -332,12 +332,12 @@ public void sessionDurationScenario_1() throws InterruptedException {
 
         countly.deviceId().changeWithMerge("ff_merge"); // this will generate a request with "session_duration" field and reset duration
 
-        assertEquals(8, TestUtils.getCurrentRQ().length);
+        assertEquals(9, TestUtils.getCurrentRQ().length);
 
-        TestUtils.validateRequest("ff", TestUtils.map("user_details", "{\"custom\":{\"prop4\":\"[sd]\"}}"), 4);
-        TestUtils.validateRequest("ff", TestUtils.map("user_details", "{\"custom\":{\"prop6\":\"{key=123}\",\"prop5\":\"{key=value}\",\"prop7\":\"{key=false}\"}}"), 5);
-        TestUtils.validateRequest("ff", TestUtils.map("user_details", "{\"custom\":{\"prop2\":\"456\",\"prop1\":\"string_a\",\"prop3\":\"true\"}}"), 6);
+        TestUtils.validateRequest("ff", TestUtils.map("user_details", "{\"custom\":{\"prop4\":\"[sd]\"}}"), 5);
+        TestUtils.validateRequest("ff", TestUtils.map("user_details", "{\"custom\":{\"prop6\":\"{key=123}\",\"prop5\":\"{key=value}\",\"prop7\":\"{key=false}\"}}"), 6);
+        TestUtils.validateRequest("ff", TestUtils.map("user_details", "{\"custom\":{\"prop2\":\"456\",\"prop1\":\"string_a\",\"prop3\":\"true\"}}"), 7);
 
-        TestUtils.validateRequest("ff_merge", TestUtils.map("old_device_id", "ff"), 7);
+        TestUtils.validateRequest("ff_merge", TestUtils.map("old_device_id", "ff"), 8);
     }
 }

sdk/src/androidTest/java/ly/count/android/sdk/scSE_SessionsTests.java

@@ -202,7 +202,6 @@ public void SE_203_CR_CG_M_id_change() throws InterruptedException {
      * *back to foreground*
      * --- Expected Requests ---
      * Check request queue and verify:
-     * 1. too lazy to calculate just let us know and lets verify together
      */
     @Test
     public void SE_204_CNR_A_id_change() throws InterruptedException {
@@ -211,15 +210,22 @@ public void SE_204_CNR_A_id_change() throws InterruptedException {
 
         flowAutomaticSessions(countly);
 
-        Assert.assertEquals(8, TestUtils.getCurrentRQ().length);
+        Assert.assertEquals(12, TestUtils.getCurrentRQ().length);
         validateSessionBeginRequest(0, TestUtils.commonDeviceId);
         TestUtils.validateRequest("newID", TestUtils.map("old_device_id", TestUtils.commonDeviceId), 1);
         validateSessionEndRequest(2, 2, "newID");
-        TestUtils.validateRequest("newID", TestUtils.map("old_device_id", "newID_2"), 3);
-        validateSessionBeginRequest(4, "newID_2");
-        TestUtils.validateRequest("newID", TestUtils.map("old_device_id", "newID_2"), 5);
-        validateSessionEndRequest(6, null, "newID");
-        validateSessionBeginRequest(7, "newID");
+
+        validateSessionBeginRequest(3, "newID_2");
+        TestUtils.validateRequest("newID", TestUtils.map("old_device_id", "newID_2"), 4);
+        validateSessionEndRequest(5, 2, "newID");
+
+        validateSessionBeginRequest(6, "newID_2");
+        validateSessionEndRequest(7, 1, "newID_2");
+
+        validateSessionBeginRequest(8, "newID_2");
+        TestUtils.validateRequest("newID", TestUtils.map("old_device_id", "newID_2"), 9);
+        validateSessionEndRequest(10, null, "newID");
+        validateSessionBeginRequest(11, "newID");
     }
 
     /**

sdk/src/androidTest/java/ly/count/android/sdk/scUP_UserProfileTests.java

@@ -94,7 +94,7 @@ public void eventSaveScenario_onTimer() throws InterruptedException, JSONExcepti
     /**
      * Related user properties should be saved before event recordings
      * call order, user property with "dark_mode", event, user property with "light_mode"
-     * generated request order first user property request + 3 events + user property request with light_mode
+     * generated request order first user property request + 3 events + user property request with light_mode + begin session
      */
     @Test
     public void eventSaveScenario_changeDeviceIDWithoutMerge() throws JSONException {
@@ -111,11 +111,11 @@ public void eventSaveScenario_changeDeviceIDWithoutMerge() throws JSONException
         countly.userProfile().setProperty("theme", "light_mode");
         TestUtils.assertRQSize(1); // no request is generated on the way
 
-        countly.deviceId().changeWithoutMerge("new_device_id");
+        countly.deviceId().changeWithoutMerge("new_device_id"); // this will begin a new session
 
         // first user property request + 3 events + user property request with light_mode
-        ModuleUserProfileTests.validateUserProfileRequest(0, 3, TestUtils.map(), TestUtils.map("theme", "dark_mode"));
-        ModuleUserProfileTests.validateUserProfileRequest(2, 3, TestUtils.map(), TestUtils.map("theme", "light_mode"));
+        ModuleUserProfileTests.validateUserProfileRequest(0, 4, TestUtils.map(), TestUtils.map("theme", "dark_mode"));
+        ModuleUserProfileTests.validateUserProfileRequest(2, 4, TestUtils.map(), TestUtils.map("theme", "light_mode"));
     }
 
     /**

sdk/src/main/java/ly/count/android/sdk/Countly.java

@@ -878,11 +878,11 @@ public synchronized void halt() {
         staticInit();
     }
 
-    synchronized void notifyDeviceIdChange() {
+    synchronized void notifyDeviceIdChange(boolean withoutMerge) {
         L.d("Notifying modules that device ID changed");
 
         for (ModuleBase module : modules) {
-            module.deviceIdChanged();
+            module.deviceIdChanged(withoutMerge);
         }
     }
 

sdk/src/main/java/ly/count/android/sdk/ModuleBase.java

@@ -88,7 +88,7 @@ void callbackOnActivityStopped(Activity activity) {
     //}
 
     //notify the SDK modules that the device ID has changed
-    void deviceIdChanged() {
+    void deviceIdChanged(boolean withoutMerge) {
     }
 
     //notify the SDK modules that consent was updated

sdk/src/main/java/ly/count/android/sdk/ModuleDeviceId.java

@@ -131,7 +131,10 @@ void changeDeviceIdWithoutMergeInternal(@NonNull String deviceId) {
         //update remote config_ values after id change if automatic update is enabled
         _cly.moduleRemoteConfig.clearAndDownloadAfterIdChange();
 
-        _cly.moduleSessions.endSessionInternal();
+        if (!_cly.moduleSessions.manualSessionControlEnabled) {
+            //if manual session control is not enabled, end the current session
+            _cly.moduleSessions.endSessionInternal(); // this will check consent
+        }
 
         //remove all consent
         _cly.moduleConsent.removeConsentAllInternal(ModuleConsent.ConsentChangeSource.DeviceIDChangedNotMerged);
@@ -146,7 +149,7 @@ void changeDeviceIdWithoutMergeInternal(@NonNull String deviceId) {
 
         //clear automated star rating session values because now we have a new user
         _cly.moduleRatings.clearAutomaticStarRatingSessionCountInternal();
-        _cly.notifyDeviceIdChange();
+        _cly.notifyDeviceIdChange(true);
     }
 
     /**
@@ -192,7 +195,7 @@ void changeDeviceIdWithMergeInternal(@NonNull String deviceId) {
             _cly.moduleRemoteConfig.clearAndDownloadAfterIdChange();
             requestQueueProvider.changeDeviceId(deviceId, deviceIdInstance.getCurrentId());
             deviceIdInstance.changeToCustomId(deviceId);
-            _cly.notifyDeviceIdChange();
+            _cly.notifyDeviceIdChange(false);
         }
     }
 

sdk/src/main/java/ly/count/android/sdk/ModuleRemoteConfig.java

@@ -433,7 +433,7 @@ void onConsentChanged(@NonNull final List<String> consentChangeDelta, final bool
     }
 
     @Override
-    void deviceIdChanged() {
+    void deviceIdChanged(boolean withoutMerge) {
         L.v("[RemoteConfig] Device ID changed will update values: [" + updateRemoteConfigAfterIdChange + "]");
 
         if (updateRemoteConfigAfterIdChange) {

sdk/src/main/java/ly/count/android/sdk/ModuleSessions.java

@@ -171,6 +171,14 @@ void halt() {
         sessionRunning = false;
     }
 
+    @Override
+    void deviceIdChanged(boolean withoutMerge) {
+        if (!manualSessionControlEnabled && withoutMerge) {
+            L.d("[ModuleSessions] deviceIdChanged, automatic session control enabled and device id changed without merge, starting a new session");
+            beginSessionInternal();
+        }
+    }
+
     public class Sessions {
         public void beginSession() {
             synchronized (_cly) {