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/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/dao/MessageDao.kt b/app/src/main/kotlin/org/libremail/data/local/dao/MessageDao.kt index 88579f3..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 @@ -118,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/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/Folder.kt b/app/src/main/kotlin/org/libremail/domain/model/Folder.kt index 0b8de14..3cbb648 100644 --- a/app/src/main/kotlin/org/libremail/domain/model/Folder.kt +++ b/app/src/main/kotlin/org/libremail/domain/model/Folder.kt @@ -33,6 +33,25 @@ enum class FolderRole { ; companion object { + /** + * Single source of truth mapping a lowercase IMAP SPECIAL-USE attribute to the role it implies: + * RFC 6154's six attributes, plus Gmail's `\All` and RFC 8457's `\Important`. A `null` role + * marks an attribute that flags a folder as server-provisioned (not user-created) without + * implying one of the friendly [FolderRole]s. [roleOf] returns the first role-bearing entry the + * folder advertises; [isServerSpecial] treats every key as special-use. Insertion order sets + * [roleOf]'s precedence when a folder advertises more than one role-bearing attribute. + */ + private val ATTRIBUTE_ROLES: Map = linkedMapOf( + "\\sent" to SENT, + "\\drafts" to DRAFTS, + "\\junk" to SPAM, + "\\trash" to TRASH, + "\\archive" to ARCHIVE, + "\\all" to null, + "\\flagged" to null, + "\\important" to null, + ) + /** * Classifies a folder from its name and IMAP SPECIAL-USE attributes (RFC 6154). Prefers the * server-advertised attribute; falls back to a case-insensitive name match because many @@ -41,27 +60,14 @@ enum class FolderRole { fun roleOf(fullName: String, displayName: String, attributes: List): FolderRole { if (fullName.equals("INBOX", ignoreCase = true)) return INBOX val attrs = attributes.map { it.lowercase() } - val byAttribute = when { - "\\sent" in attrs -> SENT - "\\drafts" in attrs -> DRAFTS - "\\junk" in attrs -> SPAM - "\\trash" in attrs -> TRASH - "\\archive" in attrs -> ARCHIVE - else -> null + val byAttribute = ATTRIBUTE_ROLES.entries.firstNotNullOfOrNull { (attribute, role) -> + role?.takeIf { attribute in attrs } } 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 } + fun isServerSpecial(attributes: List): Boolean = attributes.any { it.lowercase() in ATTRIBUTE_ROLES } /** 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()) { 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/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/res/values/strings.xml b/app/src/main/res/values/strings.xml index 75f6c41..fcc4e8d 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -159,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/local/FolderMapperTest.kt b/app/src/test/kotlin/org/libremail/data/local/FolderMapperTest.kt index ca01e64..75fd901 100644 --- a/app/src/test/kotlin/org/libremail/data/local/FolderMapperTest.kt +++ b/app/src/test/kotlin/org/libremail/data/local/FolderMapperTest.kt @@ -42,11 +42,12 @@ class FolderMapperTest { } @Test - fun `the all-mail and starred attributes mark the folder special but drive no role of their own`() { - // Gmail's "All Mail" (\All) and "Starred" (\Flagged) are server special-use, yet roleOf maps - // neither to a role (the role still comes from the name — here neutral, so NORMAL). specialUse - // and role are independent axes; this documents the current wiring ahead of the #65 refactor. - listOf("\\All", "\\Flagged").forEach { attribute -> + fun `the all-mail, starred and important attributes mark the folder special but drive no role`() { + // Gmail's "All Mail" (\All), "Starred" (\Flagged) and "Important" (\Important, RFC 8457 — #62) + // are server special-use, yet roleOf maps none to a role (the role still comes from the name — + // here neutral, so NORMAL). specialUse and role are independent axes; the #65 unified table + // keeps them so. + listOf("\\All", "\\Flagged", "\\Important").forEach { attribute -> val entity = entityFor(listOf(attribute)) assertTrue(entity.specialUse, "specialUse for $attribute") assertEquals(FolderRole.NORMAL.name, entity.role, "role for $attribute") 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/FolderRoleTest.kt b/app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt index d426cfd..d1ccc50 100644 --- a/app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt +++ b/app/src/test/kotlin/org/libremail/domain/model/FolderRoleTest.kt @@ -37,9 +37,51 @@ class FolderRoleTest { fun `isServerSpecial is true only for special-use attributes`() { assertTrue(FolderRole.isServerSpecial(listOf("\\Junk"))) assertTrue(FolderRole.isServerSpecial(listOf("\\All"))) + // Issue #62: \Important (RFC 8457) is what Gmail advertises on [Gmail]/Important. + assertTrue(FolderRole.isServerSpecial(listOf("\\Important"))) // 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"))) } + + // Guards the #65 unified attribute-to-role table: roleOf and isServerSpecial read from one map. + // Role-bearing attributes drive a role AND mark the folder special; \All, \Flagged and \Important + // are server-special but role-less (their role comes from the display name — here "Neutral", so + // NORMAL). Pins that the refactor preserved every existing mapping and added \Important (#62). + @Test + fun `every special-use attribute maps to its role and is server-special`() { + val expectedRole = mapOf( + "\\Sent" to FolderRole.SENT, + "\\Drafts" to FolderRole.DRAFTS, + "\\Junk" to FolderRole.SPAM, + "\\Trash" to FolderRole.TRASH, + "\\Archive" to FolderRole.ARCHIVE, + "\\All" to FolderRole.NORMAL, + "\\Flagged" to FolderRole.NORMAL, + "\\Important" to FolderRole.NORMAL, + ) + expectedRole.forEach { (attribute, role) -> + assertEquals(role, FolderRole.roleOf("X", "Neutral", listOf(attribute)), "role for $attribute") + assertTrue(FolderRole.isServerSpecial(listOf(attribute)), "special-use for $attribute") + } + } + + // A folder advertising two role-bearing attributes resolves to the earlier table entry (Sent + // precedes Archive), whatever order the server listed them — preserving the old when-ladder's + // precedence now that it is table-driven. + @Test + fun `role precedence follows the table order, not the folder's attribute order`() { + assertEquals(FolderRole.SENT, FolderRole.roleOf("X", "Whatever", listOf("\\Archive", "\\Sent"))) + assertEquals(FolderRole.SENT, FolderRole.roleOf("X", "Whatever", listOf("\\Sent", "\\Archive"))) + } + + // A role-less special-use attribute must not suppress the display-name fallback: \All on a folder + // named "Sent" still classifies as SENT, and Gmail's [Gmail]/Important (\Important, RFC 8457) is + // special-use yet keeps NORMAL because "Important" is not a friendly role. + @Test + fun `a role-less special-use attribute still allows the name fallback`() { + assertEquals(FolderRole.SENT, FolderRole.roleOf("X", "Sent", listOf("\\All"))) + assertEquals(FolderRole.NORMAL, FolderRole.roleOf("[Gmail]/Important", "Important", listOf("\\Important"))) + } } 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 ")