JMR-dev/LibreMail
1
0
Fork 0
Code Issues 40 Pull Requests 2 Actions Packages Projects Releases 2 Wiki Activity

Merge main into fix-306-outlook-redundant-token

Browse Source
This commit is contained in:
Jason Ross
2026-07-04 03:21:29 -05:00
committed by GitHub
parent 97b782ecb2 5125a6674c
commit 8cb57cba50
6 changed files with 176 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/src/androidTest/kotlin/org/libremail/data/local/MessageDaoTest.kt
+88
View File
@@ -166,6 +166,41 @@ class MessageDaoTest {
)
}
@Test
fun browsePagingBreaksTimestampTiesByAscendingIdForATotalPageOrder() = runBlocking {
// Bulk mail can share a timestamp (issue #311): without a unique tiebreaker, rows tied at a
// LIMIT/OFFSET page boundary can duplicate or skip. Insertion order is scrambled so the ORDER BY
// — not the storage order — must produce the result.
dao.insertNew(
listOf(
message("tie-c", timestampMillis = 1_000),
message("tie-a", timestampMillis = 1_000),
message("tie-b", timestampMillis = 1_000),
message("newer", timestampMillis = 2_000),
),
)
// Newest timestamp first, then ties broken by ascending id — a deterministic total order.
val expected = listOf("newer", "tie-a", "tie-b", "tie-c")
assertEquals(expected, dao.pagingUnifiedFolderSummaries("INBOX").refreshIds())
assertEquals(expected, dao.pagingFolderSummaries("acct", "INBOX").refreshIds())
}
@Test
fun searchPagingBreaksTimestampTiesByAscendingIdForATotalPageOrder() = runBlocking {
dao.insertNew(
listOf(
message("hit-c", subject = "report", timestampMillis = 1_000),
message("hit-a", subject = "report", timestampMillis = 1_000),
message("hit-b", subject = "report", timestampMillis = 1_000),
),
)
val expected = listOf("hit-a", "hit-b", "hit-c")
assertEquals(expected, dao.pagingUnifiedFolderSearchSummaries("INBOX", "%report%").refreshIds())
assertEquals(expected, dao.pagingFolderSearchSummaries("acct", "INBOX", "%report%").refreshIds())
}
@Test
fun getUnfetchedIdsReturnsOnlySyncedRowsMissingABody() = runBlocking {
dao.insertNew(
@@ -232,6 +267,59 @@ class MessageDaoTest {
assertEquals(true, row.inInbox)
}
@Test
fun updateHeaderContentsRefreshesEveryRowInTheBatchAndLeavesFlagsAndBodiesUntouched() = runBlocking {
dao.insertNew(
listOf(
message("acct:1", isRead = true, isStarred = false, body = "cached-1", uid = 0),
message("acct:2", isRead = false, isStarred = true, body = "cached-2", uid = 0),
),
)
// The sync path (issue #310) refreshes a whole recent window at once via this single-transaction
// batch. Only the six header fields of each passed entity are applied; the rest are ignored.
dao.updateHeaderContents(
listOf(
message(
"acct:1",
sender = "Charles",
senderEmail = "charles@example.org",
subject = "One",
timestampMillis = 5_000L,
uid = 42L,
),
message(
"acct:2",
sender = "Grace",
senderEmail = "grace@example.org",
subject = "Two",
timestampMillis = 6_000L,
uid = 43L,
),
),
)
val one = requireNotNull(dao.getById("acct:1"))
assertEquals("Charles", one.sender)
assertEquals("charles@example.org", one.senderEmail)
assertEquals("One", one.subject)
assertEquals(5_000L, one.timestampMillis)
assertEquals(42L, one.uid)
// The casefold search columns track the refreshed headers (issue #232).
assertEquals("charles", one.senderFold)
assertEquals("one", one.subjectFold)
// Flags and the cached body are deliberately left untouched.
assertTrue(one.isRead)
assertEquals("cached-1", one.body)
val two = requireNotNull(dao.getById("acct:2"))
assertEquals("Grace", two.sender)
assertEquals("Two", two.subject)
assertEquals(43L, two.uid)
assertTrue(two.isStarred)
assertEquals("cached-2", two.body)
}
@Test
fun markSyncedPromotesSearchOnlyRowsIntoTheFolder() = runBlocking {
dao.insertNew(
app/src/androidTest/kotlin/org/libremail/data/local/MigrationTest.kt
+18
View File
@@ -13,6 +13,7 @@ import org.junit.Assert.assertTrue
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.libremail.di.DatabaseModule
import java.lang.reflect.Modifier
/**
@@ -306,6 +307,23 @@ class MigrationTest {
)
}
/**
* The replay tests discover migrations reflectively, but nothing there checks that [DatabaseModule]
* actually *registers* them. With no destructive fallback, a migration authored, schema-committed,
* and replay-tested but omitted from `addMigrations` still crash-loops every upgrading user at first
* database open (issue #312). Assert the builder's registered set ([DatabaseModule.ALL_MIGRATIONS])
* is exactly the reflectively-discovered set, so such an omission fails here instead.
*/
@Test
fun databaseModuleRegistersEveryDeclaredMigration() {
assertEquals(
"DatabaseModule.ALL_MIGRATIONS must register every Migration in Migrations.kt " +
"(no destructive fallback, so a forgotten step crash-loops upgrades)",
allAppMigrations.map { it.startVersion to it.endVersion },
DatabaseModule.ALL_MIGRATIONS.sortedBy { it.startVersion }.map { it.startVersion to it.endVersion },
)
}
/**
* Creates a database at v7 (the oldest exported schema), fills it like a used install, then
* replays every migration one step at a time — `runMigrationsAndValidate` diffs the migrated
app/src/main/kotlin/org/libremail/data/local/dao/MessageDao.kt
+31 -4
View File
@@ -6,6 +6,7 @@ import androidx.room.Dao
import androidx.room.Insert
import androidx.room.OnConflictStrategy
import androidx.room.Query
import androidx.room.Transaction
import kotlinx.coroutines.flow.Flow
import org.libremail.data.local.entity.FolderUnreadCount
import org.libremail.data.local.entity.MessageEntity
@@ -36,11 +37,14 @@ interface MessageDao {
* by [pagingUnifiedFolderSearchSummaries] (issue #214). Profiling (see
* `docs/perf/issue-124-unified-inbox-paging.md`) showed the first page loads flat regardless of
* total cache size on the existing indices, so no `(folder, …)` index / schema migration is added.
* Breaks ties on the `id` primary key (issue #311): `timestampMillis` isn't unique — bulk mail
* shares a second — so without a unique tiebreaker two rows tied at a LIMIT/OFFSET page boundary
* could duplicate or skip across pages. `id` is in the projection, so the tiebreaker adds no index.
*/
@Query(
"SELECT id, accountId, sender, senderEmail, subject, snippet, timestampMillis, " +
"isRead, isStarred, folder, inInbox, bodyFetched FROM messages " +
"WHERE folder = :folder AND inInbox = 1 ORDER BY timestampMillis DESC",
"WHERE folder = :folder AND inInbox = 1 ORDER BY timestampMillis DESC, id",
)
fun pagingUnifiedFolderSummaries(folder: String): PagingSource<Int, MessageSummary>
@@ -56,7 +60,7 @@ interface MessageDao {
@Query(
"SELECT id, accountId, sender, senderEmail, subject, snippet, timestampMillis, " +
"isRead, isStarred, folder, inInbox, bodyFetched FROM messages " +
"WHERE accountId = :accountId AND folder = :folder AND inInbox = 1 ORDER BY timestampMillis DESC",
"WHERE accountId = :accountId AND folder = :folder AND inInbox = 1 ORDER BY timestampMillis DESC, id",
)
fun pagingFolderSummaries(accountId: String, folder: String): PagingSource<Int, MessageSummary>
@@ -70,13 +74,14 @@ interface MessageDao {
* server-search hits `MailRepository.searchServer` inserts — exactly what the old filter saw.
* Matches the Unicode-casefolded `*Fold` columns (issue #232) with a pattern built from the
* lowercased query, so search is case-insensitive beyond ASCII — unlike the old ASCII-only `LIKE`.
* Breaks ties on the `id` primary key for a total page order, like the browse pagers (issue #311).
*/
@Query(
"SELECT id, accountId, sender, senderEmail, subject, snippet, timestampMillis, " +
"isRead, isStarred, folder, inInbox, bodyFetched FROM messages " +
"WHERE folder = :folder AND (senderFold LIKE :pattern ESCAPE '\\' OR " +
"senderEmailFold LIKE :pattern ESCAPE '\\' OR subjectFold LIKE :pattern ESCAPE '\\' OR " +
"snippetFold LIKE :pattern ESCAPE '\\') ORDER BY timestampMillis DESC",
"snippetFold LIKE :pattern ESCAPE '\\') ORDER BY timestampMillis DESC, id",
)
fun pagingUnifiedFolderSearchSummaries(folder: String, pattern: String): PagingSource<Int, MessageSummary>
@@ -91,7 +96,7 @@ interface MessageDao {
"isRead, isStarred, folder, inInbox, bodyFetched FROM messages " +
"WHERE accountId = :accountId AND folder = :folder AND (senderFold LIKE :pattern ESCAPE '\\' OR " +
"senderEmailFold LIKE :pattern ESCAPE '\\' OR subjectFold LIKE :pattern ESCAPE '\\' OR " +
"snippetFold LIKE :pattern ESCAPE '\\') ORDER BY timestampMillis DESC",
"snippetFold LIKE :pattern ESCAPE '\\') ORDER BY timestampMillis DESC, id",
)
fun pagingFolderSearchSummaries(
accountId: String,
@@ -190,6 +195,28 @@ interface MessageDao {
uid: Long,
)
/**
* Applies [updateHeaderContent] to every [messages] row in a single transaction (issue #310). A
* foreground sync / pull-to-refresh refreshes a whole recent window at once; running each row's
* UPDATE in its own implicit transaction fsyncs the journal once per message — N commits per folder
* per sync, amplified on the encrypted cache — so this collapses them into one commit. Refreshes the
* same display fields (plus the materialized [MessageEntity.uid] and the casefold search columns) as
* the single-row overload, still leaving cached bodies and optimistic read/star flags untouched.
*/
@Transaction
suspend fun updateHeaderContents(messages: List<MessageEntity>) {
for (message in messages) {
updateHeaderContent(
id = message.id,
sender = message.sender,
senderEmail = message.senderEmail,
subject = message.subject,
timestampMillis = message.timestampMillis,
uid = message.uid,
)
}
}
/** Marks rows as folder-synced (e.g. a former search-only row that the sync now returns). */
@Query("UPDATE messages SET inInbox = 1 WHERE id IN (:ids)")
suspend fun markSynced(ids: List<String>)
app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt
+4 -11
View File
@@ -122,18 +122,11 @@ class MailSyncer @Inject constructor(
val ids = entities.map { it.id }
messageDao.insertNew(entities)
// Mark every fetched message as synced (upgrades any former search-only row) and refresh
// its display fields — without touching cached bodies or optimistic read/star flags.
// its display fields — without touching cached bodies or optimistic read/star flags. The
// per-row refreshes run in a single transaction (issue #310) so a whole recent window
// costs one commit instead of one fsync per message (amplified on the encrypted cache).
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,
)
}
messageDao.updateHeaderContents(entities)
// 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. Bound the
app/src/main/kotlin/org/libremail/di/DatabaseModule.kt
+30 -20
View File
@@ -3,6 +3,7 @@ package org.libremail.di
import android.content.Context
import androidx.room.Room
import androidx.room.migration.Migration
import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory
import dagger.Module
import dagger.Provides
@@ -46,30 +47,39 @@ import javax.inject.Singleton
@InstallIn(SingletonComponent::class)
object DatabaseModule {
/**
* Every migration the Room builder registers, in one named list so [provideDatabase] and the
* "registered == declared" safety-net test (issue #312) read the same source. There is deliberately
* no destructive fallback (see below), so a migration authored + schema-committed but forgotten here
* passes every replay test yet crash-loops ALL upgrading users at first DB open; `MigrationTest`
* asserts this list equals the reflectively-discovered set of every `Migration` val in Migrations.kt.
*/
val ALL_MIGRATIONS: Array<Migration> = arrayOf(
MIGRATION_1_2,
MIGRATION_2_3,
MIGRATION_3_4,
MIGRATION_4_5,
MIGRATION_5_6,
MIGRATION_6_7,
MIGRATION_7_8,
MIGRATION_8_9,
MIGRATION_9_10,
MIGRATION_10_11,
MIGRATION_11_12,
MIGRATION_12_13,
MIGRATION_13_14,
MIGRATION_14_15,
MIGRATION_15_16,
MIGRATION_16_17,
MIGRATION_17_18,
MIGRATION_18_19,
)
@Provides
@Singleton
fun provideDatabase(@ApplicationContext context: Context, provisioner: DatabaseProvisioner): LibreMailDatabase =
Room.databaseBuilder(context, LibreMailDatabase::class.java, DB_NAME)
.addMigrations(
MIGRATION_1_2,
MIGRATION_2_3,
MIGRATION_3_4,
MIGRATION_4_5,
MIGRATION_5_6,
MIGRATION_6_7,
MIGRATION_7_8,
MIGRATION_8_9,
MIGRATION_9_10,
MIGRATION_10_11,
MIGRATION_11_12,
MIGRATION_12_13,
MIGRATION_13_14,
MIGRATION_14_15,
MIGRATION_15_16,
MIGRATION_16_17,
MIGRATION_17_18,
MIGRATION_18_19,
)
.addMigrations(*ALL_MIGRATIONS)
// No destructive fallback: the migration chain is complete, and silently dropping the
// mail/message tables would lose cached data. A missing migration should fail loudly in
// testing instead.
app/src/test/kotlin/org/libremail/data/sync/MailSyncConcurrencyTest.kt
+5
View File
@@ -410,6 +410,11 @@ class MailSyncConcurrencyTest {
coEvery { dao.updateHeaderContent(any(), any(), any(), any(), any(), any()) } answers {
store.updateHeaderContent(firstArg(), secondArg(), thirdArg(), arg(3), arg(4), arg(5))
}
coEvery { dao.updateHeaderContents(any()) } answers {
firstArg<List<MessageEntity>>().forEach {
store.updateHeaderContent(it.id, it.sender, it.senderEmail, it.subject, it.timestampMillis, it.uid)
}
}
coEvery { dao.deleteByIds(any()) } answers { store.deleteByIds(firstArg()) }
coEvery { dao.deleteSyncedByAccountFolder(any(), any()) } answers {
store.deleteSyncedByAccountFolder(firstArg(), secondArg())