From acd05c43dbc59b93088ed7f03dcc32e93aac595f Mon Sep 17 00:00:00 2001 From: Stanislav Dmitrenko <7953703+avently@users.noreply.github.com> Date: Wed, 10 Jan 2024 23:57:34 +0700 Subject: [PATCH] mobile: chat deletion avoiding race conditions (#3650) * android, desktop: chat items deletion * rename * ios: chat items deletion * correct id * android: adding progress of deletion * ios: text color while deleting chats * change only text color --------- Co-authored-by: Avently Co-authored-by: Evgeny Poberezkin <2769109+epoberezkin@users.noreply.github.com> --- apps/ios/Shared/Model/ChatModel.swift | 1 + apps/ios/Shared/Model/SimpleXAPI.swift | 3 ++ .../Shared/Views/ChatList/ChatListView.swift | 2 +- .../Views/ChatList/ChatPreviewView.swift | 12 +++++--- .../chatlist/ChatListNavLinkView.android.kt | 4 +-- .../chat/simplex/common/model/ChatModel.kt | 2 ++ .../chat/simplex/common/model/SimpleXAPI.kt | 13 +++++---- .../views/chatlist/ChatListNavLinkView.kt | 28 +++++++++---------- .../common/views/chatlist/ChatPreviewView.kt | 24 ++++++++++++---- .../chatlist/ChatListNavLinkView.desktop.kt | 7 ++--- 10 files changed, 60 insertions(+), 36 deletions(-) diff --git a/apps/ios/Shared/Model/ChatModel.swift b/apps/ios/Shared/Model/ChatModel.swift index e5022751cd..21ef80cf6e 100644 --- a/apps/ios/Shared/Model/ChatModel.swift +++ b/apps/ios/Shared/Model/ChatModel.swift @@ -60,6 +60,7 @@ final class ChatModel: ObservableObject { @Published var laRequest: LocalAuthRequest? // list of chat "previews" @Published var chats: [Chat] = [] + @Published var deletedChats: Set = [] // map of connections network statuses, key is agent connection id @Published var networkStatuses: Dictionary = [:] // current chat diff --git a/apps/ios/Shared/Model/SimpleXAPI.swift b/apps/ios/Shared/Model/SimpleXAPI.swift index ee31dd65c1..f2aa6cbd91 100644 --- a/apps/ios/Shared/Model/SimpleXAPI.swift +++ b/apps/ios/Shared/Model/SimpleXAPI.swift @@ -691,6 +691,9 @@ func apiConnectContactViaAddress(incognito: Bool, contactId: Int64) async -> (Co } func apiDeleteChat(type: ChatType, id: Int64, notify: Bool? = nil) async throws { + let chatId = type.rawValue + id.description + DispatchQueue.main.async { ChatModel.shared.deletedChats.insert(chatId) } + defer { DispatchQueue.main.async { ChatModel.shared.deletedChats.remove(chatId) } } let r = await chatSendCmd(.apiDeleteChat(type: type, id: id, notify: notify), bgTask: false) if case .direct = type, case .contactDeleted = r { return } if case .contactConnection = type, case .contactConnectionDeleted = r { return } diff --git a/apps/ios/Shared/Views/ChatList/ChatListView.swift b/apps/ios/Shared/Views/ChatList/ChatListView.swift index 62955a1040..1b4531b08c 100644 --- a/apps/ios/Shared/Views/ChatList/ChatListView.swift +++ b/apps/ios/Shared/Views/ChatList/ChatListView.swift @@ -160,7 +160,7 @@ struct ChatListView: View { ForEach(cs, id: \.viewId) { chat in ChatListNavLink(chat: chat) .padding(.trailing, -16) - .disabled(chatModel.chatRunning != true) + .disabled(chatModel.chatRunning != true || chatModel.deletedChats.contains(chat.chatInfo.id)) } .offset(x: -8) } diff --git a/apps/ios/Shared/Views/ChatList/ChatPreviewView.swift b/apps/ios/Shared/Views/ChatList/ChatPreviewView.swift index 13d91881e6..0a53e0511a 100644 --- a/apps/ios/Shared/Views/ChatList/ChatPreviewView.swift +++ b/apps/ios/Shared/Views/ChatList/ChatPreviewView.swift @@ -13,6 +13,7 @@ struct ChatPreviewView: View { @EnvironmentObject var chatModel: ChatModel @ObservedObject var chat: Chat @Binding var progressByTimeout: Bool + @State var deleting: Bool = false @Environment(\.colorScheme) var colorScheme var darkGreen = Color(red: 0, green: 0.5, blue: 0) @@ -55,6 +56,9 @@ struct ChatPreviewView: View { .frame(maxHeight: .infinity) } .padding(.bottom, -8) + .onChange(of: chatModel.deletedChats.contains(chat.chatInfo.id)) { contains in + deleting = contains + } } @ViewBuilder private func chatPreviewImageOverlayIcon() -> some View { @@ -87,13 +91,13 @@ struct ChatPreviewView: View { let t = Text(chat.chatInfo.chatViewName).font(.title3).fontWeight(.bold) switch chat.chatInfo { case let .direct(contact): - previewTitle(contact.verified == true ? verifiedIcon + t : t) + previewTitle(contact.verified == true ? verifiedIcon + t : t).foregroundColor(deleting ? Color.secondary : nil) case let .group(groupInfo): - let v = previewTitle(t) + let v = previewTitle(t).foregroundColor(deleting ? Color.secondary : nil) switch (groupInfo.membership.memberStatus) { - case .memInvited: v.foregroundColor(chat.chatInfo.incognito ? .indigo : .accentColor) + case .memInvited: v.foregroundColor(deleting ? .secondary : chat.chatInfo.incognito ? .indigo : .accentColor) case .memAccepted: v.foregroundColor(.secondary) - default: v + default: v.foregroundColor(deleting ? Color.secondary : nil) } default: previewTitle(t) } diff --git a/apps/multiplatform/common/src/androidMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.android.kt b/apps/multiplatform/common/src/androidMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.android.kt index f8914c6653..f0f733111a 100644 --- a/apps/multiplatform/common/src/androidMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.android.kt +++ b/apps/multiplatform/common/src/androidMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.android.kt @@ -16,12 +16,12 @@ actual fun ChatListNavLinkLayout( click: () -> Unit, dropdownMenuItems: (@Composable () -> Unit)?, showMenu: MutableState, - stopped: Boolean, + disabled: Boolean, selectedChat: State, nextChatSelected: State, ) { var modifier = Modifier.fillMaxWidth() - if (!stopped) modifier = modifier + if (!disabled) modifier = modifier .combinedClickable(onClick = click, onLongClick = { showMenu.value = true }) .onRightClick { showMenu.value = true } Box(modifier) { diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt index 98c7336c2e..00d19b06f2 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/ChatModel.kt @@ -56,6 +56,8 @@ object ChatModel { // current chat val chatId = mutableStateOf(null) val chatItems = mutableStateListOf() + // rhId, chatId + val deletedChats = mutableStateOf>>(emptyList()) val chatItemStatuses = mutableMapOf() val groupMembers = mutableStateListOf() diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt index 110f12273f..363a3bcdbb 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt @@ -984,11 +984,12 @@ object ChatController { } suspend fun apiDeleteChat(rh: Long?, type: ChatType, id: Long, notify: Boolean? = null): Boolean { + chatModel.deletedChats.value += rh to type.type + id val r = sendCmd(rh, CC.ApiDeleteChat(type, id, notify)) - when { - r is CR.ContactDeleted && type == ChatType.Direct -> return true - r is CR.ContactConnectionDeleted && type == ChatType.ContactConnection -> return true - r is CR.GroupDeletedUser && type == ChatType.Group -> return true + val success = when { + r is CR.ContactDeleted && type == ChatType.Direct -> true + r is CR.ContactConnectionDeleted && type == ChatType.ContactConnection -> true + r is CR.GroupDeletedUser && type == ChatType.Group -> true else -> { val titleId = when (type) { ChatType.Direct -> MR.strings.error_deleting_contact @@ -997,9 +998,11 @@ object ChatController { ChatType.ContactConnection -> MR.strings.error_deleting_pending_contact_connection } apiErrorAlert("apiDeleteChat", generalGetString(titleId), r) + false } } - return false + chatModel.deletedChats.value -= rh to type.type + id + return success } suspend fun apiClearChat(rh: Long?, type: ChatType, id: Long): ChatInfo? { diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt index 84ad14ed77..8cff56342d 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.kt @@ -37,7 +37,7 @@ fun ChatListNavLinkView(chat: Chat, nextChatSelected: State) { val showMarkRead = remember(chat.chatStats.unreadCount, chat.chatStats.unreadChat) { chat.chatStats.unreadCount > 0 || chat.chatStats.unreadChat } - val stopped = chatModel.chatRunning.value == false + val disabled = chatModel.chatRunning.value == false || chatModel.deletedChats.value.contains(chat.remoteHostId to chat.chatInfo.id) val linkMode by remember { chatModel.controller.appPrefs.simplexLinkMode.state } LaunchedEffect(chat.id) { showMenu.value = false @@ -62,7 +62,7 @@ fun ChatListNavLinkView(chat: Chat, nextChatSelected: State) { ChatListNavLinkLayout( chatLinkPreview = { tryOrShowError("${chat.id}ChatListNavLink", error = { ErrorChatListItem() }) { - ChatPreviewView(chat, showChatPreviews, chatModel.draft.value, chatModel.draftChatId.value, chatModel.currentUser.value?.profile?.displayName, contactNetworkStatus, stopped, linkMode, inProgress = false, progressByTimeout = false) + ChatPreviewView(chat, showChatPreviews, chatModel.draft.value, chatModel.draftChatId.value, chatModel.currentUser.value?.profile?.displayName, contactNetworkStatus, disabled, linkMode, inProgress = false, progressByTimeout = false) } }, click = { directChatAction(chat.remoteHostId, chat.chatInfo.contact, chatModel) }, @@ -72,7 +72,7 @@ fun ChatListNavLinkView(chat: Chat, nextChatSelected: State) { } }, showMenu, - stopped, + disabled, selectedChat, nextChatSelected, ) @@ -81,7 +81,7 @@ fun ChatListNavLinkView(chat: Chat, nextChatSelected: State) { ChatListNavLinkLayout( chatLinkPreview = { tryOrShowError("${chat.id}ChatListNavLink", error = { ErrorChatListItem() }) { - ChatPreviewView(chat, showChatPreviews, chatModel.draft.value, chatModel.draftChatId.value, chatModel.currentUser.value?.profile?.displayName, null, stopped, linkMode, inProgress.value, progressByTimeout) + ChatPreviewView(chat, showChatPreviews, chatModel.draft.value, chatModel.draftChatId.value, chatModel.currentUser.value?.profile?.displayName, null, disabled, linkMode, inProgress.value, progressByTimeout) } }, click = { if (!inProgress.value) groupChatAction(chat.remoteHostId, chat.chatInfo.groupInfo, chatModel, inProgress) }, @@ -91,7 +91,7 @@ fun ChatListNavLinkView(chat: Chat, nextChatSelected: State) { } }, showMenu, - stopped, + disabled, selectedChat, nextChatSelected, ) @@ -109,7 +109,7 @@ fun ChatListNavLinkView(chat: Chat, nextChatSelected: State) { } }, showMenu, - stopped, + disabled, selectedChat, nextChatSelected, ) @@ -129,7 +129,7 @@ fun ChatListNavLinkView(chat: Chat, nextChatSelected: State) { } }, showMenu, - stopped, + disabled, selectedChat, nextChatSelected, ) @@ -145,7 +145,7 @@ fun ChatListNavLinkView(chat: Chat, nextChatSelected: State) { }, dropdownMenuItems = null, showMenu, - stopped, + disabled, selectedChat, nextChatSelected, ) @@ -798,7 +798,7 @@ expect fun ChatListNavLinkLayout( click: () -> Unit, dropdownMenuItems: (@Composable () -> Unit)?, showMenu: MutableState, - stopped: Boolean, + disabled: Boolean, selectedChat: State, nextChatSelected: State, ) @@ -832,7 +832,7 @@ fun PreviewChatListNavLinkDirect() { null, null, null, - stopped = false, + disabled = false, linkMode = SimplexLinkMode.DESCRIPTION, inProgress = false, progressByTimeout = false @@ -841,7 +841,7 @@ fun PreviewChatListNavLinkDirect() { click = {}, dropdownMenuItems = null, showMenu = remember { mutableStateOf(false) }, - stopped = false, + disabled = false, selectedChat = remember { mutableStateOf(false) }, nextChatSelected = remember { mutableStateOf(false) } ) @@ -877,7 +877,7 @@ fun PreviewChatListNavLinkGroup() { null, null, null, - stopped = false, + disabled = false, linkMode = SimplexLinkMode.DESCRIPTION, inProgress = false, progressByTimeout = false @@ -886,7 +886,7 @@ fun PreviewChatListNavLinkGroup() { click = {}, dropdownMenuItems = null, showMenu = remember { mutableStateOf(false) }, - stopped = false, + disabled = false, selectedChat = remember { mutableStateOf(false) }, nextChatSelected = remember { mutableStateOf(false) } ) @@ -908,7 +908,7 @@ fun PreviewChatListNavLinkContactRequest() { click = {}, dropdownMenuItems = null, showMenu = remember { mutableStateOf(false) }, - stopped = false, + disabled = false, selectedChat = remember { mutableStateOf(false) }, nextChatSelected = remember { mutableStateOf(false) } ) diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatPreviewView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatPreviewView.kt index d59dac37bf..08e95f391a 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatPreviewView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chatlist/ChatPreviewView.kt @@ -25,6 +25,7 @@ import chat.simplex.common.views.chat.item.MarkdownText import chat.simplex.common.views.helpers.* import chat.simplex.common.model.* import chat.simplex.common.model.GroupInfo +import chat.simplex.common.platform.chatModel import chat.simplex.res.MR import dev.icerock.moko.resources.ImageResource @@ -36,7 +37,7 @@ fun ChatPreviewView( chatModelDraftChatId: ChatId?, currentUserProfileDisplayName: String?, contactNetworkStatus: NetworkStatus?, - stopped: Boolean, + disabled: Boolean, linkMode: SimplexLinkMode, inProgress: Boolean, progressByTimeout: Boolean @@ -127,24 +128,35 @@ fun ChatPreviewView( @Composable fun chatPreviewTitle() { + val deleting by remember(disabled, chat.id) { mutableStateOf(chatModel.deletedChats.value.contains(chat.remoteHostId to chat.chatInfo.id)) } when (cInfo) { is ChatInfo.Direct -> Row(verticalAlignment = Alignment.CenterVertically) { if (cInfo.contact.verified) { VerifiedIcon() } - chatPreviewTitleText() + chatPreviewTitleText( + if (deleting) + MaterialTheme.colors.secondary + else + Color.Unspecified + ) } is ChatInfo.Group -> when (cInfo.groupInfo.membership.memberStatus) { GroupMemberStatus.MemInvited -> chatPreviewTitleText( - if (inProgress) + if (inProgress || deleting) MaterialTheme.colors.secondary else if (chat.chatInfo.incognito) Indigo else MaterialTheme.colors.primary ) GroupMemberStatus.MemAccepted -> chatPreviewTitleText(MaterialTheme.colors.secondary) - else -> chatPreviewTitleText() + else -> chatPreviewTitleText( + if (deleting) + MaterialTheme.colors.secondary + else + Color.Unspecified + ) } else -> chatPreviewTitleText() } @@ -293,7 +305,7 @@ fun ChatPreviewView( color = Color.White, fontSize = 11.sp, modifier = Modifier - .background(if (stopped || showNtfsIcon) MaterialTheme.colors.secondary else MaterialTheme.colors.primaryVariant, shape = CircleShape) + .background(if (disabled || showNtfsIcon) MaterialTheme.colors.secondary else MaterialTheme.colors.primaryVariant, shape = CircleShape) .badgeLayout() .padding(horizontal = 3.dp) .padding(vertical = 1.dp) @@ -374,6 +386,6 @@ fun unreadCountStr(n: Int): String { @Composable fun PreviewChatPreviewView() { SimpleXTheme { - ChatPreviewView(Chat.sampleData, true, null, null, "", contactNetworkStatus = NetworkStatus.Connected(), stopped = false, linkMode = SimplexLinkMode.DESCRIPTION, inProgress = false, progressByTimeout = false) + ChatPreviewView(Chat.sampleData, true, null, null, "", contactNetworkStatus = NetworkStatus.Connected(), disabled = false, linkMode = SimplexLinkMode.DESCRIPTION, inProgress = false, progressByTimeout = false) } } diff --git a/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.desktop.kt b/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.desktop.kt index b2fc451969..189f1842dd 100644 --- a/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.desktop.kt +++ b/apps/multiplatform/common/src/desktopMain/kotlin/chat/simplex/common/views/chatlist/ChatListNavLinkView.desktop.kt @@ -9,7 +9,6 @@ import androidx.compose.material.MaterialTheme import androidx.compose.runtime.* import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.drawscope.ContentDrawScope import androidx.compose.ui.unit.dp import chat.simplex.common.platform.onRightClick @@ -33,16 +32,16 @@ actual fun ChatListNavLinkLayout( click: () -> Unit, dropdownMenuItems: (@Composable () -> Unit)?, showMenu: MutableState, - stopped: Boolean, + disabled: Boolean, selectedChat: State, nextChatSelected: State, ) { var modifier = Modifier.fillMaxWidth() - if (!stopped) modifier = modifier + if (!disabled) modifier = modifier .combinedClickable(onClick = click, onLongClick = { showMenu.value = true }) .onRightClick { showMenu.value = true } CompositionLocalProvider( - LocalIndication provides if (selectedChat.value && !stopped) NoIndication else LocalIndication.current + LocalIndication provides if (selectedChat.value && !disabled) NoIndication else LocalIndication.current ) { Box(modifier) { Row(