[Java] Add new device ID module tests add device ID type to scenario …

…tests (#161) * feat: new device id change callback * feat: call after onDeviceId * refactor: usage of device id changed in module device id * feat: did changes * feat: add device id type to scenario tests * fix: tests * revert: scenario changes * feat: missing comments * feat: new device id module calls to scenarioo device id tests * feat: missing changelog * fix: update existing tests * fix: add check to callback * refactor: remove unnecessary callback * feat: test ccases * fix: login and logout * refactor: device id module * fix: remove unnecessary params * feat: new tests * fix: rq access index * feat: multiple device id check * fix: test * fix: device id change logic * fix: device id callbacks * fix: module events req * fix: module event check and test cases comments * fix: new ids * fix: added missings tests * feat: buggy test for device id change * refactor: remove unsued * fix: changed index of features * fix: end any view * feat: finish up tests * misc * fix: without merge not started session * fix: add missing test cases --------- Co-authored-by: Artūrs Kadiķis <[email protected]>
Countly · Nov 10, 2023 · 0245a56 · 0245a56
1 parent 3ac8fa1
commit 0245a56
Show file tree

Hide file tree

Showing 13 changed files with 688 additions and 111 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,7 +14,8 @@
   * 'enableRemoteConfigAutomaticTriggers' to automatically download remote config values on init
   * 'remoteConfigRegisterGlobalCallback(RCDownloadCallback callback)' to register a remote config callback
 * Added the ability to set the user profile picture with an URL
-* Added to a way to get device id type by calling "Countly::deviceId::getType" via "instance()" call
+* Added the DeviceId interface. It is accessible through "Countly::instance()::deviceId()" call.
+* Added a way to get device id type by calling "Countly::deviceId::getType" via "instance()" call
 * The SDK now uses a different file for internal configuration. Old file will be deleted.
 
 * Fixed a bug where it was not possible to send a profile picture with binary data

diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/CoreFeature.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/CoreFeature.java
@@ -4,8 +4,8 @@
 import java.util.Map;
 
 public enum CoreFeature {
-    Sessions(1 << 1, ModuleSessions::new),
-    Events(1 << 2, ModuleEvents::new),
+    Events(1 << 1, ModuleEvents::new),
+    Sessions(1 << 2, ModuleSessions::new),
     Views(1 << 3, ModuleViews::new),
     CrashReporting(1 << 4, ModuleCrash::new),
     Location(1 << 5),

diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleBase.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleBase.java
@@ -1,11 +1,9 @@
 package ly.count.sdk.java.internal;
 
-import ly.count.sdk.java.Config;
 import ly.count.sdk.java.Session;
 
 /**
  * Created by artem on 05/01/2017.
- *
  * Contract:
  * <ul>
  *     <li>ModuleBase instances must provide empty constructor with no parameters.</li>
@@ -33,18 +31,6 @@ public void init(InternalConfig config) {
         internalConfig = config;
     }
 
-    /**
-     * Device ID has been acquired from the device id provider.
-     * Can be invoked multiple times throughout the ModuleBase lifecycle.
-     * Parameters can be instance equal (==), meaning that id haven't changed.
-     *
-     * @param config InternalConfig to run in
-     * @param deviceId deviceId valid from now on
-     * @param oldDeviceId deviceId valid previously if any
-     */
-    public void onDeviceId(InternalConfig config, Config.DID deviceId, Config.DID oldDeviceId) {
-    }
-
     /**
      * App users opted out of analytics, or the developer changed essential preferences.
      * Clear all module-related data, close any resources, and prepare to start from a clean sheet.
@@ -131,6 +117,9 @@ protected void initFinished(InternalConfig config) {
 
     /**
      * Called when the device id is changed.
+     *
+     * @param oldDeviceId old device id
+     * @param withMerge if the device id change is with merge or not
      */
     protected void deviceIdChanged(String oldDeviceId, boolean withMerge) {
 

diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleDeviceIdCore.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleDeviceIdCore.java
@@ -81,7 +81,7 @@ public void initFinished(final InternalConfig config) {
                 L.d("[ModuleDeviceIdCore] initFinished, Got developer id [" + did + "]");
             } else {
                 // regular flow - acquire id using specified strategy
-                did = acquireId(config);
+                did = acquireId();
             }
 
             config.setDeviceId(did);
@@ -97,28 +97,16 @@ public void initFinished(final InternalConfig config) {
     public void deviceIdChanged(String oldDeviceId, boolean withMerge) {
         L.d("[ModuleDeviceIdCore] deviceIdChanged, oldDeviceId [" + oldDeviceId + "] withMerge [" + withMerge + "]");
         Config.DID deviceId = internalConfig.getDeviceId();
-        SessionImpl session = SDKCore.instance.getSession();
-
-        if (deviceId != null && oldDeviceId != null && !deviceId.id.equals(oldDeviceId)) {
-            // device id changed
-            if (session != null && session.isActive()) {
-                // end previous session
-                L.d("[ModuleDeviceIdCore] Ending session because device id was changed from [" + oldDeviceId + "]");
-                session.end(null, null, oldDeviceId);
-            }
-
-            // add device id change request
-            Request request = ModuleRequests.nonSessionRequest(internalConfig);
 
-            //if we are missing the device ID, add it
-            if (!request.params.has(Params.PARAM_DEVICE_ID)) {
-                request.params.add(Params.PARAM_DEVICE_ID, deviceId.id);
-            }
-            //add the old device ID every time
-            request.params.add(Params.PARAM_OLD_DEVICE_ID, oldDeviceId);
+        if (Utils.isEmptyOrNull(deviceId.id) || Utils.isEmptyOrNull(oldDeviceId)) {
+            L.w("[ModuleDeviceIdCore] deviceIdChanged, Empty id passed to changeDeviceId method");
+            return;
+        }
 
-            ModuleRequests.pushAsync(internalConfig, request);
-            SDKCore.instance.onSignal(internalConfig, SDKCore.Signal.DID.getIndex());
+        //if without merge then we should not send this request
+        if (withMerge) {
+            // add device id change request and add the old device ID every time
+            ModuleRequests.pushAsync(internalConfig, new Request(Params.PARAM_OLD_DEVICE_ID, oldDeviceId));
         }
     }
 
@@ -139,7 +127,7 @@ public void login(String id) {
      * - send corresponding request to server.
      */
     public void logout() {
-        Config.DID did = acquireId(internalConfig);
+        Config.DID did = acquireId();
         changeDeviceIdInternal(did.id, DeviceIdType.fromInt(did.strategy, L), false);
     }
 
@@ -163,41 +151,30 @@ protected void changeDeviceIdInternal(String id, DeviceIdType type, boolean with
             return;
         }
 
-        Config.DID did = new Config.DID(type.index, id);
         internalConfig.storageProvider.setDeviceIdType(type.name());
         internalConfig.storageProvider.setDeviceID(id);
-        internalConfig.setDeviceId(did);
-
-        if (!withMerge) {
-            SessionImpl session = SDKCore.instance.getSession();
-            if (session != null) {
-                L.d("[ModuleDeviceIdCore] changeDeviceIdInternal, Ending session because device id was unset from [" + old.id + "]");
-                session.end(null, null, old.id);
-            }
-        }
+        internalConfig.setDeviceId(new Config.DID(type.index, id));
 
         SDKCore.instance.notifyModulesDeviceIdChanged(old.id, withMerge);
     }
 
     /**
      * Gets id of the strategy supplied. In case strategy is not available, returns a fallback strategy.
      * In case strategy is available but id cannot be acquired right now, returns null.
-     *
-     * @param config InternalConfig to run in
      */
-    protected Config.DID acquireId(final InternalConfig config) {
-        L.i("[ModuleDeviceIdCore] acquireId, Acquiring device id of strategy [" + config.getDeviceIdStrategy() + "]");
+    protected Config.DID acquireId() {
+        L.i("[ModuleDeviceIdCore] acquireId, Acquiring device id of strategy [" + internalConfig.getDeviceIdStrategy() + "]");
         Config.DID did = null;
 
-        int index = config.getDeviceIdStrategy();
+        int index = internalConfig.getDeviceIdStrategy();
 
         while (index >= 0) {
             DeviceIdGenerator generator = generators.get(index);
             if (generator == null) {
                 L.w("[ModuleDeviceIdCore] Device id strategy [" + index + "] is not available. Falling back to next one.");
                 index--;
             } else {
-                String id = generator.generate(config);
+                String id = generator.generate(internalConfig);
                 if (Utils.isNotEmpty(id)) {
                     did = new Config.DID(index, id);
                     break;
@@ -214,7 +191,7 @@ protected Config.DID acquireId(final InternalConfig config) {
             }
             L.d("[ModuleDeviceIdCore] acquireId, Got device id: " + did);
         } else {
-            L.i("[ModuleDeviceIdCore] acquireId, No device id of strategy [" + config.getDeviceIdStrategy() + "] is available yet");
+            L.i("[ModuleDeviceIdCore] acquireId, No device id of strategy [" + internalConfig.getDeviceIdStrategy() + "] is available yet");
         }
 
         return did;

diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleEvents.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleEvents.java
@@ -1,14 +1,16 @@
 package ly.count.sdk.java.internal;
 
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Collectors;
 import ly.count.sdk.java.Countly;
+import ly.count.sdk.java.Session;
+import ly.count.sdk.java.View;
 
 public class ModuleEvents extends ModuleBase {
     protected EventQueue eventQueue = null;
-    final Map<String, EventImpl> timedEvents = new HashMap<>();
+    final Map<String, EventImpl> timedEvents = new ConcurrentHashMap<>();
     protected Events eventsInterface = null;
 
     @Override
@@ -22,14 +24,39 @@ public void init(InternalConfig config) {
 
     @Override
     protected void onTimer() {
-        addEventsToRequestQ();
+        addEventsToRequestQ(null);
     }
 
     @Override
     public Boolean onRequest(Request request) {
         return true;
     }
 
+    @Override
+    public void deviceIdChanged(String oldDeviceId, boolean withMerge) {
+        super.deviceIdChanged(oldDeviceId, withMerge);
+        L.d("[ModuleEvents] deviceIdChanged: oldDeviceId = " + oldDeviceId + ", withMerge = " + withMerge);
+        if (!withMerge) {
+            for (Map.Entry<String, EventImpl> timedEventEntry : timedEvents.entrySet()) {
+                L.d("[ModuleEvents] deviceIdChanged, Ending timed event: [" + timedEventEntry.getKey() + "]");
+                endEventInternal(timedEventEntry.getKey(), timedEventEntry.getValue().segmentation, timedEventEntry.getValue().count, timedEventEntry.getValue().sum);
+            }
+
+            // this part is to end and record the current view if exists
+            Session session = Countly.session();
+            if ((session != null && session.isActive())) {
+                View currentView = ((SessionImpl) session).currentView;
+                if (currentView != null) {
+                    currentView.stop(true);
+                } else {
+                    Storage.pushAsync(internalConfig, ((SessionImpl) Countly.session()));
+                }
+            }
+
+            addEventsToRequestQ(oldDeviceId);
+        }
+    }
+
     @Override
     public void stop(InternalConfig config, final boolean clear) {
         super.stop(config, clear);
@@ -39,11 +66,18 @@ public void stop(InternalConfig config, final boolean clear) {
         }
     }
 
-    private synchronized void addEventsToRequestQ() {
+    private synchronized void addEventsToRequestQ(String deviceId) {
         L.d("[ModuleEvents] addEventsToRequestQ");
 
+        if (eventQueue.eventQueueMemoryCache.isEmpty()) {
+            L.d("[ModuleEvents] addEventsToRequestQ, eventQueueMemoryCache is empty, skipping");
+            return;
+        }
+
         Request request = new Request();
-        request.params.add("device_id", Countly.instance().getDeviceId());
+        if (deviceId != null) {
+            request.params.add("device_id", deviceId);
+        }
         request.params.arr("events").put(eventQueue.eventQueueMemoryCache).add();
         request.own(ModuleEvents.class);
 
@@ -93,7 +127,7 @@ private void addEventToQueue(EventImpl event) {
     private void checkEventQueueToSend(boolean forceSend) {
         L.d("[ModuleEvents] queue size:[" + eventQueue.eqSize() + "] || forceSend: " + forceSend);
         if (forceSend || (eventQueue.eqSize() >= internalConfig.getEventsBufferSize())) {
-            addEventsToRequestQ();
+            addEventsToRequestQ(null);
         }
     }
 

diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleSessions.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleSessions.java
@@ -60,12 +60,19 @@ public void stop(InternalConfig config, boolean clear) {
     /**
      * Handles one single case of device id change with auto sessions handling, see first {@code if} here:
      *
-     * @see ModuleDeviceIdCore#onDeviceId(InternalConfig, Config.DID, Config.DID)
+     * @see ModuleDeviceIdCore#deviceIdChanged(String, boolean)
      */
-    public void onDeviceId(final InternalConfig config, final Config.DID deviceId, final Config.DID oldDeviceId) {
-        L.d("[ModuleSessions] onDeviceId " + deviceId + ", old " + oldDeviceId);
-        if (deviceId != null && oldDeviceId != null && !deviceId.equals(oldDeviceId) && getSession() == null) {
-            session(config, null).begin();
+    @Override
+    public void deviceIdChanged(String oldDeviceId, boolean withMerge) {
+        Config.DID deviceId = internalConfig.getDeviceId();
+        L.d("[ModuleSessions] deviceIdChanged, " + deviceId + ", old " + oldDeviceId);
+        if (!withMerge && session != null && session.isActive()) {
+            L.d("[ModuleSessions] deviceIdChanged, Ending session because device id was unset from [" + oldDeviceId + "]");
+            session.end(null, null, oldDeviceId);
+        }
+
+        if (deviceId != null && oldDeviceId != null && (session == null || !session.isActive()) && !withMerge) {
+            session(internalConfig, null).begin();
         }
     }
 

diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/SDKCore.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/SDKCore.java
@@ -49,7 +49,7 @@ public SDKCore() {
     }
 
     protected Log L = null;
-    private static ModuleBase testDummyModule = null;//set during testing when trying to check the SDK's lifecycle
+    protected static ModuleBase testDummyModule = null;//set during testing when trying to check the SDK's lifecycle
 
     protected static void registerDefaultModuleMappings() {
         moduleMappings.put(CoreFeature.DeviceId.getIndex(), ModuleDeviceIdCore.class);
@@ -588,12 +588,14 @@ public void onCrash(final InternalConfig config, Throwable t, boolean fatal, Str
     }
 
     public void notifyModulesDeviceIdChanged(@Nullable String old, final boolean withMerge) {
-        L.d("[SDKCore] deviceIdChanged, newDeviceId:[" + config.getDeviceId() + "], oldDeviceId:[ " + old + "]");
+        L.d("[SDKCore] notifyModulesDeviceIdChanged, newDeviceId:[" + config.getDeviceId() + "], oldDeviceId:[ " + old + "]");
         Config.DID id = config.getDeviceId();
-        modules.forEach((feature, module) -> module.deviceIdChanged(old, withMerge));
-        if (id != null) {
-            user.id = id.id;
+        if (id.id.equals(old)) {
+            L.d("[SDKCore] notifyModulesDeviceIdChanged, newDeviceId is the same as oldDeviceId, skipping");
+            return;
         }
+        modules.forEach((feature, module) -> module.deviceIdChanged(old, withMerge));
+        user.id = id.id;
     }
 
     public void login(String id) {

diff --git a/sdk-java/src/main/java/ly/count/sdk/java/internal/SessionImpl.java b/sdk-java/src/main/java/ly/count/sdk/java/internal/SessionImpl.java
@@ -189,11 +189,6 @@ Future<Boolean> end(Long now, final Tasks.Callback<Boolean> callback, String did
 
         this.consents = SDKCore.instance.consents;
 
-        //        // TODO: check if needed
-        //        for (Event event: timedEvents.values()) {
-        //            event.endAndRecord();
-        //        }
-
         if (currentView != null) {
             currentView.stop(true);
         } else {