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 16cbccf..b4ed6cb 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt @@ -53,6 +53,34 @@ class LibreMailDatabaseTest { isStarred = false, ) + @Test + fun observeUnreadCountsAggregatesUnreadSyncedRowsPerAccountAndFolder() = runBlocking { + val messageDao = db.messageDao() + messageDao.insertNew( + listOf( + // acct / INBOX: two unread + one read -> counts 2. + message("acct:INBOX:1").copy(folder = "INBOX", isRead = false), + message("acct:INBOX:2").copy(folder = "INBOX", isRead = false), + message("acct:INBOX:3").copy(folder = "INBOX", isRead = true), + // acct / Archive: one unread -> counts 1. + message("acct:Archive:1").copy(folder = "Archive", isRead = false), + // An unread server-search hit (inInbox = false) must never inflate a badge. + message("acct:INBOX:search").copy(folder = "INBOX", isRead = false, inInbox = false), + // A second account's unread inbox row is counted under its own accountId. + message("acct2:INBOX:1").copy(accountId = "acct2", folder = "INBOX", isRead = false), + ), + ) + + val counts = messageDao.observeUnreadCounts().first() + .associate { (it.accountId to it.folder) to it.unreadCount } + + assertEquals(2, counts[("acct" to "INBOX")]) + assertEquals(1, counts[("acct" to "Archive")]) + assertEquals(1, counts[("acct2" to "INBOX")]) + // Fully-read folders and search-only rows produce no group at all. + assertEquals(3, counts.size) + } + @Test fun deletingMessageCascadesToItsAttachments() = runBlocking { val messageDao = db.messageDao() diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/MessageDaoRetentionTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/MessageDaoRetentionTest.kt index bb78a66..a4d750e 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/MessageDaoRetentionTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/MessageDaoRetentionTest.kt @@ -21,12 +21,12 @@ import org.libremail.data.local.entity.MessageEntity * * These queries *define* the device-only retention floor — the pruner deletes below it * ([MessageDao.syncedIdsBeyondCountInFolder] / [MessageDao.syncedIdsOlderThan]) and the backfiller - * stops above it ([MessageDao.lowestSyncedUid] / [MessageDao.countSynced] / - * [MessageDao.oldestSyncedTimestamp]) — so the whole "backfill and prune never fight over the same - * rows" guarantee rests on their SQL. The [org.libremail.data.sync.MailPruner] / - * [org.libremail.data.sync.MailBackfiller] unit tests mock the DAO, so the `ORDER BY … DESC LIMIT` - * newest-N selection, the strict age cutoff, and the windowed reconcile that spares backfilled history - * are exercised here against a real database instead. + * pages and stops around it ([MessageDao.lowestSyncedUid] / [MessageDao.countSynced]) — so the whole + * "backfill and prune never fight over the same rows" guarantee rests on their SQL. The + * [org.libremail.data.sync.MailPruner] / [org.libremail.data.sync.MailBackfiller] unit tests mock + * the DAO, so the `ORDER BY … DESC LIMIT` newest-N selection, the strict age cutoff, and the + * windowed reconcile that spares backfilled history are exercised here against a real database + * instead. */ @RunWith(AndroidJUnit4::class) class MessageDaoRetentionTest { @@ -153,6 +153,9 @@ class MessageDaoRetentionTest { /** * The backfiller's floor probes reflect only an account's synced rows in the given folder, and are * null/zero for a folder with nothing cached (so the backfiller then starts from `Long.MAX_VALUE`). + * The paging boundary additionally skips `uid <= 0` placeholder rows (#95): a row migrated before + * the `uid` column existed (backfilled to 0) or one whose UID the server failed to resolve (-1) + * must not collapse MIN(uid) to a bound the backfiller treats as "folder fully paged". */ @Test fun floorProbesReflectOnlySyncedRowsInTheFolder() = runBlocking { @@ -160,18 +163,22 @@ class MessageDaoRetentionTest { listOf( message("a", uid = 30, timestampMillis = 300), message("d", uid = 10, timestampMillis = 100), + message("legacy", uid = 0, timestampMillis = 40), // pre-uid-column migration row (#95) + message("unresolved", uid = -1, timestampMillis = 30), // UIDFolder.getUID failure (#95) message("search", uid = 1, timestampMillis = 1, inInbox = false), // excluded message("archive", uid = 5, timestampMillis = 50, folder = "Archive"), // different folder ), ) assertEquals(10L, dao.lowestSyncedUid("acct", "INBOX")) - assertEquals(2, dao.countSynced("acct", "INBOX")) - assertEquals(100L, dao.oldestSyncedTimestamp("acct", "INBOX")) + assertEquals(4, dao.countSynced("acct", "INBOX")) assertNull(dao.lowestSyncedUid("acct", "Nonexistent")) assertEquals(0, dao.countSynced("acct", "Nonexistent")) - assertNull(dao.oldestSyncedTimestamp("acct", "Nonexistent")) + + // A folder holding ONLY placeholder rows has no usable boundary: null (start from the top). + dao.insertNew(listOf(message("only-legacy", uid = 0, folder = "Imported"))) + assertNull(dao.lowestSyncedUid("acct", "Imported")) } /** Backfill / prune enumerate their targets via [syncedFolders]: distinct synced folders, per account. */ diff --git a/app/src/androidTest/kotlin/org/libremail/ui/Fakes.kt b/app/src/androidTest/kotlin/org/libremail/ui/Fakes.kt index c4dade3..b3adafa 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/Fakes.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/Fakes.kt @@ -14,6 +14,7 @@ import org.libremail.domain.model.Message import org.libremail.domain.model.OutboxMessage import org.libremail.domain.model.OutgoingMessage import org.libremail.domain.model.ReplyMode +import org.libremail.domain.model.UnreadCount import org.libremail.domain.repository.AccountRepository import org.libremail.domain.repository.MailRepository import java.io.File @@ -72,6 +73,7 @@ class FakeMailRepository( private val folders: List = emptyList(), private val attachments: List = emptyList(), private val downloadedParts: Set = emptySet(), + private val unreadCounts: List = emptyList(), ) : MailRepository { val sentMessages = mutableListOf() @@ -93,6 +95,8 @@ class FakeMailRepository( }, ) + override fun observeUnreadCounts(): Flow> = flowOf(unreadCounts) + override suspend fun refreshFolders(accountId: String): Result = Result.success(Unit) override suspend fun getMessage(id: String): Message? = messages.firstOrNull { it.id == id } 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 711d87c..2abb830 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/mailbox/FolderDrawerTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/mailbox/FolderDrawerTest.kt @@ -5,6 +5,7 @@ import androidx.activity.ComponentActivity import androidx.compose.material3.ModalDrawerSheet import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.test.ext.junit.runners.AndroidJUnit4 @@ -145,10 +146,34 @@ class FolderDrawerTest { assertTrue(unifiedTapped) } + @Test + fun folderWithUnreadMail_showsCountBadge_andReadFolderShowsNone() { + setContent( + accounts = listOf(alice), + drawerAccount = alice, + folders = listOf( + folder("imap:a", "INBOX", "INBOX", FolderRole.INBOX), + folder("imap:a", "Archive", "Archive", FolderRole.ARCHIVE), + ), + folderUnreadCounts = mapOf("INBOX" to 3), + ) + + // The inbox badge announces its exact count for screen readers. + val threeUnread = composeTestRule.activity.resources + .getQuantityString(R.plurals.folder_unread_count_description, 3, 3) + composeTestRule.onNodeWithContentDescription(threeUnread).assertIsDisplayed() + // Archive has no unread mail, so no badge is rendered for it. + val oneUnread = composeTestRule.activity.resources + .getQuantityString(R.plurals.folder_unread_count_description, 1, 1) + composeTestRule.onNodeWithContentDescription(oneUnread).assertDoesNotExist() + } + private fun setContent( accounts: List, drawerAccount: Account?, folders: List, + folderUnreadCounts: Map = emptyMap(), + accountsWithUnread: Set = emptySet(), selectedAccountId: String? = null, selectedFolder: String = "INBOX", onSelectUnifiedInbox: () -> Unit = {}, @@ -162,6 +187,8 @@ class FolderDrawerTest { accounts = accounts, drawerAccount = drawerAccount, folders = folders, + folderUnreadCounts = folderUnreadCounts, + accountsWithUnread = accountsWithUnread, selectedAccountId = selectedAccountId, selectedFolder = selectedFolder, onSelectUnifiedInbox = onSelectUnifiedInbox, diff --git a/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OnboardingFlowTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OnboardingFlowTest.kt index b35a7bf..580543f 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OnboardingFlowTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OnboardingFlowTest.kt @@ -53,7 +53,7 @@ class OnboardingFlowTest { @get:Rule val composeTestRule = createAndroidComposeRule() - private fun string(resId: Int) = composeTestRule.activity.getString(resId) + private fun string(resId: Int, vararg args: Any) = composeTestRule.activity.getString(resId, *args) // Generous cap for the slow, animation-disabled CI matrix emulators; waitUntil returns as soon // as the text appears, so the happy path is unaffected. @@ -206,6 +206,10 @@ class OnboardingFlowTest { // add" button sit below the fold of this scrolling screen, and a positional click on an // off-screen button is a silent no-op (which is why this passed only on API 37's taller AVD). waitForText(string(R.string.app_password_email)) + // Gmail requires 2-Step Verification before app passwords, so its screen (and only its + // screen — see yahooSetup_hasNoTwoFactorHelpLink) links Google's setup article (issue #98). + composeTestRule.onNodeWithText(string(R.string.app_password_2fa_help)) + .performScrollTo().assertIsDisplayed() composeTestRule.onNodeWithText(string(R.string.app_password_email)) .performScrollTo().performTextInput("e2e@gmail.com") composeTestRule.onNodeWithText(string(R.string.app_password_field)) @@ -221,4 +225,18 @@ class OnboardingFlowTest { waitForText("E2E first message") composeTestRule.onNodeWithText("E2E first message").assertIsDisplayed() } + + @Test + fun yahooSetup_hasNoTwoFactorHelpLink() { + setOnboardingContent(FakeAccountRepository(), FakeMailRepository()) + + composeTestRule.onNodeWithText(string(R.string.onboarding_add_account)).performClick() + waitForText("Yahoo Mail") + composeTestRule.onNodeWithText("Yahoo Mail").performClick() + + // Yahoo's setup screen keeps its app-password link… + waitForText(string(R.string.app_password_open_page, "Yahoo Mail")) + // …but gains no 2-Step Verification link: that prerequisite is Gmail-specific (issue #98). + composeTestRule.onNodeWithText(string(R.string.app_password_2fa_help)).assertDoesNotExist() + } } 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 36b3ed2..20e1173 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt @@ -10,6 +10,7 @@ import androidx.lifecycle.SavedStateHandle import androidx.room.Room import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.work.WorkManager import kotlinx.coroutines.runBlocking import org.junit.Rule import org.junit.Test @@ -27,6 +28,7 @@ import org.libremail.domain.model.ServerConfig import org.libremail.ui.FakeAccountRepository import org.libremail.ui.navigation.Routes import org.libremail.ui.theme.LibreMailTheme +import javax.inject.Provider /** * End-to-end test for the per-account settings screen: editing the signature and toggling the @@ -68,7 +70,7 @@ class AccountSettingsScreenTest { accountRepository = FakeAccountRepository(accounts = listOf(account)), accountSettingsRepository = repository, signatureRepository = SignatureRepository(db.signatureDao()), - syncScheduler = SyncScheduler(context), + syncScheduler = SyncScheduler(Provider { WorkManager.getInstance(context) }), ) composeTestRule.setContent { LibreMailTheme(darkTheme = false, dynamicColor = false) { diff --git a/app/src/androidTest/kotlin/org/libremail/ui/settings/SettingsScreenTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/settings/SettingsScreenTest.kt index 84d2832..4db2142 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/settings/SettingsScreenTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/settings/SettingsScreenTest.kt @@ -8,6 +8,7 @@ import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performScrollTo import androidx.test.ext.junit.runners.AndroidJUnit4 import androidx.test.platform.app.InstrumentationRegistry +import androidx.work.WorkManager import kotlinx.coroutines.runBlocking import org.junit.Rule import org.junit.Test @@ -25,6 +26,7 @@ import org.libremail.data.sync.SyncScheduler import org.libremail.push.BatteryOptimizationManager import org.libremail.ui.FakeAccountRepository import org.libremail.ui.theme.LibreMailTheme +import javax.inject.Provider /** * End-to-end test for the top-level message-downloading setting: tapping a policy must round-trip @@ -54,7 +56,7 @@ class SettingsScreenTest { appLockManager, keyStore, BatteryOptimizationManager(context), - SyncScheduler(context), + SyncScheduler(Provider { WorkManager.getInstance(context) }), ) composeTestRule.setContent { 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 3247fbe..c05bbb9 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Mappers.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Mappers.kt @@ -8,6 +8,7 @@ import org.libremail.data.local.entity.AccountSettingsEntity 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.FolderUnreadCount import org.libremail.data.local.entity.MessageEntity import org.libremail.data.local.entity.MessageSummary import org.libremail.data.local.entity.OutboxEntity @@ -26,6 +27,7 @@ import org.libremail.domain.model.OutboxMessage import org.libremail.domain.model.OutgoingAttachment import org.libremail.domain.model.ServerConfig import org.libremail.domain.model.SmtpParams +import org.libremail.domain.model.UnreadCount import org.libremail.mail.AttachmentPart import org.libremail.mail.FetchedFolder import org.libremail.mail.FetchedMessage @@ -168,6 +170,12 @@ internal fun FetchedFolder.toEntity(accountId: String, sortOrder: Int): FolderEn specialUse = FolderRole.isServerSpecial(attributes), ) +internal fun FolderUnreadCount.toDomain(): UnreadCount = UnreadCount( + accountId = accountId, + folder = folder, + count = unreadCount, +) + internal fun AttachmentEntity.toDomain(): Attachment = Attachment( messageId = messageId, partIndex = partIndex, 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 51e3704..b0399cc 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 @@ -6,6 +6,7 @@ import androidx.room.Insert import androidx.room.OnConflictStrategy import androidx.room.Query import kotlinx.coroutines.flow.Flow +import org.libremail.data.local.entity.FolderUnreadCount import org.libremail.data.local.entity.MessageEntity import org.libremail.data.local.entity.MessageSummary @@ -23,6 +24,19 @@ interface MessageDao { ) fun observeSummaries(): Flow> + /** + * Live per-(account, folder) unread counts for the drawer's folder badges and the bold styling of + * accounts with unread mail. Counts only folder-synced rows (`inInbox = 1`), so transient + * server-search hits never inflate a badge; read rows and folders with no unread mail are simply + * absent from the result. A pure `COUNT(*)` aggregate — no message rows are pulled into memory — + * whose `GROUP BY accountId, folder` is served by the existing `(accountId, folder, uid)` index. + */ + @Query( + "SELECT accountId, folder, COUNT(*) AS unreadCount FROM messages " + + "WHERE inInbox = 1 AND isRead = 0 GROUP BY accountId, folder", + ) + fun observeUnreadCounts(): Flow> + @Query("SELECT * FROM messages WHERE id = :id LIMIT 1") suspend fun getById(id: String): MessageEntity? @@ -104,21 +118,25 @@ interface MessageDao { ) suspend fun deleteSyncedInWindowNotIn(accountId: String, folder: String, minWindowUid: Long, keepIds: List) - /** Lowest cached UID among an account's synced rows in [folder] — the backfill boundary. Null if none. */ - @Query("SELECT MIN(uid) FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1") + /** + * Lowest cached *resolved* UID among an account's synced rows in [folder] — the backfill + * boundary. Placeholder rows with `uid <= 0` (a row migrated before the `uid` column existed, or + * a fetch where the server failed to resolve the UID) are excluded: letting one collapse + * MIN(uid) to `<= 0` would make the backfiller page below a bound `fetchOlderThan` treats as + * "nothing older", falsely marking the folder fully backfilled (#95, matching the + * `minWindowUid` guard in MailSyncer). Null when no resolved-UID row exists, in which case + * backfill starts over from the newest message. + */ + @Query( + "SELECT MIN(uid) FROM messages WHERE accountId = :accountId AND folder = :folder " + + "AND inInbox = 1 AND uid > 0", + ) suspend fun lowestSyncedUid(accountId: String, folder: String): Long? /** Number of an account's synced rows in [folder] (count-based retention floor / prune sizing). */ @Query("SELECT COUNT(*) FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1") suspend fun countSynced(accountId: String, folder: String): Int - /** Oldest cached timestamp among an account's synced rows in [folder] (age-based retention floor). Null if none. */ - @Query( - "SELECT MIN(timestampMillis) FROM messages " + - "WHERE accountId = :accountId AND folder = :folder AND inInbox = 1", - ) - suspend fun oldestSyncedTimestamp(accountId: String, folder: String): Long? - /** Distinct folders that have at least one synced row for [accountId] (backfill/prune targets). */ @Query("SELECT DISTINCT folder FROM messages WHERE accountId = :accountId AND inInbox = 1") suspend fun syncedFolders(accountId: String): List diff --git a/app/src/main/kotlin/org/libremail/data/local/entity/FolderUnreadCount.kt b/app/src/main/kotlin/org/libremail/data/local/entity/FolderUnreadCount.kt new file mode 100644 index 0000000..9f53a00 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/local/entity/FolderUnreadCount.kt @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local.entity + +/** + * Aggregate projection of [MessageEntity] counting unread, folder-synced rows per (account, folder). + * Produced by `MessageDao.observeUnreadCounts`' `GROUP BY` query — never a stored table — so the + * counts come straight from SQLite without pulling any message rows into memory. + */ +data class FolderUnreadCount(val accountId: String, val folder: String, val unreadCount: Int) 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 ba3f629..61ff531 100644 --- a/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt +++ b/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt @@ -36,6 +36,7 @@ import org.libremail.domain.model.OutboxMessage import org.libremail.domain.model.OutgoingAttachment import org.libremail.domain.model.OutgoingMessage import org.libremail.domain.model.ReplyMode +import org.libremail.domain.model.UnreadCount import org.libremail.domain.repository.MailRepository import org.libremail.mail.ImapClient import java.io.File @@ -68,6 +69,10 @@ class MailRepositoryImpl @Inject constructor( rows.map { it.toDomain() } } + override fun observeUnreadCounts(): Flow> = messageDao.observeUnreadCounts().map { rows -> + rows.map { it.toDomain() } + } + override suspend fun refreshFolders(accountId: String): Result = runCatching { val account = accountDao.getById(accountId)?.toDomain() ?: error("Account not found") val params = connectionFactory.imapParamsFor(account) diff --git a/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt b/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt index 8a2e822..7d8fa92 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt @@ -54,12 +54,15 @@ class MailBackfiller @Inject constructor( private val mailRepository: MailRepository, private val maintenanceGate: MailMaintenanceGate, ) { - private data class FolderResult(val batches: Int, val complete: Boolean) + /** One folder's slice outcome: pages fetched, and whether an immediate follow-up slice has work to do. */ + private data class FolderResult(val batches: Int, val moreWork: Boolean) /** * Runs one bounded slice of backfill across all accounts and their synced folders. Does at most - * [maxBatches] server pages total, persisting progress after each, then returns whether any - * folder still has history left to fetch (so the caller may schedule another run sooner). + * [maxBatches] server pages total, persisting progress after each, then returns whether an + * immediate follow-up slice has more work to do (so the caller may chain another run). A folder + * that stalled (see the unresolved-UID guard in [backfillFolder]) stays incomplete but does not + * count as more work — it is retried on a future scheduled run instead of spun on back-to-back. */ suspend fun runBackfill(maxBatches: Int = DEFAULT_MAX_BATCHES): Boolean = maintenanceGate.mutex.withLock { var remaining = maxBatches @@ -71,9 +74,9 @@ class MailBackfiller @Inject constructor( if (remaining <= 0) return@withLock true // Per-folder failures (e.g. a transient server error) must not abort the whole slice. val result = runCatching { backfillFolder(account, params, folder, policy, remaining) } - .getOrElse { FolderResult(batches = 0, complete = false) } + .getOrElse { FolderResult(batches = 0, moreWork = true) } remaining -= result.batches - if (!result.complete) moreWork = true + if (result.moreWork) moreWork = true } } moreWork @@ -88,57 +91,88 @@ class MailBackfiller @Inject constructor( ): FolderResult { val progress = backfillProgressDao.get(account.id, folder) if (progress?.complete == true) { - return FolderResult(batches = 0, complete = true) + return FolderResult(batches = 0, moreWork = false) } // Resume from the persisted low-water mark so paging is monotonic: it never re-descends into a // region an earlier run already reached, even after the pruner deletes rows above it. Falls back // to the lowest currently-cached UID on the very first run, before any progress is persisted. - var beforeUid = progress?.nextBeforeUid + // Both sources are guarded against unresolved-UID placeholders (#95): lowestSyncedUid excludes + // `uid <= 0` rows at the SQL level, and a stale persisted boundary `<= 0` is discarded rather + // than trusted — fetchOlderThan treats such a bound as "nothing older", which would falsely + // mark the folder fully backfilled. + var beforeUid = progress?.nextBeforeUid?.takeIf { it > 0L } ?: messageDao.lowestSyncedUid(account.id, folder) ?: Long.MAX_VALUE var batches = 0 + var complete = false + var stalled = false while (batches < maxBatches) { currentCoroutineContext().ensureActive() - // Retention floor (#13 precedence): once the folder holds everything retention keeps, mark it - // complete and stop. Marking complete (rather than pausing) is what keeps backfill and the - // pruner from fighting: otherwise the pruner deleting aged-out rows would raise the oldest - // cached timestamp back above the age cutoff and re-open paging on the next run, forever. A - // retention change resets progress (AccountRepository.resetBackfillProgress) so loosening - // still resumes paging. - if (reachedRetentionFloor(account.id, folder, policy)) { - markComplete(account.id, folder, beforeUid) - return FolderResult(batches, complete = true) + // Count floor (#13 precedence): once the folder holds as many messages as retention keeps, + // stop before fetching another page. Unlike the age floor below, this can be decided from + // the cache alone: the count pruner keeps the newest-N by UID — exactly the order paging + // descends in — so a Date/UID inversion cannot make it stop early. + if (reachedCountFloor(account.id, folder, policy)) { + complete = true + break } val fetched = imapClient.fetchOlderThan(params, folder, beforeUid, BACKFILL_BATCH_SIZE) batches++ - if (fetched.isEmpty()) { - // Genuine end of the folder — mark complete so it is skipped on future runs. - markComplete(account.id, folder, beforeUid) - return FolderResult(batches, complete = true) - } val entities = fetched.map { it.toEntity(account.id, folder) } + // Stop at the genuine end of the folder, or at the age floor (#13): a page ENTIRELY older + // than the Date cutoff. The age decision is made from the page actually fetched, NOT from + // the oldest cached timestamp — paging is by UID (arrival order) while the floor cuts by + // the Date header, so a single high-UID message with an old Date (moved/imported mail) + // would drag the cached minimum below the cutoff and end paging while within-retention + // history is still unfetched (#94). An entirely-old page is pure prune-fodder, so it is + // not persisted either. Both cases mark the folder complete (rather than pausing): the + // pruner deleting aged-out rows must never re-open paging on the next run, forever + // re-downloading what was just pruned. A retention change resets progress + // (AccountRepository.resetBackfillProgress) so loosening still resumes paging. + if (fetched.isEmpty() || entirelyBeyondAgeFloor(entities, policy)) { + complete = true + break + } persistBatch(entities) - beforeUid = entities.minOf { it.uid } + // Derive the next boundary only from resolved UIDs (#95): a row whose UID the server + // failed to resolve (UIDFolder.getUID returns -1) must not collapse the boundary to <= 0, + // where fetchOlderThan reads "nothing older" and the folder would be FALSELY marked fully + // backfilled. If a whole page came back unresolved, stall the folder: not complete (so a + // later run retries once the server behaves) but claiming no more work either — an + // immediate follow-up slice would just spin on the same page. + val nextBeforeUid = entities.mapNotNull { entity -> entity.uid.takeIf { it > 0L } }.minOrNull() + if (nextBeforeUid == null) { + stalled = true + break + } + beforeUid = nextBeforeUid backfillProgressDao.upsert(BackfillProgressEntity(account.id, folder, beforeUid, complete = false)) prefetchIfEnabled(entities.map { it.id }) // Breathe between pages so a large mailbox doesn't hammer the server. delay(BACKFILL_BATCH_DELAY_MS) } - return FolderResult(batches, complete = false) + if (complete) markComplete(account.id, folder, beforeUid) + return FolderResult(batches, moreWork = !complete && !stalled) } - /** True once the folder already holds as much as the retention policy would keep (or more). */ - private suspend fun reachedRetentionFloor(accountId: String, folder: String, policy: RetentionPolicy): Boolean { - if (policy.isUnlimited) return false - policy.countLimit?.let { limit -> - if (messageDao.countSynced(accountId, folder) >= limit) return true - } - policy.ageCutoffMillis(System.currentTimeMillis())?.let { cutoff -> - val oldest = messageDao.oldestSyncedTimestamp(accountId, folder) - if (oldest != null && oldest < cutoff) return true - } - return false + /** True once the folder already holds as many messages as the count retention keeps (or more). */ + private suspend fun reachedCountFloor(accountId: String, folder: String, policy: RetentionPolicy): Boolean { + val limit = policy.countLimit ?: return false + return messageDao.countSynced(accountId, folder) >= limit + } + + /** + * True when a fetched page sits entirely below the age retention floor — every message on it is + * older than the policy's Date cutoff. Deciding per page (rather than from the single oldest + * cached timestamp) makes the floor robust to Date/UID inversions (#94): one old-Dated high-UID + * message ends paging only if a whole page around it is old too. The trade is deliberate — a + * pathologically interleaved mailbox may over-fetch (the pruner reclaims the excess), but + * backfill never silently gaps within-retention history. + */ + private fun entirelyBeyondAgeFloor(page: List, policy: RetentionPolicy): Boolean { + val cutoff = policy.ageCutoffMillis(System.currentTimeMillis()) ?: return false + return page.isNotEmpty() && page.all { it.timestampMillis < cutoff } } /** Inserts backfilled headers; never deletes. Uncancellable so a persisted boundary always has its rows. */ diff --git a/app/src/main/kotlin/org/libremail/data/sync/SyncScheduler.kt b/app/src/main/kotlin/org/libremail/data/sync/SyncScheduler.kt index 5cd91b4..d23806f 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/SyncScheduler.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/SyncScheduler.kt @@ -1,7 +1,6 @@ // SPDX-License-Identifier: GPL-3.0-or-later package org.libremail.data.sync -import android.content.Context import androidx.work.Constraints import androidx.work.ExistingPeriodicWorkPolicy import androidx.work.ExistingWorkPolicy @@ -10,15 +9,20 @@ import androidx.work.OneTimeWorkRequestBuilder import androidx.work.OutOfQuotaPolicy import androidx.work.PeriodicWorkRequestBuilder import androidx.work.WorkManager -import dagger.hilt.android.qualifiers.ApplicationContext import java.util.concurrent.TimeUnit import javax.inject.Inject +import javax.inject.Provider import javax.inject.Singleton /** Schedules background mail sync, full-history backfill, and retention pruning via WorkManager. */ @Singleton -class SyncScheduler @Inject constructor(@ApplicationContext private val context: Context) { - private val workManager get() = WorkManager.getInstance(context) +class SyncScheduler @Inject constructor( + // A Provider (not the WorkManager itself) so WorkManager.getInstance() is resolved lazily at + // schedule time — never during Hilt's Application field injection, which can run before the + // HiltWorkerFactory that on-demand WorkManager initialization needs is set. + private val workManagerProvider: Provider, +) { + private val workManager get() = workManagerProvider.get() private val networkConstraint = Constraints.Builder() .setRequiredNetworkType(NetworkType.CONNECTED) @@ -41,7 +45,7 @@ class SyncScheduler @Inject constructor(@ApplicationContext private val context: val request = PeriodicWorkRequestBuilder(15, TimeUnit.MINUTES) .setConstraints(networkConstraint) .build() - workManager.enqueueUniquePeriodicWork(PERIODIC_WORK, ExistingPeriodicWorkPolicy.KEEP, request) + workManager.enqueueUniquePeriodicWork(PERIODIC_WORK, PERIODIC_POLICY, request) } /** One-shot sync, e.g. right after an account is added. */ @@ -55,13 +59,13 @@ class SyncScheduler @Inject constructor(@ApplicationContext private val context: /** * Periodic full-history backfill (issue #12). Each run pages a bounded slice and persists its - * boundary, so history fills in over successive runs; KEEP preserves an already-scheduled cadence. + * boundary, so history fills in over successive runs. */ fun schedulePeriodicBackfill() { val request = PeriodicWorkRequestBuilder(30, TimeUnit.MINUTES) .setConstraints(backfillConstraint) .build() - workManager.enqueueUniquePeriodicWork(PERIODIC_BACKFILL, ExistingPeriodicWorkPolicy.KEEP, request) + workManager.enqueueUniquePeriodicWork(PERIODIC_BACKFILL, PERIODIC_POLICY, request) } /** Kicks an immediate backfill slice (e.g. just after an account is added) without waiting for the cadence. */ @@ -79,7 +83,7 @@ class SyncScheduler @Inject constructor(@ApplicationContext private val context: val request = PeriodicWorkRequestBuilder(12, TimeUnit.HOURS) .setConstraints(pruneConstraint) .build() - workManager.enqueueUniquePeriodicWork(PERIODIC_PRUNE, ExistingPeriodicWorkPolicy.KEEP, request) + workManager.enqueueUniquePeriodicWork(PERIODIC_PRUNE, PERIODIC_POLICY, request) } /** Runs pruning promptly, e.g. right after the user tightens a retention limit. */ @@ -95,5 +99,13 @@ class SyncScheduler @Inject constructor(@ApplicationContext private val context: const val ONESHOT_BACKFILL = "libremail_oneshot_backfill" const val PERIODIC_PRUNE = "libremail_periodic_prune" const val ONESHOT_PRUNE = "libremail_oneshot_prune" + + // UPDATE, not KEEP (issue #96). These periodic jobs are re-enqueued at every app start, so KEEP + // pinned an already-installed device to the interval/constraints from the version that first + // scheduled it — later tuning never reached upgraders. UPDATE re-applies the current spec while + // preserving the running period's progress: an unchanged spec is effectively a no-op, so this + // does NOT reset the schedule on launch the way REPLACE (cancel + re-enqueue) would. UPDATE is + // available since WorkManager 2.8. + val PERIODIC_POLICY = ExistingPeriodicWorkPolicy.UPDATE } } diff --git a/app/src/main/kotlin/org/libremail/di/WorkManagerModule.kt b/app/src/main/kotlin/org/libremail/di/WorkManagerModule.kt new file mode 100644 index 0000000..5df1c41 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/di/WorkManagerModule.kt @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.di + +import android.content.Context +import androidx.work.WorkManager +import dagger.Module +import dagger.Provides +import dagger.hilt.InstallIn +import dagger.hilt.android.qualifiers.ApplicationContext +import dagger.hilt.components.SingletonComponent +import javax.inject.Singleton + +/** + * Provides the process-wide [WorkManager] so schedulers can inject it (and unit tests can substitute + * a fake) instead of reaching for the [WorkManager.getInstance] static directly. + */ +@Module +@InstallIn(SingletonComponent::class) +object WorkManagerModule { + + @Provides + @Singleton + fun provideWorkManager(@ApplicationContext context: Context): WorkManager = WorkManager.getInstance(context) +} 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 927b32e..20f25a8 100644 --- a/app/src/main/kotlin/org/libremail/domain/model/MailProvider.kt +++ b/app/src/main/kotlin/org/libremail/domain/model/MailProvider.kt @@ -29,6 +29,12 @@ enum class MailProvider( val displayName: String, /** The page where the user creates an app password for this provider. */ val appPasswordHelpUrl: String, + /** + * Setup instructions for the provider's two-factor prerequisite, or null when there isn't one. + * Only Gmail refuses to create app passwords until 2-Step Verification is on, so only Gmail + * links its setup article; Yahoo and iCloud gate nothing on it. + */ + val twoFactorHelpUrl: String? = null, private val imapHost: String, private val smtpHost: String, private val smtpPort: Int, @@ -38,6 +44,9 @@ enum class MailProvider( key = "gmail", displayName = "Gmail", appPasswordHelpUrl = "https://myaccount.google.com/apppasswords", + // Google's "Turn on 2-Step Verification" article — the app-passwords page above bounces + // accounts that haven't enabled it yet, so the setup screen offers this as a way out. + twoFactorHelpUrl = "https://support.google.com/accounts/answer/185839", imapHost = "imap.gmail.com", smtpHost = "smtp.gmail.com", // Google documents smtp.gmail.com:587 with STARTTLS as the standard submission endpoint. diff --git a/app/src/main/kotlin/org/libremail/domain/model/UnreadCount.kt b/app/src/main/kotlin/org/libremail/domain/model/UnreadCount.kt new file mode 100644 index 0000000..3e0db33 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/domain/model/UnreadCount.kt @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.domain.model + +/** + * The number of unread, folder-synced messages in one account's folder. Emitted per (account, folder) + * pair that currently holds unread mail; pairs with no unread mail are simply absent. Feeds the + * drawer's per-folder unread badges (#83) and the bold styling of accounts that have unread mail (#84). + */ +data class UnreadCount(val accountId: String, val folder: String, val count: Int) diff --git a/app/src/main/kotlin/org/libremail/domain/repository/MailRepository.kt b/app/src/main/kotlin/org/libremail/domain/repository/MailRepository.kt index 1a761b5..4126fc6 100644 --- a/app/src/main/kotlin/org/libremail/domain/repository/MailRepository.kt +++ b/app/src/main/kotlin/org/libremail/domain/repository/MailRepository.kt @@ -9,6 +9,7 @@ import org.libremail.domain.model.Message import org.libremail.domain.model.OutboxMessage import org.libremail.domain.model.OutgoingMessage import org.libremail.domain.model.ReplyMode +import org.libremail.domain.model.UnreadCount import java.io.File /** @@ -21,6 +22,13 @@ interface MailRepository { /** The account's cached IMAP folders for the navigation drawer. */ fun observeFolders(accountId: String): Flow> + /** + * Live per-(account, folder) unread counts across every account, for the drawer's folder badges + * and the bold styling of accounts with unread mail. Only (account, folder) pairs that currently + * hold unread, folder-synced mail are emitted. + */ + fun observeUnreadCounts(): Flow> + /** Refreshes the account's folder list from the server into the cache. */ suspend fun refreshFolders(accountId: String): Result diff --git a/app/src/main/kotlin/org/libremail/push/IdleService.kt b/app/src/main/kotlin/org/libremail/push/IdleService.kt index a65307c..336dee2 100644 --- a/app/src/main/kotlin/org/libremail/push/IdleService.kt +++ b/app/src/main/kotlin/org/libremail/push/IdleService.kt @@ -134,8 +134,9 @@ class IdleService : Service() { startAsForeground(mode) if (mode == PushMode.POLLING) { AppLog.i(TAG, "Battery low: pausing IMAP IDLE; mail arrives via 15-minute periodic sync") - // The periodic fallback is scheduled at every app start with KEEP, so this is normally a - // no-op — re-asserted here so the fallback provably exists whenever push is paused. + // The periodic fallback is already scheduled at every app start (UPDATE), so re-asserting it + // here is effectively a no-op — done anyway so the fallback provably exists whenever push is + // paused, without disturbing the running period. syncScheduler.schedulePeriodicSync() } else { AppLog.i(TAG, "Battery recovered: resuming IMAP IDLE push") diff --git a/app/src/main/kotlin/org/libremail/ui/accountsetup/AppPasswordSetupScreen.kt b/app/src/main/kotlin/org/libremail/ui/accountsetup/AppPasswordSetupScreen.kt index ff910e9..1a640ba 100644 --- a/app/src/main/kotlin/org/libremail/ui/accountsetup/AppPasswordSetupScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/accountsetup/AppPasswordSetupScreen.kt @@ -78,6 +78,12 @@ fun AppPasswordSetupScreen( val scope = rememberCoroutineScope() // Resolved up front so the failure handler (a non-composable lambda) can use it. val openFailedMessage = stringResource(R.string.app_password_open_failed) + // Shared by every outbound link on this screen: openUri throws if no browser/handler is + // installed, so surface that as an inline snackbar instead of crashing. + val openUrl: (String) -> Unit = { url -> + runCatching { uriHandler.openUri(url) } + .onFailure { scope.launch { snackbarHostState.showSnackbar(openFailedMessage) } } + } LaunchedEffect(form.status, form.addedAccountId) { if (form.status == SetupStatus.DONE) { @@ -146,15 +152,23 @@ fun AppPasswordSetupScreen( Spacer(Modifier.height(12.dp)) OutlinedButton( - onClick = { - // openUri throws if no browser/handler is installed; surface it instead of crashing. - runCatching { uriHandler.openUri(provider.appPasswordHelpUrl) } - .onFailure { scope.launch { snackbarHostState.showSnackbar(openFailedMessage) } } - }, + onClick = { openUrl(provider.appPasswordHelpUrl) }, modifier = Modifier.fillMaxWidth(), ) { Text(stringResource(R.string.app_password_open_page, provider.displayName)) } + // Only Gmail has a two-factor prerequisite (see MailProvider.twoFactorHelpUrl): its + // app-passwords page rejects accounts without 2-Step Verification, so give those users + // a way to set it up instead of a dead end. + provider.twoFactorHelpUrl?.let { twoFactorHelpUrl -> + Spacer(Modifier.height(8.dp)) + OutlinedButton( + onClick = { openUrl(twoFactorHelpUrl) }, + modifier = Modifier.fillMaxWidth(), + ) { + Text(stringResource(R.string.app_password_2fa_help)) + } + } Spacer(Modifier.height(20.dp)) OutlinedTextField( 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 de9a918..8695334 100644 --- a/app/src/main/kotlin/org/libremail/ui/mailbox/FolderDrawer.kt +++ b/app/src/main/kotlin/org/libremail/ui/mailbox/FolderDrawer.kt @@ -27,7 +27,11 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.vector.ImageVector +import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.clearAndSetSemantics +import androidx.compose.ui.semantics.contentDescription +import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import org.libremail.R @@ -37,13 +41,17 @@ import org.libremail.domain.model.FolderRole /** * The navigation drawer's contents: an optional account switcher and "All Inboxes" entry (only with - * 2+ accounts), then the drawer account's folders — standard folders (Inbox, Sent, …) first. + * 2+ accounts), then the drawer account's folders — standard folders (Inbox, Sent, …) first. Each + * folder shows its unread count as a trailing badge ([folderUnreadCounts]), and accounts with unread + * mail ([accountsWithUnread]) render their email in bold. */ @Composable fun FolderDrawer( accounts: List, drawerAccount: Account?, folders: List, + folderUnreadCounts: Map, + accountsWithUnread: Set, selectedAccountId: String?, selectedFolder: String, onSelectUnifiedInbox: () -> Unit, @@ -63,7 +71,7 @@ fun FolderDrawer( ) if (accounts.size >= 2 && drawerAccount != null) { - AccountSwitcher(accounts, drawerAccount, onSelectDrawerAccount) + AccountSwitcher(accounts, drawerAccount, accountsWithUnread, onSelectDrawerAccount) NavigationDrawerItem( label = { Text(stringResource(R.string.folder_all_inboxes)) }, icon = { Icon(Icons.Filled.Email, contentDescription = null) }, @@ -87,9 +95,15 @@ fun FolderDrawer( val iconContent: (@Composable () -> Unit)? = folderIcon(folder.role)?.let { vector -> { Icon(vector, contentDescription = null) } } + val unread = folderUnreadCounts[folder.fullName] ?: 0 NavigationDrawerItem( label = { Text(resolvedLabels[folder.fullName] ?: folderDisplayLabel(folder)) }, icon = iconContent, + badge = if (unread > 0) { + { UnreadBadgeLabel(unread) } + } else { + null + }, selected = isSelected, onClick = { if (folder.selectable && drawerAccount != null) { @@ -103,19 +117,34 @@ fun FolderDrawer( } @Composable -private fun AccountSwitcher(accounts: List, current: Account, onSelect: (String) -> Unit) { +private fun AccountSwitcher( + accounts: List, + current: Account, + accountsWithUnread: Set, + onSelect: (String) -> Unit, +) { var expanded by remember { mutableStateOf(false) } TextButton( onClick = { expanded = true }, modifier = Modifier.padding(horizontal = 16.dp), ) { - Text(current.email, maxLines = 1, overflow = TextOverflow.Ellipsis) + Text( + current.email, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + fontWeight = if (current.id in accountsWithUnread) FontWeight.Bold else FontWeight.Normal, + ) Icon(Icons.Filled.ArrowDropDown, contentDescription = stringResource(R.string.drawer_switch_account)) } DropdownMenu(expanded = expanded, onDismissRequest = { expanded = false }) { accounts.forEach { account -> DropdownMenuItem( - text = { Text(account.email) }, + text = { + Text( + account.email, + fontWeight = if (account.id in accountsWithUnread) FontWeight.Bold else FontWeight.Normal, + ) + }, onClick = { onSelect(account.id) expanded = false @@ -125,6 +154,29 @@ private fun AccountSwitcher(accounts: List, current: Account, onSelect: } } +/** Cap for a folder's visible unread badge; higher counts render as "99+" to keep the row compact. */ +private const val UNREAD_BADGE_CAP = 99 + +/** + * A folder row's trailing unread-count badge. The visible label is capped at [UNREAD_BADGE_CAP] as + * "99+" so a large count can't blow out the row, while the semantics announce the exact count as + * "N unread messages" (overriding the terse glyph) for screen readers. + */ +@Composable +private fun UnreadBadgeLabel(count: Int) { + val display = if (count > UNREAD_BADGE_CAP) { + stringResource(R.string.folder_unread_overflow) + } else { + count.toString() + } + val description = pluralStringResource(R.plurals.folder_unread_count_description, count, count) + Text( + text = display, + style = MaterialTheme.typography.labelMedium, + modifier = Modifier.clearAndSetSemantics { contentDescription = description }, + ) +} + /** * De-duplicated display labels for [folders], keyed by [Folder.fullName] — the one resolution used * by the drawer, the move-to picker, and the app-bar title, so a folder reads the same everywhere. diff --git a/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxScreen.kt b/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxScreen.kt index fc4f1aa..663aeaa 100644 --- a/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxScreen.kt @@ -106,6 +106,8 @@ fun MailboxScreen( val selectedAccountId by viewModel.selectedAccountId.collectAsStateWithLifecycle() val selectedFolder by viewModel.selectedFolder.collectAsStateWithLifecycle() val folders by viewModel.folders.collectAsStateWithLifecycle() + val folderUnreadCounts by viewModel.folderUnreadCounts.collectAsStateWithLifecycle() + val accountsWithUnread by viewModel.accountsWithUnread.collectAsStateWithLifecycle() val drawerAccount by viewModel.drawerAccount.collectAsStateWithLifecycle() val hasAccounts by viewModel.hasAccounts.collectAsStateWithLifecycle() val draftCount by viewModel.draftCount.collectAsStateWithLifecycle() @@ -156,6 +158,8 @@ fun MailboxScreen( accounts = accounts, drawerAccount = drawerAccount, folders = folders, + folderUnreadCounts = folderUnreadCounts, + accountsWithUnread = accountsWithUnread, selectedAccountId = selectedAccountId, selectedFolder = selectedFolder, onSelectUnifiedInbox = { @@ -256,6 +260,7 @@ fun MailboxScreen( AccountFilterRow( accounts = accounts, selectedId = selectedAccountId, + accountsWithUnread = accountsWithUnread, onSelect = viewModel::selectAccount, ) } @@ -411,7 +416,12 @@ private fun OutboxEntry(count: Int, onClick: () -> Unit) { } @Composable -private fun AccountFilterRow(accounts: List, selectedId: String?, onSelect: (String?) -> Unit) { +private fun AccountFilterRow( + accounts: List, + selectedId: String?, + accountsWithUnread: Set, + onSelect: (String?) -> Unit, +) { Row( modifier = Modifier .fillMaxWidth() @@ -428,7 +438,13 @@ private fun AccountFilterRow(accounts: List, selectedId: String?, onSel FilterChip( selected = selectedId == account.id, onClick = { onSelect(account.id) }, - label = { Text(account.email, maxLines = 1) }, + label = { + Text( + account.email, + maxLines = 1, + fontWeight = if (account.id in accountsWithUnread) FontWeight.Bold else FontWeight.Normal, + ) + }, ) } } diff --git a/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt b/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt index 180b9a4..d2d60fd 100644 --- a/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/mailbox/MailboxViewModel.kt @@ -28,6 +28,7 @@ import org.libremail.domain.model.Folder import org.libremail.domain.model.FolderRole import org.libremail.domain.model.Message import org.libremail.domain.model.ReplyMode +import org.libremail.domain.model.UnreadCount import org.libremail.domain.repository.AccountRepository import org.libremail.domain.repository.MailRepository import org.libremail.ui.navigation.Routes @@ -87,6 +88,28 @@ class MailboxViewModel @Inject constructor( } .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList()) + /** + * Live unread counts across every account, shared by the two drawer indicators below so the + * underlying `COUNT` query is collected once no matter how many observers derive from it. + */ + private val unreadCounts: StateFlow> = mailRepository.observeUnreadCounts() + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList()) + + /** #83: unread-message count keyed by folder full name for the drawer account (absent = none). */ + val folderUnreadCounts: StateFlow> = + combine(drawerAccount, unreadCounts) { account, counts -> + if (account == null) { + emptyMap() + } else { + counts.filter { it.accountId == account.id }.associate { it.folder to it.count } + } + }.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyMap()) + + /** #84: ids of accounts that currently have unread mail in any folder. */ + val accountsWithUnread: StateFlow> = unreadCounts + .map { counts -> counts.filter { it.count > 0 }.map { it.accountId }.toSet() } + .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptySet()) + private val _searchActive = MutableStateFlow(false) val searchActive: StateFlow = _searchActive.asStateFlow() diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 9b713a0..fcc4e8d 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -68,6 +68,13 @@ Archive Spam Trash + + 99+ + + + %1$d unread message + %1$d unread messages + From @@ -152,6 +159,7 @@ An app password is a one-off password that lets an app sign in to your account without your main password or a two-factor code. Store this app password carefully — it grants full access to your email. LibreMail keeps it only on this device. Create an app password for %1$s + How to turn on 2-Step Verification Couldn\'t open your browser Email address App password diff --git a/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt index c900ba4..7cbd260 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt @@ -34,6 +34,7 @@ import org.libremail.domain.model.AccountSettings import org.libremail.domain.model.ImapConnectionParams import org.libremail.domain.model.MailSecurity import org.libremail.domain.repository.MailRepository +import org.libremail.mail.FetchedMessage import org.libremail.mail.ImapClient import org.libremail.power.BatteryStatus import org.libremail.power.BatteryStatusProvider @@ -41,6 +42,7 @@ import java.util.Date import java.util.Properties import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNotEquals import kotlin.test.assertNotNull import kotlin.test.assertTrue @@ -221,11 +223,131 @@ class MailBackfillerTest { coVerify(atLeast = 1) { requireNotNull(lastMailRepository).prefetchMessage(any()) } } - /** Builds a backfiller wired to GreenMail with the in-memory fakes and the given account settings. */ + /** + * Regression for #94: backfill pages by UID (arrival order) while the age floor cuts by the Date + * header. A single message with a HIGH UID but an OLD Date (mail moved/imported into the folder) + * used to drag the oldest cached timestamp below the cutoff and stop paging after zero pages, + * silently gapping every older-UID message whose Date is still within retention. The floor must + * instead be decided from the pages actually fetched. + */ + @Test + fun `a high-UID old-Date message must not stop the age floor while history is unfetched`() = runTest { + val now = System.currentTimeMillis() + // UIDs 1..20: genuinely old (~8 months). UIDs 21..99: within retention, Dates ascending with + // UID. UID 100: a recent arrival whose Date header is 8 months old — the Date/UID inversion — + // which lands inside the seeded foreground window. + val old = (1..20).map { now - 8 * MONTH_MILLIS - it * DAY_MILLIS } + val recent = (1..79).map { now - (80 - it) * DAY_MILLIS } + val inverted = listOf(now - 8 * MONTH_MILLIS) + appendMessages(old + recent + inverted) + seedForegroundWindow() + + val backfiller = backfiller(AccountSettings("acct", retentionMonths = 6)) + var guard = 0 + while (backfiller.runBackfill() && guard++ < 10) { /* drive to completion */ } + + val cutoff = now - 6 * MONTH_MILLIS + assertEquals( + recent.size, + cached.count { it.timestampMillis >= cutoff }, + "every within-retention message must be cached — no silent history gap", + ) + assertEquals(true, progress["acct" to "INBOX"]?.complete, "paging still terminates at the floor") + assertNoDeletes() + } + + /** + * The flip side of the page-based age floor (#94): a page ENTIRELY older than the cutoff ends + * paging — the folder is marked complete without caching that page, which is pure prune-fodder + * (persisting it would just make the pruner delete it again, the #12/#13 churn the sticky floor + * exists to prevent). + */ + @Test + fun `an entirely-old page ends age paging without caching beyond the floor`() = runTest { + val now = System.currentTimeMillis() + // UIDs 1..50 are ~8 months old; UIDs 51..100 are within retention and fill the whole window. + val old = (1..50).map { now - 8 * MONTH_MILLIS - (51 - it) * DAY_MILLIS } + val recent = (1..50).map { now - (51 - it) * DAY_MILLIS } + appendMessages(old + recent) + seedForegroundWindow() + + backfiller(AccountSettings("acct", retentionMonths = 6)).runBackfill() + + assertEquals(true, progress["acct" to "INBOX"]?.complete, "one entirely-old page ends the folder") + assertEquals(0, totalOffered, "the beyond-the-floor page must not be cached") + val cutoff = now - 6 * MONTH_MILLIS + assertTrue(cached.none { it.timestampMillis < cutoff }, "nothing older than the cutoff is cached") + assertNoDeletes() + } + + /** + * Regression for #95: a cached row with `uid = 0` — a row migrated before the `uid` column + * existed (MIGRATION_12_13 backfills a non-numeric id tail to 0) — used to collapse MIN(uid) to + * 0, and `fetchOlderThan` treats a bound `<= 1` as "nothing older": the folder was marked fully + * backfilled after caching NOTHING. The boundary must come from resolved (positive) UIDs only. + */ + @Test + fun `a uid 0 placeholder row must not poison the backfill boundary`() = runTest { + appendMessages(TOTAL) + seedForegroundWindow() + // A pre-uid-column row, exactly as MIGRATION_12_13 leaves one whose id tail isn't numeric. + cached += cached.first().copy(id = "acct:INBOX:legacy", uid = 0L) + + val backfiller = backfiller(AccountSettings("acct")) + var guard = 0 + while (backfiller.runBackfill() && guard++ < 10) { /* drive to completion */ } + + assertEquals( + TOTAL, + cached.mapTo(HashSet()) { it.uid }.count { it > 0L }, + "the placeholder row must not stop backfill from paging the full history", + ) + assertEquals(TOTAL - WINDOW, totalOffered, "each backfilled message fetched exactly once") + assertEquals(true, progress["acct" to "INBOX"]?.complete) + assertNoDeletes() + } + + /** + * Regression for #95 (server variant): when every UID on a fetched page is unresolvable + * (UIDFolder.getUID returned -1), the boundary must not descend to -1 — the next fetch would come + * back empty and the folder would be FALSELY marked fully backfilled. The folder must instead + * stall: stay incomplete (retryable on a future run) without claiming immediate more-work (which + * would make the worker's slice-chaining loop spin on the same page). + */ + @Test + fun `a page of unresolvable UIDs stalls the folder instead of falsely completing it`() = runTest { + // One cached window row makes INBOX a backfill target with boundary UID 60. + cached += fetchedMessage(uid = "60").toEntity("acct", "INBOX") + + val imapClient = mockk() + coEvery { imapClient.fetchOlderThan(any(), any(), any(), any()) } answers { + // The real client's contract: a collapsed boundary (<= 1) means "nothing older". + if (thirdArg() <= 1L) emptyList() else listOf(fetchedMessage(uid = "-1")) + } + + val moreWork = backfiller(AccountSettings("acct"), imapClient = imapClient).runBackfill() + + assertNotEquals(true, progress["acct" to "INBOX"]?.complete, "the folder must stay retryable") + assertFalse(moreWork, "a stalled folder must not spin the worker's slice-chaining loop") + coVerify(exactly = 0) { imapClient.fetchOlderThan(any(), any(), match { it <= 1L }, any()) } + } + + private fun fetchedMessage(uid: String) = FetchedMessage( + uid = uid, + sender = "Sender", + senderEmail = "sender@example.org", + subject = "Message $uid", + timestampMillis = System.currentTimeMillis(), + isRead = false, + isFlagged = false, + ) + + /** Builds a backfiller wired to [imapClient] (GreenMail-backed by default) with the in-memory fakes. */ private fun backfiller( accountSettings: AccountSettings, fetchPolicy: FetchPolicy = FetchPolicy.ON_DEMAND, battery: BatteryStatus = BatteryStatus(percent = 100, isCharging = false), + imapClient: ImapClient = client, ): MailBackfiller { val accountDao = mockk() coEvery { accountDao.getAll() } returns listOf(accountEntity) @@ -241,16 +363,13 @@ class MailBackfillerTest { } coEvery { messageDao.lowestSyncedUid("acct", any()) } answers { val folder = secondArg() - cached.filter { it.inInbox && it.folder == folder }.minOfOrNull { it.uid } + // Mirrors the real query's `uid > 0` guard (#95): placeholder rows never drive the boundary. + cached.filter { it.inInbox && it.folder == folder && it.uid > 0L }.minOfOrNull { it.uid } } coEvery { messageDao.countSynced("acct", any()) } answers { val folder = secondArg() cached.count { it.inInbox && it.folder == folder } } - coEvery { messageDao.oldestSyncedTimestamp("acct", any()) } answers { - val folder = secondArg() - cached.filter { it.inInbox && it.folder == folder }.minOfOrNull { it.timestampMillis } - } val backfillProgressDao = mockk(relaxed = true) coEvery { backfillProgressDao.get("acct", any()) } answers { progress["acct" to secondArg()] } @@ -277,7 +396,7 @@ class MailBackfillerTest { accountDao = accountDao, messageDao = messageDao, backfillProgressDao = backfillProgressDao, - imapClient = client, + imapClient = imapClient, connectionFactory = connectionFactory, settingsRepository = settingsRepository, accountSettingsRepository = accountSettingsRepository, diff --git a/app/src/test/kotlin/org/libremail/data/sync/SyncSchedulerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/SyncSchedulerTest.kt new file mode 100644 index 0000000..24092ed --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/sync/SyncSchedulerTest.kt @@ -0,0 +1,107 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import androidx.work.ExistingPeriodicWorkPolicy +import androidx.work.ExistingWorkPolicy +import androidx.work.OneTimeWorkRequest +import androidx.work.PeriodicWorkRequest +import androidx.work.WorkManager +import io.mockk.mockk +import io.mockk.verify +import org.junit.Test +import javax.inject.Provider + +/** + * The enqueue methods are thin wrappers over WorkManager, so these tests pin the one thing that carries + * behaviour: the existing-work policy each unique job is enqueued with. + */ +class SyncSchedulerTest { + + private val workManager = mockk(relaxed = true) + private val scheduler = SyncScheduler(Provider { workManager }) + + // Issue #96: periodic jobs must re-enqueue with UPDATE (not KEEP). At every app start KEEP would pin + // an already-installed device to the interval/constraints of whichever version first scheduled the + // job, so cadence/constraint tuning never reached upgraders; UPDATE re-applies the current spec. + + @Test + fun `periodic sync is enqueued with UPDATE so upgrades re-apply its schedule`() { + scheduler.schedulePeriodicSync() + + verify { + workManager.enqueueUniquePeriodicWork( + "libremail_periodic_sync", + ExistingPeriodicWorkPolicy.UPDATE, + any(), + ) + } + } + + @Test + fun `periodic backfill is enqueued with UPDATE so upgrades re-apply its schedule`() { + scheduler.schedulePeriodicBackfill() + + verify { + workManager.enqueueUniquePeriodicWork( + "libremail_periodic_backfill", + ExistingPeriodicWorkPolicy.UPDATE, + any(), + ) + } + } + + @Test + fun `periodic prune is enqueued with UPDATE so upgrades re-apply its schedule`() { + scheduler.schedulePeriodicPrune() + + verify { + workManager.enqueueUniquePeriodicWork( + "libremail_periodic_prune", + ExistingPeriodicWorkPolicy.UPDATE, + any(), + ) + } + } + + // The one-shot kicks are a separate concern from #96 and keep their existing policies: syncNow and + // pruneNow REPLACE for a clean immediate attempt; backfillNow KEEPs an in-flight all-account run. + + @Test + fun `syncNow replaces any pending one-shot sync`() { + scheduler.syncNow() + + verify { + workManager.enqueueUniqueWork( + "libremail_oneshot_sync", + ExistingWorkPolicy.REPLACE, + any(), + ) + } + } + + @Test + fun `backfillNow keeps an already-running backfill`() { + scheduler.backfillNow() + + verify { + workManager.enqueueUniqueWork( + "libremail_oneshot_backfill", + ExistingWorkPolicy.KEEP, + any(), + ) + } + } + + @Test + fun `pruneNow replaces any pending one-shot prune`() { + scheduler.pruneNow() + + verify { + workManager.enqueueUniqueWork( + "libremail_oneshot_prune", + ExistingWorkPolicy.REPLACE, + any(), + ) + } + } +} diff --git a/app/src/test/kotlin/org/libremail/domain/model/MailProviderTest.kt b/app/src/test/kotlin/org/libremail/domain/model/MailProviderTest.kt index 52a8519..e8f868e 100644 --- a/app/src/test/kotlin/org/libremail/domain/model/MailProviderTest.kt +++ b/app/src/test/kotlin/org/libremail/domain/model/MailProviderTest.kt @@ -80,6 +80,21 @@ class MailProviderTest { } } + @Test + fun `only gmail links two-factor setup help, over https`() { + // Gmail's app-passwords page rejects accounts without 2-Step Verification, so its setup + // screen must offer the setup article as a way out (issue #98). + val gmailUrl = assertNotNull( + MailProvider.GMAIL.twoFactorHelpUrl, + "gmail must expose a 2-Step Verification help URL", + ) + assertTrue(gmailUrl.startsWith("https://"), "gmail 2FA help URL must be https") + + // Yahoo and iCloud gate nothing on two-factor, so they must not grow the extra link. + assertNull(MailProvider.YAHOO.twoFactorHelpUrl) + assertNull(MailProvider.ICLOUD.twoFactorHelpUrl) + } + @Test fun `createAccount trims the email and derives a stable id and display name`() { val account = MailProvider.GMAIL.createAccount(" User@Gmail.com ") diff --git a/app/src/test/kotlin/org/libremail/ui/mailbox/MailboxViewModelTest.kt b/app/src/test/kotlin/org/libremail/ui/mailbox/MailboxViewModelTest.kt index b73e4dd..02d2699 100644 --- a/app/src/test/kotlin/org/libremail/ui/mailbox/MailboxViewModelTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/mailbox/MailboxViewModelTest.kt @@ -30,6 +30,7 @@ import org.libremail.domain.model.MailSecurity import org.libremail.domain.model.Message import org.libremail.domain.model.ReplyMode import org.libremail.domain.model.ServerConfig +import org.libremail.domain.model.UnreadCount import org.libremail.domain.repository.AccountRepository import org.libremail.domain.repository.MailRepository import org.libremail.ui.navigation.Routes @@ -429,10 +430,55 @@ class MailboxViewModelTest { assertEquals(listOf("imap:a:INBOX:1"), vm.messages.value.map { it.id }) } + @Test + fun `folderUnreadCounts maps the drawer account's folders to their unread counts`() = runTest(testDispatcher) { + val vm = createViewModel( + accounts = listOf(alice, bob), + messages = emptyList(), + unreadCounts = listOf( + UnreadCount("imap:a", "INBOX", 3), + UnreadCount("imap:a", "[Gmail]/Spam", 1), + UnreadCount("imap:b", "INBOX", 9), + ), + ) + backgroundScope.launch { vm.folderUnreadCounts.collect {} } + + // The drawer defaults to the first account (alice); bob's counts are excluded. + assertEquals(mapOf("INBOX" to 3, "[Gmail]/Spam" to 1), vm.folderUnreadCounts.value) + } + + @Test + fun `folderUnreadCounts follows the drawer-account switch`() = runTest(testDispatcher) { + val vm = createViewModel( + accounts = listOf(alice, bob), + messages = emptyList(), + unreadCounts = listOf(UnreadCount("imap:a", "INBOX", 3), UnreadCount("imap:b", "INBOX", 9)), + ) + backgroundScope.launch { vm.folderUnreadCounts.collect {} } + + vm.setDrawerAccount("imap:b") + + assertEquals(mapOf("INBOX" to 9), vm.folderUnreadCounts.value) + } + + @Test + fun `accountsWithUnread lists every account that has unread mail in any folder`() = runTest(testDispatcher) { + val vm = createViewModel( + accounts = listOf(alice, bob), + messages = emptyList(), + // alice has unread mail (in a non-inbox folder too); bob has none. + unreadCounts = listOf(UnreadCount("imap:a", "INBOX", 3), UnreadCount("imap:a", "Archive", 2)), + ) + backgroundScope.launch { vm.accountsWithUnread.collect {} } + + assertEquals(setOf("imap:a"), vm.accountsWithUnread.value) + } + private fun createViewModel( accounts: List, messages: List, folders: Map> = emptyMap(), + unreadCounts: List = emptyList(), syncer: MailSyncer = mockk(relaxed = true), repo: MailRepository = mockk(relaxed = true), initialAccountId: String? = null, @@ -440,6 +486,7 @@ class MailboxViewModelTest { every { repo.observeMessages() } returns MutableStateFlow(messages) every { repo.observeDrafts() } returns flowOf(emptyList()) every { repo.observeOutbox() } returns flowOf(emptyList()) + every { repo.observeUnreadCounts() } returns MutableStateFlow(unreadCounts) accounts.forEach { account -> every { repo.observeFolders(account.id) } returns MutableStateFlow(folders[account.id] ?: emptyList()) }