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

This commit is contained in:
Jason Ross
2026-07-04 03:01:19 -05:00
committed by GitHub
13 changed files with 297 additions and 13 deletions
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-3.0-or-later
package org.libremail.data.repository
import android.content.Context
import app.cash.turbine.test
import io.mockk.Runs
import io.mockk.coEvery
@@ -13,11 +14,15 @@ import io.mockk.verify
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.test.runTest
import org.junit.Test
import org.libremail.data.attachment.AttachmentUriGrants
import org.libremail.data.attachmentCacheDir
import org.libremail.data.local.dao.AccountDao
import org.libremail.data.local.dao.BackfillProgressDao
import org.libremail.data.local.dao.DraftDao
import org.libremail.data.local.dao.FolderDao
import org.libremail.data.local.dao.MessageDao
import org.libremail.data.local.entity.AccountEntity
import org.libremail.data.local.entity.DraftEntity
import org.libremail.data.local.entity.ServerConfigEmbedded
import org.libremail.data.security.CredentialStore
import org.libremail.data.settings.AccountSettingsRepository
@@ -30,6 +35,7 @@ import org.libremail.domain.model.ServerConfig
import org.libremail.mail.FetchedFolder
import org.libremail.mail.ImapClient
import org.libremail.notifications.MailNotifier
import java.io.File
import kotlin.test.assertEquals
import kotlin.test.assertFalse
import kotlin.test.assertTrue
@@ -42,26 +48,32 @@ import kotlin.test.assertTrue
*/
class AccountRepositoryImplTest {
private val context = mockk<Context>(relaxed = true)
private val accountDao = mockk<AccountDao>()
private val messageDao = mockk<MessageDao>(relaxed = true)
private val folderDao = mockk<FolderDao>(relaxed = true)
private val backfillProgressDao = mockk<BackfillProgressDao>(relaxed = true)
private val draftDao = mockk<DraftDao>(relaxed = true)
private val credentialStore = mockk<CredentialStore>(relaxed = true)
private val imapClient = mockk<ImapClient>()
private val syncScheduler = mockk<SyncScheduler>(relaxed = true)
private val accountSettingsRepository = mockk<AccountSettingsRepository>(relaxed = true)
private val mailNotifier = mockk<MailNotifier>(relaxed = true)
private val attachmentUriGrants = mockk<AttachmentUriGrants>(relaxed = true)
private val repository = AccountRepositoryImpl(
context = context,
accountDao = accountDao,
messageDao = messageDao,
folderDao = folderDao,
backfillProgressDao = backfillProgressDao,
draftDao = draftDao,
credentialStore = credentialStore,
imapClient = imapClient,
syncScheduler = syncScheduler,
accountSettingsRepository = accountSettingsRepository,
mailNotifier = mailNotifier,
attachmentUriGrants = attachmentUriGrants,
)
@Test
@@ -180,6 +192,37 @@ class AccountRepositoryImplTest {
coVerify { messageDao.deleteByAccount("acct") }
coVerify { folderDao.deleteForAccount("acct") }
coVerify { backfillProgressDao.deleteForAccount("acct") }
coVerify { draftDao.deleteByAccount("acct") }
}
@Test
fun `deleteAccount purges on-disk attachment caches and releases draft URI grants`() = runTest {
// A real temp cacheDir with one message's attachment cache dir populated on disk.
val cacheDir = File(System.getProperty("java.io.tmpdir"), "libremail-delete-test-${System.nanoTime()}")
every { context.cacheDir } returns cacheDir
val messageId = "acct:INBOX:1"
val attachmentDir = attachmentCacheDir(cacheDir, messageId).apply { mkdirs() }
File(attachmentDir, "0/report.pdf").apply { parentFile?.mkdirs() }.writeText("bytes")
// The account owns that message and a draft carrying a picked attachment URI.
coEvery { accountDao.deleteById("acct") } just Runs
coEvery { messageDao.getIdsForAccount("acct") } returns listOf(messageId)
coEvery { draftDao.getByAccount("acct") } returns listOf(
draftEntity(id = "d1", attachments = """[{"uri":"content://pick/1","name":"report.pdf"}]"""),
)
val released = slot<Collection<String>>()
coEvery { attachmentUriGrants.releaseUnreferenced(capture(released)) } just Runs
repository.deleteAccount("acct")
// The message ids must be read BEFORE the rows are deleted, and the on-disk cache purged.
coVerify { messageDao.getIdsForAccount("acct") }
assertFalse(attachmentDir.exists(), "attachment cache dir must be deleted with the account")
// The deleted draft's picked-URI grant is released.
coVerify { draftDao.deleteByAccount("acct") }
assertEquals(listOf("content://pick/1"), released.captured.toList())
cacheDir.deleteRecursively()
}
@Test
@@ -227,6 +270,17 @@ class AccountRepositoryImplTest {
smtp = ServerConfigEmbedded("smtp.example.org", 465, "SSL_TLS"),
)
private fun draftEntity(id: String, attachments: String) = DraftEntity(
id = id,
accountId = "acct",
toAddresses = "",
ccAddresses = "",
subject = "",
body = "",
updatedAt = 0L,
attachments = attachments,
)
private val params = ImapConnectionParams(
host = "imap.example.org",
port = 993,
@@ -0,0 +1,58 @@
// SPDX-License-Identifier: GPL-3.0-or-later
package org.libremail.domain.model
import org.junit.Test
import kotlin.test.assertEquals
/**
* Locks down account-id normalization (issue #305): the id is the primary key, so casing that varies
* between adds must not spawn a second account syncing the same mailbox. [normalizeEmailForAccountId]
* always lowercases the domain and — for consumer providers — the whole address; the generic path
* leaves the local part as typed. [Account.outlook] derives a deterministic id from the address.
*/
class AccountTest {
@Test
fun `normalize always lowercases the domain, keeping the local part when not a consumer provider`() {
assertEquals("User@example.org", normalizeEmailForAccountId("User@Example.ORG", lowercaseLocalPart = false))
assertEquals("ADA@gmail.com", normalizeEmailForAccountId("ADA@GMAIL.COM", lowercaseLocalPart = false))
}
@Test
fun `normalize lowercases the whole address for consumer providers`() {
assertEquals("user@example.org", normalizeEmailForAccountId("User@Example.ORG", lowercaseLocalPart = true))
assertEquals("ada@gmail.com", normalizeEmailForAccountId("ADA@GMAIL.COM", lowercaseLocalPart = true))
}
@Test
fun `normalize trims surrounding whitespace either way`() {
assertEquals("user@example.org", normalizeEmailForAccountId(" user@Example.org ", lowercaseLocalPart = false))
assertEquals("user@example.org", normalizeEmailForAccountId(" User@Example.org ", lowercaseLocalPart = true))
}
@Test
fun `normalize maps different casings of one address to the same value`() {
assertEquals(
normalizeEmailForAccountId("User@Gmail.com", lowercaseLocalPart = true),
normalizeEmailForAccountId("user@gmail.com", lowercaseLocalPart = true),
)
}
@Test
fun `normalize handles an address with no at-sign without throwing`() {
// No domain to isolate: lowercased whole for consumer, left as-is for generic.
assertEquals("root", normalizeEmailForAccountId("Root", lowercaseLocalPart = true))
assertEquals("Root", normalizeEmailForAccountId("Root", lowercaseLocalPart = false))
}
@Test
fun `outlook derives a lowercased, deterministic id but keeps the displayed email casing`() {
val account = Account.outlook("User@Outlook.com")
assertEquals("outlook:user@outlook.com", account.id)
assertEquals("User@Outlook.com", account.email)
assertEquals(AuthType.OAUTH_OUTLOOK, account.authType)
// Any casing of the same address resolves to one id, so a re-auth never duplicates the account.
assertEquals(Account.outlook("USER@OUTLOOK.COM").id, account.id)
}
}
@@ -145,11 +145,23 @@ class MailProviderTest {
fun `createAccount trims the email and derives a stable id and display name`() {
val account = MailProvider.GMAIL.createAccount(" User@Gmail.com ")
// The displayed email/name keep the user's casing, but the id lowercases the whole address so
// it is stable across casing (issue #305).
assertEquals("User@Gmail.com", account.email)
assertEquals("imap:User@Gmail.com", account.id)
assertEquals("imap:user@gmail.com", account.id)
assertEquals("User@Gmail.com", account.displayName)
}
@Test
fun `createAccount lowercases the whole address in the id so casing never duplicates an account`() {
// A consumer provider treats the address case-insensitively, so any casing maps to one id.
assertEquals(
MailProvider.GMAIL.createAccount("User@Gmail.com").id,
MailProvider.GMAIL.createAccount("user@gmail.com").id,
)
assertEquals("imap:user@gmail.com", MailProvider.GMAIL.createAccount("USER@GMAIL.COM").id)
}
@Test
fun `createAccount keeps an explicit non-blank display name`() {
val account = MailProvider.GMAIL.createAccount("user@gmail.com", displayName = "Work")
@@ -135,6 +135,25 @@ class ManualSetupViewModelTest {
assertEquals("imap:user@example.org", vm.form.value.addedAccountId)
}
@Test
fun `testAndSave lowercases the domain in the id but keeps the local-part casing`() = runTest(dispatcher) {
// Generic IMAP servers may treat the local part case-sensitively, so only the domain is
// lowercased for the id (issue #305); the displayed email keeps the typed casing.
val repo = mockk<AccountRepository>()
val account = slot<Account>()
coEvery { repo.addImapAccount(capture(account), any()) } returns Result.success(emptyList())
val vm = ManualSetupViewModel(repo)
vm.onEmail(" User@Example.ORG ")
vm.onPassword("secret")
vm.onImapHost("imap.example.org")
vm.onSmtpHost("smtp.example.org")
vm.testAndSave()
assertEquals("imap:User@example.org", account.captured.id)
assertEquals("User@Example.ORG", account.captured.email)
}
@Test
fun `testAndSave falls back to the default ports when the port fields are blank`() = runTest(dispatcher) {
val repo = mockk<AccountRepository>()