Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding generateAnonymousId config option #1989

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions bugsnag-android-core/api/bugsnag-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ public class com/bugsnag/android/Configuration : com/bugsnag/android/CallbackAwa
public fun getEnabledErrorTypes ()Lcom/bugsnag/android/ErrorTypes;
public fun getEnabledReleaseStages ()Ljava/util/Set;
public fun getEndpoints ()Lcom/bugsnag/android/EndpointConfiguration;
public fun getGenerateAnonymousId ()Z
public fun getLaunchDurationMillis ()J
public fun getLogger ()Lcom/bugsnag/android/Logger;
public fun getMaxBreadcrumbs ()I
Expand Down Expand Up @@ -214,6 +215,7 @@ public class com/bugsnag/android/Configuration : com/bugsnag/android/CallbackAwa
public fun setEnabledErrorTypes (Lcom/bugsnag/android/ErrorTypes;)V
public fun setEnabledReleaseStages (Ljava/util/Set;)V
public fun setEndpoints (Lcom/bugsnag/android/EndpointConfiguration;)V
public fun setGenerateAnonymousId (Z)V
public fun setLaunchDurationMillis (J)V
public fun setLogger (Lcom/bugsnag/android/Logger;)V
public fun setMaxBreadcrumbs (I)V
Expand Down Expand Up @@ -861,7 +863,7 @@ public final class com/bugsnag/android/internal/DateUtils {
}

