Skip to content

Commit

Permalink
Additional internal device ID refactoring. Adding null checks.
Browse files Browse the repository at this point in the history
  • Loading branch information
@ArtursKadikis
ArtursKadikis committed Oct 2, 2022
1 parent d516365 commit 242720b
Show file tree
Hide file tree
Showing 10 changed files with 135 additions and 156 deletions.
40 changes: 7 additions & 33 deletions sdk/src/androidTest/java/ly/count/android/sdk/DeviceIdTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

/**
Expand Down
6 changes: 5 additions & 1 deletion sdk/src/main/java/ly/count/android/sdk/ConnectionProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
22 changes: 18 additions & 4 deletions sdk/src/main/java/ly/count/android/sdk/Countly.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions sdk/src/main/java/ly/count/android/sdk/CountlyStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 5 additions & 1 deletion sdk/src/main/java/ly/count/android/sdk/CrashDetails.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
129 changes: 54 additions & 75 deletions sdk/src/main/java/ly/count/android/sdk/DeviceId.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
}
}
}
Expand Down Expand Up @@ -158,6 +136,7 @@ protected String getCurrentId() {

/**
* Used only for tests
*
* @param type
* @param id
*/
Expand All @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion sdk/src/main/java/ly/count/android/sdk/ModuleCrash.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {

Expand Down
Loading

0 comments on commit 242720b

Please sign in to comment.