From bad597bc42b7e79ea0e07bdde84c79a8ed9dd122 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 1 Jul 2026 00:27:04 -0500 Subject: [PATCH 1/7] feat(sync): default fetch-all history + device-only retention (#12, #13) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the fixed 50-message-per-folder header cap with a background, resumable full-history backfill, and add a user-configurable device-only retention limit that prunes local mail beyond it without ever deleting from the server. - ImapClient.fetchOlderThan pages a folder backwards in bounded batches, locating the boundary by binary search over message numbers (O(log n) tiny UID fetches, memory bounded to one batch). - MailBackfiller + BackfillWorker page each synced folder newest→oldest, persisting a per-folder boundary in a new backfill_progress table so a run interrupted by process death / network loss resumes exactly where it stopped. Runs off the sync mutex, so foreground sync / pull-to-refresh stay responsive. - MailSyncer now reconciles server deletions only within the recent UID window (deleteSyncedInWindowNotIn) instead of wiping everything outside the recent 50, so backfilled history survives each foreground sync. A materialized messages.uid column powers the windowed reconcile and backfill boundary. - Body/attachment prefetch still honours FetchPolicy (headers first). - Per-account count/age overrides (nullable) with a global default; 0 = keep everything (the default, matching #12). - MailPruner + PruneWorker delete local rows beyond the limit (cascading attachment rows + on-disk cache), never issuing a server delete. Deletes are chunked under SQLite's 999-parameter limit. - Precedence with backfill: backfill pauses (does not complete) at the retention floor and both jobs share a maintenance mutex, so they never contend; foreground fetch is also capped by the count so it can't re-download what pruning just trimmed. - Settings UI for the global default and per-account override, with copy making clear it is device-only, not the server. Room schema v9→v10 (migration + exported schema + MigrationTestHelper test). GreenMail tests prove the backfill caches >50 and resumes after interruption; pruning tests cover count/age limits and never touch the server. Co-Authored-By: Claude Opus 4.8 --- app/build.gradle.kts | 4 + .../12.json | 653 ++++++++++++++++++ .../data/local/Migration11To12Test.kt | 85 +++ .../ui/settings/AccountSettingsScreenTest.kt | 2 + .../ui/settings/SettingsScreenTest.kt | 2 + .../org/libremail/LibreMailApplication.kt | 4 + .../libremail/data/local/LibreMailDatabase.kt | 6 +- .../org/libremail/data/local/Mappers.kt | 5 + .../org/libremail/data/local/Migrations.kt | 32 + .../data/local/dao/BackfillProgressDao.kt | 23 + .../libremail/data/local/dao/MessageDao.kt | 57 +- .../local/entity/AccountSettingsEntity.kt | 8 + .../local/entity/BackfillProgressEntity.kt | 25 + .../data/local/entity/MessageEntity.kt | 7 + .../data/repository/AccountRepositoryImpl.kt | 9 +- .../data/repository/MailRepositoryImpl.kt | 1 + .../settings/AccountSettingsRepository.kt | 9 + .../data/settings/RetentionPolicy.kt | 51 ++ .../data/settings/SettingsRepository.kt | 21 + .../org/libremail/data/sync/BackfillWorker.kt | 28 + .../org/libremail/data/sync/MailBackfiller.kt | 205 ++++++ .../data/sync/MailMaintenanceGate.kt | 22 + .../org/libremail/data/sync/MailPruner.kt | 91 +++ .../org/libremail/data/sync/MailSyncer.kt | 40 +- .../org/libremail/data/sync/PruneWorker.kt | 26 + .../org/libremail/data/sync/SyncScheduler.kt | 53 +- .../kotlin/org/libremail/di/DatabaseModule.kt | 6 + .../libremail/domain/model/AccountSettings.kt | 9 +- .../kotlin/org/libremail/mail/ImapClient.kt | 66 ++ .../ui/settings/AccountSettingsScreen.kt | 10 + .../ui/settings/AccountSettingsViewModel.kt | 17 + .../ui/settings/SettingsComponents.kt | 99 +++ .../libremail/ui/settings/SettingsScreen.kt | 37 +- .../ui/settings/SettingsViewModel.kt | 13 + app/src/main/res/values/strings.xml | 16 + .../data/settings/RetentionPolicyTest.kt | 74 ++ .../libremail/data/sync/MailBackfillerTest.kt | 253 +++++++ .../org/libremail/data/sync/MailPrunerTest.kt | 139 ++++ .../org/libremail/data/sync/MailSyncerTest.kt | 43 +- .../libremail/mail/ImapClientBackfillTest.kt | 148 ++++ gradle/libs.versions.toml | 2 + 41 files changed, 2362 insertions(+), 39 deletions(-) create mode 100644 app/schemas/org.libremail.data.local.LibreMailDatabase/12.json create mode 100644 app/src/androidTest/kotlin/org/libremail/data/local/Migration11To12Test.kt create mode 100644 app/src/main/kotlin/org/libremail/data/local/dao/BackfillProgressDao.kt create mode 100644 app/src/main/kotlin/org/libremail/data/local/entity/BackfillProgressEntity.kt create mode 100644 app/src/main/kotlin/org/libremail/data/settings/RetentionPolicy.kt create mode 100644 app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt create mode 100644 app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt create mode 100644 app/src/main/kotlin/org/libremail/data/sync/MailMaintenanceGate.kt create mode 100644 app/src/main/kotlin/org/libremail/data/sync/MailPruner.kt create mode 100644 app/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt create mode 100644 app/src/test/kotlin/org/libremail/data/settings/RetentionPolicyTest.kt create mode 100644 app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt create mode 100644 app/src/test/kotlin/org/libremail/data/sync/MailPrunerTest.kt create mode 100644 app/src/test/kotlin/org/libremail/mail/ImapClientBackfillTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 1440adb..6b365c4 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -96,6 +96,9 @@ android { buildConfig = true } + // Ship the exported Room schemas as androidTest assets so MigrationTestHelper can load them. + sourceSets.getByName("androidTest").assets.srcDir("$projectDir/schemas") + packaging { resources { // Angus Mail / Jakarta Activation (added later) ship duplicate META-INF entries. @@ -200,4 +203,5 @@ dependencies { androidTestImplementation(libs.androidx.espresso.intents) androidTestImplementation(platform(libs.androidx.compose.bom)) androidTestImplementation(libs.androidx.compose.ui.test.junit4) + androidTestImplementation(libs.androidx.room.testing) } diff --git a/app/schemas/org.libremail.data.local.LibreMailDatabase/12.json b/app/schemas/org.libremail.data.local.LibreMailDatabase/12.json new file mode 100644 index 0000000..5401539 --- /dev/null +++ b/app/schemas/org.libremail.data.local.LibreMailDatabase/12.json @@ -0,0 +1,653 @@ +{ + "formatVersion": 1, + "database": { + "version": 12, + "identityHash": "33102d5c0f1df8eb539d9b70c1df2534", + "entities": [ + { + "tableName": "accounts", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `email` TEXT NOT NULL, `displayName` TEXT NOT NULL, `authType` TEXT NOT NULL, `imap_host` TEXT NOT NULL, `imap_port` INTEGER NOT NULL, `imap_security` TEXT NOT NULL, `smtp_host` TEXT NOT NULL, `smtp_port` INTEGER NOT NULL, `smtp_security` TEXT NOT NULL, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "email", + "columnName": "email", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "displayName", + "columnName": "displayName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "authType", + "columnName": "authType", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "imap.host", + "columnName": "imap_host", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "imap.port", + "columnName": "imap_port", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "imap.security", + "columnName": "imap_security", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "smtp.host", + "columnName": "smtp_host", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "smtp.port", + "columnName": "smtp_port", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "smtp.security", + "columnName": "smtp_security", + "affinity": "TEXT", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "account_settings", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `signature` TEXT NOT NULL, `signatureEnabled` INTEGER NOT NULL, `notificationsEnabled` INTEGER NOT NULL, `retentionCount` INTEGER, `retentionMonths` INTEGER, PRIMARY KEY(`accountId`), FOREIGN KEY(`accountId`) REFERENCES `accounts`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "signature", + "columnName": "signature", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "signatureEnabled", + "columnName": "signatureEnabled", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "notificationsEnabled", + "columnName": "notificationsEnabled", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "retentionCount", + "columnName": "retentionCount", + "affinity": "INTEGER" + }, + { + "fieldPath": "retentionMonths", + "columnName": "retentionMonths", + "affinity": "INTEGER" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId" + ] + }, + "foreignKeys": [ + { + "table": "accounts", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "accountId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "messages", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, `sender` TEXT NOT NULL, `senderEmail` TEXT NOT NULL, `subject` TEXT NOT NULL, `snippet` TEXT NOT NULL, `body` TEXT NOT NULL, `isHtml` INTEGER NOT NULL, `timestampMillis` INTEGER NOT NULL, `isRead` INTEGER NOT NULL, `isStarred` INTEGER NOT NULL, `folder` TEXT NOT NULL DEFAULT 'INBOX', `inInbox` INTEGER NOT NULL, `bodyFetched` INTEGER NOT NULL, `uid` INTEGER NOT NULL DEFAULT 0, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "sender", + "columnName": "sender", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "senderEmail", + "columnName": "senderEmail", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "subject", + "columnName": "subject", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "snippet", + "columnName": "snippet", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "body", + "columnName": "body", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "isHtml", + "columnName": "isHtml", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "timestampMillis", + "columnName": "timestampMillis", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "isRead", + "columnName": "isRead", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "isStarred", + "columnName": "isStarred", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "folder", + "columnName": "folder", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "'INBOX'" + }, + { + "fieldPath": "inInbox", + "columnName": "inInbox", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "bodyFetched", + "columnName": "bodyFetched", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "uid", + "columnName": "uid", + "affinity": "INTEGER", + "notNull": true, + "defaultValue": "0" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_messages_accountId", + "unique": false, + "columnNames": [ + "accountId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_messages_accountId` ON `${TABLE_NAME}` (`accountId`)" + }, + { + "name": "index_messages_timestampMillis", + "unique": false, + "columnNames": [ + "timestampMillis" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_messages_timestampMillis` ON `${TABLE_NAME}` (`timestampMillis`)" + } + ] + }, + { + "tableName": "credentials", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `encryptedSecret` TEXT NOT NULL, PRIMARY KEY(`accountId`))", + "fields": [ + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "encryptedSecret", + "columnName": "encryptedSecret", + "affinity": "TEXT", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId" + ] + } + }, + { + "tableName": "attachments", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`messageId` TEXT NOT NULL, `partIndex` INTEGER NOT NULL, `filename` TEXT NOT NULL, `mimeType` TEXT NOT NULL, `sizeBytes` INTEGER NOT NULL, PRIMARY KEY(`messageId`, `partIndex`), FOREIGN KEY(`messageId`) REFERENCES `messages`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "messageId", + "columnName": "messageId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "partIndex", + "columnName": "partIndex", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "filename", + "columnName": "filename", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "mimeType", + "columnName": "mimeType", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "sizeBytes", + "columnName": "sizeBytes", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "messageId", + "partIndex" + ] + }, + "indices": [ + { + "name": "index_attachments_messageId", + "unique": false, + "columnNames": [ + "messageId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_attachments_messageId` ON `${TABLE_NAME}` (`messageId`)" + } + ], + "foreignKeys": [ + { + "table": "messages", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "messageId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "outbox", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, `toAddresses` TEXT NOT NULL, `ccAddresses` TEXT NOT NULL, `bccAddresses` TEXT NOT NULL DEFAULT '', `subject` TEXT NOT NULL, `body` TEXT NOT NULL, `createdAt` INTEGER NOT NULL, `lastError` TEXT, `bodyHtml` TEXT, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "toAddresses", + "columnName": "toAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "ccAddresses", + "columnName": "ccAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "bccAddresses", + "columnName": "bccAddresses", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "subject", + "columnName": "subject", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "body", + "columnName": "body", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "createdAt", + "columnName": "createdAt", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "lastError", + "columnName": "lastError", + "affinity": "TEXT" + }, + { + "fieldPath": "bodyHtml", + "columnName": "bodyHtml", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "drafts", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT, `toAddresses` TEXT NOT NULL, `ccAddresses` TEXT NOT NULL, `bccAddresses` TEXT NOT NULL DEFAULT '', `subject` TEXT NOT NULL, `body` TEXT NOT NULL, `updatedAt` INTEGER NOT NULL, `attachments` TEXT NOT NULL, `bodyHtml` TEXT, PRIMARY KEY(`id`))", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT" + }, + { + "fieldPath": "toAddresses", + "columnName": "toAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "ccAddresses", + "columnName": "ccAddresses", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "bccAddresses", + "columnName": "bccAddresses", + "affinity": "TEXT", + "notNull": true, + "defaultValue": "''" + }, + { + "fieldPath": "subject", + "columnName": "subject", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "body", + "columnName": "body", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "updatedAt", + "columnName": "updatedAt", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "attachments", + "columnName": "attachments", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "bodyHtml", + "columnName": "bodyHtml", + "affinity": "TEXT" + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + } + }, + { + "tableName": "folders", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `fullName` TEXT NOT NULL, `displayName` TEXT NOT NULL, `role` TEXT NOT NULL, `selectable` INTEGER NOT NULL, `sortOrder` INTEGER NOT NULL, PRIMARY KEY(`accountId`, `fullName`))", + "fields": [ + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "fullName", + "columnName": "fullName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "displayName", + "columnName": "displayName", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "role", + "columnName": "role", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "selectable", + "columnName": "selectable", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "sortOrder", + "columnName": "sortOrder", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId", + "fullName" + ] + } + }, + { + "tableName": "signatures", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `accountId` TEXT NOT NULL, `name` TEXT NOT NULL, `contentHtml` TEXT NOT NULL, `isDefault` INTEGER NOT NULL, PRIMARY KEY(`id`), FOREIGN KEY(`accountId`) REFERENCES `accounts`(`id`) ON UPDATE NO ACTION ON DELETE CASCADE )", + "fields": [ + { + "fieldPath": "id", + "columnName": "id", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "name", + "columnName": "name", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "contentHtml", + "columnName": "contentHtml", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "isDefault", + "columnName": "isDefault", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "id" + ] + }, + "indices": [ + { + "name": "index_signatures_accountId", + "unique": false, + "columnNames": [ + "accountId" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_signatures_accountId` ON `${TABLE_NAME}` (`accountId`)" + } + ], + "foreignKeys": [ + { + "table": "accounts", + "onDelete": "CASCADE", + "onUpdate": "NO ACTION", + "columns": [ + "accountId" + ], + "referencedColumns": [ + "id" + ] + } + ] + }, + { + "tableName": "backfill_progress", + "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`accountId` TEXT NOT NULL, `folder` TEXT NOT NULL, `nextBeforeUid` INTEGER NOT NULL, `complete` INTEGER NOT NULL, PRIMARY KEY(`accountId`, `folder`))", + "fields": [ + { + "fieldPath": "accountId", + "columnName": "accountId", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "folder", + "columnName": "folder", + "affinity": "TEXT", + "notNull": true + }, + { + "fieldPath": "nextBeforeUid", + "columnName": "nextBeforeUid", + "affinity": "INTEGER", + "notNull": true + }, + { + "fieldPath": "complete", + "columnName": "complete", + "affinity": "INTEGER", + "notNull": true + } + ], + "primaryKey": { + "autoGenerate": false, + "columnNames": [ + "accountId", + "folder" + ] + } + } + ], + "setupQueries": [ + "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '33102d5c0f1df8eb539d9b70c1df2534')" + ] + } +} \ No newline at end of file diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/Migration11To12Test.kt b/app/src/androidTest/kotlin/org/libremail/data/local/Migration11To12Test.kt new file mode 100644 index 0000000..dcbb9fd --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/data/local/Migration11To12Test.kt @@ -0,0 +1,85 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local + +import androidx.room.testing.MigrationTestHelper +import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Validates the v11 -> v12 migration (issues #12/#13): `messages.uid` is added and backfilled from the + * id's numeric tail, `account_settings` gains the nullable retention overrides, and the + * `backfill_progress` table is created. `runMigrationsAndValidate` additionally checks the whole + * migrated schema matches the exported v12 schema. + */ +@RunWith(AndroidJUnit4::class) +class Migration11To12Test { + + @get:Rule + val helper = MigrationTestHelper( + InstrumentationRegistry.getInstrumentation(), + LibreMailDatabase::class.java, + emptyList(), + FrameworkSQLiteOpenHelperFactory(), + ) + + @Test + fun migrate11To12_backfillsUidAndAddsRetentionAndBackfillTables() { + helper.createDatabase(DB_NAME, 11).apply { + // A plain inbox row, a folder name containing special characters, and a folder name that + // ends in a digit — all must recover the trailing UID correctly. + insertV11Message("acct:INBOX:42") + insertV11Message("acct:[Gmail]/Sent Mail:7") + insertV11Message("acct:Folder2:15") + close() + } + + val db = helper.runMigrationsAndValidate(DB_NAME, 12, true, MIGRATION_11_12) + + // uid is materialized from the id's numeric tail. + assertEquals(42L, uidOf(db, "acct:INBOX:42")) + assertEquals(7L, uidOf(db, "acct:[Gmail]/Sent Mail:7")) + assertEquals(15L, uidOf(db, "acct:Folder2:15")) + + // The new retention columns exist and default to NULL (= inherit the global default). + db.query("SELECT retentionCount, retentionMonths FROM account_settings").use { c -> + // No rows required; the query succeeding proves the columns exist. + assertTrue(c.columnCount == 2) + } + + // The backfill_progress table exists and accepts a row. + db.execSQL( + "INSERT INTO backfill_progress (accountId, folder, nextBeforeUid, complete) " + + "VALUES ('acct', 'INBOX', 41, 0)", + ) + db.query("SELECT nextBeforeUid FROM backfill_progress WHERE accountId='acct' AND folder='INBOX'").use { c -> + assertTrue(c.moveToFirst()) + assertEquals(41L, c.getLong(0)) + } + db.close() + } + + private fun uidOf(db: androidx.sqlite.db.SupportSQLiteDatabase, id: String): Long = + db.query("SELECT uid FROM messages WHERE id = ?", arrayOf(id)).use { c -> + assertTrue("row $id must exist", c.moveToFirst()) + c.getLong(0) + } + + private fun androidx.sqlite.db.SupportSQLiteDatabase.insertV11Message(id: String) { + execSQL( + "INSERT INTO messages (id, accountId, sender, senderEmail, subject, snippet, body, isHtml, " + + "timestampMillis, isRead, isStarred, folder, inInbox, bodyFetched) " + + "VALUES (?, 'acct', 'Ada', 'ada@example.org', 'Hi', '', '', 0, 1000, 0, 0, 'INBOX', 1, 0)", + arrayOf(id), + ) + } + + private companion object { + const val DB_NAME = "migration-11-12-test.db" + } +} 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 8bafcf7..36b3ed2 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/settings/AccountSettingsScreenTest.kt @@ -19,6 +19,7 @@ import org.libremail.data.local.LibreMailDatabase import org.libremail.data.local.toEntity import org.libremail.data.settings.AccountSettingsRepository import org.libremail.data.settings.SignatureRepository +import org.libremail.data.sync.SyncScheduler import org.libremail.domain.model.Account import org.libremail.domain.model.AuthType import org.libremail.domain.model.MailSecurity @@ -67,6 +68,7 @@ class AccountSettingsScreenTest { accountRepository = FakeAccountRepository(accounts = listOf(account)), accountSettingsRepository = repository, signatureRepository = SignatureRepository(db.signatureDao()), + syncScheduler = SyncScheduler(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 4bdf2ce..00b8893 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/settings/SettingsScreenTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/settings/SettingsScreenTest.kt @@ -15,6 +15,7 @@ import org.junit.runner.RunWith import org.libremail.R import org.libremail.data.settings.FetchPolicy import org.libremail.data.settings.SettingsRepository +import org.libremail.data.sync.SyncScheduler import org.libremail.push.BatteryOptimizationManager import org.libremail.ui.FakeAccountRepository import org.libremail.ui.theme.LibreMailTheme @@ -40,6 +41,7 @@ class SettingsScreenTest { FakeAccountRepository(), settingsRepository, BatteryOptimizationManager(context), + SyncScheduler(context), ) composeTestRule.setContent { diff --git a/app/src/main/kotlin/org/libremail/LibreMailApplication.kt b/app/src/main/kotlin/org/libremail/LibreMailApplication.kt index 8c24366..42d19b1 100644 --- a/app/src/main/kotlin/org/libremail/LibreMailApplication.kt +++ b/app/src/main/kotlin/org/libremail/LibreMailApplication.kt @@ -64,6 +64,10 @@ class LibreMailApplication : // touching DataStore on the crashing thread. appScope.launch { runCatching { diagnosticsCollector.warmSettingsCache() } } syncScheduler.schedulePeriodicSync() + // Full-history backfill (#12) and device-only retention pruning (#13) run as their own bounded, + // resumable background jobs so they never block foreground sync / pull-to-refresh. + syncScheduler.schedulePeriodicBackfill() + syncScheduler.schedulePeriodicPrune() // Run the IMAP IDLE push service only while it has something to do: the push setting is on // AND at least one account exists. This starts it when the first account is added and stops // it when the last is removed, reactively. diff --git a/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt b/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt index d65ee85..dc050a2 100644 --- a/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt +++ b/app/src/main/kotlin/org/libremail/data/local/LibreMailDatabase.kt @@ -6,6 +6,7 @@ import androidx.room.RoomDatabase import org.libremail.data.local.dao.AccountDao import org.libremail.data.local.dao.AccountSettingsDao import org.libremail.data.local.dao.AttachmentDao +import org.libremail.data.local.dao.BackfillProgressDao import org.libremail.data.local.dao.CredentialDao import org.libremail.data.local.dao.DraftDao import org.libremail.data.local.dao.FolderDao @@ -15,6 +16,7 @@ import org.libremail.data.local.dao.SignatureDao import org.libremail.data.local.entity.AccountEntity import org.libremail.data.local.entity.AccountSettingsEntity import org.libremail.data.local.entity.AttachmentEntity +import org.libremail.data.local.entity.BackfillProgressEntity import org.libremail.data.local.entity.CredentialEntity import org.libremail.data.local.entity.DraftEntity import org.libremail.data.local.entity.FolderEntity @@ -33,8 +35,9 @@ import org.libremail.data.local.entity.SignatureEntity DraftEntity::class, FolderEntity::class, SignatureEntity::class, + BackfillProgressEntity::class, ], - version = 11, + version = 12, exportSchema = true, ) abstract class LibreMailDatabase : RoomDatabase() { @@ -47,4 +50,5 @@ abstract class LibreMailDatabase : RoomDatabase() { abstract fun draftDao(): DraftDao abstract fun folderDao(): FolderDao abstract fun signatureDao(): SignatureDao + abstract fun backfillProgressDao(): BackfillProgressDao } diff --git a/app/src/main/kotlin/org/libremail/data/local/Mappers.kt b/app/src/main/kotlin/org/libremail/data/local/Mappers.kt index a6b444f..be6afb8 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Mappers.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Mappers.kt @@ -52,6 +52,8 @@ internal fun AccountSettingsEntity.toDomain(): AccountSettings = AccountSettings signature = signature, signatureEnabled = signatureEnabled, notificationsEnabled = notificationsEnabled, + retentionCount = retentionCount, + retentionMonths = retentionMonths, ) internal fun AccountSettings.toEntity(): AccountSettingsEntity = AccountSettingsEntity( @@ -59,6 +61,8 @@ internal fun AccountSettings.toEntity(): AccountSettingsEntity = AccountSettings signature = signature, signatureEnabled = signatureEnabled, notificationsEnabled = notificationsEnabled, + retentionCount = retentionCount, + retentionMonths = retentionMonths, ) internal fun Account.toImapParams( @@ -119,6 +123,7 @@ internal fun FetchedMessage.toEntity(accountId: String, folder: String, inInbox: folder = folder, inInbox = inInbox, bodyFetched = false, + uid = uid.toLongOrNull() ?: 0L, ) internal fun FolderEntity.toDomain(): Folder = Folder( diff --git a/app/src/main/kotlin/org/libremail/data/local/Migrations.kt b/app/src/main/kotlin/org/libremail/data/local/Migrations.kt index dcece0e..cf072f8 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Migrations.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Migrations.kt @@ -216,3 +216,35 @@ val MIGRATION_10_11 = object : Migration(10, 11) { ) } } + +/** + * v11 -> v12: full-history backfill + device-only retention (issues #12/#13; preserves existing data). + * - `messages`: add the materialized `uid` column (`DEFAULT 0`, matching the entity's + * `@ColumnInfo(defaultValue = "0")`) and backfill it from the numeric tail of the existing + * "accountId:folder:uid" id. `rtrim(id, '0123456789')` strips the trailing digits, leaving the + * prefix up to and including the final ':'; the remainder is the UID. Non-numeric tails cast to 0 + * and are refreshed to the real UID on the next sync. + * - `account_settings`: add nullable `retentionCount` / `retentionMonths` overrides (NULL = inherit + * the global default), declared without SQL defaults to match the entity's nullable columns. + * - add the `backfill_progress` table tracking each folder's paging boundary so the backfill resumes + * after process death / network loss. + */ +val MIGRATION_11_12 = object : Migration(11, 12) { + override fun migrate(db: SupportSQLiteDatabase) { + db.execSQL("ALTER TABLE `messages` ADD COLUMN `uid` INTEGER NOT NULL DEFAULT 0") + db.execSQL( + "UPDATE `messages` SET `uid` = " + + "CAST(substr(`id`, length(rtrim(`id`, '0123456789')) + 1) AS INTEGER)", + ) + + db.execSQL("ALTER TABLE `account_settings` ADD COLUMN `retentionCount` INTEGER") + db.execSQL("ALTER TABLE `account_settings` ADD COLUMN `retentionMonths` INTEGER") + + db.execSQL( + "CREATE TABLE IF NOT EXISTS `backfill_progress` (" + + "`accountId` TEXT NOT NULL, `folder` TEXT NOT NULL, " + + "`nextBeforeUid` INTEGER NOT NULL, `complete` INTEGER NOT NULL, " + + "PRIMARY KEY(`accountId`, `folder`))", + ) + } +} diff --git a/app/src/main/kotlin/org/libremail/data/local/dao/BackfillProgressDao.kt b/app/src/main/kotlin/org/libremail/data/local/dao/BackfillProgressDao.kt new file mode 100644 index 0000000..b2c62f6 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/local/dao/BackfillProgressDao.kt @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local.dao + +import androidx.room.Dao +import androidx.room.Insert +import androidx.room.OnConflictStrategy +import androidx.room.Query +import org.libremail.data.local.entity.BackfillProgressEntity + +@Dao +interface BackfillProgressDao { + @Query("SELECT * FROM backfill_progress WHERE accountId = :accountId AND folder = :folder LIMIT 1") + suspend fun get(accountId: String, folder: String): BackfillProgressEntity? + + @Query("SELECT * FROM backfill_progress WHERE accountId = :accountId") + suspend fun getForAccount(accountId: String): List + + @Insert(onConflict = OnConflictStrategy.REPLACE) + suspend fun upsert(progress: BackfillProgressEntity) + + @Query("DELETE FROM backfill_progress WHERE accountId = :accountId") + suspend fun deleteForAccount(accountId: String) +} diff --git a/app/src/main/kotlin/org/libremail/data/local/dao/MessageDao.kt b/app/src/main/kotlin/org/libremail/data/local/dao/MessageDao.kt index db2495b..596280b 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 @@ -32,13 +32,14 @@ interface MessageDao { suspend fun insertNew(messages: List) /** - * Refreshes the display fields from the server without touching the cached body, the local - * read/star flags (which may hold an optimistic change the server hasn't reflected yet), or the - * inbox membership. + * Refreshes the display fields (and the materialized [MessageEntity.uid], keeping it fresh for + * rows migrated before the column existed) from the server without touching the cached body, the + * local read/star flags (which may hold an optimistic change the server hasn't reflected yet), or + * the inbox membership. */ @Query( "UPDATE messages SET sender = :sender, senderEmail = :senderEmail, subject = :subject, " + - "timestampMillis = :timestampMillis WHERE id = :id", + "timestampMillis = :timestampMillis, uid = :uid WHERE id = :id", ) suspend fun updateHeaderContent( id: String, @@ -46,6 +47,7 @@ interface MessageDao { senderEmail: String, subject: String, timestampMillis: Long, + uid: Long, ) /** Marks rows as folder-synced (e.g. a former search-only row that the sync now returns). */ @@ -82,6 +84,53 @@ interface MessageDao { ) suspend fun deleteSyncedNotIn(accountId: String, folder: String, keepIds: List) + /** + * Windowed deletion reconcile for full-history sync (issue #12): within [folder], delete synced + * rows whose UID falls inside the freshly-fetched recent window (`uid >= minWindowUid`) but which + * the server no longer returns ([keepIds]). Rows below the window — older history fetched by the + * background backfill — are deliberately left intact, unlike [deleteSyncedNotIn]. + */ + @Query( + "DELETE FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1 " + + "AND uid >= :minWindowUid AND id NOT IN (:keepIds)", + ) + 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") + 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 + + /** Ids of an account's synced rows older than [cutoffMillis] (age-based prune candidates, all folders). */ + @Query("SELECT id FROM messages WHERE accountId = :accountId AND inInbox = 1 AND timestampMillis < :cutoffMillis") + suspend fun syncedIdsOlderThan(accountId: String, cutoffMillis: Long): List + + /** + * Ids of an account's synced rows in [folder] beyond the newest [keep] by recency (count-based + * prune candidates). Ties broken by UID so the boundary is deterministic. + */ + @Query( + "SELECT id FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1 " + + "AND id NOT IN (" + + "SELECT id FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1 " + + "ORDER BY timestampMillis DESC, uid DESC LIMIT :keep)", + ) + suspend fun syncedIdsBeyondCountInFolder(accountId: String, folder: String, keep: Int): List + /** Removes transient server-search hits (called when search closes). */ @Query("DELETE FROM messages WHERE inInbox = 0") suspend fun deleteSearchRows() diff --git a/app/src/main/kotlin/org/libremail/data/local/entity/AccountSettingsEntity.kt b/app/src/main/kotlin/org/libremail/data/local/entity/AccountSettingsEntity.kt index 938da36..1e1ed0b 100644 --- a/app/src/main/kotlin/org/libremail/data/local/entity/AccountSettingsEntity.kt +++ b/app/src/main/kotlin/org/libremail/data/local/entity/AccountSettingsEntity.kt @@ -26,4 +26,12 @@ data class AccountSettingsEntity( val signature: String = "", val signatureEnabled: Boolean = true, val notificationsEnabled: Boolean = true, + /** + * Per-account device-only retention overrides (issue #13). `null` means "use the global default"; + * `0` means an explicit "keep everything" (no limit). A positive value caps how many messages + * ([retentionCount], newest per folder) or how many months of history ([retentionMonths]) are kept + * on this device — the server copy is never touched. + */ + val retentionCount: Int? = null, + val retentionMonths: Int? = null, ) diff --git a/app/src/main/kotlin/org/libremail/data/local/entity/BackfillProgressEntity.kt b/app/src/main/kotlin/org/libremail/data/local/entity/BackfillProgressEntity.kt new file mode 100644 index 0000000..0535f6a --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/local/entity/BackfillProgressEntity.kt @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local.entity + +import androidx.room.Entity + +/** + * Per-(account, folder) progress of the full-history backfill (issue #12). Lets the background + * backfill worker page backwards through a folder resumably: it survives process death and network + * loss because the boundary is persisted after every batch. + * + * Not foreign-keyed to `accounts` (matching `messages`/`folders`); it is cleared explicitly when an + * account is removed. + */ +@Entity(tableName = "backfill_progress", primaryKeys = ["accountId", "folder"]) +data class BackfillProgressEntity( + val accountId: String, + val folder: String, + /** + * Exclusive upper UID bound for the next page: the next batch fetches server messages with + * UID < this value. Lowered to the batch's lowest UID after each successful page. + */ + val nextBeforeUid: Long, + /** True once the whole folder (down to the retention floor, if any) has been cached. */ + val complete: Boolean = false, +) diff --git a/app/src/main/kotlin/org/libremail/data/local/entity/MessageEntity.kt b/app/src/main/kotlin/org/libremail/data/local/entity/MessageEntity.kt index e59e110..24170d7 100644 --- a/app/src/main/kotlin/org/libremail/data/local/entity/MessageEntity.kt +++ b/app/src/main/kotlin/org/libremail/data/local/entity/MessageEntity.kt @@ -28,4 +28,11 @@ data class MessageEntity( val inInbox: Boolean = true, /** True once the body has been fetched from the server (distinguishes "not fetched" from "empty body"). */ val bodyFetched: Boolean = false, + /** + * The server IMAP UID as a number (also embedded in [id]). Materialized as a column so full-history + * backfill can page by "lowest cached UID" and foreground sync can reconcile only the recent UID + * window without deleting older, backfilled history. 0 for rows migrated before this column existed + * (refreshed to the real UID on the next sync). + */ + @ColumnInfo(defaultValue = "0") val uid: Long = 0L, ) diff --git a/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt b/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt index 7a7354c..d123f78 100644 --- a/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt +++ b/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt @@ -4,6 +4,7 @@ package org.libremail.data.repository import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map import org.libremail.data.local.dao.AccountDao +import org.libremail.data.local.dao.BackfillProgressDao import org.libremail.data.local.dao.FolderDao import org.libremail.data.local.dao.MessageDao import org.libremail.data.local.toDomain @@ -25,6 +26,7 @@ class AccountRepositoryImpl @Inject constructor( private val accountDao: AccountDao, private val messageDao: MessageDao, private val folderDao: FolderDao, + private val backfillProgressDao: BackfillProgressDao, private val credentialStore: CredentialStore, private val imapClient: ImapClient, private val syncScheduler: SyncScheduler, @@ -47,6 +49,7 @@ class AccountRepositoryImpl @Inject constructor( credentialStore.saveSecret(account.id, password) mailNotifier.ensureAccountChannel(account) syncScheduler.syncNow() + syncScheduler.backfillNow() // start caching this account's full history in the background (#12) folders.map { it.fullName } } @@ -62,6 +65,7 @@ class AccountRepositoryImpl @Inject constructor( credentialStore.saveSecret(account.id, authStateJson) mailNotifier.ensureAccountChannel(account) syncScheduler.syncNow() + syncScheduler.backfillNow() // start caching this account's full history in the background (#12) folders.map { it.fullName } } @@ -69,9 +73,10 @@ class AccountRepositoryImpl @Inject constructor( accountDao.deleteById(id) credentialStore.delete(id) mailNotifier.deleteAccountChannel(id) - // Remove the account's cached mail (attachment rows cascade via the foreign key) and folders. - // The account_settings row is removed automatically by its cascading foreign key. + // Remove the account's cached mail (attachment rows cascade via the foreign key), folders, and + // backfill progress. The account_settings row is removed automatically by its cascading FK. messageDao.deleteByAccount(id) folderDao.deleteForAccount(id) + backfillProgressDao.deleteForAccount(id) } } diff --git a/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt b/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt index d6ec723..92fbda3 100644 --- a/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt +++ b/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt @@ -342,6 +342,7 @@ class MailRepositoryImpl @Inject constructor( senderEmail = it.senderEmail, subject = it.subject, timestampMillis = it.timestampMillis, + uid = it.uid, ) } } diff --git a/app/src/main/kotlin/org/libremail/data/settings/AccountSettingsRepository.kt b/app/src/main/kotlin/org/libremail/data/settings/AccountSettingsRepository.kt index 50a4001..53279fe 100644 --- a/app/src/main/kotlin/org/libremail/data/settings/AccountSettingsRepository.kt +++ b/app/src/main/kotlin/org/libremail/data/settings/AccountSettingsRepository.kt @@ -40,6 +40,15 @@ class AccountSettingsRepository @Inject constructor(private val dao: AccountSett it.copy(notificationsEnabled = enabled) } + /** Per-account device-only retention overrides (null = inherit the global default; 0 = keep everything). */ + suspend fun setRetentionCount(accountId: String, count: Int?) = update(accountId) { + it.copy(retentionCount = count?.coerceAtLeast(0)) + } + + suspend fun setRetentionMonths(accountId: String, months: Int?) = update(accountId) { + it.copy(retentionMonths = months?.coerceAtLeast(0)) + } + private suspend inline fun update(accountId: String, transform: (AccountSettings) -> AccountSettings) { dao.upsert(transform(get(accountId)).toEntity()) } diff --git a/app/src/main/kotlin/org/libremail/data/settings/RetentionPolicy.kt b/app/src/main/kotlin/org/libremail/data/settings/RetentionPolicy.kt new file mode 100644 index 0000000..ffe63ab --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/settings/RetentionPolicy.kt @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.settings + +import java.time.Instant +import java.time.ZoneOffset + +/** + * The effective device-only retention limits for one account (issue #13), after resolving its + * per-account overrides against the global default. `0` in either dimension means "unlimited" (keep + * everything for that dimension). Both limits are independent ceilings: a message is kept only if it + * is within the newest [count] of its folder AND newer than the [months] age cutoff; violating either + * makes it prunable. The retention floor for backfill is therefore whichever limit is hit first. + * + * Retention is enforced purely on the device — the server copy is never deleted. + */ +data class RetentionPolicy(val count: Int, val months: Int) { + + /** True when nothing is limited, so no pruning runs and backfill may page a folder to the very end. */ + val isUnlimited: Boolean get() = count <= 0 && months <= 0 + + /** The count ceiling, or null when unlimited. */ + val countLimit: Int? get() = count.takeIf { it > 0 } + + /** + * Epoch-millis age cutoff derived from [months]: messages strictly older than this are prunable. + * Null when the age limit is unlimited. Uses calendar months (UTC) so "3 months" tracks the + * calendar rather than a fixed 30-day approximation. + */ + fun ageCutoffMillis(nowMillis: Long): Long? = months.takeIf { it > 0 }?.let { + Instant.ofEpochMilli(nowMillis) + .atZone(ZoneOffset.UTC) + .minusMonths(it.toLong()) + .toInstant() + .toEpochMilli() + } + + companion object { + /** The default policy: keep everything on device (matches #12's fetch-all default). */ + val KEEP_EVERYTHING = RetentionPolicy(count = 0, months = 0) + + /** + * Resolves the effective policy for an account: a non-null per-account override wins per + * dimension, otherwise the global default applies. Negative inputs are clamped to 0 (unlimited). + */ + fun resolve(accountCount: Int?, accountMonths: Int?, defaultCount: Int, defaultMonths: Int): RetentionPolicy = + RetentionPolicy( + count = (accountCount ?: defaultCount).coerceAtLeast(0), + months = (accountMonths ?: defaultMonths).coerceAtLeast(0), + ) + } +} diff --git a/app/src/main/kotlin/org/libremail/data/settings/SettingsRepository.kt b/app/src/main/kotlin/org/libremail/data/settings/SettingsRepository.kt index 2d5f0d6..f56ffb9 100644 --- a/app/src/main/kotlin/org/libremail/data/settings/SettingsRepository.kt +++ b/app/src/main/kotlin/org/libremail/data/settings/SettingsRepository.kt @@ -7,6 +7,7 @@ import androidx.datastore.core.DataStore import androidx.datastore.preferences.core.Preferences import androidx.datastore.preferences.core.booleanPreferencesKey import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.intPreferencesKey import androidx.datastore.preferences.core.stringPreferencesKey import androidx.datastore.preferences.preferencesDataStore import dagger.hilt.android.qualifiers.ApplicationContext @@ -44,6 +45,12 @@ data class AppSettings( val encryptCache: Boolean = false, val includeInBackup: Boolean = false, val fetchPolicy: FetchPolicy = FetchPolicy.ALWAYS, + /** + * Global device-only retention defaults (issue #13), applied to accounts that don't override them. + * `0` means "keep everything" (the default), matching the fetch-all history behaviour of #12. + */ + val retentionCount: Int = 0, + val retentionMonths: Int = 0, ) private object Keys { @@ -55,6 +62,8 @@ private object Keys { val ENCRYPT_CACHE = booleanPreferencesKey("encrypt_cache") val INCLUDE_IN_BACKUP = booleanPreferencesKey("include_in_backup") val FETCH_POLICY = stringPreferencesKey("fetch_policy") + val RETENTION_COUNT = intPreferencesKey("retention_count") + val RETENTION_MONTHS = intPreferencesKey("retention_months") val BATTERY_PROMPT_HANDLED = booleanPreferencesKey("battery_prompt_handled") } @@ -72,6 +81,8 @@ internal fun Preferences.toAppSettings(): AppSettings = AppSettings( includeInBackup = this[Keys.INCLUDE_IN_BACKUP] ?: false, fetchPolicy = this[Keys.FETCH_POLICY]?.let { runCatching { FetchPolicy.valueOf(it) }.getOrNull() } ?: FetchPolicy.ALWAYS, + retentionCount = this[Keys.RETENTION_COUNT] ?: 0, + retentionMonths = this[Keys.RETENTION_MONTHS] ?: 0, ) @Singleton @@ -117,6 +128,16 @@ class SettingsRepository @Inject constructor(@ApplicationContext private val con context.settingsDataStore.edit { it[Keys.FETCH_POLICY] = value.name } } + /** Global default retention by message count (newest N per folder); 0 = keep everything. */ + suspend fun setRetentionCount(value: Int) { + context.settingsDataStore.edit { it[Keys.RETENTION_COUNT] = value.coerceAtLeast(0) } + } + + /** Global default retention by age in months; 0 = keep everything. */ + suspend fun setRetentionMonths(value: Int) { + context.settingsDataStore.edit { it[Keys.RETENTION_MONTHS] = value.coerceAtLeast(0) } + } + private suspend fun put(key: Preferences.Key, value: Boolean) { context.settingsDataStore.edit { it[key] = value } } diff --git a/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt b/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt new file mode 100644 index 0000000..68211d2 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import android.content.Context +import androidx.hilt.work.HiltWorker +import androidx.work.CoroutineWorker +import androidx.work.WorkerParameters +import dagger.assisted.Assisted +import dagger.assisted.AssistedInject + +/** + * Runs one bounded slice of the full-history backfill (issue #12). Cancellable (WorkManager stops it + * on constraint loss or system pressure) and resumable — [MailBackfiller] persists its per-folder + * boundary after every page, so the periodic schedule simply continues from where a stopped run left + * off. Retries with WorkManager backoff on failure. + */ +@HiltWorker +class BackfillWorker @AssistedInject constructor( + @Assisted appContext: Context, + @Assisted workerParams: WorkerParameters, + private val backfiller: MailBackfiller, +) : CoroutineWorker(appContext, workerParams) { + + override suspend fun doWork(): Result = runCatching { backfiller.runBackfill() }.fold( + onSuccess = { Result.success() }, + onFailure = { Result.retry() }, + ) +} diff --git a/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt b/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt new file mode 100644 index 0000000..b8653e2 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt @@ -0,0 +1,205 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import android.content.Context +import android.net.ConnectivityManager +import android.net.NetworkCapabilities +import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.delay +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext +import org.libremail.data.local.dao.AccountDao +import org.libremail.data.local.dao.BackfillProgressDao +import org.libremail.data.local.dao.MessageDao +import org.libremail.data.local.entity.BackfillProgressEntity +import org.libremail.data.local.entity.MessageEntity +import org.libremail.data.local.toDomain +import org.libremail.data.local.toEntity +import org.libremail.data.settings.AccountSettingsRepository +import org.libremail.data.settings.FetchPolicy +import org.libremail.data.settings.RetentionPolicy +import org.libremail.data.settings.SettingsRepository +import org.libremail.domain.model.Account +import org.libremail.domain.model.ImapConnectionParams +import org.libremail.domain.repository.MailRepository +import org.libremail.mail.ImapClient +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Pages the *entire* history of every account's synced folders into the local cache (issue #12), + * newest-to-oldest, one bounded slice per invocation so a single WorkManager run stays comfortably + * under the OS time limit. Progress is a persisted per-folder UID boundary + * ([BackfillProgressEntity]), so a run interrupted by process death or network loss resumes exactly + * where it left off; foreground sync / pull-to-refresh are never blocked (this runs off the sync + * mutex). + * + * Backfill only ever *inserts* older headers — it performs no deletion reconcile — and it stops + * paging a folder once the account's device-only retention floor (#13) is reached, so it can never + * fight the pruner over the same messages. + */ +@Singleton +class MailBackfiller @Inject constructor( + @ApplicationContext private val context: Context, + private val accountDao: AccountDao, + private val messageDao: MessageDao, + private val backfillProgressDao: BackfillProgressDao, + private val imapClient: ImapClient, + private val connectionFactory: MailConnectionFactory, + private val settingsRepository: SettingsRepository, + private val accountSettingsRepository: AccountSettingsRepository, + private val mailRepository: MailRepository, + private val maintenanceGate: MailMaintenanceGate, +) { + private data class FolderResult(val batches: Int, val complete: 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). + */ + suspend fun runBackfill(maxBatches: Int = DEFAULT_MAX_BATCHES): Boolean = maintenanceGate.mutex.withLock { + var remaining = maxBatches + var moreWork = false + for (account in accountDao.getAll().map { it.toDomain() }) { + val params = runCatching { connectionFactory.imapParamsFor(account) }.getOrNull() ?: continue + val policy = effectivePolicy(account.id) + for (folder in messageDao.syncedFolders(account.id)) { + 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) } + remaining -= result.batches + if (!result.complete) moreWork = true + } + } + moreWork + } + + private suspend fun backfillFolder( + account: Account, + params: ImapConnectionParams, + folder: String, + policy: RetentionPolicy, + maxBatches: Int, + ): FolderResult { + if (backfillProgressDao.get(account.id, folder)?.complete == true) { + return FolderResult(batches = 0, complete = true) + } + + // Always page strictly below the LOWEST currently-cached UID. Deriving the boundary from the + // cache (rather than a stored cursor) keeps backfill gap-free even after the pruner raised the + // floor, and lets a later loosening of retention resume filling automatically. The mutex in + // runBackfill keeps the pruner from moving this boundary mid-run. + var beforeUid = messageDao.lowestSyncedUid(account.id, folder) ?: Long.MAX_VALUE + var batches = 0 + while (batches < maxBatches) { + currentCoroutineContext().ensureActive() + // Retention floor (#13 precedence): pause — but do NOT mark complete — once the device-only + // limit is reached, so backfill and the pruner never contend for the same messages and a + // later loosening of the limit resumes paging from where it stopped. + if (reachedRetentionFloor(account.id, folder, policy)) { + return FolderResult(batches, complete = false) + } + 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) } + persistBatch(entities) + beforeUid = entities.minOf { it.uid } + 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) + } + + /** 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 + } + + /** Inserts backfilled headers; never deletes. Uncancellable so a persisted boundary always has its rows. */ + private suspend fun persistBatch(entities: List) = withContext(NonCancellable) { + messageDao.insertNew(entities) + val ids = entities.map { it.id } + messageDao.markSynced(ids) + entities.forEach { + messageDao.updateHeaderContent( + id = it.id, + sender = it.sender, + senderEmail = it.senderEmail, + subject = it.subject, + timestampMillis = it.timestampMillis, + uid = it.uid, + ) + } + } + + private suspend fun markComplete(accountId: String, folder: String, nextBeforeUid: Long) { + backfillProgressDao.upsert(BackfillProgressEntity(accountId, folder, nextBeforeUid, complete = true)) + } + + /** + * Pre-caches each backfilled message's body/attachments per the [FetchPolicy] (headers first, + * bodies per policy — issue #12). Best-effort and cancellable between messages so an interruption + * stops promptly; anything not fetched is filled in lazily when the message is opened. + */ + private suspend fun prefetchIfEnabled(ids: List) { + val shouldPrefetch = when (settingsRepository.fetchPolicy()) { + FetchPolicy.ALWAYS -> true + FetchPolicy.WIFI_ONLY -> isUnmetered() + FetchPolicy.ON_DEMAND -> false + } + if (!shouldPrefetch) return + for (id in ids) { + currentCoroutineContext().ensureActive() + mailRepository.prefetchMessage(id) + } + } + + private suspend fun effectivePolicy(accountId: String): RetentionPolicy { + val account = accountSettingsRepository.get(accountId) + val global = settingsRepository.settings.first() + return RetentionPolicy.resolve( + accountCount = account.retentionCount, + accountMonths = account.retentionMonths, + defaultCount = global.retentionCount, + defaultMonths = global.retentionMonths, + ) + } + + private fun isUnmetered(): Boolean { + val manager = context.getSystemService(ConnectivityManager::class.java) ?: return false + val capabilities = manager.getNetworkCapabilities(manager.activeNetwork) ?: return false + return capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED) + } + + private companion object { + /** Headers fetched per server page. */ + const val BACKFILL_BATCH_SIZE = 50 + + /** Server pages per WorkManager run, keeping one run well under the OS execution limit. */ + const val DEFAULT_MAX_BATCHES = 20 + + /** Pause between pages to spread server load on large mailboxes. */ + const val BACKFILL_BATCH_DELAY_MS = 250L + } +} diff --git a/app/src/main/kotlin/org/libremail/data/sync/MailMaintenanceGate.kt b/app/src/main/kotlin/org/libremail/data/sync/MailMaintenanceGate.kt new file mode 100644 index 0000000..b0dc04b --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/sync/MailMaintenanceGate.kt @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import kotlinx.coroutines.sync.Mutex +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Process-wide lock serializing the two background maintenance jobs that both touch older cached + * history: the full-history backfill ([MailBackfiller], #12) and the retention pruner + * ([MailPruner], #13). + * + * The primary correctness guarantee is the retention *floor* — backfill never pages below the limit + * and pruning only deletes below it, so their working sets are disjoint. This mutex is defence in + * depth: it guarantees they never interleave even if their views of the floor momentarily disagree, + * so a prune can never delete a message a backfill is mid-write on. Foreground sync / pull-to-refresh + * are intentionally NOT gated here (they use [MailSyncer]'s own mutex) so the UI stays responsive. + */ +@Singleton +class MailMaintenanceGate @Inject constructor() { + val mutex = Mutex() +} diff --git a/app/src/main/kotlin/org/libremail/data/sync/MailPruner.kt b/app/src/main/kotlin/org/libremail/data/sync/MailPruner.kt new file mode 100644 index 0000000..562fff6 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/sync/MailPruner.kt @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import android.content.Context +import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.currentCoroutineContext +import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.sync.withLock +import org.libremail.data.local.dao.AccountDao +import org.libremail.data.local.dao.MessageDao +import org.libremail.data.settings.AccountSettingsRepository +import org.libremail.data.settings.RetentionPolicy +import org.libremail.data.settings.SettingsRepository +import java.io.File +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Enforces device-only retention (issue #13): deletes locally cached messages (and, via the Room + * foreign key, their attachment metadata) that exceed each account's retention limit, then removes + * their on-disk attachment cache files. It NEVER contacts the server — pruning is purely local, so + * the mail stays on the server and is re-fetchable later. + * + * Precedence with the #12 backfill is guaranteed two ways: backfill stops paging at the same + * retention floor this pruner deletes below (their working sets are disjoint), and both jobs share + * [MailMaintenanceGate] so they never run at once. + */ +@Singleton +class MailPruner @Inject constructor( + @ApplicationContext private val context: Context, + private val accountDao: AccountDao, + private val messageDao: MessageDao, + private val settingsRepository: SettingsRepository, + private val accountSettingsRepository: AccountSettingsRepository, + private val maintenanceGate: MailMaintenanceGate, +) { + /** Prunes every account to its effective retention policy. Returns the number of messages removed. */ + suspend fun prune(nowMillis: Long = System.currentTimeMillis()): Int = maintenanceGate.mutex.withLock { + val global = settingsRepository.settings.first() + var removed = 0 + for (account in accountDao.getAll()) { + currentCoroutineContext().ensureActive() + val settings = accountSettingsRepository.get(account.id) + val policy = RetentionPolicy.resolve( + accountCount = settings.retentionCount, + accountMonths = settings.retentionMonths, + defaultCount = global.retentionCount, + defaultMonths = global.retentionMonths, + ) + if (policy.isUnlimited) continue + removed += pruneAccount(account.id, policy, nowMillis) + } + removed + } + + private suspend fun pruneAccount(accountId: String, policy: RetentionPolicy, nowMillis: Long): Int { + val victimIds = LinkedHashSet() + + // Age limit: prunable across every folder in one query. + policy.ageCutoffMillis(nowMillis)?.let { cutoff -> + victimIds += messageDao.syncedIdsOlderThan(accountId, cutoff) + } + // Count limit: keep the newest N of EACH folder; prune the rest. + policy.countLimit?.let { limit -> + for (folder in messageDao.syncedFolders(accountId)) { + victimIds += messageDao.syncedIdsBeyondCountInFolder(accountId, folder, limit) + } + } + + if (victimIds.isEmpty()) return 0 + val ids = victimIds.toList() + // Delete DB rows first (attachment metadata cascades via the foreign key), then their cache files. + // Chunk the id list so a first prune of a large backfilled mailbox stays under SQLite's + // host-parameter limit (999 on API 29) for the `IN (...)` clause. + ids.chunked(DELETE_CHUNK).forEach { chunk -> messageDao.deleteByIds(chunk) } + ids.forEach { deleteCacheFiles(it) } + return ids.size + } + + /** Removes the per-message on-disk attachment cache (keyed the same way MailRepositoryImpl writes it). */ + private fun deleteCacheFiles(messageId: String) { + val safeId = messageId.replace(Regex("[^A-Za-z0-9._-]"), "_") + runCatching { File(context.cacheDir, "attachments/$safeId").deleteRecursively() } + } + + private companion object { + /** Ids per DELETE, kept under SQLite's 999-host-parameter limit on older Android. */ + const val DELETE_CHUNK = 500 + } +} diff --git a/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt b/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt index c2eda57..b6a80b2 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt @@ -8,6 +8,7 @@ import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.ensureActive +import kotlinx.coroutines.flow.first import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext @@ -17,6 +18,7 @@ import org.libremail.data.local.toDomain import org.libremail.data.local.toEntity import org.libremail.data.settings.AccountSettingsRepository import org.libremail.data.settings.FetchPolicy +import org.libremail.data.settings.RetentionPolicy import org.libremail.data.settings.SettingsRepository import org.libremail.domain.model.Account import org.libremail.domain.repository.MailRepository @@ -87,7 +89,10 @@ class MailSyncer @Inject constructor( private suspend fun syncFolderHeaders(account: Account, folder: String, notify: Boolean): Result = runCatching { val params = connectionFactory.imapParamsFor(account) - val fetched = imapClient.fetchRecent(params, folder, FETCH_LIMIT) // cancellable network I/O + // Never fetch more of the recent window than device-only retention (#13) would keep. Without + // this, a count limit BELOW the window would make foreground sync re-download the same rows + // the pruner just trimmed, on every sync — an endless re-download/re-prune fight. + val fetched = imapClient.fetchRecent(params, folder, recentWindowFor(account)) // cancellable network I/O val entities = fetched.map { it.toEntity(account.id, folder) } // Persist and notify atomically with respect to cancellation: an IDLE renewal that cancels @@ -102,6 +107,8 @@ class MailSyncer @Inject constructor( } if (entities.isEmpty()) { + // An empty recent window means the server folder itself is empty, so nothing (not + // even backfilled history) should remain cached for it. messageDao.deleteSyncedByAccountFolder(account.id, folder) } else { val ids = entities.map { it.id } @@ -116,9 +123,14 @@ class MailSyncer @Inject constructor( senderEmail = it.senderEmail, subject = it.subject, timestampMillis = it.timestampMillis, + uid = it.uid, ) } - messageDao.deleteSyncedNotIn(account.id, folder, ids) + // Reconcile server-side deletions ONLY within the fetched recent-UID window, so older + // history paged in by the background backfill (issue #12) survives each foreground sync + // instead of being wiped by a whole-folder "not in the recent 50" delete. + val minWindowUid = entities.minOf { it.uid } + messageDao.deleteSyncedInWindowNotIn(account.id, folder, minWindowUid, ids) } val shouldNotify = notify && @@ -132,6 +144,23 @@ class MailSyncer @Inject constructor( fetched.size } + /** + * The number of recent headers to fetch: the standard [FETCH_LIMIT], but capped by the account's + * effective device-only retention count so foreground sync never re-downloads rows the pruner + * would immediately trim. Age-only or unlimited retention leaves the full window in place. + */ + private suspend fun recentWindowFor(account: Account): Int { + val accountSettings = accountSettingsRepository.get(account.id) + val global = settingsRepository.settings.first() + val policy = RetentionPolicy.resolve( + accountCount = accountSettings.retentionCount, + accountMonths = accountSettings.retentionMonths, + defaultCount = global.retentionCount, + defaultMonths = global.retentionMonths, + ) + return policy.countLimit?.let { minOf(FETCH_LIMIT, it) } ?: FETCH_LIMIT + } + /** * Aggressively pre-caches each not-yet-fetched message's full content (body + attachments) per the * user's [FetchPolicy]. Runs outside [syncMutex] so these downloads don't block pull-to-refresh or @@ -159,6 +188,13 @@ class MailSyncer @Inject constructor( private companion object { const val INBOX = "INBOX" + + /** + * Size of the "recent window" each foreground sync / pull-to-refresh fetches (newest N headers). + * This is no longer the history cap — the background backfill ([MailBackfiller], issue #12) + * pages in everything older; foreground sync just keeps this recent window fresh and reconciles + * deletions within it. + */ const val FETCH_LIMIT = 50 } } diff --git a/app/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt b/app/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt new file mode 100644 index 0000000..a6dc3ba --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/sync/PruneWorker.kt @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import android.content.Context +import androidx.hilt.work.HiltWorker +import androidx.work.CoroutineWorker +import androidx.work.WorkerParameters +import dagger.assisted.Assisted +import dagger.assisted.AssistedInject + +/** + * Enforces device-only retention (issue #13) by running [MailPruner]. Purely local — it never + * contacts the server — so it needs no network constraint. Retries with backoff on failure. + */ +@HiltWorker +class PruneWorker @AssistedInject constructor( + @Assisted appContext: Context, + @Assisted workerParams: WorkerParameters, + private val pruner: MailPruner, +) : CoroutineWorker(appContext, workerParams) { + + override suspend fun doWork(): Result = runCatching { pruner.prune() }.fold( + onSuccess = { Result.success() }, + onFailure = { Result.retry() }, + ) +} 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 42a7f22..5cd91b4 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/SyncScheduler.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/SyncScheduler.kt @@ -15,7 +15,7 @@ import java.util.concurrent.TimeUnit import javax.inject.Inject import javax.inject.Singleton -/** Schedules background mail sync via WorkManager. */ +/** 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) @@ -24,6 +24,18 @@ class SyncScheduler @Inject constructor(@ApplicationContext private val context: .setRequiredNetworkType(NetworkType.CONNECTED) .build() + // Backfill is bulk, non-urgent work: require a network AND a healthy battery so it never competes + // with foreground use or drains the device while paging a large mailbox. + private val backfillConstraint = Constraints.Builder() + .setRequiredNetworkType(NetworkType.CONNECTED) + .setRequiresBatteryNotLow(true) + .build() + + // Pruning is purely local (no server calls), so it needs no network — only a healthy battery. + private val pruneConstraint = Constraints.Builder() + .setRequiresBatteryNotLow(true) + .build() + /** Periodic background sync (WorkManager's 15-minute floor). */ fun schedulePeriodicSync() { val request = PeriodicWorkRequestBuilder(15, TimeUnit.MINUTES) @@ -41,8 +53,47 @@ class SyncScheduler @Inject constructor(@ApplicationContext private val context: workManager.enqueueUniqueWork(ONESHOT_WORK, ExistingWorkPolicy.REPLACE, request) } + /** + * 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. + */ + fun schedulePeriodicBackfill() { + val request = PeriodicWorkRequestBuilder(30, TimeUnit.MINUTES) + .setConstraints(backfillConstraint) + .build() + workManager.enqueueUniquePeriodicWork(PERIODIC_BACKFILL, ExistingPeriodicWorkPolicy.KEEP, request) + } + + /** Kicks an immediate backfill slice (e.g. just after an account is added) without waiting for the cadence. */ + fun backfillNow() { + val request = OneTimeWorkRequestBuilder() + .setConstraints(backfillConstraint) + .build() + // KEEP: if a backfill is already running/enqueued it already covers every account, so don't + // restart it; once that one finishes a later kick will start a fresh slice. + workManager.enqueueUniqueWork(ONESHOT_BACKFILL, ExistingWorkPolicy.KEEP, request) + } + + /** Periodic retention pruning (issue #13); also enforces age limits as messages get older over time. */ + fun schedulePeriodicPrune() { + val request = PeriodicWorkRequestBuilder(12, TimeUnit.HOURS) + .setConstraints(pruneConstraint) + .build() + workManager.enqueueUniquePeriodicWork(PERIODIC_PRUNE, ExistingPeriodicWorkPolicy.KEEP, request) + } + + /** Runs pruning promptly, e.g. right after the user tightens a retention limit. */ + fun pruneNow() { + val request = OneTimeWorkRequestBuilder().build() + workManager.enqueueUniqueWork(ONESHOT_PRUNE, ExistingWorkPolicy.REPLACE, request) + } + private companion object { const val PERIODIC_WORK = "libremail_periodic_sync" const val ONESHOT_WORK = "libremail_oneshot_sync" + const val PERIODIC_BACKFILL = "libremail_periodic_backfill" + const val ONESHOT_BACKFILL = "libremail_oneshot_backfill" + const val PERIODIC_PRUNE = "libremail_periodic_prune" + const val ONESHOT_PRUNE = "libremail_oneshot_prune" } } diff --git a/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt b/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt index 1920ea3..78c29fb 100644 --- a/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt +++ b/app/src/main/kotlin/org/libremail/di/DatabaseModule.kt @@ -14,6 +14,7 @@ import net.zetetic.database.sqlcipher.SupportOpenHelperFactory import org.libremail.data.local.DatabaseEncryption import org.libremail.data.local.LibreMailDatabase import org.libremail.data.local.MIGRATION_10_11 +import org.libremail.data.local.MIGRATION_11_12 import org.libremail.data.local.MIGRATION_1_2 import org.libremail.data.local.MIGRATION_2_3 import org.libremail.data.local.MIGRATION_3_4 @@ -26,6 +27,7 @@ import org.libremail.data.local.MIGRATION_9_10 import org.libremail.data.local.dao.AccountDao import org.libremail.data.local.dao.AccountSettingsDao import org.libremail.data.local.dao.AttachmentDao +import org.libremail.data.local.dao.BackfillProgressDao import org.libremail.data.local.dao.CredentialDao import org.libremail.data.local.dao.DraftDao import org.libremail.data.local.dao.FolderDao @@ -59,6 +61,7 @@ object DatabaseModule { MIGRATION_8_9, MIGRATION_9_10, MIGRATION_10_11, + MIGRATION_11_12, ) // No destructive fallback: the migration chain is complete, and silently dropping the // accounts/credentials/mail tables would lose stored secrets. A missing migration should @@ -108,5 +111,8 @@ object DatabaseModule { @Provides fun provideSignatureDao(database: LibreMailDatabase): SignatureDao = database.signatureDao() + @Provides + fun provideBackfillProgressDao(database: LibreMailDatabase): BackfillProgressDao = database.backfillProgressDao() + private const val DB_NAME = "libremail.db" } diff --git a/app/src/main/kotlin/org/libremail/domain/model/AccountSettings.kt b/app/src/main/kotlin/org/libremail/domain/model/AccountSettings.kt index e63dcb6..4341c86 100644 --- a/app/src/main/kotlin/org/libremail/domain/model/AccountSettings.kt +++ b/app/src/main/kotlin/org/libremail/domain/model/AccountSettings.kt @@ -1,12 +1,19 @@ // SPDX-License-Identifier: GPL-3.0-or-later package org.libremail.domain.model -/** Per-account user preferences (signature, notification gating). Defaults apply when unset. */ +/** Per-account user preferences (signature, notification gating, retention). Defaults apply when unset. */ data class AccountSettings( val accountId: String, val signature: String = "", val signatureEnabled: Boolean = true, val notificationsEnabled: Boolean = true, + /** + * Device-only retention overrides (issue #13). `null` = inherit the global default; `0` = an + * explicit "keep everything"; a positive value caps the newest-N messages ([retentionCount]) or + * the last-N months ([retentionMonths]) kept locally. Never affects the server copy. + */ + val retentionCount: Int? = null, + val retentionMonths: Int? = null, ) { /** * The block to append to a compose body, or "" when disabled or blank. Uses the RFC 3676 diff --git a/app/src/main/kotlin/org/libremail/mail/ImapClient.kt b/app/src/main/kotlin/org/libremail/mail/ImapClient.kt index b62ac92..1744f2e 100644 --- a/app/src/main/kotlin/org/libremail/mail/ImapClient.kt +++ b/app/src/main/kotlin/org/libremail/mail/ImapClient.kt @@ -134,6 +134,72 @@ class ImapClient @Inject constructor() { } } + /** + * Fetches up to [limit] headers of [folder] immediately older than [beforeUid] (i.e. with a + * server UID strictly less than it), newest-first — the backwards page used by the full-history + * backfill (issue #12). Pass [Long.MAX_VALUE] to start from the newest message. + * + * Bounded in both memory and network cost: the boundary message number is located with a + * binary search over message numbers by UID (UIDs increase monotonically with message number), + * costing O(log n) tiny `UID FETCH` round trips, and only the [limit]-sized batch is materialized. + * Returns empty when nothing older exists, which the caller treats as "folder fully backfilled". + */ + suspend fun fetchOlderThan( + params: ImapConnectionParams, + folder: String, + beforeUid: Long, + limit: Int, + ): List = withContext(Dispatchers.IO) { + withStore(params) { store -> + val mailbox = store.getFolder(folder) + mailbox.open(Folder.READ_ONLY) + try { + val total = mailbox.messageCount + if (total == 0 || beforeUid <= 1L) return@withStore emptyList() + val uidFolder = mailbox as UIDFolder + // Highest message number whose UID < beforeUid (0 when nothing is older). + val boundary = highestMessageNumberBelowUid(mailbox, uidFolder, total, beforeUid) + if (boundary == 0) return@withStore emptyList() + + val start = maxOf(1, boundary - limit + 1) + val messages = mailbox.getMessages(start, boundary) + mailbox.fetch( + messages, + FetchProfile().apply { + add(FetchProfile.Item.ENVELOPE) + add(FetchProfile.Item.FLAGS) + add(UIDFolder.FetchProfileItem.UID) + }, + ) + messages.reversed().map { it.toFetchedMessage(uidFolder) } + } finally { + runCatching { mailbox.close(false) } + } + } + } + + /** + * Binary-searches message numbers `1..total` for the highest one whose UID is `< beforeUid`. + * Robust to expunges (it reads live UIDs), so it works even if the message that had exactly + * [beforeUid] has since been removed. Returns 0 when every message's UID is `>= beforeUid`. + */ + private fun highestMessageNumberBelowUid(mailbox: Folder, uidFolder: UIDFolder, total: Int, beforeUid: Long): Int { + var lo = 1 + var hi = total + var boundary = 0 + while (lo <= hi) { + val mid = (lo + hi) ushr 1 + val midUid = uidFolder.getUID(mailbox.getMessage(mid)) + if (midUid < beforeUid) { + boundary = mid + lo = mid + 1 + } else { + hi = mid - 1 + } + } + return boundary + } + /** Runs an IMAP SEARCH over [folder] (subject/from/body) and returns matching headers. */ suspend fun search(params: ImapConnectionParams, folder: String, query: String, limit: Int): List = withContext(Dispatchers.IO) { diff --git a/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsScreen.kt b/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsScreen.kt index bdb68b5..b8ad7cf 100644 --- a/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsScreen.kt @@ -94,6 +94,16 @@ fun AccountSettingsScreen( ) HorizontalDivider() + // Per-account device-only retention override (issue #13); "use default" inherits the global setting. + RetentionSection( + count = settings.retentionCount, + months = settings.retentionMonths, + includeUseDefault = true, + onCountChange = viewModel::setRetentionCount, + onMonthsChange = viewModel::setRetentionMonths, + ) + HorizontalDivider() + ClickRow( title = stringResource(R.string.account_remove), titleColor = MaterialTheme.colorScheme.error, diff --git a/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsViewModel.kt b/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsViewModel.kt index 733cd74..f999bcd 100644 --- a/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsViewModel.kt @@ -12,6 +12,7 @@ import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch import org.libremail.data.settings.AccountSettingsRepository import org.libremail.data.settings.SignatureRepository +import org.libremail.data.sync.SyncScheduler import org.libremail.domain.model.Account import org.libremail.domain.model.AccountSettings import org.libremail.domain.repository.AccountRepository @@ -25,6 +26,7 @@ class AccountSettingsViewModel @Inject constructor( private val accountRepository: AccountRepository, private val accountSettingsRepository: AccountSettingsRepository, signatureRepository: SignatureRepository, + private val syncScheduler: SyncScheduler, ) : ViewModel() { private val accountId: String = @@ -59,6 +61,21 @@ class AccountSettingsViewModel @Inject constructor( viewModelScope.launch { accountSettingsRepository.setNotificationsEnabled(accountId, value) } } + /** Per-account device-only retention overrides (null = inherit the global default). Prunes promptly. */ + fun setRetentionCount(value: Int?) { + viewModelScope.launch { + accountSettingsRepository.setRetentionCount(accountId, value) + syncScheduler.pruneNow() + } + } + + fun setRetentionMonths(value: Int?) { + viewModelScope.launch { + accountSettingsRepository.setRetentionMonths(accountId, value) + syncScheduler.pruneNow() + } + } + fun removeAccount(onRemoved: () -> Unit) { viewModelScope.launch { accountRepository.deleteAccount(accountId) diff --git a/app/src/main/kotlin/org/libremail/ui/settings/SettingsComponents.kt b/app/src/main/kotlin/org/libremail/ui/settings/SettingsComponents.kt index 5f60e0d..196e01e 100644 --- a/app/src/main/kotlin/org/libremail/ui/settings/SettingsComponents.kt +++ b/app/src/main/kotlin/org/libremail/ui/settings/SettingsComponents.kt @@ -9,13 +9,16 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.width import androidx.compose.material3.MaterialTheme +import androidx.compose.material3.RadioButton import androidx.compose.material3.Switch import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color +import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp +import org.libremail.R /** Shared row/header composables used by both the global and per-account settings screens. */ @@ -79,3 +82,99 @@ internal fun ClickRow( } } } + +@Composable +internal fun RadioRow(title: String, subtitle: String? = null, selected: Boolean, onClick: () -> Unit) { + Row( + modifier = Modifier + .fillMaxWidth() + .clickable(onClick = onClick) + .padding(horizontal = 16.dp, vertical = 12.dp), + verticalAlignment = Alignment.CenterVertically, + ) { + RadioButton(selected = selected, onClick = onClick) + Spacer(Modifier.width(8.dp)) + Column(Modifier.weight(1f)) { + Text(title, style = MaterialTheme.typography.bodyLarge) + if (subtitle != null) { + Text( + subtitle, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + } + } +} + +/** A preset retention choice: its persisted value ([value], null = "use global default") and its label. */ +private data class RetentionOption(val value: Int?, val labelRes: Int) + +private val COUNT_OPTIONS = listOf( + RetentionOption(0, R.string.retention_keep_all), + RetentionOption(500, R.string.retention_count_500), + RetentionOption(1000, R.string.retention_count_1000), + RetentionOption(5000, R.string.retention_count_5000), +) + +private val AGE_OPTIONS = listOf( + RetentionOption(0, R.string.retention_keep_all), + RetentionOption(3, R.string.retention_age_3m), + RetentionOption(6, R.string.retention_age_6m), + RetentionOption(12, R.string.retention_age_1y), + RetentionOption(24, R.string.retention_age_2y), +) + +/** + * Device-only retention controls (issue #13): a message-count group and an age group. When + * [includeUseDefault] is true (the per-account screen) each group also offers "use the global + * default" (persisted as null); the global screen omits it. Copy makes clear this never touches the + * server. + */ +@Composable +internal fun RetentionSection( + count: Int?, + months: Int?, + includeUseDefault: Boolean, + onCountChange: (Int?) -> Unit, + onMonthsChange: (Int?) -> Unit, +) { + SectionHeader(stringResource(R.string.settings_retention)) + Text( + text = stringResource(R.string.settings_retention_summary), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(horizontal = 16.dp), + ) + RetentionGroup(R.string.retention_count_title, COUNT_OPTIONS, count, includeUseDefault, onCountChange) + RetentionGroup(R.string.retention_age_title, AGE_OPTIONS, months, includeUseDefault, onMonthsChange) +} + +@Composable +private fun RetentionGroup( + titleRes: Int, + options: List, + current: Int?, + includeUseDefault: Boolean, + onChange: (Int?) -> Unit, +) { + Text( + text = stringResource(titleRes), + style = MaterialTheme.typography.titleSmall, + modifier = Modifier.padding(start = 16.dp, end = 16.dp, top = 12.dp, bottom = 4.dp), + ) + if (includeUseDefault) { + RadioRow( + title = stringResource(R.string.retention_use_default), + selected = current == null, + onClick = { onChange(null) }, + ) + } + options.forEach { option -> + RadioRow( + title = stringResource(option.labelRes), + selected = current == option.value, + onClick = { onChange(option.value) }, + ) + } +} diff --git a/app/src/main/kotlin/org/libremail/ui/settings/SettingsScreen.kt b/app/src/main/kotlin/org/libremail/ui/settings/SettingsScreen.kt index 4ea2d77..2ad97bb 100644 --- a/app/src/main/kotlin/org/libremail/ui/settings/SettingsScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/settings/SettingsScreen.kt @@ -5,11 +5,9 @@ import androidx.compose.animation.AnimatedVisibility import androidx.compose.foundation.clickable import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row -import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.width import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.verticalScroll import androidx.compose.material.icons.Icons @@ -18,7 +16,6 @@ import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.HorizontalDivider import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.RadioButton import androidx.compose.material3.Scaffold import androidx.compose.material3.Text import androidx.compose.material3.TopAppBar @@ -139,6 +136,16 @@ fun SettingsScreen( ) HorizontalDivider() + // Global device-only retention default (issue #13); accounts may override it. + RetentionSection( + count = settings.retentionCount, + months = settings.retentionMonths, + includeUseDefault = false, + onCountChange = { viewModel.setRetentionCount(it ?: 0) }, + onMonthsChange = { viewModel.setRetentionMonths(it ?: 0) }, + ) + HorizontalDivider() + AdvancedHeader(expanded = advancedExpanded, onToggle = viewModel::toggleAdvanced) AnimatedVisibility(visible = advancedExpanded) { Column { @@ -181,30 +188,6 @@ fun SettingsScreen( } } -@Composable -private fun RadioRow(title: String, subtitle: String?, selected: Boolean, onClick: () -> Unit) { - Row( - modifier = Modifier - .fillMaxWidth() - .clickable(onClick = onClick) - .padding(horizontal = 16.dp, vertical = 12.dp), - verticalAlignment = Alignment.CenterVertically, - ) { - RadioButton(selected = selected, onClick = onClick) - Spacer(Modifier.width(8.dp)) - Column(Modifier.weight(1f)) { - Text(title, style = MaterialTheme.typography.bodyLarge) - if (subtitle != null) { - Text( - subtitle, - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - } - } - } -} - @Composable private fun AdvancedHeader(expanded: Boolean, onToggle: () -> Unit) { Row( diff --git a/app/src/main/kotlin/org/libremail/ui/settings/SettingsViewModel.kt b/app/src/main/kotlin/org/libremail/ui/settings/SettingsViewModel.kt index fcd9883..95094b7 100644 --- a/app/src/main/kotlin/org/libremail/ui/settings/SettingsViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/settings/SettingsViewModel.kt @@ -15,6 +15,7 @@ import kotlinx.coroutines.launch import org.libremail.data.settings.AppSettings import org.libremail.data.settings.FetchPolicy import org.libremail.data.settings.SettingsRepository +import org.libremail.data.sync.SyncScheduler import org.libremail.domain.model.Account import org.libremail.domain.repository.AccountRepository import org.libremail.push.BatteryOptimizationManager @@ -25,6 +26,7 @@ class SettingsViewModel @Inject constructor( private val accountRepository: AccountRepository, private val settingsRepository: SettingsRepository, private val batteryOptimizationManager: BatteryOptimizationManager, + private val syncScheduler: SyncScheduler, ) : ViewModel() { val accounts: StateFlow> = accountRepository.observeAccounts() @@ -60,6 +62,17 @@ class SettingsViewModel @Inject constructor( fun setIncludeInBackup(value: Boolean) = update { settingsRepository.setIncludeInBackup(value) } fun setFetchPolicy(value: FetchPolicy) = update { settingsRepository.setFetchPolicy(value) } + /** Global retention defaults; kick a prune so a newly-tightened limit takes effect promptly (#13). */ + fun setRetentionCount(value: Int) = update { + settingsRepository.setRetentionCount(value) + syncScheduler.pruneNow() + } + + fun setRetentionMonths(value: Int) = update { + settingsRepository.setRetentionMonths(value) + syncScheduler.pruneNow() + } + private inline fun update(crossinline action: suspend () -> Unit) { viewModelScope.launch { action() } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index da43daa..faf80b4 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -196,6 +196,22 @@ Include settings in Android Backup Let Android back up your LibreMail preferences (Google Auto Backup) so they restore when you set up a new device. Your mail, accounts, passwords, and encryption keys are never backed up — only app settings. Off by default; uses Google infrastructure. + + Storage on this device + These limits apply to this device only. Mail beyond them is removed from local storage but never deleted from the server, so it can always be downloaded again. By default LibreMail keeps everything. + Default for all accounts + Keep by message count + Keep by age + Use the global default + Keep everything + Newest 500 per folder + Newest 1,000 per folder + Newest 5,000 per folder + Last 3 months + Last 6 months + Last year + Last 2 years + Account Signature diff --git a/app/src/test/kotlin/org/libremail/data/settings/RetentionPolicyTest.kt b/app/src/test/kotlin/org/libremail/data/settings/RetentionPolicyTest.kt new file mode 100644 index 0000000..b3c6f17 --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/settings/RetentionPolicyTest.kt @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.settings + +import org.junit.Test +import java.time.Instant +import java.time.ZoneOffset +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class RetentionPolicyTest { + + @Test + fun `per-account override wins over the global default per dimension`() { + val policy = RetentionPolicy.resolve( + accountCount = 500, + accountMonths = null, + defaultCount = 1000, + defaultMonths = 6, + ) + assertEquals(500, policy.count) // account override + assertEquals(6, policy.months) // inherited default + } + + @Test + fun `null overrides inherit the global default`() { + val policy = RetentionPolicy.resolve(null, null, defaultCount = 1000, defaultMonths = 12) + assertEquals(1000, policy.count) + assertEquals(12, policy.months) + } + + @Test + fun `an explicit account zero overrides a non-zero global default back to unlimited`() { + val policy = + RetentionPolicy.resolve(accountCount = 0, accountMonths = 0, defaultCount = 1000, defaultMonths = 6) + assertTrue(policy.isUnlimited) + assertNull(policy.countLimit) + } + + @Test + fun `keep-everything default is unlimited and has no cutoffs`() { + val policy = RetentionPolicy.KEEP_EVERYTHING + assertTrue(policy.isUnlimited) + assertNull(policy.countLimit) + assertNull(policy.ageCutoffMillis(System.currentTimeMillis())) + } + + @Test + fun `count limit is exposed only when positive`() { + assertEquals(500, RetentionPolicy(count = 500, months = 0).countLimit) + assertNull(RetentionPolicy(count = 0, months = 0).countLimit) + assertFalse(RetentionPolicy(count = 500, months = 0).isUnlimited) + } + + @Test + fun `age cutoff is N calendar months before now`() { + val now = Instant.parse("2026-06-30T00:00:00Z").toEpochMilli() + val cutoff = RetentionPolicy(count = 0, months = 3).ageCutoffMillis(now) + val expected = Instant.parse("2026-06-30T00:00:00Z") + .atZone(ZoneOffset.UTC).minusMonths(3).toInstant().toEpochMilli() + assertEquals(expected, cutoff) + // Sanity: the cutoff really is in the past. + assertTrue(cutoff!! < now) + } + + @Test + fun `negative inputs are clamped to unlimited`() { + val policy = RetentionPolicy.resolve(accountCount = -5, accountMonths = -1, defaultCount = 0, defaultMonths = 0) + assertEquals(0, policy.count) + assertEquals(0, policy.months) + assertTrue(policy.isUnlimited) + } +} diff --git a/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt new file mode 100644 index 0000000..3e032a4 --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt @@ -0,0 +1,253 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import android.content.Context +import com.icegreen.greenmail.util.GreenMail +import com.icegreen.greenmail.util.ServerSetupTest +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import jakarta.mail.Folder +import jakarta.mail.Message +import jakarta.mail.Session +import jakarta.mail.internet.InternetAddress +import jakarta.mail.internet.MimeMessage +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.libremail.data.local.dao.AccountDao +import org.libremail.data.local.dao.BackfillProgressDao +import org.libremail.data.local.dao.MessageDao +import org.libremail.data.local.entity.AccountEntity +import org.libremail.data.local.entity.BackfillProgressEntity +import org.libremail.data.local.entity.MessageEntity +import org.libremail.data.local.entity.ServerConfigEmbedded +import org.libremail.data.local.toEntity +import org.libremail.data.settings.AccountSettingsRepository +import org.libremail.data.settings.AppSettings +import org.libremail.data.settings.FetchPolicy +import org.libremail.data.settings.SettingsRepository +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.ImapClient +import java.util.Properties +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertTrue + +/** + * End-to-end backfill (issue #12) against a real in-process IMAP server with in-memory DAO fakes: + * the backfiller caches the WHOLE mailbox (far more than the 50-message foreground window), persists + * a resumable boundary, resumes correctly after a simulated interruption, honours the retention floor + * (issue #13 precedence), and never deletes anything. + */ +class MailBackfillerTest { + + private lateinit var greenMail: GreenMail + private val client = ImapClient() + + private val accountEntity = AccountEntity( + id = "acct", + email = "alice@example.org", + displayName = "Alice", + authType = "PASSWORD_IMAP", + imap = ServerConfigEmbedded("127.0.0.1", 993, "NONE"), + smtp = ServerConfigEmbedded("127.0.0.1", 465, "NONE"), + ) + + // In-memory stand-ins for the two tables the backfiller writes. + private val cached = mutableListOf() + private val progress = mutableMapOf, BackfillProgressEntity>() + + @Before + fun setUp() { + greenMail = GreenMail(ServerSetupTest.SMTP_IMAP) + greenMail.start() + greenMail.setUser("alice@example.org", "secret") + } + + @After + fun tearDown() = greenMail.stop() + + private fun params() = ImapConnectionParams( + host = "127.0.0.1", + port = greenMail.imap.port, + security = MailSecurity.NONE, + username = "alice@example.org", + secret = "secret", + useXoauth2 = false, + ) + + @Test + fun `backfills the entire history over successive runs, far beyond the 50-message window`() = runTest { + appendMessages(TOTAL) + seedForegroundWindow() + assertEquals(WINDOW, cached.size) + + val backfiller = backfiller(AccountSettings("acct")) + // Drive it to completion; each run does a bounded slice. + var guard = 0 + while (backfiller.runBackfill() && guard++ < 10) { /* keep going until no more work */ } + + assertEquals(TOTAL, distinctCachedUids().size, "backfill must cache every message") + assertTrue(cached.size > WINDOW, "that is strictly more than the foreground window") + assertEquals(true, progress["acct" to "INBOX"]?.complete) + assertNoDeletes() + } + + @Test + fun `resumes from the persisted boundary after an interruption`() = runTest { + appendMessages(TOTAL) + seedForegroundWindow() + + // First run: only ONE page, then "process death" — keep just cached rows + persisted boundary. + val partial = backfiller(AccountSettings("acct")).runBackfill(maxBatches = 1) + assertTrue(partial, "one page can't finish 120 messages") + assertTrue(cached.size in (WINDOW + 1) until TOTAL) + val boundary = progress["acct" to "INBOX"] + assertNotNull(boundary) + assertFalse(boundary.complete) + + // Fresh backfiller instance (new process) resumes ONLY from what was persisted. + val resumed = backfiller(AccountSettings("acct")) + var guard = 0 + while (resumed.runBackfill() && guard++ < 10) { /* finish */ } + + assertEquals(TOTAL, distinctCachedUids().size, "resume completes the full history") + assertEquals(TOTAL, cached.size, "no message is fetched twice") + assertNoDeletes() + } + + @Test + fun `stops at the retention floor instead of paging the whole folder`() = runTest { + appendMessages(TOTAL) + seedForegroundWindow() + + // Keep only the newest 60: backfill pages until it has at least 60, then stops well short of 120. + val backfiller = backfiller(AccountSettings("acct", retentionCount = 60)) + backfiller.runBackfill() + val afterFirst = cached.size + + assertTrue(afterFirst >= 60, "must fetch at least up to the retention floor") + assertTrue(afterFirst < TOTAL, "must NOT page the entire 120-message history") + // Paused at the floor (NOT marked complete, so a later loosening could resume), and stable: + // running again fetches nothing more. + assertFalse(progress["acct" to "INBOX"]!!.complete) + backfiller.runBackfill() + assertEquals(afterFirst, cached.size, "at the floor, further runs must not fetch more") + assertNoDeletes() + } + + /** Builds a backfiller wired to GreenMail with the in-memory fakes and the given account settings. */ + private fun backfiller(accountSettings: AccountSettings): MailBackfiller { + val accountDao = mockk() + coEvery { accountDao.getAll() } returns listOf(accountEntity) + + val messageDao = mockk(relaxed = true) + coEvery { messageDao.insertNew(any()) } answers { + firstArg>().forEach { e -> if (cached.none { it.id == e.id }) cached += e } + } + coEvery { messageDao.syncedFolders("acct") } answers { + cached.filter { it.inInbox }.map { it.folder }.distinct() + } + coEvery { messageDao.lowestSyncedUid("acct", any()) } answers { + val folder = secondArg() + cached.filter { it.inInbox && it.folder == folder }.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()] } + coEvery { backfillProgressDao.upsert(any()) } answers { + val p = firstArg() + progress[p.accountId to p.folder] = p + } + + val connectionFactory = mockk() + coEvery { connectionFactory.imapParamsFor(any()) } returns params() + + val settingsRepository = mockk() + coEvery { settingsRepository.fetchPolicy() } returns FetchPolicy.ON_DEMAND + every { settingsRepository.settings } returns flowOf(AppSettings()) + + val accountSettingsRepository = mockk() + coEvery { accountSettingsRepository.get("acct") } returns accountSettings + + return MailBackfiller( + context = mockk(relaxed = true), + accountDao = accountDao, + messageDao = messageDao, + backfillProgressDao = backfillProgressDao, + imapClient = client, + connectionFactory = connectionFactory, + settingsRepository = settingsRepository, + accountSettingsRepository = accountSettingsRepository, + mailRepository = mockk(relaxed = true), + maintenanceGate = MailMaintenanceGate(), + ).also { lastMessageDao = messageDao } + } + + private var lastMessageDao: MessageDao? = null + + /** Seeds the in-memory cache with the newest [WINDOW] headers, mimicking a prior foreground sync. */ + private suspend fun seedForegroundWindow() { + client.fetchRecent(params(), "INBOX", limit = WINDOW) + .map { it.toEntity("acct", "INBOX") } + .forEach { cached += it } + } + + private fun distinctCachedUids(): Set = cached.map { it.uid }.toSet() + + private fun assertNoDeletes() { + val dao = lastMessageDao ?: return + coVerify(exactly = 0) { dao.deleteByIds(any()) } + coVerify(exactly = 0) { dao.deleteSyncedNotIn(any(), any(), any()) } + coVerify(exactly = 0) { dao.deleteSyncedInWindowNotIn(any(), any(), any(), any()) } + coVerify(exactly = 0) { dao.deleteSyncedByAccountFolder(any(), any()) } + } + + private fun appendMessages(count: Int) { + val props = Properties().apply { + put("mail.store.protocol", "imap") + put("mail.imap.host", "127.0.0.1") + put("mail.imap.port", greenMail.imap.port.toString()) + } + val session = Session.getInstance(props) + val store = session.getStore("imap") + store.connect("127.0.0.1", greenMail.imap.port, "alice@example.org", "secret") + try { + val inbox = store.getFolder("INBOX") + inbox.open(Folder.READ_WRITE) + val messages = (1..count).map { i -> + MimeMessage(session).apply { + setFrom(InternetAddress("sender$i@example.org")) + setRecipient(Message.RecipientType.TO, InternetAddress("alice@example.org")) + subject = "Message $i" + setText("Body of message $i") + } + }.toTypedArray() + inbox.appendMessages(messages) + inbox.close(false) + } finally { + store.close() + } + } + + private companion object { + const val TOTAL = 120 + const val WINDOW = 50 + } +} diff --git a/app/src/test/kotlin/org/libremail/data/sync/MailPrunerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/MailPrunerTest.kt new file mode 100644 index 0000000..acc09e8 --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/sync/MailPrunerTest.kt @@ -0,0 +1,139 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import android.content.Context +import io.mockk.Runs +import io.mockk.coEvery +import io.mockk.coVerify +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.slot +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.runTest +import org.junit.Test +import org.libremail.data.local.dao.AccountDao +import org.libremail.data.local.dao.MessageDao +import org.libremail.data.local.entity.AccountEntity +import org.libremail.data.local.entity.ServerConfigEmbedded +import org.libremail.data.settings.AccountSettingsRepository +import org.libremail.data.settings.AppSettings +import org.libremail.data.settings.SettingsRepository +import org.libremail.domain.model.AccountSettings +import java.io.File +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Retention pruning (issue #13) is purely LOCAL: [MailPruner] has no IMAP collaborator at all, so it + * is structurally incapable of issuing a server delete — these tests assert it removes exactly the + * over-limit local rows (by count and by age) and nothing when retention is unlimited. + */ +class MailPrunerTest { + + private val account = AccountEntity( + id = "acct", + email = "a@example.org", + displayName = "A", + authType = "PASSWORD_IMAP", + imap = ServerConfigEmbedded("imap.example.org", 993, "SSL_TLS"), + smtp = ServerConfigEmbedded("smtp.example.org", 465, "SSL_TLS"), + ) + + private fun pruner(global: AppSettings, accountSettings: AccountSettings, messageDao: MessageDao): MailPruner { + val accountDao = mockk() + coEvery { accountDao.getAll() } returns listOf(account) + val settingsRepository = mockk() + every { settingsRepository.settings } returns flowOf(global) + val accountSettingsRepository = mockk() + coEvery { accountSettingsRepository.get("acct") } returns accountSettings + val context = mockk(relaxed = true) + every { context.cacheDir } returns File(System.getProperty("java.io.tmpdir"), "libremail-prune-test") + return MailPruner( + context = context, + accountDao = accountDao, + messageDao = messageDao, + settingsRepository = settingsRepository, + accountSettingsRepository = accountSettingsRepository, + maintenanceGate = MailMaintenanceGate(), + ) + } + + @Test + fun `count limit prunes the over-limit rows of each folder and never the server`() = runTest { + val messageDao = mockk(relaxed = true) + coEvery { messageDao.syncedFolders("acct") } returns listOf("INBOX", "Archive") + coEvery { messageDao.syncedIdsBeyondCountInFolder("acct", "INBOX", 2) } returns + listOf("acct:INBOX:1", "acct:INBOX:2") + coEvery { messageDao.syncedIdsBeyondCountInFolder("acct", "Archive", 2) } returns listOf("acct:Archive:9") + val deleted = slot>() + coEvery { messageDao.deleteByIds(capture(deleted)) } just Runs + + val removed = pruner( + global = AppSettings(retentionCount = 2, retentionMonths = 0), + accountSettings = AccountSettings("acct"), + messageDao = messageDao, + ).prune() + + assertEquals(setOf("acct:INBOX:1", "acct:INBOX:2", "acct:Archive:9"), deleted.captured.toSet()) + assertEquals(3, removed) + // Age was unlimited, so the age query is never issued. + coVerify(exactly = 0) { messageDao.syncedIdsOlderThan(any(), any()) } + } + + @Test + fun `age limit prunes rows older than the cutoff across all folders`() = runTest { + val messageDao = mockk(relaxed = true) + val cutoff = slot() + coEvery { messageDao.syncedIdsOlderThan(eq("acct"), capture(cutoff)) } returns + listOf("acct:INBOX:1", "acct:Sent:4") + val deleted = slot>() + coEvery { messageDao.deleteByIds(capture(deleted)) } just Runs + + val now = 1_800_000_000_000L + val removed = pruner( + global = AppSettings(retentionCount = 0, retentionMonths = 6), + accountSettings = AccountSettings("acct"), + messageDao = messageDao, + ).prune(nowMillis = now) + + assertEquals(setOf("acct:INBOX:1", "acct:Sent:4"), deleted.captured.toSet()) + assertEquals(2, removed) + assertTrue(cutoff.captured < now, "age cutoff must be strictly before now") + // Count was unlimited, so no per-folder count query runs. + coVerify(exactly = 0) { messageDao.syncedIdsBeyondCountInFolder(any(), any(), any()) } + } + + @Test + fun `per-account override takes precedence over the global default`() = runTest { + val messageDao = mockk(relaxed = true) + coEvery { messageDao.syncedFolders("acct") } returns listOf("INBOX") + coEvery { messageDao.syncedIdsBeyondCountInFolder("acct", "INBOX", 10) } returns listOf("acct:INBOX:1") + coEvery { messageDao.deleteByIds(any()) } just Runs + + // Global default is unlimited; the account overrides count to 10, so pruning still runs at 10. + pruner( + global = AppSettings(retentionCount = 0, retentionMonths = 0), + accountSettings = AccountSettings("acct", retentionCount = 10), + messageDao = messageDao, + ).prune() + + coVerify { messageDao.syncedIdsBeyondCountInFolder("acct", "INBOX", 10) } + } + + @Test + fun `keeps everything and deletes nothing when retention is unlimited`() = runTest { + val messageDao = mockk(relaxed = true) + + val removed = pruner( + global = AppSettings(retentionCount = 0, retentionMonths = 0), + accountSettings = AccountSettings("acct"), + messageDao = messageDao, + ).prune() + + assertEquals(0, removed) + coVerify(exactly = 0) { messageDao.deleteByIds(any()) } + coVerify(exactly = 0) { messageDao.syncedIdsOlderThan(any(), any()) } + coVerify(exactly = 0) { messageDao.syncedIdsBeyondCountInFolder(any(), any(), any()) } + } +} diff --git a/app/src/test/kotlin/org/libremail/data/sync/MailSyncerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/MailSyncerTest.kt index 154b851..186834c 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/MailSyncerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/MailSyncerTest.kt @@ -9,6 +9,7 @@ import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.mockk +import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest import org.junit.Test import org.libremail.data.local.dao.AccountDao @@ -16,6 +17,7 @@ import org.libremail.data.local.dao.MessageDao import org.libremail.data.local.entity.AccountEntity import org.libremail.data.local.entity.ServerConfigEmbedded import org.libremail.data.settings.AccountSettingsRepository +import org.libremail.data.settings.AppSettings import org.libremail.data.settings.FetchPolicy import org.libremail.data.settings.SettingsRepository import org.libremail.domain.model.AccountSettings @@ -36,11 +38,16 @@ class MailSyncerTest { smtp = ServerConfigEmbedded("smtp.example.org", 465, "SSL_TLS"), ) + /** The IMAP client of the most recently built [syncer], for verifying the fetch window size. */ + private lateinit var lastImapClient: ImapClient + /** A syncer whose header sync is a no-op (no server messages) so tests focus on the prefetch step. */ private fun syncer( policy: FetchPolicy, mailRepository: MailRepository, context: Context = mockk(relaxed = true), + accountSettings: AccountSettings = AccountSettings("acct"), + globalSettings: AppSettings = AppSettings(), ): MailSyncer { val accountDao = mockk() coEvery { accountDao.getById("acct") } returns account @@ -49,12 +56,14 @@ class MailSyncerTest { coEvery { messageDao.getUnfetchedIds("acct", "INBOX") } returns listOf("acct:INBOX:1") val imapClient = mockk() coEvery { imapClient.fetchRecent(any(), any(), any()) } returns emptyList() + lastImapClient = imapClient val connectionFactory = mockk() coEvery { connectionFactory.imapParamsFor(any()) } returns mockk() val settingsRepository = mockk() coEvery { settingsRepository.fetchPolicy() } returns policy + every { settingsRepository.settings } returns flowOf(globalSettings) val accountSettingsRepository = mockk() - coEvery { accountSettingsRepository.get(any()) } returns AccountSettings("acct") + coEvery { accountSettingsRepository.get(any()) } returns accountSettings return MailSyncer( context = context, accountDao = accountDao, @@ -87,6 +96,37 @@ class MailSyncerTest { coVerify(exactly = 0) { repo.prefetchMessage(any()) } } + @Test + fun `caps the recent fetch window to a retention count below the default`() = runTest { + val repo = mockk(relaxed = true) + + syncer(FetchPolicy.ON_DEMAND, repo, accountSettings = AccountSettings("acct", retentionCount = 20)) + .syncFolder("acct", "INBOX") + + // Foreground must not fetch more than retention keeps, or it would re-download pruned rows. + coVerify { lastImapClient.fetchRecent(any(), "INBOX", 20) } + } + + @Test + fun `uses the full window when the retention count exceeds it`() = runTest { + val repo = mockk(relaxed = true) + + syncer(FetchPolicy.ON_DEMAND, repo, accountSettings = AccountSettings("acct", retentionCount = 5000)) + .syncFolder("acct", "INBOX") + + coVerify { lastImapClient.fetchRecent(any(), "INBOX", 50) } + } + + @Test + fun `age-only retention leaves the full recent window intact`() = runTest { + val repo = mockk(relaxed = true) + + syncer(FetchPolicy.ON_DEMAND, repo, accountSettings = AccountSettings("acct", retentionMonths = 6)) + .syncFolder("acct", "INBOX") + + coVerify { lastImapClient.fetchRecent(any(), "INBOX", 50) } + } + @Test fun `WIFI_ONLY prefetches on an unmetered network`() = runTest { val repo = mockk() @@ -148,6 +188,7 @@ class MailSyncerTest { val settingsRepository = mockk() coEvery { settingsRepository.isNewMailNotificationsEnabled() } returns globalEnabled coEvery { settingsRepository.fetchPolicy() } returns FetchPolicy.ON_DEMAND + every { settingsRepository.settings } returns flowOf(AppSettings()) val accountSettingsRepository = mockk() coEvery { accountSettingsRepository.get("acct") } returns AccountSettings("acct", notificationsEnabled = accountEnabled) diff --git a/app/src/test/kotlin/org/libremail/mail/ImapClientBackfillTest.kt b/app/src/test/kotlin/org/libremail/mail/ImapClientBackfillTest.kt new file mode 100644 index 0000000..b8dce46 --- /dev/null +++ b/app/src/test/kotlin/org/libremail/mail/ImapClientBackfillTest.kt @@ -0,0 +1,148 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.mail + +import com.icegreen.greenmail.util.GreenMail +import com.icegreen.greenmail.util.ServerSetupTest +import jakarta.mail.Folder +import jakarta.mail.Message +import jakarta.mail.Session +import jakarta.mail.internet.InternetAddress +import jakarta.mail.internet.MimeMessage +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.libremail.domain.model.ImapConnectionParams +import org.libremail.domain.model.MailSecurity +import java.util.Properties +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * Exercises the full-history backfill paging primitive ([ImapClient.fetchOlderThan]) against a real + * in-process IMAP server (issue #12): it pages a mailbox far larger than the 50-message foreground + * window, and a page sequence RESUMED from a persisted boundary still yields the complete history + * with no gaps or duplicates. + */ +class ImapClientBackfillTest { + + private lateinit var greenMail: GreenMail + private val client = ImapClient() + + @Before + fun setUp() { + greenMail = GreenMail(ServerSetupTest.SMTP_IMAP) + greenMail.start() + greenMail.setUser("alice@example.org", "secret") + } + + @After + fun tearDown() = greenMail.stop() + + private fun params() = ImapConnectionParams( + host = "127.0.0.1", + port = greenMail.imap.port, + security = MailSecurity.NONE, + username = "alice@example.org", + secret = "secret", + useXoauth2 = false, + ) + + @Test + fun `pages a mailbox larger than the foreground window down to the first message`() = runTest { + appendMessages(TOTAL) + + // The foreground window would only ever cache the newest 50; backfill must reach everything. + val recent = client.fetchRecent(params(), "INBOX", limit = 50) + assertEquals(50, recent.size) + + val allUids = pageEntireHistory(pageSize = 50) + + assertEquals(TOTAL, allUids.size, "backfill must page in the whole mailbox, not just 50") + assertEquals(TOTAL, allUids.toSet().size, "every page must be distinct (no overlap)") + assertTrue(allUids.size > 50, "backfill caches strictly MORE than the 50-message window") + } + + @Test + fun `resumes from a persisted boundary after an interruption without gaps or duplicates`() = runTest { + appendMessages(TOTAL) + + // First run: page a couple of batches, then "crash" — keep only the persisted boundary UID. + val firstRun = mutableListOf() + var boundary = Long.MAX_VALUE + repeat(2) { + val page = client.fetchOlderThan(params(), "INBOX", boundary, PAGE) + firstRun += page.map { it.uid.toLong() } + boundary = page.minOf { it.uid.toLong() } + } + assertEquals(2 * PAGE, firstRun.size) + + // Second run (fresh process): resume ONLY from the stored boundary and finish the history. + val resumed = mutableListOf() + while (true) { + val page = client.fetchOlderThan(params(), "INBOX", boundary, PAGE) + if (page.isEmpty()) break + resumed += page.map { it.uid.toLong() } + boundary = page.minOf { it.uid.toLong() } + } + + val combined = firstRun + resumed + assertEquals(TOTAL, combined.size, "resume must complete the full history") + assertEquals(TOTAL, combined.toSet().size, "resume must not re-fetch or skip any message") + } + + @Test + fun `returns empty once the oldest message is reached`() = runTest { + appendMessages(3) + val oldest = pageEntireHistory(pageSize = 2).min() + + // Nothing exists below the very first UID. + assertTrue(client.fetchOlderThan(params(), "INBOX", oldest, 50).isEmpty()) + } + + /** Pages the entire mailbox backwards via [ImapClient.fetchOlderThan], returning every UID seen. */ + private suspend fun pageEntireHistory(pageSize: Int): List { + val uids = mutableListOf() + var boundary = Long.MAX_VALUE + while (true) { + val page = client.fetchOlderThan(params(), "INBOX", boundary, pageSize) + if (page.isEmpty()) break + uids += page.map { it.uid.toLong() } + boundary = page.minOf { it.uid.toLong() } + } + return uids + } + + /** Appends [count] distinct messages to INBOX via IMAP (faster than SMTP delivery for bulk). */ + private fun appendMessages(count: Int) { + val props = Properties().apply { + put("mail.store.protocol", "imap") + put("mail.imap.host", "127.0.0.1") + put("mail.imap.port", greenMail.imap.port.toString()) + } + val session = Session.getInstance(props) + val store = session.getStore("imap") + store.connect("127.0.0.1", greenMail.imap.port, "alice@example.org", "secret") + try { + val inbox = store.getFolder("INBOX") + inbox.open(Folder.READ_WRITE) + val messages = (1..count).map { i -> + MimeMessage(session).apply { + setFrom(InternetAddress("sender$i@example.org")) + setRecipient(Message.RecipientType.TO, InternetAddress("alice@example.org")) + subject = "Message $i" + setText("Body of message $i") + } + }.toTypedArray() + inbox.appendMessages(messages) + inbox.close(false) + } finally { + store.close() + } + } + + private companion object { + const val TOTAL = 120 + const val PAGE = 50 + } +} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index f3b1b1e..83df43b 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -63,6 +63,8 @@ androidx-hilt-compiler = { group = "androidx.hilt", name = "hilt-compiler", vers androidx-room-runtime = { group = "androidx.room", name = "room-runtime", version.ref = "room" } androidx-room-ktx = { group = "androidx.room", name = "room-ktx", version.ref = "room" } androidx-room-compiler = { group = "androidx.room", name = "room-compiler", version.ref = "room" } +# Room MigrationTestHelper (instrumented migration tests). +androidx-room-testing = { group = "androidx.room", name = "room-testing", version.ref = "room" } # SQLCipher — opt-in at-rest encryption of the Room cache. sqlcipher-android = { group = "net.zetetic", name = "sqlcipher-android", version.ref = "sqlcipher" } From b3ac6d4353f8c215b2f57b0201d3249943e56a13 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 1 Jul 2026 12:56:02 -0500 Subject: [PATCH 2/7] fix(build): pin kotlinx-serialization to 1.8.1 for Room migration tests AGP 9's consistent resolution shares the runtime serialization version with the androidTest classpath, where androidx.savedstate pins it to 1.7.3. Room 2.8's schema-bundle serializers are compiled against >= 1.8.0, so MigrationTestHelper threw AbstractMethodError on GeneratedSerializer.typeParametersSerializers(), failing every E2E job. Import the serialization BOM as a platform to force 1.8.1. Verified on-device (API 37): Migration9To10Test fails unpatched, passes patched. Co-Authored-By: Claude Opus 4.8 --- app/build.gradle.kts | 5 +++++ gradle/libs.versions.toml | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 6b365c4..c7b0e30 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -189,6 +189,11 @@ dependencies { ksp(libs.androidx.room.compiler) implementation(libs.sqlcipher.android) + // Raise kotlinx-serialization to the version Room's schema-bundle serializers were compiled + // against (see libs.versions.toml). AGP 9 consistent resolution shares it with the androidTest + // classpath so MigrationTestHelper can parse the exported schema JSON. + implementation(platform(libs.kotlinx.serialization.bom)) + testImplementation(libs.junit) testImplementation(libs.kotlin.test) testImplementation(libs.kotlinx.coroutines.test) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 83df43b..9d72eef 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -15,6 +15,10 @@ sqlcipher = "4.16.0" datastore = "1.2.1" work = "2.11.2" coroutines = "1.10.2" +# Forced above androidx.savedstate's transitive 1.7.3 (shared with the androidTest classpath via +# AGP 9 consistent resolution); Room's schema-bundle serializers need >= 1.8.0 or MigrationTestHelper +# throws AbstractMethodError on GeneratedSerializer.typeParametersSerializers(). +kotlinxSerialization = "1.8.1" appauth = "0.11.1" angusMail = "2.0.5" junit = "4.13.2" @@ -76,6 +80,10 @@ androidx-work-runtime-ktx = { group = "androidx.work", name = "work-runtime-ktx" kotlinx-coroutines-android = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-android", version.ref = "coroutines" } kotlinx-coroutines-test = { group = "org.jetbrains.kotlinx", name = "kotlinx-coroutines-test", version.ref = "coroutines" } +# Serialization BOM — imported as a platform (not used directly) to raise the transitive +# kotlinx-serialization runtime to the version Room's schema bundles were compiled against. +kotlinx-serialization-bom = { group = "org.jetbrains.kotlinx", name = "kotlinx-serialization-bom", version.ref = "kotlinxSerialization" } + # Email transport / OAuth — wired in later increments appauth = { group = "net.openid", name = "appauth", version.ref = "appauth" } angus-mail = { group = "org.eclipse.angus", name = "angus-mail", version.ref = "angusMail" } From ec60348c89aba07ef035cca8c8c9d758d181e2a3 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 1 Jul 2026 14:22:40 -0500 Subject: [PATCH 3/7] test(sync): pin MessageDao retention/backfill boundary SQL on real SQLite The MailPruner/MailBackfiller unit tests mock the DAO, so the queries that actually define the device-only retention floor were never exercised against a real database: the newest-N-by-(timestampMillis, uid) prune selection, the strict age cutoff, the windowed reconcile that spares backfilled history, and the lowest-uid / count / oldest floor probes the backfiller stops on. Add an instrumented MessageDao test on an in-memory Room DB covering all of them, including the timestamp/uid tie-break direction (a flipped ORDER BY would locally delete the user's newest mail) and inInbox/folder/account scoping. This closes the disjoint-sets safety argument at the SQL-boundary level, not just the orchestration level. Verified on the API 37 emulator (5/5). Co-Authored-By: Claude Opus 4.8 --- .../data/local/MessageDaoRetentionTest.kt | 186 ++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 app/src/androidTest/kotlin/org/libremail/data/local/MessageDaoRetentionTest.kt diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/MessageDaoRetentionTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/MessageDaoRetentionTest.kt new file mode 100644 index 0000000..ecb2a74 --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/data/local/MessageDaoRetentionTest.kt @@ -0,0 +1,186 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.local + +import android.content.Context +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.runBlocking +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.libremail.data.local.dao.MessageDao +import org.libremail.data.local.entity.MessageEntity + +/** + * Real-SQLite behavior of the retention / backfill boundary queries on [MessageDao] (issues #12/#13). + * + * 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. + */ +@RunWith(AndroidJUnit4::class) +class MessageDaoRetentionTest { + + private lateinit var db: LibreMailDatabase + private lateinit var dao: MessageDao + + @Before + fun setUp() { + val context = ApplicationProvider.getApplicationContext() + db = Room.inMemoryDatabaseBuilder(context, LibreMailDatabase::class.java).build() + dao = db.messageDao() + } + + @After + fun tearDown() = db.close() + + private fun message( + id: String, + accountId: String = "acct", + folder: String = "INBOX", + uid: Long = 0L, + timestampMillis: Long = 1_000L, + inInbox: Boolean = true, + ) = MessageEntity( + id = id, + accountId = accountId, + sender = "Ada", + senderEmail = "ada@example.org", + subject = "Hi", + snippet = "", + body = "", + timestampMillis = timestampMillis, + isRead = false, + isStarred = false, + folder = folder, + inInbox = inInbox, + uid = uid, + ) + + /** + * The count-based prune boundary keeps the newest [keep] by recency and returns the REST for + * deletion, breaking timestamp ties by the higher UID. This pins the ordering *direction* (a + * flipped `DESC` would keep the OLDEST rows — i.e. locally delete the user's most recent mail) and + * the tie-break, neither of which the mocked-DAO unit tests can catch. + */ + @Test + fun syncedIdsBeyondCountInFolderKeepsNewestByTimestampThenUid() = runBlocking { + dao.insertNew( + listOf( + message("A", uid = 30, timestampMillis = 300), // newest + message("B", uid = 25, timestampMillis = 200), // ties C on timestamp; higher uid => newer + message("C", uid = 20, timestampMillis = 200), + message("D", uid = 10, timestampMillis = 100), // oldest + // Scoping decoys: a search-only row and another folder must never enter the ranking. + message("SR", uid = 99, timestampMillis = 999, inInbox = false), + message("AR", uid = 5, timestampMillis = 50, folder = "Archive"), + ), + ) + + // Keep the newest 2 (A, B); the rest are prunable. B is kept over C purely by the uid tie-break. + assertEquals( + setOf("C", "D"), + dao.syncedIdsBeyondCountInFolder("acct", "INBOX", keep = 2).toSet(), + ) + // Keeping at least as many as exist prunes nothing. + assertEquals(emptyList(), dao.syncedIdsBeyondCountInFolder("acct", "INBOX", keep = 4)) + } + + /** + * The age-based prune boundary selects rows STRICTLY older than the cutoff, across every folder, + * scoped to the account and to synced (non-search) rows only. + */ + @Test + fun syncedIdsOlderThanCutsStrictlyBelowAcrossFoldersAndScopesToAccount() = runBlocking { + dao.insertNew( + listOf( + message("boundary", uid = 25, timestampMillis = 200), // == cutoff -> kept (strict `<`) + message("old-inbox", uid = 10, timestampMillis = 100), + message("old-archive", uid = 5, timestampMillis = 150, folder = "Archive"), + message("old-search", uid = 1, timestampMillis = 1, inInbox = false), // not synced + message("old-other-account", accountId = "acct2", uid = 1, timestampMillis = 1), + ), + ) + + assertEquals( + setOf("old-inbox", "old-archive"), + dao.syncedIdsOlderThan("acct", cutoffMillis = 200).toSet(), + ) + } + + /** + * The windowed reconcile deletes only synced rows at/above the recent-UID window that the server no + * longer returns; older backfilled history (below the window), other folders, and search rows are + * left intact — the core guarantee that a foreground sync no longer wipes backfilled history (#12). + */ + @Test + fun deleteSyncedInWindowNotInSparesBelowWindowHistoryAndOtherFolders() = runBlocking { + dao.insertNew( + listOf( + message("below", uid = 10), // below the window -> spared + message("kept", uid = 20), // in window, in keep set -> spared + message("gone-1", uid = 30), // in window, not kept -> deleted + message("gone-2", uid = 40), // in window, not kept -> deleted + message("search", uid = 22, inInbox = false), // not synced -> spared + message("other-folder", uid = 25, folder = "Archive"), // different folder -> spared + ), + ) + + dao.deleteSyncedInWindowNotIn("acct", "INBOX", minWindowUid = 20, keepIds = listOf("kept")) + + assertEquals( + setOf("below", "kept", "search", "other-folder"), + dao.observeAll().first().map { it.id }.toSet(), + ) + } + + /** + * 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`). + */ + @Test + fun floorProbesReflectOnlySyncedRowsInTheFolder() = runBlocking { + dao.insertNew( + listOf( + message("a", uid = 30, timestampMillis = 300), + message("d", uid = 10, timestampMillis = 100), + 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")) + + assertNull(dao.lowestSyncedUid("acct", "Nonexistent")) + assertEquals(0, dao.countSynced("acct", "Nonexistent")) + assertNull(dao.oldestSyncedTimestamp("acct", "Nonexistent")) + } + + /** Backfill / prune enumerate their targets via [syncedFolders]: distinct synced folders, per account. */ + @Test + fun syncedFoldersReturnsDistinctSyncedFoldersForTheAccount() = runBlocking { + dao.insertNew( + listOf( + message("i1", folder = "INBOX"), + message("i2", folder = "INBOX"), + message("ar", folder = "Archive"), + message("sr", folder = "Search", inInbox = false), // search-only folder excluded + message("other", accountId = "acct2", folder = "Spam"), // other account excluded + ), + ) + + assertEquals(setOf("INBOX", "Archive"), dao.syncedFolders("acct").toSet()) + } +} From a73b6410a203d897aedf6305efe0e44166f06b5a Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 1 Jul 2026 14:33:13 -0500 Subject: [PATCH 4/7] test(sync): tally rows offered to insertNew to prove no double-fetch The backfiller's "no message fetched twice" claim (full-history + resume tests) previously rested on the insertNew fake de-duping by id, so a re-fetched page was silently absorbed and `cached.size == TOTAL` could not fail on it. Count the rows offered to insertNew BEFORE de-dupe and assert it equals TOTAL - WINDOW, so any re-request of an already-cached page now fails the test. This isolates the real boundary-descent guarantee and, unlike a fetchOlderThan call-count, is independent of BACKFILL_BATCH_SIZE. Co-Authored-By: Claude Opus 4.8 --- .../org/libremail/data/sync/MailBackfillerTest.kt | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) 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 3e032a4..a50feaf 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt @@ -65,6 +65,10 @@ class MailBackfillerTest { private val cached = mutableListOf() private val progress = mutableMapOf, BackfillProgressEntity>() + // Rows offered to insertNew, counted BEFORE de-dupe: a re-fetched page inflates this even + // though `cached` would silently absorb it. Isolates the "no message fetched twice" guarantee. + private var totalOffered = 0 + @Before fun setUp() { greenMail = GreenMail(ServerSetupTest.SMTP_IMAP) @@ -96,6 +100,7 @@ class MailBackfillerTest { while (backfiller.runBackfill() && guard++ < 10) { /* keep going until no more work */ } assertEquals(TOTAL, distinctCachedUids().size, "backfill must cache every message") + assertEquals(TOTAL - WINDOW, totalOffered, "each backfilled message fetched exactly once") assertTrue(cached.size > WINDOW, "that is strictly more than the foreground window") assertEquals(true, progress["acct" to "INBOX"]?.complete) assertNoDeletes() @@ -120,7 +125,8 @@ class MailBackfillerTest { while (resumed.runBackfill() && guard++ < 10) { /* finish */ } assertEquals(TOTAL, distinctCachedUids().size, "resume completes the full history") - assertEquals(TOTAL, cached.size, "no message is fetched twice") + assertEquals(TOTAL - WINDOW, totalOffered, "no re-fetch across the interruption") + assertEquals(TOTAL, cached.size, "and nothing is double-inserted") assertNoDeletes() } @@ -151,7 +157,9 @@ class MailBackfillerTest { val messageDao = mockk(relaxed = true) coEvery { messageDao.insertNew(any()) } answers { - firstArg>().forEach { e -> if (cached.none { it.id == e.id }) cached += e } + val batch = firstArg>() + totalOffered += batch.size // count BEFORE de-dupe (see field) + batch.forEach { e -> if (cached.none { it.id == e.id }) cached += e } } coEvery { messageDao.syncedFolders("acct") } answers { cached.filter { it.inInbox }.map { it.folder }.distinct() From 1d4103747f9848eed0907aea9339ac5b2944c8b2 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 1 Jul 2026 14:52:36 -0500 Subject: [PATCH 5/7] test(sync): assert MailMaintenanceGate serializes backfill and prune The gate's "backfill and prune never interleave" guarantee was only argued structurally: the MailBackfiller/MailPruner unit tests each build a throwaway, uncontended gate, so the serialization is never exercised. Add MailMaintenanceGateTest: - oneGateSerializesConcurrentCriticalSections: 50 coroutines contend on one gate; an overlap counter must never exceed 1 (guards against a per-access mutex). - aConcurrentPruneWaitsForAnInFlightBackfillToReleaseTheGate: a real MailBackfiller parks inside the gate (its fetchOlderThan suspended) while a real MailPruner is launched concurrently; the two share ONLY the gate, and the prune provably cannot enter its critical section until the backfill releases. Turns the defence-in-depth guarantee from argued to asserted. Co-Authored-By: Claude Opus 4.8 --- .../data/sync/MailMaintenanceGateTest.kt | 201 ++++++++++++++++++ 1 file changed, 201 insertions(+) create mode 100644 app/src/test/kotlin/org/libremail/data/sync/MailMaintenanceGateTest.kt diff --git a/app/src/test/kotlin/org/libremail/data/sync/MailMaintenanceGateTest.kt b/app/src/test/kotlin/org/libremail/data/sync/MailMaintenanceGateTest.kt new file mode 100644 index 0000000..340d135 --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/sync/MailMaintenanceGateTest.kt @@ -0,0 +1,201 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.joinAll +import kotlinx.coroutines.launch +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.yield +import org.junit.Test +import org.libremail.data.local.dao.AccountDao +import org.libremail.data.local.dao.MessageDao +import org.libremail.data.local.entity.AccountEntity +import org.libremail.data.local.entity.ServerConfigEmbedded +import org.libremail.data.settings.AccountSettingsRepository +import org.libremail.data.settings.AppSettings +import org.libremail.data.settings.SettingsRepository +import org.libremail.domain.model.AccountSettings +import org.libremail.domain.model.ImapConnectionParams +import org.libremail.domain.model.MailSecurity +import org.libremail.mail.FetchedMessage +import org.libremail.mail.ImapClient +import java.util.Collections +import java.util.concurrent.atomic.AtomicInteger +import kotlin.test.assertEquals + +/** + * The [MailMaintenanceGate]'s job is to serialize the two background maintenance jobs that both touch + * older cached history — the full-history backfill ([MailBackfiller], #12) and the retention pruner + * ([MailPruner], #13) — so they can never interleave (defence in depth behind the disjoint retention + * floor). The `MailBackfiller`/`MailPruner` unit tests each construct a throwaway, *uncontended* gate, + * so the serialization itself is never exercised there. These tests do: one asserts the gate's core + * contract (a single exclusive lock), the other wires a real backfiller and a real pruner to the SAME + * gate — the only object they share — and proves a concurrent prune blocks until the backfill releases. + */ +class MailMaintenanceGateTest { + + private val accountEntity = AccountEntity( + id = "acct", + email = "alice@example.org", + displayName = "Alice", + authType = "PASSWORD_IMAP", + imap = ServerConfigEmbedded("127.0.0.1", 993, "NONE"), + smtp = ServerConfigEmbedded("127.0.0.1", 465, "NONE"), + ) + + /** + * The gate's contract: everyone who acquires the *same* gate takes the *same* exclusive lock, so no + * two critical sections overlap. Guards against a regression that hands out a fresh lock per access + * (e.g. `val mutex get() = Mutex()`), which would compile but silently drop all serialization. + */ + @Test + fun oneGateSerializesConcurrentCriticalSections() = runBlocking { + val gate = MailMaintenanceGate() + val inside = AtomicInteger(0) + val maxObserved = AtomicInteger(0) + + val jobs = (1..50).map { + launch(Dispatchers.Default) { + gate.mutex.withLock { + val depth = inside.incrementAndGet() + maxObserved.getAndUpdate { current -> maxOf(current, depth) } + yield() // invite a sibling to (wrongly) enter while we still hold the lock + inside.decrementAndGet() + } + } + } + jobs.joinAll() + + assertEquals(1, maxObserved.get(), "one gate must never allow two critical sections at once") + } + + /** + * A real backfill slice holds the gate across its (here, suspended) server fetch; a prune launched + * concurrently must not enter its own critical section until the backfill releases the gate. The + * backfiller and pruner share *only* the [MailMaintenanceGate], so a pass is attributable solely to + * the gate. + */ + @Test + fun aConcurrentPruneWaitsForAnInFlightBackfillToReleaseTheGate() = runBlocking { + val gate = MailMaintenanceGate() + val events = Collections.synchronizedList(mutableListOf()) + val backfillInside = CompletableDeferred() + val releaseBackfill = CompletableDeferred() + + val backfiller = backfiller(gate, events, backfillInside, releaseBackfill) + val pruner = pruner(gate, events) + + // Backfill grabs the gate and parks inside it (its fetch is suspended on releaseBackfill). + val backfillJob = launch(Dispatchers.Default) { backfiller.runBackfill() } + backfillInside.await() + + // Prune now tries to run; it must block on the gate the backfill still holds. + val pruneJob = launch(Dispatchers.Default) { pruner.prune() } + delay(200) + assertEquals( + listOf("backfill-in"), + events.toList(), + "prune must not enter its critical section while backfill holds the gate", + ) + + // Release the backfill; only now may prune acquire the gate and run. + releaseBackfill.complete(Unit) + backfillJob.join() + pruneJob.join() + + assertEquals( + listOf("backfill-in", "prune-in"), + events.toList(), + "prune runs only after backfill releases the gate — never interleaved", + ) + } + + /** + * A [MailBackfiller] whose single server fetch parks inside the gate: it logs `backfill-in`, signals + * [backfillInside], then suspends on [releaseBackfill] while still holding the lock. Everything else + * is stubbed so exactly one bounded page is attempted. + */ + private fun backfiller( + gate: MailMaintenanceGate, + events: MutableList, + backfillInside: CompletableDeferred, + releaseBackfill: CompletableDeferred, + ): MailBackfiller { + val accountDao = mockk() + coEvery { accountDao.getAll() } returns listOf(accountEntity) + + val messageDao = mockk(relaxed = true) + coEvery { messageDao.syncedFolders("acct") } returns listOf("INBOX") + + val imapClient = mockk() + coEvery { imapClient.fetchOlderThan(any(), any(), any(), any()) } coAnswers { + events.add("backfill-in") + backfillInside.complete(Unit) + releaseBackfill.await() + emptyList() + } + + val connectionFactory = mockk() + coEvery { connectionFactory.imapParamsFor(any()) } returns ImapConnectionParams( + host = "h", + port = 143, + security = MailSecurity.NONE, + username = "u", + secret = "s", + useXoauth2 = false, + ) + + val settingsRepository = mockk() + every { settingsRepository.settings } returns flowOf(AppSettings()) + + val accountSettingsRepository = mockk() + coEvery { accountSettingsRepository.get("acct") } returns AccountSettings("acct") + + return MailBackfiller( + context = mockk(relaxed = true), + accountDao = accountDao, + messageDao = messageDao, + backfillProgressDao = mockk(relaxed = true), + imapClient = imapClient, + connectionFactory = connectionFactory, + settingsRepository = settingsRepository, + accountSettingsRepository = accountSettingsRepository, + mailRepository = mockk(relaxed = true), + maintenanceGate = gate, + ) + } + + /** + * A [MailPruner] that logs `prune-in` the instant it enters its critical section, then no-ops + * because retention is unlimited. + */ + private fun pruner(gate: MailMaintenanceGate, events: MutableList): MailPruner { + val accountDao = mockk() + coEvery { accountDao.getAll() } returns listOf(accountEntity) + + val settingsRepository = mockk() + every { settingsRepository.settings } answers { + events.add("prune-in") // first read happens inside prune()'s withLock + flowOf(AppSettings()) + } + + val accountSettingsRepository = mockk() + coEvery { accountSettingsRepository.get("acct") } returns AccountSettings("acct") + + return MailPruner( + context = mockk(relaxed = true), + accountDao = accountDao, + messageDao = mockk(relaxed = true), + settingsRepository = settingsRepository, + accountSettingsRepository = accountSettingsRepository, + maintenanceGate = gate, + ) + } +} From eb0e649c30f989fc485d7278dc82b5ae89011596 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 1 Jul 2026 15:08:16 -0500 Subject: [PATCH 6/7] fix(test): avoid observeAll() in MessageDaoRetentionTest (CI compile glitch) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The androidTest compile failed ONLY in CI with "Unresolved reference 'observeAll'" on the single line using dao.observeAll(), while every other MessageDao call in the same file resolved, the identical observeAll().first().map{}.toSet() in LibreMailDatabaseTest compiled fine in the same unit, and the file compiled cleanly locally (even `clean --no-build-cache`). That points to a Kotlin incremental-compilation artifact specific to the newly-added file, not a code error. Replace the observeAll()-based readback with explicit getById point lookups — a clearer per-row assertion that also sidesteps the glitch. Verified on the API 37 emulator (5/5). Co-Authored-By: Claude Opus 4.8 --- .../data/local/MessageDaoRetentionTest.kt | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) 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 ecb2a74..9e4e2e5 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/MessageDaoRetentionTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/MessageDaoRetentionTest.kt @@ -5,10 +5,10 @@ import android.content.Context import androidx.room.Room import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 -import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull import org.junit.Before import org.junit.Test @@ -138,10 +138,14 @@ class MessageDaoRetentionTest { dao.deleteSyncedInWindowNotIn("acct", "INBOX", minWindowUid = 20, keepIds = listOf("kept")) - assertEquals( - setOf("below", "kept", "search", "other-folder"), - dao.observeAll().first().map { it.id }.toSet(), - ) + // Read survivors back with point lookups (getById) rather than observeAll(): explicit about each + // row's fate, and it keeps the assertion off the Flow API. + assertNull("gone-1 is in-window and unkept -> deleted", dao.getById("gone-1")) + assertNull("gone-2 is in-window and unkept -> deleted", dao.getById("gone-2")) + assertNotNull("below-window history must survive", dao.getById("below")) + assertNotNull("the kept row must survive", dao.getById("kept")) + assertNotNull("search rows are not synced -> untouched", dao.getById("search")) + assertNotNull("other folders are untouched", dao.getById("other-folder")) } /** From 6ea02f588da9fa7a37b6d717a9c28f52c323a4e2 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 1 Jul 2026 21:53:43 -0500 Subject: [PATCH 7/7] fix(sync): resolve code-review findings on fetch-all history + retention Addresses the review of PR #46 (#12/#13): - Age-retention backfill/prune loop: mark a folder complete at the retention floor and resume from the persisted nextBeforeUid low-water mark; loosening resumes via AccountRepository.resetBackfillProgress. - Guard the windowed reconcile bound to the lowest positive UID so a getUID==-1 message can't collapse it and wipe backfilled history. - Order count-based retention by uid DESC to match the fetch window, ending the re-fetch/re-prune churn for high-UID/old-Date messages. - BackfillWorker chains slices while work remains. - Extract shared effectiveRetention / isActiveNetworkUnmetered / attachmentCacheDir helpers; remove dead deleteSyncedNotIn/getForAccount; refresh only pre-existing rows in persistBatch; add composite index (accountId, folder, uid) with migration + regenerated 13.json. Adds an age-floor prune regression test. Fast gate + androidTest compile green on JDK 21. Follow-ups filed for below-the-cut findings: #93, #94, #95, #96. Co-Authored-By: Claude Opus 4.8 --- .../13.json | 15 +++- .../data/local/LibreMailDatabaseTest.kt | 5 +- .../data/local/MessageDaoRetentionTest.kt | 24 +++--- .../kotlin/org/libremail/ui/Fakes.kt | 2 + .../org/libremail/data/AttachmentCache.kt | 15 ++++ .../org/libremail/data/local/Migrations.kt | 6 ++ .../data/local/dao/BackfillProgressDao.kt | 7 +- .../libremail/data/local/dao/MessageDao.kt | 22 ++--- .../data/local/entity/MessageEntity.kt | 5 +- .../data/repository/AccountRepositoryImpl.kt | 5 ++ .../data/repository/MailRepositoryImpl.kt | 4 +- .../data/settings/RetentionPolicy.kt | 23 ++++++ .../org/libremail/data/sync/BackfillWorker.kt | 10 ++- .../org/libremail/data/sync/MailBackfiller.kt | 80 +++++++++---------- .../org/libremail/data/sync/MailPruner.kt | 16 +--- .../org/libremail/data/sync/MailSyncer.kt | 36 +++------ .../org/libremail/data/sync/NetworkStatus.kt | 17 ++++ .../domain/repository/AccountRepository.kt | 7 ++ .../ui/settings/AccountSettingsViewModel.kt | 2 + .../ui/settings/SettingsViewModel.kt | 2 + .../libremail/data/sync/MailBackfillerTest.kt | 59 ++++++++++++-- 21 files changed, 243 insertions(+), 119 deletions(-) create mode 100644 app/src/main/kotlin/org/libremail/data/AttachmentCache.kt create mode 100644 app/src/main/kotlin/org/libremail/data/sync/NetworkStatus.kt diff --git a/app/schemas/org.libremail.data.local.LibreMailDatabase/13.json b/app/schemas/org.libremail.data.local.LibreMailDatabase/13.json index fd2565d..a66d542 100644 --- a/app/schemas/org.libremail.data.local.LibreMailDatabase/13.json +++ b/app/schemas/org.libremail.data.local.LibreMailDatabase/13.json @@ -2,7 +2,7 @@ "formatVersion": 1, "database": { "version": 13, - "identityHash": "c064a4da054e0f98b4688ea2d04ecaec", + "identityHash": "e4f7ef1e0d780324d6eec3efced768ae", "entities": [ { "tableName": "accounts", @@ -256,6 +256,17 @@ ], "orders": [], "createSql": "CREATE INDEX IF NOT EXISTS `index_messages_timestampMillis` ON `${TABLE_NAME}` (`timestampMillis`)" + }, + { + "name": "index_messages_accountId_folder_uid", + "unique": false, + "columnNames": [ + "accountId", + "folder", + "uid" + ], + "orders": [], + "createSql": "CREATE INDEX IF NOT EXISTS `index_messages_accountId_folder_uid` ON `${TABLE_NAME}` (`accountId`, `folder`, `uid`)" } ] }, @@ -654,7 +665,7 @@ ], "setupQueries": [ "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)", - "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'c064a4da054e0f98b4688ea2d04ecaec')" + "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, 'e4f7ef1e0d780324d6eec3efced768ae')" ] } } \ No newline at end of file diff --git a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt index 61ce90d..909467e 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/LibreMailDatabaseTest.kt @@ -169,8 +169,9 @@ class LibreMailDatabaseTest { assertEquals(setOf("acct:INBOX:1", "acct:INBOX:2"), messageDao.getSyncedIds("acct", "INBOX").toSet()) assertEquals(listOf("acct:Archive:1"), messageDao.getSyncedIds("acct", "Archive")) - // Reconciling the inbox must not touch other folders' rows. - messageDao.deleteSyncedNotIn("acct", "INBOX", listOf("acct:INBOX:1")) + // Reconciling the inbox must not touch other folders' rows (windowed reconcile; whole-inbox + // window since these rows have uid 0). + messageDao.deleteSyncedInWindowNotIn("acct", "INBOX", minWindowUid = 0, keepIds = listOf("acct:INBOX:1")) assertEquals( setOf("acct:INBOX:1", "acct:Archive:1"), messageDao.observeSummaries().first().map { it.id }.toSet(), 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 9e4e2e5..bb78a66 100644 --- a/app/src/androidTest/kotlin/org/libremail/data/local/MessageDaoRetentionTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/data/local/MessageDaoRetentionTest.kt @@ -68,28 +68,30 @@ class MessageDaoRetentionTest { ) /** - * The count-based prune boundary keeps the newest [keep] by recency and returns the REST for - * deletion, breaking timestamp ties by the higher UID. This pins the ordering *direction* (a - * flipped `DESC` would keep the OLDEST rows — i.e. locally delete the user's most recent mail) and - * the tie-break, neither of which the mocked-DAO unit tests can catch. + * The count-based prune boundary keeps the newest [keep] by ARRIVAL (server UID) and returns the + * REST for deletion. Keeping by UID — not by the Date header — matches the newest-by-UID recent + * window foreground sync re-fetches, so a high-UID/old-Date message isn't re-downloaded every sync + * and re-pruned every cycle. This pins the ordering column (a Date-ordered keep would evict the + * high-UID/old-Date row) and its direction (a flipped DESC would keep the OLDEST arrivals). */ @Test - fun syncedIdsBeyondCountInFolderKeepsNewestByTimestampThenUid() = runBlocking { + fun syncedIdsBeyondCountInFolderKeepsNewestByArrivalUid() = runBlocking { dao.insertNew( listOf( - message("A", uid = 30, timestampMillis = 300), // newest - message("B", uid = 25, timestampMillis = 200), // ties C on timestamp; higher uid => newer - message("C", uid = 20, timestampMillis = 200), - message("D", uid = 10, timestampMillis = 100), // oldest + message("recent-old-date", uid = 100, timestampMillis = 50), // newest arrival, oldest Date + message("A", uid = 30, timestampMillis = 300), + message("B", uid = 25, timestampMillis = 200), + message("D", uid = 10, timestampMillis = 100), // Scoping decoys: a search-only row and another folder must never enter the ranking. message("SR", uid = 99, timestampMillis = 999, inInbox = false), message("AR", uid = 5, timestampMillis = 50, folder = "Archive"), ), ) - // Keep the newest 2 (A, B); the rest are prunable. B is kept over C purely by the uid tie-break. + // Keep the newest 2 by UID (recent-old-date, A); the rest are prunable. A Date-ordered keep would + // wrongly evict recent-old-date (oldest Date) and keep B. assertEquals( - setOf("C", "D"), + setOf("B", "D"), dao.syncedIdsBeyondCountInFolder("acct", "INBOX", keep = 2).toSet(), ) // Keeping at least as many as exist prunes nothing. diff --git a/app/src/androidTest/kotlin/org/libremail/ui/Fakes.kt b/app/src/androidTest/kotlin/org/libremail/ui/Fakes.kt index c73df14..c4dade3 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/Fakes.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/Fakes.kt @@ -58,6 +58,8 @@ class FakeAccountRepository( override suspend fun deleteAccount(id: String) { accountsFlow.value = accountsFlow.value.filterNot { it.id == id } } + + override suspend fun resetBackfillProgress(accountId: String?) = Unit } /** diff --git a/app/src/main/kotlin/org/libremail/data/AttachmentCache.kt b/app/src/main/kotlin/org/libremail/data/AttachmentCache.kt new file mode 100644 index 0000000..2b88008 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/AttachmentCache.kt @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data + +import java.io.File + +/** + * The per-message on-disk attachment cache directory, keyed by a filesystem-safe form of the message + * id. Shared by the writer ([org.libremail.data.repository.MailRepositoryImpl]) and the retention + * pruner ([org.libremail.data.sync.MailPruner]) so the two can never disagree on where a message's + * attachments live — a divergence would silently leak orphaned files that the pruner no longer finds. + */ +internal fun attachmentCacheDir(cacheDir: File, messageId: String): File { + val safeId = messageId.replace(Regex("[^A-Za-z0-9._-]"), "_") + return File(cacheDir, "attachments/$safeId") +} diff --git a/app/src/main/kotlin/org/libremail/data/local/Migrations.kt b/app/src/main/kotlin/org/libremail/data/local/Migrations.kt index b8f5cc4..5a6a32b 100644 --- a/app/src/main/kotlin/org/libremail/data/local/Migrations.kt +++ b/app/src/main/kotlin/org/libremail/data/local/Migrations.kt @@ -259,5 +259,11 @@ val MIGRATION_12_13 = object : Migration(12, 13) { "`nextBeforeUid` INTEGER NOT NULL, `complete` INTEGER NOT NULL, " + "PRIMARY KEY(`accountId`, `folder`))", ) + + // Index the folder-scoped UID probes the backfill/reconcile hot paths run on every page/sync. + db.execSQL( + "CREATE INDEX IF NOT EXISTS `index_messages_accountId_folder_uid` " + + "ON `messages` (`accountId`, `folder`, `uid`)", + ) } } diff --git a/app/src/main/kotlin/org/libremail/data/local/dao/BackfillProgressDao.kt b/app/src/main/kotlin/org/libremail/data/local/dao/BackfillProgressDao.kt index b2c62f6..7e9bc1e 100644 --- a/app/src/main/kotlin/org/libremail/data/local/dao/BackfillProgressDao.kt +++ b/app/src/main/kotlin/org/libremail/data/local/dao/BackfillProgressDao.kt @@ -12,12 +12,13 @@ interface BackfillProgressDao { @Query("SELECT * FROM backfill_progress WHERE accountId = :accountId AND folder = :folder LIMIT 1") suspend fun get(accountId: String, folder: String): BackfillProgressEntity? - @Query("SELECT * FROM backfill_progress WHERE accountId = :accountId") - suspend fun getForAccount(accountId: String): List - @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun upsert(progress: BackfillProgressEntity) @Query("DELETE FROM backfill_progress WHERE accountId = :accountId") suspend fun deleteForAccount(accountId: String) + + /** Clears all backfill progress (e.g. when the global retention default changes) so it re-evaluates. */ + @Query("DELETE FROM backfill_progress") + suspend fun deleteAll() } 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 12fe62a..51e3704 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 @@ -41,6 +41,10 @@ interface MessageDao { @Insert(onConflict = OnConflictStrategy.IGNORE) suspend fun insertNew(messages: List) + /** Of the given [ids], those that already have a row — lets a caller refresh only pre-existing rows. */ + @Query("SELECT id FROM messages WHERE id IN (:ids)") + suspend fun existingIds(ids: List): List + /** * Refreshes the display fields (and the materialized [MessageEntity.uid], keeping it fresh for * rows migrated before the column existed) from the server without touching the cached body, the @@ -87,18 +91,12 @@ interface MessageDao { @Query("DELETE FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1") suspend fun deleteSyncedByAccountFolder(accountId: String, folder: String) - /** Drops synced rows in [folder] for an account that are no longer present on the server. */ - @Query( - "DELETE FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1 " + - "AND id NOT IN (:keepIds)", - ) - suspend fun deleteSyncedNotIn(accountId: String, folder: String, keepIds: List) - /** * Windowed deletion reconcile for full-history sync (issue #12): within [folder], delete synced * rows whose UID falls inside the freshly-fetched recent window (`uid >= minWindowUid`) but which * the server no longer returns ([keepIds]). Rows below the window — older history fetched by the - * background backfill — are deliberately left intact, unlike [deleteSyncedNotIn]. + * background backfill — are deliberately left intact, unlike a whole-folder "not in the recent + * set" reconcile, which would wipe that backfilled history. */ @Query( "DELETE FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1 " + @@ -130,14 +128,16 @@ interface MessageDao { suspend fun syncedIdsOlderThan(accountId: String, cutoffMillis: Long): List /** - * Ids of an account's synced rows in [folder] beyond the newest [keep] by recency (count-based - * prune candidates). Ties broken by UID so the boundary is deterministic. + * Ids of an account's synced rows in [folder] beyond the newest [keep] by ARRIVAL (server UID — + * count-based prune candidates). Ordering by UID (not by the Date header) matches the newest-by-UID + * recent window [org.libremail.mail.ImapClient.fetchRecent] keeps fresh, so a message with a high + * UID but an old Date isn't re-fetched by every sync and re-pruned by every cycle. */ @Query( "SELECT id FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1 " + "AND id NOT IN (" + "SELECT id FROM messages WHERE accountId = :accountId AND folder = :folder AND inInbox = 1 " + - "ORDER BY timestampMillis DESC, uid DESC LIMIT :keep)", + "ORDER BY uid DESC LIMIT :keep)", ) suspend fun syncedIdsBeyondCountInFolder(accountId: String, folder: String, keep: Int): List diff --git a/app/src/main/kotlin/org/libremail/data/local/entity/MessageEntity.kt b/app/src/main/kotlin/org/libremail/data/local/entity/MessageEntity.kt index 24170d7..0fb4b08 100644 --- a/app/src/main/kotlin/org/libremail/data/local/entity/MessageEntity.kt +++ b/app/src/main/kotlin/org/libremail/data/local/entity/MessageEntity.kt @@ -8,7 +8,10 @@ import androidx.room.PrimaryKey @Entity( tableName = "messages", - indices = [Index("accountId"), Index("timestampMillis")], + // The (accountId, folder, uid) index serves the folder-scoped UID probes the backfill/reconcile + // hot paths run on every page/sync: MIN(uid) (lowestSyncedUid) and the uid >= window bound + // (deleteSyncedInWindowNotIn / syncedIdsBeyondCountInFolder). + indices = [Index("accountId"), Index("timestampMillis"), Index("accountId", "folder", "uid")], ) data class MessageEntity( @PrimaryKey val id: String, diff --git a/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt b/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt index d123f78..f682538 100644 --- a/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt +++ b/app/src/main/kotlin/org/libremail/data/repository/AccountRepositoryImpl.kt @@ -79,4 +79,9 @@ class AccountRepositoryImpl @Inject constructor( folderDao.deleteForAccount(id) backfillProgressDao.deleteForAccount(id) } + + override suspend fun resetBackfillProgress(accountId: String?) { + if (accountId != null) backfillProgressDao.deleteForAccount(accountId) else backfillProgressDao.deleteAll() + syncScheduler.backfillNow() + } } 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 1f7d1ac..be3a569 100644 --- a/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt +++ b/app/src/main/kotlin/org/libremail/data/repository/MailRepositoryImpl.kt @@ -9,6 +9,7 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map import org.libremail.data.ReplyBuilder import org.libremail.data.SignatureBlock +import org.libremail.data.attachmentCacheDir import org.libremail.data.local.dao.AccountDao import org.libremail.data.local.dao.AttachmentDao import org.libremail.data.local.dao.DraftDao @@ -357,9 +358,8 @@ class MailRepositoryImpl @Inject constructor( * and avoids filename collisions between messages. */ private fun attachmentFile(messageId: String, partIndex: Int, filename: String): File { - val safeId = messageId.replace(Regex("[^A-Za-z0-9._-]"), "_") val safeName = filename.substringAfterLast('/').substringAfterLast('\\').ifBlank { "attachment" } - return File(context.cacheDir, "attachments/$safeId/$partIndex/$safeName") + return File(attachmentCacheDir(context.cacheDir, messageId), "$partIndex/$safeName") } } diff --git a/app/src/main/kotlin/org/libremail/data/settings/RetentionPolicy.kt b/app/src/main/kotlin/org/libremail/data/settings/RetentionPolicy.kt index ffe63ab..3e1e637 100644 --- a/app/src/main/kotlin/org/libremail/data/settings/RetentionPolicy.kt +++ b/app/src/main/kotlin/org/libremail/data/settings/RetentionPolicy.kt @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-3.0-or-later package org.libremail.data.settings +import kotlinx.coroutines.flow.first import java.time.Instant import java.time.ZoneOffset @@ -49,3 +50,25 @@ data class RetentionPolicy(val count: Int, val months: Int) { ) } } + +/** + * The effective retention policy for [accountId], resolving its per-account overrides against the + * global default. Read through the same [AccountSettingsRepository] / [SettingsRepository] every + * enforcement site uses — the foreground fetch window ([org.libremail.data.sync.MailSyncer]), the + * backfill floor ([org.libremail.data.sync.MailBackfiller]), and the pruner + * ([org.libremail.data.sync.MailPruner]) — so none of them can resolve a different floor, the + * divergence that would otherwise let backfill and prune fight over the same rows. + */ +internal suspend fun AccountSettingsRepository.effectiveRetention( + settings: SettingsRepository, + accountId: String, +): RetentionPolicy { + val account = get(accountId) + val global = settings.settings.first() + return RetentionPolicy.resolve( + accountCount = account.retentionCount, + accountMonths = account.retentionMonths, + defaultCount = global.retentionCount, + defaultMonths = global.retentionMonths, + ) +} diff --git a/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt b/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt index 68211d2..6d44e14 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/BackfillWorker.kt @@ -7,6 +7,7 @@ import androidx.work.CoroutineWorker import androidx.work.WorkerParameters import dagger.assisted.Assisted import dagger.assisted.AssistedInject +import kotlinx.coroutines.CancellationException /** * Runs one bounded slice of the full-history backfill (issue #12). Cancellable (WorkManager stops it @@ -21,8 +22,13 @@ class BackfillWorker @AssistedInject constructor( private val backfiller: MailBackfiller, ) : CoroutineWorker(appContext, workerParams) { - override suspend fun doWork(): Result = runCatching { backfiller.runBackfill() }.fold( + override suspend fun doWork(): Result = runCatching { + // Chain bounded slices back-to-back while history remains, so a large mailbox isn't limited to + // one slice per periodic run. runBackfill() returns true while any folder still has pages left; + // isStopped lets WorkManager end a long run gracefully (the periodic schedule resumes it). + while (backfiller.runBackfill() && !isStopped) { /* page the next slice */ } + }.fold( onSuccess = { Result.success() }, - onFailure = { Result.retry() }, + onFailure = { error -> if (error is CancellationException) throw error else Result.retry() }, ) } 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 b8653e2..2a200bd 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt @@ -2,14 +2,11 @@ package org.libremail.data.sync import android.content.Context -import android.net.ConnectivityManager -import android.net.NetworkCapabilities import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.delay import kotlinx.coroutines.ensureActive -import kotlinx.coroutines.flow.first import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import org.libremail.data.local.dao.AccountDao @@ -23,6 +20,7 @@ import org.libremail.data.settings.AccountSettingsRepository import org.libremail.data.settings.FetchPolicy import org.libremail.data.settings.RetentionPolicy import org.libremail.data.settings.SettingsRepository +import org.libremail.data.settings.effectiveRetention import org.libremail.domain.model.Account import org.libremail.domain.model.ImapConnectionParams import org.libremail.domain.repository.MailRepository @@ -67,7 +65,7 @@ class MailBackfiller @Inject constructor( var moreWork = false for (account in accountDao.getAll().map { it.toDomain() }) { val params = runCatching { connectionFactory.imapParamsFor(account) }.getOrNull() ?: continue - val policy = effectivePolicy(account.id) + val policy = accountSettingsRepository.effectiveRetention(settingsRepository, account.id) for (folder in messageDao.syncedFolders(account.id)) { if (remaining <= 0) return@withLock true // Per-folder failures (e.g. a transient server error) must not abort the whole slice. @@ -87,23 +85,29 @@ class MailBackfiller @Inject constructor( policy: RetentionPolicy, maxBatches: Int, ): FolderResult { - if (backfillProgressDao.get(account.id, folder)?.complete == true) { + val progress = backfillProgressDao.get(account.id, folder) + if (progress?.complete == true) { return FolderResult(batches = 0, complete = true) } - // Always page strictly below the LOWEST currently-cached UID. Deriving the boundary from the - // cache (rather than a stored cursor) keeps backfill gap-free even after the pruner raised the - // floor, and lets a later loosening of retention resume filling automatically. The mutex in - // runBackfill keeps the pruner from moving this boundary mid-run. - var beforeUid = messageDao.lowestSyncedUid(account.id, folder) ?: Long.MAX_VALUE + // 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 + ?: messageDao.lowestSyncedUid(account.id, folder) + ?: Long.MAX_VALUE var batches = 0 while (batches < maxBatches) { currentCoroutineContext().ensureActive() - // Retention floor (#13 precedence): pause — but do NOT mark complete — once the device-only - // limit is reached, so backfill and the pruner never contend for the same messages and a - // later loosening of the limit resumes paging from where it stopped. + // 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)) { - return FolderResult(batches, complete = false) + markComplete(account.id, folder, beforeUid) + return FolderResult(batches, complete = true) } val fetched = imapClient.fetchOlderThan(params, folder, beforeUid, BACKFILL_BATCH_SIZE) batches++ @@ -138,18 +142,25 @@ class MailBackfiller @Inject constructor( /** Inserts backfilled headers; never deletes. Uncancellable so a persisted boundary always has its rows. */ private suspend fun persistBatch(entities: List) = withContext(NonCancellable) { - messageDao.insertNew(entities) val ids = entities.map { it.id } - messageDao.markSynced(ids) - entities.forEach { - messageDao.updateHeaderContent( - id = it.id, - sender = it.sender, - senderEmail = it.senderEmail, - subject = it.subject, - timestampMillis = it.timestampMillis, - uid = it.uid, - ) + // insertNew (IGNORE) writes brand-new rows in full — headers, uid, and inInbox = 1 — so only rows + // that ALREADY existed (e.g. a former search-only row) need their membership/header refreshed. + // Limiting the updates to those avoids a redundant per-row UPDATE for every freshly-inserted row. + val preexisting = messageDao.existingIds(ids).toHashSet() + messageDao.insertNew(entities) + val toRefresh = entities.filter { it.id in preexisting } + if (toRefresh.isNotEmpty()) { + messageDao.markSynced(toRefresh.map { it.id }) + toRefresh.forEach { + messageDao.updateHeaderContent( + id = it.id, + sender = it.sender, + senderEmail = it.senderEmail, + subject = it.subject, + timestampMillis = it.timestampMillis, + uid = it.uid, + ) + } } } @@ -165,7 +176,7 @@ class MailBackfiller @Inject constructor( private suspend fun prefetchIfEnabled(ids: List) { val shouldPrefetch = when (settingsRepository.fetchPolicy()) { FetchPolicy.ALWAYS -> true - FetchPolicy.WIFI_ONLY -> isUnmetered() + FetchPolicy.WIFI_ONLY -> context.isActiveNetworkUnmetered() FetchPolicy.ON_DEMAND -> false } if (!shouldPrefetch) return @@ -175,23 +186,6 @@ class MailBackfiller @Inject constructor( } } - private suspend fun effectivePolicy(accountId: String): RetentionPolicy { - val account = accountSettingsRepository.get(accountId) - val global = settingsRepository.settings.first() - return RetentionPolicy.resolve( - accountCount = account.retentionCount, - accountMonths = account.retentionMonths, - defaultCount = global.retentionCount, - defaultMonths = global.retentionMonths, - ) - } - - private fun isUnmetered(): Boolean { - val manager = context.getSystemService(ConnectivityManager::class.java) ?: return false - val capabilities = manager.getNetworkCapabilities(manager.activeNetwork) ?: return false - return capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED) - } - private companion object { /** Headers fetched per server page. */ const val BACKFILL_BATCH_SIZE = 50 diff --git a/app/src/main/kotlin/org/libremail/data/sync/MailPruner.kt b/app/src/main/kotlin/org/libremail/data/sync/MailPruner.kt index 562fff6..b922dcf 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/MailPruner.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/MailPruner.kt @@ -5,14 +5,14 @@ import android.content.Context import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.ensureActive -import kotlinx.coroutines.flow.first import kotlinx.coroutines.sync.withLock +import org.libremail.data.attachmentCacheDir import org.libremail.data.local.dao.AccountDao import org.libremail.data.local.dao.MessageDao import org.libremail.data.settings.AccountSettingsRepository import org.libremail.data.settings.RetentionPolicy import org.libremail.data.settings.SettingsRepository -import java.io.File +import org.libremail.data.settings.effectiveRetention import javax.inject.Inject import javax.inject.Singleton @@ -37,17 +37,10 @@ class MailPruner @Inject constructor( ) { /** Prunes every account to its effective retention policy. Returns the number of messages removed. */ suspend fun prune(nowMillis: Long = System.currentTimeMillis()): Int = maintenanceGate.mutex.withLock { - val global = settingsRepository.settings.first() var removed = 0 for (account in accountDao.getAll()) { currentCoroutineContext().ensureActive() - val settings = accountSettingsRepository.get(account.id) - val policy = RetentionPolicy.resolve( - accountCount = settings.retentionCount, - accountMonths = settings.retentionMonths, - defaultCount = global.retentionCount, - defaultMonths = global.retentionMonths, - ) + val policy = accountSettingsRepository.effectiveRetention(settingsRepository, account.id) if (policy.isUnlimited) continue removed += pruneAccount(account.id, policy, nowMillis) } @@ -80,8 +73,7 @@ class MailPruner @Inject constructor( /** Removes the per-message on-disk attachment cache (keyed the same way MailRepositoryImpl writes it). */ private fun deleteCacheFiles(messageId: String) { - val safeId = messageId.replace(Regex("[^A-Za-z0-9._-]"), "_") - runCatching { File(context.cacheDir, "attachments/$safeId").deleteRecursively() } + runCatching { attachmentCacheDir(context.cacheDir, messageId).deleteRecursively() } } private companion object { diff --git a/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt b/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt index b6a80b2..425a996 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt @@ -2,13 +2,10 @@ package org.libremail.data.sync import android.content.Context -import android.net.ConnectivityManager -import android.net.NetworkCapabilities import dagger.hilt.android.qualifiers.ApplicationContext import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.ensureActive -import kotlinx.coroutines.flow.first import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext @@ -18,8 +15,8 @@ import org.libremail.data.local.toDomain import org.libremail.data.local.toEntity import org.libremail.data.settings.AccountSettingsRepository import org.libremail.data.settings.FetchPolicy -import org.libremail.data.settings.RetentionPolicy import org.libremail.data.settings.SettingsRepository +import org.libremail.data.settings.effectiveRetention import org.libremail.domain.model.Account import org.libremail.domain.repository.MailRepository import org.libremail.mail.ImapClient @@ -43,7 +40,7 @@ class MailSyncer @Inject constructor( // Serializes all syncing: syncAll/syncAccount/syncFolder are invoked concurrently by the periodic // worker, pull-to-refresh, one-shot syncs, folder opens, and one IDLE watcher per account. Without // this, two runs can both compute the same message as "new" (double-notify) or let a stale - // deleteSyncedNotIn snapshot delete a row another run just inserted. + // deleteSyncedInWindowNotIn snapshot delete a row another run just inserted. private val syncMutex = Mutex() /** Syncs every account's inbox. Succeeds if at least one account synced (or there are none). */ @@ -128,9 +125,14 @@ class MailSyncer @Inject constructor( } // Reconcile server-side deletions ONLY within the fetched recent-UID window, so older // history paged in by the background backfill (issue #12) survives each foreground sync - // instead of being wiped by a whole-folder "not in the recent 50" delete. - val minWindowUid = entities.minOf { it.uid } - messageDao.deleteSyncedInWindowNotIn(account.id, folder, minWindowUid, ids) + // instead of being wiped by a whole-folder "not in the recent 50" delete. Bound the + // window by the lowest POSITIVE fetched UID: a message whose UID couldn't be resolved + // (UIDFolder.getUID returns -1) must not collapse the bound to <= 0 and turn this into a + // whole-folder delete that wipes the backfilled history below the window. + val minWindowUid = entities.mapNotNull { entity -> entity.uid.takeIf { it > 0L } }.minOrNull() + if (minWindowUid != null) { + messageDao.deleteSyncedInWindowNotIn(account.id, folder, minWindowUid, ids) + } } val shouldNotify = notify && @@ -150,14 +152,7 @@ class MailSyncer @Inject constructor( * would immediately trim. Age-only or unlimited retention leaves the full window in place. */ private suspend fun recentWindowFor(account: Account): Int { - val accountSettings = accountSettingsRepository.get(account.id) - val global = settingsRepository.settings.first() - val policy = RetentionPolicy.resolve( - accountCount = accountSettings.retentionCount, - accountMonths = accountSettings.retentionMonths, - defaultCount = global.retentionCount, - defaultMonths = global.retentionMonths, - ) + val policy = accountSettingsRepository.effectiveRetention(settingsRepository, account.id) return policy.countLimit?.let { minOf(FETCH_LIMIT, it) } ?: FETCH_LIMIT } @@ -169,7 +164,7 @@ class MailSyncer @Inject constructor( private suspend fun prefetchIfEnabled(account: Account, folder: String) { val shouldPrefetch = when (settingsRepository.fetchPolicy()) { FetchPolicy.ALWAYS -> true - FetchPolicy.WIFI_ONLY -> isUnmetered() + FetchPolicy.WIFI_ONLY -> context.isActiveNetworkUnmetered() FetchPolicy.ON_DEMAND -> false } if (!shouldPrefetch) return @@ -179,13 +174,6 @@ class MailSyncer @Inject constructor( } } - /** True when the active network is unmetered (e.g. Wi-Fi), used by [FetchPolicy.WIFI_ONLY]. */ - private fun isUnmetered(): Boolean { - val manager = context.getSystemService(ConnectivityManager::class.java) ?: return false - val capabilities = manager.getNetworkCapabilities(manager.activeNetwork) ?: return false - return capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED) - } - private companion object { const val INBOX = "INBOX" diff --git a/app/src/main/kotlin/org/libremail/data/sync/NetworkStatus.kt b/app/src/main/kotlin/org/libremail/data/sync/NetworkStatus.kt new file mode 100644 index 0000000..49565bd --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/sync/NetworkStatus.kt @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import android.content.Context +import android.net.ConnectivityManager +import android.net.NetworkCapabilities + +/** + * True when the device's active network is unmetered (e.g. Wi-Fi). Shared by [MailSyncer] and + * [MailBackfiller] so both background jobs agree on what `Wi-Fi only` prefetch means; a divergent + * copy would let one job download on cellular while the other doesn't. + */ +internal fun Context.isActiveNetworkUnmetered(): Boolean { + val manager = getSystemService(ConnectivityManager::class.java) ?: return false + val capabilities = manager.getNetworkCapabilities(manager.activeNetwork) ?: return false + return capabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_NOT_METERED) +} diff --git a/app/src/main/kotlin/org/libremail/domain/repository/AccountRepository.kt b/app/src/main/kotlin/org/libremail/domain/repository/AccountRepository.kt index a571b2c..f25c0be 100644 --- a/app/src/main/kotlin/org/libremail/domain/repository/AccountRepository.kt +++ b/app/src/main/kotlin/org/libremail/domain/repository/AccountRepository.kt @@ -19,4 +19,11 @@ interface AccountRepository { suspend fun addOutlookAccount(email: String, accessToken: String, authStateJson: String): Result> suspend fun deleteAccount(id: String) + + /** + * Discards full-history backfill progress so it re-evaluates against the current retention floor + * after a retention change: tightening re-hits the (tighter) floor cheaply, loosening resumes + * paging older history. [accountId] null clears every account (a global-default change). + */ + suspend fun resetBackfillProgress(accountId: String?) } diff --git a/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsViewModel.kt b/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsViewModel.kt index f999bcd..dabf463 100644 --- a/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/settings/AccountSettingsViewModel.kt @@ -66,6 +66,7 @@ class AccountSettingsViewModel @Inject constructor( viewModelScope.launch { accountSettingsRepository.setRetentionCount(accountId, value) syncScheduler.pruneNow() + accountRepository.resetBackfillProgress(accountId) } } @@ -73,6 +74,7 @@ class AccountSettingsViewModel @Inject constructor( viewModelScope.launch { accountSettingsRepository.setRetentionMonths(accountId, value) syncScheduler.pruneNow() + accountRepository.resetBackfillProgress(accountId) } } diff --git a/app/src/main/kotlin/org/libremail/ui/settings/SettingsViewModel.kt b/app/src/main/kotlin/org/libremail/ui/settings/SettingsViewModel.kt index 95094b7..1356f8f 100644 --- a/app/src/main/kotlin/org/libremail/ui/settings/SettingsViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/settings/SettingsViewModel.kt @@ -66,11 +66,13 @@ class SettingsViewModel @Inject constructor( fun setRetentionCount(value: Int) = update { settingsRepository.setRetentionCount(value) syncScheduler.pruneNow() + accountRepository.resetBackfillProgress(null) } fun setRetentionMonths(value: Int) = update { settingsRepository.setRetentionMonths(value) syncScheduler.pruneNow() + accountRepository.resetBackfillProgress(null) } private inline fun update(crossinline action: suspend () -> Unit) { 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 a50feaf..d325e9d 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt @@ -35,6 +35,7 @@ import org.libremail.domain.model.ImapConnectionParams import org.libremail.domain.model.MailSecurity import org.libremail.domain.repository.MailRepository import org.libremail.mail.ImapClient +import java.util.Date import java.util.Properties import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -142,14 +143,52 @@ class MailBackfillerTest { assertTrue(afterFirst >= 60, "must fetch at least up to the retention floor") assertTrue(afterFirst < TOTAL, "must NOT page the entire 120-message history") - // Paused at the floor (NOT marked complete, so a later loosening could resume), and stable: - // running again fetches nothing more. - assertFalse(progress["acct" to "INBOX"]!!.complete) + // Marked complete at the floor so the pruner deleting aged-out rows can't re-open paging (a + // retention change resets progress to resume); stable — running again fetches nothing more. + assertTrue(progress["acct" to "INBOX"]!!.complete) backfiller.runBackfill() assertEquals(afterFirst, cached.size, "at the floor, further runs must not fetch more") assertNoDeletes() } + /** + * Regression for the #12/#13 AGE-retention contention: reaching the age floor marks the folder + * complete, so the pruner deleting aged-out rows — which raises the oldest cached timestamp back + * above the cutoff — can't re-open paging. Before the fix, the next run re-fetched exactly the rows + * the pruner had just deleted, an endless re-download/re-prune loop. (The count floor was already a + * fixpoint, so only an age-based case exercises this.) + */ + @Test + fun `reaching the age floor is sticky across a prune, so backfill never re-fetches`() = runTest { + val now = System.currentTimeMillis() + // UID order follows append order: the first 60 are ~8 months old (beyond the 6-month cutoff), + // the last 60 are recent (within retention). + val old = (1..60).map { now - 8 * MONTH_MILLIS - it * DAY_MILLIS } + val recent = (1..60).map { now - it * DAY_MILLIS } + appendMessages(old + recent) + seedForegroundWindow() + + val backfiller = backfiller(AccountSettings("acct", retentionMonths = 6)) + backfiller.runBackfill() + + assertEquals( + true, + progress["acct" to "INBOX"]!!.complete, + "backfill marks the folder complete at the age floor", + ) + val offeredBeforePrune = totalOffered + + // Simulate the pruner: drop every cached row older than the 6-month cutoff. This raises the + // oldest cached timestamp back above the cutoff — the state that used to re-open paging. + val cutoff = now - 6 * MONTH_MILLIS + cached.removeAll { it.timestampMillis < cutoff } + + backfiller.runBackfill() + + assertEquals(offeredBeforePrune, totalOffered, "a floored folder must not re-fetch after a prune") + assertNoDeletes() + } + /** Builds a backfiller wired to GreenMail with the in-memory fakes and the given account settings. */ private fun backfiller(accountSettings: AccountSettings): MailBackfiller { val accountDao = mockk() @@ -222,12 +261,17 @@ class MailBackfillerTest { private fun assertNoDeletes() { val dao = lastMessageDao ?: return coVerify(exactly = 0) { dao.deleteByIds(any()) } - coVerify(exactly = 0) { dao.deleteSyncedNotIn(any(), any(), any()) } coVerify(exactly = 0) { dao.deleteSyncedInWindowNotIn(any(), any(), any(), any()) } coVerify(exactly = 0) { dao.deleteSyncedByAccountFolder(any(), any()) } } - private fun appendMessages(count: Int) { + private fun appendMessages(count: Int) = appendMessages(List(count) { null }) + + /** + * Appends messages to INBOX with the given per-message sent dates (null = server default, ~now). + * UID order follows list order, so earlier entries get lower UIDs. + */ + private fun appendMessages(sentDates: List) { val props = Properties().apply { put("mail.store.protocol", "imap") put("mail.imap.host", "127.0.0.1") @@ -239,12 +283,13 @@ class MailBackfillerTest { try { val inbox = store.getFolder("INBOX") inbox.open(Folder.READ_WRITE) - val messages = (1..count).map { i -> + val messages = sentDates.mapIndexed { i, millis -> MimeMessage(session).apply { setFrom(InternetAddress("sender$i@example.org")) setRecipient(Message.RecipientType.TO, InternetAddress("alice@example.org")) subject = "Message $i" setText("Body of message $i") + if (millis != null) sentDate = Date(millis) } }.toTypedArray() inbox.appendMessages(messages) @@ -257,5 +302,7 @@ class MailBackfillerTest { private companion object { const val TOTAL = 120 const val WINDOW = 50 + private const val DAY_MILLIS = 24L * 60 * 60 * 1000 + private const val MONTH_MILLIS = 30L * DAY_MILLIS } }