diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt index deed6e0dbc..f3f0173ebd 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt @@ -32,8 +32,7 @@ import com.nextcloud.talk.models.json.generic.GenericOverall import com.nextcloud.talk.models.json.participants.Participant import com.nextcloud.talk.utils.bundle.BundleKeys import com.nextcloud.talk.utils.message.SendMessageUtils -import com.nextcloud.talk.utils.revertOnCancellation -import com.nextcloud.talk.utils.withRetry +import com.nextcloud.talk.utils.optimisticAction import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow @@ -802,31 +801,24 @@ class OfflineFirstChatRepository @Inject constructor( messageId: Long, text: String ): Result { - val restore = applyLocalEdit(messageId, text) + val result = optimisticAction( + apply = { applyLocalEdit(messageId, text) }, + isConfirmed = ::editAccepted, + request = { network.editChatMessage(credentials, url, text) } + ) - return try { - val response = revertOnCancellation({ restore?.invoke() }) { - withRetry(retries = 1, initialDelayMillis = RETRY_DELAY_MS, retryOn = ::isRetryable) { - network.editChatMessage(credentials, url, text) - } - } - val statusCode = response.ocs?.meta?.statusCode - if (statusCode != null && statusCode != HTTP_OK) { - // the server refused the edit, e.g. because the message is too old - restore?.invoke() - } else { - persistEditedMessage(messageId, response) - } - Result.success(response) - } catch (e: HttpException) { - restore?.invoke() - Result.failure(e) - } catch (e: IOException) { - restore?.invoke() - Result.failure(e) - } + result.getOrNull()?.takeIf(::editAccepted)?.let { persistEditedMessage(messageId, it) } + + return result } + /** + * A status code other than 200 means the server refused the edit, for instance because the + * message is too old, and the optimistic edit has to be taken back. + */ + private fun editAccepted(response: ChatOverallSingleMessage): Boolean = + response.ocs?.meta?.statusCode.let { it == null || it == HTTP_OK } + /** * Writes [text] into the cached message together with the edit metadata the bubble shows, and * returns the action that restores the previous version. The revert only applies while the message @@ -884,29 +876,19 @@ class OfflineFirstChatRepository @Inject constructor( url: String, messageId: Long, deletedPlaceholder: String - ): Result { - val restore = applyLocalDeletion(messageId, deletedPlaceholder) - - return try { - val response = revertOnCancellation({ restore?.invoke() }) { - withRetry(retries = 1, initialDelayMillis = RETRY_DELAY_MS, retryOn = ::isRetryable) { + ): Result = + optimisticAction( + apply = { applyLocalDeletion(messageId, deletedPlaceholder) }, + request = { + try { network.deleteChatMessage(credentials, url) + } catch (e: HttpException) { + // the server does not know the message any more, so it is gone either way and the + // local deletion stands + if (e.code() == HTTP_NOT_FOUND) null else throw e } } - Result.success(response) - } catch (e: HttpException) { - if (e.code() == HTTP_NOT_FOUND) { - // the server does not know the message any more, so it is gone either way - Result.success(null) - } else { - restore?.invoke() - Result.failure(e) - } - } catch (e: IOException) { - restore?.invoke() - Result.failure(e) - } - } + ) /** * Renders the message as deleted in the local database and returns the action that puts it back, @@ -1006,12 +988,6 @@ class OfflineFirstChatRepository @Inject constructor( network.unPinMessage(credentials, url) } - private fun isRetryable(error: Exception): Boolean = - when (error) { - is HttpException -> error.code() == HTTP_TOO_MANY_REQUESTS || error.code() >= HTTP_INTERNAL_SERVER_ERROR - else -> error is IOException - } - private suspend fun withLocalPinnedMessage( messageId: Long, pinned: Boolean, @@ -1025,23 +1001,14 @@ class OfflineFirstChatRepository @Inject constructor( null } - return try { - val overall = revertOnCancellation({ restore?.invoke() }) { - withRetry(retries = 1, initialDelayMillis = RETRY_DELAY_MS, retryOn = ::isRetryable) { request() } + return optimisticAction(apply = { restore }, request = request) + .onSuccess { + // the room refresh that follows is computed after this change reached the server, so + // the guard has done its job and must not outlive the request + conversationListUpdater.clearPendingPinnedMessage(internalConversationId) } - // the room refresh that follows is computed after this change reached the server, so the - // guard has done its job and must not outlive the request - conversationListUpdater.clearPendingPinnedMessage(internalConversationId) - Result.success(overall.ocs?.data?.toDomainModel()) - } catch (e: HttpException) { - Log.e(TAG, "Error while pinning or unpinning a message: $e") - restore?.invoke() - Result.failure(e) - } catch (e: IOException) { - Log.e(TAG, "Error while pinning or unpinning a message: $e") - restore?.invoke() - Result.failure(e) - } + .onFailure { Log.e(TAG, "Error while pinning or unpinning a message: $it") } + .map { it.ocs?.data?.toDomainModel() } } private fun isChatDataInitialized(): Boolean = @@ -1055,23 +1022,10 @@ class OfflineFirstChatRepository @Inject constructor( null } - return try { - revertOnCancellation({ restore?.invoke() }) { - withRetry(retries = 1, initialDelayMillis = RETRY_DELAY_MS, retryOn = ::isRetryable) { - network.hidePinnedMessage(credentials, url) - } - } - conversationListUpdater.clearPendingHiddenPinnedMessage(internalConversationId) - Result.success(true) - } catch (e: HttpException) { - Log.e(TAG, "Error while hiding the pinned message: $e") - restore?.invoke() - Result.failure(e) - } catch (e: IOException) { - Log.e(TAG, "Error while hiding the pinned message: $e") - restore?.invoke() - Result.failure(e) - } + return optimisticAction(apply = { restore }, request = { network.hidePinnedMessage(credentials, url) }) + .onSuccess { conversationListUpdater.clearPendingHiddenPinnedMessage(internalConversationId) } + .onFailure { Log.e(TAG, "Error while hiding the pinned message: $it") } + .map { true } } override suspend fun onSignalingChatMessageReceived(chatMessages: List) { @@ -1286,8 +1240,5 @@ class OfflineFirstChatRepository @Inject constructor( private const val MESSAGE_TYPE_DELETED = "comment_deleted" private const val HTTP_OK = 200 private const val HTTP_NOT_FOUND = 404 - private const val HTTP_TOO_MANY_REQUESTS = 429 - private const val HTTP_INTERNAL_SERVER_ERROR = 500 - private const val RETRY_DELAY_MS = 500L } } diff --git a/app/src/main/java/com/nextcloud/talk/conversationinfo/viewmodel/ConversationInfoViewModel.kt b/app/src/main/java/com/nextcloud/talk/conversationinfo/viewmodel/ConversationInfoViewModel.kt index 33c23580c0..976f4886be 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationinfo/viewmodel/ConversationInfoViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationinfo/viewmodel/ConversationInfoViewModel.kt @@ -50,6 +50,7 @@ import com.nextcloud.talk.utils.DateConstants import com.nextcloud.talk.utils.DisplayUtils import com.nextcloud.talk.utils.ParticipantRoleUtils import com.nextcloud.talk.utils.SpreedFeatures +import com.nextcloud.talk.utils.optimisticAction import com.nextcloud.talk.utils.preferences.preferencestorage.DatabaseStorageModule import io.reactivex.Observer import io.reactivex.android.schedulers.AndroidSchedulers @@ -806,10 +807,17 @@ class ConversationInfoViewModel @Inject constructor( } fun toggleCallNotifications() { - val newEnabled = !_uiState.value.callNotificationsEnabled - _uiState.update { it.copy(callNotificationsEnabled = newEnabled) } + val previousEnabled = _uiState.value.callNotificationsEnabled + val newEnabled = !previousEnabled + viewModelScope.launch { - databaseStorageModule?.saveBoolean("call_notifications_switch", newEnabled) + optimisticAction( + apply = { + _uiState.update { it.copy(callNotificationsEnabled = newEnabled) } + suspend { _uiState.update { it.copy(callNotificationsEnabled = previousEnabled) } } + }, + request = { databaseStorageModule?.saveBoolean("call_notifications_switch", newEnabled) } + ).onFailure { throwable -> reportSettingFailure("call notifications", throwable) } } } @@ -817,11 +825,22 @@ class ConversationInfoViewModel @Inject constructor( val res = NextcloudTalkApplication.sharedApplication!!.resources val values = res.getStringArray(R.array.message_notification_levels_entry_values) val descriptions = res.getStringArray(R.array.message_notification_levels) - if (position in values.indices && position in descriptions.indices) { - _uiState.update { it.copy(notificationLevel = descriptions[position]) } - viewModelScope.launch { - databaseStorageModule?.saveString("conversation_info_message_notifications_dropdown", values[position]) - } + if (position !in values.indices || position !in descriptions.indices) return + + val previousLevel = _uiState.value.notificationLevel + viewModelScope.launch { + optimisticAction( + apply = { + _uiState.update { it.copy(notificationLevel = descriptions[position]) } + suspend { _uiState.update { it.copy(notificationLevel = previousLevel) } } + }, + request = { + databaseStorageModule?.saveString( + "conversation_info_message_notifications_dropdown", + values[position] + ) + } + ).onFailure { throwable -> reportSettingFailure("the notification level", throwable) } } } @@ -829,14 +848,25 @@ class ConversationInfoViewModel @Inject constructor( val res = NextcloudTalkApplication.sharedApplication!!.resources val values = res.getStringArray(R.array.message_expiring_values) val descriptions = res.getStringArray(R.array.message_expiring_descriptions) - if (position in values.indices && position in descriptions.indices) { - _uiState.update { it.copy(messageExpirationLabel = descriptions[position]) } - viewModelScope.launch { - databaseStorageModule?.saveString("conversation_settings_dropdown", values[position]) - } + if (position !in values.indices || position !in descriptions.indices) return + + val previousLabel = _uiState.value.messageExpirationLabel + viewModelScope.launch { + optimisticAction( + apply = { + _uiState.update { it.copy(messageExpirationLabel = descriptions[position]) } + suspend { _uiState.update { it.copy(messageExpirationLabel = previousLabel) } } + }, + request = { databaseStorageModule?.saveString("conversation_settings_dropdown", values[position]) } + ).onFailure { throwable -> reportSettingFailure("the message expiration", throwable) } } } + private suspend fun reportSettingFailure(setting: String, throwable: Throwable) { + _uiEvent.emit(ConversationInfoUiEvent.ShowSnackbar(R.string.nc_common_error_sorry)) + Log.e(TAG, "failed to save $setting", throwable) + } + fun setUpcomingEvent(summary: String?, time: String?) { _uiState.update { it.copy(upcomingEventSummary = summary, upcomingEventTime = time) } } @@ -857,18 +887,23 @@ class ConversationInfoViewModel @Inject constructor( fun toggleImportantConversation(credentials: String, baseUrl: String, roomToken: String) { val previousValue = _uiState.value.importantConversation val newValue = !previousValue - _uiState.update { it.copy(importantConversation = newValue) } + viewModelScope.launch { - try { - if (newValue) { - conversationsRepository.markConversationAsImportant(credentials, baseUrl, roomToken) - } else { - conversationsRepository.markConversationAsUnImportant(credentials, baseUrl, roomToken) + optimisticAction( + apply = { + _uiState.update { it.copy(importantConversation = newValue) } + suspend { _uiState.update { it.copy(importantConversation = previousValue) } } + }, + request = { + if (newValue) { + conversationsRepository.markConversationAsImportant(credentials, baseUrl, roomToken) + } else { + conversationsRepository.markConversationAsUnImportant(credentials, baseUrl, roomToken) + } } - } catch (exception: Exception) { - _uiState.update { it.copy(importantConversation = previousValue) } + ).onFailure { throwable -> _uiEvent.emit(ConversationInfoUiEvent.ShowSnackbar(R.string.nc_common_error_sorry)) - Log.e(TAG, "failed to toggle important conversation state", exception) + Log.e(TAG, "failed to toggle important conversation state", throwable) } } } @@ -877,18 +912,23 @@ class ConversationInfoViewModel @Inject constructor( fun toggleSensitiveConversation(credentials: String, baseUrl: String, roomToken: String) { val previousValue = _uiState.value.sensitiveConversation val newValue = !previousValue - _uiState.update { it.copy(sensitiveConversation = newValue) } + viewModelScope.launch { - try { - if (newValue) { - conversationsRepository.markConversationAsSensitive(credentials, baseUrl, roomToken) - } else { - conversationsRepository.markConversationAsInsensitive(credentials, baseUrl, roomToken) + optimisticAction( + apply = { + _uiState.update { it.copy(sensitiveConversation = newValue) } + suspend { _uiState.update { it.copy(sensitiveConversation = previousValue) } } + }, + request = { + if (newValue) { + conversationsRepository.markConversationAsSensitive(credentials, baseUrl, roomToken) + } else { + conversationsRepository.markConversationAsInsensitive(credentials, baseUrl, roomToken) + } } - } catch (exception: Exception) { - _uiState.update { it.copy(sensitiveConversation = previousValue) } + ).onFailure { throwable -> _uiEvent.emit(ConversationInfoUiEvent.ShowSnackbar(R.string.nc_common_error_sorry)) - Log.e(TAG, "failed to toggle sensitive conversation state", exception) + Log.e(TAG, "failed to toggle sensitive conversation state", throwable) } } } 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 1b41c8082c..ae3c5dbd05 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/ConversationsListActivity.kt @@ -1241,7 +1241,7 @@ class ConversationsListActivity : BaseActivity() { @SuppressLint("StringFormatInvalid") private fun showLeaveConversationSnackbar(conversation: ConversationModel) { val token = conversation.token ?: return - conversationsListViewModel.markConversationPendingLeave(token) + conversationsListViewModel.markConversationPendingRemoval(token) lifecycleScope.launch { val result = snackbarHostState.showSnackbar( message = String.format(resources.getString(R.string.left_conversation), conversation.displayName), @@ -1249,7 +1249,7 @@ class ConversationsListActivity : BaseActivity() { duration = SnackbarDuration.Long ) when (result) { - SnackbarResult.ActionPerformed -> conversationsListViewModel.clearConversationPendingLeave(token) + SnackbarResult.ActionPerformed -> conversationsListViewModel.clearConversationPendingRemoval(token) SnackbarResult.Dismissed -> leaveConversation(conversation) } } @@ -1277,11 +1277,11 @@ class ConversationsListActivity : BaseActivity() { resources.getString(R.string.nc_shortcut_conversation_deleted) ) } - conversationsListViewModel.clearConversationPendingLeave(token) + conversationsListViewModel.clearConversationPendingRemoval(token) fetchRooms() } WorkInfo.State.FAILED -> { - conversationsListViewModel.clearConversationPendingLeave(token) + conversationsListViewModel.clearConversationPendingRemoval(token) showSnackbar(resources.getString(R.string.nc_common_error_sorry)) } else -> {} @@ -1298,6 +1298,7 @@ class ConversationsListActivity : BaseActivity() { .setTitle(R.string.nc_delete_call) .setMessage(R.string.nc_delete_conversation_more) .setPositiveButton(R.string.nc_delete) { _, _ -> + conversation.token?.let { conversationsListViewModel.markConversationPendingRemoval(it) } deleteConversation(conversation) } .setNegativeButton(R.string.nc_cancel) { _, _ -> @@ -1515,31 +1516,31 @@ class ConversationsListActivity : BaseActivity() { WorkManager.getInstance(context).getWorkInfoByIdLiveData(deleteConversationWorker.id) .observeForever { workInfo: WorkInfo? -> - if (workInfo != null) { - when (workInfo.state) { - WorkInfo.State.SUCCEEDED -> { - currentUser?.id?.let { userId -> - ShortcutManagerHelper.disableConversationShortcut( - context, - conversation.token, - userId, - context.resources.getString(R.string.nc_shortcut_conversation_deleted) - ) - } - showSnackbar( - String.format( - context.resources.getString(R.string.deleted_conversation), - conversation.displayName - ) + when (workInfo?.state) { + WorkInfo.State.SUCCEEDED -> { + currentUser?.id?.let { userId -> + ShortcutManagerHelper.disableConversationShortcut( + context, + conversation.token, + userId, + context.resources.getString(R.string.nc_shortcut_conversation_deleted) ) } + conversation.token?.let { conversationsListViewModel.clearConversationPendingRemoval(it) } + showSnackbar( + String.format( + context.resources.getString(R.string.deleted_conversation), + conversation.displayName + ) + ) + } - WorkInfo.State.FAILED -> { - showSnackbar(context.resources.getString(R.string.nc_common_error_sorry)) - } + WorkInfo.State.FAILED -> { + conversation.token?.let { conversationsListViewModel.clearConversationPendingRemoval(it) } + showSnackbar(context.resources.getString(R.string.nc_common_error_sorry)) + } - else -> { - } + else -> { } } } 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 31547455b0..55aeb31156 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 @@ -42,7 +42,7 @@ import com.nextcloud.talk.utils.CapabilitiesUtil.hasSpreedFeatureCapability import com.nextcloud.talk.utils.SpreedFeatures import com.nextcloud.talk.utils.UserIdUtils import com.nextcloud.talk.utils.database.user.CurrentUserProviderOld -import com.nextcloud.talk.utils.withRetry +import com.nextcloud.talk.utils.optimisticAction import io.reactivex.Observer import io.reactivex.android.schedulers.AndroidSchedulers import io.reactivex.disposables.Disposable @@ -242,10 +242,10 @@ class ConversationsListViewModel @Inject constructor( private val hideRoomToken = MutableStateFlow(null) - /** Tokens of rooms being left; hidden optimistically while the leave-undo snackbar is showing. */ - private val pendingLeaveTokens = MutableStateFlow>(emptySet()) + /** Tokens of rooms being left or deleted; hidden optimistically until the request finished. */ + private val pendingRemovalTokens = MutableStateFlow>(emptySet()) - private val excludedRoomTokens = combine(hideRoomToken, pendingLeaveTokens) { hideToken, pendingTokens -> + private val excludedRoomTokens = combine(hideRoomToken, pendingRemovalTokens) { hideToken, pendingTokens -> if (hideToken != null) pendingTokens + hideToken else pendingTokens } @@ -387,14 +387,14 @@ class ConversationsListViewModel @Inject constructor( hideRoomToken.value = token } - /** Optimistically hide a room while its leave-undo snackbar is showing. */ - fun markConversationPendingLeave(token: String) { - pendingLeaveTokens.value = pendingLeaveTokens.value + token + /** Optimistically hide a room that is being left or deleted. */ + fun markConversationPendingRemoval(token: String) { + pendingRemovalTokens.value = pendingRemovalTokens.value + token } - /** Un-hide a room, either because the leave was undone or because it finished/failed. */ - fun clearConversationPendingLeave(token: String) { - pendingLeaveTokens.value = pendingLeaveTokens.value - token + /** Un-hide a room, either because the removal was undone or because it finished or failed. */ + fun clearConversationPendingRemoval(token: String) { + pendingRemovalTokens.value = pendingRemovalTokens.value - token } fun getFederationInvitations() { @@ -782,22 +782,21 @@ class ConversationsListViewModel @Inject constructor( ) val url = ApiUtils.getUrlForChatReadMarker(apiVersion, currentUser.baseUrl, conversation.token) viewModelScope.launch { - messageId?.let { conversationListUpdater.markPendingReadMarker(conversation.internalId, it) } - withContext(Dispatchers.IO) { - repository.updateConversation(optimistic) - } - try { - withContext(Dispatchers.IO) { - withRetry(1) { conversationsRepository.markConversationAsRead(credentials, url, messageId) } - } - _readUnreadState.value = ConversationReadUnreadUiState.Success - } catch (e: Exception) { - messageId?.let { conversationListUpdater.clearPendingReadMarker(conversation.internalId, it) } - withContext(Dispatchers.IO) { - repository.updateConversation(original) - } - _readUnreadState.value = ConversationReadUnreadUiState.Error - } + val result = optimisticAction( + apply = { + messageId?.let { conversationListUpdater.markPendingReadMarker(conversation.internalId, it) } + applyLocally(optimistic) + revertTo(original) { + messageId?.let { conversationListUpdater.clearPendingReadMarker(conversation.internalId, it) } + } + }, + request = { conversationsRepository.markConversationAsRead(credentials, url, messageId) } + ) + + _readUnreadState.value = result.fold( + onSuccess = { ConversationReadUnreadUiState.Success }, + onFailure = { ConversationReadUnreadUiState.Error } + ) } } @@ -811,25 +810,36 @@ class ConversationsListViewModel @Inject constructor( ) val url = ApiUtils.getUrlForChatReadMarker(apiVersion, currentUser.baseUrl, conversation.token) viewModelScope.launch { - conversationListUpdater.markPendingUnread(conversation.internalId) - withContext(Dispatchers.IO) { - repository.updateConversation(optimistic) - } - try { - withContext(Dispatchers.IO) { - withRetry(1) { conversationsRepository.markConversationAsUnread(credentials, url) } - } - _readUnreadState.value = ConversationReadUnreadUiState.Success - } catch (e: Exception) { - conversationListUpdater.clearPendingUnread(conversation.internalId) - withContext(Dispatchers.IO) { - repository.updateConversation(original) - } - _readUnreadState.value = ConversationReadUnreadUiState.Error - } + val result = optimisticAction( + apply = { + conversationListUpdater.markPendingUnread(conversation.internalId) + applyLocally(optimistic) + revertTo(original) { conversationListUpdater.clearPendingUnread(conversation.internalId) } + }, + request = { conversationsRepository.markConversationAsUnread(credentials, url) } + ) + + _readUnreadState.value = result.fold( + onSuccess = { ConversationReadUnreadUiState.Success }, + onFailure = { ConversationReadUnreadUiState.Error } + ) } } + private suspend fun applyLocally(conversation: ConversationModel) { + withContext(Dispatchers.IO) { repository.updateConversation(conversation) } + } + + /** + * The action that puts [original] back and releases whatever guard the caller registered, for + * [optimisticAction] to run when the request finally fails or the screen is left mid-request. + */ + private fun revertTo(original: ConversationModel, releaseGuard: () -> Unit): suspend () -> Unit = + { + releaseGuard() + applyLocally(original) + } + fun resetFavoriteState() { _favoriteState.value = FavoriteUiState.None } @@ -846,28 +856,27 @@ class ConversationsListViewModel @Inject constructor( val apiVersion = ApiUtils.getConversationApiVersion(currentUser, intArrayOf(ApiUtils.API_V4, ApiUtils.API_V1)) val url = ApiUtils.getUrlForArchive(apiVersion, currentUser.baseUrl, conversation.token) viewModelScope.launch { - conversationListUpdater.markPendingArchived(conversation.internalId, desiredArchived) - withContext(Dispatchers.IO) { - repository.updateConversation(optimistic) - } - try { - withContext(Dispatchers.IO) { - withRetry(1) { - if (desiredArchived) { - conversationsRepository.archiveConversation(credentials, url) - } else { - conversationsRepository.unarchiveConversation(credentials, url) - } + 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) } } - _archiveState.value = ArchiveUiState.Success(desiredArchived, conversation.displayName) - } catch (e: Exception) { - conversationListUpdater.clearPendingArchived(conversation.internalId, desiredArchived) - withContext(Dispatchers.IO) { - repository.updateConversation(original) - } - _archiveState.value = ArchiveUiState.Error - } + ) + + _archiveState.value = result.fold( + onSuccess = { ArchiveUiState.Success(desiredArchived, conversation.displayName) }, + onFailure = { ArchiveUiState.Error } + ) } } @@ -878,22 +887,21 @@ class ConversationsListViewModel @Inject constructor( val apiVersion = ApiUtils.getConversationApiVersion(currentUser, intArrayOf(ApiUtils.API_V4, ApiUtils.API_V1)) val url = ApiUtils.getUrlForRoomFavorite(apiVersion, currentUser.baseUrl, conversation.token) viewModelScope.launch { - conversationListUpdater.markPendingFavorite(conversation.internalId, favorite = true) - withContext(Dispatchers.IO) { - repository.updateConversation(optimistic) - } - try { - withContext(Dispatchers.IO) { - withRetry(1) { conversationsRepository.addConversationToFavorites(credentials, url) } - } - _favoriteState.value = FavoriteUiState.Success - } catch (e: Exception) { - conversationListUpdater.clearPendingFavorite(conversation.internalId, favorite = true) - withContext(Dispatchers.IO) { - repository.updateConversation(original) - } - _favoriteState.value = FavoriteUiState.Error - } + 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) } + ) + + _favoriteState.value = result.fold( + onSuccess = { FavoriteUiState.Success }, + onFailure = { FavoriteUiState.Error } + ) } } @@ -904,22 +912,21 @@ class ConversationsListViewModel @Inject constructor( val apiVersion = ApiUtils.getConversationApiVersion(currentUser, intArrayOf(ApiUtils.API_V4, ApiUtils.API_V1)) val url = ApiUtils.getUrlForRoomFavorite(apiVersion, currentUser.baseUrl, conversation.token) viewModelScope.launch { - conversationListUpdater.markPendingFavorite(conversation.internalId, favorite = false) - withContext(Dispatchers.IO) { - repository.updateConversation(optimistic) - } - try { - withContext(Dispatchers.IO) { - withRetry(1) { conversationsRepository.removeConversationFromFavorites(credentials, url) } - } - _favoriteState.value = FavoriteUiState.Success - } catch (e: Exception) { - conversationListUpdater.clearPendingFavorite(conversation.internalId, favorite = false) - withContext(Dispatchers.IO) { - repository.updateConversation(original) - } - _favoriteState.value = FavoriteUiState.Error - } + 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) } + ) + + _favoriteState.value = result.fold( + onSuccess = { FavoriteUiState.Success }, + onFailure = { FavoriteUiState.Error } + ) } } diff --git a/app/src/main/java/com/nextcloud/talk/repositories/reactions/ReactionsRepositoryImpl.kt b/app/src/main/java/com/nextcloud/talk/repositories/reactions/ReactionsRepositoryImpl.kt index 46c683df54..9a01933cc3 100644 --- a/app/src/main/java/com/nextcloud/talk/repositories/reactions/ReactionsRepositoryImpl.kt +++ b/app/src/main/java/com/nextcloud/talk/repositories/reactions/ReactionsRepositoryImpl.kt @@ -12,12 +12,10 @@ import com.nextcloud.talk.chat.data.model.ChatMessage import com.nextcloud.talk.data.database.dao.ChatMessagesDao import com.nextcloud.talk.models.domain.ReactionAddedModel import com.nextcloud.talk.models.domain.ReactionDeletedModel -import com.nextcloud.talk.utils.revertOnCancellation -import com.nextcloud.talk.utils.withRetry +import com.nextcloud.talk.utils.optimisticAction import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import retrofit2.HttpException -import java.io.IOException import javax.inject.Inject /** @@ -45,23 +43,14 @@ class ReactionsRepositoryImpl @Inject constructor( val internalConversationId = "$userId@$roomToken" val messageId = message.jsonMessageId.toLong() - val applied = applyLocalAdd(internalConversationId, messageId, emoji) - val revert: suspend () -> Unit = { - if (applied) applyLocalRemove(internalConversationId, messageId, emoji) - } - - val confirmed = revertOnCancellation(revert) { - requestSucceeds( - successStatusCodes = ADD_SUCCESS_CODES, - alreadyAppliedHttpCodes = emptySet() - ) { - ncApiCoroutines.sendReaction(credentials, url, emoji).ocs?.meta?.statusCode - } - } - - if (!confirmed) { - revert() - } + val confirmed = optimisticAction( + apply = { + val applied = applyLocalAdd(internalConversationId, messageId, emoji) + revertWith(applied) { applyLocalRemove(internalConversationId, messageId, emoji) } + }, + isConfirmed = { statusCode -> statusCode in ADD_SUCCESS_CODES }, + request = { ncApiCoroutines.sendReaction(credentials, url, emoji).ocs?.meta?.statusCode } + ).logFailure().getOrNull() in ADD_SUCCESS_CODES return ReactionAddedModel(message, emoji, confirmed) } @@ -77,77 +66,32 @@ class ReactionsRepositoryImpl @Inject constructor( val internalConversationId = "$userId@$roomToken" val messageId = message.jsonMessageId.toLong() - val applied = applyLocalRemove(internalConversationId, messageId, emoji) - val revert: suspend () -> Unit = { - if (applied) applyLocalAdd(internalConversationId, messageId, emoji) - } - - val confirmed = revertOnCancellation(revert) { - requestSucceeds( - successStatusCodes = DELETE_SUCCESS_CODES, - alreadyAppliedHttpCodes = DELETE_ALREADY_APPLIED_CODES - ) { - ncApiCoroutines.deleteReaction(credentials, url, emoji).ocs?.meta?.statusCode - } - } - - if (!confirmed) { - revert() - } + val confirmed = optimisticAction( + apply = { + val applied = applyLocalRemove(internalConversationId, messageId, emoji) + revertWith(applied) { applyLocalAdd(internalConversationId, messageId, emoji) } + }, + isConfirmed = { statusCode -> statusCode in DELETE_SUCCESS_CODES }, + request = { deleteReactionStatusCode(credentials, url, emoji) } + ).logFailure().getOrNull() in DELETE_SUCCESS_CODES return ReactionDeletedModel(message, emoji, confirmed) } - private suspend fun requestSucceeds( - successStatusCodes: Set, - alreadyAppliedHttpCodes: Set, - call: suspend () -> Int? - ): Boolean = - try { - withRetry(retries = 1, initialDelayMillis = RETRY_DELAY_MS, retryOn = ::isRetryable) { - attemptRequest(successStatusCodes, alreadyAppliedHttpCodes, call) - } - } catch (e: IOException) { - Log.w(TAG, "Reaction request failed, the retry failed as well: $e") - false - } catch (e: HttpException) { - Log.w(TAG, "Reaction request failed with HTTP ${e.code()}, the retry failed as well: $e") - false - } - /** - * Returns whether the server applied the reaction, and throws for a failure that is worth another - * attempt: a connection problem, a rate limit or a server error. + * A reaction the server no longer knows is deleted as far as this call is concerned. */ - private suspend fun attemptRequest( - successStatusCodes: Set, - alreadyAppliedHttpCodes: Set, - call: suspend () -> Int? - ): Boolean = + private suspend fun deleteReactionStatusCode(credentials: String?, url: String, emoji: String): Int? = try { - val statusCode = call() - if (statusCode in successStatusCodes) { - true - } else { - Log.w(TAG, "Reaction request answered with unexpected status code $statusCode") - false - } + ncApiCoroutines.deleteReaction(credentials, url, emoji).ocs?.meta?.statusCode } catch (e: HttpException) { - when { - e.code() in alreadyAppliedHttpCodes -> true - isRetryable(e) -> throw e - else -> { - Log.w(TAG, "Reaction request rejected with HTTP ${e.code()}: $e") - false - } - } + if (e.code() == HTTP_NOT_FOUND) HTTP_OK else throw e } - private fun isRetryable(error: Exception): Boolean = - when (error) { - is HttpException -> error.code() == HTTP_TOO_MANY_REQUESTS || error.code() >= HTTP_INTERNAL_SERVER_ERROR - else -> error is IOException - } + private fun revertWith(applied: Boolean, revert: suspend () -> Unit): (suspend () -> Unit)? = + revert.takeIf { applied } + + private fun Result.logFailure(): Result = onFailure { Log.w(TAG, "Reaction request failed: $it") } /** * Adds the reaction to the cached message and reports whether that changed anything. A reaction the @@ -191,12 +135,8 @@ class ReactionsRepositoryImpl @Inject constructor( private const val HTTP_OK: Int = 200 private const val HTTP_CREATED: Int = 201 private const val HTTP_NOT_FOUND: Int = 404 - private const val HTTP_TOO_MANY_REQUESTS: Int = 429 - private const val HTTP_INTERNAL_SERVER_ERROR: Int = 500 - private const val RETRY_DELAY_MS: Long = 500 private val ADD_SUCCESS_CODES = setOf(HTTP_OK, HTTP_CREATED) private val DELETE_SUCCESS_CODES = setOf(HTTP_OK) - private val DELETE_ALREADY_APPLIED_CODES = setOf(HTTP_NOT_FOUND) } } diff --git a/app/src/main/java/com/nextcloud/talk/utils/OptimisticAction.kt b/app/src/main/java/com/nextcloud/talk/utils/OptimisticAction.kt new file mode 100644 index 0000000000..6637c22313 --- /dev/null +++ b/app/src/main/java/com/nextcloud/talk/utils/OptimisticAction.kt @@ -0,0 +1,71 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +package com.nextcloud.talk.utils + +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.withContext +import retrofit2.HttpException +import java.io.IOException + +private const val HTTP_TOO_MANY_REQUESTS = 429 +private const val HTTP_INTERNAL_SERVER_ERROR = 500 +private const val RETRY_DELAY_MS = 500L + +/** + * Whether a failed request is worth another attempt: a connection problem, a rate limit or a server + * error. An answer the server actually gave - a refusal, a permission error - is not. + */ +fun isTransientFailure(error: Exception): Boolean = + when (error) { + is HttpException -> error.code() == HTTP_TOO_MANY_REQUESTS || error.code() >= HTTP_INTERNAL_SERVER_ERROR + else -> error is IOException + } + +/** + * Applies a change locally, has the server confirm it and undoes it when that fails, which is how + * every user action that the client can predict should behave: the screen reacts to the tap, the + * request follows, and only a request that finally fails takes the change back. + * + * [apply] writes the local state and returns the action that undoes it again, or null when there was + * nothing to change - in which case nothing is undone later either. [request] is retried once for a + * transient failure. [isConfirmed] decides whether an answer that did not throw still counts as a + * refusal, for endpoints that report one in the payload rather than in the status code. A request + * that fails for any other reason is not retried, but the change is still taken back. + * + * Cancellation is part of the contract: closing the screen while the request is in flight undoes the + * change as well, instead of leaving it applied although the server may never have heard of it. + */ +@Suppress("TooGenericExceptionCaught") +suspend fun optimisticAction( + apply: suspend () -> (suspend () -> Unit)?, + isConfirmed: (T) -> Boolean = { true }, + request: suspend () -> T +): Result { + val revert = apply() + // a revert that is itself cancelled would leave the change applied, which is the very thing it + // is there to prevent + val revertUninterruptibly: suspend () -> Unit = { withContext(NonCancellable) { revert?.invoke() } } + + return try { + val answer = revertOnCancellation(revertUninterruptibly) { + withRetry(retries = 1, initialDelayMillis = RETRY_DELAY_MS, retryOn = ::isTransientFailure) { + request() + } + } + if (!isConfirmed(answer)) { + revertUninterruptibly() + } + Result.success(answer) + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + revertUninterruptibly() + Result.failure(e) + } +} diff --git a/app/src/main/java/com/nextcloud/talk/utils/preferences/preferencestorage/DatabaseStorageModule.kt b/app/src/main/java/com/nextcloud/talk/utils/preferences/preferencestorage/DatabaseStorageModule.kt index 449858e123..cd515aed4e 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/preferences/preferencestorage/DatabaseStorageModule.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/preferences/preferencestorage/DatabaseStorageModule.kt @@ -8,8 +8,6 @@ */ package com.nextcloud.talk.utils.preferences.preferencestorage -import android.text.TextUtils -import android.util.Log import autodagger.AutoInjector import com.nextcloud.talk.api.NcApi import com.nextcloud.talk.api.NcApiCoroutines @@ -63,7 +61,6 @@ class DatabaseStorageModule(conversationUser: User, conversationToken: String) { this.conversationToken = conversationToken } - @Suppress("Detekt.TooGenericExceptionCaught") suspend fun saveBoolean(key: String, value: Boolean) { if ("call_notifications_switch" == key) { val apiVersion = getConversationApiVersion(conversationUser, intArrayOf(ApiUtils.API_V4)) @@ -71,12 +68,7 @@ class DatabaseStorageModule(conversationUser: User, conversationToken: String) { val credentials = getCredentials(conversationUser.username, conversationUser.token) val notificationLevel = if (value) 1 else 0 withContext(Dispatchers.IO) { - try { - ncApiCoroutines!!.notificationCalls(credentials!!, url, notificationLevel) - Log.d(TAG, "Toggled notification calls") - } catch (e: Exception) { - Log.e(TAG, "Error when trying to toggle notification calls", e) - } + ncApiCoroutines!!.notificationCalls(credentials!!, url, notificationLevel) } } if ("lobby_switch" != key) { @@ -91,64 +83,51 @@ class DatabaseStorageModule(conversationUser: User, conversationToken: String) { } } - @Suppress("Detekt.TooGenericExceptionCaught") suspend fun saveString(key: String, value: String) { when (key) { - "conversation_settings_dropdown" -> { - try { - val apiVersion = getConversationApiVersion(conversationUser, intArrayOf(API_VERSION_4)) - val trimmedValue = value.replace("expire_", "") - val valueInt = trimmedValue.toInt() - withContext(Dispatchers.IO) { - ncApiCoroutines!!.setMessageExpiration( - getCredentials(conversationUser.username, conversationUser.token)!!, - getUrlForMessageExpiration(apiVersion, conversationUser.baseUrl, conversationToken), - valueInt - ) - messageExpiration = valueInt - } - } catch (exception: Exception) { - Log.e(TAG, "Error when trying to set message expiration", exception) - } - } - "conversation_info_message_notifications_dropdown" -> { - try { - if (hasSpreedFeatureCapability( - conversationUser.capabilities!!.spreedCapability!!, - SpreedFeatures.NOTIFICATION_LEVELS - ) - ) { - if (TextUtils.isEmpty(messageNotificationLevel) || messageNotificationLevel != value) { - val intValue = when (value) { - "never" -> NOTIFICATION_NEVER - "mention" -> NOTIFICATION_MENTION - "always" -> NOTIFICATION_ALWAYS - else -> 0 - } - val apiVersion = getConversationApiVersion(conversationUser, intArrayOf(ApiUtils.API_V4, 1)) - withContext(Dispatchers.IO) { - ncApiCoroutines!!.setNotificationLevel( - getCredentials(conversationUser.username, conversationUser.token)!!, - getUrlForRoomNotificationLevel( - apiVersion, - conversationUser.baseUrl, - conversationToken - ), - intValue - ) - messageNotificationLevel = value - } - } else { - messageNotificationLevel = value - } - } - } catch (exception: Exception) { - Log.e(TAG, "Error trying to set notification level", exception) - } - } - else -> { - arbitraryStorageManager!!.storeStorageSetting(accountIdentifier, key, value, conversationToken) - } + "conversation_settings_dropdown" -> saveMessageExpiration(value) + "conversation_info_message_notifications_dropdown" -> saveNotificationLevel(value) + else -> arbitraryStorageManager!!.storeStorageSetting(accountIdentifier, key, value, conversationToken) + } + } + + private suspend fun saveMessageExpiration(value: String) { + val apiVersion = getConversationApiVersion(conversationUser, intArrayOf(API_VERSION_4)) + val valueInt = value.replace("expire_", "").toInt() + withContext(Dispatchers.IO) { + ncApiCoroutines!!.setMessageExpiration( + getCredentials(conversationUser.username, conversationUser.token)!!, + getUrlForMessageExpiration(apiVersion, conversationUser.baseUrl, conversationToken), + valueInt + ) + messageExpiration = valueInt + } + } + + private suspend fun saveNotificationLevel(value: String) { + val spreedCapability = conversationUser.capabilities?.spreedCapability ?: return + if (!hasSpreedFeatureCapability(spreedCapability, SpreedFeatures.NOTIFICATION_LEVELS)) { + messageNotificationLevel = value + return + } + if (messageNotificationLevel == value) { + return + } + + val intValue = when (value) { + "never" -> NOTIFICATION_NEVER + "mention" -> NOTIFICATION_MENTION + "always" -> NOTIFICATION_ALWAYS + else -> 0 + } + val apiVersion = getConversationApiVersion(conversationUser, intArrayOf(ApiUtils.API_V4, 1)) + withContext(Dispatchers.IO) { + ncApiCoroutines!!.setNotificationLevel( + getCredentials(conversationUser.username, conversationUser.token)!!, + getUrlForRoomNotificationLevel(apiVersion, conversationUser.baseUrl, conversationToken), + intValue + ) + messageNotificationLevel = value } } @@ -186,7 +165,6 @@ class DatabaseStorageModule(conversationUser: User, conversationToken: String) { } companion object { - private const val TAG = "DatabaseStorageModule" private const val EXPIRE_1_HOUR = 3600 private const val EXPIRE_8_HOURS = 28800 private const val EXPIRE_1_DAY = 86400 diff --git a/app/src/test/java/com/nextcloud/talk/conversationinfo/viewmodel/ConversationInfoViewModelTest.kt b/app/src/test/java/com/nextcloud/talk/conversationinfo/viewmodel/ConversationInfoViewModelTest.kt index 991a9be850..459db74ffd 100644 --- a/app/src/test/java/com/nextcloud/talk/conversationinfo/viewmodel/ConversationInfoViewModelTest.kt +++ b/app/src/test/java/com/nextcloud/talk/conversationinfo/viewmodel/ConversationInfoViewModelTest.kt @@ -108,6 +108,34 @@ class ConversationInfoViewModelTest { assertFalse(model.uiState.value.guestsAllowed) } + @Test + fun `an important toggle that the server never accepts goes back to what it was`() = + runTest(dispatcher) { + val repository = FakeConversationsRepository().apply { failingImportantRequests = Int.MAX_VALUE } + val model = viewModel(repository) + + model.toggleImportantConversation("credentials", user.baseUrl!!, "token") + dispatcher.scheduler.runCurrent() + assertEquals(true, model.uiState.value.importantConversation) + + dispatcher.scheduler.advanceUntilIdle() + + assertFalse(model.uiState.value.importantConversation) + } + + @Test + fun `an important toggle survives a single connection problem`() = + runTest(dispatcher) { + val repository = FakeConversationsRepository().apply { failingImportantRequests = 1 } + val model = viewModel(repository) + + model.toggleImportantConversation("credentials", user.baseUrl!!, "token") + dispatcher.scheduler.advanceUntilIdle() + + assertEquals(2, repository.importantRequests) + assertEquals(true, model.uiState.value.importantConversation) + } + @Test fun `createConversationNameByParticipants should combine names correctly`() { val original = listOf("Dave", null, "Charlie") diff --git a/app/src/test/java/com/nextcloud/talk/conversationinfo/viewmodel/FakeConversationsRepository.kt b/app/src/test/java/com/nextcloud/talk/conversationinfo/viewmodel/FakeConversationsRepository.kt index 189e7f6be1..4d41bcc755 100644 --- a/app/src/test/java/com/nextcloud/talk/conversationinfo/viewmodel/FakeConversationsRepository.kt +++ b/app/src/test/java/com/nextcloud/talk/conversationinfo/viewmodel/FakeConversationsRepository.kt @@ -14,6 +14,7 @@ import com.nextcloud.talk.models.json.participants.TalkBan import com.nextcloud.talk.models.json.profile.Profile import com.nextcloud.talk.repositories.conversations.ConversationsRepository import io.reactivex.Observable +import java.io.IOException /** * Answers with what a test needs of [ConversationsRepository.allowGuests], and fails everything @@ -24,6 +25,10 @@ class FakeConversationsRepository : ConversationsRepository { var failAllowGuests = false var lastAllowGuestsPassword: String? = null + /** How many of the next important-conversation requests fail with a connection problem. */ + var failingImportantRequests = 0 + var importantRequests = 0 + override suspend fun allowGuests( user: User, url: String, @@ -92,7 +97,14 @@ class FakeConversationsRepository : ConversationsRepository { credentials: String, baseUrl: String, roomToken: String - ): GenericOverall = throw UnsupportedOperationException() + ): GenericOverall { + importantRequests++ + if (failingImportantRequests > 0) { + failingImportantRequests-- + throw IOException("no connection") + } + return GenericOverall() + } override suspend fun markConversationAsUnImportant( credentials: String, diff --git a/app/src/test/java/com/nextcloud/talk/utils/OptimisticActionTest.kt b/app/src/test/java/com/nextcloud/talk/utils/OptimisticActionTest.kt new file mode 100644 index 0000000000..6e48a2e0e3 --- /dev/null +++ b/app/src/test/java/com/nextcloud/talk/utils/OptimisticActionTest.kt @@ -0,0 +1,139 @@ +/* + * Nextcloud Talk - Android Client + * + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: GPL-3.0-or-later + */ + +package com.nextcloud.talk.utils + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.launch +import kotlinx.coroutines.suspendCancellableCoroutine +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import okhttp3.MediaType.Companion.toMediaType +import okhttp3.ResponseBody.Companion.toResponseBody +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import retrofit2.HttpException +import retrofit2.Response +import java.io.IOException + +/** + * The contract every optimistic action in the app relies on: the change is visible before the answer + * arrives, it survives a hiccup, and it is taken back whenever the request does not finally succeed - + * including when the screen is left while the request is still in flight. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class OptimisticActionTest { + + private var state = "before" + + private val applyChange: suspend () -> (suspend () -> Unit)? = { + val previous = state + state = "after" + { state = previous } + } + + @Test + fun `a change the server accepts stays applied`() = + runTest { + val result = optimisticAction(apply = applyChange, request = { "ok" }) + + assertEquals("ok", result.getOrNull()) + assertEquals("after", state) + } + + @Test + fun `a change the server refuses is taken back`() = + runTest { + val result = optimisticAction(apply = applyChange, request = { throw forbidden() }) + + assertTrue(result.isFailure) + assertEquals("before", state) + } + + @Test + fun `a connection problem is retried once before the change is taken back`() = + runTest { + var attempts = 0 + + val result = optimisticAction( + apply = applyChange, + request = { + attempts++ + throw IOException("no connection") + } + ) + + assertEquals(2, attempts) + assertTrue(result.isFailure) + assertEquals("before", state) + } + + @Test + fun `a single connection problem does not cost the change`() = + runTest { + var attempts = 0 + + optimisticAction( + apply = applyChange, + request = { + attempts++ + if (attempts == 1) throw IOException("no connection") else "ok" + } + ) + + assertEquals("after", state) + } + + @Test + fun `a refusal the server reports in the payload is taken back as well`() = + runTest { + val result = optimisticAction( + apply = applyChange, + isConfirmed = { answer: String -> answer == "ok" }, + request = { "refused" } + ) + + // the request itself did not fail, so the caller still sees the answer it got + assertEquals("refused", result.getOrNull()) + assertEquals("before", state) + } + + @Test + fun `nothing is taken back when nothing was applied`() = + runTest { + val result = optimisticAction(apply = { null }, request = { throw forbidden() }) + + assertTrue(result.isFailure) + assertEquals("before", state) + } + + @Test + fun `leaving the screen while the request is in flight takes the change back`() = + runTest { + val job = launch { + optimisticAction(apply = applyChange, request = { awaitForever() }) + } + runCurrent() + assertEquals("after", state) + + job.cancel() + advanceUntilIdle() + + assertEquals("before", state) + } + + private suspend fun awaitForever(): String = suspendCancellableCoroutine { } + + private fun forbidden(): HttpException = + HttpException(Response.error(HTTP_FORBIDDEN, "".toResponseBody("text/plain".toMediaType()))) + + private companion object { + const val HTTP_FORBIDDEN = 403 + } +}