From 3de0ba3c8c8cfea4a6b76bde125b79a0a65fab21 Mon Sep 17 00:00:00 2001 From: jason Date: Tue, 23 Jul 2024 11:07:41 +0100 Subject: [PATCH 1/9] refactor(startup): rework startup by not immediately blocking for each background task --- bugsnag-android-core/build.gradle.kts | 10 +- bugsnag-android-core/detekt-baseline.xml | 5 +- .../com/bugsnag/android/BugsnagTestUtils.java | 145 ------------------ .../com/bugsnag/android/DeviceIdStoreTest.kt | 67 ++++---- .../bugsnag/android/LastRunInfoStoreTest.kt | 2 + .../java/com/bugsnag/android/UserStoreTest.kt | 74 +++++++-- .../main/java/com/bugsnag/android/Client.java | 16 +- .../bugsnag/android/DataCollectionModule.kt | 33 ++-- .../bugsnag/android/DeviceDataCollector.kt | 24 +-- .../java/com/bugsnag/android/DeviceIdStore.kt | 52 +++++-- .../com/bugsnag/android/EventStorageModule.kt | 6 +- .../java/com/bugsnag/android/FileStore.kt | 4 - .../com/bugsnag/android/LastRunInfoStore.kt | 16 +- .../com/bugsnag/android/SharedPrefMigrator.kt | 54 ++++--- .../java/com/bugsnag/android/StorageModule.kt | 47 +++--- .../java/com/bugsnag/android/UserStore.kt | 11 +- .../android/internal/dag/ConfigModule.kt | 1 - .../android/internal/dag/DependencyModule.kt | 36 +---- .../com/bugsnag/android/BugsnagTestUtils.java | 43 +++++- .../java/com/bugsnag/android/ValueFuture.kt | 12 ++ .../android/ConfigChangeReceiverTest.kt | 5 +- .../com/bugsnag/android/DataCollectorTest.kt | 7 +- .../DeviceDataCollectorSerializationTest.kt | 6 +- .../DeviceMetadataSerializationTest.kt | 6 +- .../com/bugsnag/android/EventRedactionTest.kt | 27 +++- .../bugsnag/android/EventSerializationTest.kt | 7 +- .../bugsnag/android/ImmutableConfigTest.kt | 4 +- .../bugsnag/android/SharedPrefMigratorTest.kt | 31 ++-- .../com/bugsnag/android/BugsnagBuildPlugin.kt | 1 + .../kotlin/com/bugsnag/android/Versions.kt | 4 +- .../MultiProcessHandledExceptionScenario.kt | 9 +- .../multiprocess/MultiProcessService.kt | 6 +- 32 files changed, 390 insertions(+), 381 deletions(-) delete mode 100644 bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java create mode 100644 bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueFuture.kt diff --git a/bugsnag-android-core/build.gradle.kts b/bugsnag-android-core/build.gradle.kts index d6774d451a..52c5d28871 100644 --- a/bugsnag-android-core/build.gradle.kts +++ b/bugsnag-android-core/build.gradle.kts @@ -15,7 +15,15 @@ bugsnagBuildOptions { java.srcDirs("dsl-json/library/src/main/java") } named("test") { - java.srcDirs("dsl-json/library/src/test/java") + java.srcDirs( + "dsl-json/library/src/test/java", + "src/sharedTest/java" + ) + } + named("androidTest") { + java.srcDirs( + "src/sharedTest/java" + ) } } } diff --git a/bugsnag-android-core/detekt-baseline.xml b/bugsnag-android-core/detekt-baseline.xml index e4e75d4c39..5e9988e00a 100644 --- a/bugsnag-android-core/detekt-baseline.xml +++ b/bugsnag-android-core/detekt-baseline.xml @@ -9,10 +9,10 @@ LongParameterList:AppDataCollector.kt$AppDataCollector$( appContext: Context, private val packageManager: PackageManager?, private val config: ImmutableConfig, private val sessionTracker: SessionTracker, private val activityManager: ActivityManager?, private val launchCrashTracker: LaunchCrashTracker, private val memoryTrimState: MemoryTrimState ) LongParameterList:AppWithState.kt$AppWithState$( binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, buildUuid: String?, type: String?, versionCode: Number?, /** * The number of milliseconds the application was running before the event occurred */ var duration: Number?, /** * The number of milliseconds the application was running in the foreground before the * event occurred */ var durationInForeground: Number?, /** * Whether the application was in the foreground when the event occurred */ var inForeground: Boolean?, /** * Whether the application was launching when the event occurred */ var isLaunching: Boolean? ) LongParameterList:AppWithState.kt$AppWithState$( config: ImmutableConfig, binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, duration: Number?, durationInForeground: Number?, inForeground: Boolean?, isLaunching: Boolean? ) - LongParameterList:DataCollectionModule.kt$DataCollectionModule$( contextModule: ContextModule, configModule: ConfigModule, systemServiceModule: SystemServiceModule, trackerModule: TrackerModule, bgTaskService: BackgroundTaskService, connectivity: Connectivity, deviceId: String?, internalDeviceId: String?, memoryTrimState: MemoryTrimState ) + LongParameterList:DataCollectionModule.kt$DataCollectionModule$( contextModule: ContextModule, configModule: ConfigModule, systemServiceModule: SystemServiceModule, trackerModule: TrackerModule, bgTaskService: BackgroundTaskService, connectivity: Connectivity, deviceIdStore: Future<DeviceIdStore.DeviceIds?>, memoryTrimState: MemoryTrimState ) LongParameterList:Device.kt$Device$( buildInfo: DeviceBuildInfo, /** * The Application Binary Interface used */ var cpuAbi: Array<String>?, /** * Whether the device has been jailbroken */ var jailbroken: Boolean?, /** * A UUID generated by Bugsnag and used for the individual application on a device */ var id: String?, /** * The IETF language tag of the locale used */ var locale: String?, /** * The total number of bytes of memory on the device */ var totalMemory: Long?, /** * A collection of names and their versions of the primary languages, frameworks or * runtimes that the application is running on */ runtimeVersions: MutableMap<String, Any>? ) LongParameterList:DeviceBuildInfo.kt$DeviceBuildInfo$( val manufacturer: String?, val model: String?, val osVersion: String?, val apiLevel: Int?, val osBuild: String?, val fingerprint: String?, val tags: String?, val brand: String?, val cpuAbis: Array<String>? ) - LongParameterList:DeviceDataCollector.kt$DeviceDataCollector$( private val connectivity: Connectivity, private val appContext: Context, resources: Resources, private val deviceId: String?, private val internalDeviceId: String?, private val buildInfo: DeviceBuildInfo, private val dataDirectory: File, rootDetector: RootDetector, private val bgTaskService: BackgroundTaskService, private val logger: Logger ) + LongParameterList:DeviceDataCollector.kt$DeviceDataCollector$( private val connectivity: Connectivity, private val appContext: Context, resources: Resources, private val deviceIdStore: Future<DeviceIdStore.DeviceIds?>, private val buildInfo: DeviceBuildInfo, private val dataDirectory: File, private val rootedFuture: Future<Boolean>?, private val bgTaskService: BackgroundTaskService, private val logger: Logger ) LongParameterList:DeviceWithState.kt$DeviceWithState$( buildInfo: DeviceBuildInfo, jailbroken: Boolean?, id: String?, locale: String?, totalMemory: Long?, runtimeVersions: MutableMap<String, Any>, /** * The number of free bytes of storage available on the device */ var freeDisk: Long?, /** * The number of free bytes of memory available on the device */ var freeMemory: Long?, /** * The orientation of the device when the event occurred: either portrait or landscape */ var orientation: String?, /** * The timestamp on the device when the event occurred */ var time: Date? ) LongParameterList:EventFilenameInfo.kt$EventFilenameInfo.Companion$( obj: Any, uuid: String = UUID.randomUUID().toString(), apiKey: String?, timestamp: Long = System.currentTimeMillis(), config: ImmutableConfig, isLaunching: Boolean? = null ) LongParameterList:EventInternal.kt$EventInternal$( apiKey: String, logger: Logger, breadcrumbs: MutableList<Breadcrumb> = mutableListOf(), discardClasses: Set<Pattern> = setOf(), errors: MutableList<Error> = mutableListOf(), metadata: Metadata = Metadata(), featureFlags: FeatureFlags = FeatureFlags(), originalError: Throwable? = null, projectPackages: Collection<String> = setOf(), severityReason: SeverityReason = SeverityReason.newInstance(SeverityReason.REASON_HANDLED_EXCEPTION), threads: MutableList<Thread> = mutableListOf(), user: User = User(), redactionKeys: Set<Pattern>? = null ) @@ -63,7 +63,6 @@ SwallowedException:ImmutableConfig.kt$e: Exception SwallowedException:JsonHelperTest.kt$JsonHelperTest$e: IllegalArgumentException SwallowedException:PluginClient.kt$PluginClient$exc: ClassNotFoundException - SwallowedException:SharedPrefMigrator.kt$SharedPrefMigrator$e: RuntimeException ThrowsCount:JsonHelper.kt$JsonHelper$fun jsonToLong(value: Any?): Long? TooManyFunctions:ConfigInternal.kt$ConfigInternal : CallbackAwareMetadataAwareUserAwareFeatureFlagAware TooManyFunctions:DeviceDataCollector.kt$DeviceDataCollector diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java deleted file mode 100644 index 077bdce16e..0000000000 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BugsnagTestUtils.java +++ /dev/null @@ -1,145 +0,0 @@ -package com.bugsnag.android; - -import com.bugsnag.android.internal.ImmutableConfig; -import com.bugsnag.android.internal.ImmutableConfigKt; - -import androidx.annotation.NonNull; -import androidx.test.core.app.ApplicationProvider; - -import org.jetbrains.annotations.NotNull; -import org.json.JSONArray; -import org.json.JSONException; -import org.json.JSONObject; - -import java.io.File; -import java.io.IOException; -import java.io.StringWriter; -import java.util.Comparator; -import java.util.Date; -import java.util.HashMap; - -final class BugsnagTestUtils { - - private BugsnagTestUtils() { - } - - static HashMap runtimeVersions = new HashMap<>(); - - static { - runtimeVersions.put("osBuild", "bulldog"); - runtimeVersions.put("androidApiLevel", "24"); - } - - private static String streamableToString(JsonStream.Streamable streamable) throws IOException { - StringWriter writer = new StringWriter(); - JsonStream jsonStream = new JsonStream(writer); - streamable.toStream(jsonStream); - return writer.toString(); - } - - static JSONObject streamableToJson(JsonStream.Streamable streamable) - throws JSONException, IOException { - return new JSONObject(streamableToString(streamable)); - } - - static JSONArray streamableToJsonArray(JsonStream.Streamable streamable) - throws JSONException, IOException { - return new JSONArray(streamableToString(streamable)); - } - - static Client generateClient(Configuration config) { - config.setDelivery(generateDelivery()); - return new Client(ApplicationProvider.getApplicationContext(), config); - } - - static Client generateClient() { - return generateClient(new Configuration("5d1ec5bd39a74caa1267142706a7fb21")); - } - - static Session generateSession() { - return new Session("test", new Date(), new User(), false, - new Notifier(), NoopLogger.INSTANCE, "TEST APIKEY"); - } - - static Event generateEvent() { - Throwable exc = new RuntimeException(); - Event event = new Event( - exc, - BugsnagTestUtils.generateImmutableConfig(), - SeverityReason.newInstance(SeverityReason.REASON_HANDLED_EXCEPTION), - NoopLogger.INSTANCE - ); - event.setApp(generateAppWithState()); - event.setDevice(generateDeviceWithState()); - return event; - } - - static Configuration generateConfiguration() { - Configuration configuration = new Configuration("5d1ec5bd39a74caa1267142706a7fb21"); - configuration.setDelivery(generateDelivery()); - configuration.setLogger(NoopLogger.INSTANCE); - return configuration; - } - - static ImmutableConfig generateImmutableConfig() { - return convert(generateConfiguration()); - } - - static ImmutableConfig convert(Configuration config) { - try { - config.setPersistenceDirectory(File.createTempFile("tmp", null)); - } catch (IOException ignored) { - // swallow - } - return ImmutableConfigKt.convertToImmutableConfig(config, null, null, null); - } - - static Device generateDevice() { - DeviceBuildInfo buildInfo = DeviceBuildInfo.Companion.defaultInfo(); - return new Device(buildInfo, new String[]{}, null, null, null, - 109230923452L, runtimeVersions); - } - - static DeviceWithState generateDeviceWithState() { - DeviceBuildInfo buildInfo = DeviceBuildInfo.Companion.defaultInfo(); - return new DeviceWithState(buildInfo, null, null, null, - 109230923452L, runtimeVersions, 22234423124L, 92340255592L, - "portrait", new Date(0)); - } - - public static Delivery generateDelivery() { - return new Delivery() { - @NotNull - @Override - public DeliveryStatus deliver(@NonNull EventPayload payload, - @NonNull DeliveryParams deliveryParams) { - return DeliveryStatus.DELIVERED; - } - - @NonNull - @Override - public DeliveryStatus deliver(@NonNull Session payload, - @NonNull DeliveryParams deliveryParams) { - return DeliveryStatus.DELIVERED; - } - }; - } - - public static AppWithState generateAppWithState() { - return new AppWithState(generateImmutableConfig(), null, null, null, - null, null, null, null, null, null); - } - - public static App generateApp() { - return new App(generateImmutableConfig(), null, null, null, null, null); - } - - static Comparator featureFlagComparator() { - return new Comparator() { - @Override - public int compare(FeatureFlag f1, FeatureFlag f2) { - return f1.getName().compareTo(f2.getName()); - } - }; - } -} diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DeviceIdStoreTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DeviceIdStoreTest.kt index 89da8224fa..19c27b09fb 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DeviceIdStoreTest.kt +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DeviceIdStoreTest.kt @@ -68,13 +68,15 @@ internal class DeviceIdStoreTest { uuidProvider(0), nonExistentInternalFile, uuidProvider(1), - sharedPrefMigrator = prefMigrator, + sharedPrefMigrator = ValueFuture(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) - assertEquals(ids[0], store.loadDeviceId()!!) - assertEquals(ids[1], store.loadInternalDeviceId()!!) + val loaded = store.call() + + assertEquals(ids[0], loaded?.deviceId) + assertEquals(ids[1], loaded?.internalDeviceId) } /** @@ -92,13 +94,15 @@ internal class DeviceIdStoreTest { uuidProvider(0), fileInternal, uuidProvider(1), - prefMigrator, + ValueFuture(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) - assertEquals(ids[0], store.loadDeviceId()!!) - assertEquals(ids[1], store.loadInternalDeviceId()!!) + val loaded = store.call() + + assertEquals(ids[0], loaded?.deviceId) + assertEquals(ids[1], loaded?.internalDeviceId) } /** @@ -114,13 +118,15 @@ internal class DeviceIdStoreTest { uuidProvider(0), fileInternal, uuidProvider(1), - prefMigrator, + ValueFuture(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) - assertEquals(ids[0], store.loadDeviceId()!!) - assertEquals(ids[1], store.loadInternalDeviceId()!!) + val loaded = store.call() + + assertEquals(ids[0], loaded?.deviceId) + assertEquals(ids[1], loaded?.internalDeviceId) } /** @@ -136,7 +142,7 @@ internal class DeviceIdStoreTest { uuidProvider(2), fileInternal, uuidProvider(3), - prefMigrator, + ValueFuture(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) @@ -146,18 +152,21 @@ internal class DeviceIdStoreTest { uuidProvider(0), fileInternal, uuidProvider(1), - prefMigrator, + ValueFuture(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) + val loadedA = storeA.call() + val loadedB = storeB.call() + // device ID is loaded without writing file - assertEquals(ids[0], storeA.loadDeviceId()!!) - assertEquals(ids[1], storeA.loadInternalDeviceId()!!) + assertEquals(ids[0], loadedA?.deviceId) + assertEquals(ids[1], loadedA?.internalDeviceId) // same device ID is retrieved as before - assertEquals(ids[0], storeB.loadDeviceId()!!) - assertEquals(ids[1], storeB.loadInternalDeviceId()!!) + assertEquals(ids[0], loadedB?.deviceId) + assertEquals(ids[1], loadedB?.internalDeviceId) } /** @@ -181,12 +190,13 @@ internal class DeviceIdStoreTest { uuidProvider(0), nonReadableInternalFile, uuidProvider(1), - prefMigrator, + ValueFuture(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) - assertNull(store.loadDeviceId()) - assertNull(store.loadInternalDeviceId()) + + val loaded = store.call() + assertNull(loaded) } /** @@ -200,7 +210,7 @@ internal class DeviceIdStoreTest { uuidProvider(0), fileInternal, uuidProvider(1), - prefMigrator, + ValueFuture(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) @@ -215,7 +225,7 @@ internal class DeviceIdStoreTest { repeat(attempts) { executor.submit { - deviceIds.add(store.loadDeviceId()!!) + store.call()?.deviceId?.let { deviceIds.add(it) } latch.countDown() } } @@ -237,7 +247,7 @@ internal class DeviceIdStoreTest { uuidProvider(0), fileInternal, uuidProvider(1), - prefMigrator, + ValueFuture(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) @@ -247,8 +257,13 @@ internal class DeviceIdStoreTest { context.getSharedPreferences("com.bugsnag.android", Context.MODE_PRIVATE) prefs.edit().putString("install.iud", prefsId).commit() - val prefDeviceId = SharedPrefMigrator(context).loadDeviceId(false) - val storeDeviceId = store.loadDeviceId()!! + val prefDeviceId = prefMigrator + .apply { call() } + .loadDeviceId(false) + + val loaded = store.call() + + val storeDeviceId = loaded?.deviceId assertEquals(prefsId, storeDeviceId) assertEquals(prefDeviceId, storeDeviceId) } @@ -266,12 +281,12 @@ internal class DeviceIdStoreTest { uuidProvider(0), fileInternal, uuidProvider(1), - sharedPrefMigrator = prefMigrator, + sharedPrefMigrator = ValueFuture(prefMigrator), logger = NoopLogger, config = generateConfig(false) ) - assertNull(store.loadDeviceId()) - assertNull(store.loadInternalDeviceId()) + val loaded = store.call() + assertNull(loaded) } } diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/LastRunInfoStoreTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/LastRunInfoStoreTest.kt index 82c470cc10..1900991b0c 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/LastRunInfoStoreTest.kt +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/LastRunInfoStoreTest.kt @@ -71,6 +71,7 @@ internal class LastRunInfoStoreTest { */ @Test fun readValidFileContents() { + file.parentFile?.mkdirs() file.writeText("consecutiveLaunchCrashes=5\ncrashed=true\ncrashedDuringLaunch=false\n") val info = requireNotNull(lastRunInfoStore.load()) assertEquals(5, info.consecutiveLaunchCrashes) @@ -116,6 +117,7 @@ internal class LastRunInfoStoreTest { fun nonWritableFile() { file.apply { delete() + parentFile?.mkdirs() createNewFile() setWritable(false) } diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/UserStoreTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/UserStoreTest.kt index 27bcb70a3e..e70edd2527 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/UserStoreTest.kt +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/UserStoreTest.kt @@ -51,7 +51,15 @@ internal class UserStoreTest { .putString("user.name", "Jane Fonda") .commit() - val store = UserStore(generateConfig(true), "0asdf", file, prefMigrator, NoopLogger) + prefMigrator.call() + + val store = UserStore( + generateConfig(true), + ValueFuture(DeviceIdStore.DeviceIds("0asdf", null)), + file, + ValueFuture(prefMigrator), + NoopLogger + ) val user = store.load(User()).user assertEquals("jf123", user.id) assertEquals("Jane Fonda", user.name) @@ -65,7 +73,13 @@ internal class UserStoreTest { fun nonExistentFile() { val nonExistentFile = File(storageDir, "foo") nonExistentFile.delete() - val store = UserStore(generateConfig(true), "device-id", file, prefMigrator, NoopLogger) + val store = UserStore( + generateConfig(true), + ValueFuture(DeviceIdStore.DeviceIds("device-id", null)), + file, + ValueFuture(prefMigrator), + NoopLogger + ) val user = store.load(User()).user assertEquals("device-id", user.id) assertNull(user.email) @@ -77,7 +91,13 @@ internal class UserStoreTest { */ @Test fun emptyFile() { - val store = UserStore(generateConfig(true), "device-id", file, prefMigrator, NoopLogger) + val store = UserStore( + generateConfig(true), + ValueFuture(DeviceIdStore.DeviceIds("device-id", null)), + file, + ValueFuture(prefMigrator), + NoopLogger + ) val user = store.load(User()).user assertEquals("device-id", user.id) assertNull(user.email) @@ -90,7 +110,13 @@ internal class UserStoreTest { @Test fun invalidFileContents() { file.writeText("{\"hamster\": 2}") - val store = UserStore(generateConfig(true), "device-id", file, prefMigrator, NoopLogger) + val store = UserStore( + generateConfig(true), + ValueFuture(DeviceIdStore.DeviceIds("device-id", null)), + file, + ValueFuture(prefMigrator), + NoopLogger + ) val user = store.load(User()).user assertEquals("device-id", user.id) assertNull(user.email) @@ -109,9 +135,9 @@ internal class UserStoreTest { } val store = UserStore( generateConfig(true), - "device-id", + ValueFuture(DeviceIdStore.DeviceIds("device-id", null)), nonReadableFile, - prefMigrator, + ValueFuture(prefMigrator), NoopLogger ) val user = store.load(User()).user @@ -127,7 +153,13 @@ internal class UserStoreTest { fun validFileContents() { file.writeText("{\"id\":\"jf123\",\"email\":\"test@example.com\",\"name\":\"Jane Fonda\"}") - val store = UserStore(generateConfig(true), "0asdf", file, prefMigrator, NoopLogger) + val store = UserStore( + generateConfig(true), + ValueFuture(DeviceIdStore.DeviceIds("0asdf", null)), + file, + ValueFuture(prefMigrator), + NoopLogger + ) val user = store.load(User()).user assertEquals("jf123", user.id) assertEquals("Jane Fonda", user.name) @@ -141,9 +173,9 @@ internal class UserStoreTest { fun loadWithoutPersistUser() { val store = UserStore( generateConfig(false), - "device-id-123", + ValueFuture(DeviceIdStore.DeviceIds("device-id-123", null)), file, - prefMigrator, + ValueFuture(prefMigrator), NoopLogger ) store.load(User()).user @@ -155,7 +187,13 @@ internal class UserStoreTest { */ @Test fun saveWithoutPersistUser() { - val store = UserStore(generateConfig(false), "", file, prefMigrator, NoopLogger) + val store = UserStore( + generateConfig(false), + ValueFuture(DeviceIdStore.DeviceIds("", null)), + file, + ValueFuture(prefMigrator), + NoopLogger + ) store.save(User("123", "joe@yahoo.com", "Joe Bloggs")) assertFalse(file.exists()) } @@ -165,7 +203,13 @@ internal class UserStoreTest { */ @Test fun saveWithPersistUser() { - val store = UserStore(generateConfig(true), "0asdf", file, prefMigrator, NoopLogger) + val store = UserStore( + generateConfig(true), + ValueFuture(DeviceIdStore.DeviceIds("0asdf", null)), + file, + ValueFuture(prefMigrator), + NoopLogger + ) val user = User("jf123", "test@example.com", "Jane Fonda") store.save(user) val expected = "{\"id\":\"jf123\",\"email\":\"test@example.com\",\"name\":\"Jane Fonda\"}" @@ -177,7 +221,13 @@ internal class UserStoreTest { */ @Test fun userRequiresChangeForDiskIO() { - val store = UserStore(generateConfig(true), "0asdf", file, prefMigrator, NoopLogger) + val store = UserStore( + generateConfig(true), + ValueFuture(DeviceIdStore.DeviceIds("0asdf", null)), + file, + ValueFuture(prefMigrator), + NoopLogger + ) val user = User("jf123", "test@example.com", "Jane Fonda") store.save(user) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java index afc6675044..d98c842289 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java @@ -165,15 +165,15 @@ public Unit invoke(Boolean hasConnection, String networkState) { + "Bugsnag.start(context.getApplicationContext()). " + "For further info see: " + "https://docs.bugsnag.com/platforms/android/#basic-configuration"); - } BugsnagStoreMigrator.moveToNewDirectory( - immutableConfig.getPersistenceDirectory().getValue()); + immutableConfig.getPersistenceDirectory().getValue() + ); // setup storage as soon as possible final StorageModule storageModule = new StorageModule(appContext, - immutableConfig, logger); + immutableConfig, bgTaskService); // setup state trackers for bugsnag BugsnagStateModule bugsnagStateModule = new BugsnagStateModule( @@ -188,9 +188,6 @@ public Unit invoke(Boolean hasConnection, String networkState) { // lookup system services final SystemServiceModule systemServiceModule = new SystemServiceModule(contextModule); - // block until storage module has resolved everything - storageModule.resolveDependencies(bgTaskService, TaskType.IO); - // setup further state trackers and data collection TrackerModule trackerModule = new TrackerModule(configModule, storageModule, this, bgTaskService, callbackState); @@ -199,20 +196,17 @@ public Unit invoke(Boolean hasConnection, String networkState) { DataCollectionModule dataCollectionModule = new DataCollectionModule(contextModule, configModule, systemServiceModule, trackerModule, - bgTaskService, connectivity, storageModule.getDeviceId(), - storageModule.getInternalDeviceId(), memoryTrimState); - dataCollectionModule.resolveDependencies(bgTaskService, TaskType.IO); + bgTaskService, connectivity, storageModule.getDeviceIdStore(), + memoryTrimState); appDataCollector = dataCollectionModule.getAppDataCollector(); deviceDataCollector = dataCollectionModule.getDeviceDataCollector(); // load the device + user information userState = storageModule.getUserStore().load(configuration.getUser()); - storageModule.getSharedPrefMigrator().deleteLegacyPrefs(); EventStorageModule eventStorageModule = new EventStorageModule(contextModule, configModule, dataCollectionModule, bgTaskService, trackerModule, systemServiceModule, notifier, callbackState); - eventStorageModule.resolveDependencies(bgTaskService, TaskType.IO); eventStore = eventStorageModule.getEventStore(); deliveryDelegate = new DeliveryDelegate(logger, eventStore, diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt index 70e9bd78f1..582a33f4c6 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt @@ -2,10 +2,14 @@ package com.bugsnag.android import android.os.Environment import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.TaskType import com.bugsnag.android.internal.dag.ConfigModule import com.bugsnag.android.internal.dag.ContextModule import com.bugsnag.android.internal.dag.DependencyModule import com.bugsnag.android.internal.dag.SystemServiceModule +import java.util.concurrent.Callable +import java.util.concurrent.Future +import java.util.concurrent.RejectedExecutionException /** * A dependency module which constructs the objects that collect data in Bugsnag. For example, this @@ -18,8 +22,7 @@ internal class DataCollectionModule( trackerModule: TrackerModule, bgTaskService: BackgroundTaskService, connectivity: Connectivity, - deviceId: String?, - internalDeviceId: String?, + deviceIdStore: Future, memoryTrimState: MemoryTrimState ) : DependencyModule() { @@ -29,7 +32,7 @@ internal class DataCollectionModule( private val deviceBuildInfo: DeviceBuildInfo = DeviceBuildInfo.defaultInfo() private val dataDir = Environment.getDataDirectory() - val appDataCollector by future { + val appDataCollector = AppDataCollector( ctx, ctx.packageManager, @@ -39,24 +42,30 @@ internal class DataCollectionModule( trackerModule.launchCrashTracker, memoryTrimState ) - } - - private val rootDetector by future { - RootDetector(logger = logger, deviceBuildInfo = deviceBuildInfo) - } - val deviceDataCollector by future { + val deviceDataCollector = DeviceDataCollector( connectivity, ctx, ctx.resources, - deviceId, - internalDeviceId, + deviceIdStore, deviceBuildInfo, dataDir, - rootDetector, + rootDetectionFuture(bgTaskService), bgTaskService, logger ) + + private fun rootDetectionFuture(bgTaskService: BackgroundTaskService): Future? = try { + bgTaskService.submitTask( + TaskType.IO, + Callable { + val rootDetector = RootDetector(logger = logger, deviceBuildInfo = deviceBuildInfo) + rootDetector.isRooted() + } + ) + } catch (exc: RejectedExecutionException) { + logger.w("Failed to perform root detection checks", exc) + null } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt index 493763a493..16fc655ac1 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt @@ -28,11 +28,10 @@ internal class DeviceDataCollector( private val connectivity: Connectivity, private val appContext: Context, resources: Resources, - private val deviceId: String?, - private val internalDeviceId: String?, + private val deviceIdStore: Future, private val buildInfo: DeviceBuildInfo, private val dataDirectory: File, - rootDetector: RootDetector, + private val rootedFuture: Future?, private val bgTaskService: BackgroundTaskService, private val logger: Logger ) { @@ -45,7 +44,6 @@ internal class DeviceDataCollector( private val locale = Locale.getDefault().toString() private val cpuAbi = getCpuAbi() private var runtimeVersions: MutableMap - private val rootedFuture: Future? private val totalMemoryFuture: Future? = retrieveTotalDeviceMemory() private var orientation = AtomicInteger(resources.configuration.orientation) @@ -54,25 +52,13 @@ internal class DeviceDataCollector( buildInfo.apiLevel?.let { map["androidApiLevel"] = it } buildInfo.osBuild?.let { map["osBuild"] = it } runtimeVersions = map - - rootedFuture = try { - bgTaskService.submitTask( - TaskType.IO, - Callable { - rootDetector.isRooted() - } - ) - } catch (exc: RejectedExecutionException) { - logger.w("Failed to perform root detection checks", exc) - null - } } fun generateDevice() = Device( buildInfo, cpuAbi, checkIsRooted(), - deviceId, + deviceIdStore.get()?.deviceId, locale, totalMemoryFuture.runCatching { this?.get() }.getOrNull(), runtimeVersions.toMutableMap() @@ -81,7 +67,7 @@ internal class DeviceDataCollector( fun generateDeviceWithState(now: Long) = DeviceWithState( buildInfo, checkIsRooted(), - deviceId, + deviceIdStore.get()?.deviceId, locale, totalMemoryFuture.runCatching { this?.get() }.getOrNull(), runtimeVersions.toMutableMap(), @@ -94,7 +80,7 @@ internal class DeviceDataCollector( fun generateInternalDeviceWithState(now: Long) = DeviceWithState( buildInfo, checkIsRooted(), - internalDeviceId, + deviceIdStore.get()?.internalDeviceId, locale, totalMemoryFuture.runCatching { this?.get() }.getOrNull(), runtimeVersions.toMutableMap(), diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceIdStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceIdStore.kt index 74dc3e87e9..b86a9d02cd 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceIdStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceIdStore.kt @@ -1,9 +1,12 @@ package com.bugsnag.android import android.content.Context +import com.bugsnag.android.DeviceIdStore.DeviceIds import com.bugsnag.android.internal.ImmutableConfig import java.io.File import java.util.UUID +import java.util.concurrent.Callable +import java.util.concurrent.Future /** * This class is responsible for persisting and retrieving the device ID and internal device ID, @@ -11,23 +14,20 @@ import java.util.UUID */ 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, + private val deviceIdFile: File = File(context.filesDir, "device-id"), + private val deviceIdGenerator: () -> UUID = { UUID.randomUUID() }, + private val internalDeviceIdFile: File = File(context.filesDir, "internal-device-id"), + private val internalDeviceIdGenerator: () -> UUID = { UUID.randomUUID() }, + private val sharedPrefMigrator: Future, config: ImmutableConfig, - logger: Logger -) { + private val logger: Logger +) : Callable { - private val persistence: DeviceIdPersistence - private val internalPersistence: DeviceIdPersistence + private lateinit var persistence: DeviceIdPersistence + private lateinit var internalPersistence: DeviceIdPersistence private val generateId = config.generateAnonymousId - init { - persistence = DeviceIdFilePersistence(deviceIdfile, deviceIdGenerator, logger) - internalPersistence = DeviceIdFilePersistence(internalDeviceIdfile, internalDeviceIdGenerator, logger) - } + var deviceIds: DeviceIds? = null /** * Loads the device ID from @@ -37,7 +37,7 @@ internal class DeviceIdStore @JvmOverloads @Suppress("LongParameterList") constr * If no device ID exists then the legacy value stored in [SharedPreferences] will * be used. If no value is present then a random UUID will be generated and persisted. */ - fun loadDeviceId(): String? { + private 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 @@ -48,14 +48,14 @@ internal class DeviceIdStore @JvmOverloads @Suppress("LongParameterList") constr if (result != null) { return result } - result = sharedPrefMigrator.loadDeviceId(false) + result = sharedPrefMigrator.get().loadDeviceId(false) if (result != null) { return result } return persistence.loadDeviceId(true) } - fun loadInternalDeviceId(): String? { + private 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 @@ -64,4 +64,24 @@ internal class DeviceIdStore @JvmOverloads @Suppress("LongParameterList") constr } return internalPersistence.loadDeviceId(true) } + + override fun call(): DeviceIds? { + persistence = DeviceIdFilePersistence(deviceIdFile, deviceIdGenerator, logger) + internalPersistence = + DeviceIdFilePersistence(internalDeviceIdFile, internalDeviceIdGenerator, logger) + + val deviceId = loadDeviceId() + val internalDeviceId = loadInternalDeviceId() + + if (deviceId != null || internalDeviceId != null) { + deviceIds = DeviceIds(deviceId, internalDeviceId) + } + + return deviceIds + } + + data class DeviceIds( + val deviceId: String?, + val internalDeviceId: String? + ) } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStorageModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStorageModule.kt index bad4b68c39..08def2960e 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStorageModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStorageModule.kt @@ -22,8 +22,8 @@ internal class EventStorageModule( private val cfg = configModule.config - private val delegate by future { - if (cfg.telemetry.contains(Telemetry.INTERNAL_ERRORS) == true) + private val delegate by lazy { + if (cfg.telemetry.contains(Telemetry.INTERNAL_ERRORS)) InternalReportDelegate( contextModule.ctx, cfg.logger, @@ -37,7 +37,7 @@ internal class EventStorageModule( ) else null } - val eventStore by future { + val eventStore by lazy { EventStore( cfg, cfg.logger, diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt index 34b5bb403e..69f7c77632 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.kt @@ -31,10 +31,6 @@ internal abstract class FileStore( private val lock: Lock = ReentrantLock() private val queuedFiles: MutableCollection = ConcurrentSkipListSet() - init { - isStorageDirValid(storageDir) - } - /** * Checks whether the storage directory is a writable directory. If it is not, * this method will attempt to create the directory. diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/LastRunInfoStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/LastRunInfoStore.kt index d5b1a93d42..24d4a718a9 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/LastRunInfoStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/LastRunInfoStore.kt @@ -2,6 +2,7 @@ package com.bugsnag.android import com.bugsnag.android.internal.ImmutableConfig import java.io.File +import java.util.concurrent.Callable import java.util.concurrent.locks.ReentrantReadWriteLock import kotlin.concurrent.withLock @@ -14,12 +15,24 @@ private const val KEY_CRASHED_DURING_LAUNCH = "crashedDuringLaunch" * Persists/loads [LastRunInfo] on disk, which allows Bugsnag to determine * whether the previous application launch crashed or not. This class is thread-safe. */ -internal class LastRunInfoStore(config: ImmutableConfig) { +internal class LastRunInfoStore(config: ImmutableConfig) : Callable { val file: File = File(config.persistenceDirectory.value, "bugsnag/last-run-info") private val logger: Logger = config.logger private val lock = ReentrantReadWriteLock() + var lastRunInfo: LastRunInfo? = null + + override fun call(): LastRunInfoStore { + val info = load() + val currentRunInfo = LastRunInfo(0, crashed = false, crashedDuringLaunch = false) + persist(currentRunInfo) + + lastRunInfo = info + + return this + } + fun persist(lastRunInfo: LastRunInfo) { lock.writeLock().withLock { try { @@ -36,6 +49,7 @@ internal class LastRunInfoStore(config: ImmutableConfig) { add(KEY_CRASHED, lastRunInfo.crashed) add(KEY_CRASHED_DURING_LAUNCH, lastRunInfo.crashedDuringLaunch) }.toString() + file.parentFile?.mkdirs() file.writeText(text) logger.d("Persisted: $text") } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/SharedPrefMigrator.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/SharedPrefMigrator.kt index 2f7ccc238f..d5365446cb 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/SharedPrefMigrator.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/SharedPrefMigrator.kt @@ -2,45 +2,61 @@ package com.bugsnag.android import android.annotation.SuppressLint import android.content.Context -import android.content.SharedPreferences +import android.os.Build +import java.util.concurrent.Callable /** * Reads legacy information left in SharedPreferences and migrates it to the new location. */ -internal class SharedPrefMigrator(context: Context) : DeviceIdPersistence { +internal class SharedPrefMigrator(private val context: Context) : + DeviceIdPersistence, + Callable { - private val prefs: SharedPreferences? = + private var installId: String? = null + private var userId: String? = null + private var userEmail: String? = null + private var userName: String? = null + + override fun call(): SharedPrefMigrator { try { - context.getSharedPreferences("com.bugsnag.android", Context.MODE_PRIVATE) - } catch (e: RuntimeException) { - null + val prefs = context.getSharedPreferences(SHARED_PREFS_NAME, Context.MODE_PRIVATE) + ?: return this + + installId = prefs.getString(INSTALL_ID_KEY, null) + userId = prefs.getString(USER_ID_KEY, null) + userEmail = prefs.getString(USER_EMAIL_KEY, null) + userName = prefs.getString(USER_NAME_KEY, null) + + @SuppressLint("ApplySharedPref") + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + context.deleteSharedPreferences(SHARED_PREFS_NAME) + } else { + prefs.edit().clear().commit() + } + } catch (_: RuntimeException) { } + return this + } + /** * This implementation will never create an ID; it will only fetch one if present. */ - override fun loadDeviceId(requestCreateIfDoesNotExist: Boolean) = - prefs?.getString(INSTALL_ID_KEY, null) + override fun loadDeviceId(requestCreateIfDoesNotExist: Boolean) = installId fun loadUser(deviceId: String?) = User( - prefs?.getString(USER_ID_KEY, deviceId), - prefs?.getString(USER_EMAIL_KEY, null), - prefs?.getString(USER_NAME_KEY, null) + userId ?: deviceId, + userEmail, + userName ) - fun hasPrefs() = prefs?.contains(INSTALL_ID_KEY) == true - - @SuppressLint("ApplySharedPref") - fun deleteLegacyPrefs() { - if (hasPrefs()) { - prefs?.edit()?.clear()?.commit() - } - } + fun hasPrefs() = installId != null companion object { private const val INSTALL_ID_KEY = "install.iud" private const val USER_ID_KEY = "user.id" private const val USER_NAME_KEY = "user.name" private const val USER_EMAIL_KEY = "user.email" + private const val SHARED_PREFS_NAME = "com.bugsnag.android" } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/StorageModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/StorageModule.kt index 8defc58ac4..0ca701141f 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/StorageModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/StorageModule.kt @@ -1,7 +1,9 @@ package com.bugsnag.android import android.content.Context +import com.bugsnag.android.internal.BackgroundTaskService import com.bugsnag.android.internal.ImmutableConfig +import com.bugsnag.android.internal.TaskType import com.bugsnag.android.internal.dag.DependencyModule /** @@ -10,47 +12,46 @@ import com.bugsnag.android.internal.dag.DependencyModule internal class StorageModule( appContext: Context, immutableConfig: ImmutableConfig, - logger: Logger + bgTaskService: BackgroundTaskService ) : DependencyModule() { - val sharedPrefMigrator by future { SharedPrefMigrator(appContext) } + val sharedPrefMigrator = bgTaskService.submitTask( + TaskType.IO, + SharedPrefMigrator(appContext) + ) - private val deviceIdStore by future { + val deviceIdStore = bgTaskService.submitTask( + TaskType.IO, DeviceIdStore( appContext, sharedPrefMigrator = sharedPrefMigrator, - logger = logger, + logger = immutableConfig.logger, config = immutableConfig ) - } + ) - val deviceId by future { deviceIdStore.loadDeviceId() } + val userStore = UserStore( + immutableConfig, + deviceIdStore, + sharedPrefMigrator = sharedPrefMigrator, + logger = immutableConfig.logger + ) - val internalDeviceId by future { deviceIdStore.loadInternalDeviceId() } + val lastRunInfoStore = LastRunInfoStore(immutableConfig) - val userStore by future { - UserStore( - immutableConfig, - deviceId, - sharedPrefMigrator = sharedPrefMigrator, - logger = logger - ) - } - - val lastRunInfoStore by future { LastRunInfoStore(immutableConfig) } - - val sessionStore by future { + val sessionStore = SessionStore( immutableConfig, - logger, + immutableConfig.logger, null ) - } - val lastRunInfo by future { + val lastRunInfo = lastRunInfo() + + private fun lastRunInfo(): LastRunInfo? { val info = lastRunInfoStore.load() val currentRunInfo = LastRunInfo(0, crashed = false, crashedDuringLaunch = false) lastRunInfoStore.persist(currentRunInfo) - info + return info } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/UserStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/UserStore.kt index 548bcdbc35..13701d07a2 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/UserStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/UserStore.kt @@ -3,6 +3,7 @@ package com.bugsnag.android import com.bugsnag.android.internal.ImmutableConfig import com.bugsnag.android.internal.StateObserver import java.io.File +import java.util.concurrent.Future import java.util.concurrent.atomic.AtomicReference /** @@ -10,9 +11,9 @@ import java.util.concurrent.atomic.AtomicReference */ internal class UserStore @JvmOverloads constructor( private val config: ImmutableConfig, - private val deviceId: String?, + private val deviceIdStore: Future, file: File = File(config.persistenceDirectory.value, "bugsnag/user-info"), - private val sharedPrefMigrator: SharedPrefMigrator, + private val sharedPrefMigrator: Future, private val logger: Logger ) { @@ -50,7 +51,7 @@ internal class UserStore @JvmOverloads constructor( 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)) + else -> UserState(User(deviceIdStore.get()?.deviceId, null, null)) } userState.addObserver( @@ -81,8 +82,8 @@ internal class UserStore @JvmOverloads constructor( user.id != null || user.name != null || user.email != null private fun loadPersistedUser(): User? { - return if (sharedPrefMigrator.hasPrefs()) { - val legacyUser = sharedPrefMigrator.loadUser(deviceId) + return if (sharedPrefMigrator.get().hasPrefs()) { + val legacyUser = sharedPrefMigrator.get().loadUser(deviceIdStore.get()?.deviceId) save(legacyUser) legacyUser } else if ( diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/ConfigModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/ConfigModule.kt index 842057c2bc..2d55c808be 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/ConfigModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/ConfigModule.kt @@ -15,6 +15,5 @@ internal class ConfigModule( connectivity: Connectivity, bgTaskExecutor: BackgroundTaskService ) : DependencyModule() { - val config = sanitiseConfiguration(contextModule.ctx, configuration, connectivity, bgTaskExecutor) } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt index 731e5adfc5..b49a0fedd7 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt @@ -1,37 +1,3 @@ package com.bugsnag.android.internal.dag -import com.bugsnag.android.internal.BackgroundTaskService -import com.bugsnag.android.internal.TaskType - -internal abstract class DependencyModule { - - private val properties = mutableListOf>() - - /** - * Creates a new [Lazy] property that is marked as an object that should be resolved off the - * main thread when [resolveDependencies] is called. - */ - fun future(initializer: () -> T): Lazy { - val lazy = lazy { - initializer() - } - properties.add(lazy) - return lazy - } - - /** - * Blocks until all dependencies in the module have been constructed. This provides the option - * for modules to construct objects in a background thread, then have a user block on another - * thread until all the objects have been constructed. - */ - fun resolveDependencies(bgTaskService: BackgroundTaskService, taskType: TaskType) { - kotlin.runCatching { - bgTaskService.submitTask( - taskType, - Runnable { - properties.forEach { it.value } - } - ).get() - } - } -} +internal abstract class DependencyModule diff --git a/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/BugsnagTestUtils.java b/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/BugsnagTestUtils.java index 938a3b884e..57fdfb5318 100644 --- a/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/BugsnagTestUtils.java +++ b/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/BugsnagTestUtils.java @@ -4,11 +4,18 @@ import com.bugsnag.android.internal.ImmutableConfigKt; import androidx.annotation.NonNull; +import androidx.test.core.app.ApplicationProvider; + +import org.json.JSONArray; +import org.json.JSONException; +import org.json.JSONObject; import java.io.File; import java.io.IOException; +import java.io.StringWriter; import java.nio.file.Files; import java.util.Collections; +import java.util.Comparator; import java.util.Date; import java.util.HashMap; @@ -25,7 +32,6 @@ static Configuration generateConfiguration() { Configuration configuration = new Configuration("5d1ec5bd39a74caa1267142706a7fb21"); configuration.setDelivery(generateDelivery()); configuration.setLogger(NoopLogger.INSTANCE); - configuration.setProjectPackages(Collections.singleton("com.example.foo")); try { File dir = Files.createTempDirectory("test").toFile(); configuration.setPersistenceDirectory(dir); @@ -132,4 +138,39 @@ public static App generateApp() { static MetadataState generateMetadataState() { return new MetadataState(); } + + private static String streamableToString(JsonStream.Streamable streamable) throws IOException { + StringWriter writer = new StringWriter(); + JsonStream jsonStream = new JsonStream(writer); + streamable.toStream(jsonStream); + return writer.toString(); + } + + static JSONObject streamableToJson(JsonStream.Streamable streamable) + throws JSONException, IOException { + return new JSONObject(streamableToString(streamable)); + } + + static JSONArray streamableToJsonArray(JsonStream.Streamable streamable) + throws JSONException, IOException { + return new JSONArray(streamableToString(streamable)); + } + + static Client generateClient(Configuration config) { + config.setDelivery(generateDelivery()); + return new Client(ApplicationProvider.getApplicationContext(), config); + } + + static Client generateClient() { + return generateClient(new Configuration("5d1ec5bd39a74caa1267142706a7fb21")); + } + + static Comparator featureFlagComparator() { + return new Comparator() { + @Override + public int compare(FeatureFlag f1, FeatureFlag f2) { + return f1.getName().compareTo(f2.getName()); + } + }; + } } \ No newline at end of file diff --git a/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueFuture.kt b/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueFuture.kt new file mode 100644 index 0000000000..9d47936cf6 --- /dev/null +++ b/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueFuture.kt @@ -0,0 +1,12 @@ +package com.bugsnag.android + +import java.util.concurrent.Future +import java.util.concurrent.TimeUnit + +class ValueFuture(private val value: V) : Future { + override fun cancel(mayInterruptIfRunning: Boolean): Boolean = false + override fun isCancelled(): Boolean = false + override fun isDone(): Boolean = true + override fun get(): V = value + override fun get(timeout: Long, unit: TimeUnit?): V = get() +} diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt index 160308534d..fafb9dd42a 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt @@ -51,11 +51,10 @@ internal class ConfigChangeReceiverTest { connectivity, appContext, resources, - "", - "", + ValueFuture(null), buildInfo, dataDirectory, - rootDetector, + ValueFuture(false), bgTaskService, NoopLogger ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DataCollectorTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DataCollectorTest.kt index 307151bc20..357c160882 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DataCollectorTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/DataCollectorTest.kt @@ -16,7 +16,7 @@ import java.io.File import kotlin.concurrent.thread @RunWith(MockitoJUnitRunner.Silent::class) -class DataCollectorTest { +internal class DataCollectorTest { private lateinit var collector: DeviceDataCollector @@ -35,11 +35,10 @@ class DataCollectorTest { Mockito.mock(Connectivity::class.java), Mockito.mock(Context::class.java), res, - "fakeDevice", - "internalFakeDevice", + ValueFuture(DeviceIdStore.DeviceIds("fakeDevice", "internalFakeDevice")), Mockito.mock(DeviceBuildInfo::class.java), File("/tmp/javatest"), - Mockito.mock(RootDetector::class.java), + ValueFuture(false), Mockito.mock(BackgroundTaskService::class.java), Mockito.mock(Logger::class.java) ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt index 09ca3d3ab3..da2c7b5c8b 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt @@ -28,7 +28,6 @@ internal class DeviceDataCollectorSerializationTest { val res = mock(Resources::class.java) val conf = mock(Configuration::class.java) val connectivity = mock(Connectivity::class.java) - val rootDetector = mock(RootDetector::class.java) val prefs = mock(SharedPreferences::class.java) val editor = mock(SharedPreferences.Editor::class.java) @@ -54,11 +53,10 @@ internal class DeviceDataCollectorSerializationTest { connectivity, context, res, - "123", - "456", + ValueFuture(DeviceIdStore.DeviceIds("123", "456")), buildInfo, File(""), - rootDetector, + ValueFuture(false), BackgroundTaskService(), NoopLogger ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt index c3b8c3d079..2a213c6850 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt @@ -28,7 +28,6 @@ internal class DeviceMetadataSerializationTest { val res = mock(Resources::class.java) val conf = mock(Configuration::class.java) val connectivity = mock(Connectivity::class.java) - val rootDetector = mock(RootDetector::class.java) val prefs = mock(SharedPreferences::class.java) val editor = mock(SharedPreferences.Editor::class.java) @@ -54,11 +53,10 @@ internal class DeviceMetadataSerializationTest { connectivity, context, res, - "123", - "456", + ValueFuture(DeviceIdStore.DeviceIds("123", "456")), buildInfo, File(""), - rootDetector, + ValueFuture(false), BackgroundTaskService(), NoopLogger ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/EventRedactionTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/EventRedactionTest.kt index 24c41ccf75..7bf54dcdd6 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/EventRedactionTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/EventRedactionTest.kt @@ -1,12 +1,12 @@ package com.bugsnag.android import com.bugsnag.android.BugsnagTestUtils.generateAppWithState +import com.bugsnag.android.BugsnagTestUtils.generateConfiguration import com.bugsnag.android.BugsnagTestUtils.generateDeviceWithState import com.bugsnag.android.BugsnagTestUtils.generateImmutableConfig import org.junit.Test import java.io.StringWriter import java.util.Date -import java.util.regex.Pattern internal class EventRedactionTest { @@ -14,8 +14,18 @@ internal class EventRedactionTest { fun testEventRedaction() { val event = Event( null, - generateImmutableConfig() - .copy(redactedKeys = setOf(Pattern.compile(".*password.*"), Pattern.compile(".*changeme.*"))), + generateImmutableConfig( + generateConfiguration().apply { + redactedKeys = setOf( + ".*password.*".toPattern(), + ".*changeme.*".toPattern() + ) + + projectPackages = setOf( + "com.example.foo" + ) + } + ), SeverityReason.newInstance(SeverityReason.REASON_HANDLED_EXCEPTION), NoopLogger ) @@ -27,7 +37,13 @@ internal class EventRedactionTest { fun testDefaultEventRedaction() { val event = Event( null, - generateImmutableConfig(), + generateImmutableConfig( + generateConfiguration().apply { + projectPackages = setOf( + "com.example.foo" + ) + } + ), SeverityReason.newInstance(SeverityReason.REASON_HANDLED_EXCEPTION), NoopLogger ) @@ -43,7 +59,8 @@ internal class EventRedactionTest { event.addMetadata("device", "password", "bar") event.impl.metadata.addMetadata("baz", "password", "hunter2") val metadata = mutableMapOf(Pair("changeme", "whoops")) - event.breadcrumbs = listOf(Breadcrumb("Whoops", BreadcrumbType.LOG, metadata, Date(0), NoopLogger)) + event.breadcrumbs = + listOf(Breadcrumb("Whoops", BreadcrumbType.LOG, metadata, Date(0), NoopLogger)) event.threads.clear() event.device.cpuAbi = emptyArray() diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/EventSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/EventSerializationTest.kt index 84175e34e7..f34cd729ef 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/EventSerializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/EventSerializationTest.kt @@ -1,6 +1,7 @@ package com.bugsnag.android import com.bugsnag.android.BugsnagTestUtils.generateAppWithState +import com.bugsnag.android.BugsnagTestUtils.generateConfiguration import com.bugsnag.android.BugsnagTestUtils.generateDeviceWithState import com.bugsnag.android.BugsnagTestUtils.generateImmutableConfig import org.junit.Test @@ -99,7 +100,11 @@ internal class EventSerializationTest { private fun createEvent(cb: (event: Event) -> Unit = {}): Event { val event = Event( null, - generateImmutableConfig(), + generateImmutableConfig( + generateConfiguration().apply { + projectPackages = setOf("com.example.foo") + } + ), SeverityReason.newInstance(SeverityReason.REASON_HANDLED_EXCEPTION), NoopLogger ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ImmutableConfigTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/ImmutableConfigTest.kt index 1d36ec5f48..37862f6c25 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ImmutableConfigTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ImmutableConfigTest.kt @@ -32,7 +32,9 @@ import java.util.regex.Pattern @RunWith(MockitoJUnitRunner::class) internal class ImmutableConfigTest { - private val seed = generateConfiguration() + private val seed = generateConfiguration().apply { + projectPackages = setOf("com.example.foo") + } @Mock lateinit var delivery: Delivery diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/SharedPrefMigratorTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/SharedPrefMigratorTest.kt index 21a889aa74..16716a564f 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/SharedPrefMigratorTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/SharedPrefMigratorTest.kt @@ -9,11 +9,10 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.any import org.mockito.ArgumentMatchers.anyInt import org.mockito.ArgumentMatchers.eq import org.mockito.Mock -import org.mockito.Mockito.times -import org.mockito.Mockito.verify import org.mockito.Mockito.`when` import org.mockito.junit.MockitoJUnitRunner @@ -40,34 +39,37 @@ internal class SharedPrefMigratorTest { @Test fun nullSharedPreferences() { `when`(context.getSharedPreferences(eq("com.bugsnag.android"), anyInt())).thenReturn(null) - prefMigrator = SharedPrefMigrator(context) + prefMigrator.call() assertFalse(prefMigrator.hasPrefs()) } @Test fun gettingSharedPreferencesWithException() { `when`(context.getSharedPreferences(eq("com.bugsnag.android"), anyInt())).thenThrow(RuntimeException()) - prefMigrator = SharedPrefMigrator(context) + prefMigrator.call() assertFalse(prefMigrator.hasPrefs()) } @Test fun nullDeviceId() { `when`(prefs.getString("install.iud", null)).thenReturn(null) + prefMigrator.call() assertNull(prefMigrator.loadDeviceId(true)) } @Test fun validDeviceId() { `when`(prefs.getString("install.iud", null)).thenReturn("f09asdfb") + prefMigrator.call() assertEquals("f09asdfb", prefMigrator.loadDeviceId(true)) } @Test fun emptyUser() { - `when`(prefs.getString("user.id", "f09asdfb")).thenReturn("f09asdfb") + `when`(prefs.getString(eq("user.id"), any())).thenReturn("f09asdfb") `when`(prefs.getString("user.email", null)).thenReturn(null) `when`(prefs.getString("user.name", null)).thenReturn(null) + prefMigrator.call() val observed = prefMigrator.loadUser("f09asdfb") assertEquals("f09asdfb", observed.id) @@ -77,9 +79,10 @@ internal class SharedPrefMigratorTest { @Test fun populatedUser() { - `when`(prefs.getString("user.id", "f09asdfb")).thenReturn("abc75092") + `when`(prefs.getString(eq("user.id"), any())).thenReturn("abc75092") `when`(prefs.getString("user.email", null)).thenReturn("test@example.com") `when`(prefs.getString("user.name", null)).thenReturn("Joe") + prefMigrator.call() val expected = User("abc75092", "test@example.com", "Joe") val observed = prefMigrator.loadUser("f09asdfb") @@ -88,25 +91,17 @@ internal class SharedPrefMigratorTest { assertEquals(expected.name, observed.name) } - @Test - fun deletePrefs() { - `when`(prefs.contains("install.iud")).thenReturn(true) - `when`(prefs.edit()).thenReturn(editor) - `when`(editor.clear()).thenReturn(editor) - prefMigrator.deleteLegacyPrefs() - verify(editor, times(1)).clear() - verify(editor, times(1)).commit() - } - @Test fun hasPrefsTrue() { - `when`(prefs.contains("install.iud")).thenReturn(true) + `when`(prefs.getString("install.iud", null)).thenReturn("abc123") + prefMigrator.call() assertTrue(prefMigrator.hasPrefs()) } @Test fun hasPrefsFalse() { - `when`(prefs.contains("install.iud")).thenReturn(false) + `when`(prefs.getString("install.iud", null)).thenReturn(null) + prefMigrator.call() assertFalse(prefMigrator.hasPrefs()) } } diff --git a/buildSrc/src/main/kotlin/com/bugsnag/android/BugsnagBuildPlugin.kt b/buildSrc/src/main/kotlin/com/bugsnag/android/BugsnagBuildPlugin.kt index 3f0d29e096..2277e7ea3d 100644 --- a/buildSrc/src/main/kotlin/com/bugsnag/android/BugsnagBuildPlugin.kt +++ b/buildSrc/src/main/kotlin/com/bugsnag/android/BugsnagBuildPlugin.kt @@ -165,6 +165,7 @@ class BugsnagBuildPlugin : Plugin { add("testImplementation", "junit:junit:${Versions.junitTestLib}") add("testImplementation", "org.mockito:mockito-core:${Versions.mockitoTestLib}") add("testImplementation", "org.mockito:mockito-inline:${Versions.mockitoTestLib}") + add("testImplementation", "androidx.test:core:${Versions.supportTestLib}") add( "androidTestImplementation", diff --git a/buildSrc/src/main/kotlin/com/bugsnag/android/Versions.kt b/buildSrc/src/main/kotlin/com/bugsnag/android/Versions.kt index ae6b8ecc45..24f88355c9 100644 --- a/buildSrc/src/main/kotlin/com/bugsnag/android/Versions.kt +++ b/buildSrc/src/main/kotlin/com/bugsnag/android/Versions.kt @@ -25,6 +25,6 @@ object Versions { val supportLib = "1.1.0" val supportTestLib = "1.2.0" val espressoTestLib = "3.1.0" - val junitTestLib = "4.12" - val mockitoTestLib = "2.28.2" + val junitTestLib = "4.13.2" + val mockitoTestLib = "4.11.0" } \ No newline at end of file diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/MultiProcessHandledExceptionScenario.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/MultiProcessHandledExceptionScenario.kt index c04a12f81b..65df0c85fc 100644 --- a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/MultiProcessHandledExceptionScenario.kt +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/mazerunner/scenarios/MultiProcessHandledExceptionScenario.kt @@ -4,6 +4,7 @@ import android.content.Context import com.bugsnag.android.Bugsnag import com.bugsnag.android.Configuration import com.bugsnag.android.mazerunner.BugsnagIntentParams +import com.bugsnag.android.mazerunner.log /** * Sends handled exceptions to Bugsnag from two different processes @@ -24,10 +25,16 @@ internal class MultiProcessHandledExceptionScenario( eventMetadata ) ) { - Bugsnag.setUser("2", "background@example.com", "MultiProcessHandledExceptionScenario") + log("Sending event from background") + Bugsnag.setUser( + "2", + "background@example.com", + "MultiProcessHandledExceptionScenario" + ) Bugsnag.notify(generateException()) } } else { + log("Sending event from foreground") Bugsnag.setUser("1", "foreground@example.com", "MultiProcessHandledExceptionScenario") Bugsnag.notify(generateException()) } diff --git a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/multiprocess/MultiProcessService.kt b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/multiprocess/MultiProcessService.kt index f75377ab91..4b8aaffdf5 100644 --- a/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/multiprocess/MultiProcessService.kt +++ b/features/fixtures/mazerunner/jvm-scenarios/src/main/java/com/bugsnag/android/multiprocess/MultiProcessService.kt @@ -23,7 +23,11 @@ class MultiProcessService : Service() { if (intent != null) { val params = BugsnagIntentParams.decode(intent) log("Multiprocess service received start command: $params") - sendBroadcast(Intent(ACTION_LAUNCHED_MULTI_PROCESS)) + sendBroadcast( + Intent(ACTION_LAUNCHED_MULTI_PROCESS).apply { + `package` = this@MultiProcessService.packageName + } + ) // start work in a background thread like regular services val handlerThread = HandlerThread("multi-process-service") From 3068ab42783de18036130c2acce4fb7285672347 Mon Sep 17 00:00:00 2001 From: jason Date: Tue, 6 Aug 2024 14:53:13 +0100 Subject: [PATCH 2/9] refactor(startup): rework startup modules to use a new Provider structure instead of Runnables and Futures --- .../api/bugsnag-android-core.api | 15 +++ bugsnag-android-core/detekt-baseline.xml | 7 +- .../android/BugsnagStoreMigratorTest.kt | 12 +- .../com/bugsnag/android/DeviceIdStoreTest.kt | 40 +++--- .../java/com/bugsnag/android/UserStoreTest.kt | 79 ++++++------ .../com/bugsnag/android/BugsnagStateModule.kt | 2 +- .../main/java/com/bugsnag/android/Client.java | 95 ++++++++------- .../bugsnag/android/DataCollectionModule.kt | 39 +++--- .../bugsnag/android/DeviceDataCollector.kt | 5 +- .../java/com/bugsnag/android/DeviceIdStore.kt | 17 +-- .../com/bugsnag/android/EventStorageModule.kt | 14 +-- .../android/InternalReportDelegate.java | 7 +- .../com/bugsnag/android/LastRunInfoStore.kt | 15 +-- .../bugsnag/android/SessionFilenameInfo.kt | 8 +- .../java/com/bugsnag/android/SessionStore.kt | 13 +- .../com/bugsnag/android/SharedPrefMigrator.kt | 54 +++------ .../java/com/bugsnag/android/StorageModule.kt | 60 +++++---- .../java/com/bugsnag/android/TrackerModule.kt | 23 ++-- .../java/com/bugsnag/android/UserStore.kt | 15 ++- .../android/internal/BackgroundTaskService.kt | 19 ++- .../android/internal/BugsnagStoreMigrator.kt | 13 +- .../android/internal/dag/ConfigModule.kt | 2 +- .../android/internal/dag/ContextModule.kt | 6 +- .../android/internal/dag/DependencyModule.kt | 25 +++- .../bugsnag/android/internal/dag/Provider.kt | 114 ++++++++++++++++++ .../internal/dag/SystemServiceModule.kt | 6 +- .../java/com/bugsnag/android/ValueFuture.kt | 12 -- .../java/com/bugsnag/android/ValueProvider.kt | 8 ++ .../com/bugsnag/android/ClientFacadeTest.java | 2 +- .../android/ConfigChangeReceiverTest.kt | 4 +- .../com/bugsnag/android/DataCollectorTest.kt | 4 +- .../DeviceDataCollectorSerializationTest.kt | 4 +- .../DeviceMetadataSerializationTest.kt | 4 +- .../InternalEventPayloadDelegateTest.kt | 2 +- .../android/SessionStoreMaxLimitTest.kt | 4 +- .../bugsnag/android/SharedPrefMigratorTest.kt | 31 +++-- ... BackgroundRunnableProviderServiceTest.kt} | 2 +- 37 files changed, 469 insertions(+), 313 deletions(-) create mode 100644 bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/Provider.kt delete mode 100644 bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueFuture.kt create mode 100644 bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueProvider.kt rename bugsnag-android-core/src/test/java/com/bugsnag/android/internal/{BackgroundTaskServiceTest.kt => BackgroundRunnableProviderServiceTest.kt} (99%) diff --git a/bugsnag-android-core/api/bugsnag-android-core.api b/bugsnag-android-core/api/bugsnag-android-core.api index 2ffe8e2f0f..4b934864b7 100644 --- a/bugsnag-android-core/api/bugsnag-android-core.api +++ b/bugsnag-android-core/api/bugsnag-android-core.api @@ -851,6 +851,8 @@ public final class com/bugsnag/android/internal/BackgroundTaskService { public fun ()V public fun (Ljava/util/concurrent/ExecutorService;Ljava/util/concurrent/ExecutorService;Ljava/util/concurrent/ExecutorService;Ljava/util/concurrent/ExecutorService;Ljava/util/concurrent/ExecutorService;)V public synthetic fun (Ljava/util/concurrent/ExecutorService;Ljava/util/concurrent/ExecutorService;Ljava/util/concurrent/ExecutorService;Ljava/util/concurrent/ExecutorService;Ljava/util/concurrent/ExecutorService;ILkotlin/jvm/internal/DefaultConstructorMarker;)V + public final fun execute (Lcom/bugsnag/android/internal/TaskType;Ljava/lang/Runnable;)V + public final fun provider (Lcom/bugsnag/android/internal/TaskType;Lkotlin/jvm/functions/Function0;)Lcom/bugsnag/android/internal/dag/RunnableProvider; public final fun shutdown ()V public final fun submitTask (Lcom/bugsnag/android/internal/TaskType;Ljava/lang/Runnable;)Ljava/util/concurrent/Future; public final fun submitTask (Lcom/bugsnag/android/internal/TaskType;Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future; @@ -1009,3 +1011,16 @@ public final class com/bugsnag/android/internal/TaskType : java/lang/Enum { public static fun values ()[Lcom/bugsnag/android/internal/TaskType; } +public abstract interface class com/bugsnag/android/internal/dag/Provider { + public abstract fun get ()Ljava/lang/Object; + public abstract fun getOrNull ()Ljava/lang/Object; +} + +public abstract class com/bugsnag/android/internal/dag/RunnableProvider : com/bugsnag/android/internal/dag/Provider, java/lang/Runnable { + public fun ()V + public fun get ()Ljava/lang/Object; + public fun getOrNull ()Ljava/lang/Object; + public abstract fun invoke ()Ljava/lang/Object; + public final fun run ()V +} + diff --git a/bugsnag-android-core/detekt-baseline.xml b/bugsnag-android-core/detekt-baseline.xml index 5e9988e00a..b12f5c70a5 100644 --- a/bugsnag-android-core/detekt-baseline.xml +++ b/bugsnag-android-core/detekt-baseline.xml @@ -9,10 +9,10 @@ LongParameterList:AppDataCollector.kt$AppDataCollector$( appContext: Context, private val packageManager: PackageManager?, private val config: ImmutableConfig, private val sessionTracker: SessionTracker, private val activityManager: ActivityManager?, private val launchCrashTracker: LaunchCrashTracker, private val memoryTrimState: MemoryTrimState ) LongParameterList:AppWithState.kt$AppWithState$( binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, buildUuid: String?, type: String?, versionCode: Number?, /** * The number of milliseconds the application was running before the event occurred */ var duration: Number?, /** * The number of milliseconds the application was running in the foreground before the * event occurred */ var durationInForeground: Number?, /** * Whether the application was in the foreground when the event occurred */ var inForeground: Boolean?, /** * Whether the application was launching when the event occurred */ var isLaunching: Boolean? ) LongParameterList:AppWithState.kt$AppWithState$( config: ImmutableConfig, binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, duration: Number?, durationInForeground: Number?, inForeground: Boolean?, isLaunching: Boolean? ) - LongParameterList:DataCollectionModule.kt$DataCollectionModule$( contextModule: ContextModule, configModule: ConfigModule, systemServiceModule: SystemServiceModule, trackerModule: TrackerModule, bgTaskService: BackgroundTaskService, connectivity: Connectivity, deviceIdStore: Future<DeviceIdStore.DeviceIds?>, memoryTrimState: MemoryTrimState ) + LongParameterList:DataCollectionModule.kt$DataCollectionModule$( contextModule: ContextModule, configModule: ConfigModule, systemServiceModule: SystemServiceModule, trackerModule: TrackerModule, bgTaskService: BackgroundTaskService, connectivity: Connectivity, deviceIdStore: Provider<DeviceIdStore>, memoryTrimState: MemoryTrimState ) LongParameterList:Device.kt$Device$( buildInfo: DeviceBuildInfo, /** * The Application Binary Interface used */ var cpuAbi: Array<String>?, /** * Whether the device has been jailbroken */ var jailbroken: Boolean?, /** * A UUID generated by Bugsnag and used for the individual application on a device */ var id: String?, /** * The IETF language tag of the locale used */ var locale: String?, /** * The total number of bytes of memory on the device */ var totalMemory: Long?, /** * A collection of names and their versions of the primary languages, frameworks or * runtimes that the application is running on */ runtimeVersions: MutableMap<String, Any>? ) LongParameterList:DeviceBuildInfo.kt$DeviceBuildInfo$( val manufacturer: String?, val model: String?, val osVersion: String?, val apiLevel: Int?, val osBuild: String?, val fingerprint: String?, val tags: String?, val brand: String?, val cpuAbis: Array<String>? ) - LongParameterList:DeviceDataCollector.kt$DeviceDataCollector$( private val connectivity: Connectivity, private val appContext: Context, resources: Resources, private val deviceIdStore: Future<DeviceIdStore.DeviceIds?>, private val buildInfo: DeviceBuildInfo, private val dataDirectory: File, private val rootedFuture: Future<Boolean>?, private val bgTaskService: BackgroundTaskService, private val logger: Logger ) + LongParameterList:DeviceDataCollector.kt$DeviceDataCollector$( private val connectivity: Connectivity, private val appContext: Context, resources: Resources, private val deviceIdStore: Provider<DeviceIdStore.DeviceIds?>, private val buildInfo: DeviceBuildInfo, private val dataDirectory: File, private val rootedFuture: Provider<Boolean>?, private val bgTaskService: BackgroundTaskService, private val logger: Logger ) LongParameterList:DeviceWithState.kt$DeviceWithState$( buildInfo: DeviceBuildInfo, jailbroken: Boolean?, id: String?, locale: String?, totalMemory: Long?, runtimeVersions: MutableMap<String, Any>, /** * The number of free bytes of storage available on the device */ var freeDisk: Long?, /** * The number of free bytes of memory available on the device */ var freeMemory: Long?, /** * The orientation of the device when the event occurred: either portrait or landscape */ var orientation: String?, /** * The timestamp on the device when the event occurred */ var time: Date? ) LongParameterList:EventFilenameInfo.kt$EventFilenameInfo.Companion$( obj: Any, uuid: String = UUID.randomUUID().toString(), apiKey: String?, timestamp: Long = System.currentTimeMillis(), config: ImmutableConfig, isLaunching: Boolean? = null ) LongParameterList:EventInternal.kt$EventInternal$( apiKey: String, logger: Logger, breadcrumbs: MutableList<Breadcrumb> = mutableListOf(), discardClasses: Set<Pattern> = setOf(), errors: MutableList<Error> = mutableListOf(), metadata: Metadata = Metadata(), featureFlags: FeatureFlags = FeatureFlags(), originalError: Throwable? = null, projectPackages: Collection<String> = setOf(), severityReason: SeverityReason = SeverityReason.newInstance(SeverityReason.REASON_HANDLED_EXCEPTION), threads: MutableList<Thread> = mutableListOf(), user: User = User(), redactionKeys: Set<Pattern>? = null ) @@ -63,13 +63,14 @@ SwallowedException:ImmutableConfig.kt$e: Exception SwallowedException:JsonHelperTest.kt$JsonHelperTest$e: IllegalArgumentException SwallowedException:PluginClient.kt$PluginClient$exc: ClassNotFoundException + SwallowedException:SharedPrefMigrator.kt$SharedPrefMigrator$e: RuntimeException ThrowsCount:JsonHelper.kt$JsonHelper$fun jsonToLong(value: Any?): Long? TooManyFunctions:ConfigInternal.kt$ConfigInternal : CallbackAwareMetadataAwareUserAwareFeatureFlagAware TooManyFunctions:DeviceDataCollector.kt$DeviceDataCollector TooManyFunctions:EventInternal.kt$EventInternal : FeatureFlagAwareStreamableMetadataAwareUserAware UnusedPrivateProperty:ManifestConfigLoader.kt$ManifestConfigLoader.Companion$private const val LAUNCH_CRASH_THRESHOLD_MS = "$BUGSNAG_NS.LAUNCH_CRASH_THRESHOLD_MS" UnusedPrivateProperty:ThreadStateTest.kt$ThreadStateTest$private val configuration = generateImmutableConfig() - UseCheckOrError:BackgroundTaskServiceTest.kt$BackgroundTaskServiceTest$throw IllegalStateException() + UseCheckOrError:BackgroundRunnableProviderServiceTest.kt$BackgroundRunnableProviderServiceTest$throw IllegalStateException() UseCheckOrError:BugsnagEventMapper.kt$BugsnagEventMapper$throw IllegalStateException("cannot find json property '$key'") UseCheckOrError:StacktraceSerializationTest.kt$StacktraceSerializationTest.Companion$throw IllegalStateException() UseCheckOrError:SynchronizedStreamableStoreTest.kt$CrashyStreamable$throw IllegalStateException() diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BugsnagStoreMigratorTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BugsnagStoreMigratorTest.kt index af1e70edce..6bc4a12815 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BugsnagStoreMigratorTest.kt +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/BugsnagStoreMigratorTest.kt @@ -35,7 +35,7 @@ class BugsnagStoreMigratorTest { filesToMove.forEach { (from, to) -> val file = File(tmpdir, from).apply { mkdirs() } val newDir = File(tmpdir, to) - BugsnagStoreMigrator.moveToNewDirectory(tmpdir) + BugsnagStoreMigrator.migrateLegacyFiles(lazyOf(tmpdir)) assertFalse(file.isDirectory) assertFalse(file.exists()) assertTrue(newDir.isDirectory) @@ -47,7 +47,7 @@ class BugsnagStoreMigratorTest { fun testMoveOneFileToNewDirectory() { val file = File(tmpdir, "bugsnag-native").apply { mkdirs() } val newDirFile = File(tmpdir, "bugsnag/native") - BugsnagStoreMigrator.moveToNewDirectory(tmpdir) + BugsnagStoreMigrator.migrateLegacyFiles(lazyOf(tmpdir)) assertFalse(file.isDirectory) assertFalse(file.exists()) assertTrue(newDirFile.exists()) @@ -63,7 +63,7 @@ class BugsnagStoreMigratorTest { } assertTrue(file.isDirectory) assertTrue(file.exists()) - BugsnagStoreMigrator.moveToNewDirectory(tmpdir) + BugsnagStoreMigrator.migrateLegacyFiles(lazyOf(tmpdir)) assertFalse(newDirFile.isDirectory) assertFalse(newDirFile.exists()) assertTrue(file.isDirectory) @@ -74,7 +74,7 @@ class BugsnagStoreMigratorTest { fun testDeepPathUndefinedFile() { val file = File(tmpdir, "test/tes2/test3").apply { mkdirs() } val newDirFile = File(tmpdir, "bugsnag/test/tes2/test3") - BugsnagStoreMigrator.moveToNewDirectory(tmpdir) + BugsnagStoreMigrator.migrateLegacyFiles(lazyOf(tmpdir)) assertFalse(newDirFile.exists()) assertTrue(file.isDirectory) assertTrue(file.exists()) @@ -85,7 +85,7 @@ class BugsnagStoreMigratorTest { val file = File(tmpdir, "bugsnag-sessions").apply { mkdirs() } File(file, "test1/tes2/test3").apply { mkdirs() } val newDirFile = File(tmpdir, "bugsnag/sessions/test1/tes2/test3") - BugsnagStoreMigrator.moveToNewDirectory(tmpdir) + BugsnagStoreMigrator.migrateLegacyFiles(lazyOf(tmpdir)) assertTrue(newDirFile.exists()) assertFalse(file.isDirectory) assertFalse(file.exists()) @@ -102,7 +102,7 @@ class BugsnagStoreMigratorTest { val test1moved = File(tmpdir, "bugsnag/errors/test1") val test2moved = File(tmpdir, "bugsnag/errors/test2") - BugsnagStoreMigrator.moveToNewDirectory(tmpdir) + BugsnagStoreMigrator.migrateLegacyFiles(lazyOf(tmpdir)) assertFalse(file.isDirectory) assertFalse(file.exists()) assertFalse(test1From.exists()) diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DeviceIdStoreTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DeviceIdStoreTest.kt index 19c27b09fb..e2a06ef8b7 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DeviceIdStoreTest.kt +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DeviceIdStoreTest.kt @@ -68,12 +68,12 @@ internal class DeviceIdStoreTest { uuidProvider(0), nonExistentInternalFile, uuidProvider(1), - sharedPrefMigrator = ValueFuture(prefMigrator), + sharedPrefMigrator = ValueProvider(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) - val loaded = store.call() + val loaded = store.load() assertEquals(ids[0], loaded?.deviceId) assertEquals(ids[1], loaded?.internalDeviceId) @@ -94,12 +94,12 @@ internal class DeviceIdStoreTest { uuidProvider(0), fileInternal, uuidProvider(1), - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) - val loaded = store.call() + val loaded = store.load() assertEquals(ids[0], loaded?.deviceId) assertEquals(ids[1], loaded?.internalDeviceId) @@ -118,12 +118,12 @@ internal class DeviceIdStoreTest { uuidProvider(0), fileInternal, uuidProvider(1), - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) - val loaded = store.call() + val loaded = store.load() assertEquals(ids[0], loaded?.deviceId) assertEquals(ids[1], loaded?.internalDeviceId) @@ -142,7 +142,7 @@ internal class DeviceIdStoreTest { uuidProvider(2), fileInternal, uuidProvider(3), - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) @@ -152,13 +152,13 @@ internal class DeviceIdStoreTest { uuidProvider(0), fileInternal, uuidProvider(1), - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) - val loadedA = storeA.call() - val loadedB = storeB.call() + val loadedA = storeA.load() + val loadedB = storeB.load() // device ID is loaded without writing file assertEquals(ids[0], loadedA?.deviceId) @@ -190,12 +190,12 @@ internal class DeviceIdStoreTest { uuidProvider(0), nonReadableInternalFile, uuidProvider(1), - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) - val loaded = store.call() + val loaded = store.load() assertNull(loaded) } @@ -210,7 +210,7 @@ internal class DeviceIdStoreTest { uuidProvider(0), fileInternal, uuidProvider(1), - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) @@ -225,7 +225,7 @@ internal class DeviceIdStoreTest { repeat(attempts) { executor.submit { - store.call()?.deviceId?.let { deviceIds.add(it) } + store.load()?.deviceId?.let { deviceIds.add(it) } latch.countDown() } } @@ -247,7 +247,7 @@ internal class DeviceIdStoreTest { uuidProvider(0), fileInternal, uuidProvider(1), - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), logger = NoopLogger, config = generateConfig(true) ) @@ -257,11 +257,9 @@ internal class DeviceIdStoreTest { context.getSharedPreferences("com.bugsnag.android", Context.MODE_PRIVATE) prefs.edit().putString("install.iud", prefsId).commit() - val prefDeviceId = prefMigrator - .apply { call() } - .loadDeviceId(false) + val prefDeviceId = prefMigrator.loadDeviceId(false) - val loaded = store.call() + val loaded = store.load() val storeDeviceId = loaded?.deviceId assertEquals(prefsId, storeDeviceId) @@ -281,12 +279,12 @@ internal class DeviceIdStoreTest { uuidProvider(0), fileInternal, uuidProvider(1), - sharedPrefMigrator = ValueFuture(prefMigrator), + sharedPrefMigrator = ValueProvider(prefMigrator), logger = NoopLogger, config = generateConfig(false) ) - val loaded = store.call() + val loaded = store.load() assertNull(loaded) } } diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/UserStoreTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/UserStoreTest.kt index e70edd2527..74e774db67 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/UserStoreTest.kt +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/UserStoreTest.kt @@ -3,7 +3,6 @@ package com.bugsnag.android import android.content.Context import android.content.SharedPreferences import androidx.test.core.app.ApplicationProvider -import com.bugsnag.android.internal.ImmutableConfig import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse @@ -36,12 +35,6 @@ internal class UserStoreTest { prefs.edit().clear().commit() } - private fun generateConfig(persistUser: Boolean): ImmutableConfig { - val config = BugsnagTestUtils.generateConfiguration() - config.persistUser = persistUser - return BugsnagTestUtils.convert(config) - } - @Test fun sharedPrefMigration() { prefs.edit() @@ -51,13 +44,12 @@ internal class UserStoreTest { .putString("user.name", "Jane Fonda") .commit() - prefMigrator.call() - val store = UserStore( - generateConfig(true), - ValueFuture(DeviceIdStore.DeviceIds("0asdf", null)), + true, + ValueProvider(storageDir), + ValueProvider(DeviceIdStore.DeviceIds("0asdf", null)), file, - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), NoopLogger ) val user = store.load(User()).user @@ -74,10 +66,11 @@ internal class UserStoreTest { val nonExistentFile = File(storageDir, "foo") nonExistentFile.delete() val store = UserStore( - generateConfig(true), - ValueFuture(DeviceIdStore.DeviceIds("device-id", null)), + true, + ValueProvider(storageDir), + ValueProvider(DeviceIdStore.DeviceIds("device-id", null)), file, - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), NoopLogger ) val user = store.load(User()).user @@ -92,10 +85,11 @@ internal class UserStoreTest { @Test fun emptyFile() { val store = UserStore( - generateConfig(true), - ValueFuture(DeviceIdStore.DeviceIds("device-id", null)), + true, + ValueProvider(storageDir), + ValueProvider(DeviceIdStore.DeviceIds("device-id", null)), file, - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), NoopLogger ) val user = store.load(User()).user @@ -111,10 +105,11 @@ internal class UserStoreTest { fun invalidFileContents() { file.writeText("{\"hamster\": 2}") val store = UserStore( - generateConfig(true), - ValueFuture(DeviceIdStore.DeviceIds("device-id", null)), + true, + ValueProvider(storageDir), + ValueProvider(DeviceIdStore.DeviceIds("device-id", null)), file, - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), NoopLogger ) val user = store.load(User()).user @@ -134,10 +129,11 @@ internal class UserStoreTest { setWritable(false) } val store = UserStore( - generateConfig(true), - ValueFuture(DeviceIdStore.DeviceIds("device-id", null)), + true, + ValueProvider(storageDir), + ValueProvider(DeviceIdStore.DeviceIds("device-id", null)), nonReadableFile, - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), NoopLogger ) val user = store.load(User()).user @@ -154,10 +150,11 @@ internal class UserStoreTest { file.writeText("{\"id\":\"jf123\",\"email\":\"test@example.com\",\"name\":\"Jane Fonda\"}") val store = UserStore( - generateConfig(true), - ValueFuture(DeviceIdStore.DeviceIds("0asdf", null)), + true, + ValueProvider(storageDir), + ValueProvider(DeviceIdStore.DeviceIds("0asdf", null)), file, - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), NoopLogger ) val user = store.load(User()).user @@ -172,10 +169,11 @@ internal class UserStoreTest { @Test fun loadWithoutPersistUser() { val store = UserStore( - generateConfig(false), - ValueFuture(DeviceIdStore.DeviceIds("device-id-123", null)), + false, + ValueProvider(storageDir), + ValueProvider(DeviceIdStore.DeviceIds("device-id-123", null)), file, - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), NoopLogger ) store.load(User()).user @@ -188,10 +186,11 @@ internal class UserStoreTest { @Test fun saveWithoutPersistUser() { val store = UserStore( - generateConfig(false), - ValueFuture(DeviceIdStore.DeviceIds("", null)), + false, + ValueProvider(storageDir), + ValueProvider(DeviceIdStore.DeviceIds("", null)), file, - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), NoopLogger ) store.save(User("123", "joe@yahoo.com", "Joe Bloggs")) @@ -204,10 +203,11 @@ internal class UserStoreTest { @Test fun saveWithPersistUser() { val store = UserStore( - generateConfig(true), - ValueFuture(DeviceIdStore.DeviceIds("0asdf", null)), + true, + ValueProvider(storageDir), + ValueProvider(DeviceIdStore.DeviceIds("0asdf", null)), file, - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), NoopLogger ) val user = User("jf123", "test@example.com", "Jane Fonda") @@ -222,10 +222,11 @@ internal class UserStoreTest { @Test fun userRequiresChangeForDiskIO() { val store = UserStore( - generateConfig(true), - ValueFuture(DeviceIdStore.DeviceIds("0asdf", null)), + true, + ValueProvider(storageDir), + ValueProvider(DeviceIdStore.DeviceIds("0asdf", null)), file, - ValueFuture(prefMigrator), + ValueProvider(prefMigrator), NoopLogger ) val user = User("jf123", "test@example.com", "Jane Fonda") diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagStateModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagStateModule.kt index 2aa375226d..51d565a83d 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagStateModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagStateModule.kt @@ -10,7 +10,7 @@ import com.bugsnag.android.internal.dag.DependencyModule internal class BugsnagStateModule( cfg: ImmutableConfig, configuration: Configuration -) : DependencyModule() { +) : DependencyModule { val clientObservable = ClientObservable() diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java index d98c842289..09f2613802 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java @@ -3,7 +3,6 @@ import static com.bugsnag.android.SeverityReason.REASON_HANDLED_EXCEPTION; import com.bugsnag.android.internal.BackgroundTaskService; -import com.bugsnag.android.internal.BugsnagStoreMigrator; import com.bugsnag.android.internal.ForegroundDetector; import com.bugsnag.android.internal.ImmutableConfig; import com.bugsnag.android.internal.InternalMetrics; @@ -13,6 +12,7 @@ import com.bugsnag.android.internal.TaskType; import com.bugsnag.android.internal.dag.ConfigModule; import com.bugsnag.android.internal.dag.ContextModule; +import com.bugsnag.android.internal.dag.Provider; import com.bugsnag.android.internal.dag.SystemServiceModule; import android.app.Application; @@ -59,7 +59,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF private final InternalMetrics internalMetrics; private final ContextState contextState; private final CallbackState callbackState; - private final UserState userState; + private final Provider userState; private final Map configDifferences; final Context appContext; @@ -125,7 +125,7 @@ public Client(@NonNull Context androidContext, @NonNull String apiKey) { * @param configuration a configuration for the Client */ public Client(@NonNull Context androidContext, @NonNull final Configuration configuration) { - ContextModule contextModule = new ContextModule(androidContext); + ContextModule contextModule = new ContextModule(androidContext, bgTaskService); appContext = contextModule.getCtx(); notifier = configuration.getNotifier(); @@ -167,17 +167,13 @@ public Unit invoke(Boolean hasConnection, String networkState) { + "https://docs.bugsnag.com/platforms/android/#basic-configuration"); } - BugsnagStoreMigrator.moveToNewDirectory( - immutableConfig.getPersistenceDirectory().getValue() - ); - // setup storage as soon as possible final StorageModule storageModule = new StorageModule(appContext, immutableConfig, bgTaskService); // setup state trackers for bugsnag - BugsnagStateModule bugsnagStateModule = new BugsnagStateModule( - immutableConfig, configuration); + BugsnagStateModule bugsnagStateModule = + new BugsnagStateModule(immutableConfig, configuration); clientObservable = bugsnagStateModule.getClientObservable(); callbackState = bugsnagStateModule.getCallbackState(); breadcrumbState = bugsnagStateModule.getBreadcrumbState(); @@ -186,28 +182,26 @@ public Unit invoke(Boolean hasConnection, String networkState) { featureFlagState = bugsnagStateModule.getFeatureFlagState(); // lookup system services - final SystemServiceModule systemServiceModule = new SystemServiceModule(contextModule); + final SystemServiceModule systemServiceModule = + new SystemServiceModule(contextModule, bgTaskService); // setup further state trackers and data collection TrackerModule trackerModule = new TrackerModule(configModule, storageModule, this, bgTaskService, callbackState); - launchCrashTracker = trackerModule.getLaunchCrashTracker(); - sessionTracker = trackerModule.getSessionTracker(); DataCollectionModule dataCollectionModule = new DataCollectionModule(contextModule, configModule, systemServiceModule, trackerModule, bgTaskService, connectivity, storageModule.getDeviceIdStore(), memoryTrimState); - appDataCollector = dataCollectionModule.getAppDataCollector(); - deviceDataCollector = dataCollectionModule.getDeviceDataCollector(); // load the device + user information - userState = storageModule.getUserStore().load(configuration.getUser()); + userState = storageModule.loadUser(configuration.getUser()); EventStorageModule eventStorageModule = new EventStorageModule(contextModule, configModule, dataCollectionModule, bgTaskService, trackerModule, systemServiceModule, notifier, callbackState); - eventStore = eventStorageModule.getEventStore(); + + eventStore = eventStorageModule.getEventStore().get(); deliveryDelegate = new DeliveryDelegate(logger, eventStore, immutableConfig, callbackState, notifier, bgTaskService); @@ -215,8 +209,13 @@ public Unit invoke(Boolean hasConnection, String networkState) { exceptionHandler = new ExceptionHandler(this, logger); // load last run info - lastRunInfoStore = storageModule.getLastRunInfoStore(); - lastRunInfo = storageModule.getLastRunInfo(); + lastRunInfoStore = storageModule.getLastRunInfoStore().getOrNull(); + lastRunInfo = storageModule.getLastRunInfo().getOrNull(); + + launchCrashTracker = trackerModule.getLaunchCrashTracker(); + sessionTracker = trackerModule.getSessionTracker().get(); + appDataCollector = dataCollectionModule.getAppDataCollector().get(); + deviceDataCollector = dataCollectionModule.getDeviceDataCollector().get(); Set userPlugins = configuration.getPlugins(); pluginClient = new PluginClient(userPlugins, immutableConfig, logger); @@ -239,7 +238,7 @@ public Unit invoke(Boolean hasConnection, String networkState) { MetadataState metadataState, ContextState contextState, CallbackState callbackState, - UserState userState, + Provider userState, FeatureFlagState featureFlagState, ClientObservable clientObservable, Context appContext, @@ -446,7 +445,7 @@ void addObserver(StateObserver observer) { breadcrumbState.addObserver(observer); sessionTracker.addObserver(observer); clientObservable.addObserver(observer); - userState.addObserver(observer); + userState.get().addObserver(observer); contextState.addObserver(observer); deliveryDelegate.addObserver(observer); launchCrashTracker.addObserver(observer); @@ -459,7 +458,7 @@ void removeObserver(StateObserver observer) { breadcrumbState.removeObserver(observer); sessionTracker.removeObserver(observer); clientObservable.removeObserver(observer); - userState.removeObserver(observer); + userState.get().removeObserver(observer); contextState.removeObserver(observer); deliveryDelegate.removeObserver(observer); launchCrashTracker.removeObserver(observer); @@ -473,7 +472,7 @@ void removeObserver(StateObserver observer) { void syncInitialState() { metadataState.emitObservableEvent(); contextState.emitObservableEvent(); - userState.emitObservableEvent(); + userState.get().emitObservableEvent(); memoryTrimState.emitObservableEvent(); featureFlagState.emitObservableEvent(); } @@ -533,11 +532,10 @@ public void pauseSession() { * * stability score. * + * @return true if a previous session was resumed, false if a new session was started. * @see #startSession() * @see #pauseSession() * @see Configuration#setAutoTrackSessions(boolean) - * - * @return true if a previous session was resumed, false if a new session was started. */ public boolean resumeSession() { return sessionTracker.resumeSession(); @@ -546,18 +544,19 @@ public boolean resumeSession() { /** * Bugsnag uses the concept of "contexts" to help display and group your errors. Contexts * represent what was happening in your application at the time an error occurs. - * + *

* In an android app the "context" is automatically set as the foreground Activity. * If you would like to set this value manually, you should alter this property. */ - @Nullable public String getContext() { + @Nullable + public String getContext() { return contextState.getContext(); } /** * Bugsnag uses the concept of "contexts" to help display and group your errors. Contexts * represent what was happening in your application at the time an error occurs. - * + *

* In an android app the "context" is automatically set as the foreground Activity. * If you would like to set this value manually, you should alter this property. */ @@ -570,7 +569,7 @@ public void setContext(@Nullable String context) { */ @Override public void setUser(@Nullable String id, @Nullable String email, @Nullable String name) { - userState.setUser(new User(id, email, name)); + userState.get().setUser(new User(id, email, name)); } /** @@ -579,21 +578,21 @@ public void setUser(@Nullable String id, @Nullable String email, @Nullable Strin @NonNull @Override public User getUser() { - return userState.getUser(); + return userState.get().getUser(); } /** * Add a "on error" callback, to execute code at the point where an error report is * captured in Bugsnag. - * + *

* You can use this to add or modify information attached to an Event * before it is sent to your dashboard. You can also return * false from any callback to prevent delivery. "on error" * callbacks do not run before reports generated in the event * of immediate app termination from crashes in C/C++ code. - * + *

* For example: - * + *

* Bugsnag.addOnError(new OnErrorCallback() { * public boolean run(Event event) { * event.setSeverity(Severity.INFO); @@ -615,6 +614,7 @@ public void addOnError(@NonNull OnErrorCallback onError) { /** * Removes a previously added "on error" callback + * * @param onError the callback to remove */ @Override @@ -629,12 +629,12 @@ public void removeOnError(@NonNull OnErrorCallback onError) { /** * Add an "on breadcrumb" callback, to execute code before every * breadcrumb captured by Bugsnag. - * + *

* You can use this to modify breadcrumbs before they are stored by Bugsnag. * You can also return false from any callback to ignore a breadcrumb. - * + *

* For example: - * + *

* Bugsnag.onBreadcrumb(new OnBreadcrumbCallback() { * public boolean run(Breadcrumb breadcrumb) { * return false; // ignore the breadcrumb @@ -655,6 +655,7 @@ public void addOnBreadcrumb(@NonNull OnBreadcrumbCallback onBreadcrumb) { /** * Removes a previously added "on breadcrumb" callback + * * @param onBreadcrumb the callback to remove */ @Override @@ -669,12 +670,12 @@ public void removeOnBreadcrumb(@NonNull OnBreadcrumbCallback onBreadcrumb) { /** * Add an "on session" callback, to execute code before every * session captured by Bugsnag. - * + *

* You can use this to modify sessions before they are stored by Bugsnag. * You can also return false from any callback to ignore a session. - * + *

* For example: - * + *

* Bugsnag.onSession(new OnSessionCallback() { * public boolean run(Session session) { * return false; // ignore the session @@ -695,6 +696,7 @@ public void addOnSession(@NonNull OnSessionCallback onSession) { /** * Removes a previously added "on session" callback + * * @param onSession the callback to remove */ @Override @@ -718,9 +720,9 @@ public void notify(@NonNull Throwable exception) { /** * Notify Bugsnag of a handled exception * - * @param exc the exception to send to Bugsnag - * @param onError callback invoked on the generated error report for - * additional modification + * @param exc the exception to send to Bugsnag + * @param onError callback invoked on the generated error report for + * additional modification */ public void notify(@NonNull Throwable exc, @Nullable OnErrorCallback onError) { if (exc != null) { @@ -740,7 +742,7 @@ public void notify(@NonNull Throwable exc, @Nullable OnErrorCallback onError) { /** * Caches an error then attempts to notify. - * + *

* Should only ever be called from the {@link ExceptionHandler}. */ void notifyUnhandledException(@NonNull Throwable exc, Metadata metadata, @@ -783,7 +785,7 @@ void populateAndNotifyAndroidEvent(@NonNull Event event, event.setBreadcrumbs(breadcrumbState.copy()); // Attach user info to the event - User user = userState.getUser(); + User user = userState.get().getUser(); event.setUser(user.getId(), user.getEmail(), user.getName()); // Attach context to the event @@ -827,7 +829,7 @@ void notifyInternal(@NonNull Event event, * Returns the current buffer of breadcrumbs that will be sent with captured events. This * ordered list represents the most recent breadcrumbs to be captured up to the limit * set in {@link Configuration#getMaxBreadcrumbs()}. - * + *

* The returned collection is readonly and mutating the list will cause no effect on the * Client's state. If you wish to alter the breadcrumbs collected by the Client then you should * use {@link Configuration#setEnabledBreadcrumbTypes(Set)} and @@ -953,6 +955,7 @@ public void leaveBreadcrumb(@NonNull String message) { /** * Leave a "breadcrumb" log message representing an action or event which * occurred in your app, to aid with debugging + * * @param message A short label * @param metadata Additional diagnostic information about the app environment * @param type A category for the breadcrumb @@ -1059,7 +1062,7 @@ public void clearFeatureFlags() { /** * Retrieves information about the last launch of the application, if it has been run before. - * + *

* For example, this allows checking whether the app crashed on its last launch, which could * be used to perform conditional behaviour to recover from crashes, such as clearing the * app data cache. @@ -1073,7 +1076,7 @@ public LastRunInfo getLastRunInfo() { * Informs Bugsnag that the application has finished launching. Once this has been called * {@link AppWithState#isLaunching()} will always be false in any new error reports, * and synchronous delivery will not be attempted on the next launch for any fatal crashes. - * + *

* By default this method will be called after Bugsnag is initialized when * {@link Configuration#getLaunchDurationMillis()} has elapsed. Invoking this method manually * has precedence over the value supplied via the launchDurationMillis configuration option. diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt index 582a33f4c6..e8967de359 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/DataCollectionModule.kt @@ -2,14 +2,11 @@ package com.bugsnag.android import android.os.Environment import com.bugsnag.android.internal.BackgroundTaskService -import com.bugsnag.android.internal.TaskType +import com.bugsnag.android.internal.dag.BackgroundDependencyModule import com.bugsnag.android.internal.dag.ConfigModule import com.bugsnag.android.internal.dag.ContextModule -import com.bugsnag.android.internal.dag.DependencyModule +import com.bugsnag.android.internal.dag.Provider import com.bugsnag.android.internal.dag.SystemServiceModule -import java.util.concurrent.Callable -import java.util.concurrent.Future -import java.util.concurrent.RejectedExecutionException /** * A dependency module which constructs the objects that collect data in Bugsnag. For example, this @@ -22,9 +19,9 @@ internal class DataCollectionModule( trackerModule: TrackerModule, bgTaskService: BackgroundTaskService, connectivity: Connectivity, - deviceIdStore: Future, + deviceIdStore: Provider, memoryTrimState: MemoryTrimState -) : DependencyModule() { +) : BackgroundDependencyModule(bgTaskService) { private val ctx = contextModule.ctx private val cfg = configModule.config @@ -32,40 +29,34 @@ internal class DataCollectionModule( private val deviceBuildInfo: DeviceBuildInfo = DeviceBuildInfo.defaultInfo() private val dataDir = Environment.getDataDirectory() - val appDataCollector = + val appDataCollector = provider { AppDataCollector( ctx, ctx.packageManager, cfg, - trackerModule.sessionTracker, + trackerModule.sessionTracker.get(), systemServiceModule.activityManager, trackerModule.launchCrashTracker, memoryTrimState ) + } + + private val rootDetection = provider { + val rootDetector = RootDetector(logger = logger, deviceBuildInfo = deviceBuildInfo) + rootDetector.isRooted() + } - val deviceDataCollector = + val deviceDataCollector = provider { DeviceDataCollector( connectivity, ctx, ctx.resources, - deviceIdStore, + deviceIdStore.map { it.load() }, deviceBuildInfo, dataDir, - rootDetectionFuture(bgTaskService), + rootDetection, bgTaskService, logger ) - - private fun rootDetectionFuture(bgTaskService: BackgroundTaskService): Future? = try { - bgTaskService.submitTask( - TaskType.IO, - Callable { - val rootDetector = RootDetector(logger = logger, deviceBuildInfo = deviceBuildInfo) - rootDetector.isRooted() - } - ) - } catch (exc: RejectedExecutionException) { - logger.w("Failed to perform root detection checks", exc) - null } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt index 16fc655ac1..b818523837 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt @@ -13,6 +13,7 @@ import android.os.Build import android.provider.Settings import com.bugsnag.android.internal.BackgroundTaskService import com.bugsnag.android.internal.TaskType +import com.bugsnag.android.internal.dag.Provider import java.io.File import java.util.Date import java.util.Locale @@ -28,10 +29,10 @@ internal class DeviceDataCollector( private val connectivity: Connectivity, private val appContext: Context, resources: Resources, - private val deviceIdStore: Future, + private val deviceIdStore: Provider, private val buildInfo: DeviceBuildInfo, private val dataDirectory: File, - private val rootedFuture: Future?, + private val rootedFuture: Provider?, private val bgTaskService: BackgroundTaskService, private val logger: Logger ) { diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceIdStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceIdStore.kt index b86a9d02cd..a0a2cab06f 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceIdStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceIdStore.kt @@ -1,12 +1,10 @@ package com.bugsnag.android import android.content.Context -import com.bugsnag.android.DeviceIdStore.DeviceIds import com.bugsnag.android.internal.ImmutableConfig +import com.bugsnag.android.internal.dag.Provider import java.io.File import java.util.UUID -import java.util.concurrent.Callable -import java.util.concurrent.Future /** * This class is responsible for persisting and retrieving the device ID and internal device ID, @@ -18,16 +16,15 @@ internal class DeviceIdStore @JvmOverloads @Suppress("LongParameterList") constr private val deviceIdGenerator: () -> UUID = { UUID.randomUUID() }, private val internalDeviceIdFile: File = File(context.filesDir, "internal-device-id"), private val internalDeviceIdGenerator: () -> UUID = { UUID.randomUUID() }, - private val sharedPrefMigrator: Future, + private val sharedPrefMigrator: Provider, config: ImmutableConfig, private val logger: Logger -) : Callable { +) { private lateinit var persistence: DeviceIdPersistence private lateinit var internalPersistence: DeviceIdPersistence private val generateId = config.generateAnonymousId - - var deviceIds: DeviceIds? = null + private var deviceIds: DeviceIds? = null /** * Loads the device ID from @@ -65,7 +62,11 @@ internal class DeviceIdStore @JvmOverloads @Suppress("LongParameterList") constr return internalPersistence.loadDeviceId(true) } - override fun call(): DeviceIds? { + fun load(): DeviceIds? { + if (deviceIds != null) { + return deviceIds + } + persistence = DeviceIdFilePersistence(deviceIdFile, deviceIdGenerator, logger) internalPersistence = DeviceIdFilePersistence(internalDeviceIdFile, internalDeviceIdGenerator, logger) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStorageModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStorageModule.kt index 08def2960e..2d519eb5b1 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStorageModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStorageModule.kt @@ -1,9 +1,9 @@ package com.bugsnag.android import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.dag.BackgroundDependencyModule import com.bugsnag.android.internal.dag.ConfigModule import com.bugsnag.android.internal.dag.ContextModule -import com.bugsnag.android.internal.dag.DependencyModule import com.bugsnag.android.internal.dag.SystemServiceModule /** @@ -18,32 +18,32 @@ internal class EventStorageModule( systemServiceModule: SystemServiceModule, notifier: Notifier, callbackState: CallbackState -) : DependencyModule() { +) : BackgroundDependencyModule(bgTaskService) { private val cfg = configModule.config - private val delegate by lazy { + private val delegate = provider { if (cfg.telemetry.contains(Telemetry.INTERNAL_ERRORS)) InternalReportDelegate( contextModule.ctx, cfg.logger, cfg, systemServiceModule.storageManager, - dataCollectionModule.appDataCollector, + dataCollectionModule.appDataCollector.get(), dataCollectionModule.deviceDataCollector, - trackerModule.sessionTracker, + trackerModule.sessionTracker.get(), notifier, bgTaskService ) else null } - val eventStore by lazy { + val eventStore = provider { EventStore( cfg, cfg.logger, notifier, bgTaskService, - delegate, + delegate.getOrNull(), callbackState ) } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/InternalReportDelegate.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/InternalReportDelegate.java index a9d6a08d93..43edb9a5b5 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/InternalReportDelegate.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/InternalReportDelegate.java @@ -7,6 +7,7 @@ import com.bugsnag.android.internal.ImmutableConfig; import com.bugsnag.android.internal.JsonHelper; import com.bugsnag.android.internal.TaskType; +import com.bugsnag.android.internal.dag.Provider; import android.annotation.SuppressLint; import android.content.Context; @@ -33,7 +34,7 @@ class InternalReportDelegate implements EventStore.Delegate { final StorageManager storageManager; final AppDataCollector appDataCollector; - final DeviceDataCollector deviceDataCollector; + final Provider deviceDataCollector; final Context appContext; final SessionTracker sessionTracker; final Notifier notifier; @@ -44,7 +45,7 @@ class InternalReportDelegate implements EventStore.Delegate { ImmutableConfig immutableConfig, @Nullable StorageManager storageManager, AppDataCollector appDataCollector, - DeviceDataCollector deviceDataCollector, + Provider deviceDataCollector, SessionTracker sessionTracker, Notifier notifier, BackgroundTaskService backgroundTaskService) { @@ -102,7 +103,7 @@ void recordStorageCacheBehavior(Event event) { */ void reportInternalBugsnagError(@NonNull Event event) { event.setApp(appDataCollector.generateAppWithState()); - event.setDevice(deviceDataCollector.generateDeviceWithState(new Date().getTime())); + event.setDevice(deviceDataCollector.get().generateDeviceWithState(new Date().getTime())); event.addMetadata(INTERNAL_DIAGNOSTICS_TAB, "notifierName", notifier.getName()); event.addMetadata(INTERNAL_DIAGNOSTICS_TAB, "notifierVersion", notifier.getVersion()); diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/LastRunInfoStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/LastRunInfoStore.kt index 24d4a718a9..89f7c4516f 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/LastRunInfoStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/LastRunInfoStore.kt @@ -2,7 +2,6 @@ package com.bugsnag.android import com.bugsnag.android.internal.ImmutableConfig import java.io.File -import java.util.concurrent.Callable import java.util.concurrent.locks.ReentrantReadWriteLock import kotlin.concurrent.withLock @@ -15,24 +14,12 @@ private const val KEY_CRASHED_DURING_LAUNCH = "crashedDuringLaunch" * Persists/loads [LastRunInfo] on disk, which allows Bugsnag to determine * whether the previous application launch crashed or not. This class is thread-safe. */ -internal class LastRunInfoStore(config: ImmutableConfig) : Callable { +internal class LastRunInfoStore(config: ImmutableConfig) { val file: File = File(config.persistenceDirectory.value, "bugsnag/last-run-info") private val logger: Logger = config.logger private val lock = ReentrantReadWriteLock() - var lastRunInfo: LastRunInfo? = null - - override fun call(): LastRunInfoStore { - val info = load() - val currentRunInfo = LastRunInfo(0, crashed = false, crashedDuringLaunch = false) - persist(currentRunInfo) - - lastRunInfo = info - - return this - } - fun persist(lastRunInfo: LastRunInfo) { lock.writeLock().withLock { try { diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionFilenameInfo.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionFilenameInfo.kt index 3ea39917cd..6ce21ac750 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionFilenameInfo.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionFilenameInfo.kt @@ -1,6 +1,5 @@ package com.bugsnag.android -import com.bugsnag.android.internal.ImmutableConfig import java.io.File import java.util.UUID @@ -34,13 +33,10 @@ internal data class SessionFilenameInfo( } @JvmStatic - fun defaultFilename( - obj: Any?, - config: ImmutableConfig - ): SessionFilenameInfo { + fun defaultFilename(obj: Any?, apiKey: String): SessionFilenameInfo { val sanitizedApiKey = when (obj) { is Session -> obj.apiKey - else -> config.apiKey + else -> apiKey } return SessionFilenameInfo( diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionStore.kt index 094de50547..e6e0ce6aa8 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionStore.kt @@ -2,7 +2,6 @@ package com.bugsnag.android import com.bugsnag.android.SessionFilenameInfo.Companion.defaultFilename import com.bugsnag.android.SessionFilenameInfo.Companion.findTimestampInFilename -import com.bugsnag.android.internal.ImmutableConfig import java.io.File import java.util.Calendar import java.util.Comparator @@ -13,14 +12,14 @@ import java.util.Date * lack of network connectivity. */ internal class SessionStore( - private val config: ImmutableConfig, + bugsnagDir: File, + maxPersistedSessions: Int, + private val apiKey: String, logger: Logger, delegate: Delegate? ) : FileStore( - File( - config.persistenceDirectory.value, "bugsnag/sessions" - ), - config.maxPersistedSessions, + File(bugsnagDir, "sessions"), + maxPersistedSessions, logger, delegate ) { @@ -52,7 +51,7 @@ internal class SessionStore( } override fun getFilename(obj: Any?): String { - val sessionInfo = defaultFilename(obj, config) + val sessionInfo = defaultFilename(obj, apiKey) return sessionInfo.encode() } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/SharedPrefMigrator.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/SharedPrefMigrator.kt index d5365446cb..2f7ccc238f 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/SharedPrefMigrator.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/SharedPrefMigrator.kt @@ -2,61 +2,45 @@ package com.bugsnag.android import android.annotation.SuppressLint import android.content.Context -import android.os.Build -import java.util.concurrent.Callable +import android.content.SharedPreferences /** * Reads legacy information left in SharedPreferences and migrates it to the new location. */ -internal class SharedPrefMigrator(private val context: Context) : - DeviceIdPersistence, - Callable { +internal class SharedPrefMigrator(context: Context) : DeviceIdPersistence { - private var installId: String? = null - private var userId: String? = null - private var userEmail: String? = null - private var userName: String? = null - - override fun call(): SharedPrefMigrator { + private val prefs: SharedPreferences? = try { - val prefs = context.getSharedPreferences(SHARED_PREFS_NAME, Context.MODE_PRIVATE) - ?: return this - - installId = prefs.getString(INSTALL_ID_KEY, null) - userId = prefs.getString(USER_ID_KEY, null) - userEmail = prefs.getString(USER_EMAIL_KEY, null) - userName = prefs.getString(USER_NAME_KEY, null) - - @SuppressLint("ApplySharedPref") - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { - context.deleteSharedPreferences(SHARED_PREFS_NAME) - } else { - prefs.edit().clear().commit() - } - } catch (_: RuntimeException) { + context.getSharedPreferences("com.bugsnag.android", Context.MODE_PRIVATE) + } catch (e: RuntimeException) { + null } - return this - } - /** * This implementation will never create an ID; it will only fetch one if present. */ - override fun loadDeviceId(requestCreateIfDoesNotExist: Boolean) = installId + override fun loadDeviceId(requestCreateIfDoesNotExist: Boolean) = + prefs?.getString(INSTALL_ID_KEY, null) fun loadUser(deviceId: String?) = User( - userId ?: deviceId, - userEmail, - userName + prefs?.getString(USER_ID_KEY, deviceId), + prefs?.getString(USER_EMAIL_KEY, null), + prefs?.getString(USER_NAME_KEY, null) ) - fun hasPrefs() = installId != null + fun hasPrefs() = prefs?.contains(INSTALL_ID_KEY) == true + + @SuppressLint("ApplySharedPref") + fun deleteLegacyPrefs() { + if (hasPrefs()) { + prefs?.edit()?.clear()?.commit() + } + } companion object { private const val INSTALL_ID_KEY = "install.iud" private const val USER_ID_KEY = "user.id" private const val USER_NAME_KEY = "user.name" private const val USER_EMAIL_KEY = "user.email" - private const val SHARED_PREFS_NAME = "com.bugsnag.android" } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/StorageModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/StorageModule.kt index 0ca701141f..46a07da023 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/StorageModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/StorageModule.kt @@ -2,56 +2,72 @@ package com.bugsnag.android import android.content.Context import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.BugsnagStoreMigrator.migrateLegacyFiles import com.bugsnag.android.internal.ImmutableConfig import com.bugsnag.android.internal.TaskType -import com.bugsnag.android.internal.dag.DependencyModule +import com.bugsnag.android.internal.dag.BackgroundDependencyModule +import com.bugsnag.android.internal.dag.Provider /** * A dependency module which constructs the objects that store information to disk in Bugsnag. */ internal class StorageModule( appContext: Context, - immutableConfig: ImmutableConfig, + private val immutableConfig: ImmutableConfig, bgTaskService: BackgroundTaskService -) : DependencyModule() { +) : BackgroundDependencyModule(bgTaskService, TaskType.IO) { - val sharedPrefMigrator = bgTaskService.submitTask( - TaskType.IO, + val bugsnagDir = provider { + migrateLegacyFiles(immutableConfig.persistenceDirectory) + } + + val sharedPrefMigrator = provider { SharedPrefMigrator(appContext) - ) + } - val deviceIdStore = bgTaskService.submitTask( - TaskType.IO, + val deviceIdStore = provider { DeviceIdStore( appContext, sharedPrefMigrator = sharedPrefMigrator, logger = immutableConfig.logger, config = immutableConfig ) - ) + } - val userStore = UserStore( - immutableConfig, - deviceIdStore, - sharedPrefMigrator = sharedPrefMigrator, - logger = immutableConfig.logger - ) + val userStore = provider { + UserStore( + immutableConfig.persistUser, + bugsnagDir, + deviceIdStore.map { it.load() }, + sharedPrefMigrator = sharedPrefMigrator, + logger = immutableConfig.logger + ) + } - val lastRunInfoStore = LastRunInfoStore(immutableConfig) + val lastRunInfoStore = provider { + LastRunInfoStore(immutableConfig) + } - val sessionStore = + val sessionStore = provider { SessionStore( - immutableConfig, + bugsnagDir.get(), + immutableConfig.maxPersistedSessions, + immutableConfig.apiKey, immutableConfig.logger, null ) + } - val lastRunInfo = lastRunInfo() - - private fun lastRunInfo(): LastRunInfo? { + val lastRunInfo = lastRunInfoStore.map { lastRunInfoStore -> val info = lastRunInfoStore.load() val currentRunInfo = LastRunInfo(0, crashed = false, crashedDuringLaunch = false) lastRunInfoStore.persist(currentRunInfo) - return info + return@map info + } + + fun loadUser(initialUser: User): Provider = provider { + val userState = userStore.get().load(initialUser) + sharedPrefMigrator.getOrNull()?.deleteLegacyPrefs() + return@provider userState } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/TrackerModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/TrackerModule.kt index ae1db31f2a..8eb6c47607 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/TrackerModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/TrackerModule.kt @@ -1,8 +1,8 @@ package com.bugsnag.android import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.dag.BackgroundDependencyModule import com.bugsnag.android.internal.dag.ConfigModule -import com.bugsnag.android.internal.dag.DependencyModule /** * A dependency module which constructs objects that track launch/session related information @@ -14,18 +14,21 @@ internal class TrackerModule( client: Client, bgTaskService: BackgroundTaskService, callbackState: CallbackState -) : DependencyModule() { +) : BackgroundDependencyModule(bgTaskService) { private val config = configModule.config val launchCrashTracker = LaunchCrashTracker(config) - val sessionTracker = SessionTracker( - config, - callbackState, - client, - storageModule.sessionStore, - config.logger, - bgTaskService - ) + val sessionTracker = provider { + client.config + SessionTracker( + config, + callbackState, + client, + storageModule.sessionStore.get(), + config.logger, + bgTaskService + ) + } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/UserStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/UserStore.kt index 13701d07a2..58bf10d7ed 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/UserStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/UserStore.kt @@ -1,24 +1,23 @@ package com.bugsnag.android -import com.bugsnag.android.internal.ImmutableConfig import com.bugsnag.android.internal.StateObserver +import com.bugsnag.android.internal.dag.Provider import java.io.File -import java.util.concurrent.Future import java.util.concurrent.atomic.AtomicReference /** * This class is responsible for persisting and retrieving user information. */ -internal class UserStore @JvmOverloads constructor( - private val config: ImmutableConfig, - private val deviceIdStore: Future, - file: File = File(config.persistenceDirectory.value, "bugsnag/user-info"), - private val sharedPrefMigrator: Future, +internal class UserStore( + private val persist: Boolean, + private val persistentDir: Provider, + private val deviceIdStore: Provider, + file: File = File(persistentDir.get(), "user-info"), + private val sharedPrefMigrator: Provider, private val logger: Logger ) { private val synchronizedStreamableStore: SynchronizedStreamableStore - private val persist = config.persistUser private val previousUser = AtomicReference(null) init { diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/BackgroundTaskService.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/BackgroundTaskService.kt index ef3394fd52..738b2f7922 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/BackgroundTaskService.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/BackgroundTaskService.kt @@ -1,6 +1,7 @@ package com.bugsnag.android.internal import androidx.annotation.VisibleForTesting +import com.bugsnag.android.internal.dag.RunnableProvider import java.util.concurrent.BlockingQueue import java.util.concurrent.Callable import java.util.concurrent.ExecutorService @@ -152,7 +153,11 @@ class BackgroundTaskService( @Throws(RejectedExecutionException::class) fun submitTask(taskType: TaskType, callable: Callable): Future { val task = FutureTask(callable) + execute(taskType, task) + return SafeFuture(task, taskType) + } + fun execute(taskType: TaskType, task: Runnable) { when (taskType) { TaskType.ERROR_REQUEST -> errorExecutor.execute(task) TaskType.SESSION_REQUEST -> sessionExecutor.execute(task) @@ -160,8 +165,6 @@ class BackgroundTaskService( TaskType.INTERNAL_REPORT -> internalReportExecutor.execute(task) TaskType.DEFAULT -> defaultExecutor.execute(task) } - - return SafeFuture(task, taskType) } /** @@ -185,6 +188,18 @@ class BackgroundTaskService( ioExecutor.awaitTerminationSafe() } + inline fun provider( + taskType: TaskType, + crossinline provider: () -> R + ): RunnableProvider { + val task = object : RunnableProvider() { + override fun invoke(): R = provider() + } + + execute(taskType, task) + return task + } + private fun ExecutorService.awaitTerminationSafe() { try { awaitTermination(SHUTDOWN_WAIT_MS, TimeUnit.MILLISECONDS) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/BugsnagStoreMigrator.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/BugsnagStoreMigrator.kt index 87b223c511..7affbf4fca 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/BugsnagStoreMigrator.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/BugsnagStoreMigrator.kt @@ -5,8 +5,9 @@ import java.io.File internal object BugsnagStoreMigrator { @JvmStatic - fun moveToNewDirectory(persistenceDir: File) { - val bugsnagDir = File(persistenceDir, "bugsnag") + fun migrateLegacyFiles(persistenceDir: Lazy): File { + val originalDir = persistenceDir.value + val bugsnagDir = File(originalDir, "bugsnag") if (!bugsnagDir.isDirectory) { bugsnagDir.mkdirs() } @@ -19,12 +20,12 @@ internal object BugsnagStoreMigrator { ) filesToMove.forEach { (from, to) -> - val fromFile = File(persistenceDir, from) + val fromFile = File(originalDir, from) if (fromFile.exists()) { - fromFile.renameTo( - File(bugsnagDir, to) - ) + fromFile.renameTo(File(bugsnagDir, to)) } } + + return bugsnagDir } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/ConfigModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/ConfigModule.kt index 2d55c808be..aece93e476 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/ConfigModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/ConfigModule.kt @@ -14,6 +14,6 @@ internal class ConfigModule( configuration: Configuration, connectivity: Connectivity, bgTaskExecutor: BackgroundTaskService -) : DependencyModule() { +) : BackgroundDependencyModule(bgTaskExecutor) { val config = sanitiseConfiguration(contextModule.ctx, configuration, connectivity, bgTaskExecutor) } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/ContextModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/ContextModule.kt index 9ece4ff604..0d36ce64bf 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/ContextModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/ContextModule.kt @@ -1,14 +1,16 @@ package com.bugsnag.android.internal.dag import android.content.Context +import com.bugsnag.android.internal.BackgroundTaskService /** * A dependency module which accesses the application context object, falling back to the supplied * context if it is the base context. */ internal class ContextModule( - appContext: Context -) : DependencyModule() { + appContext: Context, + bgTaskService: BackgroundTaskService +) : BackgroundDependencyModule(bgTaskService) { val ctx: Context = when (appContext.applicationContext) { null -> appContext diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt index b49a0fedd7..51e3ff2725 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt @@ -1,3 +1,26 @@ package com.bugsnag.android.internal.dag -internal abstract class DependencyModule +import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.TaskType + +internal interface DependencyModule + +internal abstract class BackgroundDependencyModule( + @JvmField + val bgTaskService: BackgroundTaskService, + @JvmField + val taskType: TaskType = TaskType.DEFAULT +) : DependencyModule { + inline fun provider(crossinline provider: () -> R): RunnableProvider { + return bgTaskService.provider(taskType, provider) + } + + internal inline fun Provider.map(crossinline mapping: (E) -> R): RunnableProvider { + val task = object : RunnableProvider() { + override fun invoke(): R = mapping(this@map.get()) + } + + bgTaskService.execute(taskType, task) + return task + } +} diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/Provider.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/Provider.kt new file mode 100644 index 0000000000..89d0119b69 --- /dev/null +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/Provider.kt @@ -0,0 +1,114 @@ +package com.bugsnag.android.internal.dag + +import android.os.Looper +import java.util.concurrent.atomic.AtomicInteger + +/** + * A lightweight abstraction similar to `Lazy` or `Future` allowing values to be calculated on + * separate threads, or to be pre-computed. + */ +interface Provider { + /** + * Same as [get] but will return `null` instead of throwing an exception if the value could + * not be computed. + */ + fun getOrNull(): E? + + /** + * Return the value sourced from this provider, throwing an exception if the provider failed + * to calculate a value. Anything thrown from here will have been captured when attempting + * to calculate the value. + */ + fun get(): E +} + +/** + * The primary implementation of [Provider], usually created using the + * [BackgroundDependencyModule.provider] function. Similar conceptually to + * [java.util.concurrent.FutureTask] but with a more compact implementation. The implementation + * of [RunnableProvider.get] is special because it behaves more like [Lazy.value] in that getting + * a value that is still pending will cause it to be run on the current thread instead of waiting + * for it to be run "sometime in the future". This makes RunnableProviders less bug-prone when + * dealing with single-thread executors (such as those in [BackgroundTaskService]). RunnableProvider + * also has special handling for the main-thread, ensuring no computational work (such as IO) is + * done on the main thread. + */ +abstract class RunnableProvider : Provider, Runnable { + private val state = AtomicInteger(TASK_STATE_PENDING) + + @Volatile + private var value: Any? = null + + abstract operator fun invoke(): E + + override fun getOrNull(): E? { + return getOr { return null } + } + + override fun get(): E { + return getOr { throw value as Throwable } + } + + private inline fun getOr(failureHandler: () -> E): E { + while (true) { + when (state.get()) { + TASK_STATE_RUNNING -> awaitResult() + TASK_STATE_PENDING -> { + if (isMainThread()) { + awaitResult() + } else { + run() + } + } + + TASK_STATE_COMPLETE -> @Suppress("UNCHECKED_CAST") return value as E + TASK_STATE_FAILED -> failureHandler() + } + } + } + + private fun isMainThread(): Boolean { + return Thread.currentThread() === mainThread + } + + private fun awaitResult() { + synchronized(this) { + while (!isComplete()) { + @Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN") + (this as Object).wait() + } + } + } + + private fun isComplete() = when (state.get()) { + TASK_STATE_PENDING, TASK_STATE_RUNNING -> false + else -> true + } + + final override fun run() { + if (state.compareAndSet(TASK_STATE_PENDING, TASK_STATE_RUNNING)) { + try { + value = invoke() + state.set(TASK_STATE_COMPLETE) + } catch (ex: Throwable) { + value = ex + state.set(TASK_STATE_FAILED) + } finally { + synchronized(this) { + // wakeup any waiting threads + @Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN") + (this as Object).notifyAll() + } + } + } + } + + private companion object { + private const val TASK_STATE_PENDING = 0 + private const val TASK_STATE_RUNNING = 1 + private const val TASK_STATE_COMPLETE = 2 + private const val TASK_STATE_FAILED = 999 + + private val mainThread = Looper.getMainLooper().thread + } +} diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/SystemServiceModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/SystemServiceModule.kt index eef01cbc63..1c7cd6cf02 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/SystemServiceModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/SystemServiceModule.kt @@ -2,13 +2,15 @@ package com.bugsnag.android.internal.dag import com.bugsnag.android.getActivityManager import com.bugsnag.android.getStorageManager +import com.bugsnag.android.internal.BackgroundTaskService /** * A dependency module which provides a reference to Android system services. */ internal class SystemServiceModule( - contextModule: ContextModule -) : DependencyModule() { + contextModule: ContextModule, + bgTaskService: BackgroundTaskService +) : BackgroundDependencyModule(bgTaskService) { val storageManager = contextModule.ctx.getStorageManager() val activityManager = contextModule.ctx.getActivityManager() diff --git a/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueFuture.kt b/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueFuture.kt deleted file mode 100644 index 9d47936cf6..0000000000 --- a/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueFuture.kt +++ /dev/null @@ -1,12 +0,0 @@ -package com.bugsnag.android - -import java.util.concurrent.Future -import java.util.concurrent.TimeUnit - -class ValueFuture(private val value: V) : Future { - override fun cancel(mayInterruptIfRunning: Boolean): Boolean = false - override fun isCancelled(): Boolean = false - override fun isDone(): Boolean = true - override fun get(): V = value - override fun get(timeout: Long, unit: TimeUnit?): V = get() -} diff --git a/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueProvider.kt b/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueProvider.kt new file mode 100644 index 0000000000..42a62e46ee --- /dev/null +++ b/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueProvider.kt @@ -0,0 +1,8 @@ +package com.bugsnag.android + +import com.bugsnag.android.internal.dag.Provider + +class ValueProvider(private val value: E) : Provider { + override fun getOrNull(): E? = value + override fun get(): E = value +} diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java b/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java index ca13a2d103..a63a5b7fc3 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java @@ -116,7 +116,7 @@ public void setUp() { metadataState, contextState, callbackState, - userState, + new ValueProvider(userState), featureFlagState, clientObservable, appContext, diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt index fafb9dd42a..8ba751f42d 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt @@ -51,10 +51,10 @@ internal class ConfigChangeReceiverTest { connectivity, appContext, resources, - ValueFuture(null), + ValueProvider(null), buildInfo, dataDirectory, - ValueFuture(false), + ValueProvider(false), bgTaskService, NoopLogger ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DataCollectorTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DataCollectorTest.kt index 357c160882..b36bc41914 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DataCollectorTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/DataCollectorTest.kt @@ -35,10 +35,10 @@ internal class DataCollectorTest { Mockito.mock(Connectivity::class.java), Mockito.mock(Context::class.java), res, - ValueFuture(DeviceIdStore.DeviceIds("fakeDevice", "internalFakeDevice")), + ValueProvider(DeviceIdStore.DeviceIds("fakeDevice", "internalFakeDevice")), Mockito.mock(DeviceBuildInfo::class.java), File("/tmp/javatest"), - ValueFuture(false), + ValueProvider(false), Mockito.mock(BackgroundTaskService::class.java), Mockito.mock(Logger::class.java) ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt index da2c7b5c8b..60ccc80da7 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt @@ -53,10 +53,10 @@ internal class DeviceDataCollectorSerializationTest { connectivity, context, res, - ValueFuture(DeviceIdStore.DeviceIds("123", "456")), + ValueProvider(DeviceIdStore.DeviceIds("123", "456")), buildInfo, File(""), - ValueFuture(false), + ValueProvider(false), BackgroundTaskService(), NoopLogger ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt index 2a213c6850..b5549098fc 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt @@ -53,10 +53,10 @@ internal class DeviceMetadataSerializationTest { connectivity, context, res, - ValueFuture(DeviceIdStore.DeviceIds("123", "456")), + ValueProvider(DeviceIdStore.DeviceIds("123", "456")), buildInfo, File(""), - ValueFuture(false), + ValueProvider(false), BackgroundTaskService(), NoopLogger ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/InternalEventPayloadDelegateTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/InternalEventPayloadDelegateTest.kt index d7f7724789..6ced18091f 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/InternalEventPayloadDelegateTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/InternalEventPayloadDelegateTest.kt @@ -53,7 +53,7 @@ internal class InternalEventPayloadDelegateTest { config, storageManager, appDataCollector, - deviceDataCollector, + ValueProvider(deviceDataCollector), sessionTracker, Notifier(), BackgroundTaskService() diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionStoreMaxLimitTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionStoreMaxLimitTest.kt index 1593a85776..1a3e4c6f24 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionStoreMaxLimitTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/SessionStoreMaxLimitTest.kt @@ -79,7 +79,9 @@ class SessionStoreMaxLimitTest { private fun createSessionStore(config: ImmutableConfig): SessionStore { return SessionStore( - config, + File(config.persistenceDirectory.value, "bugsnag"), + config.maxPersistedSessions, + config.apiKey, NoopLogger, object : Delegate { override fun onErrorIOFailure( diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/SharedPrefMigratorTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/SharedPrefMigratorTest.kt index 16716a564f..21a889aa74 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/SharedPrefMigratorTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/SharedPrefMigratorTest.kt @@ -9,10 +9,11 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith -import org.mockito.ArgumentMatchers.any import org.mockito.ArgumentMatchers.anyInt import org.mockito.ArgumentMatchers.eq import org.mockito.Mock +import org.mockito.Mockito.times +import org.mockito.Mockito.verify import org.mockito.Mockito.`when` import org.mockito.junit.MockitoJUnitRunner @@ -39,37 +40,34 @@ internal class SharedPrefMigratorTest { @Test fun nullSharedPreferences() { `when`(context.getSharedPreferences(eq("com.bugsnag.android"), anyInt())).thenReturn(null) - prefMigrator.call() + prefMigrator = SharedPrefMigrator(context) assertFalse(prefMigrator.hasPrefs()) } @Test fun gettingSharedPreferencesWithException() { `when`(context.getSharedPreferences(eq("com.bugsnag.android"), anyInt())).thenThrow(RuntimeException()) - prefMigrator.call() + prefMigrator = SharedPrefMigrator(context) assertFalse(prefMigrator.hasPrefs()) } @Test fun nullDeviceId() { `when`(prefs.getString("install.iud", null)).thenReturn(null) - prefMigrator.call() assertNull(prefMigrator.loadDeviceId(true)) } @Test fun validDeviceId() { `when`(prefs.getString("install.iud", null)).thenReturn("f09asdfb") - prefMigrator.call() assertEquals("f09asdfb", prefMigrator.loadDeviceId(true)) } @Test fun emptyUser() { - `when`(prefs.getString(eq("user.id"), any())).thenReturn("f09asdfb") + `when`(prefs.getString("user.id", "f09asdfb")).thenReturn("f09asdfb") `when`(prefs.getString("user.email", null)).thenReturn(null) `when`(prefs.getString("user.name", null)).thenReturn(null) - prefMigrator.call() val observed = prefMigrator.loadUser("f09asdfb") assertEquals("f09asdfb", observed.id) @@ -79,10 +77,9 @@ internal class SharedPrefMigratorTest { @Test fun populatedUser() { - `when`(prefs.getString(eq("user.id"), any())).thenReturn("abc75092") + `when`(prefs.getString("user.id", "f09asdfb")).thenReturn("abc75092") `when`(prefs.getString("user.email", null)).thenReturn("test@example.com") `when`(prefs.getString("user.name", null)).thenReturn("Joe") - prefMigrator.call() val expected = User("abc75092", "test@example.com", "Joe") val observed = prefMigrator.loadUser("f09asdfb") @@ -91,17 +88,25 @@ internal class SharedPrefMigratorTest { assertEquals(expected.name, observed.name) } + @Test + fun deletePrefs() { + `when`(prefs.contains("install.iud")).thenReturn(true) + `when`(prefs.edit()).thenReturn(editor) + `when`(editor.clear()).thenReturn(editor) + prefMigrator.deleteLegacyPrefs() + verify(editor, times(1)).clear() + verify(editor, times(1)).commit() + } + @Test fun hasPrefsTrue() { - `when`(prefs.getString("install.iud", null)).thenReturn("abc123") - prefMigrator.call() + `when`(prefs.contains("install.iud")).thenReturn(true) assertTrue(prefMigrator.hasPrefs()) } @Test fun hasPrefsFalse() { - `when`(prefs.getString("install.iud", null)).thenReturn(null) - prefMigrator.call() + `when`(prefs.contains("install.iud")).thenReturn(false) assertFalse(prefMigrator.hasPrefs()) } } diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/internal/BackgroundTaskServiceTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/internal/BackgroundRunnableProviderServiceTest.kt similarity index 99% rename from bugsnag-android-core/src/test/java/com/bugsnag/android/internal/BackgroundTaskServiceTest.kt rename to bugsnag-android-core/src/test/java/com/bugsnag/android/internal/BackgroundRunnableProviderServiceTest.kt index b06daee347..a258ae33dd 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/internal/BackgroundTaskServiceTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/internal/BackgroundRunnableProviderServiceTest.kt @@ -12,7 +12,7 @@ import java.util.concurrent.TimeUnit private const val WAIT_TIME_MS = 200L private const val CONFINEMENT_TEST_ATTEMPTS = 20 -internal class BackgroundTaskServiceTest { +internal class BackgroundRunnableProviderServiceTest { /** * Verifies that the task type submits a Runnable to the correct executor. From d84cb83e7ff2075a8c8c5986c3e7116822659a70 Mon Sep 17 00:00:00 2001 From: jason Date: Fri, 6 Sep 2024 15:08:05 +0100 Subject: [PATCH 3/9] chore(provider): addition docs and comments based on review discussion --- .../main/java/com/bugsnag/android/Client.java | 30 ++++++------- .../android/internal/dag/DependencyModule.kt | 13 ++++++ .../bugsnag/android/internal/dag/Provider.kt | 44 +++++++++++++++++++ 3 files changed, 72 insertions(+), 15 deletions(-) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java index 09f2613802..cea27385ac 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java @@ -544,7 +544,7 @@ public boolean resumeSession() { /** * Bugsnag uses the concept of "contexts" to help display and group your errors. Contexts * represent what was happening in your application at the time an error occurs. - *

+ * * In an android app the "context" is automatically set as the foreground Activity. * If you would like to set this value manually, you should alter this property. */ @@ -556,7 +556,7 @@ public String getContext() { /** * Bugsnag uses the concept of "contexts" to help display and group your errors. Contexts * represent what was happening in your application at the time an error occurs. - *

+ * * In an android app the "context" is automatically set as the foreground Activity. * If you would like to set this value manually, you should alter this property. */ @@ -584,15 +584,15 @@ public User getUser() { /** * Add a "on error" callback, to execute code at the point where an error report is * captured in Bugsnag. - *

+ * * You can use this to add or modify information attached to an Event * before it is sent to your dashboard. You can also return * false from any callback to prevent delivery. "on error" * callbacks do not run before reports generated in the event * of immediate app termination from crashes in C/C++ code. - *

+ * * For example: - *

+ * * Bugsnag.addOnError(new OnErrorCallback() { * public boolean run(Event event) { * event.setSeverity(Severity.INFO); @@ -629,12 +629,12 @@ public void removeOnError(@NonNull OnErrorCallback onError) { /** * Add an "on breadcrumb" callback, to execute code before every * breadcrumb captured by Bugsnag. - *

+ * * You can use this to modify breadcrumbs before they are stored by Bugsnag. * You can also return false from any callback to ignore a breadcrumb. - *

+ * * For example: - *

+ * * Bugsnag.onBreadcrumb(new OnBreadcrumbCallback() { * public boolean run(Breadcrumb breadcrumb) { * return false; // ignore the breadcrumb @@ -670,12 +670,12 @@ public void removeOnBreadcrumb(@NonNull OnBreadcrumbCallback onBreadcrumb) { /** * Add an "on session" callback, to execute code before every * session captured by Bugsnag. - *

+ * * You can use this to modify sessions before they are stored by Bugsnag. * You can also return false from any callback to ignore a session. - *

+ * * For example: - *

+ * * Bugsnag.onSession(new OnSessionCallback() { * public boolean run(Session session) { * return false; // ignore the session @@ -742,7 +742,7 @@ public void notify(@NonNull Throwable exc, @Nullable OnErrorCallback onError) { /** * Caches an error then attempts to notify. - *

+ * * Should only ever be called from the {@link ExceptionHandler}. */ void notifyUnhandledException(@NonNull Throwable exc, Metadata metadata, @@ -829,7 +829,7 @@ void notifyInternal(@NonNull Event event, * Returns the current buffer of breadcrumbs that will be sent with captured events. This * ordered list represents the most recent breadcrumbs to be captured up to the limit * set in {@link Configuration#getMaxBreadcrumbs()}. - *

+ * * The returned collection is readonly and mutating the list will cause no effect on the * Client's state. If you wish to alter the breadcrumbs collected by the Client then you should * use {@link Configuration#setEnabledBreadcrumbTypes(Set)} and @@ -1062,7 +1062,7 @@ public void clearFeatureFlags() { /** * Retrieves information about the last launch of the application, if it has been run before. - *

+ * * For example, this allows checking whether the app crashed on its last launch, which could * be used to perform conditional behaviour to recover from crashes, such as clearing the * app data cache. @@ -1076,7 +1076,7 @@ public LastRunInfo getLastRunInfo() { * Informs Bugsnag that the application has finished launching. Once this has been called * {@link AppWithState#isLaunching()} will always be false in any new error reports, * and synchronous delivery will not be attempted on the next launch for any fatal crashes. - *

+ * * By default this method will be called after Bugsnag is initialized when * {@link Configuration#getLaunchDurationMillis()} has elapsed. Invoking this method manually * has precedence over the value supplied via the launchDurationMillis configuration option. diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt index 51e3ff2725..a44ca147f4 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt @@ -11,10 +11,23 @@ internal abstract class BackgroundDependencyModule( @JvmField val taskType: TaskType = TaskType.DEFAULT ) : DependencyModule { + /** + * Convenience function to create and schedule a `RunnableProvider` of [taskType] with + * [bgTaskService]. The returned `RunnableProvider` will be implemented using the `provider` + * lambda as its `invoke` implementation. + */ inline fun provider(crossinline provider: () -> R): RunnableProvider { return bgTaskService.provider(taskType, provider) } + /** + * Return a `RunnableProvider` containing the result of applying the given [mapping] to + * this `Provider`. The `RunnableProvider` will be scheduled with [bgTaskService] as a + * [taskType] when this function returns. + * + * This function behaves similar to `List.map` or `Any.let` but with `Provider` encapsulation + * to handle value reuse and threading. + */ internal inline fun Provider.map(crossinline mapping: (E) -> R): RunnableProvider { val task = object : RunnableProvider() { override fun invoke(): R = mapping(this@map.get()) diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/Provider.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/Provider.kt index 89d0119b69..c021663305 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/Provider.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/Provider.kt @@ -39,6 +39,11 @@ abstract class RunnableProvider : Provider, Runnable { @Volatile private var value: Any? = null + /** + * Calculate the value of this [Provider]. This function will be called at-most once by [run]. + * Do not call this function directly, instead use [get] and [getOrNull] which implement the + * correct threading behaviour and will reuse the value if it has been previously calculated. + */ abstract operator fun invoke(): E override fun getOrNull(): E? { @@ -55,8 +60,16 @@ abstract class RunnableProvider : Provider, Runnable { TASK_STATE_RUNNING -> awaitResult() TASK_STATE_PENDING -> { if (isMainThread()) { + // When the calling thread is the 'main' thread, we *always* wait for the + // background workers to [invoke] this Provider, assuming that the Provider + // is performing some kind of IO that should be kept away from the main + // thread. Ideally this doesn't happen, but this behaviour avoids the + // need for complicated callback mechanisms. awaitResult() } else { + // If the Provider has yet to be computed, we will try and run it on the + // current thread. This potentially causes run() to happen on a different + // Thread to the expected worker (TaskType), effectively like work-stealing. run() } } @@ -71,6 +84,10 @@ abstract class RunnableProvider : Provider, Runnable { return Thread.currentThread() === mainThread } + /** + * Cause the current thread to wait (block) until this `Provider` [isComplete]. Upon returning + * the [isComplete] function will return `true`. + */ private fun awaitResult() { synchronized(this) { while (!isComplete()) { @@ -85,6 +102,14 @@ abstract class RunnableProvider : Provider, Runnable { else -> true } + /** + * The main entry point for a provider, typically called by a worker thread from + * [BackgroundTaskService]. If [run] has already been called this will be a no-op (including + * a reentrant thread), as such the task state *must* be checked after calling this. + * + * This should not be called, and instead [get] or [getOrNull] should be used to obtain the + * value produced by [invoke]. + */ final override fun run() { if (state.compareAndSet(TASK_STATE_PENDING, TASK_STATE_RUNNING)) { try { @@ -104,9 +129,28 @@ abstract class RunnableProvider : Provider, Runnable { } private companion object { + /** + * The `Provider` task state before the provider has started actually running. This state + * indicates that the task has been constructed, has typically been scheduled but has + * not actually started running yet. + */ private const val TASK_STATE_PENDING = 0 + + /** + * The `Provider` task state when running. Once the [run] function returns the state will + * be either [TASK_STATE_COMPLETE] or [TASK_STATE_FAILED]. + */ private const val TASK_STATE_RUNNING = 1 + + /** + * The `Provider` state of a successfully completed task. When this is the state the + * provider value can be obtained immediately without error. + */ private const val TASK_STATE_COMPLETE = 2 + + /** + * The `Provider` state of a task where [invoke] failed with an error or exception. + */ private const val TASK_STATE_FAILED = 999 private val mainThread = Looper.getMainLooper().thread From ab16c628d57138a1d236c0cd811daadd580a22bc Mon Sep 17 00:00:00 2001 From: jason Date: Fri, 4 Oct 2024 14:44:14 +0100 Subject: [PATCH 4/9] refactor(startup): move Build UUID retrieval / calculation into a provider --- .../api/bugsnag-android-core.api | 27 +++++--- bugsnag-android-core/detekt-baseline.xml | 6 +- .../com/bugsnag/android/DeviceIdStoreTest.kt | 1 + .../java/com/bugsnag/android/UserStoreTest.kt | 1 + .../src/main/java/com/bugsnag/android/App.kt | 12 +++- .../java/com/bugsnag/android/AppWithState.kt | 62 ++++++++++++++++++- .../com/bugsnag/android/BugsnagEventMapper.kt | 3 +- .../com/bugsnag/android/ClientObservable.kt | 2 +- .../android/internal/ImmutableConfig.kt | 26 +++----- .../bugsnag/android/internal/dag/Provider.kt | 27 +++++++- .../java/com/bugsnag/android/ValueProvider.kt | 8 --- .../com/bugsnag/android/ClientFacadeTest.java | 1 + .../android/ConfigChangeReceiverTest.kt | 1 + .../com/bugsnag/android/DataCollectorTest.kt | 1 + .../DeviceDataCollectorSerializationTest.kt | 1 + .../DeviceMetadataSerializationTest.kt | 1 + .../bugsnag/android/ImmutableConfigTest.kt | 16 +++-- .../InternalEventPayloadDelegateTest.kt | 1 + .../com/bugsnag/android/AppDeserializer.java | 4 +- .../com/bugsnag/android/ConfigSerializer.java | 6 +- .../java/com/bugsnag/android/TestData.java | 3 +- 21 files changed, 157 insertions(+), 53 deletions(-) delete mode 100644 bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueProvider.kt diff --git a/bugsnag-android-core/api/bugsnag-android-core.api b/bugsnag-android-core/api/bugsnag-android-core.api index 4b934864b7..57175ab5bb 100644 --- a/bugsnag-android-core/api/bugsnag-android-core.api +++ b/bugsnag-android-core/api/bugsnag-android-core.api @@ -878,11 +878,11 @@ public final class com/bugsnag/android/internal/DateUtils { } public final class com/bugsnag/android/internal/ImmutableConfig { - public fun (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 fun (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;Lcom/bugsnag/android/internal/dag/Provider;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; - public final fun component12 ()Ljava/lang/String; + public final fun component12 ()Lcom/bugsnag/android/internal/dag/Provider; public final fun component13 ()Ljava/lang/String; public final fun component14 ()Ljava/lang/Integer; public final fun component15 ()Ljava/lang/String; @@ -911,8 +911,8 @@ public final class com/bugsnag/android/internal/ImmutableConfig { 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;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 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;Lcom/bugsnag/android/internal/dag/Provider;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;Lcom/bugsnag/android/internal/dag/Provider;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; @@ -921,7 +921,7 @@ public final class com/bugsnag/android/internal/ImmutableConfig { public final fun getAttemptDeliveryOnCrash ()Z public final fun getAutoDetectErrors ()Z public final fun getAutoTrackSessions ()Z - public final fun getBuildUuid ()Ljava/lang/String; + public final fun getBuildUuid ()Lcom/bugsnag/android/internal/dag/Provider; public final fun getDelivery ()Lcom/bugsnag/android/Delivery; public final fun getDiscardClasses ()Ljava/util/Collection; public final fun getEnabledBreadcrumbTypes ()Ljava/util/Set; @@ -957,9 +957,9 @@ public final class com/bugsnag/android/internal/ImmutableConfig { public final class com/bugsnag/android/internal/ImmutableConfigKt { public static final fun convertToImmutableConfig (Lcom/bugsnag/android/Configuration;)Lcom/bugsnag/android/internal/ImmutableConfig; - public static final fun convertToImmutableConfig (Lcom/bugsnag/android/Configuration;Ljava/lang/String;)Lcom/bugsnag/android/internal/ImmutableConfig; - public static final fun convertToImmutableConfig (Lcom/bugsnag/android/Configuration;Ljava/lang/String;Landroid/content/pm/PackageInfo;)Lcom/bugsnag/android/internal/ImmutableConfig; - public static final fun convertToImmutableConfig (Lcom/bugsnag/android/Configuration;Ljava/lang/String;Landroid/content/pm/PackageInfo;Landroid/content/pm/ApplicationInfo;)Lcom/bugsnag/android/internal/ImmutableConfig; + public static final fun convertToImmutableConfig (Lcom/bugsnag/android/Configuration;Lcom/bugsnag/android/internal/dag/Provider;)Lcom/bugsnag/android/internal/ImmutableConfig; + public static final fun convertToImmutableConfig (Lcom/bugsnag/android/Configuration;Lcom/bugsnag/android/internal/dag/Provider;Landroid/content/pm/PackageInfo;)Lcom/bugsnag/android/internal/ImmutableConfig; + public static final fun convertToImmutableConfig (Lcom/bugsnag/android/Configuration;Lcom/bugsnag/android/internal/dag/Provider;Landroid/content/pm/PackageInfo;Landroid/content/pm/ApplicationInfo;)Lcom/bugsnag/android/internal/ImmutableConfig; public static final fun isInvalidApiKey (Ljava/lang/String;)Z } @@ -1024,3 +1024,14 @@ public abstract class com/bugsnag/android/internal/dag/RunnableProvider : com/bu public final fun run ()V } +public final class com/bugsnag/android/internal/dag/ValueProvider : com/bugsnag/android/internal/dag/Provider { + public fun (Ljava/lang/Object;)V + public final fun copy (Ljava/lang/Object;)Lcom/bugsnag/android/internal/dag/ValueProvider; + public static synthetic fun copy$default (Lcom/bugsnag/android/internal/dag/ValueProvider;Ljava/lang/Object;ILjava/lang/Object;)Lcom/bugsnag/android/internal/dag/ValueProvider; + public fun equals (Ljava/lang/Object;)Z + public fun get ()Ljava/lang/Object; + public fun getOrNull ()Ljava/lang/Object; + public fun hashCode ()I + public fun toString ()Ljava/lang/String; +} + diff --git a/bugsnag-android-core/detekt-baseline.xml b/bugsnag-android-core/detekt-baseline.xml index b12f5c70a5..d30229e5de 100644 --- a/bugsnag-android-core/detekt-baseline.xml +++ b/bugsnag-android-core/detekt-baseline.xml @@ -5,9 +5,10 @@ CyclomaticComplexMethod:AppDataCollector.kt$AppDataCollector$@SuppressLint("SwitchIntDef") @Suppress("DEPRECATION") private fun getProcessImportance(): String? CyclomaticComplexMethod:ConfigInternal.kt$ConfigInternal$fun getConfigDifferences(): Map<String, Any> ImplicitDefaultLocale:DeliveryHeaders.kt$String.format("%02x", byte) - LongParameterList:App.kt$App$( /** * The architecture of the running application binary */ var binaryArch: String?, /** * The package name of the application */ var id: String?, /** * The release stage set in [Configuration.releaseStage] */ var releaseStage: String?, /** * The version of the application set in [Configuration.version] */ var version: String?, /** The revision ID from the manifest (React Native apps only) */ var codeBundleId: String?, /** * The unique identifier for the build of the application set in [Configuration.buildUuid] */ var buildUuid: String?, /** * The application type set in [Configuration#version] */ var type: String?, /** * The version code of the application set in [Configuration.versionCode] */ var versionCode: Number? ) + LongParameterList:App.kt$App$( /** * The architecture of the running application binary */ var binaryArch: String?, /** * The package name of the application */ var id: String?, /** * The release stage set in [Configuration.releaseStage] */ var releaseStage: String?, /** * The version of the application set in [Configuration.version] */ var version: String?, /** The revision ID from the manifest (React Native apps only) */ var codeBundleId: String?, /** * The unique identifier for the build of the application set in [Configuration.buildUuid] */ buildUuid: Provider<String?>?, /** * The application type set in [Configuration#version] */ var type: String?, /** * The version code of the application set in [Configuration.versionCode] */ var versionCode: Number? ) LongParameterList:AppDataCollector.kt$AppDataCollector$( appContext: Context, private val packageManager: PackageManager?, private val config: ImmutableConfig, private val sessionTracker: SessionTracker, private val activityManager: ActivityManager?, private val launchCrashTracker: LaunchCrashTracker, private val memoryTrimState: MemoryTrimState ) - LongParameterList:AppWithState.kt$AppWithState$( binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, buildUuid: String?, type: String?, versionCode: Number?, /** * The number of milliseconds the application was running before the event occurred */ var duration: Number?, /** * The number of milliseconds the application was running in the foreground before the * event occurred */ var durationInForeground: Number?, /** * Whether the application was in the foreground when the event occurred */ var inForeground: Boolean?, /** * Whether the application was launching when the event occurred */ var isLaunching: Boolean? ) + LongParameterList:AppWithState.kt$AppWithState$( binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, buildUuid: Provider<String?>?, type: String?, versionCode: Number?, /** * The number of milliseconds the application was running before the event occurred */ var duration: Number?, /** * The number of milliseconds the application was running in the foreground before the * event occurred */ var durationInForeground: Number?, /** * Whether the application was in the foreground when the event occurred */ var inForeground: Boolean?, /** * Whether the application was launching when the event occurred */ var isLaunching: Boolean? ) + LongParameterList:AppWithState.kt$AppWithState$( binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, buildUuid: String?, type: String?, versionCode: Number?, /** * The number of milliseconds the application was running before the event occurred */ duration: Number?, /** * The number of milliseconds the application was running in the foreground before the * event occurred */ durationInForeground: Number?, /** * Whether the application was in the foreground when the event occurred */ inForeground: Boolean?, /** * Whether the application was launching when the event occurred */ isLaunching: Boolean? ) LongParameterList:AppWithState.kt$AppWithState$( config: ImmutableConfig, binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, duration: Number?, durationInForeground: Number?, inForeground: Boolean?, isLaunching: Boolean? ) LongParameterList:DataCollectionModule.kt$DataCollectionModule$( contextModule: ContextModule, configModule: ConfigModule, systemServiceModule: SystemServiceModule, trackerModule: TrackerModule, bgTaskService: BackgroundTaskService, connectivity: Connectivity, deviceIdStore: Provider<DeviceIdStore>, memoryTrimState: MemoryTrimState ) LongParameterList:Device.kt$Device$( buildInfo: DeviceBuildInfo, /** * The Application Binary Interface used */ var cpuAbi: Array<String>?, /** * Whether the device has been jailbroken */ var jailbroken: Boolean?, /** * A UUID generated by Bugsnag and used for the individual application on a device */ var id: String?, /** * The IETF language tag of the locale used */ var locale: String?, /** * The total number of bytes of memory on the device */ var totalMemory: Long?, /** * A collection of names and their versions of the primary languages, frameworks or * runtimes that the application is running on */ runtimeVersions: MutableMap<String, Any>? ) @@ -60,7 +61,6 @@ SwallowedException:DeviceIdFilePersistence.kt$DeviceIdFilePersistence$exc: OverlappingFileLockException SwallowedException:EventStore.kt$EventStore$exception: RejectedExecutionException SwallowedException:ForegroundDetector.kt$ForegroundDetector$e: Exception - SwallowedException:ImmutableConfig.kt$e: Exception SwallowedException:JsonHelperTest.kt$JsonHelperTest$e: IllegalArgumentException SwallowedException:PluginClient.kt$PluginClient$exc: ClassNotFoundException SwallowedException:SharedPrefMigrator.kt$SharedPrefMigrator$e: RuntimeException diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DeviceIdStoreTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DeviceIdStoreTest.kt index e2a06ef8b7..17f36ab019 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DeviceIdStoreTest.kt +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/DeviceIdStoreTest.kt @@ -3,6 +3,7 @@ package com.bugsnag.android import android.content.Context import androidx.test.core.app.ApplicationProvider import com.bugsnag.android.internal.ImmutableConfig +import com.bugsnag.android.internal.dag.ValueProvider import junit.framework.TestCase.assertNull import org.junit.Assert.assertEquals import org.junit.Before diff --git a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/UserStoreTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/UserStoreTest.kt index 74e774db67..23c1b167dc 100644 --- a/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/UserStoreTest.kt +++ b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/UserStoreTest.kt @@ -3,6 +3,7 @@ package com.bugsnag.android import android.content.Context import android.content.SharedPreferences import androidx.test.core.app.ApplicationProvider +import com.bugsnag.android.internal.dag.ValueProvider import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/App.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/App.kt index e603b2926e..78c9a3d5ab 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/App.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/App.kt @@ -1,6 +1,7 @@ package com.bugsnag.android import com.bugsnag.android.internal.ImmutableConfig +import com.bugsnag.android.internal.dag.Provider import java.io.IOException /** @@ -36,7 +37,7 @@ open class App internal constructor( /** * The unique identifier for the build of the application set in [Configuration.buildUuid] */ - var buildUuid: String?, + buildUuid: Provider?, /** * The application type set in [Configuration#version] @@ -49,6 +50,15 @@ open class App internal constructor( var versionCode: Number? ) : JsonStream.Streamable { + private var buildUuidProvider: Provider? = buildUuid + + var buildUuid: String? = null + get() = field ?: buildUuidProvider?.getOrNull() + set(value) { + field = value + buildUuidProvider = null + } + internal constructor( config: ImmutableConfig, binaryArch: String?, diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/AppWithState.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/AppWithState.kt index bf05c4af6b..b087a2d8bd 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/AppWithState.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/AppWithState.kt @@ -1,18 +1,20 @@ package com.bugsnag.android import com.bugsnag.android.internal.ImmutableConfig +import com.bugsnag.android.internal.dag.Provider +import com.bugsnag.android.internal.dag.ValueProvider /** * Stateful information set by the notifier about your app can be found on this class. These values * can be accessed and amended if necessary. */ -class AppWithState( +class AppWithState internal constructor( binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, - buildUuid: String?, + buildUuid: Provider?, type: String?, versionCode: Number?, @@ -36,7 +38,61 @@ class AppWithState( * Whether the application was launching when the event occurred */ var isLaunching: Boolean? -) : App(binaryArch, id, releaseStage, version, codeBundleId, buildUuid, type, versionCode) { +) : App( + binaryArch, + id, + releaseStage, + version, + codeBundleId, + buildUuid, + type, + versionCode +) { + + constructor( + binaryArch: String?, + id: String?, + releaseStage: String?, + version: String?, + codeBundleId: String?, + buildUuid: String?, + type: String?, + versionCode: Number?, + + /** + * The number of milliseconds the application was running before the event occurred + */ + duration: Number?, + + /** + * The number of milliseconds the application was running in the foreground before the + * event occurred + */ + durationInForeground: Number?, + + /** + * Whether the application was in the foreground when the event occurred + */ + inForeground: Boolean?, + + /** + * Whether the application was launching when the event occurred + */ + isLaunching: Boolean? + ) : this( + binaryArch, + id, + releaseStage, + version, + codeBundleId, + buildUuid?.let(::ValueProvider), + type, + versionCode, + duration, + durationInForeground, + inForeground, + isLaunching + ) internal constructor( config: ImmutableConfig, diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt index b63606de39..4fae3a1fe4 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt @@ -2,6 +2,7 @@ package com.bugsnag.android import com.bugsnag.android.internal.DateUtils import com.bugsnag.android.internal.InternalMetricsImpl +import com.bugsnag.android.internal.dag.ValueProvider import java.text.DateFormat import java.text.SimpleDateFormat import java.util.Date @@ -151,7 +152,7 @@ internal class BugsnagEventMapper( app["releaseStage"] as? String, app["version"] as? String, app["codeBundleId"] as? String, - app["buildUUID"] as? String, + (app["buildUUID"] as? String)?.let(::ValueProvider), app["type"] as? String, (app["versionCode"] as? Number)?.toInt(), (app["duration"] as? Number)?.toLong(), diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/ClientObservable.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/ClientObservable.kt index b099c7edc3..989ea30fba 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/ClientObservable.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/ClientObservable.kt @@ -18,7 +18,7 @@ internal class ClientObservable : BaseObservable() { conf.apiKey, conf.enabledErrorTypes.ndkCrashes, conf.appVersion, - conf.buildUuid, + conf.buildUuid?.getOrNull(), conf.releaseStage, lastRunInfoPath, consecutiveLaunchCrashes, diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt index 17e195727b..1c9c89096a 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt @@ -22,10 +22,11 @@ import com.bugsnag.android.Session import com.bugsnag.android.Telemetry import com.bugsnag.android.ThreadSendPolicy import com.bugsnag.android.errorApiHeaders +import com.bugsnag.android.internal.dag.Provider +import com.bugsnag.android.internal.dag.ValueProvider import com.bugsnag.android.safeUnrollCauses import com.bugsnag.android.sessionApiHeaders import java.io.File -import java.util.concurrent.Callable import java.util.regex.Pattern data class ImmutableConfig( @@ -40,7 +41,7 @@ data class ImmutableConfig( val enabledBreadcrumbTypes: Set?, val telemetry: Set, val releaseStage: String?, - val buildUuid: String?, + val buildUuid: Provider?, val appVersion: String?, val versionCode: Int?, val appType: String?, @@ -140,7 +141,7 @@ data class ImmutableConfig( @JvmOverloads internal fun convertToImmutableConfig( config: Configuration, - buildUuid: String? = null, + buildUuid: Provider? = null, packageInfo: PackageInfo? = null, appInfo: ApplicationInfo? = null, persistenceDir: Lazy = lazy { requireNotNull(config.persistenceDirectory) } @@ -275,25 +276,16 @@ internal fun sanitiseConfiguration( private fun collectBuildUuid( appInfo: ApplicationInfo?, backgroundTaskService: BackgroundTaskService -): String? { +): Provider? { val bundle = appInfo?.metaData return when { - bundle?.containsKey(BUILD_UUID) == true -> { + bundle?.containsKey(BUILD_UUID) == true -> ValueProvider( (bundle.getString(BUILD_UUID) ?: bundle.getInt(BUILD_UUID).toString()) .takeIf { it.isNotEmpty() } - } + ) - appInfo != null -> { - try { - backgroundTaskService - .submitTask( - TaskType.IO, - Callable { DexBuildIdGenerator.generateBuildId(appInfo) } - ) - .get() - } catch (e: Exception) { - null - } + appInfo != null -> backgroundTaskService.provider(TaskType.IO) { + DexBuildIdGenerator.generateBuildId(appInfo) } else -> null diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/Provider.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/Provider.kt index c021663305..6eb4408439 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/Provider.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/internal/dag/Provider.kt @@ -1,6 +1,7 @@ package com.bugsnag.android.internal.dag import android.os.Looper +import androidx.annotation.VisibleForTesting import java.util.concurrent.atomic.AtomicInteger /** @@ -128,7 +129,8 @@ abstract class RunnableProvider : Provider, Runnable { } } - private companion object { + @VisibleForTesting + internal companion object { /** * The `Provider` task state before the provider has started actually running. This state * indicates that the task has been constructed, has typically been scheduled but has @@ -153,6 +155,27 @@ abstract class RunnableProvider : Provider, Runnable { */ private const val TASK_STATE_FAILED = 999 - private val mainThread = Looper.getMainLooper().thread + /** + * We cache the main thread to avoid any locks within [Looper.getMainLooper]. This is + * settable for unit tests, so that there doesn't have to be a valid Looper when they run. + * + * Actually access is done via the [mainThread] property. + */ + @VisibleForTesting + @Suppress("ObjectPropertyNaming") // backing property from 'mainThread' + internal var _mainThread: Thread? = null + get() { + if (field == null) { + field = Looper.getMainLooper().thread + } + return field + } + + internal val mainThread: Thread get() = _mainThread!! } } + +data class ValueProvider(private val value: T) : Provider { + override fun getOrNull(): T? = get() + override fun get(): T = value +} diff --git a/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueProvider.kt b/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueProvider.kt deleted file mode 100644 index 42a62e46ee..0000000000 --- a/bugsnag-android-core/src/sharedTest/java/com/bugsnag/android/ValueProvider.kt +++ /dev/null @@ -1,8 +0,0 @@ -package com.bugsnag.android - -import com.bugsnag.android.internal.dag.Provider - -class ValueProvider(private val value: E) : Provider { - override fun getOrNull(): E? = value - override fun get(): E = value -} diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java b/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java index a63a5b7fc3..5774acd085 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ClientFacadeTest.java @@ -11,6 +11,7 @@ import com.bugsnag.android.internal.ImmutableConfig; import com.bugsnag.android.internal.InternalMetrics; import com.bugsnag.android.internal.StateObserver; +import com.bugsnag.android.internal.dag.ValueProvider; import android.content.Context; diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt index 8ba751f42d..4be5712061 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ConfigChangeReceiverTest.kt @@ -4,6 +4,7 @@ import android.content.Context import android.content.res.Configuration import android.content.res.Resources import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.dag.ValueProvider import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DataCollectorTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DataCollectorTest.kt index b36bc41914..e7ab9887c1 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DataCollectorTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/DataCollectorTest.kt @@ -4,6 +4,7 @@ import android.content.Context import android.content.res.Configuration import android.content.res.Resources import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.dag.ValueProvider import org.junit.Assert import org.junit.Before import org.junit.Test diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt index 60ccc80da7..1e586a9b64 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt @@ -7,6 +7,7 @@ import android.content.res.Resources import android.util.DisplayMetrics import com.bugsnag.android.BugsnagTestUtils.generateDeviceBuildInfo import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.dag.ValueProvider import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.Parameterized diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt index b5549098fc..a3a926ef02 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt @@ -7,6 +7,7 @@ import android.content.res.Resources import android.util.DisplayMetrics import com.bugsnag.android.BugsnagTestUtils.generateDeviceBuildInfo import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.dag.ValueProvider import org.junit.Test import org.junit.runner.RunWith import org.junit.runners.Parameterized diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/ImmutableConfigTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/ImmutableConfigTest.kt index 37862f6c25..75d7c6a800 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/ImmutableConfigTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/ImmutableConfigTest.kt @@ -8,6 +8,8 @@ import android.os.Bundle import com.bugsnag.android.BugsnagTestUtils.generateConfiguration import com.bugsnag.android.internal.BackgroundTaskService import com.bugsnag.android.internal.convertToImmutableConfig +import com.bugsnag.android.internal.dag.RunnableProvider +import com.bugsnag.android.internal.dag.ValueProvider import com.bugsnag.android.internal.isInvalidApiKey import com.bugsnag.android.internal.sanitiseConfiguration import org.junit.After @@ -57,6 +59,8 @@ internal class ImmutableConfigTest { seed.delivery = delivery seed.logger = NoopLogger + RunnableProvider._mainThread = java.lang.Thread.currentThread() + // we use a real BackgroundTaskService backgroundTaskService = BackgroundTaskService() } @@ -135,7 +139,7 @@ internal class ImmutableConfigTest { seed.sendLaunchCrashesSynchronously = false // verify overrides are copied across - with(convertToImmutableConfig(seed, "f7ab")) { + with(convertToImmutableConfig(seed, ValueProvider("f7ab"))) { assertEquals("5d1ec5bd39a74caa1267142706a7fb21", apiKey) // detection @@ -154,7 +158,7 @@ internal class ImmutableConfigTest { // identifiers assertEquals("1.2.3", seed.appVersion) - assertEquals("f7ab", buildUuid) + assertEquals("f7ab", buildUuid?.getOrNull()) assertEquals("custom", seed.appType) // network config @@ -242,7 +246,7 @@ internal class ImmutableConfigTest { // validate build uuid val seed = Configuration("5d1ec5bd39a74caa1267142706a7fb21") val config = sanitiseConfiguration(context, seed, connectivity, backgroundTaskService) - assertEquals("6533e9f7-0e98-40fe-84b4-0e4ed6df6866", config.buildUuid) + assertEquals("6533e9f7-0e98-40fe-84b4-0e4ed6df6866", config.buildUuid?.getOrNull()) } @Test @@ -260,7 +264,7 @@ internal class ImmutableConfigTest { // validate build uuid val seed = Configuration("5d1ec5bd39a74caa1267142706a7fb21") val config = sanitiseConfiguration(context, seed, connectivity, backgroundTaskService) - assertNull(config.buildUuid) + assertNull(config.buildUuid?.getOrNull()) } @Test @@ -279,7 +283,7 @@ internal class ImmutableConfigTest { // validate build uuid val seed = Configuration("5d1ec5bd39a74caa1267142706a7fb21") val config = sanitiseConfiguration(context, seed, connectivity, backgroundTaskService) - assertEquals("590265330", config.buildUuid) + assertEquals("590265330", config.buildUuid?.getOrNull()) } @Test @@ -296,7 +300,7 @@ internal class ImmutableConfigTest { // validate build uuid val seed = Configuration("5d1ec5bd39a74caa1267142706a7fb21") val config = sanitiseConfiguration(context, seed, connectivity, backgroundTaskService) - assertNull(config.buildUuid) + assertNull(config.buildUuid?.getOrNull()) } @Test(expected = IllegalArgumentException::class) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/InternalEventPayloadDelegateTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/InternalEventPayloadDelegateTest.kt index 6ced18091f..d17dac578f 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/InternalEventPayloadDelegateTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/InternalEventPayloadDelegateTest.kt @@ -6,6 +6,7 @@ import com.bugsnag.android.BugsnagTestUtils.generateAppWithState import com.bugsnag.android.BugsnagTestUtils.generateDeviceWithState import com.bugsnag.android.BugsnagTestUtils.generateImmutableConfig import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.dag.ValueProvider import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull import org.junit.Test diff --git a/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/AppDeserializer.java b/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/AppDeserializer.java index b864053085..9f56d2d0ca 100644 --- a/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/AppDeserializer.java +++ b/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/AppDeserializer.java @@ -1,5 +1,7 @@ package com.bugsnag.android; +import com.bugsnag.android.internal.dag.ValueProvider; + import java.util.Map; class AppDeserializer implements MapDeserializer { @@ -12,7 +14,7 @@ public AppWithState deserialize(Map map) { MapUtils.getOrNull(map, "releaseStage"), MapUtils.getOrNull(map, "version"), MapUtils.getOrNull(map, "codeBundleId"), - MapUtils.getOrNull(map, "buildUuid"), + new ValueProvider<>(MapUtils.getOrNull(map, "buildUuid")), MapUtils.getOrNull(map, "type"), MapUtils.getOrNull(map, "versionCode"), MapUtils.getOrNull(map, "duration"), diff --git a/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/ConfigSerializer.java b/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/ConfigSerializer.java index d064c1a987..d2e63a14cf 100644 --- a/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/ConfigSerializer.java +++ b/bugsnag-plugin-react-native/src/main/java/com/bugsnag/android/ConfigSerializer.java @@ -1,6 +1,7 @@ package com.bugsnag.android; import com.bugsnag.android.internal.ImmutableConfig; +import com.bugsnag.android.internal.dag.Provider; import java.util.Collection; import java.util.HashMap; @@ -19,7 +20,10 @@ public void serialize(Map map, ImmutableConfig config) { map.put("projectPackages", config.getProjectPackages()); map.put("enabledReleaseStages", config.getEnabledReleaseStages()); map.put("releaseStage", config.getReleaseStage()); - map.put("buildUuid", config.getBuildUuid()); + Provider buildUuid = config.getBuildUuid(); + if (buildUuid != null) { + map.put("buildUuid", buildUuid.getOrNull()); + } if (config.getAppVersion() != null) { map.put("appVersion", config.getAppVersion()); } diff --git a/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java b/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java index b08eae88d2..e3630a92f3 100644 --- a/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java +++ b/bugsnag-plugin-react-native/src/test/java/com/bugsnag/android/TestData.java @@ -1,6 +1,7 @@ package com.bugsnag.android; import com.bugsnag.android.internal.ImmutableConfig; +import com.bugsnag.android.internal.dag.ValueProvider; import kotlin.LazyKt; import kotlin.jvm.functions.Function0; @@ -28,7 +29,7 @@ static ImmutableConfig generateConfig() throws IOException { new HashSet<>(Collections.singletonList(BreadcrumbType.MANUAL)), EnumSet.of(Telemetry.INTERNAL_ERRORS, Telemetry.USAGE), "production", - "builduuid-123", + new ValueProvider<>("builduuid-123"), "1.4.3", 55, "android", From d797cc253a3ffd1b7ed2e63007f42f7e4c8259f4 Mon Sep 17 00:00:00 2001 From: jason Date: Tue, 12 Nov 2024 10:13:06 +0000 Subject: [PATCH 5/9] chore(docs): updated the PROJECT_STRUCTURE.md docs --- docs/PROJECT_STRUCTURE.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/PROJECT_STRUCTURE.md b/docs/PROJECT_STRUCTURE.md index b23744c205..0c4b68661f 100644 --- a/docs/PROJECT_STRUCTURE.md +++ b/docs/PROJECT_STRUCTURE.md @@ -9,7 +9,8 @@ The project consists of [6 Gradle modules](https://gradle.org/), which are linke - [bugsnag-android-core](../bugsnag-android-core/README.md) - contains the core functionality required to capture and deliver JVM errors to Bugsnag. - [bugsnag-plugin-android-ndk](../bugsnag-plugin-android-ndk/README.md) - contains optional functionality that installs signal handlers and captures NDK errors. - [bugsnag-plugin-android-anr](../bugsnag-plugin-android-ndk/README.md) - contains optional functionality that installs a signal handler to capture ANR errors. -- [bugsnag-plugin-android-okhttp](../bugsnag-plugin-android-okhttp/README.md) - contains optional functionality that captures breadcrumbs for OkHttp requests. +- [bugsnag-plugin-android-okhttp](../bugsnag-plugin-android-okhttp/README.md) - contains optional functionality for OkHttp. +- [bugsnag-plugin-android-exitinfo](../bugsnag-plugin-android-exitinfo) - contains optional functionality to collect additional data using [historical process exit reasons](https://developer.android.com/reference/android/app/ActivityManager#getHistoricalProcessExitReasons(java.lang.String,%20int,%20int)). - [bugsnag-plugin-react-native](../bugsnag-plugin-react-native/README.md) - contains optional functionality that serializes information into a format that can be understood by the React Native bridge. - [bugsnag-android](../bugsnag-android/README.md) - an anchor package which allows users to pull in all the required modules. - [bugsnag-android-ndk](../bugsnag-android-ndk/README.md) - an anchor package which allows users to pull in all the required modules. Published for legacy reasons. From 719d602b4b7c05a7fc0f6245ecec6af81213c99b Mon Sep 17 00:00:00 2001 From: jason Date: Tue, 12 Nov 2024 10:47:24 +0000 Subject: [PATCH 6/9] chore(cmake): bumped the CMake version to 3.22.1 and the minimum version to 3.19 --- bugsnag-android-core/CMakeLists.txt | 3 ++- bugsnag-plugin-android-anr/CMakeLists.txt | 3 ++- bugsnag-plugin-android-ndk/CMakeLists.txt | 3 ++- .../src/main/kotlin/com/bugsnag/android/BugsnagBuildPlugin.kt | 2 ++ buildSrc/src/main/kotlin/com/bugsnag/android/Versions.kt | 1 + dockerfiles/Dockerfile.android-common | 2 +- 6 files changed, 10 insertions(+), 4 deletions(-) diff --git a/bugsnag-android-core/CMakeLists.txt b/bugsnag-android-core/CMakeLists.txt index 3d76ae5f2a..7d9d8c9f9e 100644 --- a/bugsnag-android-core/CMakeLists.txt +++ b/bugsnag-android-core/CMakeLists.txt @@ -1,2 +1,3 @@ -cmake_minimum_required(VERSION 3.4.1) +cmake_minimum_required(VERSION 3.19) +project(bugsnag-android-core) add_subdirectory(src/main) diff --git a/bugsnag-plugin-android-anr/CMakeLists.txt b/bugsnag-plugin-android-anr/CMakeLists.txt index 3d76ae5f2a..241fbec19f 100644 --- a/bugsnag-plugin-android-anr/CMakeLists.txt +++ b/bugsnag-plugin-android-anr/CMakeLists.txt @@ -1,2 +1,3 @@ -cmake_minimum_required(VERSION 3.4.1) +cmake_minimum_required(VERSION 3.19) +project(bugsnag-plugin-android-anr) add_subdirectory(src/main) diff --git a/bugsnag-plugin-android-ndk/CMakeLists.txt b/bugsnag-plugin-android-ndk/CMakeLists.txt index f16af6162d..1ea3a01a9f 100644 --- a/bugsnag-plugin-android-ndk/CMakeLists.txt +++ b/bugsnag-plugin-android-ndk/CMakeLists.txt @@ -1,4 +1,5 @@ -cmake_minimum_required(VERSION 3.8.0) +cmake_minimum_required(VERSION 3.19) +project(bugsnag-plugin-android-ndk) project(TEST) add_subdirectory(src/main) diff --git a/buildSrc/src/main/kotlin/com/bugsnag/android/BugsnagBuildPlugin.kt b/buildSrc/src/main/kotlin/com/bugsnag/android/BugsnagBuildPlugin.kt index 3f0d29e096..f9cacde8e0 100644 --- a/buildSrc/src/main/kotlin/com/bugsnag/android/BugsnagBuildPlugin.kt +++ b/buildSrc/src/main/kotlin/com/bugsnag/android/BugsnagBuildPlugin.kt @@ -92,6 +92,7 @@ class BugsnagBuildPlugin : Plugin { "-DANDROID_STL=c++_static" ) + val override: String? = project.findProperty("ABI_FILTERS") as String? val abis = override?.split(",") ?: mutableSetOf( "arm64-v8a", @@ -102,6 +103,7 @@ class BugsnagBuildPlugin : Plugin { ndk.setAbiFilters(abis) } externalNativeBuild.cmake.path = project.file("CMakeLists.txt") + externalNativeBuild.cmake.version = Versions.cmakeVersion } /** diff --git a/buildSrc/src/main/kotlin/com/bugsnag/android/Versions.kt b/buildSrc/src/main/kotlin/com/bugsnag/android/Versions.kt index ae6b8ecc45..5b1855569e 100644 --- a/buildSrc/src/main/kotlin/com/bugsnag/android/Versions.kt +++ b/buildSrc/src/main/kotlin/com/bugsnag/android/Versions.kt @@ -13,6 +13,7 @@ object Versions { val java = JavaVersion.VERSION_1_8 val kotlin = "1.5.10" val kotlinLang = "1.5" + val cmakeVersion = "3.22.1" // plugins val androidGradlePlugin = "7.0.4" diff --git a/dockerfiles/Dockerfile.android-common b/dockerfiles/Dockerfile.android-common index 216e590600..db6b623797 100644 --- a/dockerfiles/Dockerfile.android-common +++ b/dockerfiles/Dockerfile.android-common @@ -34,7 +34,7 @@ RUN yes | sdkmanager "ndk;19.2.5345600" > /dev/null RUN yes | sdkmanager "ndk;21.4.7075529" > /dev/null RUN yes | sdkmanager "cmake;3.6.4111459" > /dev/null -RUN yes | sdkmanager "cmake;3.10.2.4988404" > /dev/null +RUN yes | sdkmanager "cmake;3.22.1" > /dev/null RUN yes | sdkmanager "build-tools;30.0.3" > /dev/null # Install bundletool From 0b87abf0bcf17961e571d069be9763e6ec146b14 Mon Sep 17 00:00:00 2001 From: jason Date: Tue, 12 Nov 2024 11:38:50 +0000 Subject: [PATCH 7/9] chore(cmake): added project directives to the mazerunner modules --- .../fixtures/mazerunner/cxx-scenarios-bugsnag/CMakeLists.txt | 1 + features/fixtures/mazerunner/cxx-scenarios/CMakeLists.txt | 1 + 2 files changed, 2 insertions(+) diff --git a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/CMakeLists.txt b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/CMakeLists.txt index d997836e8c..1fd4c47e4a 100644 --- a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/CMakeLists.txt +++ b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/CMakeLists.txt @@ -1,4 +1,5 @@ cmake_minimum_required(VERSION 3.4.1) +project(cxx-scenarios-bugsnag) find_package(bugsnag-plugin-android-ndk REQUIRED CONFIG) diff --git a/features/fixtures/mazerunner/cxx-scenarios/CMakeLists.txt b/features/fixtures/mazerunner/cxx-scenarios/CMakeLists.txt index 1a3f3945aa..2233799aae 100644 --- a/features/fixtures/mazerunner/cxx-scenarios/CMakeLists.txt +++ b/features/fixtures/mazerunner/cxx-scenarios/CMakeLists.txt @@ -1,4 +1,5 @@ cmake_minimum_required(VERSION 3.4.1) +project(cxx-scenarios) add_library(cxx-scenarios SHARED src/main/cpp/cxx-scenarios.cpp From 0d6d09f96427b6e7f09247a785794d2194b0be94 Mon Sep 17 00:00:00 2001 From: jason Date: Thu, 14 Nov 2024 10:38:52 +0000 Subject: [PATCH 8/9] chore(startup): added CHANGELOG entry for the startup work --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d01509630f..4da029d3d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## TBD + +### Enhancements + +* Restructured Bugsnag.start to reduce the chance of deadlocks and allow further reductions in startup overhead + [#2089](https://github.com/bugsnag/bugsnag-android/pull/2089) + ## 6.9.0 (2024-11-07) ### Enhancements From 3cef911daafc1b96e2ef8343403c04bc141ee262 Mon Sep 17 00:00:00 2001 From: YYChen01988 Date: Thu, 14 Nov 2024 15:22:33 +0000 Subject: [PATCH 9/9] release v6.10.0 --- CHANGELOG.md | 2 +- .../src/main/java/com/bugsnag/android/Notifier.kt | 2 +- examples/sdk-app-example/app/build.gradle | 4 ++-- gradle.properties | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4da029d3d3..e1e3b868c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## TBD +## 6.10.0 (2024-11-14) ### Enhancements diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Notifier.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/Notifier.kt index 117d0da716..e8c0693cdd 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Notifier.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Notifier.kt @@ -7,7 +7,7 @@ import java.io.IOException */ class Notifier @JvmOverloads constructor( var name: String = "Android Bugsnag Notifier", - var version: String = "6.9.0", + var version: String = "6.10.0", var url: String = "https://bugsnag.com" ) : JsonStream.Streamable { diff --git a/examples/sdk-app-example/app/build.gradle b/examples/sdk-app-example/app/build.gradle index 7b423075f9..604e26a0f0 100644 --- a/examples/sdk-app-example/app/build.gradle +++ b/examples/sdk-app-example/app/build.gradle @@ -42,8 +42,8 @@ android { } dependencies { - implementation "com.bugsnag:bugsnag-android:6.9.0" - implementation "com.bugsnag:bugsnag-plugin-android-okhttp:6.9.0" + implementation "com.bugsnag:bugsnag-android:6.10.0" + implementation "com.bugsnag:bugsnag-plugin-android-okhttp:6.10.0" implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version" implementation "androidx.appcompat:appcompat:1.6.1" implementation "com.google.android.material:material:1.11.0" diff --git a/gradle.properties b/gradle.properties index 5c5ba7faea..f60eadcec2 100644 --- a/gradle.properties +++ b/gradle.properties @@ -11,7 +11,7 @@ org.gradle.jvmargs=-Xmx4096m # This option should only be used with decoupled projects. More details, visit # http://www.gradle.org/docs/current/userguide/multi_project_builds.html#sec:decoupled_projects org.gradle.parallel=true -VERSION_NAME=6.9.0 +VERSION_NAME=6.10.0 GROUP=com.bugsnag POM_SCM_URL=https://github.com/bugsnag/bugsnag-android POM_SCM_CONNECTION=scm:git@github.com:bugsnag/bugsnag-android.git