Additional internal device ID refactoring. Adding null checks.

ArtursKadikis · ArtursKadikis · commit 242720baef55 · 2022-10-02T21:09:25.000+03:00
diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/DeviceIdTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/DeviceIdTests.java
@@ -167,48 +167,22 @@ public void getId() {
      * Validating 'changeToId' with developer supplied values
      */
     @Test
-    public void changeToIdDevSupplied() {
+    public void changeToCustomIdAndEnterTempIDMode() {
         DeviceId did = new DeviceId(DeviceIdType.DEVELOPER_SUPPLIED, "abc", store, mock(ModuleLog.class), null);
         assertEquals("abc", did.getCurrentId());
-
-        did.changeToId(DeviceIdType.DEVELOPER_SUPPLIED, "123");
-        assertEquals("123", did.getCurrentId());
         assertEquals(DeviceIdType.DEVELOPER_SUPPLIED, did.getType());
 
-        did.changeToId(DeviceIdType.DEVELOPER_SUPPLIED, "456");
-        assertEquals("456", did.getCurrentId());
+        did.changeToCustomId("123");
+        assertEquals("123", did.getCurrentId());
         assertEquals(DeviceIdType.DEVELOPER_SUPPLIED, did.getType());
-    }
-
-    /**
-     * Validating 'changeToId' around openUDID
-     */
-    @Test
-    public void changeToIdOpenUDID() {
-        DeviceId did = new DeviceId(DeviceIdType.DEVELOPER_SUPPLIED, "abc", store, mock(ModuleLog.class), openUDIDProvider);
-        assertEquals("abc", did.getCurrentId());
 
-        //set first value without running init, should use the provided value
-        did.changeToId(DeviceIdType.OPEN_UDID, "123");
-        assertEquals("123", did.getCurrentId());
-        assertEquals(DeviceIdType.OPEN_UDID, did.getType());
+        did.enterTempIDMode();
+        assertEquals(DeviceId.temporaryCountlyDeviceId, did.getCurrentId());
+        assertEquals(DeviceIdType.TEMPORARY_ID, did.getType());
 
-        //do a reset
-        did.changeToId(DeviceIdType.DEVELOPER_SUPPLIED, "aaa");
+        did.changeToCustomId("aaa");
         assertEquals("aaa", did.getCurrentId());
         assertEquals(DeviceIdType.DEVELOPER_SUPPLIED, did.getType());
-
-        //change with init, since a value is specified, it should take precedence
-        currentOpenUDIDValue = "uio|";
-        did.changeToId(DeviceIdType.OPEN_UDID, "456");
-        assertEquals("456", did.getCurrentId());
-        assertEquals(DeviceIdType.OPEN_UDID, did.getType());
-
-        //change with init, it should use it's own value because a null device ID is provided
-        currentOpenUDIDValue = "sdfh";
-        did.changeToId(DeviceIdType.OPEN_UDID, null);
-        assertEquals(currentOpenUDIDValue, did.getCurrentId());
-        assertEquals(DeviceIdType.OPEN_UDID, did.getType());
     }
 
     /**
diff --git a/sdk/src/main/java/ly/count/android/sdk/ConnectionProcessor.java b/sdk/src/main/java/ly/count/android/sdk/ConnectionProcessor.java
@@ -362,7 +362,11 @@ public void run() {
                         storageProvider_.removeRequest(storedEvents[0]);
 
                         if (deviceIdChange) {
-                            deviceIdProvider_.getDeviceIdInstance().changeToId(DeviceIdType.DEVELOPER_SUPPLIED, newId);//todo needs to be refactored
+                            if(newId != null && !newId.isEmpty()) {
+                                deviceIdProvider_.getDeviceIdInstance().changeToCustomId(newId);//todo needs to be refactored
+                            } else {
+                                L.e("[Connection Processor] Failed to change device ID with merging because the new ID was empty or null. [" + newId + "]");
+                            }
                         }
 
                         if (deviceIdChange || deviceIdOverride) {
diff --git a/sdk/src/main/java/ly/count/android/sdk/Countly.java b/sdk/src/main/java/ly/count/android/sdk/Countly.java
@@ -482,6 +482,11 @@ public synchronized Countly init(CountlyConfig config) {
                 L.d("[Init] Parameter tampering protection salt set");
             }
 
+            if(connectionQueue_ == null) {
+                L.e("[Init] SDK failed to initialize because the connection queue failed to be created");
+                return this;
+            }
+
             //check legacy access methods
             if (locationFallback != null && config.locationCountyCode == null && config.locationCity == null && config.locationLocation == null && config.locationIpAddress == null) {
                 //if the fallback was set and config did not contain any location, use the fallback info
@@ -557,9 +562,8 @@ public synchronized Countly init(CountlyConfig config) {
             if (config.customNetworkRequestHeaders != null) {
                 L.i("[Countly] Calling addCustomNetworkRequestHeaders");
                 requestHeaderCustomValues = config.customNetworkRequestHeaders;
-                if (connectionQueue_ != null) {
-                    connectionQueue_.setRequestHeaderCustomValues(requestHeaderCustomValues);
-                }
+
+                connectionQueue_.setRequestHeaderCustomValues(requestHeaderCustomValues);
             }
 
             if (config.httpPostForced) {
@@ -932,7 +936,17 @@ public void changeDeviceIdWithoutMerge(DeviceId.Type type, String deviceId) {
             return;
         }
 
-        moduleDeviceId.changeDeviceIdWithoutMergeInternal(ModuleDeviceId.fromOldDeviceIdToNew(type), deviceId);
+        if(deviceId == null || deviceId.isEmpty()) {
+            L.e("changeDeviceIdWithoutMerge, can't change device ID to and empty or null value");
+            return;
+        }
+
+        if(type != DeviceId.Type.DEVELOPER_SUPPLIED) {
+            L.e("changeDeviceIdWithoutMerge, provided device ID type mus be 'DEVELOPER_SUPPLIED'");
+            return;
+        }
+
+        moduleDeviceId.changeDeviceIdWithoutMergeInternal(deviceId);
     }
 
     /**
diff --git a/sdk/src/main/java/ly/count/android/sdk/CountlyStore.java b/sdk/src/main/java/ly/count/android/sdk/CountlyStore.java
@@ -113,7 +113,7 @@ static SharedPreferences createPreferencesPush(Context context) {
         return context.getSharedPreferences(PREFERENCES_PUSH, Context.MODE_PRIVATE);
     }
 
-    private @Nullable String storageReadRequestQueue() {
+    private @NonNull String storageReadRequestQueue() {
         if(explicitStorageModeEnabled) {
             //L.v("[CountlyStore] Returning RQ from cache");
             if(esRequestQueueCache == null) {
@@ -145,7 +145,7 @@ private void storageWriteRequestQueue(@Nullable String requestQueue, boolean wri
         }
     }
 
-    private @Nullable String storageReadEventQueue() {
+    private @NonNull String storageReadEventQueue() {
         if(explicitStorageModeEnabled) {
             //L.v("[CountlyStore] Returning EQ from cache");
             if(esEventQueueCache == null) {
diff --git a/sdk/src/main/java/ly/count/android/sdk/CrashDetails.java b/sdk/src/main/java/ly/count/android/sdk/CrashDetails.java
@@ -74,7 +74,11 @@ private static long getTotalRAM() {
                     value = m.group(1);
                 }
                 try {
-                    totalMemory = Long.parseLong(value) / 1024;
+                    if(value != null) {
+                        totalMemory = Long.parseLong(value) / 1024;
+                    } else {
+                        totalMemory = 0;
+                    }
                 } catch (NumberFormatException ex) {
                     totalMemory = 0;
                 }
diff --git a/sdk/src/main/java/ly/count/android/sdk/DeviceId.java b/sdk/src/main/java/ly/count/android/sdk/DeviceId.java
@@ -6,6 +6,7 @@
 public class DeviceId {
     /**
      * Enum used throughout Countly which controls what kind of ID Countly should use.
+     *
      * @deprecated Replace this type with "DeviceIdType"
      */
     public enum Type {
@@ -34,92 +35,69 @@ public enum Type {
     @NonNull
     OpenUDIDProvider openUDIDProvider;
 
-    protected DeviceId(@NonNull DeviceIdType type, @Nullable String developerSuppliedId, @NonNull StorageProvider givenStorageProvider, @NonNull ModuleLog moduleLog, @NonNull OpenUDIDProvider openUDIDProvider) {
-        if(type == DeviceIdType.DEVELOPER_SUPPLIED) {
-            if (developerSuppliedId == null || "".equals(developerSuppliedId)) {
-                throw new IllegalStateException("If using developer supplied type, a valid device ID should be provided, [" + developerSuppliedId + "]");
+    protected DeviceId(@NonNull DeviceIdType providedType, @Nullable String providedId, @NonNull StorageProvider givenStorageProvider, @NonNull ModuleLog moduleLog, @NonNull OpenUDIDProvider openUDIDProvider) {
+        if (providedType == DeviceIdType.DEVELOPER_SUPPLIED) {
+            if (providedId == null || "".equals(providedId)) {
+                throw new IllegalStateException("If using developer supplied type, a valid device ID should be provided, [" + providedId + "]");
             }
-        } else if(type == DeviceIdType.TEMPORARY_ID) {
-            if (developerSuppliedId == null || "".equals(developerSuppliedId)) {
-                throw new IllegalStateException("If using temporary ID type, a valid device ID should be provided, [" + developerSuppliedId + "]");
+        } else if (providedType == DeviceIdType.TEMPORARY_ID) {
+            if (providedId == null || "".equals(providedId)) {
+                throw new IllegalStateException("If using temporary ID type, a valid device ID should be provided, [" + providedId + "]");
             }
 
-            if(!developerSuppliedId.equals(temporaryCountlyDeviceId)) {
-                throw new IllegalStateException("If using temporary ID type, the device ID value should be the required one, [" + developerSuppliedId + "]");
+            if (!providedId.equals(temporaryCountlyDeviceId)) {
+                throw new IllegalStateException("If using temporary ID type, the device ID value should be the required one, [" + providedId + "]");
             }
-
-        } else if(type == DeviceIdType.OPEN_UDID || type == DeviceIdType.ADVERTISING_ID){
-
+        } else if (providedType == DeviceIdType.OPEN_UDID || providedType == DeviceIdType.ADVERTISING_ID) {
+            //just adding this check for completeness
         } else {
             throw new IllegalStateException("Null device ID type is not allowed");
         }
 
         storageProvider = givenStorageProvider;
         this.openUDIDProvider = openUDIDProvider;
-
-        //setup the preferred device ID type
-        this.type = type;
-        this.id = developerSuppliedId;
         L = moduleLog;
 
-
-        L.d("[DeviceId-int] initialising with values, device ID:[" + this.id + "] type:[" + this.type + "]");
+        L.d("[DeviceId-int] initialising with values, device ID:[" + providedId + "] type:[" + providedType + "]");
 
         //check if there wasn't a value set before. Read if from storage
         String storedId = storageProvider.getDeviceID();
-        if (storedId != null) {
-            //if there was a value saved previously, set it and it's type. Overwrite the in constructor set ones
-            this.id = storedId;
-            this.type = retrieveStoredType();
-
-            L.d("[DeviceId-int] retrieveId, Retrieving a previously set device ID:[" + this.id + "] type:[" + this.type + "]");
-        } else {
-            L.d("[DeviceId-int] retrieveId, no previous ID stored");
-        }
-
-        //call init implicitly
-        init();
-    }
-
-    /**
-     * Initialize device ID generation
-     */
-    private void init() {
         DeviceIdType storedType = retrieveStoredType();
-        L.d("[DeviceId-int] init, current type:[" + type + "] overriddenType:[" + storedType + "]");
 
-        // Some time ago some ID generation strategy was not available and SDK fell back to
-        // some other strategy. We still have to use that strategy.
-        if (storedType != null && storedType != type) {
-            L.i("[DeviceId-int] init, Overridden device ID generation strategy detected: " + storedType + ", using it instead of " + this.type);
-            type = storedType;
-        }
+        L.d("[DeviceId-int] The following values were stored, device ID:[" + storedId + "] type:[" + storedType + "]");
 
-        if (type == null) {
-            L.e("[DeviceId-int] init, device id type currently is null, falling back to OPEN_UDID");
-            type = DeviceIdType.OPEN_UDID;
-        }
+        if (storedId != null && storedType != null) {
+            //values are set, just use them and ignore the provided ones
+            id = storedId;
+            type = storedType;
+        } else {
+            if (storedType == null && storedId != null) {
+                L.e("[DeviceId-int] init, device id type currently is null, falling back to OPEN_UDID");
+                setAndStoreId(DeviceIdType.OPEN_UDID, storedId);
+            }
 
-        String storedID = storageProvider.getDeviceID();
-
-        if (storedID == null) {
-            //id value will be regenerated only if the values isn't already set
-            //this is to prevent the device id to change in case the underlying mechanism for openUDID or advertising ID changes
-            switch (type) {
-                case DEVELOPER_SUPPLIED:
-                    // no initialization for developer id
-                    // we just store the provided value so that it's
-                    setAndStoreId(DeviceIdType.DEVELOPER_SUPPLIED, id);
-                    break;
-                case OPEN_UDID:
-                    L.i("[DeviceId-int] Using OpenUDID");
-                    fallbackToOpenUDID();
-                    break;
-                case ADVERTISING_ID:
-                    // Fall back to OpenUDID on devices without google play services set up
-                    L.i("[DeviceId-int] Use of Advertising ID is deprecated, falling back to OpenUDID");
-                    fallbackToOpenUDID();
-                    break;
+            if (storedId == null) {
+                //id value will be regenerated only if the values isn't already set
+                //this is to prevent the device id to change in case the underlying mechanism for openUDID or advertising ID changes
+                switch (providedType) {
+                    case TEMPORARY_ID:
+                        setAndStoreId(DeviceIdType.TEMPORARY_ID, providedId);
+                        break;
+                    case DEVELOPER_SUPPLIED:
+                        // no initialization for developer id
+                        // we just store the provided value so that it's
+                        setAndStoreId(DeviceIdType.DEVELOPER_SUPPLIED, providedId);
+                        break;
+                    case OPEN_UDID:
+                        L.i("[DeviceId-int] Using OpenUDID");
+                        setAndStoreId(DeviceIdType.OPEN_UDID, openUDIDProvider.getOpenUDID());
+                        break;
+                    case ADVERTISING_ID:
+                        // Fall back to OpenUDID on devices without google play services set up
+                        L.i("[DeviceId-int] Use of Advertising ID is deprecated, falling back to OpenUDID");
+                        setAndStoreId(DeviceIdType.OPEN_UDID, openUDIDProvider.getOpenUDID());
+                        break;
+                }
             }
         }
     }
@@ -158,6 +136,7 @@ protected String getCurrentId() {
 
     /**
      * Used only for tests
+     *
      * @param type
      * @param id
      */
@@ -168,19 +147,19 @@ protected void setId(DeviceIdType type, String id) {
         this.id = id;
     }
 
-    protected void fallbackToOpenUDID() {
-        setAndStoreId(DeviceIdType.OPEN_UDID, openUDIDProvider.getOpenUDID());
-    }
-
     /**
      * If a value is provided, it will take precedence and will not matter what the type is
      *
-     * @param type
      * @param deviceId
      */
-    protected void changeToId(@NonNull DeviceIdType type, @Nullable String deviceId) {
-        L.v("[DeviceId-int] changeToId, Device ID is " + id + " (type " + type + ")");
-        setAndStoreId(type, deviceId);
+    protected void changeToCustomId(@NonNull String deviceId) {
+        L.v("[DeviceId-int] changeToCustomId, Device ID is " + id);
+        setAndStoreId(DeviceIdType.DEVELOPER_SUPPLIED, deviceId);
+    }
+
+    protected void enterTempIDMode() {
+        L.v("[DeviceId-int] enterTempIDMode");
+        setAndStoreId(DeviceIdType.DEVELOPER_SUPPLIED, ly.count.android.sdk.DeviceId.temporaryCountlyDeviceId);
     }
 
     void setAndStoreId(DeviceIdType type, String deviceId) {
diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleCrash.java b/sdk/src/main/java/ly/count/android/sdk/ModuleCrash.java
@@ -163,7 +163,7 @@ void enableCrashReporting() {
         Thread.UncaughtExceptionHandler handler = new Thread.UncaughtExceptionHandler() {
 
             @Override
-            public void uncaughtException(@NonNull Thread t, Throwable e) {
+            public void uncaughtException(@NonNull Thread t, @NonNull Throwable e) {
                 L.d("[ModuleCrash] Uncaught crash handler triggered");
                 if (consentProvider.getConsent(Countly.CountlyFeatureNames.crashes)) {
 
diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleDeviceId.java b/sdk/src/main/java/ly/count/android/sdk/ModuleDeviceId.java
diff --git a/sdk/src/main/java/ly/count/android/sdk/ModuleEvents.java b/sdk/src/main/java/ly/count/android/sdk/ModuleEvents.java
diff --git a/sdk/src/main/java/ly/count/android/sdk/messaging/CountlyPushActivity.java b/sdk/src/main/java/ly/count/android/sdk/messaging/CountlyPushActivity.java

Original file line number	Diff line number	Diff line change
`@@ -113,7 +113,7 @@ static SharedPreferences createPreferencesPush(Context context) {`
`113`	`113`	`return context.getSharedPreferences(PREFERENCES_PUSH, Context.MODE_PRIVATE);`
`114`	`114`	`}`
`115`	`115`
`116`		`- private @Nullable String storageReadRequestQueue() {`
	`116`	`+ private @NonNull String storageReadRequestQueue() {`
`117`	`117`	`if(explicitStorageModeEnabled) {`
`118`	`118`	`//L.v("[CountlyStore] Returning RQ from cache");`
`119`	`119`	`if(esRequestQueueCache == null) {`
`@@ -145,7 +145,7 @@ private void storageWriteRequestQueue(@Nullable String requestQueue, boolean wri`
`145`	`145`	`}`
`146`	`146`	`}`
`147`	`147`
`148`		`- private @Nullable String storageReadEventQueue() {`
	`148`	`+ private @NonNull String storageReadEventQueue() {`
`149`	`149`	`if(explicitStorageModeEnabled) {`
`150`	`150`	`//L.v("[CountlyStore] Returning EQ from cache");`
`151`	`151`	`if(esEventQueueCache == null) {`
Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,11 @@ private static long getTotalRAM() {`
`74`	`74`	`value = m.group(1);`
`75`	`75`	`}`
`76`	`76`	`try {`
`77`		`- totalMemory = Long.parseLong(value) / 1024;`
	`77`	`+ if(value != null) {`
	`78`	`+ totalMemory = Long.parseLong(value) / 1024;`
	`79`	`+ } else {`
	`80`	`+ totalMemory = 0;`
	`81`	`+ }`
`78`	`82`	`} catch (NumberFormatException ex) {`
`79`	`83`	`totalMemory = 0;`
`80`	`84`	`}`