From fc92f0bd1e7aacb1012fb35794ad54ee10c51a91 Mon Sep 17 00:00:00 2001 From: CTLalit Date: Fri, 1 Mar 2024 17:41:09 +0530 Subject: [PATCH 1/5] Bug: Correct method calling sequence - fixed init of variables in class first before using them in deviceInfo --- .../main/java/com/clevertap/android/sdk/CleverTapFactory.java | 1 + .../src/main/java/com/clevertap/android/sdk/DeviceInfo.java | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.java b/clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.java index d82d8bb4c..8c102f56d 100644 --- a/clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.java +++ b/clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.java @@ -86,6 +86,7 @@ static CoreState getCoreState(Context context, CleverTapInstanceConfig cleverTap DeviceInfo deviceInfo = new DeviceInfo(context, config, cleverTapID, coreMetaData); coreState.setDeviceInfo(deviceInfo); + deviceInfo.onInitDeviceInfo(cleverTapID); CTPreferenceCache.getInstance(context, config); diff --git a/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java b/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java index 64c4e4003..fb16cb035 100644 --- a/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java +++ b/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java @@ -503,8 +503,6 @@ public static int getDeviceType(final Context context) { this.library = null; this.customLocale = null; mCoreMetaData = coreMetaData; - onInitDeviceInfo(cleverTapID); - getConfigLogger().verbose(config.getAccountId() + ":async_deviceID", "DeviceInfo() called"); } public void forceNewDeviceID() { @@ -758,6 +756,7 @@ private int getLocalInAppCountFromPreference() { } void onInitDeviceInfo(final String cleverTapID) { + getConfigLogger().verbose(config.getAccountId() + ":async_deviceID", "DeviceInfo() called"); Task taskDeviceCachedInfo = CTExecutorFactory.executors(config).ioTask(); taskDeviceCachedInfo.execute("getDeviceCachedInfo", new Callable() { @Override From e4b79481f1504e767710b4296d2a185480322c30 Mon Sep 17 00:00:00 2001 From: CTLalit Date: Fri, 1 Mar 2024 20:02:25 +0530 Subject: [PATCH 2/5] Bug: Pure function return type - did not use device id from state variable (shared pref map) - instead passed device id back from func as it is created on fly - this reduces risk of state variable changing over time as it is accessed in future --- .../com/clevertap/android/sdk/DeviceInfo.java | 65 ++++++++++--------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java b/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java index fb16cb035..ccde7ff97 100644 --- a/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java +++ b/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java @@ -510,16 +510,19 @@ public void forceNewDeviceID() { forceUpdateDeviceId(deviceID); } - public void forceUpdateCustomCleverTapID(String cleverTapID) { + public String forceUpdateCustomCleverTapID(String cleverTapID) { if (Utils.validateCTID(cleverTapID)) { getConfigLogger() .info(config.getAccountId(), "Setting CleverTap ID to custom CleverTap ID : " + cleverTapID); - forceUpdateDeviceId(Constants.CUSTOM_CLEVERTAP_ID_PREFIX + cleverTapID); + String id = Constants.CUSTOM_CLEVERTAP_ID_PREFIX + cleverTapID; + forceUpdateDeviceId(id); + return id; } else { - setOrGenerateFallbackDeviceID(); + String fallbackId = generateFallbackDeviceID(); removeDeviceID(); String error = recordDeviceError(Constants.INVALID_CT_CUSTOM_ID, cleverTapID, getFallBackDeviceID()); getConfigLogger().info(config.getAccountId(), error); + return fallbackId; } } @@ -766,22 +769,21 @@ public Void call() throws Exception { } }); - Task task = CTExecutorFactory.executors(config).ioTask(); - task.addOnSuccessListener(new OnSuccessListener() { + Task task = CTExecutorFactory.executors(config).ioTask(); + task.addOnSuccessListener(new OnSuccessListener() { // callback on main thread @Override - public void onSuccess(final Void aVoid) { + public void onSuccess(final String deviceId) { getConfigLogger().verbose(config.getAccountId() + ":async_deviceID", "DeviceID initialized successfully!" + Thread.currentThread()); // No need to put getDeviceID() on background thread because prefs already loaded - CleverTapAPI.instanceWithConfig(context, config).deviceIDCreated(getDeviceID()); + CleverTapAPI.instanceWithConfig(context, config).deviceIDCreated(deviceId); } }); - task.execute("initDeviceID", new Callable() { + task.execute("initDeviceID", new Callable() { @Override - public Void call() throws Exception { - initDeviceID(cleverTapID); - return null; + public String call() throws Exception { + return initDeviceID(cleverTapID); } }); @@ -862,7 +864,7 @@ private synchronized void fetchGoogleAdID() { } } - private synchronized void generateDeviceID() { + private synchronized String generateDeviceID() { getConfigLogger().verbose(config.getAccountId() + ":async_deviceID", "generateDeviceID() called!"); String generatedDeviceID; String adId = getGoogleAdID(); @@ -875,6 +877,7 @@ private synchronized void generateDeviceID() { } forceUpdateDeviceId(generatedDeviceID); getConfigLogger().verbose(config.getAccountId() + ":async_deviceID", "generateDeviceID() done executing!"); + return generatedDeviceID; } private String generateGUID() { @@ -904,7 +907,7 @@ private String getFallbackIdStorageKey() { return Constants.FALLBACK_ID_TAG + ":" + this.config.getAccountId(); } - private void initDeviceID(String cleverTapID) { + private String initDeviceID(String cleverTapID) { getConfigLogger().verbose(config.getAccountId() + ":async_deviceID", "Called initDeviceID()"); //Show logging as per Manifest flag if (config.getEnableCustomCleverTapId()) { @@ -928,27 +931,27 @@ private void initDeviceID(String cleverTapID) { String error = recordDeviceError(Constants.UNABLE_TO_SET_CT_CUSTOM_ID, deviceID, cleverTapID); getConfigLogger().info(config.getAccountId(), error); } - return; + return deviceID; } if (this.config.getEnableCustomCleverTapId()) { - forceUpdateCustomCleverTapID(cleverTapID); - return; + return forceUpdateCustomCleverTapID(cleverTapID); } if (!this.config.isUseGoogleAdId()) { getConfigLogger().verbose(config.getAccountId() + ":async_deviceID", "Calling generateDeviceID()"); - generateDeviceID(); + String genId = generateDeviceID(); getConfigLogger().verbose(config.getAccountId() + ":async_deviceID", "Called generateDeviceID()"); - return; + return genId; } // fetch the googleAdID to generate GUID //has to be called on background thread fetchGoogleAdID(); - generateDeviceID(); + String genId = generateDeviceID(); getConfigLogger().verbose(config.getAccountId() + ":async_deviceID", "initDeviceID() done executing!"); + return genId; } private String recordDeviceError(int messageCode, String... varargs) { @@ -961,18 +964,18 @@ private void removeDeviceID() { StorageHelper.remove(this.context, getDeviceIdStorageKey()); } - private synchronized void setOrGenerateFallbackDeviceID() { - if (getFallBackDeviceID() == null) { - synchronized (deviceIDLock) { - String fallbackDeviceID = Constants.ERROR_PROFILE_PREFIX + UUID.randomUUID().toString() - .replace("-", ""); - if (fallbackDeviceID.trim().length() > 2) { - updateFallbackID(fallbackDeviceID); - } else { - getConfigLogger() - .verbose(this.config.getAccountId(), "Unable to generate fallback error device ID"); - } - } + private synchronized String generateFallbackDeviceID() { + String existingId = getFallBackDeviceID(); + + if (existingId != null) { + return existingId; + } + + synchronized (deviceIDLock) { + String fallbackDeviceID = Constants.ERROR_PROFILE_PREFIX + UUID.randomUUID().toString() + .replace("-", ""); + updateFallbackID(fallbackDeviceID); + return fallbackDeviceID; } } From 906e3ad39806970998a304daf82fe8644fa10ec9 Mon Sep 17 00:00:00 2001 From: CTLalit Date: Fri, 1 Mar 2024 20:06:41 +0530 Subject: [PATCH 3/5] Bug: Simplified redundant condition - simplified ternary as it would always return null in both cases --- .../main/java/com/clevertap/android/sdk/DeviceInfo.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java b/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java index ccde7ff97..ef192add8 100644 --- a/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java +++ b/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java @@ -807,12 +807,7 @@ void setDeviceNetworkInfoReportingFromStorage() { private String _getDeviceID() { synchronized (deviceIDLock) { - if (this.config.isDefaultInstance()) { - String _new = StorageHelper.getString(this.context, getDeviceIdStorageKey(), null); - return _new != null ? _new : StorageHelper.getString(this.context, Constants.DEVICE_ID_TAG, null); - } else { - return StorageHelper.getString(this.context, getDeviceIdStorageKey(), null); - } + return StorageHelper.getString(this.context, getDeviceIdStorageKey(), null); } } From bb27b1bcb510f719f2da2d636fdab6707ec23a77 Mon Sep 17 00:00:00 2001 From: CTLalit Date: Fri, 1 Mar 2024 20:15:07 +0530 Subject: [PATCH 4/5] Bug: Use passed device id - the device id passed as method return type to be used - did not fetch device id from state (shared pref map) --- .../java/com/clevertap/android/sdk/CleverTapAPI.java | 4 ++-- .../com/clevertap/android/sdk/CleverTapFactory.java | 4 ++-- .../java/com/clevertap/android/sdk/StoreProvider.kt | 12 ++++++------ .../android/sdk/inapp/ImpressionManagerTest.kt | 2 +- .../clevertap/android/sdk/inapp/StoreProviderTest.kt | 4 ++-- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapAPI.java b/clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapAPI.java index c2b6522f6..d0e3c8ff4 100644 --- a/clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapAPI.java +++ b/clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapAPI.java @@ -2767,13 +2767,13 @@ void deviceIDCreated(String deviceId) { StoreProvider storeProvider = StoreProvider.getInstance(); if (storeRegistry.getInAppStore() == null) { - InAppStore inAppStore = storeProvider.provideInAppStore(context, cryptHandler, deviceInfo, + InAppStore inAppStore = storeProvider.provideInAppStore(context, cryptHandler, deviceId, accountId); storeRegistry.setInAppStore(inAppStore); coreState.getCallbackManager().addChangeUserCallback(inAppStore); } if (storeRegistry.getImpressionStore() == null) { - ImpressionStore impStore = storeProvider.provideImpressionStore(context, deviceInfo, + ImpressionStore impStore = storeProvider.provideImpressionStore(context, deviceId, accountId); storeRegistry.setImpressionStore(impStore); coreState.getCallbackManager().addChangeUserCallback(impStore); diff --git a/clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.java b/clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.java index 8c102f56d..f67404540 100644 --- a/clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.java +++ b/clevertap-core/src/main/java/com/clevertap/android/sdk/CleverTapFactory.java @@ -125,14 +125,14 @@ static CoreState getCoreState(Context context, CleverTapInstanceConfig cleverTap } if (coreState.getDeviceInfo() != null && coreState.getDeviceInfo().getDeviceID() != null) { if (storeRegistry.getInAppStore() == null) { - InAppStore inAppStore = storeProvider.provideInAppStore(context, cryptHandler, deviceInfo, + InAppStore inAppStore = storeProvider.provideInAppStore(context, cryptHandler, deviceInfo.getDeviceID(), config.getAccountId()); storeRegistry.setInAppStore(inAppStore); evaluationManager.loadSuppressedCSAndEvaluatedSSInAppsIds(); callbackManager.addChangeUserCallback(inAppStore); } if (storeRegistry.getImpressionStore() == null) { - ImpressionStore impStore = storeProvider.provideImpressionStore(context, deviceInfo, + ImpressionStore impStore = storeProvider.provideImpressionStore(context, deviceInfo.getDeviceID(), config.getAccountId()); storeRegistry.setImpressionStore(impStore); callbackManager.addChangeUserCallback(impStore); diff --git a/clevertap-core/src/main/java/com/clevertap/android/sdk/StoreProvider.kt b/clevertap-core/src/main/java/com/clevertap/android/sdk/StoreProvider.kt index 48e43e5a7..46ba97a2a 100644 --- a/clevertap-core/src/main/java/com/clevertap/android/sdk/StoreProvider.kt +++ b/clevertap-core/src/main/java/com/clevertap/android/sdk/StoreProvider.kt @@ -70,17 +70,17 @@ class StoreProvider { * * @param context The Android application context. * @param cryptHandler The handler used for encryption and decryption of In-App messages. - * @param deviceInfo The information about the device. + * @param deviceId The device id for user. * @param accountId The unique account identifier. * @return An instance of [InAppStore]. */ fun provideInAppStore( context: Context, cryptHandler: CryptHandler, - deviceInfo: DeviceInfo, + deviceId: String, accountId: String ): InAppStore { - val prefName = constructStorePreferenceName(STORE_TYPE_INAPP, deviceInfo.deviceID, accountId) + val prefName = constructStorePreferenceName(STORE_TYPE_INAPP, deviceId, accountId) return InAppStore(getCTPreference(context, prefName), cryptHandler) } @@ -88,16 +88,16 @@ class StoreProvider { * Provides an instance of [ImpressionStore] using the given parameters. * * @param context The Android application context. - * @param deviceInfo The information about the device. + * @param deviceId The device id for user. * @param accountId The unique account identifier. * @return An instance of [ImpressionStore]. */ fun provideImpressionStore( context: Context, - deviceInfo: DeviceInfo, + deviceId: String, accountId: String ): ImpressionStore { - val prefName = constructStorePreferenceName(STORE_TYPE_IMPRESSION, deviceInfo.deviceID, accountId) + val prefName = constructStorePreferenceName(STORE_TYPE_IMPRESSION, deviceId, accountId) return ImpressionStore(getCTPreference(context, prefName)) } diff --git a/clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/ImpressionManagerTest.kt b/clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/ImpressionManagerTest.kt index af89eece7..9e9e0945c 100644 --- a/clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/ImpressionManagerTest.kt +++ b/clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/ImpressionManagerTest.kt @@ -41,7 +41,7 @@ class ImpressionManagerTest : BaseTestCase() { `when`(deviceInfo.deviceID).thenReturn("device_id") impressionStore = StoreProvider.getInstance().provideImpressionStore( - appCtx, deviceInfo, "account_id" + appCtx, deviceInfo.deviceID, "account_id" ) storeRegistry.impressionStore = impressionStore diff --git a/clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/StoreProviderTest.kt b/clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/StoreProviderTest.kt index 8b16fe354..6d180796b 100644 --- a/clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/StoreProviderTest.kt +++ b/clevertap-core/src/test/java/com/clevertap/android/sdk/inapp/StoreProviderTest.kt @@ -39,7 +39,7 @@ class StoreProviderTest { every { storeProvider.constructStorePreferenceName(STORE_TYPE_INAPP, mockDeviceInfo.deviceID, accountId) } returns prefName // Act - val inAppStore = storeProvider.provideInAppStore(mockContext, mockCryptHandler, mockDeviceInfo, accountId) + val inAppStore = storeProvider.provideInAppStore(mockContext, mockCryptHandler, mockDeviceInfo.deviceID, accountId) // Assert verify { storeProvider.getCTPreference(mockContext, prefName) } @@ -54,7 +54,7 @@ class StoreProviderTest { every { storeProvider.constructStorePreferenceName(STORE_TYPE_IMPRESSION, mockDeviceInfo.deviceID, accountId) } returns prefName // Act - val impressionStore = storeProvider.provideImpressionStore(mockContext, mockDeviceInfo, accountId) + val impressionStore = storeProvider.provideImpressionStore(mockContext, mockDeviceInfo.deviceID, accountId) // Assert verify { storeProvider.getCTPreference(mockContext, prefName) } From d506463ddf121ca98d0686d36c072aac65b4ded9 Mon Sep 17 00:00:00 2001 From: CTLalit Date: Tue, 19 Mar 2024 20:32:55 +0530 Subject: [PATCH 5/5] Chore: Fixes pr comments - comment fixed - seems some unreachable code can be due to legacy --- .../java/com/clevertap/android/sdk/DeviceInfo.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java b/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java index ef192add8..3cb2038ae 100644 --- a/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java +++ b/clevertap-core/src/main/java/com/clevertap/android/sdk/DeviceInfo.java @@ -583,7 +583,8 @@ public int getDPI() { } public String getDeviceID() { - return _getDeviceID() != null ? _getDeviceID() : getFallBackDeviceID(); + String currentId = _getDeviceID(); + return currentId != null ? currentId : getFallBackDeviceID(); } public String getGoogleAdID() { @@ -806,8 +807,12 @@ void setDeviceNetworkInfoReportingFromStorage() { } private String _getDeviceID() { - synchronized (deviceIDLock) { - return StorageHelper.getString(this.context, getDeviceIdStorageKey(), null); + String _new = StorageHelper.getString(this.context, getDeviceIdStorageKey(), null); + + if (this.config.isDefaultInstance()) { + return _new != null ? _new : StorageHelper.getString(this.context, Constants.DEVICE_ID_TAG, null); + } else { + return _new; } }