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