Bug(persistedUser) warning would not show when persisted user file is…

… not loaded
bugsnag · Sep 28, 2023 · a23352c · a23352c
1 parent fa51e42
commit a23352c
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 19 deletions.
diff --git a/bugsnag-android-core/detekt-baseline.xml b/bugsnag-android-core/detekt-baseline.xml
@@ -51,6 +51,7 @@
     <ID>SwallowedException:JsonHelperTest.kt$JsonHelperTest$e: IllegalArgumentException</ID>
     <ID>SwallowedException:PluginClient.kt$PluginClient$exc: ClassNotFoundException</ID>
     <ID>SwallowedException:SharedPrefMigrator.kt$SharedPrefMigrator$e: RuntimeException</ID>
+    <ID>SwallowedException:UserStore.kt$UserStore$e: Exception</ID>
     <ID>ThrowsCount:JsonHelper.kt$JsonHelper$fun jsonToLong(value: Any?): Long?</ID>
     <ID>TooManyFunctions:ConfigInternal.kt$ConfigInternal : CallbackAwareMetadataAwareUserAwareFeatureFlagAware</ID>
     <ID>TooManyFunctions:DeviceDataCollector.kt$DeviceDataCollector</ID>

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
@@ -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", "[email protected]", "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
@@ -13,7 +13,7 @@ import kotlin.concurrent.withLock
  * This class is made thread safe through the use of a [ReadWriteLock].
  */
 internal class SynchronizedStreamableStore<T : JsonStream.Streamable>(
-    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
@@ -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<User?>(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
         }
     }
 }