From a23352ca68ccb7f52a358fb0dbb25349f825f8cd Mon Sep 17 00:00:00 2001 From: "yingying.chen" Date: Wed, 27 Sep 2023 18:34:30 +0100 Subject: [PATCH] Bug(persistedUser) warning would not show when persisted user file is not loaded --- bugsnag-android-core/detekt-baseline.xml | 1 + .../java/com/bugsnag/android/UserStoreTest.kt | 13 +++++------- .../android/SynchronizedStreamableStore.kt | 2 +- .../java/com/bugsnag/android/UserStore.kt | 20 +++++++++---------- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/bugsnag-android-core/detekt-baseline.xml b/bugsnag-android-core/detekt-baseline.xml index 77bea1c93c..86da28d3e6 100644 --- a/bugsnag-android-core/detekt-baseline.xml +++ b/bugsnag-android-core/detekt-baseline.xml @@ -51,6 +51,7 @@ SwallowedException:JsonHelperTest.kt$JsonHelperTest$e: IllegalArgumentException SwallowedException:PluginClient.kt$PluginClient$exc: ClassNotFoundException SwallowedException:SharedPrefMigrator.kt$SharedPrefMigrator$e: RuntimeException + SwallowedException:UserStore.kt$UserStore$e: Exception 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/UserStoreTest.kt b/bugsnag-android-core/src/androidTest/java/com/bugsnag/android/UserStoreTest.kt index 8125e6ff6a..9ea96b9cd3 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 @@ -136,7 +136,7 @@ internal class UserStoreTest { /** * If persistUser is false a user is still returned */ - @Test + @Test(expected = Exception::class) fun loadWithoutPersistUser() { val store = UserStore( generateConfig(false), @@ -145,21 +145,18 @@ internal class UserStoreTest { prefMigrator, NoopLogger ) - val user = store.load(User()).user - assertEquals("device-id-123", user.id) - assertNull(user.email) - assertNull(user.name) - assertEquals("", file.readText()) + store.load(User()).user + file.readText() } /** * If persistUser is false a user is not saved */ - @Test + @Test(expected = Exception::class) fun saveWithoutPersistUser() { val store = UserStore(generateConfig(false), "", file, prefMigrator, NoopLogger) store.save(User("123", "joe@yahoo.com", "Joe Bloggs")) - assertEquals("", file.readText()) + file.readText() } /** diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/SynchronizedStreamableStore.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/SynchronizedStreamableStore.kt index 66bf136e56..f7d53367c9 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/SynchronizedStreamableStore.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/SynchronizedStreamableStore.kt @@ -13,7 +13,7 @@ import kotlin.concurrent.withLock * This class is made thread safe through the use of a [ReadWriteLock]. */ internal class SynchronizedStreamableStore( - private val file: File + internal val file: File ) { private val lock = ReentrantReadWriteLock() 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 1bd0b19201..29818abe87 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,7 +3,6 @@ package com.bugsnag.android import com.bugsnag.android.internal.ImmutableConfig import com.bugsnag.android.internal.StateObserver import java.io.File -import java.io.IOException import java.util.concurrent.atomic.AtomicReference /** @@ -22,11 +21,6 @@ internal class UserStore @JvmOverloads constructor( private val previousUser = AtomicReference(null) init { - try { - file.createNewFile() - } catch (exc: IOException) { - logger.w("Failed to created device ID file", exc) - } this.synchronizedStreamableStore = SynchronizedStreamableStore(file) } @@ -87,13 +81,19 @@ internal class UserStore @JvmOverloads constructor( val legacyUser = sharedPrefMigrator.loadUser(deviceId) save(legacyUser) legacyUser - } else { - return try { + } else if ( + synchronizedStreamableStore.file.canRead() && + synchronizedStreamableStore.file.length() > 0L && + persist + ) { + try { synchronizedStreamableStore.load(User.Companion::fromReader) - } catch (exc: Exception) { - logger.w("Failed to load user info", exc) + } catch (e: Exception) { + logger.w("Failed to load user info") null } + } else { + null } } }