From 0b078db1c8c05340d4a1abb399b447546dd30cfb Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 21 Sep 2026 17:01:12 +0200 Subject: [PATCH 1/3] refactor(conversations): make the room list sync awaitable getRooms launches into the repository's own scope and returns straight away, which is what the conversation list wants and what a background worker cannot use: WorkManager tears the process down the moment the worker returns, and the sync would be cut off mid-request. Add a suspend entry point that completes when the sync and the message catch-up it triggers are done. The catch-up is launched detached on the existing path and awaited on this one, so a worker cannot report success for messages it never fetched. It deliberately leaves the observed account alone. That selects what the conversation list screen shows, and a worker walking several accounts in one run must not move it. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger --- .../data/OfflineConversationsRepository.kt | 12 ++++++++++++ .../OfflineFirstConversationsRepository.kt | 17 +++++++++++++++-- 2 files changed, 27 insertions(+), 2 deletions(-) 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 27c0637258..53987e2b67 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 @@ -50,6 +50,18 @@ interface OfflineConversationsRepository { @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. + * + * [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. + */ + suspend fun syncRooms(user: User, forceFullSync: Boolean = false): 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 27d78fa578..97b367f54c 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 @@ -122,6 +122,9 @@ class OfflineFirstConversationsRepository @Inject constructor( } } + override suspend fun syncRooms(user: User, forceFullSync: Boolean): Boolean = + getRoomsFromServer(user, forceFullSync = forceFullSync, awaitCatchUp = true) != null + @Suppress("Detekt.TooGenericExceptionCaught") override fun getRoom(user: User, roomToken: String): Job = scope.launch { @@ -169,7 +172,11 @@ class OfflineFirstConversationsRepository @Inject constructor( } @Suppress("Detekt.TooGenericExceptionCaught") - private suspend fun getRoomsFromServer(user: User, forceFullSync: Boolean = false): List? { + private suspend fun getRoomsFromServer( + user: User, + forceFullSync: Boolean = false, + awaitCatchUp: Boolean = false + ): List? { var conversationsFromSync: List? = null if (!networkMonitor.isOnline.value) { @@ -224,7 +231,13 @@ class OfflineFirstConversationsRepository @Inject constructor( rememberSyncedState(accountId, roomList) val roomsWithNewMessages = getRoomsWithNewMessages(conversationsFromSync, previousConversations) - scope.launch { catchUpRoomsWithNewMessages(user, roomsWithNewMessages) } + if (awaitCatchUp) { + // A worker that returns before the catch-up is done reports success for messages it + // never fetched, and is torn down mid-request. + catchUpRoomsWithNewMessages(user, roomsWithNewMessages) + } else { + scope.launch { catchUpRoomsWithNewMessages(user, roomsWithNewMessages) } + } } catch (e: Exception) { Log.e(TAG, "Something went wrong when fetching conversations", e) // A delta anchored on a sync that did not land is worse than fetching everything again: From 18cb9cea0fc2afe9a12431808223491f76e72c68 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 21 Sep 2026 17:03:47 +0200 Subject: [PATCH 2/3] feat(conversations): sync conversations periodically in background A backgrounded app learns nothing about its conversations until a push arrives. Where push is disabled, unavailable or simply does not turn up, that means the list, the unread counts and the cached messages stay as they were until the user opens the app. Add a periodic worker that runs the conversation list sync and its message catch-up for every configured account. Accounts are synced one after another rather than at once, because each fans out into its own bounded set of message requests and running them together multiplies that into a burst on a single wake-up. It stands down in battery saver and while the app is in the foreground, where the list refreshes itself, and leaves reachability to the work request's network constraint - the connectivity flow is frozen in a process with no UI collector, and pre-checking it there is what silently disabled the message prefetch before. WorkManager's period floor is 15 minutes and Doze defers it further, so this bounds staleness rather than delivering immediacy. Scheduling keeps an existing run instead of replacing it, so an app opened often still reaches one. The battery saver check would have been copied a third time here, so it moves to a Context extension the repository and both workers share. The worker's decision is kept apart from doWork, which reaches for the application singleton to inject itself and can therefore not be driven from a unit test. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger --- .../application/NextcloudTalkApplication.kt | 3 + .../talk/extensions/ContextExtensions.kt | 13 ++ .../talk/jobs/ChatMessageCatchUpWorker.kt | 9 +- .../talk/jobs/ConversationsSyncWorker.kt | 142 ++++++++++++++++++ 4 files changed, 160 insertions(+), 7 deletions(-) create mode 100644 app/src/main/java/com/nextcloud/talk/extensions/ContextExtensions.kt create mode 100644 app/src/main/java/com/nextcloud/talk/jobs/ConversationsSyncWorker.kt diff --git a/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt b/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt index b8bc0993fe..d2563704c5 100644 --- a/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt +++ b/app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt @@ -50,6 +50,7 @@ import com.nextcloud.talk.dagger.modules.UtilsModule import com.nextcloud.talk.dagger.modules.ViewModelModule import com.nextcloud.talk.filebrowser.webdav.DavUtils import com.nextcloud.talk.jobs.AccountRemovalWorker +import com.nextcloud.talk.jobs.ConversationsSyncWorker import com.nextcloud.talk.jobs.CapabilitiesSyncWorker import com.nextcloud.talk.jobs.SignalingSettingsWorker import com.nextcloud.talk.jobs.WebsocketConnectionsWorker @@ -262,6 +263,8 @@ class NextcloudTalkApplication : ExistingPeriodicWorkPolicy.REPLACE, periodicCapabilitiesUpdateWork ) + + ConversationsSyncWorker.schedule(applicationContext) } override fun onTerminate() { diff --git a/app/src/main/java/com/nextcloud/talk/extensions/ContextExtensions.kt b/app/src/main/java/com/nextcloud/talk/extensions/ContextExtensions.kt new file mode 100644 index 0000000000..4a575edebe --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/extensions/ContextExtensions.kt @@ -0,0 +1,13 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Andy Scherzinger + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.extensions + +import android.content.Context +import android.os.PowerManager + +/** Whether the device is currently in battery saver mode. */ +fun Context.isPowerSaveMode(): Boolean = (getSystemService(Context.POWER_SERVICE) as PowerManager).isPowerSaveMode diff --git a/app/src/main/java/com/nextcloud/talk/jobs/ChatMessageCatchUpWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/ChatMessageCatchUpWorker.kt index 3e3bc44a09..b6e583c49e 100644 --- a/app/src/main/java/com/nextcloud/talk/jobs/ChatMessageCatchUpWorker.kt +++ b/app/src/main/java/com/nextcloud/talk/jobs/ChatMessageCatchUpWorker.kt @@ -7,7 +7,6 @@ package com.nextcloud.talk.jobs import android.content.Context -import android.os.PowerManager import android.util.Log import androidx.work.BackoffPolicy import androidx.work.Constraints @@ -22,6 +21,7 @@ import androidx.work.WorkerParameters import autodagger.AutoInjector import com.nextcloud.talk.application.NextcloudTalkApplication import com.nextcloud.talk.application.NextcloudTalkApplication.Companion.sharedApplication +import com.nextcloud.talk.extensions.isPowerSaveMode import com.nextcloud.talk.chat.data.network.ChatMessageSyncer import com.nextcloud.talk.data.database.dao.ConversationsDao import com.nextcloud.talk.users.UserManager @@ -71,7 +71,7 @@ class ChatMessageCatchUpWorker(context: Context, workerParams: WorkerParameters) Result.failure() } - isPowerSaveMode() -> { + applicationContext.isPowerSaveMode() -> { Log.d(TAG, "Battery saver is active, skipping message catch-up for room $roomToken") Result.success() } @@ -127,11 +127,6 @@ class ChatMessageCatchUpWorker(context: Context, workerParams: WorkerParameters) private fun retryOrFail(): Result = if (runAttemptCount < MAX_RUN_ATTEMPTS - 1) Result.retry() else Result.failure() - private fun isPowerSaveMode(): Boolean { - val powerManager = applicationContext.getSystemService(Context.POWER_SERVICE) as PowerManager - return powerManager.isPowerSaveMode - } - companion object { private val TAG: String = ChatMessageCatchUpWorker::class.java.simpleName private const val CHAT_API_VERSION = 1 diff --git a/app/src/main/java/com/nextcloud/talk/jobs/ConversationsSyncWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/ConversationsSyncWorker.kt new file mode 100644 index 0000000000..9b94ff14f0 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/jobs/ConversationsSyncWorker.kt @@ -0,0 +1,142 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Andy Scherzinger + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.jobs + +import android.content.Context +import android.util.Log +import androidx.annotation.VisibleForTesting +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.ProcessLifecycleOwner +import androidx.work.Constraints +import androidx.work.CoroutineWorker +import androidx.work.ExistingPeriodicWorkPolicy +import androidx.work.NetworkType +import androidx.work.PeriodicWorkRequest +import androidx.work.WorkManager +import androidx.work.WorkerParameters +import autodagger.AutoInjector +import com.nextcloud.talk.application.NextcloudTalkApplication +import com.nextcloud.talk.application.NextcloudTalkApplication.Companion.sharedApplication +import com.nextcloud.talk.conversationlist.data.OfflineConversationsRepository +import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.extensions.isPowerSaveMode +import com.nextcloud.talk.users.UserManager +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.withContext +import java.util.concurrent.TimeUnit +import javax.inject.Inject + +/** + * Periodic worker that syncs the conversation list, and the messages that sync prefetches, for + * every configured account. + * + * Accounts are synced one after another. The run is skipped in battery saver mode and while the app + * is in the foreground, and a run in which any account failed is retried up to [MAX_RUN_ATTEMPTS] + * times. Network availability is enforced by the [NetworkType.CONNECTED] constraint on the request + * rather than checked here. + */ +@AutoInjector(NextcloudTalkApplication::class) +class ConversationsSyncWorker(context: Context, workerParams: WorkerParameters) : + CoroutineWorker(context, workerParams) { + + @Inject + lateinit var userManager: UserManager + + @Inject + lateinit var conversationsRepository: OfflineConversationsRepository + + override suspend fun doWork(): Result { + sharedApplication!!.componentApplication.inject(this) + return sync() + } + + /** Runs the sync, or reports success without syncing when a guard stands the run down. */ + @VisibleForTesting + internal suspend fun sync(): Result = + when { + applicationContext.isPowerSaveMode() -> { + Log.d(TAG, "Battery saver is active, skipping the background conversation sync") + Result.success() + } + + isAppInForeground() -> { + // ponytail: this stands down for any foreground screen, not only the conversation + // list that refreshes itself. Sitting in a chat for an hour therefore refreshes + // nothing until the list is opened again, which syncs on resume anyway. Narrow it + // to the list being resumed if that ever costs someone a stale unread count. + Log.d(TAG, "App is in the foreground, skipping the background conversation sync") + Result.success() + } + + else -> syncAccounts() + } + + private suspend fun syncAccounts(): Result { + // ponytail: every account in one wake-up, sequentially. N accounts cost N room list + // requests plus their message catch-ups; cap the run and take the stalest accounts first if + // that ever shows up in request volume. + val accounts = runCatching { userManager.users.blockingGet() }.getOrElse { throwable -> + Log.e(TAG, "Could not read the accounts to sync", throwable) + emptyList() + } + + if (accounts.isEmpty()) { + Log.d(TAG, "No account to sync") + return Result.success() + } + + val failed = accounts.count { !syncAccount(it) } + + return if (failed == 0) { + Result.success() + } else { + Log.w(TAG, "$failed of ${accounts.size} accounts did not sync (attempt ${runAttemptCount + 1})") + retryOrFail() + } + } + + /** Syncs a single account, returning whether it succeeded. Failures are logged, never thrown. */ + private suspend fun syncAccount(user: User): Boolean = + runCatching { conversationsRepository.syncRooms(user) }.getOrElse { throwable -> + Log.e(TAG, "Background conversation sync failed for account ${user.id}", throwable) + false + } + + private fun retryOrFail(): Result = if (runAttemptCount < MAX_RUN_ATTEMPTS - 1) Result.retry() else Result.failure() + + private suspend fun isAppInForeground(): Boolean = + withContext(Dispatchers.Main.immediate) { + ProcessLifecycleOwner.get().lifecycle.currentState.isAtLeast(Lifecycle.State.STARTED) + } + + companion object { + private val TAG: String = ConversationsSyncWorker::class.java.simpleName + private const val MAX_RUN_ATTEMPTS = 3 + private const val REPEAT_INTERVAL_MINUTES = 15L + const val UNIQUE_WORK_NAME = "PeriodicConversationsSync" + + /** + * Schedules the worker to run every [REPEAT_INTERVAL_MINUTES] minutes while a network is + * available, leaving an already scheduled run in place. + */ + fun schedule(context: Context) { + val work = PeriodicWorkRequest.Builder( + ConversationsSyncWorker::class.java, + REPEAT_INTERVAL_MINUTES, + TimeUnit.MINUTES + ).setConstraints( + Constraints.Builder().setRequiredNetworkType(NetworkType.CONNECTED).build() + ).build() + + WorkManager.getInstance(context).enqueueUniquePeriodicWork( + UNIQUE_WORK_NAME, + ExistingPeriodicWorkPolicy.KEEP, + work + ) + } + } +} From 523a0fa07aa24d866d2630cdec09b4e382d131b6 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 21 Sep 2026 17:15:49 +0200 Subject: [PATCH 3/3] test(conversations): cover the periodic conversation sync worker What is worth pinning down about a background worker is when it stands down and how many accounts it touches when it does not: a guard that quietly stops working is invisible until it turns up as battery drain or request volume, long after the change that broke it. Covers battery saver standing the run down without reading a single account, every configured account being synced rather than only the current one, a failed account asking for another attempt while its neighbours still sync, the attempt cap ending the retries, and an account lookup that throws being survivable. Unit tests gain the WorkManager testing artifact, which was available to instrumented tests only. Assisted-by: Claude Code:claude-opus-5 Signed-off-by: Andy Scherzinger --- app/build.gradle.kts | 1 + .../talk/jobs/ConversationsSyncWorkerTest.kt | 136 ++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 app/src/test/java/com/nextcloud/talk/jobs/ConversationsSyncWorkerTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index af6451084d..7ccd7a7e32 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -372,6 +372,7 @@ dependencies { androidTestImplementation("org.jetbrains.kotlinx:kotlinx-coroutines-test:1.11.0") androidTestImplementation("androidx.test:core-ktx:1.7.0") androidTestImplementation("org.mockito:mockito-android:5.22.0") + testImplementation("androidx.work:work-testing:$workVersion") androidTestImplementation("androidx.work:work-testing:$workVersion") androidTestImplementation("androidx.test.espresso:espresso-core:$espressoVersion") { exclude(group = "com.android.support", module = "support-annotations") diff --git a/app/src/test/java/com/nextcloud/talk/jobs/ConversationsSyncWorkerTest.kt b/app/src/test/java/com/nextcloud/talk/jobs/ConversationsSyncWorkerTest.kt new file mode 100644 index 0000000000..6250114ba6 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/jobs/ConversationsSyncWorkerTest.kt @@ -0,0 +1,136 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Andy Scherzinger + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +package com.nextcloud.talk.jobs + +import android.app.Application +import android.content.Context +import android.os.PowerManager +import androidx.test.core.app.ApplicationProvider +import androidx.work.ListenableWorker +import androidx.work.testing.TestListenableWorkerBuilder +import com.nextcloud.talk.conversationlist.data.OfflineConversationsRepository +import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.users.UserManager +import io.reactivex.Single +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.kotlin.any +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyBlocking +import org.mockito.kotlin.whenever +import org.mockito.kotlin.wheneverBlocking +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config + +/** + * Tests for [ConversationsSyncWorker]: which accounts a run syncs, when a run stands down without + * syncing, and how a failed run reports itself. + */ +@RunWith(RobolectricTestRunner::class) +@Config(application = Application::class, sdk = [33]) +class ConversationsSyncWorkerTest { + + private val userManager: UserManager = mock() + private val repository: OfflineConversationsRepository = mock() + + @Test + fun `every account is synced, not just the current one`() { + val worker = worker() + whenever(userManager.users).thenReturn(Single.just(listOf(user(1), user(2), user(3)))) + wheneverBlocking { repository.syncRooms(any(), any()) }.thenReturn(true) + + val result = runBlocking { worker.sync() } + + assertEquals(ListenableWorker.Result.success(), result) + verifyBlocking(repository) { syncRooms(user(1), false) } + verifyBlocking(repository) { syncRooms(user(2), false) } + verifyBlocking(repository) { syncRooms(user(3), false) } + } + + @Test + fun `battery saver stands the sync down entirely`() { + val worker = worker() + shadowOf(applicationContext().getSystemService(Context.POWER_SERVICE) as PowerManager) + .setIsPowerSaveMode(true) + + val result = runBlocking { worker.sync() } + + assertEquals(ListenableWorker.Result.success(), result) + verify(userManager, never()).users + verifyBlocking(repository, never()) { syncRooms(any(), any()) } + } + + @Test + fun `an account with nothing to sync is not an error`() { + val worker = worker() + whenever(userManager.users).thenReturn(Single.just(emptyList())) + + val result = runBlocking { worker.sync() } + + assertEquals(ListenableWorker.Result.success(), result) + verifyBlocking(repository, never()) { syncRooms(any(), any()) } + } + + @Test + fun `a failed account asks for another attempt`() { + val worker = worker(runAttempt = 0) + whenever(userManager.users).thenReturn(Single.just(listOf(user(1), user(2)))) + wheneverBlocking { repository.syncRooms(user(1), false) }.thenReturn(true) + wheneverBlocking { repository.syncRooms(user(2), false) }.thenReturn(false) + + val result = runBlocking { worker.sync() } + + assertEquals(ListenableWorker.Result.retry(), result) + // the account that did sync was still synced, rather than abandoned with its neighbour + verifyBlocking(repository) { syncRooms(user(1), false) } + } + + @Test + fun `a sync that keeps failing gives up instead of retrying for ever`() { + val worker = worker(runAttempt = 2) + whenever(userManager.users).thenReturn(Single.just(listOf(user(1)))) + wheneverBlocking { repository.syncRooms(any(), any()) }.thenReturn(false) + + val result = runBlocking { worker.sync() } + + assertEquals(ListenableWorker.Result.failure(), result) + } + + @Test + fun `an account whose lookup throws does not take the other accounts down with it`() { + val worker = worker() + whenever(userManager.users).thenReturn(Single.error(IllegalStateException("database is gone"))) + + val result = runBlocking { worker.sync() } + + assertEquals(ListenableWorker.Result.success(), result) + verifyBlocking(repository, never()) { syncRooms(any(), any()) } + } + + private fun worker(runAttempt: Int = 0): ConversationsSyncWorker = + TestListenableWorkerBuilder(applicationContext()) + .setRunAttemptCount(runAttempt) + .build() + .also { + it.userManager = userManager + it.conversationsRepository = repository + } + + private fun applicationContext(): Context = ApplicationProvider.getApplicationContext() + + private fun user(id: Long): User = User(id = id, userId = "user$id", username = "user$id", baseUrl = BASE_URL) + + companion object { + private const val BASE_URL = "https://server.example.com" + } +}