Merge pull request #365 from JMR-dev/feat-358-reader-perf-logging
feat(reporting): PII-free latency breadcrumbs on the message-open path (#358)
This commit was merged in pull request #365.
This commit is contained in:
@@ -41,6 +41,7 @@ import org.libremail.data.settings.AccountSettingsRepository
|
||||
import org.libremail.data.settings.SignatureRepository
|
||||
import org.libremail.data.sync.MailConnectionFactory
|
||||
import org.libremail.data.sync.SendScheduler
|
||||
import org.libremail.data.sync.logSafeFolderLabel
|
||||
import org.libremail.domain.model.Attachment
|
||||
import org.libremail.domain.model.Draft
|
||||
import org.libremail.domain.model.Folder
|
||||
@@ -56,6 +57,8 @@ import org.libremail.domain.model.UnreadCount
|
||||
import org.libremail.domain.model.sanitizeAttachmentName
|
||||
import org.libremail.domain.repository.MailRepository
|
||||
import org.libremail.mail.ImapClient
|
||||
import org.libremail.reporting.AppLog
|
||||
import org.libremail.reporting.accountLogRef
|
||||
import java.io.File
|
||||
import java.util.UUID
|
||||
import javax.inject.Inject
|
||||
@@ -153,11 +156,15 @@ class MailRepositoryImpl @Inject constructor(
|
||||
|
||||
override suspend fun openMessage(id: String): Result<Message> = withContext(Dispatchers.IO) {
|
||||
runCatching {
|
||||
// Time the whole open so a debug report shows what the reader's spinner is waiting on — a
|
||||
// cached open is a local read; a first open blocks on the IMAP body fetch below (issue #358).
|
||||
val startNanos = System.nanoTime()
|
||||
// Route on the body-less projection: a cached, already-read message needs no account, no
|
||||
// credentials, and no network, so it skips the Keystore decrypt + DataStore read that
|
||||
// resolving connection params costs (issue #186). Only the fetch / SEEN-push branches below
|
||||
// pull the account and resolve params, and each does so lazily right where it is needed.
|
||||
val routing = messageDao.getRouting(id) ?: error("Message not found")
|
||||
val fetchedBody = !routing.bodyFetched
|
||||
if (!routing.bodyFetched || !routing.isRead) {
|
||||
val account = accountDao.getById(routing.accountId)?.toDomain()
|
||||
if (account != null && !routing.bodyFetched) {
|
||||
@@ -177,7 +184,14 @@ class MailRepositoryImpl @Inject constructor(
|
||||
}
|
||||
}
|
||||
// The single full-body read, reserved for the value the reader actually renders (issue #186).
|
||||
messageDao.getById(id)?.toDomain() ?: error("Message not found")
|
||||
val message = messageDao.getById(id)?.toDomain() ?: error("Message not found")
|
||||
// PII-free: hashed account ref, system-folder label only, plus the branch taken and elapsed ms.
|
||||
AppLog.i(
|
||||
READER_TAG,
|
||||
"openMessage ${accountLogRef(routing.accountId)} folder=${logSafeFolderLabel(routing.folder)} " +
|
||||
"fetchedBody=$fetchedBody took=${(System.nanoTime() - startNanos) / NANOS_PER_MS}ms",
|
||||
)
|
||||
message
|
||||
}
|
||||
}
|
||||
|
||||
@@ -535,6 +549,10 @@ class MailRepositoryImpl @Inject constructor(
|
||||
|
||||
private const val SEARCH_LIMIT = 50
|
||||
|
||||
/** Perf-breadcrumb tag and ns→ms divisor for the reader-open timing (issue #358). */
|
||||
private const val READER_TAG = "MailReader"
|
||||
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
|
||||
|
||||
|
||||
@@ -34,6 +34,7 @@ import org.libremail.domain.model.ImapConnectionParams
|
||||
import org.libremail.domain.model.MailSecurity
|
||||
import org.libremail.reporting.AppLog
|
||||
import java.util.Properties
|
||||
import java.util.concurrent.atomic.AtomicInteger
|
||||
import javax.inject.Inject
|
||||
import javax.inject.Singleton
|
||||
|
||||
@@ -190,7 +191,7 @@ class ImapClient(private val reuseConnections: Boolean) {
|
||||
beforeUid: Long,
|
||||
limit: Int,
|
||||
): List<FetchedMessage> = withContext(Dispatchers.IO) {
|
||||
withStore(params) { store ->
|
||||
withStore(params, op = "backfill-page") { store ->
|
||||
val mailbox = store.getFolder(folder)
|
||||
mailbox.open(Folder.READ_ONLY)
|
||||
try {
|
||||
@@ -270,15 +271,29 @@ class ImapClient(private val reuseConnections: Boolean) {
|
||||
/** Fetches a message body by UID from [folder] and marks it \Seen on the server. */
|
||||
suspend fun fetchBodyMarkingSeen(params: ImapConnectionParams, folder: String, uid: String): MessageContent =
|
||||
withContext(Dispatchers.IO) {
|
||||
withStore(params) { store ->
|
||||
withStore(params, op = "body-fetch") { store ->
|
||||
val mailbox = store.getFolder(folder)
|
||||
val selectStart = System.nanoTime()
|
||||
mailbox.open(Folder.READ_WRITE)
|
||||
val selectMs = (System.nanoTime() - selectStart) / NANOS_PER_MS
|
||||
try {
|
||||
val message = (mailbox as UIDFolder).getMessageByUID(uid.toLong())
|
||||
?: error("Message $uid not found")
|
||||
val fetchStart = System.nanoTime()
|
||||
val content = (extractBody(message) ?: MessageContent("", isHtml = false))
|
||||
.copy(attachments = collectAttachments(message))
|
||||
val fetchMs = (System.nanoTime() - fetchStart) / NANOS_PER_MS
|
||||
// RFC822.SIZE (server-reported wire size) is the download-budget signal; body char
|
||||
// count and attachment count round it out. All are numbers — never message content.
|
||||
val rfc822Bytes = runCatching { message.size }.getOrDefault(-1)
|
||||
val flagStart = System.nanoTime()
|
||||
message.setFlag(Flags.Flag.SEEN, true)
|
||||
val flagMs = (System.nanoTime() - flagStart) / NANOS_PER_MS
|
||||
AppLog.d(
|
||||
PERF_TAG,
|
||||
"body-fetch select=${selectMs}ms body=${fetchMs}ms flag=${flagMs}ms " +
|
||||
"rfc822=${rfc822Bytes}B chars=${content.body.length} att=${content.attachments.size}",
|
||||
)
|
||||
content
|
||||
} finally {
|
||||
runCatching { mailbox.close(false) }
|
||||
@@ -292,7 +307,7 @@ class ImapClient(private val reuseConnections: Boolean) {
|
||||
*/
|
||||
suspend fun fetchBodyPeek(params: ImapConnectionParams, folder: String, uid: String): MessageContent =
|
||||
withContext(Dispatchers.IO) {
|
||||
withStore(params) { store ->
|
||||
withStore(params, op = "prefetch-body") { store ->
|
||||
val mailbox = store.getFolder(folder)
|
||||
mailbox.open(Folder.READ_ONLY)
|
||||
try {
|
||||
@@ -314,7 +329,7 @@ class ImapClient(private val reuseConnections: Boolean) {
|
||||
uid: String,
|
||||
partIndex: Int,
|
||||
): DownloadedAttachment = withContext(Dispatchers.IO) {
|
||||
withStore(params) { store ->
|
||||
withStore(params, op = "attachment") { store ->
|
||||
val mailbox = store.getFolder(folder)
|
||||
mailbox.open(Folder.READ_ONLY)
|
||||
try {
|
||||
@@ -337,7 +352,7 @@ class ImapClient(private val reuseConnections: Boolean) {
|
||||
|
||||
suspend fun setFlag(params: ImapConnectionParams, folder: String, uid: String, flag: Flags.Flag, value: Boolean) =
|
||||
withContext(Dispatchers.IO) {
|
||||
withStore(params) { store ->
|
||||
withStore(params, op = "flag") { store ->
|
||||
val mailbox = store.getFolder(folder)
|
||||
mailbox.open(Folder.READ_WRITE)
|
||||
try {
|
||||
@@ -596,22 +611,44 @@ class ImapClient(private val reuseConnections: Boolean) {
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Live count of connect-per-operation IMAP connections currently open through [withStore] (this
|
||||
* excludes the single long-lived IDLE connection, which does not go through here). Logged per op so
|
||||
* a debug report can show how close we run to a provider's simultaneous-connection ceiling — Gmail
|
||||
* allows 15 — under concurrent backfill + interactive load.
|
||||
*/
|
||||
private val liveConnectionCount = AtomicInteger(0)
|
||||
|
||||
/**
|
||||
* Runs [block] against a connected [Store]. With the reuse flag OFF (default) this is the original
|
||||
* behaviour: a fresh, authenticated connection per call, torn down in `finally`. With it ON, the
|
||||
* call borrows a kept-alive per-account connection from [connectionCache] (established once, reused
|
||||
* across folder-opens) instead — see issue #125.
|
||||
*
|
||||
* [op] is a short, PII-free label for the caller's intent (e.g. `body-fetch`, `backfill-page`) used
|
||||
* only in the perf breadcrumb below, so a debug report can attribute latency to connection
|
||||
* establishment (`connect`) vs the operation's own server work (`work`).
|
||||
*/
|
||||
private suspend fun <T> withStore(params: ImapConnectionParams, block: (Store) -> T): T {
|
||||
private suspend fun <T> withStore(params: ImapConnectionParams, op: String = "imap", block: (Store) -> T): T {
|
||||
val cache = connectionCache
|
||||
return if (cache != null) {
|
||||
cache.withStore(params, block)
|
||||
} else {
|
||||
// Time CONNECT + TLS + LOGIN separately from the op's own work, and record how many
|
||||
// connect-per-op sockets are live at once, so a slow op can be attributed and the provider
|
||||
// connection ceiling observed. Logged in `finally` so a thrown/timed-out op is captured too.
|
||||
val connectStart = System.nanoTime()
|
||||
val store = openConnectedStore(params)
|
||||
val connectMs = (System.nanoTime() - connectStart) / NANOS_PER_MS
|
||||
val live = liveConnectionCount.incrementAndGet()
|
||||
val workStart = System.nanoTime()
|
||||
try {
|
||||
block(store)
|
||||
} finally {
|
||||
val workMs = (System.nanoTime() - workStart) / NANOS_PER_MS
|
||||
AppLog.d(PERF_TAG, "$op connect=${connectMs}ms work=${workMs}ms live=$live")
|
||||
runCatching { store.close() }
|
||||
liveConnectionCount.decrementAndGet()
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -664,6 +701,8 @@ class ImapClient(private val reuseConnections: Boolean) {
|
||||
private companion object {
|
||||
const val TIMEOUT_MS = "15000"
|
||||
const val TAG = "LibreMailIdle"
|
||||
const val PERF_TAG = "ImapPerf"
|
||||
const val NANOS_PER_MS = 1_000_000L
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -19,6 +19,7 @@ import org.libremail.domain.model.InlineImage
|
||||
import org.libremail.domain.model.Message
|
||||
import org.libremail.domain.model.ReplyMode
|
||||
import org.libremail.domain.repository.MailRepository
|
||||
import org.libremail.reporting.AppLog
|
||||
import org.libremail.ui.navigation.Routes
|
||||
import java.io.File
|
||||
import javax.inject.Inject
|
||||
@@ -74,6 +75,10 @@ class ReaderViewModel @Inject constructor(
|
||||
}
|
||||
}
|
||||
viewModelScope.launch {
|
||||
// Time the spinner: from launch to the state update that clears `loading` — what the user
|
||||
// actually waits through. Covers openMessage (first-open body fetch) plus inline-image
|
||||
// resolution, so a slow render can be split from a slow open in a debug report (issue #358).
|
||||
val startNanos = System.nanoTime()
|
||||
repository.openMessage(messageId).fold(
|
||||
onSuccess = { message ->
|
||||
// Resolve inline cid: images BEFORE the first render and publish them in the SAME
|
||||
@@ -86,6 +91,11 @@ class ReaderViewModel @Inject constructor(
|
||||
emptyMap()
|
||||
}
|
||||
_state.update { it.copy(loading = false, message = message, inlineImages = images) }
|
||||
AppLog.d(
|
||||
READER_TAG,
|
||||
"reader ready took=${(System.nanoTime() - startNanos) / NANOS_PER_MS}ms " +
|
||||
"html=${message.isHtml} inline=${images.size}",
|
||||
)
|
||||
},
|
||||
onFailure = { e ->
|
||||
_state.update {
|
||||
@@ -95,6 +105,10 @@ class ReaderViewModel @Inject constructor(
|
||||
e.message ?: "Could not load message",
|
||||
)
|
||||
}
|
||||
AppLog.w(
|
||||
READER_TAG,
|
||||
"reader load failed took=${(System.nanoTime() - startNanos) / NANOS_PER_MS}ms",
|
||||
)
|
||||
},
|
||||
)
|
||||
}
|
||||
@@ -155,4 +169,9 @@ class ReaderViewModel @Inject constructor(
|
||||
_state.update { it.copy(deleted = true) }
|
||||
}
|
||||
}
|
||||
|
||||
private companion object {
|
||||
const val READER_TAG = "Reader"
|
||||
const val NANOS_PER_MS = 1_000_000L
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ package org.libremail.data.repository
|
||||
import android.content.ContentResolver
|
||||
import android.content.Context
|
||||
import android.net.Uri
|
||||
import android.util.Log
|
||||
import app.cash.turbine.test
|
||||
import io.mockk.Runs
|
||||
import io.mockk.coEvery
|
||||
@@ -19,6 +20,7 @@ import jakarta.mail.Flags
|
||||
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.attachment.AttachmentUriGrants
|
||||
import org.libremail.data.local.dao.AccountDao
|
||||
@@ -97,6 +99,17 @@ class MailRepositoryImplCoverageTest {
|
||||
attachmentUriGrants = mockk<AttachmentUriGrants>(relaxed = true),
|
||||
)
|
||||
|
||||
// openMessage now breadcrumbs via AppLog (issue #358); android.util.Log is a no-op stub under plain
|
||||
// JVM tests, so mock it class-wide so no test crashes on the unmocked method.
|
||||
@Before
|
||||
fun setUp() {
|
||||
mockkStatic(Log::class)
|
||||
every { Log.d(any(), any()) } returns 0
|
||||
every { Log.i(any(), any()) } returns 0
|
||||
every { Log.w(any<String>(), any<String>()) } returns 0
|
||||
every { Log.e(any(), any(), any()) } returns 0
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() = unmockkAll()
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
package org.libremail.data.repository
|
||||
|
||||
import android.content.Context
|
||||
import android.util.Log
|
||||
import androidx.paging.PagingSource
|
||||
import androidx.paging.PagingState
|
||||
import androidx.paging.testing.asSnapshot
|
||||
@@ -12,13 +13,17 @@ 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 jakarta.mail.Flags
|
||||
import kotlinx.coroutines.CompletableDeferred
|
||||
import kotlinx.coroutines.flow.flowOf
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import kotlinx.coroutines.withTimeout
|
||||
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.AttachmentDao
|
||||
@@ -47,6 +52,9 @@ import org.libremail.mail.FetchedFolder
|
||||
import org.libremail.mail.ImapClient
|
||||
import org.libremail.mail.MessageContent
|
||||
import org.libremail.mail.ReplyContext
|
||||
import org.libremail.reporting.AppLog
|
||||
import org.libremail.reporting.RingLogBuffer
|
||||
import org.libremail.reporting.accountLogRef
|
||||
import java.io.File
|
||||
import java.nio.file.Files
|
||||
import kotlin.test.assertEquals
|
||||
@@ -83,6 +91,20 @@ class MailRepositoryImplTest {
|
||||
attachmentUriGrants = mockk(relaxed = true),
|
||||
)
|
||||
|
||||
// openMessage now breadcrumbs via AppLog (issue #358); android.util.Log is a no-op stub under plain
|
||||
// JVM tests, so mock it class-wide so no test crashes on the unmocked method.
|
||||
@Before
|
||||
fun setUp() {
|
||||
mockkStatic(Log::class)
|
||||
every { Log.d(any(), any()) } returns 0
|
||||
every { Log.i(any(), any()) } returns 0
|
||||
every { Log.w(any<String>(), any<String>()) } returns 0
|
||||
every { Log.e(any(), any(), any()) } returns 0
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() = unmockkAll()
|
||||
|
||||
@Test
|
||||
fun `pagedUnifiedFolderMessages maps the paged summaries to domain messages`() = runTest {
|
||||
every { messageDao.pagingUnifiedFolderSummaries("INBOX") } returns FakeSummaryPagingSource(
|
||||
@@ -214,6 +236,30 @@ class MailRepositoryImplTest {
|
||||
coVerify { imapClient.fetchBodyMarkingSeen(any(), "Archive", "5") }
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `openMessage breadcrumbs a PII-free open via the account ref and folder label`() = runTest {
|
||||
val buffer = RingLogBuffer()
|
||||
AppLog.install(buffer)
|
||||
val id = "acct:INBOX:7"
|
||||
coEvery { messageDao.getRouting(id) } returns messageRouting(id, "INBOX")
|
||||
coEvery { messageDao.getById(id) } returns messageEntity(id, "INBOX")
|
||||
coEvery { accountDao.getById("acct") } returns accountEntity()
|
||||
coEvery { connectionFactory.imapParamsFor(any()) } returns imapParams()
|
||||
coEvery { imapClient.fetchBodyMarkingSeen(any(), "INBOX", "7") } returns
|
||||
MessageContent("Body text", isHtml = false)
|
||||
coEvery { messageDao.updateBody(id, any(), any(), any()) } just Runs
|
||||
coEvery { messageDao.setRead(id, true) } just Runs
|
||||
|
||||
repository.openMessage(id)
|
||||
|
||||
val breadcrumb = buffer.snapshot().map { it.message }.single { it.startsWith("openMessage ") }
|
||||
// Account via the hashed accountLogRef (never the raw id), system folder by name, branch + timing.
|
||||
assertTrue(breadcrumb.contains(accountLogRef("acct")), breadcrumb)
|
||||
assertTrue(breadcrumb.contains("folder=INBOX"), breadcrumb)
|
||||
assertTrue(breadcrumb.contains("fetchedBody=true"), breadcrumb)
|
||||
assertTrue(breadcrumb.contains("took="), breadcrumb)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `openMessage derives a readable plain-text snippet from an HTML body`() = runTest {
|
||||
val id = "acct:INBOX:20"
|
||||
|
||||
@@ -3,6 +3,9 @@ package org.libremail.mail
|
||||
|
||||
import com.icegreen.greenmail.util.GreenMail
|
||||
import com.icegreen.greenmail.util.ServerSetupTest
|
||||
import io.mockk.every
|
||||
import io.mockk.mockkStatic
|
||||
import io.mockk.unmockkAll
|
||||
import jakarta.mail.Folder
|
||||
import jakarta.mail.Message
|
||||
import jakarta.mail.Session
|
||||
@@ -34,10 +37,21 @@ class ImapClientBackfillTest {
|
||||
greenMail = GreenMail(ServerSetupTest.SMTP_IMAP)
|
||||
greenMail.start()
|
||||
greenMail.setUser("alice@example.org", "secret")
|
||||
|
||||
// Every IMAP op now breadcrumbs through AppLog (per-op connect/work timings, issue #358), and
|
||||
// android.util.Log is a no-op stub under plain JVM tests. Mock it class-wide — fully qualified so
|
||||
// this file still never imports android.util.Log — so no test crashes on the unmocked method.
|
||||
mockkStatic(android.util.Log::class)
|
||||
every { android.util.Log.d(any(), any()) } returns 0
|
||||
every { android.util.Log.i(any(), any()) } returns 0
|
||||
every { android.util.Log.w(any<String>(), any<String>()) } returns 0
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() = greenMail.stop()
|
||||
fun tearDown() {
|
||||
greenMail.stop()
|
||||
unmockkAll()
|
||||
}
|
||||
|
||||
private fun params() = ImapConnectionParams(
|
||||
host = "127.0.0.1",
|
||||
|
||||
@@ -49,6 +49,14 @@ class ImapClientTest {
|
||||
greenMail = GreenMail(ServerSetupTest.SMTP_IMAP)
|
||||
greenMail.start()
|
||||
greenMail.setUser("alice@example.org", "secret")
|
||||
|
||||
// Every IMAP op now breadcrumbs through AppLog (per-op connect/work timings, issue #358), and
|
||||
// android.util.Log is a no-op stub under plain JVM tests. Mock it class-wide — fully qualified so
|
||||
// this file still never imports android.util.Log — so no test crashes on the unmocked method.
|
||||
mockkStatic(android.util.Log::class)
|
||||
every { android.util.Log.d(any(), any()) } returns 0
|
||||
every { android.util.Log.i(any(), any()) } returns 0
|
||||
every { android.util.Log.w(any<String>(), any<String>()) } returns 0
|
||||
}
|
||||
|
||||
@After
|
||||
@@ -102,6 +110,35 @@ class ImapClientTest {
|
||||
assertTrue(client.fetchRecent(params(), "INBOX", limit = 50).first().isRead, "should be marked read")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `fetchBodyMarkingSeen breadcrumbs connect and phase timings without leaking PII`() = runTest {
|
||||
GreenMailUtil.sendTextEmailTest("alice@example.org", "bob@example.org", "Secret subject", "Body.")
|
||||
greenMail.waitForIncomingEmail(1)
|
||||
val uid = client.fetchRecent(params(), "INBOX", limit = 50).first().uid
|
||||
val buffer = RingLogBuffer()
|
||||
AppLog.install(buffer)
|
||||
|
||||
client.fetchBodyMarkingSeen(params(), "INBOX", uid)
|
||||
|
||||
val messages = buffer.snapshot().map { it.message }
|
||||
// withStore labels the op and splits connect (CONNECT+TLS+LOGIN) from work — timings only.
|
||||
assertTrue(
|
||||
messages.any { it.startsWith("body-fetch connect=") && it.contains("work=") && it.contains("live=") },
|
||||
"messages=$messages",
|
||||
)
|
||||
// fetchBodyMarkingSeen adds the select/body/flag phase split plus PII-free size counts.
|
||||
assertTrue(
|
||||
messages.any { it.startsWith("body-fetch select=") && it.contains("rfc822=") && it.contains("att=") },
|
||||
"messages=$messages",
|
||||
)
|
||||
// Breadcrumbs are numbers and fixed labels only — never the subject, sender, or recipient.
|
||||
messages.forEach { message ->
|
||||
assertFalse(message.contains("Secret subject"), message)
|
||||
assertFalse(message.contains("bob@example.org"), message)
|
||||
assertFalse(message.contains("alice@example.org"), message)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `search returns only messages matching the query`() = runTest {
|
||||
GreenMailUtil.sendTextEmailTest("alice@example.org", "bob@example.org", "Vacation plans", "Beach trip")
|
||||
|
||||
@@ -4,6 +4,9 @@ package org.libremail.mail
|
||||
import com.icegreen.greenmail.util.GreenMail
|
||||
import com.icegreen.greenmail.util.GreenMailUtil
|
||||
import com.icegreen.greenmail.util.ServerSetupTest
|
||||
import io.mockk.every
|
||||
import io.mockk.mockkStatic
|
||||
import io.mockk.unmockkAll
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import kotlinx.coroutines.test.runTest
|
||||
import org.junit.After
|
||||
@@ -48,6 +51,14 @@ class ImapFolderOpenLatencyTest {
|
||||
greenMail.setUser("alice@example.org", "secret")
|
||||
// All IMAP traffic goes through the proxy so we can count it; the proxy forwards to GreenMail.
|
||||
proxy = CountingImapProxy(backendHost = "127.0.0.1", backendPort = greenMail.imap.port)
|
||||
|
||||
// Every IMAP op now breadcrumbs through AppLog (per-op connect/work timings, issue #358), and
|
||||
// android.util.Log is a no-op stub under plain JVM tests. Mock it class-wide — fully qualified so
|
||||
// this file still never imports android.util.Log — so no test crashes on the unmocked method.
|
||||
mockkStatic(android.util.Log::class)
|
||||
every { android.util.Log.d(any(), any()) } returns 0
|
||||
every { android.util.Log.i(any(), any()) } returns 0
|
||||
every { android.util.Log.w(any<String>(), any<String>()) } returns 0
|
||||
}
|
||||
|
||||
@After
|
||||
@@ -55,6 +66,7 @@ class ImapFolderOpenLatencyTest {
|
||||
runBlocking { reuseClient.closeReusedConnections() } // release any kept-alive socket before the server stops
|
||||
proxy.close()
|
||||
greenMail.stop()
|
||||
unmockkAll()
|
||||
}
|
||||
|
||||
/** Points [ImapClient] at the counting proxy rather than directly at GreenMail. */
|
||||
|
||||
@@ -1,12 +1,15 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.ui.reader
|
||||
|
||||
import android.util.Log
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import app.cash.turbine.test
|
||||
import io.mockk.coEvery
|
||||
import io.mockk.coVerify
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import io.mockk.mockkStatic
|
||||
import io.mockk.unmockkAll
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.flow.flowOf
|
||||
@@ -44,10 +47,22 @@ class ReaderViewModelActionsTest {
|
||||
)
|
||||
|
||||
@Before
|
||||
fun setUp() = Dispatchers.setMain(dispatcher)
|
||||
fun setUp() {
|
||||
Dispatchers.setMain(dispatcher)
|
||||
// ReaderViewModel breadcrumbs open latency via AppLog on init (issue #358); android.util.Log is a
|
||||
// no-op stub under plain JVM tests, so mock it class-wide so VM construction never crashes.
|
||||
mockkStatic(Log::class)
|
||||
every { Log.d(any(), any()) } returns 0
|
||||
every { Log.i(any(), any()) } returns 0
|
||||
every { Log.w(any<String>(), any<String>()) } returns 0
|
||||
every { Log.e(any(), any(), any()) } returns 0
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() = Dispatchers.resetMain()
|
||||
fun tearDown() {
|
||||
Dispatchers.resetMain()
|
||||
unmockkAll()
|
||||
}
|
||||
|
||||
private fun attachment(partIndex: Int) =
|
||||
Attachment(messageId, partIndex, "file$partIndex.bin", "application/octet-stream", 10L)
|
||||
|
||||
@@ -1,11 +1,14 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.ui.reader
|
||||
|
||||
import android.util.Log
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import io.mockk.coEvery
|
||||
import io.mockk.coVerify
|
||||
import io.mockk.every
|
||||
import io.mockk.mockk
|
||||
import io.mockk.mockkStatic
|
||||
import io.mockk.unmockkAll
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.flow.first
|
||||
@@ -25,6 +28,8 @@ import org.libremail.domain.model.InlineImage
|
||||
import org.libremail.domain.model.Message
|
||||
import org.libremail.domain.model.ReplyMode
|
||||
import org.libremail.domain.repository.MailRepository
|
||||
import org.libremail.reporting.AppLog
|
||||
import org.libremail.reporting.RingLogBuffer
|
||||
import org.libremail.ui.navigation.Routes
|
||||
import java.io.File
|
||||
import kotlin.test.assertEquals
|
||||
@@ -43,10 +48,22 @@ class ReaderViewModelTest {
|
||||
)
|
||||
|
||||
@Before
|
||||
fun setUp() = Dispatchers.setMain(dispatcher)
|
||||
fun setUp() {
|
||||
Dispatchers.setMain(dispatcher)
|
||||
// ReaderViewModel breadcrumbs open latency via AppLog on init (issue #358); android.util.Log is a
|
||||
// no-op stub under plain JVM tests, so mock it class-wide so VM construction never crashes.
|
||||
mockkStatic(Log::class)
|
||||
every { Log.d(any(), any()) } returns 0
|
||||
every { Log.i(any(), any()) } returns 0
|
||||
every { Log.w(any<String>(), any<String>()) } returns 0
|
||||
every { Log.e(any(), any(), any()) } returns 0
|
||||
}
|
||||
|
||||
@After
|
||||
fun tearDown() = Dispatchers.resetMain()
|
||||
fun tearDown() {
|
||||
Dispatchers.resetMain()
|
||||
unmockkAll()
|
||||
}
|
||||
|
||||
private fun attachment(partIndex: Int) =
|
||||
Attachment(messageId, partIndex, "file$partIndex.bin", "application/octet-stream", 10L)
|
||||
@@ -175,4 +192,23 @@ class ReaderViewModelTest {
|
||||
assertEquals(ReaderEvent.ComposeFailed("boom"), vm.events.first())
|
||||
assertEquals(false, vm.state.value.composing)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a successful load breadcrumbs the reader-ready latency`() = runTest(dispatcher) {
|
||||
val buffer = RingLogBuffer()
|
||||
AppLog.install(buffer)
|
||||
val repo = mockk<MailRepository>(relaxed = true)
|
||||
coEvery { repo.openMessage(messageId) } returns Result.success(message)
|
||||
every { repo.observeAttachments(messageId) } returns flowOf(emptyList())
|
||||
coEvery { repo.downloadedAttachmentParts(messageId) } returns emptySet()
|
||||
|
||||
viewModel(repo)
|
||||
advanceUntilIdle()
|
||||
|
||||
val messages = buffer.snapshot().map { it.message }
|
||||
assertTrue(
|
||||
messages.any { it.startsWith("reader ready took=") && it.contains("html=") },
|
||||
"messages=$messages",
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -22,6 +22,13 @@ complexity:
|
||||
allowedFunctionsPerFile: 40
|
||||
allowedFunctionsPerClass: 40
|
||||
allowedFunctionsPerInterface: 40
|
||||
LargeClass:
|
||||
# MailRepositoryImplTest is one cohesive single-SUT suite (a test per MailRepositoryImpl method
|
||||
# plus folder-resolution / spam cases) already at detekt's LLOC boundary; the reader-path perf
|
||||
# logging (issue #358) added its required android.util.Log mock + one breadcrumb test, tipping it
|
||||
# over. Excluded rather than artificially split — same "operation-rich cohesive suite" rationale as
|
||||
# the TooManyFunctions relaxation above.
|
||||
excludes: ['**/data/repository/MailRepositoryImplTest.kt']
|
||||
|
||||
naming:
|
||||
FunctionNaming:
|
||||
@@ -63,6 +70,12 @@ style:
|
||||
- '**/data/sync/MailSyncConcurrencyTest.kt'
|
||||
- '**/data/sync/PruneWorkerTest.kt'
|
||||
- '**/data/sync/BackfillWorkerTest.kt'
|
||||
# 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'
|
||||
- '**/data/repository/MailRepositoryImplCoverageTest.kt'
|
||||
- '**/ui/reader/ReaderViewModelTest.kt'
|
||||
- '**/ui/reader/ReaderViewModelActionsTest.kt'
|
||||
MagicNumber:
|
||||
# dp / sp / duration literals are idiomatic inline in Compose.
|
||||
ignoreAnnotated: ['Composable']
|
||||
|
||||
Reference in New Issue
Block a user