From f70bda77d4a63798c63edd508dd5348561db31f8 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 1 Jul 2026 13:41:06 -0500 Subject: [PATCH 1/3] fix(mailbox): project message list to avoid CursorWindow overflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MessageDao.observeAll() ran `SELECT * FROM messages` and returned full MessageEntity rows — including the potentially large body/isHtml columns — for every cached message at once. Dragging big HTML bodies through SQLite's shared ~2 MB CursorWindow overflowed it once enough bodies were cached, crashing with "Couldn't read row N from CursorWindow" (#51). Replace it with observeSummaries(), a body-less column projection into a new lightweight MessageSummary POJO. The list never renders or searches the body, and the reader already loads it lazily per-message via getById when a message is opened, so nothing else needs it. Add a regression test that reads back rows whose bodies exceed the window. Closes #51 Co-Authored-By: Claude Opus 4.8 --- .../data/local/DatabaseEncryptionTest.kt | 4 +-- .../data/local/LibreMailDatabaseTest.kt | 24 +++++++++++++++-- .../org/libremail/data/local/Mappers.kt | 23 ++++++++++++++++ .../libremail/data/local/dao/MessageDao.kt | 14 ++++++++-- .../data/local/entity/MessageSummary.kt | 26 +++++++++++++++++++ .../data/repository/MailRepositoryImpl.kt | 2 +- .../data/repository/MailRepositoryImplTest.kt | 22 +++++++++++++--- 7 files changed, 105 insertions(+), 10 deletions(-) create mode 100644 app/src/main/kotlin/org/libremail/data/local/entity/MessageSummary.kt diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/DatabaseEncryptionTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/DatabaseEncryptionTest.kt index 0d1a748..0e0a82d 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/DatabaseEncryptionTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/DatabaseEncryptionTest.kt @@ -53,7 +53,7 @@ class DatabaseEncryptionTest { DatabaseEncryption.ensureEncrypted(dbFile, passphrase) assertTrue("file must not read as plaintext once encrypted", DatabaseEncryption.isEncrypted(dbFile)) openEncrypted().apply { - assertEquals(listOf("acct:1"), messageDao().observeAll().first().map { it.id }) + assertEquals(listOf("acct:1"), messageDao().observeSummaries().first().map { it.id }) close() } @@ -61,7 +61,7 @@ class DatabaseEncryptionTest { DatabaseEncryption.ensurePlaintext(dbFile, passphrase) assertFalse("file must be plaintext again after decrypt", DatabaseEncryption.isEncrypted(dbFile)) openPlaintext().apply { - assertEquals(listOf("acct:1"), messageDao().observeAll().first().map { it.id }) + assertEquals(listOf("acct:1"), messageDao().observeSummaries().first().map { it.id }) close() } } 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 460b2e2..59c8b69 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt @@ -102,10 +102,30 @@ class LibreMailDatabaseTest { assertEquals(listOf("acct:1"), messageDao.getSyncedIds("acct", "INBOX")) messageDao.deleteSearchRows() - val remaining = messageDao.observeAll().first().map { it.id } + val remaining = messageDao.observeSummaries().first().map { it.id } assertEquals(listOf("acct:1"), remaining) } + @Test + fun observeSummariesReadsRowsWhoseBodiesExceedTheCursorWindow() = runBlocking { + val messageDao = db.messageDao() + // Each body is larger than SQLite's shared (~2 MB) CursorWindow. The old list query did + // SELECT * and dragged these bodies through the window, overflowing it with + // "Couldn't read row … from CursorWindow" (issue #51). observeSummaries omits body, so the + // rows stay tiny and read fine. + val hugeBody = "x".repeat(3 * 1024 * 1024) + messageDao.insertNew( + listOf( + message("acct:1", body = hugeBody), + message("acct:2", body = hugeBody), + ), + ) + + val ids = messageDao.observeSummaries().first().map { it.id }.toSet() + + assertEquals(setOf("acct:1", "acct:2"), ids) + } + @Test fun foldersAreStoredOrderedAndReplaceablePerAccount() = runBlocking { val folderDao = db.folderDao() @@ -146,7 +166,7 @@ class LibreMailDatabaseTest { messageDao.deleteSyncedNotIn("acct", "INBOX", listOf("acct:INBOX:1")) assertEquals( setOf("acct:INBOX:1", "acct:Archive:1"), - messageDao.observeAll().first().map { it.id }.toSet(), + messageDao.observeSummaries().first().map { it.id }.toSet(), ) } } diff --git a/app/src/main/kotlin/org/libremail/data/local/Mappers.kt b/app/src/main/kotlin/org/libremail/data/local/Mappers.kt index a6b444f..0b970b9 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Mappers.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Mappers.kt @@ -9,6 +9,7 @@ import org.libremail.data.local.entity.AttachmentEntity 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.MessageSummary import org.libremail.data.local.entity.OutboxEntity import org.libremail.data.local.entity.ServerConfigEmbedded import org.libremail.domain.model.Account @@ -103,6 +104,28 @@ internal fun MessageEntity.toDomain(): Message = Message( bodyFetched = bodyFetched, ) +/** + * Maps a mailbox-list projection to the domain model. [Message.body]/[Message.isHtml] are left + * empty because the list never renders them — the reader loads the body on demand (see + * [MessageSummary]). + */ +internal fun MessageSummary.toDomain(): Message = Message( + id = id, + accountId = accountId, + sender = sender, + senderEmail = senderEmail, + subject = subject, + snippet = snippet, + body = "", + isHtml = false, + timestampMillis = timestampMillis, + isRead = isRead, + isStarred = isStarred, + folder = folder, + inInbox = inInbox, + bodyFetched = bodyFetched, +) + internal fun FetchedMessage.toEntity(accountId: String, folder: String, inInbox: Boolean = true): MessageEntity = MessageEntity( id = "$accountId:$folder:$uid", diff --git a/app/src/main/kotlin/org/libremail/data/local/dao/MessageDao.kt b/app/src/main/kotlin/org/libremail/data/local/dao/MessageDao.kt index db2495b..6832e45 100644 --- a/app/src/main/kotlin/org/libremail/data/local/dao/MessageDao.kt +++ b/app/src/main/kotlin/org/libremail/data/local/dao/MessageDao.kt @@ -7,11 +7,21 @@ import androidx.room.OnConflictStrategy import androidx.room.Query import kotlinx.coroutines.flow.Flow import org.libremail.data.local.entity.MessageEntity +import org.libremail.data.local.entity.MessageSummary @Dao interface MessageDao { - @Query("SELECT * FROM messages ORDER BY timestampMillis DESC") - fun observeAll(): Flow> + /** + * Mailbox-list projection ordered newest-first. Deliberately omits the large `body`/`isHtml` + * columns: the list observes every cached message at once, and pulling full bodies through + * SQLite's shared ~2 MB CursorWindow overflows it once enough large bodies are cached + * (issue #51). Bodies are loaded lazily per-message via [getById] when a message is opened. + */ + @Query( + "SELECT id, accountId, sender, senderEmail, subject, snippet, timestampMillis, " + + "isRead, isStarred, folder, inInbox, bodyFetched FROM messages ORDER BY timestampMillis DESC", + ) + fun observeSummaries(): Flow> @Query("SELECT * FROM messages WHERE id = :id LIMIT 1") suspend fun getById(id: String): MessageEntity? diff --git a/app/src/main/kotlin/org/libremail/data/local/entity/MessageSummary.kt b/app/src/main/kotlin/org/libremail/data/local/entity/MessageSummary.kt new file mode 100644 index 0000000..c9b259d --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/local/entity/MessageSummary.kt @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local.entity + +/** + * Lightweight projection of [MessageEntity] for the mailbox list: every column the list renders + * or searches, but *not* the potentially large `body`/`isHtml`. + * + * The list observes every cached message at once, so selecting full HTML bodies would drag them + * all through SQLite's shared (~2 MB) CursorWindow and overflow it once enough large bodies are + * cached — crashing with "Couldn't read row N from CursorWindow" (issue #51). Bodies are read + * lazily, one message at a time, via `MessageDao.getById` when a message is opened. + */ +data class MessageSummary( + val id: String, + val accountId: String, + val sender: String, + val senderEmail: String, + val subject: String, + val snippet: String, + val timestampMillis: Long, + val isRead: Boolean, + val isStarred: Boolean, + val folder: String, + val inInbox: Boolean, + val bodyFetched: Boolean, +) diff --git a/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt b/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt index d6ec723..6ae11cd 100644 --- a/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt +++ b/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt @@ -57,7 +57,7 @@ class MailRepositoryImpl @Inject constructor( private val signatureRepository: SignatureRepository, ) : MailRepository { - override fun observeMessages(): Flow> = messageDao.observeAll().map { rows -> + override fun observeMessages(): Flow> = messageDao.observeSummaries().map { rows -> rows.map { it.toDomain() } } diff --git a/app/src/test/kotlin/org/libremail/data/repository/MailRepositoryImplTest.kt b/app/src/test/kotlin/org/libremail/data/repository/MailRepositoryImplTest.kt index a40ed20..72ba7ac 100644 --- a/app/src/test/kotlin/org/libremail/data/repository/MailRepositoryImplTest.kt +++ b/app/src/test/kotlin/org/libremail/data/repository/MailRepositoryImplTest.kt @@ -23,6 +23,7 @@ import org.libremail.data.local.entity.AttachmentEntity 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.MessageSummary import org.libremail.data.local.entity.ServerConfigEmbedded import org.libremail.data.settings.AccountSettingsRepository import org.libremail.data.settings.SignatureRepository @@ -73,7 +74,7 @@ class MailRepositoryImplTest { @Test fun `observeMessages is empty when the cache is empty`() = runTest { - every { messageDao.observeAll() } returns flowOf(emptyList()) + every { messageDao.observeSummaries() } returns flowOf(emptyList()) repository.observeMessages().test { assertTrue(awaitItem().isEmpty()) awaitComplete() @@ -82,8 +83,7 @@ class MailRepositoryImplTest { @Test fun `observeMessages maps cached entities`() = runTest { - val entity = messageEntity("1", "INBOX") - every { messageDao.observeAll() } returns flowOf(listOf(entity)) + every { messageDao.observeSummaries() } returns flowOf(listOf(messageSummary("1", "INBOX"))) repository.observeMessages().test { val items = awaitItem() assertEquals(1, items.size) @@ -421,6 +421,22 @@ class MailRepositoryImplTest { bodyFetched = bodyFetched, ) + private fun messageSummary(id: String, folder: String, accountId: String = "acct", bodyFetched: Boolean = false) = + MessageSummary( + id = id, + accountId = accountId, + sender = "Ada", + senderEmail = "ada@example.org", + subject = "Hi", + snippet = "snippet", + timestampMillis = 1_000L, + isRead = false, + isStarred = false, + folder = folder, + inInbox = true, + bodyFetched = bodyFetched, + ) + private fun accountEntity(id: String = "acct", email: String = "ada@example.org") = AccountEntity( id = id, email = email, From 0f479b24311006a9f53da3d0a0453879805a5b86 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 1 Jul 2026 15:01:11 -0500 Subject: [PATCH 2/3] fix(mailbox): de-duplicate folder names in the drawer The drawer rendered every standard-role folder with a generic friendly name (e.g. "Drafts") and discarded the server name, so a Gmail account with both a provider built-in folder and a same-named user folder showed two identical entries (Drafts, Archive, Spam). De-duplicate labels provider-agnostically: when 2+ folders would render the same name, the provider's built-in special folder (identified by RFC 6154 SPECIAL-USE flags, now persisted on the folder cache) gets the provider name appended ("Archive - Gmail"), a nested user folder gets its parent location ("Reports (Work)"), and a top-level user folder keeps its plain name. Only triggers on a real collision, so stock accounts are unchanged. Adds a `specialUse` column to the folders table (Room v11 -> v12). Co-Authored-By: Claude Opus 4.8 --- .../12.json | 606 ++++++++++++++++++ .../data/local/LibreMailDatabaseTest.kt | 14 +- .../libremail/ui/mailbox/FolderDrawerTest.kt | 30 +- .../libremail/data/local/LibreMailDatabase.kt | 2 +- .../org/libremail/data/local/Mappers.kt | 2 + .../org/libremail/data/local/Migrations.kt | 13 + .../data/local/entity/FolderEntity.kt | 7 + .../kotlin/org/libremail/di/DatabaseModule.kt | 2 + .../org/libremail/domain/model/Folder.kt | 16 + .../libremail/domain/model/MailProvider.kt | 4 + .../org/libremail/ui/mailbox/FolderDrawer.kt | 6 +- .../org/libremail/ui/mailbox/FolderLabels.kt | 67 ++ .../libremail/domain/model/FolderRoleTest.kt | 12 + .../libremail/ui/mailbox/FolderLabelsTest.kt | 131 ++++ 14 files changed, 902 insertions(+), 10 deletions(-) create mode 100644 app/schemas/org.libremail.data.local.LibreMailDatabase/12.json create mode 100644 app/src/main/kotlin/org/libremail/ui/mailbox/FolderLabels.kt create mode 100644 app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt diff --git a/app/schemas/org.libremail.data.local.LibreMailDatabase/12.json b/app/schemas/org.libremail.data.local.LibreMailDatabase/12.json new file mode 100644 index 0000000..f27dd67 --- /dev/null +++ b/app/schemas/org.libremail.data.local.LibreMailDatabase/12.json @@ -0,0 +1,606 @@ +{ + "formatVersion": 1, + "database": { + "version": 12, + "identityHash": "45329c820e9325adbfd6b7de89a9996a", + "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": "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, 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 + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId" + ] + }, + "foreignKeys": [ + { + "table": "accounts", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "accountId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "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, 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 + } + ], + "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`)" + } + ] + }, + { + "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": "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, 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" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId", + "fullName" + ] + } + }, + { + "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, '45329c820e9325adbfd6b7de89a9996a')" + ] + } +} \ No newline at end of file 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 59c8b69..3019b28 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt @@ -132,15 +132,17 @@ class LibreMailDatabaseTest { folderDao.replaceForAccount( "acct", listOf( - FolderEntity("acct", "[Gmail]/Sent Mail", "Sent Mail", "SENT", selectable = true, sortOrder = 1), + FolderEntity( + "acct", "[Gmail]/Sent Mail", "Sent Mail", "SENT", + selectable = true, sortOrder = 1, specialUse = true, + ), FolderEntity("acct", "INBOX", "INBOX", "INBOX", selectable = true, sortOrder = 0), ), ) - // observeForAccount returns folders ordered by sortOrder. - assertEquals( - listOf("INBOX", "[Gmail]/Sent Mail"), - folderDao.observeForAccount("acct").first().map { it.fullName }, - ) + // observeForAccount returns folders ordered by sortOrder, with specialUse round-tripped. + val stored = folderDao.observeForAccount("acct").first() + assertEquals(listOf("INBOX", "[Gmail]/Sent Mail"), stored.map { it.fullName }) + assertEquals(listOf(false, true), stored.map { it.specialUse }) // replaceForAccount swaps the whole set (delete + insert). folderDao.replaceForAccount("acct", listOf(FolderEntity("acct", "Archive", "Archive", "ARCHIVE", true, 0))) diff --git a/app/src/androidTest/kotlin/org/libremail/ui/mailbox/FolderDrawerTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/mailbox/FolderDrawerTest.kt index 87516f6..fbe7abb 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/mailbox/FolderDrawerTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/mailbox/FolderDrawerTest.kt @@ -58,6 +58,27 @@ class FolderDrawerTest { composeTestRule.onNodeWithText(string(R.string.folder_all_inboxes)).assertDoesNotExist() } + @Test + fun duplicateFolderNames_areDisambiguatedWithProviderSuffix() { + val gmail = account("imap:g", "user@gmail.com").copy( + imap = ServerConfig("imap.gmail.com", 993, MailSecurity.SSL_TLS), + ) + setContent( + accounts = listOf(gmail), + drawerAccount = gmail, + folders = listOf( + folder("imap:g", "INBOX", "INBOX", FolderRole.INBOX), + // Gmail's built-in Drafts (server special-use) alongside a same-named user folder. + folder("imap:g", "[Gmail]/Drafts", "Drafts", FolderRole.DRAFTS, specialUse = true), + folder("imap:g", "Drafts", "Drafts", FolderRole.DRAFTS), + ), + ) + + // The provider's built-in folder is suffixed; the user folder keeps the plain name. + composeTestRule.onNodeWithText("Drafts - Gmail").assertIsDisplayed() + composeTestRule.onNodeWithText("Drafts").assertIsDisplayed() + } + @Test fun tappingAFolder_reportsItsAccountAndFullName() { var picked: Pair? = null @@ -131,6 +152,11 @@ class FolderDrawerTest { smtp = ServerConfig("smtp.example.org", 465, MailSecurity.SSL_TLS), ) - private fun folder(accountId: String, fullName: String, displayName: String, role: FolderRole) = - Folder(accountId, fullName, displayName, role, selectable = true) + private fun folder( + accountId: String, + fullName: String, + displayName: String, + role: FolderRole, + specialUse: Boolean = false, + ) = Folder(accountId, fullName, displayName, role, selectable = true, specialUse = specialUse) } 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 d65ee85..30e87ab 100644 --- a/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt +++ b/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt @@ -34,7 +34,7 @@ import org.libremail.data.local.entity.SignatureEntity FolderEntity::class, SignatureEntity::class, ], - version = 11, + version = 12, exportSchema = true, ) abstract class LibreMailDatabase : RoomDatabase() { diff --git a/app/src/main/kotlin/org/libremail/data/local/Mappers.kt b/app/src/main/kotlin/org/libremail/data/local/Mappers.kt index 0b970b9..831bc2f 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Mappers.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Mappers.kt @@ -150,6 +150,7 @@ internal fun FolderEntity.toDomain(): Folder = Folder( displayName = displayName, role = runCatching { FolderRole.valueOf(role) }.getOrDefault(FolderRole.NORMAL), selectable = selectable, + specialUse = specialUse, ) internal fun FetchedFolder.toEntity(accountId: String, sortOrder: Int): FolderEntity = FolderEntity( @@ -159,6 +160,7 @@ internal fun FetchedFolder.toEntity(accountId: String, sortOrder: Int): FolderEn role = FolderRole.roleOf(fullName, displayName, attributes).name, selectable = selectable, sortOrder = sortOrder, + specialUse = FolderRole.isServerSpecial(attributes), ) internal fun AttachmentEntity.toDomain(): Attachment = Attachment( 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 dcece0e..03dd030 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Migrations.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Migrations.kt @@ -216,3 +216,16 @@ val MIGRATION_10_11 = object : Migration(10, 11) { ) } } + +/** + * v11 -> v12: drawer folder de-duplication (preserves existing data). Adds a `specialUse` column to + * `folders` recording whether the server advertises the folder as special-use (RFC 6154), so the + * drawer can tell a provider's built-in folder from a same-named user folder. `DEFAULT 0` matches + * the entity's `@ColumnInfo(defaultValue = "0")` so fresh-install and migrated schemas validate + * identically (the MIGRATION_9_10 pattern); the next folder refresh backfills the real value. + */ +val MIGRATION_11_12 = object : Migration(11, 12) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("ALTER TABLE `folders` ADD COLUMN `specialUse` INTEGER NOT NULL DEFAULT 0") + } +} diff --git a/app/src/main/kotlin/org/libremail/data/local/entity/FolderEntity.kt b/app/src/main/kotlin/org/libremail/data/local/entity/FolderEntity.kt index d0523cb..10ee226 100644 --- a/app/src/main/kotlin/org/libremail/data/local/entity/FolderEntity.kt +++ b/app/src/main/kotlin/org/libremail/data/local/entity/FolderEntity.kt @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-3.0-or-later package org.libremail.data.local.entity +import androidx.room.ColumnInfo import androidx.room.Entity /** A cached IMAP folder for an account. [sortOrder] preserves the server's listing order. */ @@ -13,4 +14,10 @@ data class FolderEntity( val role: String, val selectable: Boolean, val sortOrder: Int, + /** + * True when the server advertises this as a special-use folder (RFC 6154, e.g. `\Drafts`, + * `\Junk`, `\All`). Distinguishes a provider's built-in folder from a same-named user folder + * when the drawer de-duplicates display labels. + */ + @ColumnInfo(defaultValue = "0") val specialUse: Boolean = false, ) diff --git a/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt b/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt index 1920ea3..3bbfe19 100644 --- a/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt +++ b/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt @@ -14,6 +14,7 @@ import net.zetetic.database.sqlcipher.SupportOpenHelperFactory import org.libremail.data.local.DatabaseEncryption import org.libremail.data.local.LibreMailDatabase import org.libremail.data.local.MIGRATION_10_11 +import org.libremail.data.local.MIGRATION_11_12 import org.libremail.data.local.MIGRATION_1_2 import org.libremail.data.local.MIGRATION_2_3 import org.libremail.data.local.MIGRATION_3_4 @@ -59,6 +60,7 @@ object DatabaseModule { MIGRATION_8_9, MIGRATION_9_10, MIGRATION_10_11, + MIGRATION_11_12, ) // No destructive fallback: the migration chain is complete, and silently dropping the // accounts/credentials/mail tables would lose stored secrets. A missing migration should diff --git a/app/src/main/kotlin/org/libremail/domain/model/Folder.kt b/app/src/main/kotlin/org/libremail/domain/model/Folder.kt index f3f2ba2..0b8de14 100644 --- a/app/src/main/kotlin/org/libremail/domain/model/Folder.kt +++ b/app/src/main/kotlin/org/libremail/domain/model/Folder.kt @@ -11,6 +11,11 @@ data class Folder( val role: FolderRole, /** False for \Noselect containers (e.g. Gmail's "[Gmail]" parent) that hold no messages. */ val selectable: Boolean, + /** + * True when the server advertises this as a special-use folder (RFC 6154). Lets the drawer tell + * a provider's built-in folder from a same-named user folder when de-duplicating labels. + */ + val specialUse: Boolean = false, ) /** @@ -47,6 +52,17 @@ enum class FolderRole { return byAttribute ?: roleFromDisplayName(displayName) } + /** + * The RFC 6154 SPECIAL-USE attributes (plus Gmail's `\All`) that mark a folder as one the + * server provisions itself, as opposed to a user-created folder. + */ + private val SPECIAL_USE_ATTRIBUTES = + setOf("\\all", "\\archive", "\\drafts", "\\flagged", "\\junk", "\\sent", "\\trash") + + /** True when the server advertises any SPECIAL-USE attribute for the folder (RFC 6154). */ + fun isServerSpecial(attributes: List): Boolean = + attributes.any { it.lowercase() in SPECIAL_USE_ATTRIBUTES } + /** Best-effort role from a folder's display name, for servers without SPECIAL-USE flags. */ private fun roleFromDisplayName(displayName: String): FolderRole = when (displayName.lowercase().trim()) { "sent", "sent mail", "sent items", "sent messages" -> SENT diff --git a/app/src/main/kotlin/org/libremail/domain/model/MailProvider.kt b/app/src/main/kotlin/org/libremail/domain/model/MailProvider.kt index c5a7188..927b32e 100644 --- a/app/src/main/kotlin/org/libremail/domain/model/MailProvider.kt +++ b/app/src/main/kotlin/org/libremail/domain/model/MailProvider.kt @@ -86,5 +86,9 @@ enum class MailProvider( companion object { /** Resolves a provider by its [key], or null if none matches (case-insensitive). */ fun fromKey(key: String): MailProvider? = entries.firstOrNull { it.key.equals(key, ignoreCase = true) } + + /** Resolves a provider by its IMAP host, or null if none matches (case-insensitive). */ + fun forImapHost(host: String): MailProvider? = + entries.firstOrNull { it.imapHost.equals(host, ignoreCase = true) } } } diff --git a/app/src/main/kotlin/org/libremail/ui/mailbox/FolderDrawer.kt b/app/src/main/kotlin/org/libremail/ui/mailbox/FolderDrawer.kt index a6b0a55..edef7eb 100644 --- a/app/src/main/kotlin/org/libremail/ui/mailbox/FolderDrawer.kt +++ b/app/src/main/kotlin/org/libremail/ui/mailbox/FolderDrawer.kt @@ -77,6 +77,10 @@ fun FolderDrawer( val sorted = remember(folders) { folders.sortedWith(compareBy({ it.role.ordinal }, { it.displayName.lowercase() })) } + // De-duplicate labels: two distinct folders that would render the same name (e.g. a + // provider's built-in Drafts and a same-named user folder) get disambiguated. + val baseLabels = sorted.associate { it.fullName to folderDisplayLabel(it) } + val resolvedLabels = resolveDrawerLabels(sorted, baseLabels, drawerAccount?.let(::providerLabel).orEmpty()) sorted.forEach { folder -> val isSelected = selectedAccountId != null && selectedAccountId == drawerAccount?.id && @@ -85,7 +89,7 @@ fun FolderDrawer( { Icon(vector, contentDescription = null) } } NavigationDrawerItem( - label = { Text(folderDisplayLabel(folder)) }, + label = { Text(resolvedLabels[folder.fullName] ?: folderDisplayLabel(folder)) }, icon = iconContent, selected = isSelected, onClick = { diff --git a/app/src/main/kotlin/org/libremail/ui/mailbox/FolderLabels.kt b/app/src/main/kotlin/org/libremail/ui/mailbox/FolderLabels.kt new file mode 100644 index 0000000..c65b85b --- /dev/null +++ b/app/src/main/kotlin/org/libremail/ui/mailbox/FolderLabels.kt @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.ui.mailbox + +import org.libremail.domain.model.Account +import org.libremail.domain.model.AuthType +import org.libremail.domain.model.Folder +import org.libremail.domain.model.MailProvider + +/** + * The provider name appended when de-duplicating a folder label (the "Gmail" in "Drafts - Gmail"). + * A recognized brand where possible, else the account's email domain so any account has a suffix. + */ +fun providerLabel(account: Account): String = when { + account.authType == AuthType.OAUTH_OUTLOOK || + account.imap.host.contains("outlook", ignoreCase = true) || + account.imap.host.contains("office365", ignoreCase = true) -> "Outlook" + + else -> MailProvider.forImapHost(account.imap.host)?.displayName + ?: account.email.substringAfterLast('@', account.imap.host) +} + +/** + * Resolves each folder's drawer label so no two entries collide. [baseLabels] maps a folder's + * [Folder.fullName] to its localized base label (a friendly role name, or the raw display name). + * + * A base label shared by 2+ folders is disambiguated: the provider's built-in special folder gets + * " - [providerLabel]" (e.g. "Archive - Gmail"); a nested user folder gets its parent location in + * parentheses (e.g. "Reports (Work)"); a top-level user folder keeps its base label. Among colliding + * user folders at most one is top-level (paths are unique), so the result is distinct. A final pass + * appends the full path to any labels that still tie, guaranteeing uniqueness. + */ +fun resolveDrawerLabels( + folders: List, + baseLabels: Map, + providerLabel: String, +): Map { + fun base(folder: Folder) = baseLabels[folder.fullName] ?: folder.displayName + val duplicated = folders.groupingBy(::base).eachCount().filterValues { it > 1 }.keys + + val resolved = folders.associate { folder -> + val base = base(folder) + val label = when { + base !in duplicated -> base + folder.specialUse -> "$base - $providerLabel" + else -> parentOf(folder.fullName, folder.displayName)?.let { "$base ($it)" } ?: base + } + folder.fullName to label + } + + // Safety net for a residual tie (e.g. two special folders mapping to one role): fall back to the + // unambiguous full path so every drawer entry stays distinct. + val stillTied = resolved.values.groupingBy { it }.eachCount().filterValues { it > 1 }.keys + if (stillTied.isEmpty()) return resolved + return resolved.mapValues { (fullName, label) -> if (label in stillTied) "$label [$fullName]" else label } +} + +/** + * The immediate parent segment of [fullName] (its location), or null when the folder is top-level. + * [displayName] is the leaf, so the character just before it in [fullName] is the server's hierarchy + * separator and the segment before that is the parent. + */ +private fun parentOf(fullName: String, displayName: String): String? { + if (fullName.length <= displayName.length || !fullName.endsWith(displayName)) return null + val separator = fullName[fullName.length - displayName.length - 1] + val parentPath = fullName.substring(0, fullName.length - displayName.length - 1) + return parentPath.substringAfterLast(separator).ifEmpty { null } +} diff --git a/app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt b/app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt index aa623dc..d426cfd 100644 --- a/app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt +++ b/app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt @@ -3,6 +3,8 @@ package org.libremail.domain.model import org.junit.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue class FolderRoleTest { @@ -30,4 +32,14 @@ class FolderRoleTest { assertEquals(FolderRole.ARCHIVE, FolderRole.roleOf("Archive", "Archive", emptyList())) assertEquals(FolderRole.NORMAL, FolderRole.roleOf("Receipts", "Receipts", emptyList())) } + + @Test + fun `isServerSpecial is true only for special-use attributes`() { + assertTrue(FolderRole.isServerSpecial(listOf("\\Junk"))) + assertTrue(FolderRole.isServerSpecial(listOf("\\All"))) + // Case-insensitive, and ignores non-special-use flags mixed in. + assertTrue(FolderRole.isServerSpecial(listOf("\\HasNoChildren", "\\drafts"))) + assertFalse(FolderRole.isServerSpecial(emptyList())) + assertFalse(FolderRole.isServerSpecial(listOf("\\HasNoChildren"))) + } } diff --git a/app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt b/app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt new file mode 100644 index 0000000..ef4c4da --- /dev/null +++ b/app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.ui.mailbox + +import org.junit.Test +import org.libremail.domain.model.Account +import org.libremail.domain.model.AuthType +import org.libremail.domain.model.Folder +import org.libremail.domain.model.FolderRole +import org.libremail.domain.model.MailSecurity +import org.libremail.domain.model.ServerConfig +import kotlin.test.assertEquals + +class FolderLabelsTest { + + private fun folder( + fullName: String, + displayName: String = fullName.substringAfterLast('/'), + role: FolderRole = FolderRole.NORMAL, + specialUse: Boolean = false, + ) = Folder("acct", fullName, displayName, role, selectable = true, specialUse = specialUse) + + private fun account(email: String, imapHost: String, authType: AuthType = AuthType.PASSWORD_IMAP) = + Account( + id = "acct", + email = email, + displayName = email, + authType = authType, + imap = ServerConfig(imapHost, 993, MailSecurity.SSL_TLS), + smtp = ServerConfig("smtp", 587, MailSecurity.STARTTLS), + ) + + /** Builds base labels the way the drawer does: the friendly role name, else the raw display name. */ + private fun baseLabelsOf(folders: List, friendly: Map) = + folders.associate { it.fullName to (friendly[it.role] ?: it.displayName) } + + private val friendlyNames = mapOf( + FolderRole.INBOX to "Inbox", + FolderRole.SENT to "Sent", + FolderRole.DRAFTS to "Drafts", + FolderRole.ARCHIVE to "Archive", + FolderRole.SPAM to "Spam", + FolderRole.TRASH to "Trash", + ) + + private fun resolve(folders: List, provider: String) = + resolveDrawerLabels(folders, baseLabelsOf(folders, friendlyNames), provider) + + @Test + fun `provider special folder gets the suffix and the user folder keeps its name`() { + val folders = listOf( + folder("[Gmail]/Drafts", "Drafts", FolderRole.DRAFTS, specialUse = true), + folder("Drafts", "Drafts", FolderRole.DRAFTS), + ) + val labels = resolve(folders, "Gmail") + assertEquals("Drafts - Gmail", labels["[Gmail]/Drafts"]) + assertEquals("Drafts", labels["Drafts"]) + } + + @Test + fun `all three reported Gmail duplicates are disambiguated`() { + val folders = listOf( + folder("[Gmail]/Drafts", "Drafts", FolderRole.DRAFTS, specialUse = true), + folder("Drafts", "Drafts", FolderRole.DRAFTS), + folder("[Gmail]/All Mail", "All Mail", FolderRole.ARCHIVE, specialUse = true), + folder("Archive", "Archive", FolderRole.ARCHIVE), + folder("[Gmail]/Spam", "Spam", FolderRole.SPAM, specialUse = true), + folder("Spam", "Spam", FolderRole.SPAM), + ) + val labels = resolve(folders, "Gmail") + assertEquals("Drafts - Gmail", labels["[Gmail]/Drafts"]) + assertEquals("Drafts", labels["Drafts"]) + assertEquals("Archive - Gmail", labels["[Gmail]/All Mail"]) + assertEquals("Archive", labels["Archive"]) + assertEquals("Spam - Gmail", labels["[Gmail]/Spam"]) + assertEquals("Spam", labels["Spam"]) + } + + @Test + fun `two nested user folders with the same leaf get their parent location`() { + val folders = listOf( + folder("Work/Reports"), + folder("Personal/Reports"), + ) + val labels = resolve(folders, "example.org") + assertEquals("Reports (Work)", labels["Work/Reports"]) + assertEquals("Reports (Personal)", labels["Personal/Reports"]) + } + + @Test + fun `a top-level user folder keeps its name against a nested sibling`() { + val folders = listOf( + folder("Reports"), + folder("Work/Reports"), + ) + val labels = resolve(folders, "example.org") + assertEquals("Reports", labels["Reports"]) + assertEquals("Reports (Work)", labels["Work/Reports"]) + } + + @Test + fun `unique labels are left untouched`() { + val folders = listOf( + folder("[Gmail]/Drafts", "Drafts", FolderRole.DRAFTS, specialUse = true), + folder("Receipts"), + ) + val labels = resolve(folders, "Gmail") + assertEquals("Drafts", labels["[Gmail]/Drafts"]) + assertEquals("Receipts", labels["Receipts"]) + } + + @Test + fun `two special folders for one role stay distinct via the full-path safety net`() { + val folders = listOf( + folder("[Gmail]/All Mail", "All Mail", FolderRole.ARCHIVE, specialUse = true), + folder("Archives", "Archives", FolderRole.ARCHIVE, specialUse = true), + ) + val labels = resolve(folders, "Gmail") + assertEquals(2, labels.values.toSet().size, "every drawer entry must be unique") + } + + @Test + fun `providerLabel resolves brands, Outlook, and falls back to the email domain`() { + assertEquals("Gmail", providerLabel(account("a@gmail.com", "imap.gmail.com"))) + assertEquals("iCloud Mail", providerLabel(account("a@icloud.com", "imap.mail.me.com"))) + assertEquals( + "Outlook", + providerLabel(account("a@outlook.com", "outlook.office365.com", AuthType.OAUTH_OUTLOOK)), + ) + assertEquals("example.org", providerLabel(account("alice@example.org", "imap.example.org"))) + } +} From e20981d083387ea4cf183325a7bfc7bbcfbd592e Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 1 Jul 2026 15:15:24 -0500 Subject: [PATCH 3/3] style(test): satisfy ktlint in new folder-label tests Body expression on the signature line (function-signature) and one argument per wrapped line (argument-list-wrapping) in the tests added for drawer folder de-duplication. Co-Authored-By: Claude Opus 4.8 --- .../data/local/LibreMailDatabaseTest.kt | 9 +++++++-- .../libremail/ui/mailbox/FolderLabelsTest.kt | 17 ++++++++--------- 2 files changed, 15 insertions(+), 11 deletions(-) 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 3019b28..61ce90d 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt @@ -133,8 +133,13 @@ class LibreMailDatabaseTest { "acct", listOf( FolderEntity( - "acct", "[Gmail]/Sent Mail", "Sent Mail", "SENT", - selectable = true, sortOrder = 1, specialUse = true, + accountId = "acct", + fullName = "[Gmail]/Sent Mail", + displayName = "Sent Mail", + role = "SENT", + selectable = true, + sortOrder = 1, + specialUse = true, ), FolderEntity("acct", "INBOX", "INBOX", "INBOX", selectable = true, sortOrder = 0), ), diff --git a/app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt b/app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt index ef4c4da..65644ed 100644 --- a/app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/mailbox/FolderLabelsTest.kt @@ -19,15 +19,14 @@ class FolderLabelsTest { specialUse: Boolean = false, ) = Folder("acct", fullName, displayName, role, selectable = true, specialUse = specialUse) - private fun account(email: String, imapHost: String, authType: AuthType = AuthType.PASSWORD_IMAP) = - Account( - id = "acct", - email = email, - displayName = email, - authType = authType, - imap = ServerConfig(imapHost, 993, MailSecurity.SSL_TLS), - smtp = ServerConfig("smtp", 587, MailSecurity.STARTTLS), - ) + private fun account(email: String, imapHost: String, authType: AuthType = AuthType.PASSWORD_IMAP) = Account( + id = "acct", + email = email, + displayName = email, + authType = authType, + imap = ServerConfig(imapHost, 993, MailSecurity.SSL_TLS), + smtp = ServerConfig("smtp", 587, MailSecurity.STARTTLS), + ) /** Builds base labels the way the drawer does: the friendly role name, else the raw display name. */ private fun baseLabelsOf(folders: List, friendly: Map) =