From 3af2e7fcc3ecd28d5039e760cb6f4abd59fd36a3 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 21 Sep 2026 17:36:48 +0200 Subject: [PATCH 1/2] feat(conversations): refresh the conversation list while it is open With the list on screen, a message sent from another device shows up only when a push arrives or the user leaves the screen and comes back. That gap matters most for the people who have no working push at all - no Play Services, no UnifiedPush distributor - for whom nothing else refreshes the list while they are looking at it. Re-sync it every thirty seconds instead, matching the iOS timer. The loop is bound to the resumed lifecycle, so it cannot run behind the screen or outlive it, and it waits before the first sync rather than after: onResume already fetches, and syncing on entering the loop would fetch twice every time the screen is resumed. A tick is dropped while a sync is already in flight, so the loop can never queue behind itself, and while a search is open, where swapping the list out would move what the user is reading. Being offline and battery saver are deliberately not checked here: the first is already handled where the sync would make its request, and the second already stops the expensive half, the message catch-up it triggers. Every tick asks only for what changed, which is what makes a timer this short affordable at all. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger --- .../ConversationsListActivity.kt | 23 ++++++++++ .../data/OfflineConversationsRepository.kt | 26 ++++++----- .../OfflineFirstConversationsRepository.kt | 43 ++++++++++--------- .../viewmodels/ConversationsListViewModel.kt | 19 ++++++++ 4 files changed, 80 insertions(+), 31 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt index 246d3df227..9a3708cc58 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt @@ -31,6 +31,8 @@ import androidx.core.content.pm.ShortcutManagerCompat import androidx.core.graphics.drawable.IconCompat import androidx.core.net.toUri import androidx.lifecycle.ViewModelProvider +import androidx.lifecycle.repeatOnLifecycle +import androidx.lifecycle.Lifecycle import androidx.lifecycle.lifecycleScope import androidx.work.Data import androidx.work.OneTimeWorkRequest @@ -114,9 +116,11 @@ import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_SHARED_TEXT import com.nextcloud.talk.utils.permissions.PlatformPermissionUtil import com.nextcloud.talk.utils.power.PowerManagerUtils import com.nextcloud.talk.utils.singletons.ApplicationWideCurrentRoomHolder +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import org.greenrobot.eventbus.Subscribe @@ -207,6 +211,7 @@ class ConversationsListActivity : BaseActivity() { conversationsListViewModel = ViewModelProvider(this, viewModelFactory)[ConversationsListViewModel::class.java] conversationTagsViewModel = ViewModelProvider(this, viewModelFactory)[ConversationTagsViewModel::class.java] + startForegroundRefreshLoop() setSupportActionBar(null) forwardMessageState.value = intent.getBooleanExtra(KEY_FORWARD_MSG_FLAG, false) @@ -670,6 +675,22 @@ class ConversationsListActivity : BaseActivity() { lifecycleScope.launch { snackbarHostState.showSnackbar(text) } } + /** + * Starts a loop that refreshes the conversation list every + * [FOREGROUND_REFRESH_INTERVAL_MILLIS] milliseconds while this screen is resumed, waiting one + * interval before the first refresh. + */ + private fun startForegroundRefreshLoop() { + lifecycleScope.launch { + repeatOnLifecycle(Lifecycle.State.RESUMED) { + while (isActive) { + delay(FOREGROUND_REFRESH_INTERVAL_MILLIS) + currentUser?.let { conversationsListViewModel.refreshRoomsIfIdle(it) } + } + } + } + } + fun fetchRooms(forceFullSync: Boolean = false) { conversationsListViewModel.getRooms(currentUser!!, forceFullSync) } @@ -1574,6 +1595,8 @@ class ConversationsListActivity : BaseActivity() { companion object { private val TAG = ConversationsListActivity::class.java.simpleName + + private const val FOREGROUND_REFRESH_INTERVAL_MILLIS = 30_000L const val BOTTOM_SHEET_DELAY: Long = 2500 const val SEARCH_DEBOUNCE_INTERVAL_MS = 300 const val HTTP_UNAUTHORIZED = 401 diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt index 53987e2b67..3a12857275 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt @@ -42,26 +42,30 @@ interface OfflineConversationsRepository { * the server (when online). The synced changes surface through [roomListFlow], which * observes the database. * - * The sync asks the server only for what changed since the last one where it can. Set - * [forceFullSync] for the cases where that is not good enough and the whole list is wanted - * back - a pull to refresh, where a conversation the user left elsewhere should be gone by the - * time the indicator stops spinning, rather than within the next five minutes. + * The sync asks the server only for the conversations changed since the last one where it + * can. [forceFullSync] makes it fetch the whole list instead. */ @Deprecated("use observeConversation") fun getRooms(user: User, forceFullSync: Boolean = false): Job /** - * Synchronizes [user]'s conversations with the server and returns once that sync and the - * message catch-up it triggers are done, reporting whether it worked. + * Synchronizes [user]'s conversations with the server, suspending until that sync and the + * message catch-up it triggers have finished, and returns whether the sync succeeded. * - * [getRooms] launches into the repository's own scope and returns immediately, which is what - * the conversation list wants and what a background worker cannot use: WorkManager tears the - * process down once the worker returns, mid-request. This does not select the observed account - * either - that is what the conversation list screen shows, and a worker walking several - * accounts must not move it. + * Unlike [getRooms] this neither returns early nor changes which account [roomListFlow] + * observes. */ suspend fun syncRooms(user: User, forceFullSync: Boolean = false): Boolean + /** + * Whether a sync for [user] right now would either ask only for the conversations that changed, + * or be the full refresh that falls due every five minutes. + * + * False means the next sync would fetch the whole conversation list without the cadence calling + * for it. + */ + suspend fun isPeriodicSyncDue(user: User): Boolean + /** * Called once onStart to emit a conversation to [conversationFlow] * to be handled asynchronously. diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt index fc0ac941fa..47e6b105e3 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt @@ -11,7 +11,6 @@ package com.nextcloud.talk.conversationlist.data.network import android.content.Context import android.database.sqlite.SQLiteConstraintException import android.net.ConnectivityManager -import android.os.PowerManager import android.util.Log import com.nextcloud.talk.arbitrarystorage.ArbitraryStorageManager import com.nextcloud.talk.chat.data.network.ChatMessageSyncer @@ -23,6 +22,7 @@ import com.nextcloud.talk.data.database.mappers.toDomainModel import com.nextcloud.talk.data.database.model.ConversationEntity import com.nextcloud.talk.data.network.NetworkMonitor import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.extensions.isPowerSaveMode import com.nextcloud.talk.logger.Logger import com.nextcloud.talk.models.domain.ConversationModel import com.nextcloud.talk.utils.ApiUtils @@ -48,6 +48,7 @@ import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.launch import kotlinx.coroutines.sync.Semaphore import kotlinx.coroutines.sync.withPermit +import kotlinx.coroutines.withContext import javax.inject.Inject import kotlin.collections.map @@ -242,31 +243,38 @@ class OfflineFirstConversationsRepository @Inject constructor( } /** - * The value to send as `modifiedSince`, or null when this sync has to be a full one. + * The stored timestamp to send as `modifiedSince`, or null when the sync has to fetch the whole + * list. * - * A filtered response cannot express a removal, so the server asks clients to refresh in full - * regularly (`docs/conversation.md`): at least every five minutes, and always when the internal - * signaling backend is in use, since there is no signaling server to announce a change out of - * band. On top of that a caller can demand one, for a pull to refresh or an account switch. + * Null is returned when [forceFullSync] is set, when the account uses the internal signaling + * backend, when the last full sync is older than [FULL_SYNC_INTERVAL_MILLIS], and when no + * timestamp is stored. */ private fun modifiedSinceFor(user: User, forceFullSync: Boolean): Long? { val accountId = user.id!! - val lastFullSyncAt = readTimestamp(accountId, KEY_LAST_FULL_SYNC_AT) - val fullSyncIsRecent = lastFullSyncAt != null && - System.currentTimeMillis() - lastFullSyncAt in 0 until FULL_SYNC_INTERVAL_MILLIS val usesExternalSignaling = !user.externalSignalingServer?.externalSignalingServer.isNullOrEmpty() - return if (!forceFullSync && usesExternalSignaling && fullSyncIsRecent) { + return if (!forceFullSync && usesExternalSignaling && fullSyncIsRecent(accountId)) { readTimestamp(accountId, KEY_MODIFIED_SINCE) } else { null } } + private fun fullSyncIsRecent(accountId: Long): Boolean { + val lastFullSyncAt = readTimestamp(accountId, KEY_LAST_FULL_SYNC_AT) ?: return false + return System.currentTimeMillis() - lastFullSyncAt in 0 until FULL_SYNC_INTERVAL_MILLIS + } + + override suspend fun isPeriodicSyncDue(user: User): Boolean = + withContext(Dispatchers.IO) { + modifiedSinceFor(user, forceFullSync = false) != null || !fullSyncIsRecent(user.id!!) + } + /** - * Stores what the next sync needs, and only once the response is safely in the database: a - * timestamp kept ahead of a write that then failed would permanently skip the conversations - * that write was carrying. + * Stores the timestamp [roomList] reported for the next request, and, when it was a full + * response, the time of this full sync. Called after the response has been written to the + * database. */ private fun rememberSyncedState(accountId: Long, roomList: RoomListResult) { storeTimestamp(accountId, KEY_MODIFIED_SINCE, roomList.modifiedBefore) @@ -275,7 +283,7 @@ class OfflineFirstConversationsRepository @Inject constructor( } } - /** Null for anything that is not a plausible timestamp, so a bad value asks for a full sync. */ + /** The timestamp stored under [key] for [accountId], or null when none is stored or it is not a positive number. */ private fun readTimestamp(accountId: Long, key: String): Long? = arbitraryStorageManager.getStorageSetting(accountId, key, "") .blockingGet() @@ -364,7 +372,7 @@ class OfflineFirstConversationsRepository @Inject constructor( false } - isPowerSaveMode() -> { + context.isPowerSaveMode() -> { Log.d(TAG, "Battery saver is active, skipping message catch-up") false } @@ -377,11 +385,6 @@ class OfflineFirstConversationsRepository @Inject constructor( else -> true } - private fun isPowerSaveMode(): Boolean { - val powerManager = context.getSystemService(Context.POWER_SERVICE) as PowerManager - return powerManager.isPowerSaveMode - } - private fun isBackgroundDataRestricted(): Boolean { val connectivityManager = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager return connectivityManager.isActiveNetworkMetered && diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt index 6508a1697e..5aff793c5a 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModel.kt @@ -563,6 +563,25 @@ class ConversationsListViewModel @Inject constructor( } } + /** + * Refreshes the conversation list, unless a sync is already running, a search is open, or the + * sync would fetch the whole list without the refresh cadence calling for it. + */ + suspend fun refreshRoomsIfIdle(user: User) { + when { + _isLoadingRooms.value -> + Log.d(TAG, "Foreground refresh skipped: a sync is already in flight") + + _isSearchActiveFlow.value -> + Log.d(TAG, "Foreground refresh skipped: a search is active") + + !repository.isPeriodicSyncDue(user) -> + Log.d(TAG, "Foreground refresh skipped: no delta available and no full sync due") + + else -> getRooms(user) + } + } + fun getRooms(user: User, forceFullSync: Boolean = false) { val startNanoTime = System.nanoTime() Log.d(TAG, "fetchData - getRooms - calling: $startNanoTime") From 6736c5ff2782e53b88958377065810d6e37e09eb Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 21 Sep 2026 17:36:49 +0200 Subject: [PATCH 2/2] test(conversations): cover the foreground refresh loop The loop fires whatever else is going on, so what decides whether the list stays usable is when a tick is dropped: one that is not dropped while a sync is running queues requests behind each other, and one that is not dropped during a search swaps the list out from under what the user is reading. The two cases pin each other down. A tick during an in-flight sync must leave the call count at one, and a tick after the previous sync finished must take it to two, so neither dropping every tick nor dropping none of them passes both. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger --- ...tionsListViewModelForegroundRefreshTest.kt | 170 ++++++++++++++++++ 1 file changed, 170 insertions(+) create mode 100644 app/src/test/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModelForegroundRefreshTest.kt diff --git a/app/src/test/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModelForegroundRefreshTest.kt b/app/src/test/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModelForegroundRefreshTest.kt new file mode 100644 index 0000000000..20100e024b --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/conversationlist/viewmodels/ConversationsListViewModelForegroundRefreshTest.kt @@ -0,0 +1,170 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Andy Scherzinger + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +package com.nextcloud.talk.conversationlist.viewmodels + +import android.app.Application +import com.nextcloud.talk.arbitrarystorage.ArbitraryStorageManager +import com.nextcloud.talk.contacts.ContactsRepository +import com.nextcloud.talk.conversationlist.data.OfflineConversationsRepository +import com.nextcloud.talk.conversationlist.data.network.ConversationListUpdater +import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.invitation.data.InvitationsRepository +import com.nextcloud.talk.logger.Logger +import com.nextcloud.talk.openconversations.data.OpenConversationsRepository +import com.nextcloud.talk.repositories.conversations.ConversationsRepository +import com.nextcloud.talk.repositories.unifiedsearch.UnifiedSearchRepository +import com.nextcloud.talk.threadsoverview.data.ThreadsRepository +import com.nextcloud.talk.users.UserManager +import com.nextcloud.talk.utils.database.user.CurrentUserProviderOld +import io.reactivex.Maybe +import kotlinx.coroutines.CompletableJob +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.setMain +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.kotlin.any +import org.mockito.kotlin.eq +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever +import org.mockito.kotlin.wheneverBlocking +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * Tests for [ConversationsListViewModel.refreshRoomsIfIdle]: which refresh ticks reach the + * repository and which are dropped. + */ +@OptIn(ExperimentalCoroutinesApi::class) +@RunWith(RobolectricTestRunner::class) +@Config(application = Application::class, sdk = [33]) +class ConversationsListViewModelForegroundRefreshTest { + + private val repository: OfflineConversationsRepository = mock() + private val currentUserProvider: CurrentUserProviderOld = mock() + + private lateinit var viewModel: ConversationsListViewModel + + @Before + fun setUp() { + Dispatchers.setMain(UnconfinedTestDispatcher()) + wheneverBlocking { repository.isPeriodicSyncDue(any()) }.thenReturn(true) + whenever(repository.roomListFlow).thenReturn(emptyFlow()) + whenever(repository.syncErrorFlow).thenReturn(emptyFlow()) + whenever(currentUserProvider.currentUser).thenReturn(Maybe.just(USER)) + + viewModel = ConversationsListViewModel( + repository, + mock(), + currentUserProvider, + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + mock(), + mock() + ) + } + + @After + fun tearDown() { + Dispatchers.resetMain() + } + + @Test + fun `a tick refreshes the list once the previous sync has finished`() { + whenever(repository.getRooms(any(), any())).thenReturn(completedJob()) + viewModel.getRooms(USER) + + runBlocking { viewModel.refreshRoomsIfIdle(USER) } + + verify(repository, times(2)).getRooms(eq(USER), eq(false)) + } + + @Test + fun `a tick while a sync is still running is dropped rather than queued`() { + whenever(repository.getRooms(any(), any())).thenReturn(Job()) + viewModel.getRooms(USER) + + runBlocking { viewModel.refreshRoomsIfIdle(USER) } + runBlocking { viewModel.refreshRoomsIfIdle(USER) } + + verify(repository, times(1)).getRooms(eq(USER), eq(false)) + } + + @Test + fun `a tick during an open search leaves the list alone`() { + whenever(repository.getRooms(any(), any())).thenReturn(completedJob()) + viewModel.getRooms(USER) + viewModel.setIsSearchActive(true) + + runBlocking { viewModel.refreshRoomsIfIdle(USER) } + + verify(repository, times(1)).getRooms(eq(USER), eq(false)) + } + + @Test + fun `the refresh resumes once the search is closed`() { + whenever(repository.getRooms(any(), any())).thenReturn(completedJob()) + viewModel.getRooms(USER) + viewModel.setIsSearchActive(true) + runBlocking { viewModel.refreshRoomsIfIdle(USER) } + viewModel.setIsSearchActive(false) + + runBlocking { viewModel.refreshRoomsIfIdle(USER) } + + verify(repository, times(2)).getRooms(eq(USER), eq(false)) + } + + @Test + fun `a tick that would have to fetch the whole list is dropped until the cadence is due`() { + whenever(repository.getRooms(any(), any())).thenReturn(completedJob()) + wheneverBlocking { repository.isPeriodicSyncDue(any()) }.thenReturn(false) + viewModel.getRooms(USER) + + runBlocking { viewModel.refreshRoomsIfIdle(USER) } + runBlocking { viewModel.refreshRoomsIfIdle(USER) } + + verify(repository, times(1)).getRooms(eq(USER), eq(false)) + } + + @Test + fun `a tick never asks for a full sync, that is what the cadence and pull to refresh are for`() { + whenever(repository.getRooms(any(), any())).thenReturn(completedJob()) + viewModel.getRooms(USER) + + runBlocking { viewModel.refreshRoomsIfIdle(USER) } + + verify(repository, never()).getRooms(any(), eq(true)) + } + + private fun completedJob(): CompletableJob = Job().apply { complete() } + + companion object { + private val USER = User( + id = 1, + userId = "me", + username = "me", + token = "app-password", + baseUrl = "https://server.example.com" + ) + } +}