[Java] Setup properly device id callback calls (#154)

* 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

* fix: login and logout

* fix: index values

* comments

* cleanup

---------

Co-authored-by: Artūrs Kadiķis <[email protected]>

Author: arifBurakDemiray

sdk-java/src/main/java/ly/count/sdk/java/Config.java

@@ -148,7 +148,7 @@ public int hashCode() {
 
         @Override
         public String toString() {
-            return "DID " + id + " (" + strategy + ")";
+            return "DID " + id + " ( " + strategy + ")";
         }
     }
 

sdk-java/src/main/java/ly/count/sdk/java/User.java

@@ -35,13 +35,6 @@ public static Gender fromString(String v) {
         }
     }
 
-    /**
-     * Current device id
-     *
-     * @return id string if device id available, null otherwise
-     */
-    public abstract String id();
-
     /**
      * Current user name
      *

sdk-java/src/main/java/ly/count/sdk/java/internal/DeviceIdType.java

@@ -4,7 +4,13 @@
  * Device ID Type
  */
 public enum DeviceIdType {
-    DEVELOPER_SUPPLIED, SDK_GENERATED;
+    DEVELOPER_SUPPLIED(10), SDK_GENERATED(0);
+
+    public final int index;
+
+    DeviceIdType(int index) {
+        this.index = index;
+    }
 
     public static DeviceIdType fromInt(int deviceIdType, Log L) {
         switch (deviceIdType) {

sdk-java/src/main/java/ly/count/sdk/java/internal/InternalConfig.java

@@ -90,10 +90,10 @@ public static Long storageId() {
     }
 
     public DID getDeviceId() {
-        if (dids.isEmpty()) {
-            return null;
-        } else {
+        if (!dids.isEmpty()) {
             return dids.get(0);
+        } else {
+            return null;
         }
     }
 
@@ -115,6 +115,15 @@ public boolean removeDeviceId(DID did) {
         return this.dids.remove(did);
     }
 
+    public boolean removeDeviceId(String id) {
+        for (DID did : dids) {
+            if (did.id.equals(id)) {
+                return this.dids.remove(did);
+            }
+        }
+        return false;
+    }
+
     public int getFeatures1() {
         return features;
     }

sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleBase.java

@@ -132,7 +132,7 @@ protected void initFinished(InternalConfig config) {
     /**
      * Called when the device id is changed.
      */
-    protected void deviceIdChanged(Config.DID oldDeviceId, boolean withMerge) {
+    protected void deviceIdChanged(String oldDeviceId, boolean withMerge) {
 
     }
 }

sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleDeviceIdCore.java

@@ -72,39 +72,39 @@ public void initFinished(final InternalConfig config) {
         L.i("[ModuleDeviceIdCore] initFinished, Starting device ID acquisition");
         if (config.getDeviceId() == null) {
             // either fresh install, or migration from legacy SDK
-
             L.i("[ModuleDeviceIdCore] initFinished, Acquiring device id");
 
+            Config.DID did;
             if (Utils.isNotEmpty(config.getCustomDeviceId())) {
                 // developer specified id on SDK init
-                Config.DID did = new Config.DID(Config.DID.STRATEGY_CUSTOM, config.getCustomDeviceId());
+                did = new Config.DID(Config.DID.STRATEGY_CUSTOM, config.getCustomDeviceId());
                 L.d("[ModuleDeviceIdCore] initFinished, Got developer id [" + did + "]");
-                SDKCore.instance.onDeviceId(config, did, null);
             } else {
                 // regular flow - acquire id using specified strategy
-                Config.DID did = new Config.DID(config.getDeviceIdStrategy(), null);
-                acquireId(config, did);
+                did = acquireId(config);
             }
+
+            config.setDeviceId(did);
+            config.storageProvider.setDeviceID(did.id);
+            config.storageProvider.setDeviceIdType(DeviceIdType.fromInt(did.strategy, L).name());
         } else {
-            // second or next app launch, notify id is available
             Config.DID loadedDid = config.getDeviceId();
             L.d("[ModuleDeviceIdCore] initFinished, Loading previously saved device id:[" + loadedDid.id + "] strategy:[" + loadedDid.strategy + "]");
-            SDKCore.instance.onDeviceId(config, loadedDid, loadedDid);
         }
     }
 
     @Override
-    public void deviceIdChanged(Config.DID oldDeviceId, boolean withMerge) {
+    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.equals(oldDeviceId)) {
+        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.id + "]");
-                session.end(null, null, oldDeviceId.id);
+                L.d("[ModuleDeviceIdCore] Ending session because device id was changed from [" + oldDeviceId + "]");
+                session.end(null, null, oldDeviceId);
             }
 
             // add device id change request
@@ -115,56 +115,10 @@ public void deviceIdChanged(Config.DID oldDeviceId, boolean withMerge) {
                 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.id);
+            request.params.add(Params.PARAM_OLD_DEVICE_ID, oldDeviceId);
 
             ModuleRequests.pushAsync(internalConfig, request);
-
-            sendDIDSignal(internalConfig, deviceId, oldDeviceId);
-        }
-    }
-
-    @Override
-    public void onDeviceId(InternalConfig config, final Config.DID deviceId, final Config.DID oldDeviceId) {
-        L.d("[ModuleDeviceIdCore] onDeviceId [" + deviceId + "]");
-
-        SessionImpl session = SDKCore.instance.getSession();
-
-        if (deviceId == null && oldDeviceId != null) {
-            // device id is unset
-            if (session != null) {
-                L.d("[ModuleDeviceIdCore] Ending session because device id was unset from [" + oldDeviceId.id + "]");
-                session.end(null, null, oldDeviceId.id);
-            }
-
-            sendDIDSignal(config, null, oldDeviceId);
-        } else if (deviceId != null && oldDeviceId == null) {
-            // device id just acquired
-            if (this.tasks == null) {
-                this.tasks = new Tasks("deviceId", L);
-            }
-            tasks.run(new Tasks.Task<Object>(0L) {
-                @Override
-                public Object call() throws Exception {
-                    // put device_id parameter into existing requests
-                    L.i("[ModuleDeviceIdCore] Adding device_id to previous requests");
-                    boolean success = transformRequests(config, deviceId.id);
-                    if (success) {
-                        L.i("[ModuleDeviceIdCore] First transform: success");
-                    } else {
-                        L.w("[ModuleDeviceIdCore] First transform: failure");
-                    }
-
-                    // do it second time in case new requests were added during first attempt
-                    success = transformRequests(config, deviceId.id);
-                    if (!success) {
-                        L.e("[ModuleDeviceIdCore] Failed to put device_id into existing requests, following behaviour for unhandled requests is undefined.");
-                    } else {
-                        L.i("[ModuleDeviceIdCore] Second transform: success");
-                    }
-                    sendDIDSignal(config, deviceId, null);
-                    return null;
-                }
-            });
+            SDKCore.instance.onSignal(internalConfig, SDKCore.Signal.DID.getIndex());
         }
     }
 
@@ -186,55 +140,25 @@ private boolean transformRequests(final InternalConfig config, final String devi
         });
     }
 
