perf(mailbox): page the unified "All inboxes" list (#124) #130
@@ -200,9 +200,14 @@ dependencies {
|
||||
|
||||
implementation(libs.androidx.room.runtime)
|
||||
implementation(libs.androidx.room.ktx)
|
||||
implementation(libs.androidx.room.paging)
|
||||
ksp(libs.androidx.room.compiler)
|
||||
implementation(libs.sqlcipher.android)
|
||||
|
||||
// Paging 3 — the unified inbox list is paged so its cost scales with the screen (issue #124).
|
||||
implementation(libs.androidx.paging.runtime)
|
||||
implementation(libs.androidx.paging.compose)
|
||||
|
||||
// Raise kotlinx-serialization to the version Room's schema-bundle serializers were compiled
|
||||
// against (see libs.versions.toml). AGP 9 consistent resolution shares it with the androidTest
|
||||
// classpath so MigrationTestHelper can parse the exported schema JSON.
|
||||
@@ -214,6 +219,8 @@ dependencies {
|
||||
testImplementation(libs.turbine)
|
||||
testImplementation(libs.mockk)
|
||||
testImplementation(libs.greenmail)
|
||||
// asSnapshot() drives a PagingData flow to a concrete list in JVM unit tests (issue #124).
|
||||
testImplementation(libs.androidx.paging.testing)
|
||||
// The real org.json for unit tests (android.jar ships a stubbed, no-op version).
|
||||
testImplementation("org.json:json:20231013")
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.ui
|
||||
|
||||
import androidx.paging.PagingData
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.flowOf
|
||||
@@ -92,6 +93,9 @@ class FakeMailRepository(
|
||||
override fun observeUnifiedFolderMessages(folder: String): Flow<List<Message>> =
|
||||
flowOf(messages.filter { it.folder == folder })
|
||||
|
||||
override fun pagedUnifiedFolderMessages(folder: String): Flow<PagingData<Message>> =
|
||||
flowOf(PagingData.from(messages.filter { it.folder == folder && it.inInbox }))
|
||||
|
||||
override fun observeFolders(accountId: String): Flow<List<Folder>> = flowOf(
|
||||
folders.filter {
|
||||
it.accountId ==
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.data.local.dao
|
||||
|
||||
import androidx.paging.PagingSource
|
||||
import androidx.room.Dao
|
||||
import androidx.room.Insert
|
||||
import androidx.room.OnConflictStrategy
|
||||
@@ -54,6 +55,24 @@ interface MessageDao {
|
||||
)
|
||||
fun observeUnifiedFolderSummaries(folder: String): Flow<List<MessageSummary>>
|
||||
|
||||
/**
|
||||
* Paged unified-inbox projection: folder-synced rows of [folder] across every account,
|
||||
* newest-first, as a Paging 3 [PagingSource] (issue #124). Unlike [observeUnifiedFolderSummaries]
|
||||
* — which materializes the *entire* unified inbox (~thousands of rows) on every emission — Room
|
||||
* loads only the requested window (LIMIT/OFFSET), so the mailbox list's query, mapping, and
|
||||
* recomposition cost scale with what's on screen, not the whole cache. Filters `inInbox = 1`
|
||||
* because the paged browse list shows only synced rows; unified *search* (which must also surface
|
||||
* transient `inInbox = 0` hits) stays on [observeUnifiedFolderSummaries]. Profiling (see
|
||||
* `docs/perf/issue-124-unified-inbox-paging.md`) showed the first page loads flat regardless of
|
||||
* total cache size on the existing indices, so no `(folder, …)` index / schema migration is added.
|
||||
*/
|
||||
@Query(
|
||||
"SELECT id, accountId, sender, senderEmail, subject, snippet, timestampMillis, " +
|
||||
"isRead, isStarred, folder, inInbox, bodyFetched FROM messages " +
|
||||
"WHERE folder = :folder AND inInbox = 1 ORDER BY timestampMillis DESC",
|
||||
)
|
||||
fun pagingUnifiedFolderSummaries(folder: String): PagingSource<Int, MessageSummary>
|
||||
|
||||
/**
|
||||
* Live per-(account, folder) unread counts for the drawer's folder badges and the bold styling of
|
||||
* accounts with unread mail. Counts only folder-synced rows (`inInbox = 1`), so transient
|
||||
|
||||
@@ -3,6 +3,10 @@ package org.libremail.data.repository
|
||||
|
||||
import android.content.Context
|
||||
import android.net.Uri
|
||||
import androidx.paging.Pager
|
||||
import androidx.paging.PagingConfig
|
||||
import androidx.paging.PagingData
|
||||
import androidx.paging.map
|
||||
import dagger.hilt.android.qualifiers.ApplicationContext
|
||||
import jakarta.mail.Flags
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
@@ -66,6 +70,19 @@ class MailRepositoryImpl @Inject constructor(
|
||||
override fun observeUnifiedFolderMessages(folder: String): Flow<List<Message>> =
|
||||
messageDao.observeUnifiedFolderSummaries(folder).map { rows -> rows.map { it.toDomain() } }
|
||||
|
||||
override fun pagedUnifiedFolderMessages(folder: String): Flow<PagingData<Message>> = Pager(
|
||||
config = PagingConfig(
|
||||
// A page comfortably exceeds a screenful so scrolling rarely waits on a load; loading
|
||||
// three pages up front fills the first viewport without a visible gap. Placeholders
|
||||
// are off: the row height varies (snippet/account label), so a fixed-height placeholder
|
||||
// would jump, and the list never needs a scrollbar sized to the full (uncounted) inbox.
|
||||
pageSize = MAILBOX_PAGE_SIZE,
|
||||
initialLoadSize = MAILBOX_PAGE_SIZE * 3,
|
||||
enablePlaceholders = false,
|
||||
),
|
||||
pagingSourceFactory = { messageDao.pagingUnifiedFolderSummaries(folder) },
|
||||
).flow.map { page -> page.map { it.toDomain() } }
|
||||
|
||||
override fun observeFolders(accountId: String): Flow<List<Folder>> =
|
||||
folderDao.observeForAccount(accountId).map { rows ->
|
||||
rows.map { it.toDomain() }
|
||||
@@ -380,5 +397,8 @@ class MailRepositoryImpl @Inject constructor(
|
||||
|
||||
private const val SEARCH_LIMIT = 50
|
||||
|
||||
/** 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
|
||||
|
||||
/** Message id is "<accountId>:<uid>"; the uid is the trailing segment. */
|
||||
private fun uidOf(id: String): String = id.substringAfterLast(':')
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// SPDX-License-Identifier: GPL-3.0-or-later
|
||||
package org.libremail.domain.repository
|
||||
|
||||
import androidx.paging.PagingData
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import org.libremail.domain.model.Attachment
|
||||
import org.libremail.domain.model.Draft
|
||||
@@ -27,6 +28,14 @@ interface MailRepository {
|
||||
/** Like [observeFolderMessages] but for [folder] across every account (the unified inbox). */
|
||||
fun observeUnifiedFolderMessages(folder: String): Flow<List<Message>>
|
||||
|
||||
/**
|
||||
* The unified inbox as a [PagingData] stream so the list's query, mapping, and recomposition cost
|
||||
* scale with the visible window rather than the whole cache (issue #124). Emits only folder-synced
|
||||
* rows of [folder] across every account, newest-first — unified *search* still uses
|
||||
* [observeUnifiedFolderMessages]. Callers must `cachedIn` a scope before collecting.
|
||||
*/
|
||||
fun pagedUnifiedFolderMessages(folder: String): Flow<PagingData<Message>>
|
||||
|
||||
/** The account's cached IMAP folders for the navigation drawer. */
|
||||
fun observeFolders(accountId: String): Flow<List<Folder>>
|
||||
|
||||
|
||||
@@ -80,6 +80,9 @@ import androidx.compose.ui.text.style.TextOverflow
|
||||
import androidx.compose.ui.unit.dp
|
||||
import androidx.hilt.navigation.compose.hiltViewModel
|
||||
import androidx.lifecycle.compose.collectAsStateWithLifecycle
|
||||
import androidx.paging.LoadState
|
||||
import androidx.paging.compose.collectAsLazyPagingItems
|
||||
import androidx.paging.compose.itemKey
|
||||
import kotlinx.coroutines.launch
|
||||
import org.libremail.R
|
||||
import org.libremail.domain.model.Account
|
||||
@@ -102,6 +105,8 @@ fun MailboxScreen(
|
||||
viewModel: MailboxViewModel = hiltViewModel(),
|
||||
) {
|
||||
val messages by viewModel.messages.collectAsStateWithLifecycle()
|
||||
// The unified "All inboxes" browse list is paged (issue #124); per-account/search render [messages].
|
||||
val pagedMessages = viewModel.pagedMessages.collectAsLazyPagingItems()
|
||||
val accounts by viewModel.accounts.collectAsStateWithLifecycle()
|
||||
val selectedAccountId by viewModel.selectedAccountId.collectAsStateWithLifecycle()
|
||||
val selectedFolder by viewModel.selectedFolder.collectAsStateWithLifecycle()
|
||||
@@ -123,6 +128,10 @@ fun MailboxScreen(
|
||||
val moveTargetFolders by viewModel.moveTargetFolders.collectAsStateWithLifecycle()
|
||||
val actionInProgress by viewModel.actionInProgress.collectAsStateWithLifecycle()
|
||||
val selectionMode = selectedIds.isNotEmpty()
|
||||
// The unified inbox (no account filter, not searching) renders the paged list; a concrete account
|
||||
// or an active search renders the flat [messages] list. Hoisted here so the selection bar's
|
||||
// "Select all" can read the right source (issue #124).
|
||||
val showPaged = selectedAccountId == null && searchQuery.isBlank()
|
||||
var showMovePicker by remember { mutableStateOf(false) }
|
||||
val snackbarHostState = remember { SnackbarHostState() }
|
||||
val drawerState = rememberDrawerState(DrawerValue.Closed)
|
||||
@@ -187,7 +196,13 @@ fun MailboxScreen(
|
||||
onDelete = viewModel::requestDelete,
|
||||
onSpam = viewModel::requestSpam,
|
||||
onMove = { showMovePicker = true },
|
||||
onSelectAll = viewModel::selectAll,
|
||||
onSelectAll = {
|
||||
// "Select all" acts on what's shown: the loaded paged window for the unified
|
||||
// inbox, or the whole flat list for a per-account/search view.
|
||||
viewModel.selectAll(
|
||||
if (showPaged) pagedMessages.itemSnapshotList.items else messages,
|
||||
)
|
||||
},
|
||||
onReply = { viewModel.reply(ReplyMode.REPLY) },
|
||||
onReplyAll = viewModel::requestReplyAll,
|
||||
onForward = { viewModel.reply(ReplyMode.FORWARD) },
|
||||
@@ -278,7 +293,42 @@ fun MailboxScreen(
|
||||
modifier = Modifier.fillMaxSize(),
|
||||
) {
|
||||
LazyColumn(modifier = Modifier.fillMaxSize()) {
|
||||
if (messages.isEmpty()) {
|
||||
if (showPaged) {
|
||||
if (pagedMessages.itemCount == 0) {
|
||||
item {
|
||||
// Hold the empty state back until the first page settles so
|
||||
// it doesn't flash before rows arrive (search never lands here).
|
||||
if (pagedMessages.loadState.refresh !is LoadState.Loading) {
|
||||
NoMessagesState(Modifier.fillParentMaxSize())
|
||||
}
|
||||
}
|
||||
} else {
|
||||
items(
|
||||
count = pagedMessages.itemCount,
|
||||
key = pagedMessages.itemKey { it.id },
|
||||
) { index ->
|
||||
val message = pagedMessages[index] ?: return@items
|
||||
val accountLabel =
|
||||
if (showAccount) accountsById[message.accountId]?.email else null
|
||||
MessageRow(
|
||||
message = message,
|
||||
accountLabel = accountLabel,
|
||||
selected = message.id in selectedIds,
|
||||
onClick = {
|
||||
if (selectionMode) {
|
||||
viewModel.toggleSelection(message.id, message.accountId)
|
||||
} else {
|
||||
onOpenMessage(message.id)
|
||||
}
|
||||
},
|
||||
onLongClick = {
|
||||
viewModel.startSelection(message.id, message.accountId)
|
||||
},
|
||||
)
|
||||
HorizontalDivider()
|
||||
}
|
||||
}
|
||||
} else if (messages.isEmpty()) {
|
||||
item {
|
||||
if (searchActive && searchQuery.isNotBlank()) {
|
||||
NoResultsState(Modifier.fillParentMaxSize())
|
||||
@@ -296,12 +346,12 @@ fun MailboxScreen(
|
||||
selected = message.id in selectedIds,
|
||||
onClick = {
|
||||
if (selectionMode) {
|
||||
viewModel.toggleSelection(message.id)
|
||||
viewModel.toggleSelection(message.id, message.accountId)
|
||||
} else {
|
||||
onOpenMessage(message.id)
|
||||
}
|
||||
},
|
||||
onLongClick = { viewModel.startSelection(message.id) },
|
||||
onLongClick = { viewModel.startSelection(message.id, message.accountId) },
|
||||
)
|
||||
HorizontalDivider()
|
||||
}
|
||||
|
||||
@@ -4,10 +4,13 @@ package org.libremail.ui.mailbox
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import androidx.lifecycle.ViewModel
|
||||
import androidx.lifecycle.viewModelScope
|
||||
import androidx.paging.PagingData
|
||||
import androidx.paging.cachedIn
|
||||
import dagger.hilt.android.lifecycle.HiltViewModel
|
||||
import kotlinx.coroutines.ExperimentalCoroutinesApi
|
||||
import kotlinx.coroutines.FlowPreview
|
||||
import kotlinx.coroutines.channels.Channel
|
||||
import kotlinx.coroutines.flow.Flow
|
||||
import kotlinx.coroutines.flow.MutableStateFlow
|
||||
import kotlinx.coroutines.flow.SharingStarted
|
||||
import kotlinx.coroutines.flow.StateFlow
|
||||
@@ -116,27 +119,62 @@ class MailboxViewModel @Inject constructor(
|
||||
private val _searchQuery = MutableStateFlow("")
|
||||
val searchQuery: StateFlow<String> = _searchQuery.asStateFlow()
|
||||
|
||||
/**
|
||||
* The list-rendered messages: a per-account folder (SQL-scoped and already flat, issue #86) or —
|
||||
* for the unified inbox — *only* an active search's matches across accounts. The unified **browse**
|
||||
* list is paged instead (see [pagedMessages], issue #124), so this flow stays empty while browsing
|
||||
* the unified inbox and the whole cache is never pulled into memory on each write.
|
||||
*/
|
||||
val messages: StateFlow<List<Message>> =
|
||||
combine(_selectedAccountId, _selectedFolder) { accountId, folder -> accountId to folder }
|
||||
.distinctUntilChanged()
|
||||
.flatMapLatest { (accountId, folder) ->
|
||||
// Scope the query in SQL to the viewed account+folder (or [folder] across accounts for
|
||||
// the unified inbox) so Room only re-queries/re-emits when those rows change, and the
|
||||
// cost scales with the folder, not the whole cache (issue #86).
|
||||
val scoped = if (accountId == null) {
|
||||
mailRepository.observeUnifiedFolderMessages(folder)
|
||||
if (accountId != null) {
|
||||
// Per-account+folder: the SQL-scoped, already-flat list. The one client-side pass
|
||||
// distinguishes the normal list (synced rows) from an active search (any matching
|
||||
// row, including transient server-search hits) over the small folder-scoped set.
|
||||
combine(mailRepository.observeFolderMessages(accountId, folder), _searchQuery) { rows, query ->
|
||||
val q = query.trim()
|
||||
rows.filter { if (q.isEmpty()) it.inInbox else it.matchesSearch(q) }
|
||||
}
|
||||
} else {
|
||||
mailRepository.observeFolderMessages(accountId, folder)
|
||||
}
|
||||
// The only remaining client-side pass distinguishes the normal list (synced rows) from
|
||||
// an active search (any matching row, including transient server-search hits) — a match
|
||||
// over the small folder-scoped set, never the whole cache.
|
||||
combine(scoped, _searchQuery) { rows, query ->
|
||||
val q = query.trim()
|
||||
rows.filter { if (q.isEmpty()) it.inInbox else it.matchesSearch(q) }
|
||||
// Unified inbox: browse is paged via [pagedMessages]; this backs only an active
|
||||
// search over the folder's rows across accounts (bounded by the per-account search
|
||||
// limit), staying empty while browsing so the whole inbox is never materialized.
|
||||
_searchQuery.flatMapLatest { query ->
|
||||
val q = query.trim()
|
||||
if (q.isEmpty()) {
|
||||
flowOf(emptyList())
|
||||
} else {
|
||||
mailRepository.observeUnifiedFolderMessages(folder)
|
||||
.map { rows -> rows.filter { it.matchesSearch(q) } }
|
||||
}
|
||||
}
|
||||
}
|
||||
}.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList())
|
||||
|
||||
/**
|
||||
* The unified "All inboxes" browse list as a paged stream (issue #124): folder-synced rows across
|
||||
* every account, newest-first, loaded a window at a time so query/mapping/recomposition cost scale
|
||||
* with the screen, not the total cache. Emits empty paging data whenever a concrete account is
|
||||
* selected or a search is active (those render from [messages]).
|
||||
*/
|
||||
val pagedMessages: Flow<PagingData<Message>> =
|
||||
combine(
|
||||
_selectedAccountId,
|
||||
_selectedFolder,
|
||||
_searchQuery.map { it.isBlank() }.distinctUntilChanged(),
|
||||
) { accountId, folder, browsing -> Triple(accountId, folder, browsing) }
|
||||
.distinctUntilChanged()
|
||||
.flatMapLatest { (accountId, folder, browsing) ->
|
||||
if (accountId == null && browsing) {
|
||||
mailRepository.pagedUnifiedFolderMessages(folder)
|
||||
} else {
|
||||
flowOf(PagingData.empty())
|
||||
}
|
||||
}
|
||||
.cachedIn(viewModelScope)
|
||||
|
||||
val draftCount: StateFlow<Int> = mailRepository.observeDrafts()
|
||||
.map { it.size }
|
||||
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), 0)
|
||||
@@ -156,6 +194,11 @@ class MailboxViewModel @Inject constructor(
|
||||
private val _selectedIds = MutableStateFlow<Set<String>>(emptySet())
|
||||
val selectedIds: StateFlow<Set<String>> = _selectedIds.asStateFlow()
|
||||
|
||||
// Account id per selected message id, captured at selection time. The unified inbox is paged
|
||||
// (issue #124), so the full list isn't held in memory; this lets [selectionAccountId] decide
|
||||
// whether a selection sits within a single account (→ Move offered) without materializing it.
|
||||
private val selectionAccounts = MutableStateFlow<Map<String, String>>(emptyMap())
|
||||
|
||||
private val _pendingConfirm = MutableStateFlow<PendingAction?>(null)
|
||||
val pendingConfirm: StateFlow<PendingAction?> = _pendingConfirm.asStateFlow()
|
||||
|
||||
@@ -173,9 +216,8 @@ class MailboxViewModel @Inject constructor(
|
||||
|
||||
/** The single account every selected message belongs to, or null if the selection spans accounts. */
|
||||
private val selectionAccountId: StateFlow<String?> =
|
||||
combine(_selectedIds, messages) { ids, msgs ->
|
||||
msgs.filter { it.id in ids }.map { it.accountId }.distinct().singleOrNull()
|
||||
}.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), null)
|
||||
selectionAccounts.map { it.values.distinct().singleOrNull() }
|
||||
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), null)
|
||||
|
||||
/** Whether "Move" is offered: only when the whole selection sits in a single account's folder tree. */
|
||||
val canMove: StateFlow<Boolean> = selectionAccountId
|
||||
@@ -187,20 +229,30 @@ class MailboxViewModel @Inject constructor(
|
||||
.flatMapLatest { acct -> if (acct == null) flowOf(emptyList()) else mailRepository.observeFolders(acct) }
|
||||
.stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), emptyList())
|
||||
|
||||
fun startSelection(id: String) {
|
||||
fun startSelection(id: String, accountId: String) {
|
||||
_selectedIds.value = setOf(id)
|
||||
selectionAccounts.value = mapOf(id to accountId)
|
||||
}
|
||||
|
||||
fun toggleSelection(id: String) {
|
||||
_selectedIds.value = _selectedIds.value.let { if (id in it) it - id else it + id }
|
||||
fun toggleSelection(id: String, accountId: String) {
|
||||
if (id in _selectedIds.value) {
|
||||
_selectedIds.value = _selectedIds.value - id
|
||||
selectionAccounts.value = selectionAccounts.value - id
|
||||
} else {
|
||||
_selectedIds.value = _selectedIds.value + id
|
||||
selectionAccounts.value = selectionAccounts.value + (id to accountId)
|
||||
}
|
||||
}
|
||||
|
||||
fun clearSelection() {
|
||||
_selectedIds.value = emptySet()
|
||||
selectionAccounts.value = emptyMap()
|
||||
}
|
||||
|
||||
fun selectAll() {
|
||||
_selectedIds.value = messages.value.map { it.id }.toSet()
|
||||
/** Selects everything currently shown; [items] is the visible list (paged snapshot or list). */
|
||||
fun selectAll(items: List<Message>) {
|
||||
_selectedIds.value = items.map { it.id }.toSet()
|
||||
selectionAccounts.value = items.associate { it.id to it.accountId }
|
||||
}
|
||||
|
||||
fun archiveSelected() = runOnSelection { mailRepository.archive(it) }
|
||||
|
||||
@@ -2,6 +2,9 @@
|
||||
package org.libremail.data.repository
|
||||
|
||||
import android.content.Context
|
||||
import androidx.paging.PagingSource
|
||||
import androidx.paging.PagingState
|
||||
import androidx.paging.testing.asSnapshot
|
||||
import app.cash.turbine.test
|
||||
import io.mockk.Runs
|
||||
import io.mockk.coEvery
|
||||
@@ -105,6 +108,20 @@ class MailRepositoryImplTest {
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `pagedUnifiedFolderMessages maps the paged summaries to domain messages`() = runTest {
|
||||
every { messageDao.pagingUnifiedFolderSummaries("INBOX") } returns FakeSummaryPagingSource(
|
||||
listOf(messageSummary("1", "INBOX"), messageSummary("2", "INBOX", accountId = "acct2")),
|
||||
)
|
||||
|
||||
val items = repository.pagedUnifiedFolderMessages("INBOX").asSnapshot()
|
||||
|
||||
assertEquals(listOf("1", "2"), items.map { it.id })
|
||||
assertEquals("Ada", items.first().sender)
|
||||
// The list projection never carries a body — the reader loads it on demand (see MessageSummary).
|
||||
assertEquals("", items.first().body)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `observeFolders maps cached folders with their roles and server special-use flag`() = runTest {
|
||||
every { folderDao.observeForAccount("acct") } returns flowOf(
|
||||
@@ -604,4 +621,12 @@ class MailRepositoryImplTest {
|
||||
secret = "secret",
|
||||
useXoauth2 = false,
|
||||
)
|
||||
|
||||
/** Serves a fixed set of summaries as a single page, standing in for Room's generated source. */
|
||||
private class FakeSummaryPagingSource(private val rows: List<MessageSummary>) :
|
||||
PagingSource<Int, MessageSummary>() {
|
||||
override fun getRefreshKey(state: PagingState<Int, MessageSummary>): Int? = null
|
||||
override suspend fun load(params: LoadParams<Int>): LoadResult<Int, MessageSummary> =
|
||||
LoadResult.Page(data = rows, prevKey = null, nextKey = null)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
package org.libremail.ui.mailbox
|
||||
|
||||
import androidx.lifecycle.SavedStateHandle
|
||||
import androidx.paging.PagingData
|
||||
import app.cash.turbine.test
|
||||
import io.mockk.coEvery
|
||||
import io.mockk.coVerify
|
||||
@@ -53,7 +54,7 @@ class MailboxViewModelTest {
|
||||
private val bob = account("imap:b", "bob@example.org")
|
||||
|
||||
@Test
|
||||
fun `default view shows only inbox messages across all accounts`() = runTest(testDispatcher) {
|
||||
fun `unified browse keeps the whole inbox out of the in-memory list flow`() = runTest(testDispatcher) {
|
||||
val vm = createViewModel(
|
||||
accounts = listOf(alice, bob),
|
||||
messages = listOf(
|
||||
@@ -66,7 +67,24 @@ class MailboxViewModelTest {
|
||||
|
||||
assertEquals("INBOX", vm.selectedFolder.value)
|
||||
assertNull(vm.selectedAccountId.value)
|
||||
assertEquals(setOf("imap:a:INBOX:1", "imap:b:INBOX:1"), vm.messages.value.map { it.id }.toSet())
|
||||
// Unified browse is paged (issue #124): the flat list flow stays empty so the whole unified
|
||||
// inbox is never materialized on every write. The paged rows themselves are covered by
|
||||
// MailRepositoryImplTest's pagedUnifiedFolderMessages test and the MailboxScreen UI test.
|
||||
assertTrue(vm.messages.value.isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `selecting a concrete account renders the flat scoped list`() = runTest(testDispatcher) {
|
||||
val vm = createViewModel(
|
||||
accounts = listOf(alice),
|
||||
messages = listOf(msg("imap:a:INBOX:1", "imap:a", "INBOX"), msg("imap:a:Archive:1", "imap:a", "Archive")),
|
||||
)
|
||||
backgroundScope.launch { vm.messages.collect {} }
|
||||
|
||||
vm.selectAccount("imap:a")
|
||||
|
||||
// A concrete account uses the SQL-scoped, already-flat list (issue #86), not the paged path.
|
||||
assertEquals(listOf("imap:a:INBOX:1"), vm.messages.value.map { it.id })
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -200,11 +218,11 @@ class MailboxViewModelTest {
|
||||
fun `toggle adds then removes a message from the selection`() = runTest(testDispatcher) {
|
||||
val vm = createViewModel(accounts = listOf(alice), messages = emptyList())
|
||||
|
||||
vm.startSelection("a")
|
||||
vm.startSelection("a", "acct")
|
||||
assertEquals(setOf("a"), vm.selectedIds.value)
|
||||
vm.toggleSelection("b")
|
||||
vm.toggleSelection("b", "acct")
|
||||
assertEquals(setOf("a", "b"), vm.selectedIds.value)
|
||||
vm.toggleSelection("a")
|
||||
vm.toggleSelection("a", "acct")
|
||||
assertEquals(setOf("b"), vm.selectedIds.value)
|
||||
vm.clearSelection()
|
||||
assertTrue(vm.selectedIds.value.isEmpty())
|
||||
@@ -212,13 +230,11 @@ class MailboxViewModelTest {
|
||||
|
||||
@Test
|
||||
fun `selectAll selects every visible message`() = runTest(testDispatcher) {
|
||||
val vm = createViewModel(
|
||||
accounts = listOf(alice),
|
||||
messages = listOf(msg("imap:a:INBOX:1", "imap:a", "INBOX"), msg("imap:a:INBOX:2", "imap:a", "INBOX")),
|
||||
)
|
||||
backgroundScope.launch { vm.messages.collect {} }
|
||||
val shown = listOf(msg("imap:a:INBOX:1", "imap:a", "INBOX"), msg("imap:a:INBOX:2", "imap:a", "INBOX"))
|
||||
val vm = createViewModel(accounts = listOf(alice), messages = shown)
|
||||
|
||||
vm.selectAll()
|
||||
// The screen passes what's shown (paged snapshot or flat list); the VM records their ids.
|
||||
vm.selectAll(shown)
|
||||
|
||||
assertEquals(setOf("imap:a:INBOX:1", "imap:a:INBOX:2"), vm.selectedIds.value)
|
||||
}
|
||||
@@ -234,7 +250,7 @@ class MailboxViewModelTest {
|
||||
)
|
||||
backgroundScope.launch { vm.messages.collect {} }
|
||||
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
vm.requestDelete()
|
||||
val pending = vm.pendingConfirm.value
|
||||
assertTrue(pending is PendingAction.Delete && !pending.permanent)
|
||||
@@ -265,7 +281,7 @@ class MailboxViewModelTest {
|
||||
backgroundScope.launch { vm.currentFolderRole.collect {} }
|
||||
vm.selectFolder("imap:a", "Spam")
|
||||
|
||||
vm.startSelection("imap:a:Spam:1")
|
||||
vm.startSelection("imap:a:Spam:1", "imap:a")
|
||||
vm.requestDelete()
|
||||
val pending = vm.pendingConfirm.value
|
||||
assertTrue(pending is PendingAction.Delete && pending.permanent)
|
||||
@@ -285,7 +301,7 @@ class MailboxViewModelTest {
|
||||
repo = repo,
|
||||
)
|
||||
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
vm.requestSpam()
|
||||
assertTrue(vm.pendingConfirm.value is PendingAction.Spam)
|
||||
coVerify(exactly = 0) { repo.reportSpam(any()) }
|
||||
@@ -304,7 +320,7 @@ class MailboxViewModelTest {
|
||||
repo = repo,
|
||||
)
|
||||
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
vm.reply(ReplyMode.FORWARD)
|
||||
|
||||
assertEquals(MailboxEvent.OpenCompose("draft1"), vm.events.first())
|
||||
@@ -321,7 +337,7 @@ class MailboxViewModelTest {
|
||||
repo = repo,
|
||||
)
|
||||
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
vm.archiveSelected()
|
||||
|
||||
coVerify { repo.archive(listOf("imap:a:INBOX:1")) }
|
||||
@@ -338,7 +354,7 @@ class MailboxViewModelTest {
|
||||
repo = repo,
|
||||
)
|
||||
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
vm.reply(ReplyMode.REPLY)
|
||||
|
||||
assertEquals(MailboxEvent.OpenCompose("d2"), vm.events.first())
|
||||
@@ -352,7 +368,7 @@ class MailboxViewModelTest {
|
||||
messages = listOf(msg("imap:a:INBOX:1", "imap:a", "INBOX")),
|
||||
repo = repo,
|
||||
)
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
|
||||
vm.requestSpam()
|
||||
vm.dismissConfirm()
|
||||
@@ -371,10 +387,10 @@ class MailboxViewModelTest {
|
||||
backgroundScope.launch { vm.messages.collect {} }
|
||||
backgroundScope.launch { vm.canMove.collect {} }
|
||||
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
assertEquals(true, vm.canMove.value)
|
||||
|
||||
vm.toggleSelection("imap:b:INBOX:1")
|
||||
vm.toggleSelection("imap:b:INBOX:1", "imap:b")
|
||||
assertEquals(false, vm.canMove.value)
|
||||
}
|
||||
|
||||
@@ -388,7 +404,7 @@ class MailboxViewModelTest {
|
||||
repo = repo,
|
||||
)
|
||||
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
vm.moveSelected("Receipts")
|
||||
|
||||
coVerify { repo.moveToFolder(listOf("imap:a:INBOX:1"), "Receipts") }
|
||||
@@ -404,7 +420,7 @@ class MailboxViewModelTest {
|
||||
messages = listOf(msg("imap:a:INBOX:1", "imap:a", "INBOX")),
|
||||
repo = repo,
|
||||
)
|
||||
vm.startSelection("imap:a:INBOX:1")
|
||||
vm.startSelection("imap:a:INBOX:1", "imap:a")
|
||||
|
||||
vm.requestReplyAll()
|
||||
assertTrue(vm.pendingConfirm.value is PendingAction.ReplyAll)
|
||||
@@ -493,6 +509,12 @@ class MailboxViewModelTest {
|
||||
val folder = firstArg<String>()
|
||||
MutableStateFlow(messages.filter { it.folder == folder })
|
||||
}
|
||||
// The unified browse list is paged (issue #124); mirror the DAO's inInbox-scoped, folder-scoped
|
||||
// projection as a single static page.
|
||||
every { repo.pagedUnifiedFolderMessages(any()) } answers {
|
||||
val folder = firstArg<String>()
|
||||
flowOf(PagingData.from(messages.filter { it.folder == folder && it.inInbox }))
|
||||
}
|
||||
every { repo.observeDrafts() } returns flowOf(emptyList())
|
||||
every { repo.observeOutbox() } returns flowOf(emptyList())
|
||||
every { repo.observeUnreadCounts() } returns MutableStateFlow(unreadCounts)
|
||||
|
||||
@@ -0,0 +1,107 @@
|
||||
<!-- SPDX-License-Identifier: GPL-3.0-or-later -->
|
||||
# Issue #124 — paging the unified "All inboxes" view
|
||||
|
||||
Follow-up to #86. PR #123 made the per-account+folder list flat by scoping the query in SQL, but the
|
||||
unified inbox (`WHERE folder = ?`, no `accountId`) has no `folder`-leading index, so it still **scans**
|
||||
in timestamp order and — worse — materializes the **entire** unified inbox (~thousands of rows) into
|
||||
memory on every emission. This measures that path **before** the fix and **with** Paging 3, on a large
|
||||
seeded cache, to decide whether an index (schema migration) is actually needed once paged.
|
||||
|
||||
## TL;DR / verdict
|
||||
|
||||
- **Paging fixes it; no index, no schema migration.** The current whole-unified-inbox query grows
|
||||
linearly with the number of INBOX rows (**~6.8 ms at 1k INBOX → ~24.6 ms at 4k INBOX**), because it
|
||||
reads and maps every INBOX row across all accounts. The paged first page (the production initial
|
||||
load of 120 rows) is **flat at ~5–7 ms regardless of total cache size** — it stops as soon as it has
|
||||
a screenful — a **~3.6× first-emit speedup at a 20k cache**, and, more importantly, it stays flat as
|
||||
the cache keeps growing (full-history backfill, #12/#13) while the current path keeps rising.
|
||||
- **`EXPLAIN QUERY PLAN` still shows a `SCAN … index_messages_timestampMillis`** for the paged query on
|
||||
the existing indices — i.e. no `folder`-leading index is used — yet the paged first page is already
|
||||
flat, because the `LIMIT` lets the planner stop early. A `(folder, timestampMillis)` /
|
||||
`(folder, inInbox, timestampMillis)` index would only materially help a **deep scroll** (large
|
||||
`OFFSET`), which is rare and already bounded by scroll depth (not total cache), and even the worst
|
||||
deep page measured (40 rows at offset ~3.9k) is **faster than the current whole-inbox load**. So the
|
||||
ticket's strong preference holds: **Paging 3 alone captures the win — no `(folder, …)` index, no
|
||||
v15→v16 migration, no #118 version coordination.**
|
||||
- **Per-account views are untouched** (already flat from #86); only the unified browse list is paged.
|
||||
|
||||
## Setup & method
|
||||
|
||||
- Device: Gradle-managed AVD `libremail_api29` (API 29, x86, google_apis), the same device #86 used, so
|
||||
the figures are comparable. Cross-checked on a physical Pixel (API 37) — same shape (see below).
|
||||
- Harness: a throwaway instrumented probe (`UnifiedInboxPagingProbe`, removed after measuring, like
|
||||
#86's) built the real `LibreMailDatabase` in-memory (real entities, real indices, real generated
|
||||
`MessageDao`), seeded it, and timed each variant with **5 warmup + 15 measured iterations**, reporting
|
||||
**median and min** wall-clock ms with a GC between phases. `androidx.benchmark` was deliberately not
|
||||
used (as in #86: on an emulator only the **relative** A/B result is meaningful, and it would force the
|
||||
module's instrumentation runner, changing what CI's E2E jobs run under).
|
||||
- Dataset: rows spread across **3 accounts × {INBOX, Sent, Archive, Spam, Work}** (INBOX ≈ 20%, so at
|
||||
20k the unified inbox is ~4k rows), timestamps interleaved across folders (a realistic worst case —
|
||||
INBOX rows are **not** clustered at the top of the timestamp index), each row carrying a ~1 KB body so
|
||||
rows are realistically sized (the summary projection never selects the body).
|
||||
- Variants: (a) **CURRENT** = `observeUnifiedFolderSummaries("INBOX")` first Flow emission (materializes
|
||||
the whole unified inbox); (b) **PAGED first page** = `pagingUnifiedFolderSummaries("INBOX")` loaded via
|
||||
a `Refresh(key = null, loadSize = 120)` — the production `initialLoadSize` (3 × pageSize); (c) **PAGED
|
||||
deep page** = a `Refresh(key ≈ inboxCount − 60, loadSize = 40)` window near the end of the inbox
|
||||
(models scrolling to the bottom). Each query's `EXPLAIN QUERY PLAN` was captured with existing indices
|
||||
only.
|
||||
|
||||
## Results (median / min ms, lower is better)
|
||||
|
||||
| total rows | INBOX rows | CURRENT whole-inbox first-emit | PAGED first page (120) | PAGED deep page (40 @ ~end) |
|
||||
|-----------:|-----------:|-------------------------------:|-----------------------:|----------------------------:|
|
||||
| 5 000 | 1 000 | **6.80** / 4.93 | 5.44 / 4.64 | 3.67 / 2.83 |
|
||||
| 20 000 | 4 000 | **24.56** / 18.89 | 6.82 / 5.67 | 12.86 / 10.52 |
|
||||
|
||||
- **CURRENT scales with the inbox size:** 6.8 → 24.6 ms as INBOX rows go 1k → 4k (≈ linear); this cost
|
||||
is paid on **every** re-emission (any write to `messages` — IDLE delivery, a read/star toggle, a
|
||||
backfill page, a sync of any folder), and it also builds a full `List<Message>` of every INBOX row.
|
||||
- **PAGED first page is flat:** ~5.4 → ~6.8 ms (within noise) — independent of the total cache, because
|
||||
the scan stops once it has 120 INBOX rows. This is the common case (opening / reading the top of the
|
||||
inbox), and it's what the acceptance criterion asks for.
|
||||
- **PAGED deep page** (40 rows at offset ~3.9k) costs 12.9 ms at 20k — more than the first page (the
|
||||
`OFFSET` walks ~3.9k INBOX matches) but still **below the current whole-inbox load**, returns only 40
|
||||
rows (vs. the current path's 4 000), and scales with **scroll depth**, not total cache.
|
||||
|
||||
Cross-check on a physical **Pixel (API 37)** at 20k: CURRENT 27.2 / 24.8, PAGED first page 8.6 / 7.2,
|
||||
PAGED deep page 15.3 / 12.4 ms — same shape (~3.2× first-page speedup).
|
||||
|
||||
## Query plans (`EXPLAIN QUERY PLAN`, existing indices only)
|
||||
|
||||
```
|
||||
CURRENT whole-inbox : SCAN TABLE messages USING INDEX index_messages_timestampMillis
|
||||
PAGED first page : SCAN TABLE messages USING INDEX index_messages_timestampMillis (LIMIT 120)
|
||||
PAGED deep page : SCAN TABLE messages USING INDEX index_messages_timestampMillis (LIMIT 40 OFFSET ~3.9k)
|
||||
```
|
||||
|
||||
- All three walk `index_messages_timestampMillis` newest-first (no `folder`-leading index), filtering
|
||||
`folder`/`inInbox` per row. The paged queries differ only by the `LIMIT`/`OFFSET` the planner applies,
|
||||
which is exactly what makes the first page cheap: it stops after a screenful.
|
||||
- A `(folder, inInbox, timestampMillis)` index would turn the scan into a seek and remove the deep-page
|
||||
`OFFSET` walk. It is **not added**: the first page — the case the ticket targets — is already flat on
|
||||
the existing indices, deep scroll is rare and bounded by depth, and avoiding the index avoids a
|
||||
schema migration (v15→v16) and the #118 version-coordination it would require. Revisit only if deep
|
||||
scrolling the unified inbox becomes a measured problem.
|
||||
|
||||
## What was implemented
|
||||
|
||||
- `MessageDao.pagingUnifiedFolderSummaries(folder)` — a Paging 3 `PagingSource<Int, MessageSummary>`
|
||||
over `WHERE folder = ? AND inInbox = 1 ORDER BY timestampMillis DESC` (synced rows only; unified
|
||||
**search** keeps using `observeUnifiedFolderSummaries`, which also surfaces transient `inInbox = 0`
|
||||
hits).
|
||||
- `MailRepository.pagedUnifiedFolderMessages(folder)` — wraps it in a `Pager`
|
||||
(`pageSize = 40`, `initialLoadSize = 120`, no placeholders) and maps summaries to domain.
|
||||
- `MailboxViewModel.pagedMessages` — `flatMapLatest` to the paged flow while browsing the unified inbox
|
||||
(no account selected, no active search), else empty paging data; `cachedIn(viewModelScope)`. The old
|
||||
`messages` list flow now stays **empty** while browsing the unified inbox, so the whole inbox is never
|
||||
pulled into memory; it still serves per-account views and unified **search**. Selection carries each
|
||||
row's `accountId` (captured at tap time) so "Move" still works without a full in-memory list.
|
||||
- `MailboxScreen` renders the unified browse list via `collectAsLazyPagingItems()`; per-account/search
|
||||
render the flat list as before.
|
||||
|
||||
## Reproduce
|
||||
|
||||
Seed a large multi-account `messages` cache, then compare `observeUnifiedFolderSummaries("INBOX")`
|
||||
(first emit) against `pagingUnifiedFolderSummaries("INBOX")` loaded with a `Refresh(null, 120)` on an
|
||||
emulator, and inspect `EXPLAIN QUERY PLAN`. The paged first page should be ~flat (~5–7 ms) regardless
|
||||
of total cache; the whole-inbox query should grow with the INBOX row count.
|
||||
@@ -114,9 +114,10 @@ UNIFIED folder-only : SCAN TABLE messages USING INDEX index_messages_timestampMi
|
||||
|
||||
## Deferred follow-ups
|
||||
|
||||
- **Unified inbox**: add Room `PagingSource`/Paging3 so the unified list's cost scales with the screen,
|
||||
not the total inbox count; optionally a `(folder, timestampMillis)` index to turn the scan into a
|
||||
seek. (The per-account views are already flat, so Paging3 there is lower priority.)
|
||||
- **Unified inbox**: ~~add Room `PagingSource`/Paging3 so the unified list's cost scales with the
|
||||
screen, not the total inbox count~~ — done in #124 (`docs/perf/issue-124-unified-inbox-paging.md`).
|
||||
Paging alone captured the win (first page ~flat at ~5–7 ms vs. the current ~25 ms at a 20k cache);
|
||||
the `(folder, timestampMillis)` index proved unnecessary, so no schema migration was added.
|
||||
- **IMAP latency on folder open** is out of scope for this cached-render fix.
|
||||
|
||||
## Reproduce
|
||||
|
||||
@@ -12,6 +12,7 @@ webkit = "1.12.1"
|
||||
biometric = "1.1.0"
|
||||
composeBom = "2026.06.00"
|
||||
room = "2.8.4"
|
||||
paging = "3.3.6"
|
||||
sqlcipher = "4.16.0"
|
||||
datastore = "1.2.1"
|
||||
work = "2.11.2"
|
||||
@@ -72,9 +73,18 @@ androidx-room-ktx = { group = "androidx.room", name = "room-ktx", version.ref =
|
||||
androidx-room-compiler = { group = "androidx.room", name = "room-compiler", version.ref = "room" }
|
||||
# Room MigrationTestHelper (instrumented migration tests).
|
||||
androidx-room-testing = { group = "androidx.room", name = "room-testing", version.ref = "room" }
|
||||
# Room PagingSource support — lets @Query methods return androidx.paging.PagingSource.
|
||||
androidx-room-paging = { group = "androidx.room", name = "room-paging", version.ref = "room" }
|
||||
# SQLCipher — opt-in at-rest encryption of the Room cache.
|
||||
sqlcipher-android = { group = "net.zetetic", name = "sqlcipher-android", version.ref = "sqlcipher" }
|
||||
|
||||
# Paging 3 — pages the unified "All inboxes" list so its query/recomposition cost scales with the
|
||||
# visible window, not the whole cache (issue #124). runtime = Pager/PagingData; compose =
|
||||
# collectAsLazyPagingItems; testing = asSnapshot for JVM unit tests.
|
||||
androidx-paging-runtime = { group = "androidx.paging", name = "paging-runtime", version.ref = "paging" }
|
||||
androidx-paging-compose = { group = "androidx.paging", name = "paging-compose", version.ref = "paging" }
|
||||
androidx-paging-testing = { group = "androidx.paging", name = "paging-testing", version.ref = "paging" }
|
||||
|
||||
# DataStore (settings) / WorkManager (sync) — wired in later increments
|
||||
androidx-datastore-preferences = { group = "androidx.datastore", name = "datastore-preferences", version.ref = "datastore" }
|
||||
androidx-work-runtime-ktx = { group = "androidx.work", name = "work-runtime-ktx", version.ref = "work" }
|
||||
|
||||
Reference in New Issue
Block a user