From 95fb2541f645d93e0167b480a1b17f8c6ac34894 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 1 Oct 2024 14:49:13 +0200 Subject: [PATCH 1/9] check if sync succeeded Decrease message limit for retries of message loading make it possible to add any amount (up to 100) of messages to UI for initial loading. add logging only make initial request for chat messages when newest message from DB is not equal the lastReadMessage that is offered by the conversation Signed-off-by: Marcel Hibbe --- .../network/OfflineFirstChatRepository.kt | 200 +++++++++++------- 1 file changed, 128 insertions(+), 72 deletions(-) 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 e7a49ac41..262188b03 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 @@ -108,26 +108,63 @@ class OfflineFirstChatRepository @Inject constructor( override fun loadInitialMessages(withNetworkParams: Bundle): Job = scope.launch { Log.d(TAG, "---- loadInitialMessages ------------") + Log.d(TAG, "conversationModel.internalId: " + conversationModel.internalId) newXChatLastCommonRead = conversationModel.lastCommonReadMessage - val fieldMap = getFieldMap( - lookIntoFuture = false, - includeLastKnown = true, - setReadMarker = true, - lastKnown = null - ) - withNetworkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap) - withNetworkParams.putString(BundleKeys.KEY_ROOM_TOKEN, conversationModel.token) + Log.d(TAG, "conversationModel.lastReadMessage:" + conversationModel.lastReadMessage) - sync(withNetworkParams) + var newestMessageId = chatDao.getNewestMessageId(internalConversationId) + Log.d(TAG, "newestMessageId: $newestMessageId") + if (newestMessageId.toInt() == 0) { + Log.d(TAG, "newestMessageId from db was 0. Must only happen when chat is loaded for the first time") + } - val newestMessageId = chatDao.getNewestMessageId(internalConversationId) - Log.d(TAG, "newestMessageId after sync: $newestMessageId") + if (conversationModel.lastReadMessage.toLong() != newestMessageId) { + Log.d(TAG, "An online request is made because chat is not up to date") - showLast100MessagesBeforeAndEqual( + // set up field map to load the newest messages + val fieldMap = getFieldMap( + lookIntoFuture = false, + includeLastKnown = true, + setReadMarker = true, + lastKnown = null + ) + withNetworkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap) + withNetworkParams.putString(BundleKeys.KEY_ROOM_TOKEN, conversationModel.token) + + Log.d(TAG, "Starting online request for initial loading") + val chatMessageEntities = sync(withNetworkParams) + if (chatMessageEntities == null) { + Log.e(TAG, "initial loading of messages failed") + } + + newestMessageId = chatDao.getNewestMessageId(internalConversationId) + Log.d(TAG, "newestMessageId after sync: $newestMessageId") + } else { + Log.d(TAG, "Initial online request is skipped because offline messages are up to date") + } + + val chatBlock = getBlockOfMessage(newestMessageId.toInt()) + + val amountBetween = chatDao.getCountBetweenMessageIds( internalConversationId, - chatDao.getNewestMessageId(internalConversationId) + newestMessageId, + chatBlock!!.oldestMessageId + ) + + Log.d(TAG, "amount of messages between newestMessageId and oldest message of same ChatBlock:$amountBetween") + val limit = if (amountBetween > DEFAULT_MESSAGES_LIMIT) { + DEFAULT_MESSAGES_LIMIT + } else { + amountBetween + } + Log.d(TAG, "limit of messages to load for UI (max 100 to ensure performance is okay):$limit") + + showMessagesBeforeAndEqual( + internalConversationId, + newestMessageId, + limit ) // delay is a dirty workaround to make sure messages are added to adapter on initial load before dealing @@ -175,10 +212,11 @@ class OfflineFirstChatRepository @Inject constructor( val loadFromServer = hasToLoadPreviousMessagesFromServer(beforeMessageId) if (loadFromServer) { + Log.d(TAG, "Starting online request for loadMoreMessages") sync(withNetworkParams) } - showLast100MessagesBefore(internalConversationId, beforeMessageId) + showMessagesBefore(internalConversationId, beforeMessageId, DEFAULT_MESSAGES_LIMIT) updateUiForLastCommonRead() } @@ -205,6 +243,7 @@ class OfflineFirstChatRepository @Inject constructor( // sync database with server (This is a long blocking call because long polling (lookIntoFuture) is set) networkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap) + Log.d(TAG, "Starting online request for long polling") val resultsFromSync = sync(networkParams) if (!resultsFromSync.isNullOrEmpty()) { val chatMessages = resultsFromSync.map(ChatMessageEntity::asModel) @@ -240,15 +279,15 @@ class OfflineFirstChatRepository @Inject constructor( loadFromServer = false } else { // we know that beforeMessageId and blockForMessage.oldestMessageId are in the same block. - // As we want the last 100 entries before beforeMessageId, we calculate if these messages are 100 - // entries apart from each other + // As we want the last DEFAULT_MESSAGES_LIMIT entries before beforeMessageId, we calculate if these + // messages are DEFAULT_MESSAGES_LIMIT entries apart from each other val amountBetween = chatDao.getCountBetweenMessageIds( internalConversationId, beforeMessageId, blockForMessage.oldestMessageId ) - loadFromServer = amountBetween < 100 + loadFromServer = amountBetween < DEFAULT_MESSAGES_LIMIT Log.d( TAG, @@ -263,7 +302,8 @@ class OfflineFirstChatRepository @Inject constructor( lookIntoFuture: Boolean, includeLastKnown: Boolean, setReadMarker: Boolean, - lastKnown: Int? + lastKnown: Int?, + limit: Int = DEFAULT_MESSAGES_LIMIT ): HashMap { val fieldMap = HashMap() @@ -278,7 +318,7 @@ class OfflineFirstChatRepository @Inject constructor( } fieldMap["timeout"] = if (lookIntoFuture) 30 else 0 - fieldMap["limit"] = 100 + fieldMap["limit"] = limit fieldMap["lookIntoFuture"] = if (lookIntoFuture) 1 else 0 fieldMap["setReadMarker"] = if (setReadMarker) 1 else 0 @@ -294,12 +334,12 @@ class OfflineFirstChatRepository @Inject constructor( lookIntoFuture = false, includeLastKnown = true, setReadMarker = false, - lastKnown = messageId.toInt() + lastKnown = messageId.toInt(), + limit = 1 ) bundle.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap) - // Although only the single message will be returned, a server request will load 100 messages. - // If this turns out to be confusion for debugging we could load set the limit to 1 for this request. + Log.d(TAG, "Starting online request for single message (e.g. a reply)") sync(bundle) } return chatDao.getChatMessageForConversation(internalConversationId, messageId) @@ -308,59 +348,70 @@ class OfflineFirstChatRepository @Inject constructor( @Suppress("UNCHECKED_CAST") private fun getMessagesFromServer(bundle: Bundle): Pair>? { - Log.d(TAG, "An online request is made!!!!!!!!!!!!!!!!!!!!") val fieldMap = bundle.getSerializable(BundleKeys.KEY_FIELD_MAP) as HashMap - try { - val result = network.pullChatMessages(credentials, urlForChatting, fieldMap) - .subscribeOn(Schedulers.io()) - .observeOn(AndroidSchedulers.mainThread()) - // .timeout(3, TimeUnit.SECONDS) - .map { it -> - when (it.code()) { - HTTP_CODE_OK -> { - Log.d(TAG, "getMessagesFromServer HTTP_CODE_OK") - newXChatLastCommonRead = it.headers()["X-Chat-Last-Common-Read"]?.let { - Integer.parseInt(it) + var attempts = 1 + while (attempts < 5) { + Log.d(TAG, "message limit: " + fieldMap["limit"]) + try { + val result = network.pullChatMessages(credentials, urlForChatting, fieldMap) + .subscribeOn(Schedulers.io()) + .observeOn(AndroidSchedulers.mainThread()) + .map { it -> + when (it.code()) { + HTTP_CODE_OK -> { + Log.d(TAG, "getMessagesFromServer HTTP_CODE_OK") + newXChatLastCommonRead = it.headers()["X-Chat-Last-Common-Read"]?.let { + Integer.parseInt(it) + } + + return@map Pair( + HTTP_CODE_OK, + (it.body() as ChatOverall).ocs!!.data!! + ) } - return@map Pair( - HTTP_CODE_OK, - (it.body() as ChatOverall).ocs!!.data!! - ) - } + HTTP_CODE_NOT_MODIFIED -> { + Log.d(TAG, "getMessagesFromServer HTTP_CODE_NOT_MODIFIED") - HTTP_CODE_NOT_MODIFIED -> { - Log.d(TAG, "getMessagesFromServer HTTP_CODE_NOT_MODIFIED") + return@map Pair( + HTTP_CODE_NOT_MODIFIED, + listOf() + ) + } - return@map Pair( - HTTP_CODE_NOT_MODIFIED, - listOf() - ) - } + HTTP_CODE_PRECONDITION_FAILED -> { + Log.d(TAG, "getMessagesFromServer HTTP_CODE_PRECONDITION_FAILED") - HTTP_CODE_PRECONDITION_FAILED -> { - Log.d(TAG, "getMessagesFromServer HTTP_CODE_PRECONDITION_FAILED") + return@map Pair( + HTTP_CODE_PRECONDITION_FAILED, + listOf() + ) + } - return@map Pair( - HTTP_CODE_PRECONDITION_FAILED, - listOf() - ) - } - - else -> { - return@map Pair( - HTTP_CODE_PRECONDITION_FAILED, - listOf() - ) + else -> { + return@map Pair( + HTTP_CODE_PRECONDITION_FAILED, + listOf() + ) + } } } + .blockingSingle() + return result + } catch (e: Exception) { + Log.e(TAG, "Something went wrong when pulling chat messages (attempt: $attempts)", e) + attempts++ + + val newMessageLimit = when (attempts) { + 2 -> 50 + 3 -> 10 + else -> 5 } - .blockingSingle() - return result - } catch (e: Exception) { - Log.e(TAG, "Something went wrong when pulling chat messages", e) + fieldMap["limit"] = newMessageLimit + } } + Log.e(TAG, "All attempts to get messages from server failed") return null } @@ -370,7 +421,12 @@ class OfflineFirstChatRepository @Inject constructor( return null } - val result = getMessagesFromServer(bundle) ?: return listOf() + val result = getMessagesFromServer(bundle) + if (result == null) { + Log.d(TAG, "No result from server") + return null + } + var chatMessagesFromSync: List? = null val fieldMap = bundle.getSerializable(BundleKeys.KEY_FIELD_MAP) as HashMap @@ -471,7 +527,7 @@ class OfflineFirstChatRepository @Inject constructor( ChatMessage.SystemMessageType.CLEARED_CHAT -> { // for lookIntoFuture just deleting everything would be fine. // But lets say we did not open the chat for a while and in between it was cleared. - // We just load the last 100 messages but this don't contain the system message. + // We just load the last messages but this don't contain the system message. // We scroll up and load the system message. Deleting everything is not an option as we // would loose the messages that we want to keep. We only want to // delete the messages and chatBlocks older than the system message. @@ -488,13 +544,12 @@ class OfflineFirstChatRepository @Inject constructor( * 304 is returned when oldest message of chat was queried or when long polling request returned with no * modification. hasHistory is only set to false, when 304 was returned for the the oldest message */ - private fun getHasHistory(statusCode: Int, lookIntoFuture: Boolean): Boolean { - return if (statusCode == HTTP_CODE_NOT_MODIFIED) { + private fun getHasHistory(statusCode: Int, lookIntoFuture: Boolean): Boolean = + if (statusCode == HTTP_CODE_NOT_MODIFIED) { lookIntoFuture } else { true } - } private suspend fun getBlockOfMessage(queriedMessageId: Int?): ChatBlockEntity? { var blockContainingQueriedMessage: ChatBlockEntity? = null @@ -563,7 +618,7 @@ class OfflineFirstChatRepository @Inject constructor( } } - private suspend fun showLast100MessagesBeforeAndEqual(internalConversationId: String, messageId: Long) { + private suspend fun showMessagesBeforeAndEqual(internalConversationId: String, messageId: Long, limit: Int) { suspend fun getMessagesBeforeAndEqual( messageId: Long, internalConversationId: String, @@ -580,7 +635,7 @@ class OfflineFirstChatRepository @Inject constructor( val list = getMessagesBeforeAndEqual( messageId, internalConversationId, - 100 + limit ) if (list.isNotEmpty()) { @@ -589,7 +644,7 @@ class OfflineFirstChatRepository @Inject constructor( } } - private suspend fun showLast100MessagesBefore(internalConversationId: String, messageId: Long) { + private suspend fun showMessagesBefore(internalConversationId: String, messageId: Long, limit: Int) { suspend fun getMessagesBefore( messageId: Long, internalConversationId: String, @@ -606,7 +661,7 @@ class OfflineFirstChatRepository @Inject constructor( val list = getMessagesBefore( messageId, internalConversationId, - 100 + limit ) if (list.isNotEmpty()) { @@ -638,5 +693,6 @@ class OfflineFirstChatRepository @Inject constructor( private const val HTTP_CODE_PRECONDITION_FAILED = 412 private const val HALF_SECOND = 500L private const val DELAY_TO_ENSURE_MESSAGES_ARE_ADDED: Long = 100 + private const val DEFAULT_MESSAGES_LIMIT = 100 } } From c04871786ef1fa2fcab60d513f8d10b4e0a96634 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 1 Oct 2024 15:54:09 +0200 Subject: [PATCH 2/9] load conversation from DB first, then update by request if connection is available Signed-off-by: Marcel Hibbe --- .../talk/chat/viewmodels/ChatViewModel.kt | 2 +- .../data/OfflineConversationsRepository.kt | 2 +- .../OfflineFirstConversationsRepository.kt | 20 ++++++++++--------- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt index d0c665996..39a09bdcb 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/viewmodels/ChatViewModel.kt @@ -225,7 +225,7 @@ class ChatViewModel @Inject constructor( fun getRoom(user: User, token: String) { _getRoomViewState.value = GetRoomStartState - conversationRepository.getConversationSettings(token) + conversationRepository.getRoom(token) // chatNetworkDataSource.getRoom(user, token) // .subscribeOn(Schedulers.io()) diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt index a264a84ee..92591fe4e 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/data/OfflineConversationsRepository.kt @@ -35,5 +35,5 @@ interface OfflineConversationsRepository { * Called once onStart to emit a conversation to [conversationFlow] * to be handled asynchronously. */ - fun getConversationSettings(roomToken: String): Job + fun getRoom(roomToken: String): Job } diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt index 1258ee649..c9d276201 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt @@ -56,17 +56,19 @@ class OfflineFirstConversationsRepository @Inject constructor( override fun getRooms(): Job = scope.launch { - val resultsFromSync = sync() - if (!resultsFromSync.isNullOrEmpty()) { - val conversations = resultsFromSync.map(ConversationEntity::asModel) - _roomListFlow.emit(conversations) - } else { - val conversationsFromDb = getListOfConversations(user.id!!) - _roomListFlow.emit(conversationsFromDb) + val initialConversationModels = getListOfConversations(user.id!!) + _roomListFlow.emit(initialConversationModels) + + if (monitor.isOnline.first()) { + val conversationEntitiesFromSync = getRoomsFromServer() + if (!conversationEntitiesFromSync.isNullOrEmpty()) { + val conversationModelsFromSync = conversationEntitiesFromSync.map(ConversationEntity::asModel) + _roomListFlow.emit(conversationModelsFromSync) + } } } - override fun getConversationSettings(roomToken: String): Job = + override fun getRoom(roomToken: String): Job = scope.launch { val id = user.id!! val model = getConversation(id, roomToken) @@ -100,7 +102,7 @@ class OfflineFirstConversationsRepository @Inject constructor( } } - private suspend fun sync(): List? { + private suspend fun getRoomsFromServer(): List? { var conversationsFromSync: List? = null if (!monitor.isOnline.first()) { From 57cd3af904acbe74dbca7b2de82f004c050a885f Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 2 Oct 2024 15:07:37 +0200 Subject: [PATCH 3/9] delay progress bar delay progress bar for one second before showing up for slow connection Signed-off-by: Marcel Hibbe --- .../main/java/com/nextcloud/talk/chat/ChatActivity.kt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt index 1e82b1e6d..7f87c3594 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt @@ -183,6 +183,7 @@ import io.reactivex.Observer import io.reactivex.disposables.Disposable import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch @@ -416,7 +417,12 @@ class ChatActivity : messageInputViewModel = ViewModelProvider(this, viewModelFactory)[MessageInputViewModel::class.java] - binding.progressBar.visibility = View.VISIBLE + this.lifecycleScope.launch { + delay(DELAY_TO_SHOW_PROGRESS_BAR) + if (adapter?.isEmpty == true) { + binding.progressBar.visibility = View.VISIBLE + } + } onBackPressedDispatcher.addCallback(this, onBackPressedCallback) @@ -3691,5 +3697,6 @@ class ChatActivity : private const val CURRENT_AUDIO_POSITION_KEY = "CURRENT_AUDIO_POSITION" private const val CURRENT_AUDIO_WAS_PLAYING_KEY = "CURRENT_AUDIO_PLAYING" private const val RESUME_AUDIO_TAG = "RESUME_AUDIO_TAG" + private const val DELAY_TO_SHOW_PROGRESS_BAR = 1000L } } From bb607b065eb2ec92e7d7775ebfe3a75446dede48 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 9 Oct 2024 16:21:53 +0200 Subject: [PATCH 4/9] only do initial request if newestMessageIdFromDb is lower than lastReadMessage from conversation If conversation has a newer message id than DB then an online request is necessary If conversation has an older message id than DB then an online request is not necessary (this could happen when updating of DB is implemented for push notification, not yet done). If conversation has the same message id like DB than request can be skipped Signed-off-by: Marcel Hibbe --- .../network/OfflineFirstChatRepository.kt | 35 +++++++++++++------ 1 file changed, 25 insertions(+), 10 deletions(-) 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 262188b03..80fd70c84 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 @@ -114,13 +114,28 @@ class OfflineFirstChatRepository @Inject constructor( Log.d(TAG, "conversationModel.lastReadMessage:" + conversationModel.lastReadMessage) - var newestMessageId = chatDao.getNewestMessageId(internalConversationId) - Log.d(TAG, "newestMessageId: $newestMessageId") - if (newestMessageId.toInt() == 0) { + var newestMessageIdFromDb = chatDao.getNewestMessageId(internalConversationId) + Log.d(TAG, "newestMessageId: $newestMessageIdFromDb") + if (newestMessageIdFromDb.toInt() == 0) { Log.d(TAG, "newestMessageId from db was 0. Must only happen when chat is loaded for the first time") } - if (conversationModel.lastReadMessage.toLong() != newestMessageId) { + // infos from Ivan to + // "Why is it lastReadMessageId that is checked? Shouldn't it be lastMessage instead? + // https://github.com/nextcloud/talk-ios/blob/master/NextcloudTalk/NCChatController.m#L473 " + // + // answer: + // "I guess we do it with the lastReadMessageId in order to place the separator of "Unread messages" when you enter the chat" + // + // if it turns out lastMessage can be used instead lastReadMessage, use this: + // val doInitialLoadFromServer = conversationModel.lastMessage?.let { + // newestMessageIdFromDb < it.id + // } ?: true + // Log.d(TAG, "doInitialLoadFromServer:$doInitialLoadFromServer") + // + // if (doInitialLoadFromServer) { + + if (newestMessageIdFromDb < conversationModel.lastReadMessage.toLong()) { Log.d(TAG, "An online request is made because chat is not up to date") // set up field map to load the newest messages @@ -139,17 +154,17 @@ class OfflineFirstChatRepository @Inject constructor( Log.e(TAG, "initial loading of messages failed") } - newestMessageId = chatDao.getNewestMessageId(internalConversationId) - Log.d(TAG, "newestMessageId after sync: $newestMessageId") + newestMessageIdFromDb = chatDao.getNewestMessageId(internalConversationId) + Log.d(TAG, "newestMessageId after sync: $newestMessageIdFromDb") } else { Log.d(TAG, "Initial online request is skipped because offline messages are up to date") } - val chatBlock = getBlockOfMessage(newestMessageId.toInt()) + val chatBlock = getBlockOfMessage(newestMessageIdFromDb.toInt()) val amountBetween = chatDao.getCountBetweenMessageIds( internalConversationId, - newestMessageId, + newestMessageIdFromDb, chatBlock!!.oldestMessageId ) @@ -163,7 +178,7 @@ class OfflineFirstChatRepository @Inject constructor( showMessagesBeforeAndEqual( internalConversationId, - newestMessageId, + newestMessageIdFromDb, limit ) @@ -172,7 +187,7 @@ class OfflineFirstChatRepository @Inject constructor( delay(DELAY_TO_ENSURE_MESSAGES_ARE_ADDED) updateUiForLastCommonRead() - updateUiForLastReadMessage(newestMessageId) + updateUiForLastReadMessage(newestMessageIdFromDb) initMessagePolling() } From fac2f540264d71ee362dfb49191696127a09ffb7 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 16 Oct 2024 14:20:45 +0200 Subject: [PATCH 5/9] Fix unintended deletion of conversations (+related messages&chatBlocks) Mistake was, that the conversations from DB and sync could differ due to values. E.g. when a user changed the status, the conversations from DB and sync would differ. So there were conversations (+related messages&chatBlocks) deleted sometimes. This caused bugs that when entering a chat, all data was loaded again. In the previous implementation (before this PR), this error was only visible in the UI when you were offline (in this case, nothing was displayed!). To fix the bug, only the internalId's are compared. Signed-off-by: Marcel Hibbe --- .../data/network/OfflineFirstConversationsRepository.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt index c9d276201..f02eeff40 100644 --- a/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/conversationlist/data/network/OfflineFirstConversationsRepository.kt @@ -131,10 +131,12 @@ class OfflineFirstConversationsRepository @Inject constructor( } private suspend fun deleteLeftConversations(conversationsFromSync: List) { + val conversationsFromSyncIds = conversationsFromSync.map { it.internalId }.toSet() val oldConversationsFromDb = dao.getConversationsForUser(user.id!!).first() - val conversationsToDelete = oldConversationsFromDb.filterNot { conversationsFromSync.contains(it) } - val conversationIdsToDelete = conversationsToDelete.map { it.internalId } + val conversationIdsToDelete = oldConversationsFromDb + .map { it.internalId } + .filterNot { it in conversationsFromSyncIds } dao.deleteConversations(conversationIdsToDelete) } From e6b1b9fa504eb765b2955be33f8f920f4894e89d Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 16 Oct 2024 15:20:23 +0200 Subject: [PATCH 6/9] extract getCappedMessagesAmountOfChatBlock Signed-off-by: Marcel Hibbe --- .../network/OfflineFirstChatRepository.kt | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) 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 80fd70c84..bb3b2dfc2 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 @@ -160,21 +160,7 @@ class OfflineFirstChatRepository @Inject constructor( Log.d(TAG, "Initial online request is skipped because offline messages are up to date") } - val chatBlock = getBlockOfMessage(newestMessageIdFromDb.toInt()) - - val amountBetween = chatDao.getCountBetweenMessageIds( - internalConversationId, - newestMessageIdFromDb, - chatBlock!!.oldestMessageId - ) - - Log.d(TAG, "amount of messages between newestMessageId and oldest message of same ChatBlock:$amountBetween") - val limit = if (amountBetween > DEFAULT_MESSAGES_LIMIT) { - DEFAULT_MESSAGES_LIMIT - } else { - amountBetween - } - Log.d(TAG, "limit of messages to load for UI (max 100 to ensure performance is okay):$limit") + val limit = getCappedMessagesAmountOfChatBlock(newestMessageIdFromDb) showMessagesBeforeAndEqual( internalConversationId, @@ -192,6 +178,25 @@ class OfflineFirstChatRepository @Inject constructor( initMessagePolling() } + private suspend fun getCappedMessagesAmountOfChatBlock(messageId: Long): Int { + val chatBlock = getBlockOfMessage(messageId.toInt()) + + val amountBetween = chatDao.getCountBetweenMessageIds( + internalConversationId, + messageId, + chatBlock!!.oldestMessageId + ) + + Log.d(TAG, "amount of messages between newestMessageId and oldest message of same ChatBlock:$amountBetween") + val limit = if (amountBetween > DEFAULT_MESSAGES_LIMIT) { + DEFAULT_MESSAGES_LIMIT + } else { + amountBetween + } + Log.d(TAG, "limit of messages to load for UI (max 100 to ensure performance is okay):$limit") + return limit + } + private suspend fun updateUiForLastReadMessage(newestMessageId: Long) { val scrollToLastRead = conversationModel.lastReadMessage.toLong() < newestMessageId if (scrollToLastRead) { From 456fa54f1f9f457cc50af78cdab788c217b41b8a Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Wed, 16 Oct 2024 16:11:26 +0200 Subject: [PATCH 7/9] pass newestMessageIdFromDb to initMessagePolling Signed-off-by: Marcel Hibbe --- .../nextcloud/talk/chat/data/ChatMessageRepository.kt | 4 +--- .../talk/chat/data/network/OfflineFirstChatRepository.kt | 9 +++++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt b/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt index d39fb8541..a55a45803 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/data/ChatMessageRepository.kt @@ -56,10 +56,8 @@ interface ChatMessageRepository : LifecycleAwareManager { * Long polls the server for any updates to the chat, if found, it synchronizes * the database with the server and emits the new messages to [messageFlow], * else it simply retries after timeout. - * - * [withNetworkParams] credentials and url. */ - fun initMessagePolling(): Job + fun initMessagePolling(initialMessageId: Long): Job /** * Gets a individual message. 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 bb3b2dfc2..012428ed8 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 @@ -158,6 +158,8 @@ class OfflineFirstChatRepository @Inject constructor( Log.d(TAG, "newestMessageId after sync: $newestMessageIdFromDb") } else { Log.d(TAG, "Initial online request is skipped because offline messages are up to date") + // if conversationModel was not up to date and there are new messages, they will just get pulled with + // look into future.. Old messages will be displayed immediately beforehand. } val limit = getCappedMessagesAmountOfChatBlock(newestMessageIdFromDb) @@ -175,7 +177,7 @@ class OfflineFirstChatRepository @Inject constructor( updateUiForLastCommonRead() updateUiForLastReadMessage(newestMessageIdFromDb) - initMessagePolling() + initMessagePolling(newestMessageIdFromDb) } private suspend fun getCappedMessagesAmountOfChatBlock(messageId: Long): Int { @@ -240,18 +242,17 @@ class OfflineFirstChatRepository @Inject constructor( updateUiForLastCommonRead() } - override fun initMessagePolling(): Job = + override fun initMessagePolling(initialMessageId: Long): Job = scope.launch { Log.d(TAG, "---- initMessagePolling ------------") - val initialMessageId = chatDao.getNewestMessageId(internalConversationId).toInt() Log.d(TAG, "newestMessage: $initialMessageId") var fieldMap = getFieldMap( lookIntoFuture = true, includeLastKnown = false, setReadMarker = true, - lastKnown = initialMessageId + lastKnown = initialMessageId.toInt() ) val networkParams = Bundle() From b1e390744b65ecd1e30c8d5785853d6a1eeb0ced Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 17 Oct 2024 11:54:02 +0200 Subject: [PATCH 8/9] refactoring and logging Signed-off-by: Marcel Hibbe --- .../com/nextcloud/talk/chat/ChatActivity.kt | 133 +++++++++--------- .../network/OfflineFirstChatRepository.kt | 53 ++++--- 2 files changed, 91 insertions(+), 95 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt index 7f87c3594..503bd9c5c 100644 --- a/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt @@ -1235,9 +1235,7 @@ class ChatActivity : @Suppress("MagicNumber", "LongMethod") private fun updateTypingIndicator() { - fun ellipsize(text: String): String { - return DisplayUtils.ellipsize(text, TYPING_INDICATOR_MAX_NAME_LENGTH) - } + fun ellipsize(text: String): String = DisplayUtils.ellipsize(text, TYPING_INDICATOR_MAX_NAME_LENGTH) val participantNames = ArrayList() @@ -1311,10 +1309,9 @@ class ChatActivity : } } - private fun isTypingStatusEnabled(): Boolean { - return webSocketInstance != null && + private fun isTypingStatusEnabled(): Boolean = + webSocketInstance != null && !CapabilitiesUtil.isTypingStatusPrivate(conversationUser!!) - } private fun setupSwipeToReply() { if (this::participantPermissions.isInitialized && @@ -1413,15 +1410,18 @@ class ChatActivity : } fun isOneToOneConversation() = - currentConversation != null && currentConversation?.type != null && + currentConversation != null && + currentConversation?.type != null && currentConversation?.type == ConversationEnums.ConversationType.ROOM_TYPE_ONE_TO_ONE_CALL private fun isGroupConversation() = - currentConversation != null && currentConversation?.type != null && + currentConversation != null && + currentConversation?.type != null && currentConversation?.type == ConversationEnums.ConversationType.ROOM_GROUP_CALL private fun isPublicConversation() = - currentConversation != null && currentConversation?.type != null && + currentConversation != null && + currentConversation?.type != null && currentConversation?.type == ConversationEnums.ConversationType.ROOM_PUBLIC_CALL private fun updateRoomTimerHandler() { @@ -1659,11 +1659,10 @@ class ChatActivity : adapter?.notifyDataSetChanged() } - private fun isChildOfExpandableSystemMessage(chatMessage: ChatMessage): Boolean { - return isSystemMessage(chatMessage) && + private fun isChildOfExpandableSystemMessage(chatMessage: ChatMessage): Boolean = + isSystemMessage(chatMessage) && !chatMessage.expandableParent && chatMessage.lastItemOfExpandableGroup != 0 - } @SuppressLint("NotifyDataSetChanged") override fun expandSystemMessage(chatMessageToExpand: ChatMessage) { @@ -1749,12 +1748,11 @@ class ChatActivity : } } - fun isRecordAudioPermissionGranted(): Boolean { - return PermissionChecker.checkSelfPermission( + fun isRecordAudioPermissionGranted(): Boolean = + PermissionChecker.checkSelfPermission( context, Manifest.permission.RECORD_AUDIO ) == PERMISSION_GRANTED - } fun requestRecordAudioPermissions() { requestPermissions( @@ -1861,11 +1859,10 @@ class ChatActivity : } } - private fun isReadOnlyConversation(): Boolean { - return currentConversation?.conversationReadOnlyState != null && + private fun isReadOnlyConversation(): Boolean = + currentConversation?.conversationReadOnlyState != null && currentConversation?.conversationReadOnlyState == ConversationEnums.ConversationReadOnlyState.CONVERSATION_READ_ONLY - } private fun checkLobbyState() { if (currentConversation != null && @@ -1881,7 +1878,8 @@ class ChatActivity : sb.append(resources!!.getText(R.string.nc_lobby_waiting)) .append("\n\n") - if (currentConversation?.lobbyTimer != null && currentConversation?.lobbyTimer != + if (currentConversation?.lobbyTimer != null && + currentConversation?.lobbyTimer != 0L ) { val timestampMS = (currentConversation?.lobbyTimer ?: 0) * DateConstants.SECOND_DIVIDER @@ -2080,7 +2078,7 @@ class ChatActivity : if (position != null && position >= 0) { binding.messagesListView.scrollToPosition(position) } else { - // TODO show error that we don't have that message? + Log.d(TAG, "message $messageId that should be scrolled to was not found (scrollToMessageWithId)") } } @@ -2092,6 +2090,12 @@ class ChatActivity : position, binding.messagesListView.height / 2 ) + } else { + Log.d( + TAG, + "message $messageId that should be scrolled to was not found " + + "(scrollToAndCenterMessageWithId)" + ) } } } @@ -2255,11 +2259,10 @@ class ChatActivity : startActivity(intent) } - private fun validSessionId(): Boolean { - return currentConversation != null && + private fun validSessionId(): Boolean = + currentConversation != null && sessionIdAfterRoomJoined?.isNotEmpty() == true && sessionIdAfterRoomJoined != "0" - } @Suppress("Detekt.TooGenericExceptionCaught") private fun cancelNotificationsForCurrentConversation() { @@ -2312,14 +2315,11 @@ class ChatActivity : } } - private fun isActivityNotChangingConfigurations(): Boolean { - return !isChangingConfigurations - } + private fun isActivityNotChangingConfigurations(): Boolean = !isChangingConfigurations - private fun isNotInCall(): Boolean { - return !ApplicationWideCurrentRoomHolder.getInstance().isInCall && + private fun isNotInCall(): Boolean = + !ApplicationWideCurrentRoomHolder.getInstance().isInCall && !ApplicationWideCurrentRoomHolder.getInstance().isDialing - } private fun setActionBarTitle() { val title = binding.chatToolbar.findViewById(R.id.chat_toolbar_title) @@ -2756,11 +2756,10 @@ class ChatActivity : } } - private fun isSameDayNonSystemMessages(messageLeft: ChatMessage, messageRight: ChatMessage): Boolean { - return TextUtils.isEmpty(messageLeft.systemMessage) && + private fun isSameDayNonSystemMessages(messageLeft: ChatMessage, messageRight: ChatMessage): Boolean = + TextUtils.isEmpty(messageLeft.systemMessage) && TextUtils.isEmpty(messageRight.systemMessage) && DateFormatter.isSameDay(messageLeft.createdAt, messageRight.createdAt) - } override fun onLoadMore(page: Int, totalItemsCount: Int) { val id = ( @@ -2780,15 +2779,14 @@ class ChatActivity : ) } - override fun format(date: Date): String { - return if (DateFormatter.isToday(date)) { + override fun format(date: Date): String = + if (DateFormatter.isToday(date)) { resources!!.getString(R.string.nc_date_header_today) } else if (DateFormatter.isYesterday(date)) { resources!!.getString(R.string.nc_date_header_yesterday) } else { DateFormatter.format(date, DateFormatter.Template.STRING_DAY_MONTH_YEAR) } - } override fun onCreateOptionsMenu(menu: Menu): Boolean { super.onCreateOptionsMenu(menu) @@ -2848,8 +2846,8 @@ class ChatActivity : return true } - override fun onOptionsItemSelected(item: MenuItem): Boolean { - return when (item.itemId) { + override fun onOptionsItemSelected(item: MenuItem): Boolean = + when (item.itemId) { R.id.conversation_video_call -> { startACall(false, false) true @@ -2877,7 +2875,6 @@ class ChatActivity : else -> super.onOptionsItemSelected(item) } - } private fun showSharedItems() { val intent = Intent(this, SharedItemsActivity::class.java) @@ -2939,25 +2936,23 @@ class ChatActivity : return chatMessageMap.values.toList() } - private fun isInfoMessageAboutDeletion(currentMessage: MutableMap.MutableEntry): Boolean { - return currentMessage.value.parentMessageId != null && currentMessage.value.systemMessageType == ChatMessage - .SystemMessageType.MESSAGE_DELETED - } + private fun isInfoMessageAboutDeletion(currentMessage: MutableMap.MutableEntry): Boolean = + currentMessage.value.parentMessageId != null && + currentMessage.value.systemMessageType == ChatMessage + .SystemMessageType.MESSAGE_DELETED - private fun isReactionsMessage(currentMessage: MutableMap.MutableEntry): Boolean { - return currentMessage.value.systemMessageType == ChatMessage.SystemMessageType.REACTION || + private fun isReactionsMessage(currentMessage: MutableMap.MutableEntry): Boolean = + currentMessage.value.systemMessageType == ChatMessage.SystemMessageType.REACTION || currentMessage.value.systemMessageType == ChatMessage.SystemMessageType.REACTION_DELETED || currentMessage.value.systemMessageType == ChatMessage.SystemMessageType.REACTION_REVOKED - } - private fun isEditMessage(currentMessage: MutableMap.MutableEntry): Boolean { - return currentMessage.value.parentMessageId != null && currentMessage.value.systemMessageType == ChatMessage - .SystemMessageType.MESSAGE_EDITED - } + private fun isEditMessage(currentMessage: MutableMap.MutableEntry): Boolean = + currentMessage.value.parentMessageId != null && + currentMessage.value.systemMessageType == ChatMessage + .SystemMessageType.MESSAGE_EDITED - private fun isPollVotedMessage(currentMessage: MutableMap.MutableEntry): Boolean { - return currentMessage.value.systemMessageType == ChatMessage.SystemMessageType.POLL_VOTED - } + private fun isPollVotedMessage(currentMessage: MutableMap.MutableEntry): Boolean = + currentMessage.value.systemMessageType == ChatMessage.SystemMessageType.POLL_VOTED private fun startACall(isVoiceOnlyCall: Boolean, callWithoutNotification: Boolean) { currentConversation?.let { @@ -3061,9 +3056,8 @@ class ChatActivity : } } - private fun isSystemMessage(message: ChatMessage): Boolean { - return ChatMessage.MessageType.SYSTEM_MESSAGE == message.getCalculateMessageType() - } + private fun isSystemMessage(message: ChatMessage): Boolean = + ChatMessage.MessageType.SYSTEM_MESSAGE == message.getCalculateMessageType() fun deleteMessage(message: IMessage) { if (!participantPermissions.hasChatPermission()) { @@ -3306,20 +3300,26 @@ class ChatActivity : fileViewerUtils.openFileInFilesApp(link!!, keyID!!) } - private fun hasVisibleItems(message: ChatMessage): Boolean { - return !message.isDeleted || // copy message - message.replyable || // reply to - message.replyable && // reply privately - conversationUser?.userId?.isNotEmpty() == true && conversationUser!!.userId != "?" && + private fun hasVisibleItems(message: ChatMessage): Boolean = + !message.isDeleted || + // copy message + message.replyable || + // reply to + message.replyable && + // reply privately + conversationUser?.userId?.isNotEmpty() == true && + conversationUser!!.userId != "?" && message.user.id.startsWith("users/") && message.user.id.substring(ACTOR_LENGTH) != currentConversation?.actorId && currentConversation?.type != ConversationEnums.ConversationType.ROOM_TYPE_ONE_TO_ONE_CALL || - isShowMessageDeletionButton(message) || // delete - ChatMessage.MessageType.REGULAR_TEXT_MESSAGE == message.getCalculateMessageType() || // forward - message.previousMessageId > NO_PREVIOUS_MESSAGE_ID && // mark as unread + isShowMessageDeletionButton(message) || + // delete + ChatMessage.MessageType.REGULAR_TEXT_MESSAGE == message.getCalculateMessageType() || + // forward + message.previousMessageId > NO_PREVIOUS_MESSAGE_ID && + // mark as unread ChatMessage.MessageType.SYSTEM_MESSAGE != message.getCalculateMessageType() && BuildConfig.DEBUG - } private fun setMessageAsDeleted(message: IMessage?) { val messageTemp = message as ChatMessage @@ -3437,8 +3437,8 @@ class ChatActivity : return isUserAllowedByPrivileges } - override fun hasContentFor(message: ChatMessage, type: Byte): Boolean { - return when (type) { + override fun hasContentFor(message: ChatMessage, type: Byte): Boolean = + when (type) { CONTENT_TYPE_LOCATION -> message.hasGeoLocation() CONTENT_TYPE_VOICE_MESSAGE -> message.isVoiceMessage CONTENT_TYPE_POLL -> message.isPoll() @@ -3449,7 +3449,6 @@ class ChatActivity : else -> false } - } private fun processMostRecentMessage(recent: ChatMessage, chatMessageList: List) { when (recent.systemMessageType) { 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 012428ed8..ea40912da 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 @@ -108,35 +108,36 @@ class OfflineFirstChatRepository @Inject constructor( override fun loadInitialMessages(withNetworkParams: Bundle): Job = scope.launch { Log.d(TAG, "---- loadInitialMessages ------------") - Log.d(TAG, "conversationModel.internalId: " + conversationModel.internalId) - newXChatLastCommonRead = conversationModel.lastCommonReadMessage + Log.d(TAG, "conversationModel.internalId: " + conversationModel.internalId) Log.d(TAG, "conversationModel.lastReadMessage:" + conversationModel.lastReadMessage) var newestMessageIdFromDb = chatDao.getNewestMessageId(internalConversationId) - Log.d(TAG, "newestMessageId: $newestMessageIdFromDb") - if (newestMessageIdFromDb.toInt() == 0) { - Log.d(TAG, "newestMessageId from db was 0. Must only happen when chat is loaded for the first time") - } + Log.d(TAG, "newestMessageIdFromDb: $newestMessageIdFromDb") - // infos from Ivan to - // "Why is it lastReadMessageId that is checked? Shouldn't it be lastMessage instead? - // https://github.com/nextcloud/talk-ios/blob/master/NextcloudTalk/NCChatController.m#L473 " - // - // answer: - // "I guess we do it with the lastReadMessageId in order to place the separator of "Unread messages" when you enter the chat" - // - // if it turns out lastMessage can be used instead lastReadMessage, use this: - // val doInitialLoadFromServer = conversationModel.lastMessage?.let { - // newestMessageIdFromDb < it.id - // } ?: true - // Log.d(TAG, "doInitialLoadFromServer:$doInitialLoadFromServer") - // - // if (doInitialLoadFromServer) { + val weAlreadyHaveSomeOfflineMessages = newestMessageIdFromDb > 0 + val weHaveAtLeastTheLastReadMessage = newestMessageIdFromDb >= conversationModel.lastReadMessage.toLong() + Log.d(TAG, "weAlreadyHaveSomeOfflineMessages:$weAlreadyHaveSomeOfflineMessages") + Log.d(TAG, "weHaveAtLeastTheLastReadMessage:$weHaveAtLeastTheLastReadMessage") - if (newestMessageIdFromDb < conversationModel.lastReadMessage.toLong()) { - Log.d(TAG, "An online request is made because chat is not up to date") + if (weAlreadyHaveSomeOfflineMessages && weHaveAtLeastTheLastReadMessage) { + Log.d( + TAG, + "Initial online request is skipped because offline messages are up to date" + + " until lastReadMessage" + ) + Log.d(TAG, "For messages newer than lastRead, lookIntoFuture will load them.") + } else { + if (!weAlreadyHaveSomeOfflineMessages) { + Log.d(TAG, "An online request for newest 100 messages is made because offline chat is empty") + } else { + Log.d( + TAG, + "An online request for newest 100 messages is made because we don't have the lastReadMessage " + + "(gaps could be closed by scrolling up to merge the chatblocks)" + ) + } // set up field map to load the newest messages val fieldMap = getFieldMap( @@ -155,11 +156,7 @@ class OfflineFirstChatRepository @Inject constructor( } newestMessageIdFromDb = chatDao.getNewestMessageId(internalConversationId) - Log.d(TAG, "newestMessageId after sync: $newestMessageIdFromDb") - } else { - Log.d(TAG, "Initial online request is skipped because offline messages are up to date") - // if conversationModel was not up to date and there are new messages, they will just get pulled with - // look into future.. Old messages will be displayed immediately beforehand. + Log.d(TAG, "newestMessageIdFromDb after sync: $newestMessageIdFromDb") } val limit = getCappedMessagesAmountOfChatBlock(newestMessageIdFromDb) @@ -367,7 +364,7 @@ class OfflineFirstChatRepository @Inject constructor( .map(ChatMessageEntity::asModel) } - @Suppress("UNCHECKED_CAST") + @Suppress("UNCHECKED_CAST", "MagicNumber") private fun getMessagesFromServer(bundle: Bundle): Pair>? { val fieldMap = bundle.getSerializable(BundleKeys.KEY_FIELD_MAP) as HashMap From 0dc4572ba37af18a5197eb77cc275dbf8eea59c6 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Mon, 21 Oct 2024 15:33:25 +0200 Subject: [PATCH 9/9] fix to handle chats without offline messages when connection is lost avoid NPE: java.lang.NullPointerException at com.nextcloud.talk.chat.data.network.OfflineFirstChatRepository.getCappedMessagesAmountOfChatBlock(OfflineFirstChatRepository.kt:186) at com.nextcloud.talk.chat.data.network.OfflineFirstChatRepository.access$getCappedMessagesAmountOfChatBlock(OfflineFirstChatRepository.kt:43) at com.nextcloud.talk.chat.data.network.OfflineFirstChatRepository$loadInitialMessages$1.invokeSuspend(OfflineFirstChatRepository.kt:162) Signed-off-by: Marcel Hibbe --- .../network/OfflineFirstChatRepository.kt | 51 +++++++++++-------- 1 file changed, 29 insertions(+), 22 deletions(-) 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 ea40912da..4c52f7f31 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 @@ -159,20 +159,22 @@ class OfflineFirstChatRepository @Inject constructor( Log.d(TAG, "newestMessageIdFromDb after sync: $newestMessageIdFromDb") } - val limit = getCappedMessagesAmountOfChatBlock(newestMessageIdFromDb) + if (newestMessageIdFromDb.toInt() != 0) { + val limit = getCappedMessagesAmountOfChatBlock(newestMessageIdFromDb) - showMessagesBeforeAndEqual( - internalConversationId, - newestMessageIdFromDb, - limit - ) + showMessagesBeforeAndEqual( + internalConversationId, + newestMessageIdFromDb, + limit + ) - // delay is a dirty workaround to make sure messages are added to adapter on initial load before dealing - // with them (otherwise there is a race condition). - delay(DELAY_TO_ENSURE_MESSAGES_ARE_ADDED) + // delay is a dirty workaround to make sure messages are added to adapter on initial load before dealing + // with them (otherwise there is a race condition). + delay(DELAY_TO_ENSURE_MESSAGES_ARE_ADDED) - updateUiForLastCommonRead() - updateUiForLastReadMessage(newestMessageIdFromDb) + updateUiForLastCommonRead() + updateUiForLastReadMessage(newestMessageIdFromDb) + } initMessagePolling(newestMessageIdFromDb) } @@ -180,20 +182,25 @@ class OfflineFirstChatRepository @Inject constructor( private suspend fun getCappedMessagesAmountOfChatBlock(messageId: Long): Int { val chatBlock = getBlockOfMessage(messageId.toInt()) - val amountBetween = chatDao.getCountBetweenMessageIds( - internalConversationId, - messageId, - chatBlock!!.oldestMessageId - ) + if (chatBlock != null) { + val amountBetween = chatDao.getCountBetweenMessageIds( + internalConversationId, + messageId, + chatBlock.oldestMessageId + ) - Log.d(TAG, "amount of messages between newestMessageId and oldest message of same ChatBlock:$amountBetween") - val limit = if (amountBetween > DEFAULT_MESSAGES_LIMIT) { - DEFAULT_MESSAGES_LIMIT + Log.d(TAG, "amount of messages between newestMessageId and oldest message of same ChatBlock:$amountBetween") + val limit = if (amountBetween > DEFAULT_MESSAGES_LIMIT) { + DEFAULT_MESSAGES_LIMIT + } else { + amountBetween + } + Log.d(TAG, "limit of messages to load for UI (max 100 to ensure performance is okay):$limit") + return limit } else { - amountBetween + Log.e(TAG, "No chat block found. Returning 0 as limit.") + return 0 } - Log.d(TAG, "limit of messages to load for UI (max 100 to ensure performance is okay):$limit") - return limit } private suspend fun updateUiForLastReadMessage(newestMessageId: Long) {