diff --git a/app/schemas/org.libremail.data.local.AccountDatabase/1.json b/app/schemas/org.libremail.data.local.AccountDatabase/1.json new file mode 100644 index 0000000..dd0f0fa --- /dev/null +++ b/app/schemas/org.libremail.data.local.AccountDatabase/1.json @@ -0,0 +1,234 @@ +{ + "formatVersion": 1, + "database": { + "version": 1, + "identityHash": "f2bbe80e572de72f50869b14aca4c4bb", + "entities": [ + { + "tableName": "accounts", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `email` TEXT NOT NULL, `displayName` TEXT NOT NULL, `authType` TEXT NOT NULL, `imap_host` TEXT NOT NULL, `imap_port` INTEGER NOT NULL, `imap_security` TEXT NOT NULL, `smtp_host` TEXT NOT NULL, `smtp_port` INTEGER NOT NULL, `smtp_security` TEXT NOT NULL, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "email", + "columnName": "email", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "displayName", + "columnName": "displayName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "authType", + "columnName": "authType", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "imap.host", + "columnName": "imap_host", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "imap.port", + "columnName": "imap_port", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "imap.security", + "columnName": "imap_security", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "smtp.host", + "columnName": "smtp_host", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "smtp.port", + "columnName": "smtp_port", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "smtp.security", + "columnName": "smtp_security", + "affinity": "TEXT", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "credentials", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `encryptedSecret` TEXT NOT NULL, PRIMARY KEY(`accountId`))", + "fields": [ + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "encryptedSecret", + "columnName": "encryptedSecret", + "affinity": "TEXT", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId" + ] + } + }, + { + "tableName": "account_settings", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `signature` TEXT NOT NULL, `signatureEnabled` INTEGER NOT NULL, `notificationsEnabled` INTEGER NOT NULL, `retentionCount` INTEGER, `retentionMonths` INTEGER, PRIMARY KEY(`accountId`), FOREIGN KEY(`accountId`) REFERENCES `accounts`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "signature", + "columnName": "signature", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "signatureEnabled", + "columnName": "signatureEnabled", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "notificationsEnabled", + "columnName": "notificationsEnabled", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "retentionCount", + "columnName": "retentionCount", + "affinity": "INTEGER" + }, + { + "fieldPath": "retentionMonths", + "columnName": "retentionMonths", + "affinity": "INTEGER" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId" + ] + }, + "foreignKeys": [ + { + "table": "accounts", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "accountId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "signatures", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, `name` TEXT NOT NULL, `contentHtml` TEXT NOT NULL, `isDefault` INTEGER NOT NULL, PRIMARY KEY(`id`), FOREIGN KEY(`accountId`) REFERENCES `accounts`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "contentHtml", + "columnName": "contentHtml", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "isDefault", + "columnName": "isDefault", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_signatures_accountId", + "unique": false, + "columnNames": [ + "accountId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_signatures_accountId` ON `${TABLE_NAME}` (`accountId`)" + } + ], + "foreignKeys": [ + { + "table": "accounts", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "accountId" + ], + "referencedColumns": [ + "id" + ] + } + ] + } + ], + "setupQueries": [ + "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'f2bbe80e572de72f50869b14aca4c4bb')" + ] + } +} \ No newline at end of file diff --git a/app/schemas/org.libremail.data.local.LibreMailDatabase/16.json b/app/schemas/org.libremail.data.local.LibreMailDatabase/16.json new file mode 100644 index 0000000..c88d55d --- /dev/null +++ b/app/schemas/org.libremail.data.local.LibreMailDatabase/16.json @@ -0,0 +1,455 @@ +{ + "formatVersion": 1, + "database": { + "version": 16, + "identityHash": "b5c1a38d197cf1335d3092e413d55d0d", + "entities": [ + { + "tableName": "messages", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, `sender` TEXT NOT NULL, `senderEmail` TEXT NOT NULL, `subject` TEXT NOT NULL, `snippet` TEXT NOT NULL, `body` TEXT NOT NULL, `isHtml` INTEGER NOT NULL, `timestampMillis` INTEGER NOT NULL, `isRead` INTEGER NOT NULL, `isStarred` INTEGER NOT NULL, `folder` TEXT NOT NULL DEFAULT 'INBOX', `inInbox` INTEGER NOT NULL, `bodyFetched` INTEGER NOT NULL, `uid` INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "sender", + "columnName": "sender", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "senderEmail", + "columnName": "senderEmail", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "subject", + "columnName": "subject", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "snippet", + "columnName": "snippet", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "body", + "columnName": "body", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "isHtml", + "columnName": "isHtml", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "timestampMillis", + "columnName": "timestampMillis", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "isRead", + "columnName": "isRead", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "isStarred", + "columnName": "isStarred", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "folder", + "columnName": "folder", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "'INBOX'" + }, + { + "fieldPath": "inInbox", + "columnName": "inInbox", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "bodyFetched", + "columnName": "bodyFetched", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "uid", + "columnName": "uid", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_messages_accountId", + "unique": false, + "columnNames": [ + "accountId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_messages_accountId` ON `${TABLE_NAME}` (`accountId`)" + }, + { + "name": "index_messages_timestampMillis", + "unique": false, + "columnNames": [ + "timestampMillis" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_messages_timestampMillis` ON `${TABLE_NAME}` (`timestampMillis`)" + }, + { + "name": "index_messages_accountId_folder_uid", + "unique": false, + "columnNames": [ + "accountId", + "folder", + "uid" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_messages_accountId_folder_uid` ON `${TABLE_NAME}` (`accountId`, `folder`, `uid`)" + } + ] + }, + { + "tableName": "attachments", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`messageId` TEXT NOT NULL, `partIndex` INTEGER NOT NULL, `filename` TEXT NOT NULL, `mimeType` TEXT NOT NULL, `sizeBytes` INTEGER NOT NULL, PRIMARY KEY(`messageId`, `partIndex`), FOREIGN KEY(`messageId`) REFERENCES `messages`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "messageId", + "columnName": "messageId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "partIndex", + "columnName": "partIndex", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "filename", + "columnName": "filename", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "mimeType", + "columnName": "mimeType", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "sizeBytes", + "columnName": "sizeBytes", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "messageId", + "partIndex" + ] + }, + "indices": [ + { + "name": "index_attachments_messageId", + "unique": false, + "columnNames": [ + "messageId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_attachments_messageId` ON `${TABLE_NAME}` (`messageId`)" + } + ], + "foreignKeys": [ + { + "table": "messages", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "messageId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "outbox", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, `toAddresses` TEXT NOT NULL, `ccAddresses` TEXT NOT NULL, `bccAddresses` TEXT NOT NULL DEFAULT '', `subject` TEXT NOT NULL, `body` TEXT NOT NULL, `createdAt` INTEGER NOT NULL, `lastError` TEXT, `bodyHtml` TEXT, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "toAddresses", + "columnName": "toAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "ccAddresses", + "columnName": "ccAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "bccAddresses", + "columnName": "bccAddresses", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "subject", + "columnName": "subject", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "body", + "columnName": "body", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "createdAt", + "columnName": "createdAt", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "lastError", + "columnName": "lastError", + "affinity": "TEXT" + }, + { + "fieldPath": "bodyHtml", + "columnName": "bodyHtml", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "drafts", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT, `toAddresses` TEXT NOT NULL, `ccAddresses` TEXT NOT NULL, `bccAddresses` TEXT NOT NULL DEFAULT '', `subject` TEXT NOT NULL, `body` TEXT NOT NULL, `updatedAt` INTEGER NOT NULL, `attachments` TEXT NOT NULL, `bodyHtml` TEXT, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT" + }, + { + "fieldPath": "toAddresses", + "columnName": "toAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "ccAddresses", + "columnName": "ccAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "bccAddresses", + "columnName": "bccAddresses", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "subject", + "columnName": "subject", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "body", + "columnName": "body", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "updatedAt", + "columnName": "updatedAt", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "attachments", + "columnName": "attachments", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "bodyHtml", + "columnName": "bodyHtml", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "folders", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `fullName` TEXT NOT NULL, `displayName` TEXT NOT NULL, `role` TEXT NOT NULL, `selectable` INTEGER NOT NULL, `sortOrder` INTEGER NOT NULL, `specialUse` INTEGER NOT NULL DEFAULT 0, `hierarchyDelimiter` TEXT, PRIMARY KEY(`accountId`, `fullName`))", + "fields": [ + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "fullName", + "columnName": "fullName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "displayName", + "columnName": "displayName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "role", + "columnName": "role", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "selectable", + "columnName": "selectable", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "sortOrder", + "columnName": "sortOrder", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "specialUse", + "columnName": "specialUse", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + }, + { + "fieldPath": "hierarchyDelimiter", + "columnName": "hierarchyDelimiter", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId", + "fullName" + ] + } + }, + { + "tableName": "backfill_progress", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `folder` TEXT NOT NULL, `nextBeforeUid` INTEGER NOT NULL, `complete` INTEGER NOT NULL, PRIMARY KEY(`accountId`, `folder`))", + "fields": [ + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "folder", + "columnName": "folder", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "nextBeforeUid", + "columnName": "nextBeforeUid", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "complete", + "columnName": "complete", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId", + "folder" + ] + } + } + ], + "setupQueries": [ + "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'b5c1a38d197cf1335d3092e413d55d0d')" + ] + } +} \ No newline at end of file diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/AccountDataMigratorTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/AccountDataMigratorTest.kt new file mode 100644 index 0000000..eda28f0 --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/data/local/AccountDataMigratorTest.kt @@ -0,0 +1,265 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local + +import android.content.Context +import androidx.room.Room +import androidx.room.testing.MigrationTestHelper +import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import org.json.JSONObject +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.libremail.data.local.entity.CredentialEntity + +/** + * The one-time move performed by [AccountDataMigrator] (issue #111): copying accounts / credentials / + * per-account settings / signatures out of the cache database into the plaintext [AccountDatabase]. + * + * Exercises the [AccountDataMigrator.copyAccountTables] core directly (the full [AccountDataMigrator] + * also resolves the passphrase and flips the done-flag, which need the real DataStore/Keystore). A v14 + * cache is built with [MigrationTestHelper] from the exported schema, so the copy runs against exactly + * the on-disk shape an upgrading user has. + */ +@RunWith(AndroidJUnit4::class) +class AccountDataMigratorTest { + + @get:Rule + val helper = MigrationTestHelper( + InstrumentationRegistry.getInstrumentation(), + LibreMailDatabase::class.java, + emptyList(), + FrameworkSQLiteOpenHelperFactory(), + ) + + private val context = ApplicationProvider.getApplicationContext() + private val cacheName = "acct-migrator-cache-test.db" + private val accountsName = "acct-migrator-accounts-test.db" + private val cacheFile get() = context.getDatabasePath(cacheName) + private val accountsFile get() = context.getDatabasePath(accountsName) + + // 64 hex chars == a 32-byte SQLCipher passphrase. + private val passphrase = "0123456789abcdef".repeat(4) + + @Before + @After + fun clean() { + listOf(cacheName, accountsName).forEach { name -> + context.deleteDatabase(name) + context.getDatabasePath(name).parentFile + ?.listFiles { f -> f.name.startsWith(name) } + ?.forEach { it.delete() } + } + } + + /** Builds a v14 cache holding one fully-populated account plus a mail row. */ + private fun seedVersion14Cache() { + helper.createDatabase(cacheName, 14).apply { + execSQL( + "INSERT INTO accounts (id, email, displayName, authType, imap_host, imap_port, imap_security, " + + "smtp_host, smtp_port, smtp_security) VALUES ('acct', 'ada@example.org', 'Ada', " + + "'PASSWORD_IMAP', 'imap.example.org', 993, 'SSL_TLS', 'smtp.example.org', 465, 'SSL_TLS')", + ) + execSQL("INSERT INTO credentials (accountId, encryptedSecret) VALUES ('acct', 'sealed-secret')") + execSQL( + "INSERT INTO account_settings (accountId, signature, signatureEnabled, notificationsEnabled, " + + "retentionCount, retentionMonths) VALUES ('acct', 'Cheers', 1, 0, NULL, 6)", + ) + execSQL( + "INSERT INTO signatures (id, accountId, name, contentHtml, isDefault) " + + "VALUES ('sig-1', 'acct', 'Work', '

Regards

', 1)", + ) + execSQL( + "INSERT INTO messages (id, accountId, sender, senderEmail, subject, snippet, body, isHtml, " + + "timestampMillis, isRead, isStarred, folder, inInbox, bodyFetched, uid) VALUES " + + "('acct:INBOX:1', 'acct', 'Ada', 'a@x', 'Hi', '', '', 0, 1, 0, 0, 'INBOX', 1, 0, 1)", + ) + close() + } + } + + private fun openAccountsDb(): AccountDatabase = + Room.databaseBuilder(context, AccountDatabase::class.java, accountsName).build() + + @Test + fun movesEveryAccountTableOutOfAPlaintextCache() = runBlocking { + seedVersion14Cache() + + AccountDataMigrator.copyAccountTables(cacheFile, cachePassphrase = "", accountsFile = accountsFile) + + // First open lets Room stamp its identity onto the migrator-created file; reopen so a real + // session's reads run against a fully Room-owned database. + openAccountsDb().apply { + assertEquals("Ada", accountDao().getById("acct")?.displayName) + close() + } + openAccountsDb().apply { + val account = accountDao().getById("acct") + assertEquals("ada@example.org", account?.email) + assertEquals(993, account?.imap?.port) + assertEquals("smtp.example.org", account?.smtp?.host) + assertEquals("sealed-secret", credentialDao().getById("acct")?.encryptedSecret) + val settings = accountSettingsDao().get("acct") + // Seeded signatureEnabled = 1, notificationsEnabled = 0: both booleans must round-trip. + assertEquals(true, settings?.signatureEnabled) + assertEquals(false, settings?.notificationsEnabled) + assertEquals(6, settings?.retentionMonths) + assertNull(settings?.retentionCount) + val signatures = signatureDao().observeForAccount("acct").first() + assertEquals(listOf("Work"), signatures.map { it.name }) + assertTrue("the default flag must round-trip", signatures.single().isDefault) + close() + } + } + + @Test + fun movesAccountsOutOfAnEncryptedCache() = runBlocking { + seedVersion14Cache() + // Turn the cache into the SQLCipher form an app-lock + encrypted-cache user has on disk. + DatabaseEncryption.ensureEncrypted(cacheFile, passphrase) + assertTrue("precondition: the source cache is encrypted", DatabaseEncryption.isEncrypted(cacheFile)) + + AccountDataMigrator.copyAccountTables(cacheFile, cachePassphrase = passphrase, accountsFile = accountsFile) + + openAccountsDb().apply { + assertNotNull(accountDao().getById("acct")) + close() + } + openAccountsDb().apply { + assertEquals("ada@example.org", accountDao().getById("acct")?.email) + assertEquals("sealed-secret", credentialDao().getById("acct")?.encryptedSecret) + close() + } + } + + @Test + fun reRunningTheCopyIsIdempotentAndKeepsLaterEdits() = runBlocking { + seedVersion14Cache() + AccountDataMigrator.copyAccountTables(cacheFile, cachePassphrase = "", accountsFile = accountsFile) + + // Simulate the user editing an account AFTER the migration. + openAccountsDb().apply { + val edited = accountDao().getById("acct")!!.copy(displayName = "Ada Lovelace") + accountDao().upsert(edited) + close() + } + + // A re-run (e.g. after a mid-startup crash before the done-flag was set) must not clobber it. + AccountDataMigrator.copyAccountTables(cacheFile, cachePassphrase = "", accountsFile = accountsFile) + + openAccountsDb().apply { + assertEquals(1, accountDao().getAll().size) + assertEquals( + "INSERT OR IGNORE must not overwrite the post-migration edit", + "Ada Lovelace", + accountDao().getById("acct")?.displayName, + ) + close() + } + } + + @Test + fun accountsAndCredentialsSurviveACacheWipe() = runBlocking { + seedVersion14Cache() + AccountDataMigrator.copyAccountTables(cacheFile, cachePassphrase = "", accountsFile = accountsFile) + + // The "clear + re-sync" recovery wipes only the cache file; AccountDatabase is a separate file. + context.deleteDatabase(cacheName) + assertTrue("precondition: the cache file is gone", !cacheFile.exists()) + + openAccountsDb().apply { + assertNotNull("the account must outlive a cache wipe (issue #111)", accountDao().getById("acct")) + assertEquals("sealed-secret", credentialDao().getById("acct")?.encryptedSecret) + // And it is still usable: a fresh credential can be written with no cache present. + credentialDao().upsert(CredentialEntity("acct", "rotated")) + assertEquals("rotated", credentialDao().getById("acct")?.encryptedSecret) + close() + } + } + + @Test + fun copiesFromACacheOlderThanTheCurrentSchema() = runBlocking { + // A cache last written at v12 — before account_settings gained retentionCount/retentionMonths + // (v13). The copy must not choke on the columns the destination has but the source lacks + // (a device upgrade from an old install crashed the migrator here). + helper.createDatabase(cacheName, 12).apply { + execSQL( + "INSERT INTO accounts (id, email, displayName, authType, imap_host, imap_port, imap_security, " + + "smtp_host, smtp_port, smtp_security) VALUES ('acct', 'ada@example.org', 'Ada', " + + "'PASSWORD_IMAP', 'imap.example.org', 993, 'SSL_TLS', 'smtp.example.org', 465, 'SSL_TLS')", + ) + execSQL("INSERT INTO credentials (accountId, encryptedSecret) VALUES ('acct', 'sealed-secret')") + execSQL( + "INSERT INTO account_settings (accountId, signature, signatureEnabled, notificationsEnabled) " + + "VALUES ('acct', 'Sig', 0, 1)", + ) + close() + } + + AccountDataMigrator.copyAccountTables(cacheFile, cachePassphrase = "", accountsFile = accountsFile) + + openAccountsDb().apply { + assertEquals("ada@example.org", accountDao().getById("acct")?.email) + assertEquals("sealed-secret", credentialDao().getById("acct")?.encryptedSecret) + val settings = accountSettingsDao().get("acct") + assertEquals(false, settings?.signatureEnabled) + assertEquals(true, settings?.notificationsEnabled) + // Columns the v12 source lacked come across as the destination's defaults (null). + assertNull("retentionCount absent from a v12 cache must default to null", settings?.retentionCount) + assertNull(settings?.retentionMonths) + close() + } + } + + @Test + fun migratorDdlMatchesExportedAccountDatabaseSchema() { + val schema = JSONObject( + InstrumentationRegistry.getInstrumentation().context.assets + .open("org.libremail.data.local.AccountDatabase/1.json") + .bufferedReader().use { it.readText() }, + ).getJSONObject("database") + val entities = schema.getJSONArray("entities") + + var checkedIndex = false + for (i in 0 until entities.length()) { + val entity = entities.getJSONObject(i) + val table = entity.getString("tableName") + val expectedCreate = entity.getString("createSql").replace("\${TABLE_NAME}", table) + assertEquals( + "AccountDataMigrator DDL for `$table` must match the exported AccountDatabase schema", + expectedCreate, + AccountDataMigrator.CREATE_TABLE_SQL[table], + ) + if (entity.has("indices")) { + val indices = entity.getJSONArray("indices") + for (j in 0 until indices.length()) { + val index = indices.getJSONObject(j) + if (index.getString("name") == "index_signatures_accountId") { + assertEquals( + "AccountDataMigrator signatures index must match the exported schema", + index.getString("createSql").replace("\${TABLE_NAME}", table), + AccountDataMigrator.SIGNATURES_INDEX_SQL, + ) + checkedIndex = true + } + } + } + } + assertEquals( + "every migrator table DDL must correspond to an exported entity", + AccountDataMigrator.CREATE_TABLE_SQL.keys, + (0 until entities.length()).map { entities.getJSONObject(it).getString("tableName") }.toSet(), + ) + assertTrue("the signatures index must be present in the exported schema", checkedIndex) + } +} diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/AccountDatabaseTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/AccountDatabaseTest.kt new file mode 100644 index 0000000..c8131c5 --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/data/local/AccountDatabaseTest.kt @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local + +import android.content.Context +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.libremail.data.local.entity.AccountEntity +import org.libremail.data.local.entity.AccountSettingsEntity +import org.libremail.data.local.entity.CredentialEntity +import org.libremail.data.local.entity.ServerConfigEmbedded +import org.libremail.data.local.entity.SignatureEntity + +/** + * Behavior of the non-auth [AccountDatabase] that holds accounts, credentials, per-account settings + * and signatures after they were moved out of the cache database (issue #111). Confirms the tables' + * foreign keys still cascade from the account, now that they live together in this database. + */ +@RunWith(AndroidJUnit4::class) +class AccountDatabaseTest { + + private lateinit var db: AccountDatabase + + @Before + fun setUp() { + val context = ApplicationProvider.getApplicationContext() + db = Room.inMemoryDatabaseBuilder(context, AccountDatabase::class.java).build() + } + + @After + fun tearDown() = db.close() + + private fun account(id: String = "acct") = AccountEntity( + id = id, + email = "a@example.org", + displayName = "A", + authType = "PASSWORD_IMAP", + imap = ServerConfigEmbedded("imap.example.org", 993, "SSL_TLS"), + smtp = ServerConfigEmbedded("smtp.example.org", 465, "SSL_TLS"), + ) + + @Test + fun credentialRoundTripsAndIsIndependentOfTheAccountRow() = runBlocking { + db.accountDao().upsert(account()) + db.credentialDao().upsert(CredentialEntity("acct", "sealed-secret")) + + assertEquals("sealed-secret", db.credentialDao().getById("acct")?.encryptedSecret) + } + + @Test + fun accountSettingsRoundTripAndCascadeWithTheirAccount() = runBlocking { + db.accountDao().upsert(account()) + db.accountSettingsDao().upsert( + AccountSettingsEntity("acct", signature = "Hi", signatureEnabled = false, notificationsEnabled = false), + ) + assertEquals("Hi", db.accountSettingsDao().get("acct")?.signature) + + db.accountDao().deleteById("acct") + + assertNull("account_settings must cascade-delete with its account", db.accountSettingsDao().get("acct")) + } + + @Test + fun signaturesCascadeWithTheirAccount() = runBlocking { + db.accountDao().upsert(account()) + db.signatureDao().upsert(SignatureEntity("sig-1", "acct", "Work", "

Regards

", isDefault = true)) + assertEquals(1, db.signatureDao().observeForAccount("acct").first().size) + + db.accountDao().deleteById("acct") + + assertTrue( + "signatures must cascade-delete with their account", + db.signatureDao().observeForAccount("acct").first().isEmpty(), + ) + } +} diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt index b4ed6cb..5879ba1 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt @@ -9,22 +9,21 @@ import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import org.junit.After import org.junit.Assert.assertEquals -import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith -import org.libremail.data.local.entity.AccountEntity -import org.libremail.data.local.entity.AccountSettingsEntity import org.libremail.data.local.entity.AttachmentEntity import org.libremail.data.local.entity.FolderEntity import org.libremail.data.local.entity.MessageEntity -import org.libremail.data.local.entity.ServerConfigEmbedded /** * Schema-behavior tests on a fresh in-memory database at the current version. The migration DDL * itself is exercised by [MigrationTest], which replays the schema chain exported to app/schemas. * (Migrations from before v7 predate schema export, so they can't be replayed there.) + * + * Account/credential/settings/signature behavior moved to [AccountDatabaseTest] with those tables + * (issue #111). */ @RunWith(AndroidJUnit4::class) class LibreMailDatabaseTest { @@ -97,30 +96,6 @@ class LibreMailDatabaseTest { ) } - @Test - fun accountSettingsRoundTripAndCascadeWithTheirAccount() = runBlocking { - val accountDao = db.accountDao() - val settingsDao = db.accountSettingsDao() - accountDao.upsert( - AccountEntity( - id = "acct", - email = "a@example.org", - displayName = "A", - authType = "PASSWORD_IMAP", - imap = ServerConfigEmbedded("imap.example.org", 993, "SSL_TLS"), - smtp = ServerConfigEmbedded("smtp.example.org", 465, "SSL_TLS"), - ), - ) - settingsDao.upsert( - AccountSettingsEntity("acct", signature = "Hi", signatureEnabled = false, notificationsEnabled = false), - ) - assertEquals("Hi", settingsDao.get("acct")?.signature) - - accountDao.deleteById("acct") - - assertNull("account_settings must cascade-delete with its account", settingsDao.get("acct")) - } - @Test fun searchRowsAreNotInboxAndAreCleared() = runBlocking { val messageDao = db.messageDao() diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/MigrationTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/MigrationTest.kt index 6716829..1d669db 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/MigrationTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/MigrationTest.kt @@ -133,6 +133,9 @@ class MigrationTest { open?.close() val stepDb = helper.runMigrationsAndValidate(TEST_DB, migration.endVersion, true, migration) stepDb.writeMidChainData() + // v16 moves the account tables out to AccountDatabase and drops them, so assert their rows + // and backfills reached v15 intact — just before the move (issue #111). + if (stepDb.version == 15) stepDb.assertAccountDataPresentAtV15() open = stepDb } val db = checkNotNull(open) { "no migration starts at v$OLDEST_EXPORTED_SCHEMA" } @@ -140,6 +143,29 @@ class MigrationTest { assertEquals("the chain must end at the newest exported schema", latestExportedSchemaVersion(), db.version) db.assertVersion7CacheSurvived() db.assertMigrationBackfillsApplied() + db.assertAccountTablesDroppedAtV16() + db.close() + } + + /** v15 -> v16 (issue #111): the moved account tables are dropped and the mail cache is untouched. */ + @Test + fun migrate15To16_dropsMovedAccountTablesAndKeepsCache() { + helper.createDatabase(TEST_DB, 15).apply { + insertAccount() + execSQL("INSERT INTO credentials (accountId, encryptedSecret) VALUES ('acct', 'sealed')") + execSQL( + "INSERT INTO messages (id, accountId, sender, senderEmail, subject, snippet, body, isHtml, " + + "timestampMillis, isRead, isStarred, folder, inInbox, bodyFetched, uid) VALUES " + + "('acct:INBOX:1', 'acct', 'Ada', 'ada@example.org', 'Hi', '', '', 0, 1000, 0, 0, " + + "'INBOX', 1, 0, 1)", + ) + close() + } + + val db = helper.runMigrationsAndValidate(TEST_DB, 16, true, MIGRATION_15_16) + + db.assertAccountTablesDroppedAtV16() + assertEquals("the mail cache must be untouched by 15->16", 1, db.count("messages")) db.close() } @@ -203,9 +229,8 @@ class MigrationTest { } } - /** Every row cached at v7 must still be present and correct at the end of the chain. */ + /** Every mail-cache row cached at v7 must survive to v16 (account tables are checked separately). */ private fun SupportSQLiteDatabase.assertVersion7CacheSurvived() { - assertEquals(1, count("accounts")) assertEquals(2, count("messages")) assertEquals(1, count("outbox")) assertEquals(1, count("drafts")) @@ -215,24 +240,14 @@ class MigrationTest { assertEquals("Analytical engines", c.getString(1)) assertEquals(1, c.getInt(2)) } - query("SELECT encryptedSecret FROM credentials WHERE accountId = 'acct'").use { c -> - assertTrue("stored credentials must never be dropped by a migration", c.moveToFirst()) - assertEquals("sealed-secret", c.getString(0)) - } query("SELECT filename FROM attachments WHERE messageId = 'acct:1'").use { c -> assertTrue("attachment rows must survive the 6->7 style table rebuilds", c.moveToFirst()) assertEquals("notes.pdf", c.getString(0)) } } - /** Columns and rows created by the migrations themselves must hold their documented defaults. */ + /** Cache-table columns/rows the migrations backfill must hold their documented defaults at v16. */ private fun SupportSQLiteDatabase.assertMigrationBackfillsApplied() { - // 8->9 backfills one default settings row per existing account. - query("SELECT signatureEnabled, notificationsEnabled FROM account_settings").use { c -> - assertTrue("8->9 must backfill a settings row for the v7 account", c.moveToFirst()) - assertEquals(1, c.getInt(0)) - assertEquals(1, c.getInt(1)) - } // 9->10 adds bcc columns defaulting to ''; 10->11 adds nullable bodyHtml. query("SELECT bccAddresses, bodyHtml FROM outbox WHERE id = 'out-1'").use { c -> assertTrue(c.moveToFirst()) @@ -244,14 +259,6 @@ class MigrationTest { assertEquals("", c.getString(0)) assertTrue(c.isNull(1)) } - // 10->11 turns the signature written at v9 into that account's default rich-text signature. - query("SELECT name, contentHtml, isDefault FROM signatures WHERE accountId = 'acct'").use { c -> - assertTrue("10->11 must backfill the legacy per-account signature", c.moveToFirst()) - assertEquals("Signature", c.getString(0)) - assertEquals("Cheers,
Ada", c.getString(1)) - assertEquals(1, c.getInt(2)) - assertFalse("exactly one signature row must be backfilled", c.moveToNext()) - } // 11->12 stamps the folder cached at v8 as not special-use. query("SELECT specialUse FROM folders WHERE fullName = 'INBOX'").use { c -> assertTrue("folder cached at v8 must survive to the newest version", c.moveToFirst()) @@ -265,6 +272,42 @@ class MigrationTest { } } + /** + * The account tables' rows + migration backfills must be intact at v15, just before 15->16 moves + * them to [AccountDatabase] and drops them (issue #111). AccountDataMigrator's own copy is + * exercised in `AccountDataMigratorTest`; here we only assert the source rows reach the move point. + */ + private fun SupportSQLiteDatabase.assertAccountDataPresentAtV15() { + assertEquals(1, count("accounts")) + query("SELECT encryptedSecret FROM credentials WHERE accountId = 'acct'").use { c -> + assertTrue("stored credentials must reach v15 before the move", c.moveToFirst()) + assertEquals("sealed-secret", c.getString(0)) + } + // 8->9 backfills one default settings row per existing account. + query("SELECT signatureEnabled, notificationsEnabled FROM account_settings").use { c -> + assertTrue("8->9 must backfill a settings row for the v7 account", c.moveToFirst()) + assertEquals(1, c.getInt(0)) + assertEquals(1, c.getInt(1)) + } + // 10->11 turns the signature written at v9 into that account's default rich-text signature. + query("SELECT name, contentHtml, isDefault FROM signatures WHERE accountId = 'acct'").use { c -> + assertTrue("10->11 must backfill the legacy per-account signature", c.moveToFirst()) + assertEquals("Signature", c.getString(0)) + assertEquals("Cheers,
Ada", c.getString(1)) + assertEquals(1, c.getInt(2)) + assertFalse("exactly one signature row must be backfilled", c.moveToNext()) + } + } + + /** 15->16 drops the account tables from the cache (AccountDataMigrator copies them out first). */ + private fun SupportSQLiteDatabase.assertAccountTablesDroppedAtV16() { + listOf("accounts", "credentials", "account_settings", "signatures").forEach { table -> + query("SELECT name FROM sqlite_master WHERE type = 'table' AND name = '$table'").use { c -> + assertFalse("15->16 must drop `$table` from the cache database", c.moveToFirst()) + } + } + } + private fun SupportSQLiteDatabase.count(table: String): Int = query("SELECT COUNT(*) FROM $table").use { c -> c.moveToFirst() c.getInt(0) diff --git a/app/src/androidTest/kotlin/org/libremail/ui/compose/ComposeScreenTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/compose/ComposeScreenTest.kt index 25355e6..bbd2bbf 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/compose/ComposeScreenTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/compose/ComposeScreenTest.kt @@ -26,7 +26,7 @@ import org.junit.Test import org.junit.runner.RunWith import org.libremail.R import org.libremail.contacts.ContactsRepository -import org.libremail.data.local.LibreMailDatabase +import org.libremail.data.local.AccountDatabase import org.libremail.data.settings.AccountSettingsRepository import org.libremail.data.settings.SignatureRepository import org.libremail.domain.model.Account @@ -56,7 +56,7 @@ class ComposeScreenTest { smtp = ServerConfig("smtp.example.com", 465, MailSecurity.SSL_TLS), ) - private var db: LibreMailDatabase? = null + private var db: AccountDatabase? = null private fun string(resId: Int) = composeTestRule.activity.getString(resId) @@ -79,7 +79,7 @@ class ComposeScreenTest { // Build the view model once and capture it, so recomposition doesn't recreate it. private fun setContent(mailRepository: FakeMailRepository = FakeMailRepository(), onBack: () -> Unit = {}) { val context = InstrumentationRegistry.getInstrumentation().targetContext.applicationContext - val database = Room.inMemoryDatabaseBuilder(context, LibreMailDatabase::class.java).build().also { db = it } + val database = Room.inMemoryDatabaseBuilder(context, AccountDatabase::class.java).build().also { db = it } val viewModel = ComposeViewModel( savedStateHandle = SavedStateHandle(), mailRepository = mailRepository, diff --git a/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt index 20e1173..85f90c4 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt @@ -16,7 +16,7 @@ import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.libremail.R -import org.libremail.data.local.LibreMailDatabase +import org.libremail.data.local.AccountDatabase import org.libremail.data.local.toEntity import org.libremail.data.settings.AccountSettingsRepository import org.libremail.data.settings.SignatureRepository @@ -59,7 +59,7 @@ class AccountSettingsScreenTest { // stateIn/WhileSubscribed) keeps querying after the test body, so closing the in-memory DB out // from under it races and crashes ("connection pool has been closed"). The DB is reclaimed with // the test process. - val db = Room.inMemoryDatabaseBuilder(context, LibreMailDatabase::class.java).build() + val db = Room.inMemoryDatabaseBuilder(context, AccountDatabase::class.java).build() val repository = AccountSettingsRepository(db.accountSettingsDao()) runBlocking { db.accountDao().upsert(account.toEntity()) // FK parent for the account_settings row diff --git a/app/src/main/kotlin/org/libremail/data/local/AccountDataMigrator.kt b/app/src/main/kotlin/org/libremail/data/local/AccountDataMigrator.kt new file mode 100644 index 0000000..521bc81 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/local/AccountDataMigrator.kt @@ -0,0 +1,221 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local + +import android.content.Context +import android.util.Log +import androidx.datastore.core.DataStore +import androidx.datastore.preferences.core.Preferences +import androidx.datastore.preferences.core.booleanPreferencesKey +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.preferencesDataStore +import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.withContext +import net.zetetic.database.sqlcipher.SQLiteDatabase +import org.libremail.data.security.DatabaseKeyStore +import org.libremail.data.settings.SettingsRepository +import java.io.File +import javax.inject.Inject +import javax.inject.Singleton + +private val Context.accountMigrationDataStore: DataStore by + preferencesDataStore(name = "libremail_account_migration") + +/** + * One-time, crash-safe move of the account tables (`accounts`, `credentials`, `account_settings`, + * `signatures`) out of the auth-bound cache database [LibreMailDatabase] into the non-auth + * [AccountDatabase] (issue #111). Runs at startup, driven by `DatabaseModule.provideDatabase`, BEFORE + * Room opens the cache and its [MIGRATION_15_16] drops the moved tables. + * + * ### Why not a Room migration + * The copy is cross-database, so it needs `ATTACH DATABASE`, which SQLite forbids inside the + * transaction Room wraps every migration in. It therefore runs here on a dedicated SQLCipher + * connection before Room opens either database. + * + * ### Handling the encrypted source + * When the opt-in encrypted cache is on, the source `libremail.db` is SQLCipher-encrypted. The + * caller resolves and hands us its passphrase (the same one Room uses to open it); we attach the + * cache with that passphrase and copy into a plaintext `libremail-accounts.db`. When the cache is + * plaintext the passphrase is empty. Reading the source's schema validates the passphrase, so a + * genuinely wrong key fails loudly here (the same open would fail in Room) rather than losing data. + * + * The unrecoverable-key case does not reach us: `provideDatabase` wipes an undecryptable cache (and + * resets its seals) BEFORE calling us, so we then see a fresh/empty cache with nothing to move — the + * accounts trapped in that already-invalidated cache are lost regardless (the pre-existing bug), but + * no future invalidation can strand them again once they live in [AccountDatabase]. + * + * ### Crash-safety & idempotency + * - We never drop the source here; [MIGRATION_15_16] does that after we return, so if we crash the + * source rows are still intact for the next attempt. + * - The copy uses `INSERT OR IGNORE`, so a re-run after a mid-copy crash converges (existing rows + * are skipped, never duplicated, and never overwrite anything the user changed post-migration). + * - The "done" flag is only set after a successful copy; until then every start retries. Once set we + * return immediately and never touch the cache passphrase again — so after migration the account + * database opens with no Keystore dependency at all. + */ +@Singleton +class AccountDataMigrator @Inject constructor( + @ApplicationContext private val context: Context, + private val keyStore: DatabaseKeyStore, + private val settingsRepository: SettingsRepository, +) { + + /** + * Copy the account tables into [AccountDatabase] if it has not been done yet. Idempotent and + * safe to call from every `provideDatabase` construction. Throws (rather than silently skipping) + * on an unexpected copy failure so the caller does not proceed to drop the source tables — a + * crash-loop that preserves data is strictly safer than a wipe that loses it. + */ + suspend fun migrateIfNeeded() { + if (isDone()) return + val cacheFile = context.getDatabasePath(DatabaseFiles.NAME) + if (cacheFile.exists() && cacheFile.length() > 0L) { + // Read the cache in its CURRENT on-disk form. `provideDatabase` runs us before it converts + // between plaintext and encrypted, so the key is empty unless the file is encrypted now. + val cacheKey = if (DatabaseEncryption.isEncrypted(cacheFile)) { + keyStore.resolvePassphrase(settingsRepository.settings.first().appLock) + } else { + "" + } + val accountsFile = context.getDatabasePath(DatabaseFiles.ACCOUNTS_NAME) + withContext(Dispatchers.IO) { copyAccountTables(cacheFile, cacheKey, accountsFile) } + } + markDone() + } + + private suspend fun isDone(): Boolean = context.accountMigrationDataStore.data.first()[DONE] == true + + private suspend fun markDone() { + context.accountMigrationDataStore.edit { it[DONE] = true } + } + + companion object { + private const val TAG = "LibreMailAcctMigrate" + private val DONE = booleanPreferencesKey("accounts_moved_out_of_cache") + + /** The account tables, parent before children so foreign keys never block an insert. */ + private val TABLES = listOf("accounts", "credentials", "account_settings", "signatures") + + /** + * DDL for the account tables in [AccountDatabase] v1, copied verbatim from the exported Room + * schema (`schemas/org.libremail.data.local.AccountDatabase/1.json`). It MUST stay byte-for-byte + * identical to what Room generates for those entities, or Room silently accepts a subtly wrong + * schema (its identity check only compares the hash it writes, not the pre-existing tables). + * `AccountDataMigratorTest.migratorDdlMatchesExportedAccountDatabaseSchema` guards it against the + * exported schema; `internal` only so that test can read it. + */ + internal val CREATE_TABLE_SQL = mapOf( + "accounts" to + "CREATE TABLE IF NOT EXISTS `accounts` (`id` TEXT NOT NULL, `email` TEXT NOT NULL, " + + "`displayName` TEXT NOT NULL, `authType` TEXT NOT NULL, `imap_host` TEXT NOT NULL, " + + "`imap_port` INTEGER NOT NULL, `imap_security` TEXT NOT NULL, `smtp_host` TEXT NOT NULL, " + + "`smtp_port` INTEGER NOT NULL, `smtp_security` TEXT NOT NULL, PRIMARY KEY(`id`))", + "credentials" to + "CREATE TABLE IF NOT EXISTS `credentials` (`accountId` TEXT NOT NULL, " + + "`encryptedSecret` TEXT NOT NULL, PRIMARY KEY(`accountId`))", + "account_settings" to + "CREATE TABLE IF NOT EXISTS `account_settings` (`accountId` TEXT NOT NULL, " + + "`signature` TEXT NOT NULL, `signatureEnabled` INTEGER NOT NULL, " + + "`notificationsEnabled` INTEGER NOT NULL, `retentionCount` INTEGER, " + + "`retentionMonths` INTEGER, PRIMARY KEY(`accountId`), " + + "FOREIGN KEY(`accountId`) REFERENCES `accounts`(`id`) " + + "ON UPDATE NO ACTION ON DELETE CASCADE )", + "signatures" to + "CREATE TABLE IF NOT EXISTS `signatures` (`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, " + + "`name` TEXT NOT NULL, `contentHtml` TEXT NOT NULL, `isDefault` INTEGER NOT NULL, " + + "PRIMARY KEY(`id`), FOREIGN KEY(`accountId`) REFERENCES `accounts`(`id`) " + + "ON UPDATE NO ACTION ON DELETE CASCADE )", + ) + + internal const val SIGNATURES_INDEX_SQL = + "CREATE INDEX IF NOT EXISTS `index_signatures_accountId` ON `signatures` (`accountId`)" + + /** + * Copies the account tables from [cacheFile] (opened with [cachePassphrase]; empty = plaintext) + * into a plaintext [accountsFile], creating the destination schema first. Opens the destination + * as `main` and attaches the (possibly encrypted) cache as `cache`, so a plaintext connection + * can still read the encrypted source via SQLCipher's per-attach key. Visible for the migrator + * test; call [migrateIfNeeded] in production. + */ + internal fun copyAccountTables(cacheFile: File, cachePassphrase: String, accountsFile: File) { + DatabaseEncryption.ensureNativeLibraryLoaded() + val db = SQLiteDatabase.openOrCreateDatabase( + accountsFile.absolutePath, + "".toByteArray(Charsets.US_ASCII), // destination is plaintext + null, + null, + ) + try { + // No WAL: keep the destination in rollback-journal mode (as DatabaseEncryption does) + // so that after close there is no -wal/-shm holding uncommitted rows for Room to miss. + db.rawExecSQL("PRAGMA journal_mode = DELETE;") + val keyLiteral = cachePassphrase.replace("'", "''") + val cachePath = cacheFile.absolutePath.replace("'", "''") + db.rawExecSQL("ATTACH DATABASE '$cachePath' AS cache KEY '$keyLiteral';") + try { + val present = presentTables(db) + if (present.isEmpty()) return // fresh cache or already dropped: nothing to move + TABLES.forEach { db.rawExecSQL(CREATE_TABLE_SQL.getValue(it)) } + db.rawExecSQL(SIGNATURES_INDEX_SQL) + // Copy by explicit shared column names, never SELECT *: the on-disk cache may predate + // columns the current schema added (e.g. account_settings gained retentionCount / + // retentionMonths at v13), and a bare SELECT * would then supply fewer values than the + // destination has columns and fail the whole migration. Listing the columns the source + // actually has lets the destination's newer columns take their defaults (NULL). Parent + // first so an enforced foreign key would still be satisfied; INSERT OR IGNORE is idempotent. + TABLES.filter { it in present }.forEach { table -> + val cols = sharedColumns(db, table) + db.rawExecSQL("INSERT OR IGNORE INTO `$table` ($cols) SELECT $cols FROM cache.`$table`") + } + Log.d(TAG, "moved account tables into the account database: $present") + } finally { + db.rawExecSQL("DETACH DATABASE cache;") + } + } finally { + db.close() + } + // Room opens the destination next; drop any sidecars the copy left so a stale WAL/SHM can't + // confuse its first open. + val dir = accountsFile.parentFile + if (dir != null) { + listOf("-wal", "-shm", "-journal").forEach { File(dir, accountsFile.name + it).delete() } + } + } + + private fun presentTables(db: SQLiteDatabase): Set { + val names = TABLES.joinToString(",") { "'$it'" } + val present = mutableSetOf() + db.rawQuery( + "SELECT name FROM cache.sqlite_master WHERE type = 'table' AND name IN ($names)", + null, + ).use { cursor -> + while (cursor.moveToNext()) present += cursor.getString(0) + } + return present + } + + /** + * Column names present in BOTH the freshly-created destination `$table` (always the current + * schema) and the source `cache.$table` (possibly an older on-disk schema), backtick-quoted and + * comma-joined for an INSERT/SELECT column list. Destination-only columns are omitted so they + * take their defaults instead of overflowing the value list. + */ + private fun sharedColumns(db: SQLiteDatabase, table: String): String { + val source = tableColumns(db, "cache", table) + return tableColumns(db, "main", table) + .filter { it in source } + .joinToString(", ") { "`$it`" } + } + + /** The column names of `$schema.$table`, in declared order, via `PRAGMA table_info`. */ + private fun tableColumns(db: SQLiteDatabase, schema: String, table: String): List { + val columns = mutableListOf() + db.rawQuery("PRAGMA $schema.table_info(`$table`)", null).use { cursor -> + val nameIndex = cursor.getColumnIndexOrThrow("name") + while (cursor.moveToNext()) columns += cursor.getString(nameIndex) + } + return columns + } + } +} diff --git a/app/src/main/kotlin/org/libremail/data/local/AccountDatabase.kt b/app/src/main/kotlin/org/libremail/data/local/AccountDatabase.kt new file mode 100644 index 0000000..cdf4e47 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/local/AccountDatabase.kt @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local + +import androidx.room.Database +import androidx.room.RoomDatabase +import org.libremail.data.local.dao.AccountDao +import org.libremail.data.local.dao.AccountSettingsDao +import org.libremail.data.local.dao.CredentialDao +import org.libremail.data.local.dao.SignatureDao +import org.libremail.data.local.entity.AccountEntity +import org.libremail.data.local.entity.AccountSettingsEntity +import org.libremail.data.local.entity.CredentialEntity +import org.libremail.data.local.entity.SignatureEntity + +/** + * Durable store for the pieces of an account that must survive a mail-cache wipe (issue #111): the + * account itself, its sealed credential, per-account settings, and saved signatures. + * + * This lives in its OWN database file ([DatabaseFiles.ACCOUNTS_NAME]) that is deliberately NEVER + * bound to the auth-bound SQLCipher key. When app-lock + encrypted-cache are on and that key is + * invalidated (a genuine biometric re-enrollment or lock removal/re-add), only the mail cache + * ([LibreMailDatabase]) becomes undecryptable and is wiped; this database is untouched, so the user + * stays signed in instead of being dropped back into onboarding. + * + * It is plaintext on disk. The only secret it holds is [CredentialEntity.encryptedSecret], which is + * already AES-GCM ciphertext sealed at the column level by the non-auth + * [org.libremail.data.security.KeystoreCrypto] master key (and that key survives an auth-key + * invalidation), so the secret never touches disk in the clear regardless of this file's own + * encryption. Account metadata (email address, server hosts) is not a secret. Keeping the file + * plaintext is what makes it maximally resilient — it can always be opened without any Keystore key, + * so no key invalidation can ever strand it. + * + * Existing installs are migrated into this database once, at startup, by [AccountDataMigrator] + * before [MIGRATION_15_16] drops the moved tables from the cache database. + */ +@Database( + entities = [ + AccountEntity::class, + CredentialEntity::class, + AccountSettingsEntity::class, + SignatureEntity::class, + ], + version = 1, + exportSchema = true, +) +abstract class AccountDatabase : RoomDatabase() { + abstract fun accountDao(): AccountDao + abstract fun credentialDao(): CredentialDao + abstract fun accountSettingsDao(): AccountSettingsDao + abstract fun signatureDao(): SignatureDao +} diff --git a/app/src/main/kotlin/org/libremail/data/local/DatabaseEncryption.kt b/app/src/main/kotlin/org/libremail/data/local/DatabaseEncryption.kt index 515944c..6694a2c 100644 --- a/app/src/main/kotlin/org/libremail/data/local/DatabaseEncryption.kt +++ b/app/src/main/kotlin/org/libremail/data/local/DatabaseEncryption.kt @@ -40,7 +40,7 @@ object DatabaseEncryption { * tables but not that pragma, and a reset version would make Room attempt a bogus migration. */ private fun migrate(dbFile: File, sourcePassphrase: String, targetPassphrase: String) { - ensureLibraryLoaded() + ensureNativeLibraryLoaded() val dir = dbFile.parentFile ?: error("database file has no parent directory") val tmp = File(dir, dbFile.name + ".migrate").apply { delete() } @@ -98,7 +98,13 @@ object DatabaseEncryption { } @Volatile private var libraryLoaded = false - private fun ensureLibraryLoaded() { + + /** + * Load SQLCipher's native library once. Public so other startup helpers that open a database via + * [net.zetetic.database.sqlcipher.SQLiteDatabase] before Room does (e.g. [AccountDataMigrator]) + * can guarantee it is loaded first. + */ + fun ensureNativeLibraryLoaded() { if (libraryLoaded) return synchronized(this) { if (!libraryLoaded) { diff --git a/app/src/main/kotlin/org/libremail/data/local/DatabaseFiles.kt b/app/src/main/kotlin/org/libremail/data/local/DatabaseFiles.kt index 0b18f42..4e52837 100644 --- a/app/src/main/kotlin/org/libremail/data/local/DatabaseFiles.kt +++ b/app/src/main/kotlin/org/libremail/data/local/DatabaseFiles.kt @@ -9,6 +9,13 @@ object DatabaseFiles { const val NAME = "libremail.db" + /** + * The [org.libremail.data.local.AccountDatabase] file — accounts, credentials, per-account + * settings and signatures. Deliberately a DIFFERENT file from [NAME] and NEVER wiped by [clear], + * so a cache-key invalidation keeps the user signed in (issue #111). + */ + const val ACCOUNTS_NAME = "libremail-accounts.db" + /** * Delete the database and any WAL/SHM/journal sidecars. Call only when no connection is open — * used by the "clear + re-sync" path when the encryption key is invalidated and the encrypted diff --git a/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt b/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt index 24bf856..567ad08 100644 --- a/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt +++ b/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt @@ -3,52 +3,46 @@ package org.libremail.data.local import androidx.room.Database import androidx.room.RoomDatabase -import org.libremail.data.local.dao.AccountDao -import org.libremail.data.local.dao.AccountSettingsDao import org.libremail.data.local.dao.AttachmentDao import org.libremail.data.local.dao.BackfillProgressDao -import org.libremail.data.local.dao.CredentialDao import org.libremail.data.local.dao.DraftDao import org.libremail.data.local.dao.FolderDao import org.libremail.data.local.dao.MessageDao import org.libremail.data.local.dao.OutboxDao -import org.libremail.data.local.dao.SignatureDao -import org.libremail.data.local.entity.AccountEntity -import org.libremail.data.local.entity.AccountSettingsEntity import org.libremail.data.local.entity.AttachmentEntity import org.libremail.data.local.entity.BackfillProgressEntity -import org.libremail.data.local.entity.CredentialEntity import org.libremail.data.local.entity.DraftEntity import org.libremail.data.local.entity.FolderEntity import org.libremail.data.local.entity.MessageEntity import org.libremail.data.local.entity.OutboxEntity -import org.libremail.data.local.entity.SignatureEntity +/** + * The offline mail cache. Everything here is re-derivable from the server on a fresh sync, so it is + * the database that opt-in SQLCipher encryption is applied to and — when the auth-bound key is + * invalidated — the one that "clear + re-sync" wipes. + * + * Account identity and user configuration (accounts, credentials, per-account settings, signatures) + * are deliberately NOT here: they live in [AccountDatabase], a separate non-auth-bound file, so a + * cache-key invalidation can never sign the user out (issue #111). [MIGRATION_15_16] dropped those + * tables from this database; [AccountDataMigrator] copies existing rows into [AccountDatabase] first. + */ @Database( entities = [ - AccountEntity::class, - AccountSettingsEntity::class, MessageEntity::class, - CredentialEntity::class, AttachmentEntity::class, OutboxEntity::class, DraftEntity::class, FolderEntity::class, - SignatureEntity::class, BackfillProgressEntity::class, ], - version = 15, + version = 16, exportSchema = true, ) abstract class LibreMailDatabase : RoomDatabase() { abstract fun messageDao(): MessageDao - abstract fun accountDao(): AccountDao - abstract fun accountSettingsDao(): AccountSettingsDao - abstract fun credentialDao(): CredentialDao abstract fun attachmentDao(): AttachmentDao abstract fun outboxDao(): OutboxDao abstract fun draftDao(): DraftDao abstract fun folderDao(): FolderDao - abstract fun signatureDao(): SignatureDao abstract fun backfillProgressDao(): BackfillProgressDao } diff --git a/app/src/main/kotlin/org/libremail/data/local/Migrations.kt b/app/src/main/kotlin/org/libremail/data/local/Migrations.kt index 1b3a5f5..feadf8a 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Migrations.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Migrations.kt @@ -308,3 +308,25 @@ val MIGRATION_14_15 = object : Migration(14, 15) { db.execSQL("ALTER TABLE `folders` ADD COLUMN `hierarchyDelimiter` TEXT") } } + +/** + * v15 -> v16: move account identity + configuration OUT of the cache database (issue #111). The + * `accounts`, `credentials`, `account_settings` and `signatures` tables now live in [AccountDatabase] + * — a separate file that is never sealed by the auth-bound SQLCipher key — so a cache-key invalidation + * (biometric re-enrollment / lock removal) wipes only mail and can no longer sign the user out. + * + * The rows are copied into [AccountDatabase] by [AccountDataMigrator] at startup BEFORE Room opens the + * cache and runs this migration. The copy CANNOT happen here: Room wraps each migration in a + * transaction and SQLite forbids `ATTACH DATABASE` inside one, so a cross-database copy has to run on + * a separate connection before the cache is opened. This migration therefore only drops the tables + * that were moved. `DROP TABLE IF EXISTS` keeps it idempotent, and children (foreign-keyed to + * `accounts`) are dropped before the parent so the drop never trips a foreign-key check. + */ +val MIGRATION_15_16 = object : Migration(15, 16) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("DROP TABLE IF EXISTS `signatures`") + db.execSQL("DROP TABLE IF EXISTS `account_settings`") + db.execSQL("DROP TABLE IF EXISTS `credentials`") + db.execSQL("DROP TABLE IF EXISTS `accounts`") + } +} diff --git a/app/src/main/kotlin/org/libremail/di/AccountDatabaseModule.kt b/app/src/main/kotlin/org/libremail/di/AccountDatabaseModule.kt new file mode 100644 index 0000000..435f929 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/di/AccountDatabaseModule.kt @@ -0,0 +1,53 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.di + +import android.content.Context +import androidx.room.Room +import dagger.Module +import dagger.Provides +import dagger.hilt.InstallIn +import dagger.hilt.android.qualifiers.ApplicationContext +import dagger.hilt.components.SingletonComponent +import org.libremail.data.local.AccountDatabase +import org.libremail.data.local.DatabaseFiles.ACCOUNTS_NAME +import org.libremail.data.local.LibreMailDatabase +import org.libremail.data.local.dao.AccountDao +import org.libremail.data.local.dao.AccountSettingsDao +import org.libremail.data.local.dao.CredentialDao +import org.libremail.data.local.dao.SignatureDao +import javax.inject.Singleton + +/** + * Hilt wiring for [AccountDatabase] — the non-auth-bound store for accounts, credentials, per-account + * settings and signatures (issue #111). Kept separate from [DatabaseModule] so each database's + * provides stay cohesive (and neither module grows past detekt's per-object function limit). + */ +@Module +@InstallIn(SingletonComponent::class) +object AccountDatabaseModule { + + /** + * The plaintext account store. Depends on [LibreMailDatabase] purely for construction ordering: + * building the cache runs the one-time [org.libremail.data.local.AccountDataMigrator] (which + * populates this file on a dedicated connection) and then drops the moved tables, so by the time + * Room opens this file the data is already present and no other connection is touching it. + */ + @Provides + @Singleton + fun provideAccountDatabase( + @ApplicationContext context: Context, + @Suppress("UNUSED_PARAMETER") cacheDatabase: LibreMailDatabase, + ): AccountDatabase = Room.databaseBuilder(context, AccountDatabase::class.java, ACCOUNTS_NAME).build() + + @Provides + fun provideAccountDao(database: AccountDatabase): AccountDao = database.accountDao() + + @Provides + fun provideCredentialDao(database: AccountDatabase): CredentialDao = database.credentialDao() + + @Provides + fun provideAccountSettingsDao(database: AccountDatabase): AccountSettingsDao = database.accountSettingsDao() + + @Provides + fun provideSignatureDao(database: AccountDatabase): SignatureDao = database.signatureDao() +} diff --git a/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt b/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt index 792e3ef..d247ad1 100644 --- a/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt +++ b/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt @@ -11,6 +11,7 @@ import dagger.hilt.components.SingletonComponent import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import net.zetetic.database.sqlcipher.SupportOpenHelperFactory +import org.libremail.data.local.AccountDataMigrator import org.libremail.data.local.DatabaseEncryption import org.libremail.data.local.DatabaseFiles import org.libremail.data.local.LibreMailDatabase @@ -19,6 +20,7 @@ import org.libremail.data.local.MIGRATION_11_12 import org.libremail.data.local.MIGRATION_12_13 import org.libremail.data.local.MIGRATION_13_14 import org.libremail.data.local.MIGRATION_14_15 +import org.libremail.data.local.MIGRATION_15_16 import org.libremail.data.local.MIGRATION_1_2 import org.libremail.data.local.MIGRATION_2_3 import org.libremail.data.local.MIGRATION_3_4 @@ -28,16 +30,12 @@ import org.libremail.data.local.MIGRATION_6_7 import org.libremail.data.local.MIGRATION_7_8 import org.libremail.data.local.MIGRATION_8_9 import org.libremail.data.local.MIGRATION_9_10 -import org.libremail.data.local.dao.AccountDao -import org.libremail.data.local.dao.AccountSettingsDao import org.libremail.data.local.dao.AttachmentDao import org.libremail.data.local.dao.BackfillProgressDao -import org.libremail.data.local.dao.CredentialDao import org.libremail.data.local.dao.DraftDao import org.libremail.data.local.dao.FolderDao import org.libremail.data.local.dao.MessageDao import org.libremail.data.local.dao.OutboxDao -import org.libremail.data.local.dao.SignatureDao import org.libremail.data.security.DatabaseKeyStore import org.libremail.data.settings.SettingsRepository import javax.inject.Singleton @@ -52,6 +50,7 @@ object DatabaseModule { @ApplicationContext context: Context, keyStore: DatabaseKeyStore, settingsRepository: SettingsRepository, + accountDataMigrator: AccountDataMigrator, ): LibreMailDatabase { val builder = Room.databaseBuilder(context, LibreMailDatabase::class.java, DB_NAME) .addMigrations( @@ -69,10 +68,11 @@ object DatabaseModule { MIGRATION_12_13, MIGRATION_13_14, MIGRATION_14_15, + MIGRATION_15_16, ) // No destructive fallback: the migration chain is complete, and silently dropping the - // accounts/credentials/mail tables would lose stored secrets. A missing migration should - // fail loudly in testing instead. + // mail/message tables would lose cached data. A missing migration should fail loudly in + // testing instead. // Opt-in at-rest encryption of the local cache (off by default). The conversion runs here — // before the database is opened — so it never races an open connection; toggling the setting @@ -92,6 +92,8 @@ object DatabaseModule { // restarts the app; we wipe the cache HERE — at cold start, before Room opens — so the file is // never deleted from under an open connection. Crash-safe order: wipe + reset the seals, and // only THEN clear the flag, so a kill mid-wipe just repeats the idempotent wipe next start. + // Only libremail.db is wiped: accounts/credentials live in AccountDatabase (a separate file), + // so the user stays signed in across the wipe (issue #111). if (runBlocking { keyStore.isClearPending() }) { DatabaseFiles.clear(context) runBlocking { @@ -100,6 +102,12 @@ object DatabaseModule { } } + // One-time move of accounts/credentials/settings/signatures into the non-auth AccountDatabase + // (issue #111). MUST run before builder.build() below: opening the cache applies MIGRATION_15_16, + // which drops the moved tables. It runs AFTER the wipe above so an unrecoverable-key cache is + // gone first (nothing left to move) and we never block waiting on a passphrase we can't get. + runBlocking { accountDataMigrator.migrateIfNeeded() } + val settings = runBlocking { settingsRepository.settings.first() } val appLock = settings.appLock if (settings.encryptCache) { @@ -119,15 +127,6 @@ object DatabaseModule { @Provides fun provideMessageDao(database: LibreMailDatabase): MessageDao = database.messageDao() - @Provides - fun provideAccountDao(database: LibreMailDatabase): AccountDao = database.accountDao() - - @Provides - fun provideAccountSettingsDao(database: LibreMailDatabase): AccountSettingsDao = database.accountSettingsDao() - - @Provides - fun provideCredentialDao(database: LibreMailDatabase): CredentialDao = database.credentialDao() - @Provides fun provideAttachmentDao(database: LibreMailDatabase): AttachmentDao = database.attachmentDao() @@ -140,9 +139,6 @@ object DatabaseModule { @Provides fun provideFolderDao(database: LibreMailDatabase): FolderDao = database.folderDao() - @Provides - fun provideSignatureDao(database: LibreMailDatabase): SignatureDao = database.signatureDao() - @Provides fun provideBackfillProgressDao(database: LibreMailDatabase): BackfillProgressDao = database.backfillProgressDao() diff --git a/app/src/test/kotlin/org/libremail/mail/CountingImapProxy.kt b/app/src/test/kotlin/org/libremail/mail/CountingImapProxy.kt new file mode 100644 index 0000000..145754e --- /dev/null +++ b/app/src/test/kotlin/org/libremail/mail/CountingImapProxy.kt @@ -0,0 +1,163 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.mail + +import java.io.IOException +import java.net.InetAddress +import java.net.ServerSocket +import java.net.Socket +import java.util.Collections +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.atomic.AtomicInteger + +/** + * A tiny localhost TCP proxy that forwards a **cleartext** IMAP session to a real backend (GreenMail) + * while COUNTING what crosses it, so tests can measure the folder-open round-trip *structure* + * deterministically without a real network (issue #125): + * + * - [connectionCount] — how many separate TCP connections the client established. On a real network + * each new connection is a full CONNECT + TLS handshake + LOGIN/AUTH handshake group (several + * RTTs). [ImapClient] opens one [jakarta.mail.Store] — and therefore one connection — per + * operation today, so this equals the number of operations. Connection reuse / pooling would make + * it diverge (many operations, few connections); that divergence is exactly what a future fix + * should produce and what these tests are wired to detect. + * - [commandCount] — how many times each IMAP command word (LOGIN, EXAMINE, SELECT, FETCH, LOGOUT…) + * the client issued, parsed from the cleartext client → server stream. + * + * Point [ImapClient] at [port] instead of the backend's port. Cleartext only (MailSecurity.NONE): + * command parsing needs to see the bytes. Connection counting alone would work through TLS too, but + * the LibreMail unit tests already exercise the plaintext path, matching the existing GreenMail tests. + */ +class CountingImapProxy(private val backendHost: String, private val backendPort: Int) : AutoCloseable { + + private val server = ServerSocket(0, BACKLOG, InetAddress.getByName("127.0.0.1")) + private val connections = AtomicInteger(0) + private val commands = ConcurrentHashMap() + + /** Client → server pump threads, tracked so tests can wait for the parsed command stream to settle. */ + private val clientPumps = Collections.synchronizedList(mutableListOf()) + + @Volatile private var running = true + + /** The local port to point [ImapClient] at; it forwards to the backend. */ + val port: Int get() = server.localPort + + /** Total TCP connections the client has opened through the proxy. */ + val connectionCount: Int get() = connections.get() + + /** How many times the client issued [command] (case-insensitive), e.g. "LOGIN", "EXAMINE". */ + fun commandCount(command: String): Int = commands[command.uppercase()]?.get() ?: 0 + + /** Authentication round-trips: the `LOGIN` command plus any SASL `AUTHENTICATE` (e.g. XOAUTH2). */ + fun authCommandCount(): Int = commandCount("LOGIN") + commandCount("AUTHENTICATE") + + init { + Thread({ acceptLoop() }, "imap-proxy-accept").apply { isDaemon = true }.start() + } + + /** + * Joins the client → server pump threads so every command line sent before each connection closed + * has been parsed. [ImapClient] closes its store (and thus the socket) when an operation finishes, + * which ends the corresponding pump; call this before asserting on [commandCount]. [connectionCount] + * needs no settling — it is incremented synchronously as each connection is accepted. + */ + fun awaitClientStreamsSettled(timeoutMs: Long = SETTLE_TIMEOUT_MS) { + val deadline = System.currentTimeMillis() + timeoutMs + val snapshot = synchronized(clientPumps) { clientPumps.toList() } + for (thread in snapshot) { + val remaining = deadline - System.currentTimeMillis() + if (remaining > 0) thread.join(remaining) + } + } + + override fun close() { + running = false + runCatching { server.close() } + } + + private fun acceptLoop() { + while (running) { + val client = try { + server.accept() + } catch (_: IOException) { + return // server socket closed by close() + } + connections.incrementAndGet() + val backend = try { + Socket(backendHost, backendPort) + } catch (_: IOException) { + runCatching { client.close() } + continue + } + val upstream = Thread({ pumpCountingCommands(client, backend) }, "imap-proxy-up").apply { isDaemon = true } + val downstream = Thread({ pump(backend, client) }, "imap-proxy-down").apply { isDaemon = true } + clientPumps.add(upstream) + upstream.start() + downstream.start() + } + } + + /** Forwards client → server bytes verbatim while parsing each CRLF-terminated line as a command. */ + private fun pumpCountingCommands(from: Socket, to: Socket) { + val buffer = ByteArray(BUFFER_SIZE) + val line = StringBuilder() + try { + val input = from.getInputStream() + val output = to.getOutputStream() + while (true) { + val read = input.read(buffer) + if (read < 0) break + output.write(buffer, 0, read) + output.flush() + for (i in 0 until read) { + when (val ch = buffer[i].toInt().toChar()) { + '\n' -> { + recordCommand(line.toString()) + line.setLength(0) + } + '\r' -> Unit + else -> line.append(ch) + } + } + } + } catch (_: IOException) { + // Peer closed; fall through to socket cleanup. + } finally { + runCatching { from.close() } + runCatching { to.close() } + } + } + + /** Forwards server → client bytes verbatim (no parsing needed for this direction). */ + private fun pump(from: Socket, to: Socket) { + val buffer = ByteArray(BUFFER_SIZE) + try { + val input = from.getInputStream() + val output = to.getOutputStream() + while (true) { + val read = input.read(buffer) + if (read < 0) break + output.write(buffer, 0, read) + output.flush() + } + } catch (_: IOException) { + // Peer closed; fall through to socket cleanup. + } finally { + runCatching { from.close() } + runCatching { to.close() } + } + } + + /** Records the command word of an IMAP line shaped ` [args]`. */ + private fun recordCommand(rawLine: String) { + val parts = rawLine.trim().split(' ', limit = 3) + if (parts.size < 2) return + val command = parts[1].uppercase() + commands.computeIfAbsent(command) { AtomicInteger(0) }.incrementAndGet() + } + + private companion object { + const val BACKLOG = 50 + const val BUFFER_SIZE = 8192 + const val SETTLE_TIMEOUT_MS = 2_000L + } +} diff --git a/app/src/test/kotlin/org/libremail/mail/ImapFolderOpenLatencyTest.kt b/app/src/test/kotlin/org/libremail/mail/ImapFolderOpenLatencyTest.kt new file mode 100644 index 0000000..62c5dd1 --- /dev/null +++ b/app/src/test/kotlin/org/libremail/mail/ImapFolderOpenLatencyTest.kt @@ -0,0 +1,125 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.mail + +import com.icegreen.greenmail.util.GreenMail +import com.icegreen.greenmail.util.GreenMailUtil +import com.icegreen.greenmail.util.ServerSetupTest +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.libremail.domain.model.ImapConnectionParams +import org.libremail.domain.model.MailSecurity +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Measures the folder-open round-trip *structure* over a real in-process IMAP server (issue #125), + * deterministically and without a real network, by routing [ImapClient] through a [CountingImapProxy] + * that counts the TCP connections and IMAP commands it establishes. + * + * The finding these tests pin down: [ImapClient] wraps every operation in its own short-lived + * [jakarta.mail.Store] (`withStore`), so **each folder-open pays a fresh CONNECT + LOGIN + SELECT + + * FETCH + LOGOUT** — nothing is reused between operations. On a real network the CONNECT + TLS + LOGIN + * group is several RTTs of user-perceived latency that a pooled/kept-alive connection would pay only + * once. See `docs/perf/issue-125-imap-folder-open.md`. + * + * These assertions encode the *current* (no-reuse) behaviour. They are also the harness to validate a + * future connection-reuse fix: when the client reuses one authenticated connection across folder + * switches, the connection/auth counts here drop below the operation count — flip the expectations to + * assert reuse and the tests confirm the win against a real IMAP server. + */ +class ImapFolderOpenLatencyTest { + + private lateinit var greenMail: GreenMail + private lateinit var proxy: CountingImapProxy + private val client = ImapClient() + + @Before + fun setUp() { + greenMail = GreenMail(ServerSetupTest.SMTP_IMAP) + greenMail.start() + greenMail.setUser("alice@example.org", "secret") + // All IMAP traffic goes through the proxy so we can count it; the proxy forwards to GreenMail. + proxy = CountingImapProxy(backendHost = "127.0.0.1", backendPort = greenMail.imap.port) + } + + @After + fun tearDown() { + proxy.close() + greenMail.stop() + } + + /** Points [ImapClient] at the counting proxy rather than directly at GreenMail. */ + private fun params() = ImapConnectionParams( + host = "127.0.0.1", + port = proxy.port, + security = MailSecurity.NONE, + username = "alice@example.org", + secret = "secret", + useXoauth2 = false, + ) + + private fun seedInbox(count: Int) { + repeat(count) { i -> + GreenMailUtil.sendTextEmailTest("alice@example.org", "bob@example.org", "Subject $i", "Body $i") + } + greenMail.waitForIncomingEmail(count) + } + + @Test + fun `each folder-open establishes a brand-new IMAP connection (no reuse today)`() = runTest { + seedInbox(2) + + repeat(OPENS) { client.fetchRecent(params(), "INBOX", limit = 50) } + + // One TCP connection per open: nothing is pooled or kept alive across folder-opens. A + // connection-reuse fix would make this strictly less than OPENS. + assertEquals(OPENS, proxy.connectionCount, "expected one fresh connection per folder-open") + } + + @Test + fun `each folder-open pays a fresh LOGIN and its own SELECT`() = runTest { + seedInbox(2) + + repeat(OPENS) { client.fetchRecent(params(), "INBOX", limit = 50) } + proxy.awaitClientStreamsSettled() + + // The avoidable round-trip: a full authentication on every open. Reuse would drop this to 1. + assertEquals(OPENS, proxy.authCommandCount(), "expected one LOGIN per folder-open") + // The necessary per-open work: READ_ONLY open issues EXAMINE. Reuse keeps this at one-per-open. + assertEquals(OPENS, proxy.commandCount("EXAMINE"), "expected one EXAMINE per folder-open") + } + + @Test + fun `a single folder-open's round-trip sequence is CONNECT-LOGIN-EXAMINE-FETCH-LOGOUT`() = runTest { + seedInbox(3) + + client.fetchRecent(params(), "INBOX", limit = 50) + proxy.awaitClientStreamsSettled() + + assertEquals(1, proxy.connectionCount, "one connection") + assertEquals(1, proxy.authCommandCount(), "one LOGIN — the connection-setup cost, avoidable on reuse") + assertEquals(1, proxy.commandCount("EXAMINE"), "one EXAMINE — the necessary per-folder SELECT") + assertTrue(proxy.commandCount("FETCH") >= 1, "at least one FETCH — the necessary header download") + assertEquals(1, proxy.commandCount("LOGOUT"), "one LOGOUT — the connection is torn down, not kept alive") + } + + @Test + fun `opening a folder then reading a message uses two separate connections (compounding cost)`() = runTest { + seedInbox(1) + + val uid = client.fetchRecent(params(), "INBOX", limit = 50).first().uid // open folder -> connection 1 + client.fetchBodyMarkingSeen(params(), "INBOX", uid) // read a message -> connection 2 + proxy.awaitClientStreamsSettled() + + // No session is shared between listing the folder and reading a message in it: the read pays a + // second full CONNECT + LOGIN even though it targets the folder we just had open. + assertEquals(2, proxy.connectionCount, "list + read each open their own connection") + assertEquals(2, proxy.authCommandCount(), "list + read each pay a full LOGIN") + } + + private companion object { + const val OPENS = 3 + } +} diff --git a/docs/perf/issue-125-imap-folder-open.md b/docs/perf/issue-125-imap-folder-open.md new file mode 100644 index 0000000..aebc139 --- /dev/null +++ b/docs/perf/issue-125-imap-folder-open.md @@ -0,0 +1,175 @@ + +# IMAP folder-open latency (issue #125) + +Structural analysis of the IMAP round-trips paid when opening/selecting a folder, a follow-up to the +#86 profiling and distinct from the mailbox cached-render fix (#123) and the fetch policy (#88–#90). + +Performed 2026-07-02 against `main` by reading the folder-open path and confirming the round-trip +*structure* with deterministic GreenMail tests (`ImapFolderOpenLatencyTest` + +`CountingImapProxy`). + +**Verdict.** The folder-open network path re-establishes a **full, freshly-authenticated IMAP +connection on every operation** — there is no connection pooling or keep-alive. Each folder-open pays +`CONNECT + TLS + LOGIN + EXAMINE + FETCH + LOGOUT`; only the `EXAMINE + FETCH` is intrinsic to opening +a folder, and the entire `CONNECT + TLS + LOGIN` setup group (the majority of the round-trips) is +**avoidable on the second and subsequent operations** if a connection were reused. The recommended +mitigation is a per-account connection cache/keep-alive. It is **not implemented here**: the sizing, +eviction, stale-detection, and battery trade-offs are genuine latency/battery decisions that need +real-network + real-device measurement (which localhost GreenMail — ~0 RTT — cannot provide), and a +naïve implementation risks regressing the deliberate concurrency design and the IDLE connection budget. +This is the "spike first, measure before committing" the issue asks for. + +> **Note on numbers.** This document counts *protocol round-trips* (RTTs), which are deterministic and +> measurable in-process. It does **not** quote measured wall-clock latency — there is no real network +> or account in this environment. Where a millisecond figure appears it is explicitly *illustrative +> arithmetic* (`round-trips × RTT`), with RTT a placeholder for a real network's round-trip time. + +## The folder-open path + +Opening/selecting a folder in the UI runs two independent things: + +1. **Render from cache (already optimized, not the subject of #125).** + `MailboxViewModel.selectFolder()` sets `_selectedFolder` synchronously + (`MailboxViewModel.kt:307`). That immediately re-filters the cached Room rows into the list — no + network. #123 optimized this cached render. The network open below is *off* the render path, so its + cost shows up as a background refresh, not a blank screen. + +2. **Network sync (the subject of #125).** + `selectFolder()` then launches `mailSyncer.syncFolder(accountId, folder)`: + + ``` + MailboxViewModel.selectFolder() (MailboxViewModel.kt:307) + └─ MailSyncer.syncFolder() (MailSyncer.kt:80) + └─ syncFolderHeaders() (MailSyncer.kt:87) + ├─ connectionFactory.imapParamsFor(account) (resolves/refreshes credentials) + └─ imapClient.fetchRecent(params, folder, limit) (MailSyncer.kt:93) + └─ ImapClient.withStore { … } (ImapClient.kt:111, 521) + ``` + +`ImapClient.withStore()` is the crux (`ImapClient.kt:521`): + +```kotlin +private inline fun withStore(params: ImapConnectionParams, block: (Store) -> T): T { + val store = Session.getInstance(buildProps(protocol, params)).getStore(protocol) + store.connect(params.host, params.port, params.username, params.secret) // CONNECT + TLS + LOGIN + return try { block(store) } finally { runCatching { store.close() } } // LOGOUT + teardown +} +``` + +**Every** `ImapClient` operation — `fetchRecent`, `fetchOlderThan`, `search`, `fetchBodyMarkingSeen`, +`fetchBodyPeek`, `fetchAttachment`, `setFlag`, `deleteMessage`, `moveMessages`, `fetchForReply` — is a +`withStore { … }`, so each one builds and authenticates its own connection and tears it down. Nothing +is reused between operations. + +## Per-open round-trip sequence + +For one `fetchRecent` (a folder-open), the client → server exchange is: + +| # | Step | RTTs | Necessary to *open a folder*? | +|---|------|------|-------------------------------| +| 1 | TCP handshake | ~1 | Setup — avoidable on reuse | +| 2 | TLS handshake (implicit TLS / `imaps`) | 1 (TLS 1.3) – 2 (TLS 1.2) | Setup — avoidable on reuse | +| 3 | `CAPABILITY` (Angus; reused from greeting when advertised) | 0–1 | Setup — avoidable on reuse | +| 4 | `LOGIN` / `AUTHENTICATE XOAUTH2` | 1 (+1 if challenged) | Setup — avoidable on reuse | +| 5 | `CAPABILITY` post-auth (reused from `LOGIN` response when advertised) | 0–1 | Setup — avoidable on reuse | +| 6 | `EXAMINE` (READ_ONLY select of the folder) | 1 | **Necessary** per folder | +| 7 | `FETCH` recent headers (`ENVELOPE FLAGS UID`) | 1 | **Necessary** header download | +| 8 | `LOGOUT` + socket teardown | ~1 | Setup — avoidable on reuse | + +- **STARTTLS (`imap` on 143)** is worse: it inserts a pre-TLS `CAPABILITY`, the `STARTTLS` command, + then a post-TLS `CAPABILITY` *before* step 4 — roughly **6–8 setup RTTs** instead of 4–6. +- **Setup (steps 1–5, 8): ~4–6 RTT (imaps) / ~6–8 RTT (STARTTLS).** +- **Intrinsic folder work (steps 6–7): 2 RTT.** + +So the connection setup is the **majority** of the round-trips on every open, and it is exactly the +part a reused connection would skip. Illustratively, at an RTT of *R*: a cold open ≈ `(4–6)·R` setup + +`2·R` work; a warm (reused-connection) open ≈ `2·R`. The setup share — everything except the +`EXAMINE + FETCH` — is what a fix removes from the 2nd open onward. + +### Compounding across operations + +Because the pattern is per-operation, costs stack: + +- **Folder switch A → B → A:** 3 folder-opens ⇒ 3 full `CONNECT + TLS + LOGIN` setups. +- **List then open a message:** `fetchRecent` (open) + `fetchBodyMarkingSeen` (read) ⇒ 2 full setups, + even though the read targets the folder just listed (proven by the test below). +- **Prefetch after a sync** (`MailSyncer.prefetchIfEnabled`, FetchPolicy territory #88–#90, *not* + changed here): each unfetched message body is another `withStore` connection, and each attachment + another still. A folder-open that triggers prefetch of *K* messages can open `1 + K + attachments` + separate authenticated connections. This amplifies the motivation for pooling but is out of scope. + +## Deterministic evidence (no real network needed) + +`ImapFolderOpenLatencyTest` routes `ImapClient` through `CountingImapProxy` — a localhost TCP proxy +that forwards a cleartext IMAP session to in-process GreenMail while counting connections and parsing +IMAP command words. This measures the *structure* exactly, without needing real latency: + +- `each folder-open establishes a brand-new IMAP connection (no reuse today)` — N opens ⇒ **N** TCP + connections. +- `each folder-open pays a fresh LOGIN and its own SELECT` — N opens ⇒ **N** `LOGIN` **and** N + `EXAMINE` (the avoidable auth vs. the necessary select). +- `a single folder-open's round-trip sequence is CONNECT-LOGIN-EXAMINE-FETCH-LOGOUT` — pins the + sequence: 1 connection, 1 `LOGIN`, 1 `EXAMINE`, ≥1 `FETCH`, 1 `LOGOUT`. +- `opening a folder then reading a message uses two separate connections (compounding cost)` — list + + read ⇒ **2** connections and **2** `LOGIN`s. + +These assertions encode the *current* (no-reuse) behaviour and double as the **validation harness for a +future fix**: once a connection is reused across folder switches, the connection/auth counts drop below +the operation count — flip the expectations to assert reuse and the same real-IMAP tests confirm the win. + +## Recommended mitigation: per-account connection reuse / keep-alive + +Keep one authenticated `Store` alive per account and reuse it across folder-opens and message +operations instead of `withStore`'s connect-per-call, so only the first operation pays setup and +subsequent ones pay just `EXAMINE + FETCH`. Design constraints that make this **non-trivial** and why +it needs measurement before landing: + +1. **Must not disturb IMAP IDLE (#90).** `IdleService` already holds a *separate*, dedicated + long-lived `Store` per account (`ImapClient.idle`, `IdleService.watchAccount`), blocking on + `INBOX.idle()`. IMAP is serial per connection and IDLE blocks its connection, so folder-opens + cannot be multiplexed onto it. A reuse pool is therefore an **additional** persistent connection + per account (IDLE + pool), which must respect the server's per-account connection limit (Gmail + ~15; many servers 3–5) — a budget `ImapClient.idle`'s own comment already flags. +2. **Thread-safety.** `MailRepositoryImpl`'s UI operations (`openMessage`, `setStarred`, + `deleteMessage`, `moveMessages`, `setFlag`, …) are **not** serialized and can overlap `MailSyncer` + (whose `prefetchIfEnabled` deliberately runs *outside* `syncMutex` so downloads don't block + pull-to-refresh). Today's connect-per-call sidesteps this. A shared connection needs its own + discipline: a single mutex-guarded connection (simplest, but head-of-line-blocks a flag toggle + behind a slow body download — a regression of the current concurrency) **or** a small bounded pool + of N connections (more throughput, needs a size cap + eviction). Choosing between them is a + latency/throughput trade-off that needs real measurement. +3. **Stale-connection handling.** A pooled socket can be dropped by the server's idle timeout + (RFC-permitted), NAT rebinding, or a network change. Reuse must detect staleness — a `NOOP` probe + (adds 1 RTT, partly defeating the point) or catch-and-retry-once on a fresh connection — behaviour + best validated against real servers and real network transitions. +4. **Battery / lifecycle (#88/#89/#90).** Holding a socket open has a battery cost; #90 already tears + IDLE down at low battery. A reuse pool needs an idle-eviction timeout and should likely mirror that + low-battery posture. The right timeout is a battery-vs-latency trade-off that needs device + measurement. + +Because every one of these knobs (mutex vs. pool, eviction timeout, stale-probe strategy, battery +posture) trades latency against battery/complexity and can only be tuned with a real network and a +real device — which this environment cannot provide — forcing an implementation now would be guessing. +Per #125's "investigation/spike first" guidance, this change ships the measurement harness + analysis +and **defers the pool to a measured follow-up**. + +**Already correct — do not redo.** Optimistic render-from-cache is already the architecture +(`selectFolder` renders cached rows instantly; the network sync is a background refresh). #125's +"optimistic render while the network catches up" is satisfied; only connection reuse remains. + +## What a maintainer needs to fully close #125 (real device + real account) + +1. **Instrument the open.** Add timing around `syncFolder → fetchRecent → store.connect / open / fetch + / close` (or enable Angus `mail.imap` debug) and capture on a real Gmail/Outlook account over both + Wi-Fi and cellular. +2. **Attribute the wall-clock.** Break each open into connect (TCP+TLS), login, `EXAMINE`, `FETCH`, + `LOGOUT`; confirm the hypothesis that connection setup dominates and quantify its share. +3. **A/B the pool behind a flag.** Measure folder-switch latency (open A → open B → back to A) and + list-then-open-message latency, cold vs. warm-reuse, on the same accounts/networks. Expect warm + opens to fall by the connection-setup share. +4. **Battery check.** Measure the kept-alive socket's idle cost against candidate eviction timeouts; + confirm no regression versus the #88/#89/#90 posture. +5. **Resilience check.** Force server idle-timeout and network transitions; confirm transparent + reconnect with no user-visible failures, and that IDLE + pool stay within the per-account limit. +6. **Lock it in.** Flip `ImapFolderOpenLatencyTest` to assert reuse (connection/auth counts < operation + count) as the deterministic regression guard.