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 6679695d31..ee51aaae24 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) @@ -3710,5 +3716,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 } } 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 e7a49ac414..80fd70c846 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,78 @@ 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) + + 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") + } - sync(withNetworkParams) + // 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 + 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") + } + + 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 newestMessageId = chatDao.getNewestMessageId(internalConversationId) - Log.d(TAG, "newestMessageId after sync: $newestMessageId") + val chatBlock = getBlockOfMessage(newestMessageIdFromDb.toInt()) - showLast100MessagesBeforeAndEqual( + val amountBetween = chatDao.getCountBetweenMessageIds( internalConversationId, - chatDao.getNewestMessageId(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") + + showMessagesBeforeAndEqual( + internalConversationId, + newestMessageIdFromDb, + limit ) // delay is a dirty workaround to make sure messages are added to adapter on initial load before dealing @@ -135,7 +187,7 @@ class OfflineFirstChatRepository @Inject constructor( delay(DELAY_TO_ENSURE_MESSAGES_ARE_ADDED) updateUiForLastCommonRead() - updateUiForLastReadMessage(newestMessageId) + updateUiForLastReadMessage(newestMessageIdFromDb) initMessagePolling() } @@ -175,10 +227,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 +258,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 +294,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 +317,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 +333,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 +349,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 +363,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 +436,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 +542,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 +559,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 +633,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 +650,7 @@ class OfflineFirstChatRepository @Inject constructor( val list = getMessagesBeforeAndEqual( messageId, internalConversationId, - 100 + limit ) if (list.isNotEmpty()) { @@ -589,7 +659,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 +676,7 @@ class OfflineFirstChatRepository @Inject constructor( val list = getMessagesBefore( messageId, internalConversationId, - 100 + limit ) if (list.isNotEmpty()) { @@ -638,5 +708,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 } } 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 d0c6659968..39a09bdcb0 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 a264a84eee..92591fe4e1 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 1258ee649e..c9d2762018 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()) {