WIP: merge queue: checking main (6802b60) and [#439 + #435 + #438] together #441

Closed
mergify[bot] wants to merge 5 commits from mergify/merge-queue/8ece8824a2 into main
31 changed files with 566 additions and 133 deletions
@@ -88,4 +88,29 @@ class AccountSettingsDaoTest {
assertEquals(500, stored?.retentionCount)
assertEquals(6, stored?.retentionMonths)
}
@Test
fun readModifyWriteAppliesTheTransformToTheStoredRow() = runBlocking {
insertAccount()
dao.upsert(AccountSettingsEntity("acct", signature = "old", notificationsEnabled = false))
// The read + transform + write run in one transaction (issue #313); the transform gets the stored
// row and changes one field, so the un-touched fields are carried forward.
dao.readModifyWrite("acct") { stored -> stored!!.copy(signature = "new") }
val result = dao.get("acct")
assertEquals("new", result?.signature)
assertEquals(false, result?.notificationsEnabled)
}
@Test
fun readModifyWriteTransformsANullRowForAnUnconfiguredAccount() = runBlocking {
insertAccount()
// No settings row yet: the transform receives null and builds the first row.
dao.readModifyWrite("acct") { stored ->
stored?.copy(signature = "x") ?: AccountSettingsEntity("acct", signature = "seeded")
}
assertEquals("seeded", dao.get("acct")?.signature)
}
}
@@ -5,7 +5,6 @@ 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 net.zetetic.database.sqlcipher.SQLiteDatabase
import net.zetetic.database.sqlcipher.SupportOpenHelperFactory
@@ -56,7 +55,7 @@ class DatabaseEncryptionTest {
DatabaseEncryption.ensureEncrypted(dbFile, passphrase)
assertTrue("file must not read as plaintext once encrypted", DatabaseEncryption.isEncrypted(dbFile))
openEncrypted().apply {
assertEquals(listOf("acct:1"), messageDao().observeSummaries().first().map { it.id })
assertEquals("acct:1", messageDao().getById("acct:1")?.id)
close()
}
@@ -64,7 +63,7 @@ class DatabaseEncryptionTest {
DatabaseEncryption.ensurePlaintext(dbFile, passphrase)
assertFalse("file must be plaintext again after decrypt", DatabaseEncryption.isEncrypted(dbFile))
openPlaintext().apply {
assertEquals(listOf("acct:1"), messageDao().observeSummaries().first().map { it.id })
assertEquals("acct:1", messageDao().getById("acct:1")?.id)
close()
}
}
@@ -105,7 +104,7 @@ class DatabaseEncryptionTest {
DatabaseEncryption.ensureEncrypted(dbFile, passphrase)
assertTrue("the file stays encrypted", DatabaseEncryption.isEncrypted(dbFile))
openEncrypted().apply {
assertEquals(listOf("acct:1"), messageDao().observeSummaries().first().map { it.id })
assertEquals("acct:1", messageDao().getById("acct:1")?.id)
close()
}
}
@@ -122,7 +121,29 @@ class DatabaseEncryptionTest {
assertFalse(DatabaseEncryption.isEncrypted(dbFile))
openPlaintext().apply {
assertEquals(listOf("acct:1"), messageDao().observeSummaries().first().map { it.id })
assertEquals("acct:1", messageDao().getById("acct:1")?.id)
close()
}
}
@Test
fun conversionSweepsAStaleRollbackJournalSidecar() = runBlocking<Unit> {
openPlaintext().apply {
messageDao().insertNew(listOf(message("acct:1")))
close()
}
// A stray `-journal` left next to the file by an interrupted rollback-journal-mode session. The
// conversion runs in journal_mode = DELETE, so `-journal` is the sidecar that can actually linger
// (the pre-existing sweep only removed `-wal`/`-shm`) — issue #313.
val staleJournal = File(dbFile.parentFile, "$dbName-journal")
staleJournal.outputStream().use { it.write(0) }
assertTrue("precondition: a stale journal exists", staleJournal.exists())
DatabaseEncryption.ensureEncrypted(dbFile, passphrase)
assertFalse("the conversion must sweep the stale -journal sidecar", staleJournal.exists())
openEncrypted().apply {
assertEquals("the data still round-trips", "acct:1", messageDao().getById("acct:1")?.id)
close()
}
}
@@ -16,7 +16,6 @@ import io.mockk.unmockkAll
import io.mockk.unmockkObject
import io.mockk.verify
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.runBlocking
import net.zetetic.database.sqlcipher.SupportOpenHelperFactory
@@ -137,7 +136,7 @@ class DatabaseProvisionerInstrumentedTest {
// The keyed open the provisioner reported must actually succeed on real SQLCipher (no crash).
openEncrypted().apply {
assertEquals(listOf("acct:1"), messageDao().observeSummaries().first().map { it.id })
assertEquals("acct:1", messageDao().getById("acct:1")?.id)
close()
}
}
@@ -164,7 +163,7 @@ class DatabaseProvisionerInstrumentedTest {
// whatever ABI / page size this device or emulator image uses.
openEncrypted().apply {
messageDao().insertNew(listOf(message("acct:1")))
assertEquals(listOf("acct:1"), messageDao().observeSummaries().first().map { it.id })
assertEquals("acct:1", messageDao().getById("acct:1")?.id)
close()
}
assertTrue("the fresh cache was created in SQLCipher (encrypted) form", DatabaseEncryption.isEncrypted(dbFile))
@@ -182,7 +181,7 @@ class DatabaseProvisionerInstrumentedTest {
assertEquals(CacheOpenMode.Plaintext, mode)
assertFalse("the cache must be decrypted so the unkeyed open works", DatabaseEncryption.isEncrypted(dbFile))
openPlaintext().apply {
assertEquals(listOf("acct:1"), messageDao().observeSummaries().first().map { it.id })
assertEquals("acct:1", messageDao().getById("acct:1")?.id)
close()
}
}
@@ -198,7 +197,7 @@ class DatabaseProvisionerInstrumentedTest {
assertEquals(CacheOpenMode.Plaintext, mode)
assertFalse("a plaintext-with-encryption-off start converts nothing", DatabaseEncryption.isEncrypted(dbFile))
openPlaintext().apply {
assertEquals(listOf("acct:1"), messageDao().observeSummaries().first().map { it.id })
assertEquals("acct:1", messageDao().getById("acct:1")?.id)
close()
}
}
@@ -2,6 +2,7 @@
package org.libremail.data.local
import android.content.Context
import androidx.paging.PagingSource
import androidx.room.Room
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
@@ -9,6 +10,7 @@ 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.Assert.assertTrue
import org.junit.Before
import org.junit.Test
@@ -16,6 +18,7 @@ import org.junit.runner.RunWith
import org.libremail.data.local.entity.AttachmentEntity
import org.libremail.data.local.entity.FolderEntity
import org.libremail.data.local.entity.MessageEntity
import org.libremail.data.local.entity.MessageSummary
/**
* Schema-behavior tests on a fresh in-memory database at the current version. The migration DDL
@@ -52,6 +55,12 @@ class LibreMailDatabaseTest {
isStarred = false,
)
/** Refreshes a [PagingSource] and returns the first loaded page's ids in order. */
private suspend fun PagingSource<Int, MessageSummary>.refreshIds(loadSize: Int = 20): List<String> {
val result = load(PagingSource.LoadParams.Refresh(key = null, loadSize = loadSize, placeholdersEnabled = false))
return (result as PagingSource.LoadResult.Page).data.map { it.id }
}
@Test
fun observeUnreadCountsAggregatesUnreadSyncedRowsPerAccountAndFolder() = runBlocking {
val messageDao = db.messageDao()
@@ -124,17 +133,17 @@ class LibreMailDatabaseTest {
assertEquals(listOf("acct:1"), messageDao.getSyncedIds("acct", "INBOX"))
messageDao.deleteSearchRows()
val remaining = messageDao.observeSummaries().first().map { it.id }
assertEquals(listOf("acct:1"), remaining)
assertEquals("the synced inbox row survives", "acct:1", messageDao.getById("acct:1")?.id)
assertNull("the transient search-only row is cleared", messageDao.getById("acct:2"))
}
@Test
fun observeSummariesReadsRowsWhoseBodiesExceedTheCursorWindow() = runBlocking {
fun pagedSummariesReadRowsWhoseBodiesExceedTheCursorWindow() = runBlocking {
val messageDao = db.messageDao()
// Each body is larger than SQLite's shared (~2 MB) CursorWindow. The old list query did
// SELECT * and dragged these bodies through the window, overflowing it with
// "Couldn't read row … from CursorWindow" (issue #51). observeSummaries omits body, so the
// rows stay tiny and read fine.
// Each body is larger than SQLite's shared (~2 MB) CursorWindow. A `SELECT *` list query
// dragged these bodies through the window, overflowing it with "Couldn't read row … from
// CursorWindow" (issue #51). The paged mailbox projection omits body, so the rows stay tiny
// and read fine — asserted against the real production query (issue #124/#214).
val hugeBody = "x".repeat(3 * 1024 * 1024)
messageDao.insertNew(
listOf(
@@ -143,7 +152,7 @@ class LibreMailDatabaseTest {
),
)
val ids = messageDao.observeSummaries().first().map { it.id }.toSet()
val ids = messageDao.pagingUnifiedFolderSummaries("INBOX").refreshIds().toSet()
assertEquals(setOf("acct:1", "acct:2"), ids)
}
@@ -194,9 +203,8 @@ class LibreMailDatabaseTest {
// 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(),
)
assertEquals("the kept inbox row survives", "acct:INBOX:1", messageDao.getById("acct:INBOX:1")?.id)
assertNull("the reconciled-away inbox row is deleted", messageDao.getById("acct:INBOX:2"))
assertEquals("the other folder is untouched", "acct:Archive:1", messageDao.getById("acct:Archive:1")?.id)
}
}
@@ -6,7 +6,6 @@ import androidx.paging.PagingSource
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
@@ -422,7 +421,9 @@ class MessageDaoTest {
dao.deleteByIds(listOf("a", "c"))
assertEquals(listOf("b"), dao.observeSummaries().first().map { it.id })
assertNull("a is deleted", dao.getById("a"))
assertNull("c is deleted", dao.getById("c"))
assertEquals("b survives", "b", dao.getById("b")?.id)
}
@Test
@@ -437,7 +438,9 @@ class MessageDaoTest {
dao.deleteByAccount("acct")
assertEquals(listOf("acct2:1"), dao.observeSummaries().first().map { it.id })
assertNull("the account's INBOX row is deleted", dao.getById("acct:1"))
assertNull("the account's other-folder row is deleted", dao.getById("acct:Archive:1"))
assertEquals("the other account survives", "acct2:1", dao.getById("acct2:1")?.id)
}
@Test
@@ -150,4 +150,47 @@ class SignatureDaoTest {
// clearDefault in setDefault only touches the target account; acct2's default is untouched.
assertEquals("b1", dao.getDefault("acct2")?.id)
}
@Test
fun insertMakingFirstDefaultMakesOnlyTheAccountsFirstSignatureDefault() = runBlocking {
insertAccount()
// The passed isDefault is a placeholder; the transaction decides it from the current count (#313).
dao.insertMakingFirstDefault(signature("s-1", "First", isDefault = false))
dao.insertMakingFirstDefault(signature("s-2", "Second", isDefault = true))
assertEquals(true, dao.getById("s-1")?.isDefault)
assertEquals(false, dao.getById("s-2")?.isDefault)
assertEquals("s-1", dao.getDefault("acct")?.id)
}
@Test
fun deletePromotingDefaultPromotesTheFirstRemainingWhenTheDefaultIsRemoved() = runBlocking {
insertAccount()
dao.upsert(signature("s-default", "Zeta", isDefault = true))
dao.upsert(signature("s-other", "alpha")) // name-first among the remaining rows
val promoted = dao.deletePromotingDefault("s-default")
assertEquals("s-other", promoted)
assertNull("the deleted default is gone", dao.getById("s-default"))
assertEquals("the first remaining becomes default", "s-other", dao.getDefault("acct")?.id)
}
@Test
fun deletePromotingDefaultPromotesNothingForANonDefaultOrTheLastRow() = runBlocking {
insertAccount()
dao.upsert(signature("s-default", "Default", isDefault = true))
dao.upsert(signature("s-plain", "Plain"))
// Deleting a non-default leaves the account's default untouched — nothing to promote.
assertNull(dao.deletePromotingDefault("s-plain"))
assertEquals("s-default", dao.getDefault("acct")?.id)
// Deleting the last (default) signature has no remaining row to promote.
assertNull(dao.deletePromotingDefault("s-default"))
assertNull(dao.getDefault("acct"))
// A missing id is a no-op.
assertNull(dao.deletePromotingDefault("absent"))
}
}
@@ -15,7 +15,6 @@ import io.mockk.mockkObject
import io.mockk.unmockkAll
import io.mockk.verify
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.runBlocking
import org.junit.After
@@ -138,7 +137,7 @@ class DatabaseModuleInstrumentedTest {
// ever opened this genuinely-encrypted file with the plaintext framework helper instead of
// SQLCipher's, this would throw (a plaintext driver can't parse SQLCipher ciphertext) rather
// than return the seeded row.
assertEquals(listOf("acct:1"), database.messageDao().observeSummaries().first().map { it.id })
assertEquals("acct:1", database.messageDao().getById("acct:1")?.id)
} finally {
database.close()
}
@@ -157,7 +156,7 @@ class DatabaseModuleInstrumentedTest {
val database = DatabaseModule.provideDatabase(context, provisioner())
try {
database.messageDao().insertNew(listOf(message("acct:1")))
assertEquals(listOf("acct:1"), database.messageDao().observeSummaries().first().map { it.id })
assertEquals("acct:1", database.messageDao().getById("acct:1")?.id)
} finally {
database.close()
}
@@ -182,7 +181,7 @@ class DatabaseModuleInstrumentedTest {
val database = DatabaseModule.provideDatabase(context, provisioner())
try {
assertThrows(Throwable::class.java) {
runBlocking { database.messageDao().observeSummaries().first() }
runBlocking { database.messageDao().getById("acct:1") }
}
} finally {
runCatching { database.close() }
@@ -10,7 +10,6 @@ import android.os.Bundle
import androidx.room.Room
import androidx.sqlite.db.SupportSQLiteDatabase
import androidx.sqlite.db.SupportSQLiteOpenHelper
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.runBlocking
import net.zetetic.database.sqlcipher.SupportOpenHelperFactory
import org.libremail.data.local.DatabaseEncryption
@@ -112,8 +111,8 @@ class ColdOpenCacheProbe : ContentProvider() {
)
.build()
try {
val ids = runBlocking { database.messageDao().observeSummaries().first().map { it.id } }
if (ids == listOf(EXPECTED_ROW_ID)) OPEN_OK else "$OPEN_ROWS$ids"
val id = runBlocking { database.messageDao().getById(EXPECTED_ROW_ID)?.id }
if (id == EXPECTED_ROW_ID) OPEN_OK else "$OPEN_ROWS$id"
} finally {
database.close()
}
@@ -98,10 +98,11 @@ class AccountDataMigrator @Inject constructor(
private val TABLES = listOf("accounts", "credentials", "account_settings", "signatures")
/**
* DDL for the account tables in [AccountDatabase] v1, copied verbatim from the exported Room
* schema (`schemas/org.libremail.data.local.AccountDatabase/1.json`). It MUST stay byte-for-byte
* identical to what Room generates for those entities, or Room silently accepts a subtly wrong
* schema (its identity check only compares the hash it writes, not the pre-existing tables).
* DDL for the account tables in [AccountDatabase] v2, copied verbatim from the exported Room
* schema (`schemas/org.libremail.data.local.AccountDatabase/2.json` — v2 added `accounts.sortOrder`,
* issue #164). It MUST stay byte-for-byte identical to what Room generates for those entities, or
* Room silently accepts a subtly wrong schema (its identity check only compares the hash it writes,
* not the pre-existing tables).
* `AccountDataMigratorTest.migratorDdlMatchesExportedAccountDatabaseSchema` guards it against the
* exported schema; `internal` only so that test can read it.
*/
@@ -80,10 +80,14 @@ object DatabaseEncryption {
target.close()
}
// Swap the converted file into place; drop any stale WAL/SHM sidecars from either file first.
// Swap the converted file into place; drop any stale sidecars from either file first. Both files
// are in rollback-journal mode (journal_mode = DELETE), so the sidecar that can actually linger
// after an interrupted attempt is the `-journal`; the `-wal`/`-shm` deletes are belt-and-suspenders
// for a file left in WAL mode by an older build (mirrors AccountDataMigrator's sweep).
listOf(dbFile.name, tmp.name).forEach { base ->
File(dir, "$base-wal").delete()
File(dir, "$base-shm").delete()
File(dir, "$base-journal").delete()
}
if (!tmp.renameTo(dbFile)) {
tmp.copyTo(dbFile, overwrite = true)
@@ -5,6 +5,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.AccountSettingsEntity
@@ -18,4 +19,15 @@ interface AccountSettingsDao {
@Insert(onConflict = OnConflictStrategy.REPLACE)
suspend fun upsert(settings: AccountSettingsEntity)
/**
* Reads [accountId]'s row (or null when absent), applies [transform], and writes the result — all in
* one transaction so a per-field setter's read-modify-write can't interleave with a concurrent
* setter and clobber the other field (issue #313). [transform] receives the stored entity, or null
* when no row exists yet.
*/
@Transaction
suspend fun readModifyWrite(accountId: String, transform: (AccountSettingsEntity?) -> AccountSettingsEntity) {
upsert(transform(get(accountId)))
}
}
@@ -15,18 +15,6 @@ import org.libremail.data.local.entity.MessageSummary
@Dao
interface MessageDao {
/**
* Mailbox-list projection ordered newest-first. Deliberately omits the large `body`/`isHtml`
* columns: the list observes every cached message at once, and pulling full bodies through
* SQLite's shared ~2 MB CursorWindow overflows it once enough large bodies are cached
* (issue #51). Bodies are loaded lazily per-message via [getById] when a message is opened.
*/
@Query(
"SELECT id, accountId, sender, senderEmail, subject, snippet, timestampMillis, " +
"isRead, isStarred, folder, inInbox, bodyFetched FROM messages ORDER BY timestampMillis DESC",
)
fun observeSummaries(): Flow<List<MessageSummary>>
/**
* Paged unified-inbox projection: folder-synced rows of [folder] across every account,
* newest-first, as a Paging 3 [PagingSource] (issue #124). Room loads only the requested window
@@ -44,4 +44,32 @@ interface SignatureDao {
clearDefault(accountId)
markDefault(id)
}
/**
* Inserts [signature], making it the account's default when it is the account's first — the count
* and the insert run in one transaction so two concurrent first-creates can't both read "count 0"
* and both become default (issue #313). [signature]'s own `isDefault` is ignored: this method
* decides it from the current count.
*/
@Transaction
suspend fun insertMakingFirstDefault(signature: SignatureEntity) {
upsert(signature.copy(isDefault = countForAccount(signature.accountId) == 0))
}
/**
* Deletes [id] and, when it was the account's default, promotes the account's first remaining
* signature — both in one transaction so a crash between the delete and the promote can't leave an
* account with signatures but no default (issue #313). No-op when [id] is absent. Returns the id of
* the signature promoted to default, or null when nothing was promoted (id absent, the deleted row
* wasn't the default, or no signatures remain).
*/
@Transaction
suspend fun deletePromotingDefault(id: String): String? {
val existing = getById(id) ?: return null
delete(id)
if (!existing.isDefault) return null
val promoted = firstForAccount(existing.accountId) ?: return null
markDefault(promoted.id)
return promoted.id
}
}
@@ -337,16 +337,16 @@ class MailRepositoryImpl @Inject constructor(
moveByRole(ids, FolderRole.TRASH, fallbackExpunge = true)
override suspend fun expunge(ids: List<String>): Result<Unit> = runCatching {
val routings = messageDao.getRoutingByIds(ids)
messageDao.deleteByIds(ids) // optimistic
val routings = messageDao.getRoutingByIdsChunked(ids)
messageDao.deleteByIdsChunked(ids) // optimistic
forEachAccountFolder(routings) { params, folder, group ->
imapClient.deleteMessages(params, folder, group.map { uidOf(it.id) })
}
}
override suspend fun moveToFolder(ids: List<String>, destFolderFullName: String): Result<Unit> = runCatching {
val routings = messageDao.getRoutingByIds(ids)
messageDao.deleteByIds(ids) // optimistic
val routings = messageDao.getRoutingByIdsChunked(ids)
messageDao.deleteByIdsChunked(ids) // optimistic
forEachAccountFolder(routings) { params, folder, group ->
if (folder != destFolderFullName) {
imapClient.moveMessages(params, folder, group.map { uidOf(it.id) }, destFolderFullName)
@@ -393,8 +393,8 @@ class MailRepositoryImpl @Inject constructor(
*/
private suspend fun moveByRole(ids: List<String>, role: FolderRole, fallbackExpunge: Boolean): Result<Unit> =
runCatching {
val routings = messageDao.getRoutingByIds(ids)
messageDao.deleteByIds(ids) // optimistic
val routings = messageDao.getRoutingByIdsChunked(ids)
messageDao.deleteByIdsChunked(ids) // optimistic
val destByAccount = routings.map { it.accountId }.distinct()
.associateWith { resolveRoleFolder(it, role) }
forEachAccountFolder(routings) { params, folder, group ->
@@ -556,6 +556,12 @@ private const val NANOS_PER_MS = 1_000_000L
/** Rows per page for the unified inbox (issue #124) — a page is a few screenfuls of message rows. */
private const val MAILBOX_PAGE_SIZE = 40
/**
* Ids per `IN (:ids)` query in the batch move/delete/expunge paths, kept under SQLite's 999
* host-parameter limit on older Android (matches [org.libremail.data.sync.MailPruner]'s DELETE chunk).
*/
private const val SQL_IN_CHUNK = 500
/** Attempts for the background best-effort SEEN-flag push before giving up silently (issue #148). */
private const val SEEN_FLAG_PUSH_MAX_ATTEMPTS = 3
@@ -565,6 +571,20 @@ private const val SEEN_FLAG_RETRY_BACKOFF_MS = 2_000L
/** Message id is "<accountId>:<uid>"; the uid is the trailing segment. */
private fun uidOf(id: String): String = id.substringAfterLast(':')
/**
* [MessageDao.getRoutingByIds] over an arbitrarily large [ids] list, chunked so the expanded `IN (:ids)`
* never exceeds SQLite's host-parameter limit (999 on older Android). The batch move/delete/expunge
* callers are bounded by the multi-select cap today, but chunking removes the latent
* `SQLITE_MAX_VARIABLE_NUMBER` crash the same way [org.libremail.data.sync.MailPruner] does (issue #313).
*/
private suspend fun MessageDao.getRoutingByIdsChunked(ids: List<String>): List<MessageRouting> =
ids.chunked(SQL_IN_CHUNK).flatMap { getRoutingByIds(it) }
/** [MessageDao.deleteByIds] chunked under SQLite's host-parameter limit (see [getRoutingByIdsChunked]). */
private suspend fun MessageDao.deleteByIdsChunked(ids: List<String>) {
ids.chunked(SQL_IN_CHUNK).forEach { deleteByIds(it) }
}
/**
* Builds the SQL `LIKE` pattern the paged-search DAO queries take (issue #214), preserving the old
* `matchesSearch` literal-substring semantics: escape the LIKE metacharacters (`\ % _`) — the `\`
@@ -49,7 +49,14 @@ class AccountSettingsRepository @Inject constructor(private val dao: AccountSett
it.copy(retentionMonths = months?.coerceAtLeast(0))
}
private suspend inline fun update(accountId: String, transform: (AccountSettings) -> AccountSettings) {
dao.upsert(transform(get(accountId)).toEntity())
/**
* Read-modify-writes an account's settings row through the DAO's single-transaction helper so a
* concurrent per-field setter can't clobber the read-modify-write (issue #313). A missing row is
* transformed from the account's defaults, preserving the "not configured yet = defaults" contract.
*/
private suspend fun update(accountId: String, transform: (AccountSettings) -> AccountSettings) {
dao.readModifyWrite(accountId) { stored ->
transform(stored?.toDomain() ?: AccountSettings(accountId)).toEntity()
}
}
}
@@ -6,6 +6,7 @@ import kotlinx.coroutines.flow.map
import org.libremail.data.local.dao.SignatureDao
import org.libremail.data.local.entity.SignatureEntity
import org.libremail.domain.model.Signature
import org.libremail.reporting.AppLog
import java.util.UUID
import javax.inject.Inject
import javax.inject.Singleton
@@ -28,8 +29,9 @@ class SignatureRepository @Inject constructor(private val dao: SignatureDao) {
/** Creates a signature; makes it the default when it is the account's first. Returns its id. */
suspend fun create(accountId: String, name: String, html: String): String {
val id = UUID.randomUUID().toString()
val isFirst = dao.countForAccount(accountId) == 0
dao.upsert(SignatureEntity(id, accountId, name, html, isDefault = isFirst))
// The count-then-default decision runs atomically in the DAO so two concurrent first-creates
// can't both become default (issue #313); the isDefault passed here is a placeholder.
dao.insertMakingFirstDefault(SignatureEntity(id, accountId, name, html, isDefault = false))
return id
}
@@ -39,15 +41,20 @@ class SignatureRepository @Inject constructor(private val dao: SignatureDao) {
}
suspend fun delete(id: String) {
val existing = dao.getById(id) ?: return
dao.delete(id)
// If we removed the default, promote the account's first remaining signature.
if (existing.isDefault) {
dao.firstForAccount(existing.accountId)?.let { dao.markDefault(it.id) }
// Delete + promote-a-new-default run in one DAO transaction so a crash between them can't strand
// the account with signatures but no default (issue #313).
val promotedId = dao.deletePromotingDefault(id)
if (promotedId != null) {
// PII-free: no signature content, name, or account address — just the state transition.
AppLog.i(TAG, "promoted a replacement default signature after deleting the previous default")
}
}
suspend fun setDefault(accountId: String, id: String) = dao.setDefault(accountId, id)
private fun SignatureEntity.toDomain() = Signature(id, accountId, name, contentHtml, isDefault)
private companion object {
const val TAG = "LibreMailSignatures"
}
}
@@ -7,6 +7,7 @@ import kotlinx.coroutines.withContext
import org.json.JSONArray
import org.json.JSONObject
import org.libremail.domain.model.OutgoingMessage
import org.libremail.reporting.AppLog
import java.io.IOException
import java.net.HttpURLConnection
import java.net.URL
@@ -34,6 +35,14 @@ class GraphSender @Inject constructor() {
message: OutgoingMessage,
attachments: List<SendableAttachment> = emptyList(),
) = withContext(Dispatchers.IO) {
// Guard before any attachment is read into memory: Graph sendMail carries attachment bytes inline
// (base64) in a single ~4 MB request, so an oversized file would blow that request limit and risk
// an OOM from readBytes(). Fail with mayHaveSent=false so the outbox falls back to SMTP, which
// streams attachments and handles far larger files (#298).
attachments.firstOrNull { it.file.length() > MAX_ATTACHMENT_BYTES }?.let {
AppLog.w(TAG, "Attachment over Graph sendMail size limit; not sending via Graph")
throw GraphSendException("Attachment exceeds the Graph sendMail size limit", mayHaveSent = false)
}
val payload = buildSendMailPayload(message, attachments)
val connection = (URL(SEND_MAIL_URL).openConnection() as HttpURLConnection).apply {
requestMethod = "POST"
@@ -76,8 +85,13 @@ class GraphSender @Inject constructor() {
}
private companion object {
const val TAG = "GraphSender"
const val SEND_MAIL_URL = "https://graph.microsoft.com/v1.0/me/sendMail"
const val TIMEOUT_MS = 15_000
// Per-file ceiling kept below Graph sendMail's ~4 MB whole-request cap, so one attachment can never
// exceed the request limit or OOM when read into the base64 payload; larger files fall back to SMTP.
const val MAX_ATTACHMENT_BYTES = 3L * 1024 * 1024
const val HTTP_OK_MIN = 200
const val HTTP_OK_MAX = 299
const val ERROR_BODY_LIMIT = 500
@@ -12,23 +12,29 @@ package org.libremail.mail
*/
object HtmlToText {
// Hoisted out of convert() so each pattern is compiled once, not four times per call — convert()
// runs once per fetched HTML body during sync, so this is a hot path (#298).
private val SCRIPT_STYLE = Regex("(?is)<(script|style)\\b[^>]*>.*?</\\1>")
private val LIST_ITEM = Regex("(?i)<li\\b[^>]*>")
private val BLOCK_BREAK = Regex(
"(?i)</?(p|div|tr|table|ul|ol|h[1-6]|blockquote)\\b[^>]*>|<br\\s*/?>",
)
private val LIST_ITEM = Regex("(?i)<li\\b[^>]*>")
private val TAG = Regex("<[^>]*>")
private val SPACES_AND_TABS = Regex("[ \\t]+")
private val BLANK_LINES = Regex("\n{3,}")
fun convert(html: String): String {
var s = html
// Drop script/style contents outright so their text never leaks into the output.
s = s.replace(Regex("(?is)<(script|style)\\b[^>]*>.*?</\\1>"), "")
s = SCRIPT_STYLE.replace(s, "")
s = LIST_ITEM.replace(s, "\n• ")
s = BLOCK_BREAK.replace(s, "\n")
s = s.replace(Regex("<[^>]*>"), "")
s = TAG.replace(s, "")
s = decodeEntities(s)
// Collapse runs of spaces/tabs, then trim trailing spaces and cap consecutive blank lines.
s = s.replace(Regex("[ \\t]+"), " ")
s = SPACES_AND_TABS.replace(s, " ")
s = s.lineSequence().joinToString("\n") { it.trim() }
s = s.replace(Regex("\n{3,}"), "\n\n")
s = BLANK_LINES.replace(s, "\n\n")
return s.trim()
}
@@ -99,14 +99,24 @@ private fun providerLabel(account: Account): String = when (account.authType) {
AuthType.PASSWORD_IMAP -> imapProviderLabel(account.imap.host)
}
/**
* Buckets an IMAP host to a coarse provider by matching brand tokens at DNS-label boundaries rather
* than as raw substrings, so a custom domain that merely contains a brand name — e.g.
* `mail.notgmail.example` — is no longer mislabeled (here it would have read as Gmail) (#298). The
* short, common tokens (`me`/`mac`/`live`) match only as a registrable-domain suffix, never as a bare
* label, so an innocent `me.company.example` doesn't read as iCloud either.
*/
private fun imapProviderLabel(host: String): String {
val h = host.lowercase()
val labels = h.split('.')
fun hasLabel(vararg brands: String) = brands.any { it in labels }
fun hasDomain(vararg domains: String) = domains.any { h == it || h.endsWith(".$it") }
return when {
"gmail" in h || "googlemail" in h -> "Gmail"
"yahoo" in h -> "Yahoo"
"icloud" in h || "me.com" in h || "mac.com" in h -> "iCloud"
"outlook" in h || "office365" in h || "hotmail" in h || "live.com" in h -> "Outlook"
"aol" in h -> "AOL"
hasLabel("gmail", "googlemail") -> "Gmail"
hasLabel("yahoo") -> "Yahoo"
hasLabel("icloud") || hasDomain("me.com", "mac.com") -> "iCloud"
hasLabel("outlook", "office365", "hotmail") || hasDomain("live.com") -> "Outlook"
hasLabel("aol") -> "AOL"
else -> "Other"
}
}
@@ -9,6 +9,9 @@ import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.asStateFlow
import kotlinx.coroutines.launch
import java.io.File
import java.nio.file.AtomicMoveNotSupportedException
import java.nio.file.Files
import java.nio.file.StandardCopyOption
/**
* File-backed store of pending [DebugReport]s — one JSON file per report under [directory].
@@ -52,7 +55,7 @@ class ReportStore(
synchronized(lock) {
val serialized = serializeForDisk(report) ?: return
directory.mkdirs()
File(directory, fileName(report.id)).writeText(serialized)
writeAtomically(File(directory, fileName(report.id)), serialized)
_reports.value = scan()
}
}
@@ -69,7 +72,7 @@ class ReportStore(
val report = _reports.value.firstOrNull { it.id == id } ?: return
if (report.surfaced) return
val serialized = serializeForDisk(report.copy(surfaced = true)) ?: return
File(directory, fileName(id)).writeText(serialized)
writeAtomically(File(directory, fileName(id)), serialized)
_reports.value = scan()
}
}
@@ -135,10 +138,38 @@ class ReportStore(
return runCatching { DebugReport.fromStorageJson(json) }.getOrNull()
}
/**
* Writes [content] to [target] via a temp file + atomic rename, so a process death mid-write — the
* crash path that saves a report while the app is dying — can never leave a torn `.json` that [scan]
* would fail to parse and silently drop (#298). The temp file uses a non-`.json` suffix so [scan]
* ignores it (and any orphan left by an interrupted write), and the rename replaces an existing file
* (the [markSurfaced] rewrite) atomically. Falls back to a plain replace on the rare filesystem
* without atomic rename — still safer than an in-place truncate-then-write.
*/
private fun writeAtomically(target: File, content: String) {
val tmp = File(directory, target.name + TMP_SUFFIX)
tmp.writeText(content)
try {
Files.move(
tmp.toPath(),
target.toPath(),
StandardCopyOption.ATOMIC_MOVE,
StandardCopyOption.REPLACE_EXISTING,
)
} catch (e: AtomicMoveNotSupportedException) {
AppLog.w(TAG, "Atomic report write unsupported here; falling back to a non-atomic replace", e)
Files.move(tmp.toPath(), target.toPath(), StandardCopyOption.REPLACE_EXISTING)
}
}
private fun fileName(id: String) = "$id$SUFFIX"
private companion object {
const val SUFFIX = ".json"
// Suffix for the write-and-rename temp file. Deliberately NOT ending in [SUFFIX] so scan() never
// treats a half-written or orphaned temp as a report (#298).
const val TMP_SUFFIX = ".tmp"
const val TAG = "ReportStore"
/**
@@ -109,11 +109,17 @@ internal fun lineMarker(line: String): String? = when {
*/
internal fun mergeSameValueSpans(spans: List<RichSpan>): List<RichSpan> {
val merged = ArrayList<RichSpan>()
// Index of the right-most merged run for each style value. Spans are processed in ascending start
// order, and runs of one value stay non-overlapping with strictly increasing ends, so only that
// value's last run can touch the next span — track it directly instead of re-scanning `merged` for
// every span (the old O(n^2) indexOfLast). The produced list is byte-for-byte identical (#298).
val lastRunByStyle = HashMap<RichStyle, Int>()
for (span in spans.sortedWith(compareBy({ it.start }, { it.end }))) {
val i = merged.indexOfLast { it.style == span.style && span.start <= it.end }
if (i >= 0) {
val i = lastRunByStyle[span.style]
if (i != null && span.start <= merged[i].end) {
merged[i] = merged[i].copy(end = maxOf(merged[i].end, span.end))
} else {
lastRunByStyle[span.style] = merged.size
merged.add(span)
}
}
@@ -33,10 +33,15 @@ object RichTextEditing {
return content.copy(spans = (otherKinds + updated).sortedBy { it.start })
}
/** Links [[start], [end]) to [url], replacing any links that overlap the range. */
/**
* Links [[start], [end]) to [url]. A link that only partially overlaps the range keeps its
* non-overlapping remainder — the same split [toggleStyle]/[subtractRange] does for spans — instead
* of being dropped whole, so relinking part of a longer link no longer silently un-links the rest
* of it (#298). A link fully inside the range is replaced outright.
*/
fun applyLink(content: RichTextContent, start: Int, end: Int, url: String): RichTextContent {
if (start >= end || url.isBlank()) return content
val kept = content.links.filter { it.end <= start || it.start >= end }
val kept = subtractLinkRange(content.links, start, end)
return content.copy(links = (kept + RichLink(start, end, url)).sortedBy { it.start })
}
@@ -221,6 +226,17 @@ private fun subtractRange(spans: List<RichSpan>, start: Int, end: Int): List<Ric
}
}
/** Links analogue of [subtractRange]: drops the [[start], [end]) slice of each link, keeping the rest. */
private fun subtractLinkRange(links: List<RichLink>, start: Int, end: Int): List<RichLink> = links.flatMap { link ->
when {
link.end <= start || link.start >= end -> listOf(link)
else -> buildList {
if (link.start < start) add(link.copy(end = start))
if (link.end > end) add(link.copy(start = end))
}
}
}
// --- block marker helpers ---
private val ORDERED = Regex("^\\d+\\. ")
@@ -445,6 +445,30 @@ class MailRepositoryImplTest {
coVerify { imapClient.deleteMessages(any(), "Trash", listOf("3")) }
}
@Test
fun `expunge chunks the id queries under SQLite's host-parameter limit`() = runTest {
// 501 ids force a split: an unchunked IN (:ids) would bind 501 host parameters, latent-crashing
// near SQLite's 999 limit on older Android (issue #313). Chunked at 500 -> a 500 + 1 split.
val ids = (1..501).map { "acct:INBOX:$it" }
coEvery { messageDao.getRoutingByIds(any()) } coAnswers {
firstArg<List<String>>().map { messageRouting(it, "INBOX") }
}
coEvery { messageDao.deleteByIds(any()) } just Runs
coEvery { accountDao.getById("acct") } returns accountEntity()
coEvery { connectionFactory.imapParamsFor(any()) } returns imapParams()
val result = repository.expunge(ids)
assertTrue(result.isSuccess)
// Both the routing read and the optimistic delete run one query per <=500-id chunk.
coVerify(exactly = 1) { messageDao.getRoutingByIds(match { it.size == 500 }) }
coVerify(exactly = 1) { messageDao.getRoutingByIds(match { it.size == 1 }) }
coVerify(exactly = 1) { messageDao.deleteByIds(match { it.size == 500 }) }
coVerify(exactly = 1) { messageDao.deleteByIds(match { it.size == 1 }) }
// Chunking is only a DB concern: all 501 UIDs still reach the server EXPUNGE in one grouped call.
coVerify { imapClient.deleteMessages(any(), "INBOX", match { it.size == 501 }) }
}
@Test
fun `moveToFolder moves messages to the chosen destination`() = runTest {
val id = "acct:INBOX:11"
@@ -2,6 +2,7 @@
package org.libremail.data.settings
import app.cash.turbine.test
import io.mockk.CapturingSlot
import io.mockk.Runs
import io.mockk.coEvery
import io.mockk.coVerify
@@ -24,6 +25,20 @@ class AccountSettingsRepositoryTest {
private val dao = mockk<AccountSettingsDao>()
private val repository = AccountSettingsRepository(dao)
/**
* Stubs the DAO's single-transaction read-modify-write (issue #313) to apply the repository's
* transform against [current] (the stored row, or null) and capture the entity it would persist — so
* these setter tests still assert the transformed row directly, while AccountSettingsDaoTest covers
* the real transaction.
*/
private fun captureUpdate(current: AccountSettingsEntity?): CapturingSlot<AccountSettingsEntity> {
val saved = slot<AccountSettingsEntity>()
coEvery { dao.readModifyWrite(any(), any()) } coAnswers {
saved.captured = secondArg<(AccountSettingsEntity?) -> AccountSettingsEntity>().invoke(current)
}
return saved
}
@Test
fun `get returns defaults when no row exists`() = runTest {
coEvery { dao.get("acct") } returns null
@@ -38,10 +53,9 @@ class AccountSettingsRepositoryTest {
@Test
fun `setSignature reads, modifies, and writes the row`() = runTest {
coEvery { dao.get("acct") } returns
AccountSettingsEntity("acct", signature = "old", signatureEnabled = true, notificationsEnabled = false)
val saved = slot<AccountSettingsEntity>()
coEvery { dao.upsert(capture(saved)) } just Runs
val saved = captureUpdate(
AccountSettingsEntity("acct", signature = "old", signatureEnabled = true, notificationsEnabled = false),
)
repository.setSignature("acct", "new")
@@ -101,10 +115,9 @@ class AccountSettingsRepositoryTest {
@Test
fun `setSignatureEnabled reads, modifies, and writes the row`() = runTest {
coEvery { dao.get("acct") } returns
AccountSettingsEntity("acct", signature = "keep", signatureEnabled = true, notificationsEnabled = true)
val saved = slot<AccountSettingsEntity>()
coEvery { dao.upsert(capture(saved)) } just Runs
val saved = captureUpdate(
AccountSettingsEntity("acct", signature = "keep", signatureEnabled = true, notificationsEnabled = true),
)
repository.setSignatureEnabled("acct", false)
@@ -115,9 +128,7 @@ class AccountSettingsRepositoryTest {
@Test
fun `setNotificationsEnabled reads, modifies, and writes the row`() = runTest {
coEvery { dao.get("acct") } returns AccountSettingsEntity("acct", notificationsEnabled = true)
val saved = slot<AccountSettingsEntity>()
coEvery { dao.upsert(capture(saved)) } just Runs
val saved = captureUpdate(AccountSettingsEntity("acct", notificationsEnabled = true))
repository.setNotificationsEnabled("acct", false)
@@ -126,9 +137,7 @@ class AccountSettingsRepositoryTest {
@Test
fun `setRetentionCount clamps a negative override to zero`() = runTest {
coEvery { dao.get("acct") } returns AccountSettingsEntity("acct")
val saved = slot<AccountSettingsEntity>()
coEvery { dao.upsert(capture(saved)) } just Runs
val saved = captureUpdate(AccountSettingsEntity("acct"))
repository.setRetentionCount("acct", -5)
@@ -137,9 +146,7 @@ class AccountSettingsRepositoryTest {
@Test
fun `setRetentionCount preserves null as inherit-the-global-default`() = runTest {
coEvery { dao.get("acct") } returns AccountSettingsEntity("acct", retentionCount = 10)
val saved = slot<AccountSettingsEntity>()
coEvery { dao.upsert(capture(saved)) } just Runs
val saved = captureUpdate(AccountSettingsEntity("acct", retentionCount = 10))
repository.setRetentionCount("acct", null)
@@ -148,9 +155,7 @@ class AccountSettingsRepositoryTest {
@Test
fun `setRetentionMonths clamps a negative override to zero`() = runTest {
coEvery { dao.get("acct") } returns AccountSettingsEntity("acct")
val saved = slot<AccountSettingsEntity>()
coEvery { dao.upsert(capture(saved)) } just Runs
val saved = captureUpdate(AccountSettingsEntity("acct"))
repository.setRetentionMonths("acct", -3)
@@ -159,12 +164,23 @@ class AccountSettingsRepositoryTest {
@Test
fun `setRetentionMonths keeps a positive override as given`() = runTest {
coEvery { dao.get("acct") } returns AccountSettingsEntity("acct")
val saved = slot<AccountSettingsEntity>()
coEvery { dao.upsert(capture(saved)) } just Runs
val saved = captureUpdate(AccountSettingsEntity("acct"))
repository.setRetentionMonths("acct", 6)
assertEquals(6, saved.captured.retentionMonths)
}
@Test
fun `a setter transforms the account defaults when no row exists yet`() = runTest {
// The DAO hands the transform a null stored row for a never-configured account; the repository
// must transform from the account's defaults so the "not configured = defaults" contract holds.
val saved = captureUpdate(current = null)
repository.setSignature("acct", "first")
assertEquals("acct", saved.captured.accountId)
assertEquals("first", saved.captured.signature)
assertTrue(saved.captured.signatureEnabled, "defaults carry through the transform")
}
}
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-3.0-or-later
package org.libremail.data.settings
import android.util.Log
import app.cash.turbine.test
import io.mockk.Runs
import io.mockk.coEvery
@@ -8,12 +9,18 @@ import io.mockk.coVerify
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.mockkStatic
import io.mockk.slot
import io.mockk.unmockkAll
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.SignatureDao
import org.libremail.data.local.entity.SignatureEntity
import org.libremail.reporting.AppLog
import org.libremail.reporting.RingLogBuffer
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertNull
@@ -24,51 +31,70 @@ class SignatureRepositoryTest {
private val dao = mockk<SignatureDao>(relaxed = true)
private val repository = SignatureRepository(dao)
// delete() breadcrumbs a promotion via AppLog; android.util.Log is an unmocked stub in plain JVM
// tests, so mock it class-wide (mirrors MailRepositoryImplTest).
@Before
fun setUp() {
mockkStatic(Log::class)
every { Log.i(any(), any()) } returns 0
}
@After
fun tearDown() = unmockkAll()
private fun entity(id: String, isDefault: Boolean) =
SignatureEntity(id, accountId = "acct", name = "N", contentHtml = "<p>x</p>", isDefault = isDefault)
@Test
fun `the first signature for an account becomes its default`() = runTest {
coEvery { dao.countForAccount("acct") } returns 0
fun `create routes through the atomic first-default insert with the given fields`() = runTest {
// The first-becomes-default decision now lives in the DAO transaction (issue #313); the repository
// just forwards the new row (isDefault a placeholder) and returns its generated id.
val saved = slot<SignatureEntity>()
coEvery { dao.upsert(capture(saved)) } just Runs
coEvery { dao.insertMakingFirstDefault(capture(saved)) } just Runs
repository.create("acct", "Work", "<p>hi</p>")
val id = repository.create("acct", "Work", "<p>hi</p>")
assertTrue(saved.captured.isDefault)
assertEquals("acct", saved.captured.accountId)
assertEquals("Work", saved.captured.name)
assertEquals("<p>hi</p>", saved.captured.contentHtml)
assertEquals(id, saved.captured.id)
assertFalse(saved.captured.isDefault, "the DAO decides the default flag, not the repository")
}
@Test
fun `later signatures are not made default`() = runTest {
coEvery { dao.countForAccount("acct") } returns 2
val saved = slot<SignatureEntity>()
coEvery { dao.upsert(capture(saved)) } just Runs
repository.create("acct", "Personal", "<p>hey</p>")
assertFalse(saved.captured.isDefault)
}
@Test
fun `deleting the default promotes the first remaining signature`() = runTest {
coEvery { dao.getById("s1") } returns entity("s1", isDefault = true)
coEvery { dao.firstForAccount("acct") } returns entity("s2", isDefault = false)
fun `delete routes through the atomic delete-and-promote`() = runTest {
coEvery { dao.deletePromotingDefault("s1") } returns null
repository.delete("s1")
coVerify { dao.delete("s1") }
coVerify { dao.markDefault("s2") }
coVerify { dao.deletePromotingDefault("s1") }
}
@Test
fun `deleting a non-default signature promotes nothing`() = runTest {
coEvery { dao.getById("s2") } returns entity("s2", isDefault = false)
fun `deleting a default that promotes a replacement logs a breadcrumb`() = runTest {
val buffer = RingLogBuffer()
AppLog.install(buffer)
coEvery { dao.deletePromotingDefault("s1") } returns "s2"
repository.delete("s1")
assertTrue(
buffer.snapshot().any { it.message.contains("promoted a replacement default signature") },
"the promotion is recorded for a debug report",
)
}
@Test
fun `deleting without a promotion logs nothing`() = runTest {
val buffer = RingLogBuffer()
AppLog.install(buffer)
coEvery { dao.deletePromotingDefault("s2") } returns null
repository.delete("s2")
coVerify { dao.delete("s2") }
coVerify(exactly = 0) { dao.firstForAccount(any()) }
coVerify(exactly = 0) { dao.markDefault(any()) }
assertFalse(
buffer.snapshot().any { it.message.contains("promoted a replacement default") },
)
}
@Test
@@ -1,14 +1,20 @@
// SPDX-License-Identifier: GPL-3.0-or-later
package org.libremail.mail
import io.mockk.every
import io.mockk.mockkStatic
import io.mockk.unmockkAll
import kotlinx.coroutines.test.runTest
import org.junit.After
import org.junit.Before
import org.junit.Test
import org.libremail.domain.model.OutgoingMessage
import java.io.ByteArrayOutputStream
import java.io.File
import java.io.IOException
import java.io.InputStream
import java.io.OutputStream
import java.io.RandomAccessFile
import java.net.HttpURLConnection
import java.net.URL
import java.net.URLConnection
@@ -18,6 +24,7 @@ import java.util.concurrent.atomic.AtomicReference
import kotlin.test.assertEquals
import kotlin.test.assertFailsWith
import kotlin.test.assertFalse
import kotlin.test.assertNull
import kotlin.test.assertTrue
/**
@@ -30,8 +37,19 @@ import kotlin.test.assertTrue
*/
class GraphSenderSendTest {
@Before
fun setUp() {
// send() now breadcrumbs through AppLog on the oversized-attachment guard; android.util.Log is a
// no-op stub under plain JVM tests, so mock it (fully qualified, so this file never imports it).
mockkStatic(android.util.Log::class)
every { android.util.Log.w(any<String>(), any<String>()) } returns 0
}
@After
fun tearDown() = armed.set(null)
fun tearDown() {
armed.set(null)
unmockkAll()
}
private val message =
OutgoingMessage(accountId = "outlook:me@x.com", to = "bob@example.org", subject = "Hi", body = "Body")
@@ -84,6 +102,26 @@ class GraphSenderSendTest {
assertFalse(ex.mayHaveSent, "the request never reached Graph, so a retry is safe")
}
@Test
fun `an oversized attachment fails safe-to-fall-back before opening a connection`() = runTest {
val last = arm() // armed, but the guard must trip before any connection is opened
val big = File.createTempFile("graph-big", ".bin")
try {
// 4 MiB, over the 3 MiB per-file cap. setLength allocates the size without writing the bytes,
// so the guard (which reads file.length()) trips without the test materializing 4 MiB.
RandomAccessFile(big, "rw").use { it.setLength(4L * 1024 * 1024) }
val ex = assertFailsWith<GraphSendException> {
GraphSender().send("token", message, listOf(SendableAttachment(big)))
}
assertFalse(ex.mayHaveSent, "oversized never reached Graph, so SMTP fallback is safe")
assertNull(last.get(), "the guard must trip before any connection is opened")
} finally {
big.delete()
}
}
@Test
fun `GraphSendException carries its message, flag and cause`() {
val cause = IOException("boom")
@@ -169,6 +169,26 @@ class DiagnosticsCollectorTest {
)
}
@Test
fun `provider label matches brand tokens at label boundaries, not as substrings`() = runTest {
every { settingsRepository.settings } returns flowOf(AppSettings())
// Each custom host merely CONTAINS a brand name inside a longer DNS label; the old substring match
// mislabeled them (notgmail→Gmail, yahooligans→Yahoo, me.company→iCloud, kaolin→AOL). They must all
// bucket to "Other" now (#298).
every { accountRepository.observeAccounts() } returns flowOf(
listOf(
account("1@x", AuthType.PASSWORD_IMAP, "mail.notgmail.example"),
account("2@x", AuthType.PASSWORD_IMAP, "imap.yahooligans.example"),
account("3@x", AuthType.PASSWORD_IMAP, "me.company.example"),
account("4@x", AuthType.PASSWORD_IMAP, "kaolin.example"),
),
)
val report = collector.collectManual()
assertEquals(List(4) { "Other (PASSWORD_IMAP)" }, report.accounts)
}
private fun account(email: String, authType: AuthType, imapHost: String) = Account(
id = "id:$email",
email = email,
@@ -111,6 +111,29 @@ class ReportStoreTest {
assertEquals(listOf("valid"), store.reports.value.map { it.id })
}
@Test
fun `save leaves no temporary file behind (atomic write renames it into place)`() {
val store = newStore()
store.save(report("a"))
// The write-and-rename temp must not linger: only the final ".json" remains on disk (#298).
assertEquals(listOf("a.json"), tempFolder.root.listFiles()?.map { it.name }.orEmpty())
}
@Test
fun `a stray temp file from an interrupted write is never scanned as a report`() {
// A process death mid-write leaves a ".json.tmp" file, never a torn ".json". scan() filters on
// ".json", so the orphan is ignored and a valid report saved alongside still lists cleanly — the
// old in-place write could instead leave a truncated ".json" that scan() silently dropped (#298).
File(tempFolder.root, "torn.json.tmp").writeText("{ half-written")
val store = newStore()
store.save(report("valid"))
assertEquals(listOf("valid"), store.reports.value.map { it.id })
}
@Test
fun `purgeOlderThan deletes reports strictly older than the cutoff`() {
val store = newStore()
@@ -301,18 +301,53 @@ class RichTextEditingTest {
}
@Test
fun `applyLink keeps links wholly outside the range and replaces overlapping ones`() {
fun `applyLink keeps links outside the range and splits partial overlaps, keeping the remainder`() {
val content = RichTextContent(
"0123456789",
links = listOf(RichLink(0, 2, "a"), RichLink(3, 6, "b"), RichLink(7, 9, "c")),
)
val result = RichTextEditing.applyLink(content, 4, 7, "http://new")
assertEquals(
listOf(RichLink(0, 2, "a"), RichLink(4, 7, "http://new"), RichLink(7, 9, "c")),
// "a" is wholly outside; "b" (3,6) overlaps [4,7) so only its (3,4) remainder survives (it is
// no longer dropped whole); the new link takes [4,7); "c" starts at the range end, kept whole.
listOf(RichLink(0, 2, "a"), RichLink(3, 4, "b"), RichLink(4, 7, "http://new"), RichLink(7, 9, "c")),
result.links.sortedBy { it.start },
)
}
@Test
fun `applyLink over the middle of a link relinks the middle and keeps both surrounding remainders`() {
val content = RichTextContent("0123456789", links = listOf(RichLink(0, 8, "old")))
val result = RichTextEditing.applyLink(content, 3, 5, "new")
assertEquals(
listOf(RichLink(0, 3, "old"), RichLink(3, 5, "new"), RichLink(5, 8, "old")),
result.links.sortedBy { it.start },
)
}
// --- mergeSameValueSpans (shared merge used by toggleStyle and the HTML parser) ---
@Test
fun `mergeSameValueSpans coalesces touching and overlapping runs of the same value only`() {
val merged = mergeSameValueSpans(
listOf(
RichSpan(5, 8, RichStyle.Bold), // out of order, and overlaps the (3,6) run below
RichSpan(0, 3, RichStyle.Bold),
RichSpan(3, 6, RichStyle.Bold), // touches (0,3) and overlaps (5,8) → one 0..8 run
RichSpan(0, 4, RichStyle.Italic), // a different value never folds into the Bold run
RichSpan(10, 12, RichStyle.Bold), // a gap breaks the run into a fresh one
),
)
assertEquals(
listOf(
RichSpan(0, 8, RichStyle.Bold),
RichSpan(0, 4, RichStyle.Italic),
RichSpan(10, 12, RichStyle.Bold),
),
merged,
)
}
// --- styleAt / isStyled caret edges ---
@Test
+3
View File
@@ -73,6 +73,9 @@ style:
# Reader-path perf logging (issue #358): the repository's openMessage and the reader ViewModel
# log via AppLog, so their unit tests mockkStatic(Log) too.
- '**/data/repository/MailRepositoryImplTest.kt'
# SignatureRepository.delete breadcrumbs a default-promotion via AppLog (issue #313), so its unit
# test mockkStatic(Log) — it does not bypass the facade.
- '**/data/settings/SignatureRepositoryTest.kt'
- '**/data/repository/MailRepositoryImplCoverageTest.kt'
# Account-add breadcrumb (issue #403): addImapAccount/addOutlookAccount log via AppLog, so this
# suite mockkStatic(Log) so the calls don't crash on the throwing JVM stub.
+2 -1
View File
@@ -110,7 +110,8 @@ UNIFIED folder-only : SCAN TABLE messages USING INDEX index_messages_timestampMi
search (`matchesSearch`) over the small folder-scoped set — never the whole cache. `StateFlow`'s
built-in equality de-dup means an unrelated write now costs one cheap scoped re-query and no
recomposition.
- `observeSummaries()` is retained as the #51 CursorWindow regression-guard target in the DB tests.
- The #51 CursorWindow regression guard now targets the paged `pagingUnifiedFolderSummaries()`
projection in the DB tests; the superseded whole-table `observeSummaries()` was removed (issue #313).
## Deferred follow-ups