perf/fix(data): batch sync updates, paging tiebreaker, migration-registration test
#310: add a single-transaction MessageDao.updateHeaderContents(List<MessageEntity>) and route MailSyncer's per-message updateHeaderContent loop through it, so a folder's recent-window refresh commits once instead of once per message (fsync/journal write per message, amplified on the encrypted cache). #311: append the `id` primary key as a tiebreaker to the four paged MessageDao `ORDER BY timestampMillis DESC` queries for a total order, so rows sharing a second (bulk mail) can't duplicate or skip across a LIMIT/OFFSET page boundary. Pure query-text change: the exported Room schema (identityHash) is derived from table/index structure, not @Query SQL, so no schema re-export or version bump; id is already in the projection, so no new index. #312: expose the registered migration list as DatabaseModule.ALL_MIGRATIONS (spread into addMigrations) and assert in MigrationTest that it equals the reflectively-discovered set of every Migration val, so a migration forgotten in addMigrations fails a test instead of crash-looping all upgrading users at DB open (there is deliberately no destructive fallback). Tests: new MessageDaoTest cases for the batch update and the id tiebreaker (all four pagers), and the MigrationTest registration assertion; wired updateHeaderContents into MailSyncConcurrencyTest's fake DAO. Instrumented MessageDaoTest + MigrationTest (31 tests) green on a local emulator; unit tests + androidTest compile + ktlint + detekt green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -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(
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user