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") 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" + ) + } +}