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 Nov 30, 2020
1 parent 345d05e commit ce98e9c
Show file tree
Hide file tree
Showing 10 changed files with 430 additions and 58 deletions.
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,167 @@
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
import java.util.concurrent.atomic.AtomicReference

internal class DeviceIdStoreTest {

lateinit var file: File
lateinit var storageDir: File

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

@Before
fun setUp() {
val 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(nonExistentFile, NoopLogger)
val deviceId = store.loadDeviceId(
NoopLogger,
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(file, NoopLogger)
val deviceId = store.loadDeviceId(NoopLogger, 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(file, NoopLogger)
val deviceId = store.loadDeviceId(NoopLogger, 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(file, NoopLogger)
val deviceId = store.loadDeviceId(NoopLogger, 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(file, NoopLogger)

// device ID is loaded without writing file
assertEquals(
"24c51482-2ffe-11eb-adc1-0242ac120002",
store.loadDeviceId(NoopLogger, uuidProvider)
)
// same device ID is retrieved as before
assertEquals(
"24c51482-2ffe-11eb-adc1-0242ac120002",
store.loadDeviceId(NoopLogger, 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(nonReadableFile, NoopLogger)
val deviceId = store.loadDeviceId(NoopLogger, 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(file, NoopLogger)
val latch = CountDownLatch(1)

// the main thread & executor thread race to persist a device ID first.
// this asserts that the first value written to disk is used
// and thus indirectly verifies that the file is appropriately synchronized.
val firstPersistedValue = AtomicReference<String>()
var bgThreadValue: String? = null

// persist a new device ID on a background thread
Executors.newSingleThreadExecutor().submit {
bgThreadValue = store.loadDeviceId(NoopLogger, uuidProvider)
firstPersistedValue.compareAndSet(null, bgThreadValue)
latch.countDown()
}

// persist a new device ID on the main thread
val deviceId = store.loadDeviceId(NoopLogger, diffUuidProvider)
firstPersistedValue.compareAndSet(null, deviceId)
latch.await()

// validate that the device ID is consistent for each.
// the device ID of whichever thread won the race first should be used.
val expected = firstPersistedValue.get()
assertEquals(expected, deviceId)
assertEquals(expected, bgThreadValue)
}

@Test
fun sharedPrefMigration() {
val store = DeviceIdStore(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(context)
assertEquals("55670bff-9024-fc0a-b392-0242ac88ccd9", storeDeviceId)
assertEquals(prefDeviceId, storeDeviceId)
}
}
36 changes: 23 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,24 @@ 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
File deviceIdFile = new File(appContext.getFilesDir(), "device-id");
DeviceIdStore deviceIdStore = new DeviceIdStore(deviceIdFile, logger);
String deviceId = deviceIdStore.loadDeviceId(appContext);
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 +251,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 ce98e9c

Please sign in to comment.