-    /**
-     * Just a wrapper around {@link SDKCore#onSignal(InternalConfig, int)}} for {@link SDKCore.Signal#DID} case
-     *
-     * @param config InternalConfig to run in
-     * @param id new {@link Config.DID} if any
-     * @param old old {@link Config.DID} if any
-     */
-    private void sendDIDSignal(InternalConfig config, Config.DID id, Config.DID old) {
-        L.d("[ModuleDeviceIdCore] Sending device id signal: [" + id + "], was [" + old + "]");
-        SDKCore.instance.onSignal(config, SDKCore.Signal.DID.getIndex());
-    }
-
     /**
      * Logging into app-specific account:
      * - reset device id and notify modules;
      * - send corresponding request to server.
      *
-     * @param config InternalConfig to run in
      * @param id device id to change to
      */
-    public void login(InternalConfig config, String id) {
-        if (Utils.isEmptyOrNull(id)) {
-            L.e("[ModuleDeviceIdCore] Empty id passed to login method");
-        } else {
-            final Config.DID old = config.getDeviceId();
-            config.setDeviceId(new Config.DID(Config.DID.STRATEGY_CUSTOM, id));
-            config.storageProvider.setDeviceIdType(DeviceIdType.DEVELOPER_SUPPLIED.name());
-            config.storageProvider.setDeviceID(id);
-
-            // old session end & new session begin requests are supposed to happen here
-            SDKCore.instance.onDeviceId(config, config.getDeviceId(), old);
-        }
+    public void login(String id) {
+        changeDeviceIdInternal(id, DeviceIdType.DEVELOPER_SUPPLIED, true);
     }
 
     /**
      * Logging out from app-specific account and reverting back to previously used id if any:
      * - nullify device id and notify modules;
      * - send corresponding request to server.
-     *
-     * @param config context to run in
      */
-    public void logout(final InternalConfig config) {
-        final Config.DID old = config.getDeviceId();
-        config.removeDeviceId(old);
-        config.storageProvider.setDeviceID("");
-        config.storageProvider.setDeviceIdType("");
-
-        SDKCore.instance.onDeviceId(config, null, old);
-        acquireId(config, new Config.DID(config.getDeviceIdStrategy(), null));
+    public void logout() {
+        Config.DID did = acquireId(internalConfig);
+        changeDeviceIdInternal(did.id, DeviceIdType.fromInt(did.strategy, L), false);
     }
 
     /**
@@ -244,39 +168,47 @@ public void logout(final InternalConfig config) {
      *     <li>Begin new session with new id if previously ended a session</li>
      * </ul>
      *
-     * @param config context to run in
      * @param id new user id
      */
-    protected void changeDeviceIdInternal(InternalConfig config, String id, boolean withMerge) {
+    protected void changeDeviceIdInternal(String id, DeviceIdType type, boolean withMerge) {
         if (Utils.isEmptyOrNull(id)) {
             L.w("[ModuleDeviceIdCore] changeDeviceId, Empty id passed to changeDeviceId method");
             return;
         }
-
-        final Config.DID old = config.getDeviceId();
+        final Config.DID old = internalConfig.getDeviceId();
         if (old.id.equals(id)) {
             L.w("[ModuleDeviceIdCore] changeDeviceId, Same id passed to changeDeviceId method, ignoring");
             return;
         }
 
-        internalConfig.setDeviceId(new Config.DID(Config.DID.STRATEGY_CUSTOM, id));
-        internalConfig.storageProvider.setDeviceIdType(DeviceIdType.DEVELOPER_SUPPLIED.name());
+        Config.DID did = new Config.DID(type.index, id);
+        internalConfig.storageProvider.setDeviceIdType(type.name());
         internalConfig.storageProvider.setDeviceID(id);
-        SDKCore.instance.notifyModulesDeviceIdChanged(old, withMerge);
+        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);
+            }
+            changeOldRequestsDeviceIds(did);
+        }
+
+        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
-     * @param holder DID object which holds strategy and possibly other info for id generation
      */
-    protected void acquireId(final InternalConfig config, final Config.DID holder) {
-        L.i("[ModuleDeviceIdCore] acquireId, Acquiring device id of strategy [" + holder.strategy + "]");
+    protected Config.DID acquireId(final InternalConfig config) {
+        L.i("[ModuleDeviceIdCore] acquireId, Acquiring device id of strategy [" + config.getDeviceIdStrategy() + "]");
         Config.DID did = null;
 
-        int index = holder.strategy;
+        int index = config.getDeviceIdStrategy();
 
         while (index >= 0) {
             DeviceIdGenerator generator = generators.get(index);
@@ -300,10 +232,39 @@ protected void acquireId(final InternalConfig config, final Config.DID holder) {
                 L.i("[ModuleDeviceIdCore] acquireId, no custom device id. SDK has generated a random device id.");
             }
             L.d("[ModuleDeviceIdCore] acquireId, Got device id: " + did);
-            SDKCore.instance.onDeviceId(config, did, null);
         } else {
             L.i("[ModuleDeviceIdCore] acquireId, No device id of strategy [" + config.getDeviceIdStrategy() + "] is available yet");
         }
+
+        return did;
+    }
+
+    private void changeOldRequestsDeviceIds(Config.DID deviceId) {
+        if (this.tasks == null) {
+            this.tasks = new Tasks("deviceId", L);
+        }
+        tasks.run(new Tasks.Task<Object>(0L) {
+            @Override
+            public Object call() {
+                // put device_id parameter into existing requests
+                L.i("[ModuleDeviceIdCore] changeOldRequestsDeviceIds, Adding device_id to previous requests");
+                boolean success = transformRequests(internalConfig, deviceId.id);
+                if (success) {
+                    L.i("[ModuleDeviceIdCore] changeOldRequestsDeviceIds, First transform: success");
+                } else {
+                    L.w("[ModuleDeviceIdCore] changeOldRequestsDeviceIds, First transform: failure");
+                }
+
+                // do it second time in case new requests were added during first attempt
+                success = transformRequests(internalConfig, deviceId.id);
+                if (!success) {
+                    L.e("[ModuleDeviceIdCore] changeOldRequestsDeviceIds, Failed to put device_id into existing requests, following behaviour for unhandled requests is undefined.");
+                } else {
+                    L.i("[ModuleDeviceIdCore] changeOldRequestsDeviceIds, Second transform: success");
+                }
+                return null;
+            }
+        });
     }
 
     @Override
@@ -316,8 +277,7 @@ public void stop(InternalConfig config, boolean clear) {
     }
 
     protected DeviceIdType getTypeInternal() {
-        //todo impl after merge, delete the Countly one
-        return null;
+        return DeviceIdType.fromInt(internalConfig.getDeviceId().strategy, L);
     }
 
     protected String getIDInternal() {
@@ -355,7 +315,7 @@ public DeviceIdType getType() {
          */
         public void changeWithMerge(String id) {
             synchronized (Countly.instance()) {
-                changeDeviceIdInternal(internalConfig, id, true);
+                changeDeviceIdInternal(id, DeviceIdType.DEVELOPER_SUPPLIED, true);
             }
         }
 
@@ -366,7 +326,7 @@ public void changeWithMerge(String id) {
          */
         public void changeWithoutMerge(String id) {
             synchronized (Countly.instance()) {
-                changeDeviceIdInternal(internalConfig, id, false);
+                changeDeviceIdInternal(id, DeviceIdType.DEVELOPER_SUPPLIED, false);
             }
         }
     }

sdk-java/src/main/java/ly/count/sdk/java/internal/ModuleRemoteConfig.java

@@ -7,7 +7,6 @@
 import java.util.concurrent.ConcurrentHashMap;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
-import ly.count.sdk.java.Config;
 import ly.count.sdk.java.Countly;
 import org.json.JSONArray;
 import org.json.JSONObject;
@@ -273,7 +272,7 @@ private void rcAutomaticDownloadTrigger(final boolean cacheClearOldValues) {
     }
 
     @Override
-    protected void deviceIdChanged(Config.DID oldDeviceId, boolean withMerge) {
+    protected void deviceIdChanged(String oldDeviceId, boolean withMerge) {
         L.v("[ModuleRemoteConfig] deviceIdChanged, Clearing remote config values and preparing to download after ID update, " + !withMerge);
         super.deviceIdChanged(oldDeviceId, withMerge);