android, desktop: improving group members loading to prevent crashes (#5462)

This commit is contained in:
Stanislav Dmitrenko
2025-01-02 04:31:32 +07:00
committed by GitHub
parent 0dfcd60490
commit cab938b9f0
6 changed files with 50 additions and 29 deletions

View File

@@ -81,8 +81,8 @@ object ChatModel {
// rhId, chatId
val deletedChats = mutableStateOf<List<Pair<Long?, String>>>(emptyList())
val chatItemStatuses = mutableMapOf<Long, CIStatus>()
val groupMembers = mutableStateListOf<GroupMember>()
val groupMembersIndexes = mutableStateMapOf<Long, Int>()
val groupMembers = mutableStateOf<List<GroupMember>>(emptyList())
val groupMembersIndexes = mutableStateOf<Map<Long, Int>>(emptyMap())
// Chat Tags
val userTags = mutableStateOf(emptyList<ChatTag>())
@@ -322,16 +322,18 @@ object ChatModel {
fun getGroupChat(groupId: Long): Chat? = chats.value.firstOrNull { it.chatInfo is ChatInfo.Group && it.chatInfo.apiId == groupId }
fun populateGroupMembersIndexes() {
groupMembersIndexes.clear()
groupMembers.forEachIndexed { i, member ->
groupMembersIndexes[member.groupMemberId] = i
groupMembersIndexes.value = emptyMap()
val gmIndexes = groupMembersIndexes.value.toMutableMap()
groupMembers.value.forEachIndexed { i, member ->
gmIndexes[member.groupMemberId] = i
}
groupMembersIndexes.value = gmIndexes
}
fun getGroupMember(groupMemberId: Long): GroupMember? {
val memberIndex = groupMembersIndexes[groupMemberId]
val memberIndex = groupMembersIndexes.value[groupMemberId]
return if (memberIndex != null) {
groupMembers[memberIndex]
groupMembers.value[memberIndex]
} else {
null
}
@@ -697,7 +699,7 @@ object ChatModel {
}
// update current chat
return if (chatId.value == groupInfo.id) {
val memberIndex = groupMembersIndexes[member.groupMemberId]
val memberIndex = groupMembersIndexes.value[member.groupMemberId]
val updated = chatItems.value.map {
// Take into account only specific changes, not all. Other member updates are not important and can be skipped
if (it.chatDir is CIDirection.GroupRcv && it.chatDir.groupMember.groupMemberId == member.groupMemberId &&
@@ -713,12 +715,17 @@ object ChatModel {
if (updated != chatItems.value) {
chatItems.replaceAll(updated)
}
val gMembers = groupMembers.value.toMutableList()
if (memberIndex != null) {
groupMembers[memberIndex] = member
gMembers[memberIndex] = member
groupMembers.value = gMembers
false
} else {
groupMembers.add(member)
groupMembersIndexes[member.groupMemberId] = groupMembers.size - 1
gMembers.add(member)
groupMembers.value = gMembers
val gmIndexes = groupMembersIndexes.value.toMutableMap()
gmIndexes[member.groupMemberId] = groupMembers.size - 1
groupMembersIndexes.value = gmIndexes
true
}
} else {

View File

@@ -683,6 +683,8 @@ object ChatController {
Log.d(TAG, "sendCmd: ${cmd.cmdType}")
}
val json = if (rhId == null) chatSendCmd(ctrl, c) else chatSendRemoteCmd(ctrl, rhId.toInt(), c)
// coroutine was cancelled already, no need to process response (helps with apiListMembers - very heavy query in large groups)
interruptIfCancelled()
val r = APIResponse.decodeStr(json)
if (log) {
Log.d(TAG, "sendCmd response type ${r.resp.responseType}")

View File

@@ -114,6 +114,7 @@ fun ChatView(staleChatId: State<String?>, onComposed: suspend (chatId: String) -
CompositionLocalProvider(LocalAppBarHandler provides rememberAppBarHandler(chatInfo.id, keyboardCoversBar = false)) {
when (chatInfo) {
is ChatInfo.Direct, is ChatInfo.Group, is ChatInfo.Local -> {
var groupMembersJob: Job = remember { Job() }
val perChatTheme = remember(chatInfo, CurrentColors.value.base) { if (chatInfo is ChatInfo.Direct) chatInfo.contact.uiThemes?.preferredMode(!CurrentColors.value.colors.isLight) else if (chatInfo is ChatInfo.Group) chatInfo.groupInfo.uiThemes?.preferredMode(!CurrentColors.value.colors.isLight) else null }
val overrides = if (perChatTheme != null) ThemeManager.currentColors(null, perChatTheme, chatModel.currentUser.value?.uiThemes, appPrefs.themeOverrides.get()) else null
val fullDeleteAllowed = remember(chatInfo) { chatInfo.featureEnabled(ChatFeature.FullDelete) }
@@ -220,8 +221,8 @@ fun ChatView(staleChatId: State<String?>, onComposed: suspend (chatId: String) -
hideKeyboard(view)
AudioPlayer.stop()
chatModel.chatId.value = null
chatModel.groupMembers.clear()
chatModel.groupMembersIndexes.clear()
chatModel.groupMembers.value = emptyList()
chatModel.groupMembersIndexes.value = emptyMap()
},
info = {
if (ModalManager.end.hasModalsOpen()) {
@@ -229,7 +230,8 @@ fun ChatView(staleChatId: State<String?>, onComposed: suspend (chatId: String) -
return@ChatLayout
}
hideKeyboard(view)
withBGApi {
groupMembersJob.cancel()
groupMembersJob = scope.launch(Dispatchers.Default) {
// The idea is to preload information before showing a modal because large groups can take time to load all members
var preloadedContactInfo: Pair<ConnectionStats?, Profile?>? = null
var preloadedCode: String? = null
@@ -241,6 +243,8 @@ fun ChatView(staleChatId: State<String?>, onComposed: suspend (chatId: String) -
setGroupMembers(chatRh, chatInfo.groupInfo, chatModel)
preloadedLink = chatModel.controller.apiGetGroupLink(chatRh, chatInfo.groupInfo.groupId)
}
if (!isActive) return@launch
ModalManager.end.showModalCloseable(true) { close ->
val chatInfo = remember { activeChatInfo }.value
if (chatInfo is ChatInfo.Direct) {
@@ -276,7 +280,8 @@ fun ChatView(staleChatId: State<String?>, onComposed: suspend (chatId: String) -
},
showMemberInfo = { groupInfo: GroupInfo, member: GroupMember ->
hideKeyboard(view)
withBGApi {
groupMembersJob.cancel()
groupMembersJob = scope.launch(Dispatchers.Default) {
val r = chatModel.controller.apiGroupMemberInfo(chatRh, groupInfo.groupId, member.groupMemberId)
val stats = r?.second
val (_, code) = if (member.memberActive) {
@@ -286,6 +291,8 @@ fun ChatView(staleChatId: State<String?>, onComposed: suspend (chatId: String) -
member to null
}
setGroupMembers(chatRh, groupInfo, chatModel)
if (!isActive) return@launch
ModalManager.end.closeModals()
ModalManager.end.showModalCloseable(true) { close ->
remember { derivedStateOf { chatModel.getGroupMember(member.groupMemberId) } }.value?.let { mem ->
@@ -431,7 +438,7 @@ fun ChatView(staleChatId: State<String?>, onComposed: suspend (chatId: String) -
chatModel.getChat(chatId)
},
findModelMember = { memberId ->
chatModel.groupMembers.find { it.id == memberId }
chatModel.groupMembers.value.find { it.id == memberId }
},
setReaction = { cInfo, cItem, add, reaction ->
withBGApi {
@@ -451,17 +458,19 @@ fun ChatView(staleChatId: State<String?>, onComposed: suspend (chatId: String) -
}
},
showItemDetails = { cInfo, cItem ->
suspend fun loadChatItemInfo(): ChatItemInfo? {
suspend fun loadChatItemInfo(): ChatItemInfo? = coroutineScope {
val ciInfo = chatModel.controller.apiGetChatItemInfo(chatRh, cInfo.chatType, cInfo.apiId, cItem.id)
if (ciInfo != null) {
if (chatInfo is ChatInfo.Group) {
setGroupMembers(chatRh, chatInfo.groupInfo, chatModel)
if (!isActive) return@coroutineScope null
}
}
return ciInfo
ciInfo
}
withBGApi {
var initialCiInfo = loadChatItemInfo() ?: return@withBGApi
groupMembersJob.cancel()
groupMembersJob = scope.launch(Dispatchers.Default) {
var initialCiInfo = loadChatItemInfo() ?: return@launch
ModalManager.end.closeModals()
ModalManager.end.showModalCloseable(endButtons = {
ShareButton {

View File

@@ -83,7 +83,7 @@ fun AddGroupMembersView(rhId: Long?, groupInfo: GroupInfo, creatingGroup: Boolea
fun getContactsToAdd(chatModel: ChatModel, search: String): List<Contact> {
val s = search.trim().lowercase()
val memberContactIds = chatModel.groupMembers
val memberContactIds = chatModel.groupMembers.value
.filter { it.memberCurrent }
.mapNotNull { it.memberContactId }
return chatModel.chats.value

View File

@@ -40,7 +40,7 @@ import chat.simplex.common.views.chat.item.ItemAction
import chat.simplex.common.views.chatlist.*
import chat.simplex.res.MR
import dev.icerock.moko.resources.StringResource
import kotlinx.coroutines.launch
import kotlinx.coroutines.*
const val SMALL_GROUPS_RCPS_MEM_LIMIT: Int = 20
@@ -54,6 +54,7 @@ fun ModalData.GroupChatInfoView(chatModel: ChatModel, rhId: Long?, chatId: Strin
if (chat != null && chat.chatInfo is ChatInfo.Group && currentUser != null) {
val groupInfo = chat.chatInfo.groupInfo
val sendReceipts = remember { mutableStateOf(SendReceipts.fromBool(groupInfo.chatSettings.sendRcpts, currentUser.sendRcptsSmallGroups)) }
val scope = rememberCoroutineScope()
GroupChatInfoLayout(
chat,
groupInfo,
@@ -64,14 +65,16 @@ fun ModalData.GroupChatInfoView(chatModel: ChatModel, rhId: Long?, chatId: Strin
updateChatSettings(chat.remoteHostId, chat.chatInfo, chatSettings, chatModel)
sendReceipts.value = sendRcpts
},
members = chatModel.groupMembers
members = remember { chatModel.groupMembers }.value
.filter { it.memberStatus != GroupMemberStatus.MemLeft && it.memberStatus != GroupMemberStatus.MemRemoved }
.sortedByDescending { it.memberRole },
developerTools,
groupLink,
addMembers = {
withBGApi {
scope.launch(Dispatchers.Default) {
setGroupMembers(rhId, groupInfo, chatModel)
if (!isActive) return@launch
ModalManager.end.showModalCloseable(true) { close ->
AddGroupMembersView(rhId, groupInfo, false, chatModel, close)
}

View File

@@ -232,9 +232,10 @@ suspend fun apiFindMessages(ch: Chat, search: String) {
apiLoadMessages(ch.remoteHostId, ch.chatInfo.chatType, ch.chatInfo.apiId, pagination = ChatPagination.Last(ChatPagination.INITIAL_COUNT), chatModel.chatState, search = search)
}
suspend fun setGroupMembers(rhId: Long?, groupInfo: GroupInfo, chatModel: ChatModel) {
suspend fun setGroupMembers(rhId: Long?, groupInfo: GroupInfo, chatModel: ChatModel) = coroutineScope {
// groupMembers loading can take a long time and if the user already closed the screen, coroutine may be canceled
val groupMembers = chatModel.controller.apiListMembers(rhId, groupInfo.groupId)
val currentMembers = chatModel.groupMembers
val currentMembers = chatModel.groupMembers.value
val newMembers = groupMembers.map { newMember ->
val currentMember = currentMembers.find { it.id == newMember.id }
val currentMemberStats = currentMember?.activeConn?.connectionStats
@@ -245,9 +246,8 @@ suspend fun setGroupMembers(rhId: Long?, groupInfo: GroupInfo, chatModel: ChatMo
newMember
}
}
chatModel.groupMembers.clear()
chatModel.groupMembersIndexes.clear()
chatModel.groupMembers.addAll(newMembers)
chatModel.groupMembersIndexes.value = emptyMap()
chatModel.groupMembers.value = newMembers
chatModel.populateGroupMembersIndexes()
}