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 55aeb31156..b5407b3da3 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 @@ -12,7 +12,10 @@ import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import androidx.work.WorkInfo +import androidx.work.WorkManager import com.nextcloud.talk.R +import com.nextcloud.talk.application.NextcloudTalkApplication import com.nextcloud.talk.arbitrarystorage.ArbitraryStorageManager import com.nextcloud.talk.contacts.ContactsRepository import com.nextcloud.talk.conversationlist.data.OfflineConversationsRepository @@ -21,6 +24,8 @@ import com.nextcloud.talk.conversationlist.ui.ConversationListEntry import com.nextcloud.talk.data.user.model.User import com.nextcloud.talk.invitation.data.InvitationsModel import com.nextcloud.talk.invitation.data.InvitationsRepository +import com.nextcloud.talk.jobs.ConversationActionWorker +import com.nextcloud.talk.jobs.ConversationActionWorker.ConversationAction import com.nextcloud.talk.messagesearch.MessageSearchHelper import com.nextcloud.talk.messagesearch.MessageSearchHelper.MessageSearchResults import com.nextcloud.talk.models.domain.ConversationModel @@ -56,6 +61,8 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.launchIn @@ -800,29 +807,15 @@ class ConversationsListViewModel @Inject constructor( } } - @Suppress("Detekt.TooGenericExceptionCaught") fun markConversationAsUnread(conversation: ConversationModel) { - val original = conversation.copy() - val optimistic = conversation.copy(unreadMessages = 1) - val apiVersion = ApiUtils.getChatApiVersion( - currentUser.capabilities?.spreedCapability!!, - intArrayOf(ApiUtils.API_V1) - ) - val url = ApiUtils.getUrlForChatReadMarker(apiVersion, currentUser.baseUrl, conversation.token) viewModelScope.launch { - val result = optimisticAction( - apply = { - conversationListUpdater.markPendingUnread(conversation.internalId) - applyLocally(optimistic) - revertTo(original) { conversationListUpdater.clearPendingUnread(conversation.internalId) } - }, - request = { conversationsRepository.markConversationAsUnread(credentials, url) } - ) + conversationListUpdater.markPendingUnread(conversation.internalId) + applyLocally(conversation.copy(unreadMessages = 1)) - _readUnreadState.value = result.fold( - onSuccess = { ConversationReadUnreadUiState.Success }, - onFailure = { ConversationReadUnreadUiState.Error } - ) + handOver(conversation, ConversationAction.MARK_UNREAD) { + _readUnreadState.value = ConversationReadUnreadUiState.Error + } + _readUnreadState.value = ConversationReadUnreadUiState.Success } } @@ -848,85 +841,70 @@ class ConversationsListViewModel @Inject constructor( _archiveState.value = ArchiveUiState.None } - @Suppress("Detekt.TooGenericExceptionCaught") fun toggleConversationArchive(conversation: ConversationModel) { - val original = conversation.copy() val desiredArchived = !conversation.hasArchived - val optimistic = conversation.copy(hasArchived = desiredArchived) - val apiVersion = ApiUtils.getConversationApiVersion(currentUser, intArrayOf(ApiUtils.API_V4, ApiUtils.API_V1)) - val url = ApiUtils.getUrlForArchive(apiVersion, currentUser.baseUrl, conversation.token) + viewModelScope.launch { - val result = optimisticAction( - apply = { - conversationListUpdater.markPendingArchived(conversation.internalId, desiredArchived) - applyLocally(optimistic) - revertTo(original) { - conversationListUpdater.clearPendingArchived(conversation.internalId, desiredArchived) - } - }, - request = { - if (desiredArchived) { - conversationsRepository.archiveConversation(credentials, url) - } else { - conversationsRepository.unarchiveConversation(credentials, url) - } - } - ) + conversationListUpdater.markPendingArchived(conversation.internalId, desiredArchived) + applyLocally(conversation.copy(hasArchived = desiredArchived)) - _archiveState.value = result.fold( - onSuccess = { ArchiveUiState.Success(desiredArchived, conversation.displayName) }, - onFailure = { ArchiveUiState.Error } - ) + handOver(conversation, ConversationAction.ARCHIVE, desiredArchived) { + _archiveState.value = ArchiveUiState.Error + } + _archiveState.value = ArchiveUiState.Success(desiredArchived, conversation.displayName) } } - @Suppress("Detekt.TooGenericExceptionCaught") fun addConversationToFavorites(conversation: ConversationModel) { - val original = conversation.copy() - val optimistic = conversation.copy(favorite = true) - val apiVersion = ApiUtils.getConversationApiVersion(currentUser, intArrayOf(ApiUtils.API_V4, ApiUtils.API_V1)) - val url = ApiUtils.getUrlForRoomFavorite(apiVersion, currentUser.baseUrl, conversation.token) + setFavorite(conversation, favorite = true) + } + + fun removeConversationFromFavorites(conversation: ConversationModel) { + setFavorite(conversation, favorite = false) + } + + private fun setFavorite(conversation: ConversationModel, favorite: Boolean) { viewModelScope.launch { - val result = optimisticAction( - apply = { - conversationListUpdater.markPendingFavorite(conversation.internalId, favorite = true) - applyLocally(optimistic) - revertTo(original) { - conversationListUpdater.clearPendingFavorite(conversation.internalId, favorite = true) - } - }, - request = { conversationsRepository.addConversationToFavorites(credentials, url) } - ) + conversationListUpdater.markPendingFavorite(conversation.internalId, favorite) + applyLocally(conversation.copy(favorite = favorite)) - _favoriteState.value = result.fold( - onSuccess = { FavoriteUiState.Success }, - onFailure = { FavoriteUiState.Error } - ) + handOver(conversation, ConversationAction.FAVORITE, favorite) { + _favoriteState.value = FavoriteUiState.Error + } + _favoriteState.value = FavoriteUiState.Success } } - @Suppress("Detekt.TooGenericExceptionCaught") - fun removeConversationFromFavorites(conversation: ConversationModel) { - val original = conversation.copy() - val optimistic = conversation.copy(favorite = false) - val apiVersion = ApiUtils.getConversationApiVersion(currentUser, intArrayOf(ApiUtils.API_V4, ApiUtils.API_V1)) - val url = ApiUtils.getUrlForRoomFavorite(apiVersion, currentUser.baseUrl, conversation.token) + /** + * Hands the change to [ConversationActionWorker], which sends it even if this screen or the whole + * process is gone by then. While this screen is still around, a change the worker finally gave up + * on puts [conversation] back and reports itself through [onFailed]; when it is not, the worker + * has released the guard and the next room list sync brings the server state back instead. + */ + private fun handOver( + conversation: ConversationModel, + action: ConversationAction, + enabled: Boolean = true, + onFailed: () -> Unit + ) { + val context = NextcloudTalkApplication.sharedApplication!!.applicationContext + val workId = ConversationActionWorker.enqueue( + context = context, + userId = currentUser.id!!, + roomToken = conversation.token, + action = action, + enabled = enabled + ) + viewModelScope.launch { - val result = optimisticAction( - apply = { - conversationListUpdater.markPendingFavorite(conversation.internalId, favorite = false) - applyLocally(optimistic) - revertTo(original) { - conversationListUpdater.clearPendingFavorite(conversation.internalId, favorite = false) - } - }, - request = { conversationsRepository.removeConversationFromFavorites(credentials, url) } - ) + val outcome = WorkManager.getInstance(context).getWorkInfoByIdFlow(workId) + .filterNotNull() + .first { it.state.isFinished } - _favoriteState.value = result.fold( - onSuccess = { FavoriteUiState.Success }, - onFailure = { FavoriteUiState.Error } - ) + if (outcome.state == WorkInfo.State.FAILED) { + applyLocally(conversation) + onFailed() + } } } diff --git a/app/src/main/java/com/nextcloud/talk/jobs/ConversationActionWorker.kt b/app/src/main/java/com/nextcloud/talk/jobs/ConversationActionWorker.kt new file mode 100644 index 0000000000..78229e2be8 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/jobs/ConversationActionWorker.kt @@ -0,0 +1,221 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package com.nextcloud.talk.jobs + +import android.content.Context +import android.util.Log +import androidx.work.BackoffPolicy +import androidx.work.Constraints +import androidx.work.CoroutineWorker +import androidx.work.Data +import androidx.work.ExistingWorkPolicy +import androidx.work.NetworkType +import androidx.work.OneTimeWorkRequest +import androidx.work.WorkManager +import androidx.work.WorkRequest +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.network.ConversationListUpdater +import com.nextcloud.talk.data.user.model.User +import com.nextcloud.talk.repositories.conversations.ConversationsRepository +import com.nextcloud.talk.users.UserManager +import com.nextcloud.talk.utils.ApiUtils +import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_INTERNAL_USER_ID +import com.nextcloud.talk.utils.bundle.BundleKeys.KEY_ROOM_TOKEN +import java.util.UUID +import java.util.concurrent.TimeUnit +import javax.inject.Inject + +/** + * Sends a conversation change the user made in the list - marking it unread, favoriting it, + * archiving it - to the server, the way [ReadMarkerSyncWorker] sends the read marker. + * + * The caller writes the change to the local conversation entry first, so the list reacts to the tap, + * and hands the request over here. That buys two things the caller cannot do on its own: the change + * outlives the process, and a change made offline waits for a connection instead of being reverted + * within a second of a tap the user meant. + * + * Every action here sets a value rather than moving one, so repeating it changes nothing and a retry + * is always safe. Work is unique per conversation and action with [ExistingWorkPolicy.REPLACE], so + * favoriting and unfavoriting in quick succession leaves only the last intent to be sent. + * + * When the attempts are used up, the pending guard is released and the server state applies again at + * the next room list sync - the same fallback [ReadMarkerSyncWorker] uses, rather than a local revert + * that would have to guess what the entry looked like before. + */ +@AutoInjector(NextcloudTalkApplication::class) +class ConversationActionWorker(context: Context, workerParams: WorkerParameters) : + CoroutineWorker(context, workerParams) { + + @Inject + lateinit var userManager: UserManager + + @Inject + lateinit var conversationsRepository: ConversationsRepository + + @Inject + lateinit var conversationListUpdater: ConversationListUpdater + + enum class ConversationAction { + MARK_UNREAD, + FAVORITE, + ARCHIVE + } + + override suspend fun doWork(): Result { + sharedApplication!!.componentApplication.inject(this) + + val userId = inputData.getLong(KEY_INTERNAL_USER_ID, -1) + val roomToken = inputData.getString(KEY_ROOM_TOKEN) + val action = actionOf(inputData.getString(KEY_ACTION)) + + if (userId < 0 || roomToken.isNullOrEmpty() || action == null) { + Log.e(TAG, "Missing user id, room token or action, dropping the conversation action") + return Result.failure() + } + + return send(userId, roomToken, action, inputData.getBoolean(KEY_ENABLED, true)) + } + + private suspend fun send(userId: Long, roomToken: String, action: ConversationAction, enabled: Boolean): Result { + val user = userManager.getUserWithId(userId).blockingGet() + val credentials = user?.let { ApiUtils.getCredentials(it.username, it.token) } + + val sent = when { + user == null || credentials == null -> { + Log.e(TAG, "No user or credentials found for user id $userId, dropping the conversation action") + false + } + + else -> runCatching { + perform(user, credentials, roomToken, action, enabled) + }.onFailure { throwable -> + Log.w(TAG, "$action for room $roomToken could not be sent: $throwable") + }.isSuccess + } + + return when { + sent -> { + Log.d(TAG, "$action sent for room $roomToken") + releaseGuard(userId, roomToken, action, enabled) + Result.success() + } + + // a missing user is not worth another attempt, it will still be missing + credentials != null && runAttemptCount < MAX_RUN_ATTEMPTS - 1 -> Result.retry() + + else -> giveUp(userId, roomToken, action, enabled) + } + } + + private suspend fun perform( + user: User, + credentials: String, + roomToken: String, + action: ConversationAction, + enabled: Boolean + ) { + val apiVersion = ApiUtils.getConversationApiVersion(user, intArrayOf(ApiUtils.API_V4, ApiUtils.API_V1)) + + when (action) { + ConversationAction.MARK_UNREAD -> { + val chatApiVersion = ApiUtils.getChatApiVersion( + user.capabilities!!.spreedCapability!!, + intArrayOf(ApiUtils.API_V1) + ) + val url = ApiUtils.getUrlForChatReadMarker(chatApiVersion, user.baseUrl, roomToken) + conversationsRepository.markConversationAsUnread(credentials, url) + } + + ConversationAction.FAVORITE -> { + val url = ApiUtils.getUrlForRoomFavorite(apiVersion, user.baseUrl, roomToken) + if (enabled) { + conversationsRepository.addConversationToFavorites(credentials, url) + } else { + conversationsRepository.removeConversationFromFavorites(credentials, url) + } + } + + ConversationAction.ARCHIVE -> { + val url = ApiUtils.getUrlForArchive(apiVersion, user.baseUrl, roomToken) + if (enabled) { + conversationsRepository.archiveConversation(credentials, url) + } else { + conversationsRepository.unarchiveConversation(credentials, url) + } + } + } + } + + private fun releaseGuard(userId: Long, roomToken: String, action: ConversationAction, enabled: Boolean) { + val internalConversationId = "$userId@$roomToken" + when (action) { + ConversationAction.MARK_UNREAD -> conversationListUpdater.clearPendingUnread(internalConversationId) + ConversationAction.FAVORITE -> + conversationListUpdater.clearPendingFavorite(internalConversationId, enabled) + ConversationAction.ARCHIVE -> + conversationListUpdater.clearPendingArchived(internalConversationId, enabled) + } + } + + private fun giveUp(userId: Long, roomToken: String, action: ConversationAction, enabled: Boolean): Result { + releaseGuard(userId, roomToken, action, enabled) + return Result.failure() + } + + companion object { + private val TAG: String = ConversationActionWorker::class.java.simpleName + private const val KEY_ACTION = "KEY_ACTION" + private const val KEY_ENABLED = "KEY_ENABLED" + private const val MAX_RUN_ATTEMPTS = 3 + + /** + * Hands the change over and returns the id of the work that carries it, so a caller that is + * still on screen can tell the user when it finally did not go through. + */ + fun enqueue( + context: Context, + userId: Long, + roomToken: String, + action: ConversationAction, + enabled: Boolean = true + ): UUID { + val data = Data.Builder() + .putLong(KEY_INTERNAL_USER_ID, userId) + .putString(KEY_ROOM_TOKEN, roomToken) + .putString(KEY_ACTION, action.name) + .putBoolean(KEY_ENABLED, enabled) + .build() + + val work = OneTimeWorkRequest.Builder(ConversationActionWorker::class.java) + .setInputData(data) + .setConstraints(Constraints.Builder().setRequiredNetworkType(NetworkType.CONNECTED).build()) + .setBackoffCriteria(BackoffPolicy.EXPONENTIAL, WorkRequest.MIN_BACKOFF_MILLIS, TimeUnit.MILLISECONDS) + .build() + + WorkManager.getInstance(context).enqueueUniqueWork( + uniqueWorkName(userId, roomToken, action), + ExistingWorkPolicy.REPLACE, + work + ) + + return work.id + } + + /** + * The name that decides what replaces what: the same action on the same conversation supersedes + * an older intent, while a different action on it is carried separately. + */ + fun uniqueWorkName(userId: Long, roomToken: String, action: ConversationAction): String = + "conversation-action-${action.name}-$userId@$roomToken" + + /** The action an enqueued work item carries, or null when it carries nothing we know. */ + fun actionOf(name: String?): ConversationAction? = ConversationAction.entries.firstOrNull { it.name == name } + } +} diff --git a/app/src/test/java/com/nextcloud/talk/jobs/ConversationActionWorkerTest.kt b/app/src/test/java/com/nextcloud/talk/jobs/ConversationActionWorkerTest.kt new file mode 100644 index 0000000000..7c649a7d8e --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/jobs/ConversationActionWorkerTest.kt @@ -0,0 +1,59 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +package com.nextcloud.talk.jobs + +import com.nextcloud.talk.jobs.ConversationActionWorker.ConversationAction +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** + * What the enqueued work is named decides which change supersedes which, and the action travels to + * the worker as a string. Both are easy to break by renaming and hard to notice at runtime, because + * a change that is dropped looks exactly like one the server refused. + */ +class ConversationActionWorkerTest { + + @Test + fun `favoriting the same conversation twice replaces the older intent`() { + assertEquals( + ConversationActionWorker.uniqueWorkName(1, "room1", ConversationAction.FAVORITE), + ConversationActionWorker.uniqueWorkName(1, "room1", ConversationAction.FAVORITE) + ) + } + + @Test + fun `archiving a conversation does not replace marking it unread`() { + assertNotEquals( + ConversationActionWorker.uniqueWorkName(1, "room1", ConversationAction.ARCHIVE), + ConversationActionWorker.uniqueWorkName(1, "room1", ConversationAction.MARK_UNREAD) + ) + } + + @Test + fun `the same conversation token on two accounts is two changes`() { + assertNotEquals( + ConversationActionWorker.uniqueWorkName(1, "room1", ConversationAction.ARCHIVE), + ConversationActionWorker.uniqueWorkName(2, "room1", ConversationAction.ARCHIVE) + ) + } + + @Test + fun `every action survives the trip through the work input`() { + ConversationAction.entries.forEach { action -> + assertEquals(action, ConversationActionWorker.actionOf(action.name)) + } + } + + @Test + fun `an action the worker does not know is dropped rather than guessed`() { + assertNull(ConversationActionWorker.actionOf("SOMETHING_ELSE")) + assertNull(ConversationActionWorker.actionOf(null)) + } +}