public final class com/bugsnag/android/internal/ImmutableConfig {
public fun <init> (Ljava/lang/String;ZLcom/bugsnag/android/ErrorTypes;ZLcom/bugsnag/android/ThreadSendPolicy;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Set;Ljava/util/Set;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Lcom/bugsnag/android/Delivery;Lcom/bugsnag/android/EndpointConfiguration;ZJLcom/bugsnag/android/Logger;IIIIJLkotlin/Lazy;ZZLandroid/content/pm/PackageInfo;Landroid/content/pm/ApplicationInfo;Ljava/util/Collection;)V
public fun <init> (Ljava/lang/String;ZLcom/bugsnag/android/ErrorTypes;ZLcom/bugsnag/android/ThreadSendPolicy;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Set;Ljava/util/Set;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Lcom/bugsnag/android/Delivery;Lcom/bugsnag/android/EndpointConfiguration;ZJLcom/bugsnag/android/Logger;IIIIJLkotlin/Lazy;ZZZLandroid/content/pm/PackageInfo;Landroid/content/pm/ApplicationInfo;Ljava/util/Collection;)V
public final fun component1 ()Ljava/lang/String;
public final fun component10 ()Ljava/util/Set;
public final fun component11 ()Ljava/lang/String;
Expand All @@ -883,18 +885,19 @@ public final class com/bugsnag/android/internal/ImmutableConfig {
public final fun component26 ()Lkotlin/Lazy;
public final fun component27 ()Z
public final fun component28 ()Z
public final fun component29 ()Landroid/content/pm/PackageInfo;
public final fun component29 ()Z
public final fun component3 ()Lcom/bugsnag/android/ErrorTypes;
public final fun component30 ()Landroid/content/pm/ApplicationInfo;
public final fun component31 ()Ljava/util/Collection;
public final fun component30 ()Landroid/content/pm/PackageInfo;
public final fun component31 ()Landroid/content/pm/ApplicationInfo;
public final fun component32 ()Ljava/util/Collection;
public final fun component4 ()Z
public final fun component5 ()Lcom/bugsnag/android/ThreadSendPolicy;
public final fun component6 ()Ljava/util/Collection;
public final fun component7 ()Ljava/util/Collection;
public final fun component8 ()Ljava/util/Collection;
public final fun component9 ()Ljava/util/Set;
public final fun copy (Ljava/lang/String;ZLcom/bugsnag/android/ErrorTypes;ZLcom/bugsnag/android/ThreadSendPolicy;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Set;Ljava/util/Set;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Lcom/bugsnag/android/Delivery;Lcom/bugsnag/android/EndpointConfiguration;ZJLcom/bugsnag/android/Logger;IIIIJLkotlin/Lazy;ZZLandroid/content/pm/PackageInfo;Landroid/content/pm/ApplicationInfo;Ljava/util/Collection;)Lcom/bugsnag/android/internal/ImmutableConfig;
public static synthetic fun copy$default (Lcom/bugsnag/android/internal/ImmutableConfig;Ljava/lang/String;ZLcom/bugsnag/android/ErrorTypes;ZLcom/bugsnag/android/ThreadSendPolicy;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Set;Ljava/util/Set;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Lcom/bugsnag/android/Delivery;Lcom/bugsnag/android/EndpointConfiguration;ZJLcom/bugsnag/android/Logger;IIIIJLkotlin/Lazy;ZZLandroid/content/pm/PackageInfo;Landroid/content/pm/ApplicationInfo;Ljava/util/Collection;ILjava/lang/Object;)Lcom/bugsnag/android/internal/ImmutableConfig;
public final fun copy (Ljava/lang/String;ZLcom/bugsnag/android/ErrorTypes;ZLcom/bugsnag/android/ThreadSendPolicy;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Set;Ljava/util/Set;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Lcom/bugsnag/android/Delivery;Lcom/bugsnag/android/EndpointConfiguration;ZJLcom/bugsnag/android/Logger;IIIIJLkotlin/Lazy;ZZZLandroid/content/pm/PackageInfo;Landroid/content/pm/ApplicationInfo;Ljava/util/Collection;)Lcom/bugsnag/android/internal/ImmutableConfig;
public static synthetic fun copy$default (Lcom/bugsnag/android/internal/ImmutableConfig;Ljava/lang/String;ZLcom/bugsnag/android/ErrorTypes;ZLcom/bugsnag/android/ThreadSendPolicy;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Collection;Ljava/util/Set;Ljava/util/Set;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Integer;Ljava/lang/String;Lcom/bugsnag/android/Delivery;Lcom/bugsnag/android/EndpointConfiguration;ZJLcom/bugsnag/android/Logger;IIIIJLkotlin/Lazy;ZZZLandroid/content/pm/PackageInfo;Landroid/content/pm/ApplicationInfo;Ljava/util/Collection;ILjava/lang/Object;)Lcom/bugsnag/android/internal/ImmutableConfig;
public fun equals (Ljava/lang/Object;)Z
public final fun getApiKey ()Ljava/lang/String;
public final fun getAppInfo ()Landroid/content/pm/ApplicationInfo;
Expand All @@ -910,6 +913,7 @@ public final class com/bugsnag/android/internal/ImmutableConfig {
public final fun getEnabledErrorTypes ()Lcom/bugsnag/android/ErrorTypes;
public final fun getEnabledReleaseStages ()Ljava/util/Collection;
public final fun getEndpoints ()Lcom/bugsnag/android/EndpointConfiguration;
public final fun getGenerateAnonymousId ()Z
public final fun getLaunchDurationMillis ()J
public final fun getLogger ()Lcom/bugsnag/android/Logger;
public final fun getMaxBreadcrumbs ()I
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package com.bugsnag.android

import android.content.Context
import androidx.test.core.app.ApplicationProvider
import com.bugsnag.android.internal.ImmutableConfig
import junit.framework.TestCase.assertNull
import org.junit.Assert.assertEquals
import org.junit.Before
Expand Down Expand Up @@ -46,6 +47,12 @@ internal class DeviceIdStoreTest {
prefs.edit().remove("install.iud").commit()
}

private fun generateConfig(generateAnonymousId: Boolean): ImmutableConfig {
val config = BugsnagTestUtils.generateConfiguration()
config.generateAnonymousId = generateAnonymousId
return BugsnagTestUtils.convert(config)
}

/**
* A file should be created if it does not already exist
*/
Expand All @@ -61,8 +68,9 @@ internal class DeviceIdStoreTest {
uuidProvider(0),
nonExistentInternalFile,
uuidProvider(1),
prefMigrator,
NoopLogger
sharedPrefMigrator = prefMigrator,
logger = NoopLogger,
config = generateConfig(true)
)

assertEquals(ids[0], store.loadDeviceId()!!)
Expand All @@ -85,7 +93,8 @@ internal class DeviceIdStoreTest {
fileInternal,
uuidProvider(1),
prefMigrator,
NoopLogger
logger = NoopLogger,
config = generateConfig(true)
)

assertEquals(ids[0], store.loadDeviceId()!!)
Expand All @@ -106,7 +115,8 @@ internal class DeviceIdStoreTest {
fileInternal,
uuidProvider(1),
prefMigrator,
NoopLogger
logger = NoopLogger,
config = generateConfig(true)
)

assertEquals(ids[0], store.loadDeviceId()!!)
Expand All @@ -127,7 +137,8 @@ internal class DeviceIdStoreTest {
fileInternal,
uuidProvider(3),
prefMigrator,
NoopLogger
logger = NoopLogger,
config = generateConfig(true)
)
val storeA = DeviceIdStore(
ctx,
Expand All @@ -136,7 +147,8 @@ internal class DeviceIdStoreTest {
fileInternal,
uuidProvider(1),
prefMigrator,
NoopLogger
logger = NoopLogger,
config = generateConfig(true)
)

// device ID is loaded without writing file
Expand Down Expand Up @@ -170,7 +182,8 @@ internal class DeviceIdStoreTest {
nonReadableInternalFile,
uuidProvider(1),
prefMigrator,
NoopLogger
logger = NoopLogger,
config = generateConfig(true)
)
assertNull(store.loadDeviceId())
assertNull(store.loadInternalDeviceId())
Expand All @@ -188,7 +201,8 @@ internal class DeviceIdStoreTest {
fileInternal,
uuidProvider(1),
prefMigrator,
NoopLogger
logger = NoopLogger,
config = generateConfig(true)
)

// load the device ID on many different background threads.
Expand Down Expand Up @@ -224,7 +238,8 @@ internal class DeviceIdStoreTest {
fileInternal,
uuidProvider(1),
prefMigrator,
NoopLogger
logger = NoopLogger,
config = generateConfig(true)
)
val context = ApplicationProvider.getApplicationContext<Context>()

Expand All @@ -237,4 +252,26 @@ internal class DeviceIdStoreTest {
assertEquals(prefsId, storeDeviceId)
assertEquals(prefDeviceId, storeDeviceId)
}

/**
* If generateAnonymousId is false, no device ID is returned (even if one is saved)
*/
@Test
fun loadWithoutGenerateAnonymousId() {
file.writeText("{\"id\": \"${ids[0]}\"}")
fileInternal.writeText("{\"id\": \"${ids[1]}\"}")
Comment on lines +261 to +262
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these look like a good candidate for raw string literals:

Suggested change
file.writeText("{\"id\": \"${ids[0]}\"}")
fileInternal.writeText("{\"id\": \"${ids[1]}\"}")
file.writeText("""{"id": "${ids[0]}"}""")
fileInternal.writeText("""{"id": "${ids[1]}"}""")

val store = DeviceIdStore(
ctx,
file,
uuidProvider(0),
fileInternal,
uuidProvider(1),
sharedPrefMigrator = prefMigrator,
logger = NoopLogger,
config = generateConfig(false)
)

assertNull(store.loadDeviceId())
assertNull(store.loadInternalDeviceId())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ internal class ConfigInternal(
var releaseStage: String? = null
var sendThreads: ThreadSendPolicy = ThreadSendPolicy.ALWAYS
var persistUser: Boolean = true
var generateAnonymousId: Boolean = true

var launchDurationMillis: Long = DEFAULT_LAUNCH_CRASH_THRESHOLD_MS

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,26 @@ public void setPersistUser(boolean persistUser) {
impl.setPersistUser(persistUser);
}

/**
* Set whether or not Bugsnag should generate an anonymous ID and persist it in local storage
*
* If disabled, any device ID that has been persisted will not be retrieved, and no new
* device ID will be generated or stored
*/
public boolean getGenerateAnonymousId() {
return impl.getGenerateAnonymousId();
}

/**
* Set whether or not Bugsnag should generate an anonymous ID and persist it in local storage
*
* If disabled, any device ID that has been persisted will not be retrieved, and no new
* device ID will be generated or stored
*/
public void setGenerateAnonymousId(boolean generateAnonymousId) {
impl.setGenerateAnonymousId(generateAnonymousId);
}

/**
* Sets the directory where event and session JSON payloads should be persisted if a network
* request is not successful. If you use Bugsnag in multiple processes, then a unique
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,28 @@
package com.bugsnag.android

import android.content.Context
import com.bugsnag.android.internal.ImmutableConfig
import java.io.File
import java.util.UUID

/**
* This class is responsible for persisting and retrieving the device ID and internal device ID,
* which uniquely identify this device in various contexts.
*/
internal class DeviceIdStore @JvmOverloads constructor(
internal class DeviceIdStore @JvmOverloads @Suppress("LongParameterList") constructor(
context: Context,
deviceIdfile: File = File(context.filesDir, "device-id"),
deviceIdGenerator: () -> UUID = { UUID.randomUUID() },
internalDeviceIdfile: File = File(context.filesDir, "internal-device-id"),
internalDeviceIdGenerator: () -> UUID = { UUID.randomUUID() },
private val sharedPrefMigrator: SharedPrefMigrator,
config: ImmutableConfig,
logger: Logger
) {

private val persistence: DeviceIdPersistence
private val internalPersistence: DeviceIdPersistence
private val generateId = config.generateAnonymousId

init {
persistence = DeviceIdFilePersistence(deviceIdfile, deviceIdGenerator, logger)
Expand All @@ -35,6 +38,12 @@ internal class DeviceIdStore @JvmOverloads constructor(
* be used. If no value is present then a random UUID will be generated and persisted.
*/
fun loadDeviceId(): String? {
// If generateAnonymousId = false, return null
// so that a previously persisted device ID is not returned,
// or a new one is not generated and persisted
if (!generateId) {
return null
}
var result = persistence.loadDeviceId(false)
if (result != null) {
return result
Expand All @@ -47,6 +56,12 @@ internal class DeviceIdStore @JvmOverloads constructor(
}

fun loadInternalDeviceId(): String? {
// If generateAnonymousId = false, return null
// so that a previously persisted device ID is not returned,
// or a new one is not generated and persisted
if (!generateId) {
return null
}
return internalPersistence.loadDeviceId(true)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ internal class ManifestConfigLoader {
private const val AUTO_DETECT_ERRORS = "$BUGSNAG_NS.AUTO_DETECT_ERRORS"
private const val PERSIST_USER = "$BUGSNAG_NS.PERSIST_USER"
private const val SEND_THREADS = "$BUGSNAG_NS.SEND_THREADS"
private const val GENERATE_ANONYMOUS_ID = "$BUGSNAG_NS.GENERATE_ANONYMOUS_ID"

// endpoints
private const val ENDPOINT_NOTIFY = "$BUGSNAG_NS.ENDPOINT_NOTIFY"
Expand Down Expand Up @@ -108,6 +109,7 @@ internal class ManifestConfigLoader {
autoTrackSessions = data.getBoolean(AUTO_TRACK_SESSIONS, autoTrackSessions)
autoDetectErrors = data.getBoolean(AUTO_DETECT_ERRORS, autoDetectErrors)
persistUser = data.getBoolean(PERSIST_USER, persistUser)
generateAnonymousId = data.getBoolean(GENERATE_ANONYMOUS_ID, generateAnonymousId)

val str = data.getString(SEND_THREADS)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ internal class StorageModule(
DeviceIdStore(
appContext,
sharedPrefMigrator = sharedPrefMigrator,
logger = logger
logger = logger,
config = immutableConfig
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ internal class UserStore @JvmOverloads constructor(
* [Configuration.getPersistUser] is true.
*
* If no user is stored on disk, then a default [User] is used which uses the device ID
* as its ID.
* as its ID (unless the generateAnonymousId config option is set to false, in which case the
* device ID and therefore the user ID is set to
* null).
*
* The [UserState] provides a mechanism for observing value changes to its user property,
* so to avoid interfering with this the method should only be called once for each [Client].
Expand All @@ -46,6 +48,8 @@ internal class UserStore @JvmOverloads constructor(

val userState = when {
loadedUser != null && validUser(loadedUser) -> UserState(loadedUser)
// if generateAnonymousId config option is false, the deviceId should already be null
// here
else -> UserState(User(deviceId, null, null))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ data class ImmutableConfig(
val persistenceDirectory: Lazy<File>,
val sendLaunchCrashesSynchronously: Boolean,
val attemptDeliveryOnCrash: Boolean,
val generateAnonymousId: Boolean,

// results cached here to avoid unnecessary lookups in Client.
val packageInfo: PackageInfo?,
Expand Down Expand Up @@ -166,6 +167,7 @@ internal fun convertToImmutableConfig(
delivery = config.delivery,
endpoints = config.endpoints,
persistUser = config.persistUser,
generateAnonymousId = config.generateAnonymousId,
launchDurationMillis = config.launchDurationMillis,
logger = config.logger!!,
maxBreadcrumbs = config.maxBreadcrumbs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ void loadDeviceTestCase(bugsnag_event *event) {
strcpy(device->orientation, "portrait");

struct tm time = { 0, 0, 0, 1, 12, 128 };
device->time = mktime(&time);
device->time = timegm(&time);
strcpy(device->id, "f5gh7");
device->jailbroken = true;
strcpy(device->locale, "En");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public void serialize(Map<String, Object> map, ImmutableConfig config) {
map.put("versionCode", config.getVersionCode());
map.put("type", config.getAppType());
map.put("persistUser", config.getPersistUser());
map.put("generateAnonymousId", config.getGenerateAnonymousId());
map.put("launchCrashThresholdMs", (int) config.getLaunchDurationMillis());
map.put("maxBreadcrumbs", config.getMaxBreadcrumbs());
map.put("enabledBreadcrumbTypes", serializeBreadrumbTypes(config));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public File invoke() {
}),
true,
true,
true,
null,
null,
Collections.singleton(Pattern.compile(".*password.*"))
Expand Down
Loading