Skip to content

Commit

Permalink
feat: persist device ID
Browse files Browse the repository at this point in the history
  • Loading branch information
fractalwrench committed Dec 1, 2020
1 parent d56cb66 commit ca24c6c
Show file tree
Hide file tree
Showing 11 changed files with 443 additions and 58 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## TBD

* Use consistent device ID for multi process apps
[#1013](https://github.com/bugsnag/bugsnag-android/pull/1013)

* Create synchronized store for user information
[#1010](https://github.com/bugsnag/bugsnag-android/pull/1010)

Expand Down
2 changes: 1 addition & 1 deletion bugsnag-android-core/detekt-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@
<ID>TooGenericExceptionCaught:CallbackState.kt$CallbackState$ex: Throwable</ID>
<ID>TooGenericExceptionCaught:DefaultDelivery.kt$DefaultDelivery$exception: Exception</ID>
<ID>TooGenericExceptionCaught:DeviceDataCollector.kt$DeviceDataCollector$exception: Exception</ID>
<ID>TooGenericExceptionCaught:DeviceIdStore.kt$DeviceIdStore$exc: Throwable</ID>
<ID>TooGenericExceptionCaught:ManifestConfigLoader.kt$ManifestConfigLoader$exc: Exception</ID>
<ID>TooGenericExceptionCaught:PluginClient.kt$PluginClient$exc: Throwable</ID>
<ID>TooGenericExceptionCaught:Stacktrace.kt$Stacktrace$lineEx: Exception</ID>
<ID>TooGenericExceptionCaught:SynchronizedStreamableStore.kt$SynchronizedStreamableStore$exc: Throwable</ID>
<ID>TooGenericExceptionThrown:BreadcrumbStateTest.kt$BreadcrumbStateTest$throw Exception("Oh no")</ID>
<ID>TooManyFunctions:ConfigInternal.kt$ConfigInternal : CallbackAwareMetadataAwareUserAware</ID>
<ID>TooManyFunctions:DeviceDataCollector.kt$DeviceDataCollector</ID>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ public void testStoreUserInPrefsDisabled() {

// Check that the user was not stored in prefs
SharedPreferences sharedPref = getSharedPrefs(context);
assertNotNull(sharedPref.getString("install.iud", null));
assertFalse(sharedPref.contains("user.id"));
assertFalse(sharedPref.contains("user.email"));
assertFalse(sharedPref.contains("user.name"));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
package com.bugsnag.android

import android.content.Context
import androidx.test.core.app.ApplicationProvider
import junit.framework.TestCase.assertNull
import org.junit.Assert.assertEquals
import org.junit.Before
import org.junit.Test
import java.io.File
import java.util.UUID
import java.util.concurrent.CountDownLatch
import java.util.concurrent.Executors

internal class DeviceIdStoreTest {

lateinit var file: File
lateinit var storageDir: File
lateinit var ctx: Context

private val uuidProvider = {
UUID.fromString("ab0c1482-2ffe-11eb-adc1-0242ac120002")
}
private val diffUuidProvider = {
UUID.fromString("d9901bff-2ffe-11eb-adc1-0242ac120002")
}

@Before
fun setUp() {
ctx = ApplicationProvider.getApplicationContext<Context>()
storageDir = ctx.cacheDir
file = File(storageDir, "device.json")
file.delete()
}

/**
* A file should be created if it does not already exist
*/
@Test
fun nonExistentFile() {
val nonExistentFile = File(storageDir, "foo")
nonExistentFile.delete()
val store = DeviceIdStore(ctx, nonExistentFile, NoopLogger)
val deviceId = store.loadDeviceId(uuidProvider)
requireNotNull(deviceId)
assertEquals("ab0c1482-2ffe-11eb-adc1-0242ac120002", deviceId)
}

/**
* An empty file should be overwritten with a new device ID
*/
@Test
fun emptyFile() {
val store = DeviceIdStore(ctx, file, NoopLogger)
val deviceId = store.loadDeviceId(uuidProvider)
requireNotNull(deviceId)
assertEquals("ab0c1482-2ffe-11eb-adc1-0242ac120002", deviceId)
}

/**
* A file of the incorrect length should be overwritten with a new device ID
*/
@Test
fun incorrectFileLength() {
val store = DeviceIdStore(ctx, file, NoopLogger)
val deviceId = store.loadDeviceId(uuidProvider)
requireNotNull(deviceId)
assertEquals("ab0c1482-2ffe-11eb-adc1-0242ac120002", deviceId)
}

/**
* A file of the correct length with invalid contents should be overwritten with a new device ID
*/
@Test
fun invalidFileContents() {
file.writeText("{\"hamster\": 2}")
val store = DeviceIdStore(ctx, file, NoopLogger)
val deviceId = store.loadDeviceId(uuidProvider)
requireNotNull(deviceId)
assertEquals("ab0c1482-2ffe-11eb-adc1-0242ac120002", deviceId)
}

/**
* A valid file should not be overwritten with a new device ID
*/
@Test
fun validFileContents() {
file.writeText("{\"id\": \"24c51482-2ffe-11eb-adc1-0242ac120002\"}")
val store = DeviceIdStore(ctx, file, NoopLogger)

// device ID is loaded without writing file
assertEquals(
"24c51482-2ffe-11eb-adc1-0242ac120002",
store.loadDeviceId(uuidProvider)
)
// same device ID is retrieved as before
assertEquals(
"24c51482-2ffe-11eb-adc1-0242ac120002",
store.loadDeviceId(diffUuidProvider)
)
}

/**
* A non-writable file does not crash the app
*/
@Test
fun nonWritableFile() {
val nonReadableFile = File(storageDir, "foo").apply {
delete()
createNewFile()
setWritable(false)
}
val store = DeviceIdStore(ctx, nonReadableFile, NoopLogger)
val deviceId = store.loadDeviceId(uuidProvider)
assertNull(deviceId)
}

/**
* The device ID store should take out a file lock to prevent concurrent writes to the file.
*/
@Test(timeout = 2000)
fun fileLockUsed() {
val store = DeviceIdStore(ctx, file, NoopLogger)

// load the device ID on many different background threads.
// each thread races with each other, but only one should generate a device ID
// and persist it to disk.
val deviceIds = mutableSetOf<String>()
val attempts = 16
val executor = Executors.newFixedThreadPool(attempts)
val latch = CountDownLatch(attempts)

repeat(attempts) {
executor.submit {
val id = store.loadDeviceId(uuidProvider)
requireNotNull(id)
deviceIds.add(id)
latch.countDown()
}
}
latch.await()

// validate that the device ID is consistent for each.
// the device ID of whichever thread won the race first should be used.
assertEquals(1, deviceIds.size)
}

/**
* The device ID store should migrate legacy IDs from shared preferences if they are present
*/
@Test
fun sharedPrefMigration() {
val store = DeviceIdStore(ctx, file, NoopLogger)
val context = ApplicationProvider.getApplicationContext<Context>()

val prefs =
context.getSharedPreferences("com.bugsnag.android", Context.MODE_PRIVATE)
prefs.edit().putString("install.iud", "55670bff-9024-fc0a-b392-0242ac88ccd9").commit()

val prefDeviceId = SharedPrefMigrator(context).loadDeviceId()
val storeDeviceId = store.loadDeviceId()
assertEquals("55670bff-9024-fc0a-b392-0242ac88ccd9", storeDeviceId)
assertEquals(prefDeviceId, storeDeviceId)
}
}
35 changes: 22 additions & 13 deletions bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
import androidx.annotation.VisibleForTesting;

import kotlin.Unit;
import kotlin.jvm.functions.Function0;
import kotlin.jvm.functions.Function2;

import java.io.File;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
Expand All @@ -28,6 +30,7 @@
import java.util.Map;
import java.util.Observer;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.RejectedExecutionException;

/**
Expand Down Expand Up @@ -152,29 +155,23 @@ public Unit invoke(Boolean hasConnection, String networkState) {
sessionStore, logger);
metadataState = copyMetadataState(configuration);

// Set up and collect constant app and device diagnostics
sharedPrefs = appContext.getSharedPreferences(SHARED_PREF_KEY, Context.MODE_PRIVATE);

ActivityManager am =
(ActivityManager) appContext.getSystemService(Context.ACTIVITY_SERVICE);

appDataCollector = new AppDataCollector(appContext, appContext.getPackageManager(),
immutableConfig, sessionTracker, am, logger);

UserRepository userRepository = new UserRepository(sharedPrefs,
immutableConfig.getPersistUser());
userState = new UserState(userRepository);
User user = configuration.getUser();
sharedPrefs = appContext.getSharedPreferences(SHARED_PREF_KEY, Context.MODE_PRIVATE);

if (user.getId() != null || user.getEmail() != null || user.getName() != null) {
userState.setUser(user.getId(), user.getEmail(), user.getName());
}
// load the device + user information
DeviceIdStore deviceIdStore = new DeviceIdStore(appContext, logger);
String deviceId = deviceIdStore.loadDeviceId();
userState = loadUserState(configuration, deviceId);

DeviceBuildInfo info = DeviceBuildInfo.Companion.defaultInfo();
Resources resources = appContext.getResources();
String id = userRepository.getDeviceId();
deviceDataCollector = new DeviceDataCollector(connectivity, appContext, resources, id, info,
Environment.getDataDirectory(), logger);
deviceDataCollector = new DeviceDataCollector(connectivity, appContext,
resources, deviceId, info, Environment.getDataDirectory(), logger);


if (appContext instanceof Application) {
Expand Down Expand Up @@ -253,6 +250,18 @@ public void run() {
leaveAutoBreadcrumb("Bugsnag loaded", BreadcrumbType.STATE, data);
}

private UserState loadUserState(@NonNull Configuration configuration, String deviceId) {
boolean persistUser = immutableConfig.getPersistUser();
UserRepository userRepository = new UserRepository(sharedPrefs, persistUser, deviceId);
UserState state = new UserState(userRepository);
User user = configuration.getUser();

if (user.getId() != null || user.getEmail() != null || user.getName() != null) {
state.setUser(user.getId(), user.getEmail(), user.getName());
}
return state;
}

@VisibleForTesting
Client(
ImmutableConfig immutableConfig,
Expand Down
Loading

0 comments on commit ca24c6c

Please sign in to comment.