Merge pull request #9826 from rafaeltonholo/fix/9825/fix-app-crash

fix: db migration to version 90 crashing the app and causing database wipe

Author: rafaeltonholo

feature/navigation/drawer/dropdown/src/main/kotlin/net/thunderbird/feature/navigation/drawer/dropdown/domain/usecase/GetDisplayTreeFolder.kt

@@ -3,6 +3,7 @@ package net.thunderbird.feature.navigation.drawer.dropdown.domain.usecase
 import kotlinx.collections.immutable.persistentListOf
 import kotlinx.collections.immutable.toImmutableList
 import net.thunderbird.core.logging.Logger
+import net.thunderbird.feature.mail.folder.api.FOLDER_DEFAULT_PATH_DELIMITER
 import net.thunderbird.feature.mail.folder.api.Folder
 import net.thunderbird.feature.mail.folder.api.FolderPathDelimiter
 import net.thunderbird.feature.mail.folder.api.FolderType
@@ -28,7 +29,7 @@ internal class GetDisplayTreeFolder(
             )
         }
 
-        val pathDelimiter = folders.first().pathDelimiter
+        val pathDelimiter = folders.firstOrNull()?.pathDelimiter ?: FOLDER_DEFAULT_PATH_DELIMITER
         val accountFolders = folders.filterIsInstance<MailDisplayFolder>().map {
             val path = flattenPath(it.folder.name, pathDelimiter, maxDepth)
             logger.debug { "Flattened path for ${it.folder.name} → $path" }

legacy/storage/src/main/java/com/fsck/k9/storage/migrations/MigrationTo90.kt

@@ -21,6 +21,7 @@ import net.thunderbird.core.android.account.LegacyAccountDto
 import net.thunderbird.core.common.mail.Protocols
 import net.thunderbird.core.logging.Logger
 import net.thunderbird.core.logging.legacy.Log
+import okio.IOException
 import org.intellij.lang.annotations.Language
 import org.koin.core.component.KoinComponent
 import org.koin.core.component.inject
@@ -55,22 +56,26 @@ internal class MigrationTo90(
         try {
             logger.verbose(TAG) { "fetching IMAP prefix" }
             imapStore.fetchImapPrefix()
-        } catch (e: AuthenticationFailedException) {
-            logger.warn(TAG, e) { "failed to fetch IMAP prefix. skipping db migration" }
-            return
-        }
-
-        val imapPrefix = imapStore.combinedPrefix
+            val imapPrefix = imapStore.combinedPrefix
+
+            if (imapPrefix?.isNotBlank() == true) {
+                logger.verbose(TAG) { "Imap Prefix ($imapPrefix) detected, updating folder's server_id" }
+                val query = buildQuery(imapPrefix)
+                db.execSQL(query)
+            } else {
+                logger.verbose(TAG) { "No Imap Prefix detected, skipping db migration" }
+            }
 
-        if (imapPrefix?.isNotBlank() == true) {
-            logger.verbose(TAG) { "Imap Prefix ($imapPrefix) detected, updating folder's server_id" }
-            val query = buildQuery(imapPrefix)
-            db.execSQL(query)
-        } else {
-            logger.verbose(TAG) { "No Imap Prefix detected, skipping db migration" }
+            logger.verbose(TAG) { "completed db migration to version 90 for account ${account.uuid}" }
+        } catch (e: AuthenticationFailedException) {
+            logger.warn(TAG, e) {
+                "failed to fetch IMAP prefix due to authentication error. skipping db migration"
+            }
+        } catch (e: IOException) {
+            logger.warn(TAG, e) {
+                "failed to fetch IMAP prefix due to network error. skipping db migration"
+            }
         }
-
-        logger.verbose(TAG) { "completed db migration to version 90 for account ${account.uuid}" }
     }
 
     private fun createImapStore(account: LegacyAccountDto): ImapStore {

legacy/storage/src/test/java/com/fsck/k9/storage/migrations/MigrationTo90Test.kt

@@ -17,6 +17,7 @@ import com.fsck.k9.mail.store.imap.ImapStoreSettings
 import com.fsck.k9.mailstore.MigrationsHelper
 import com.fsck.k9.storage.messages.createFolder
 import com.fsck.k9.storage.messages.readFolders
+import java.io.IOException
 import net.thunderbird.core.android.account.LegacyAccountDto
 import net.thunderbird.core.common.mail.Protocols
 import net.thunderbird.core.logging.legacy.Log
@@ -220,7 +221,7 @@ class MigrationTo90Test : KoinTest {
     }
 
     @Test
-    fun `given an imap account - when can't fetch imap prefix during the migration - migration should not execute sql queries`() {
+    fun `given an imap account - when can't fetch imap prefix during the migration due to authentication error - migration should not execute sql queries`() {
         // Arrange
         populateDatabase()
         val prefix = "INBOX."
@@ -256,6 +257,43 @@ class MigrationTo90Test : KoinTest {
             }
     }
 
+    @Test
+    fun `given an imap account - when can't fetch imap prefix during the migration due to network error - migration should not execute sql queries`() {
+        // Arrange
+        populateDatabase()
+        val prefix = "INBOX."
+        val imapStore = createImapStoreSpy(
+            imapPrefix = prefix,
+            folderPathDelimiter = ".",
+            ioException = IOException("failed to fetch"),
+        )
+        val incomingServerSettings = createIncomingServerSettings()
+        val account = createAccount(incomingServerSettings)
+        val migrationHelper = createMigrationsHelper(account)
+        val dbSpy = spy<SQLiteDatabase> { database }
+        val migration = MigrationTo90(
+            db = dbSpy,
+            migrationsHelper = migrationHelper,
+            imapStoreFactory = createImapStoreFactory(imapStore),
+        )
+        val expected = database.readFolders().map { it.serverId }
+        val updateQuery = migration.buildQuery(imapPrefix = prefix)
+
+        // Act
+        migration.removeImapPrefixFromFolderServerId()
+        val actual = database.readFolders().mapNotNull { it.serverId }
+        testLogger.dumpLogs()
+
+        // Assert
+        verify(imapStore, times(1)).fetchImapPrefix()
+        verify(dbSpy, times(0)).execSQL(updateQuery)
+        assertThat(actual)
+            .all {
+                hasSize(expected.size)
+                isEqualTo(expected)
+            }
+    }
+
     private fun createIncomingServerSettings(
         protocolType: String = Protocols.IMAP,
         authType: AuthType = AuthType.NONE,
@@ -308,6 +346,7 @@ class MigrationTo90Test : KoinTest {
         imapPrefix: String? = null,
         folderPathDelimiter: FolderPathDelimiter = FOLDER_DEFAULT_PATH_DELIMITER,
         authenticationFailedException: AuthenticationFailedException? = null,
+        ioException: IOException? = null,
     ): ImapStore = spy<ImapStore> {
         on { this.combinedPrefix } doReturn imapPrefix
             ?.takeIf { it.isNotBlank() }
@@ -317,6 +356,9 @@ class MigrationTo90Test : KoinTest {
             if (authenticationFailedException != null) {
                 throw authenticationFailedException
             }
+            if (ioException != null) {
+                throw ioException
+            }
         }
     }