From 3d6674e35a95d3c65b9ca51243fda85cec7b56b7 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Mon, 6 Mar 2023 17:14:43 +0100 Subject: [PATCH 01/10] Make lookIntoFuture parameter boolean Signed-off-by: Marcel Hibbe --- .../talk/controllers/ChatController.kt | 35 +++++++++++-------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt index e2c71673b..2f58971e3 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt @@ -1414,10 +1414,10 @@ class ChatController(args: Bundle) : binding?.messageInputView?.inputEditText?.visibility = View.VISIBLE if (isFirstMessagesProcessing && pastPreconditionFailed) { pastPreconditionFailed = false - pullChatMessages(0) + pullChatMessages(false) } else if (futurePreconditionFailed) { futurePreconditionFailed = false - pullChatMessages(1) + pullChatMessages(true) } } } else { @@ -2014,9 +2014,9 @@ class ChatController(args: Bundle) : webSocketInstance?.getSignalingMessageReceiver()?.addListener(localParticipantMessageListener) if (isFirstMessagesProcessing) { - pullChatMessages(0) + pullChatMessages(false) } else { - pullChatMessages(1, 0) + pullChatMessages(true, 0) } if (webSocketInstance != null) { @@ -2054,9 +2054,9 @@ class ChatController(args: Bundle) : } if (isFirstMessagesProcessing) { - pullChatMessages(0) + pullChatMessages(false) } else { - pullChatMessages(1) + pullChatMessages(true) } } } @@ -2234,7 +2234,7 @@ class ChatController(args: Bundle) : } } - fun pullChatMessages(lookIntoFuture: Int, setReadMarker: Int = 1, xChatLastCommonRead: Int? = null) { + fun pullChatMessages(lookIntoFuture: Boolean, setReadMarker: Int = 1, xChatLastCommonRead: Int? = null) { if (!validSessionId()) { return } @@ -2252,7 +2252,7 @@ class ChatController(args: Bundle) : val fieldMap = HashMap() fieldMap["includeLastKnown"] = 0 - if (lookIntoFuture > 0) { + if (lookIntoFuture) { lookingIntoFuture = true } else if (isFirstMessagesProcessing) { if (currentConversation != null) { @@ -2270,11 +2270,16 @@ class ChatController(args: Bundle) : fieldMap["timeout"] = timeout - fieldMap["lookIntoFuture"] = lookIntoFuture + if (lookIntoFuture) { + fieldMap["lookIntoFuture"] = 1 + } else { + fieldMap["lookIntoFuture"] = 0 + } + fieldMap["limit"] = MESSAGE_PULL_LIMIT fieldMap["setReadMarker"] = setReadMarker - val lastKnown = if (lookIntoFuture > 0) { + val lastKnown = if (lookIntoFuture) { globalLastKnownFutureMessageId } else { globalLastKnownPastMessageId @@ -2291,7 +2296,7 @@ class ChatController(args: Bundle) : apiVersion = ApiUtils.getChatApiVersion(conversationUser, intArrayOf(1)) } - if (lookIntoFuture > 0) { + if (lookIntoFuture) { Log.d(TAG, "pullChatMessages - pullChatMessages[lookIntoFuture > 0] - calling") ncApi.pullChatMessages( credentials, @@ -2311,7 +2316,7 @@ class ChatController(args: Bundle) : pullChatMessagesPending = false if (response.code() == HTTP_CODE_NOT_MODIFIED) { Log.d(TAG, "pullChatMessages - quasi recursive call to pullChatMessages") - pullChatMessages(1, setReadMarker, xChatLastCommonRead) + pullChatMessages(true, setReadMarker, xChatLastCommonRead) } else if (response.code() == HTTP_CODE_PRECONDITION_FAILED) { futurePreconditionFailed = true } else { @@ -2416,7 +2421,7 @@ class ChatController(args: Bundle) : historyRead = true if (!lookingIntoFuture && validSessionId()) { - pullChatMessages(1) + pullChatMessages(true) } } } @@ -2452,7 +2457,7 @@ class ChatController(args: Bundle) : adapter?.notifyDataSetChanged() if (validSessionId()) { - pullChatMessages(1, 1, xChatLastCommonRead) + pullChatMessages(true, 1, xChatLastCommonRead) } } @@ -2645,7 +2650,7 @@ class ChatController(args: Bundle) : override fun onLoadMore(page: Int, totalItemsCount: Int) { if (!historyRead && validSessionId()) { - pullChatMessages(0) + pullChatMessages(false) } } From 21606bc9c1c9b7579042617b35d0f15b5ad685c1 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Mon, 6 Mar 2023 17:19:34 +0100 Subject: [PATCH 02/10] Make setReadMarker parameter boolean Signed-off-by: Marcel Hibbe --- .../nextcloud/talk/controllers/ChatController.kt | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt index 2f58971e3..61d9f9784 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt @@ -2016,7 +2016,7 @@ class ChatController(args: Bundle) : if (isFirstMessagesProcessing) { pullChatMessages(false) } else { - pullChatMessages(true, 0) + pullChatMessages(true, false) } if (webSocketInstance != null) { @@ -2234,7 +2234,7 @@ class ChatController(args: Bundle) : } } - fun pullChatMessages(lookIntoFuture: Boolean, setReadMarker: Int = 1, xChatLastCommonRead: Int? = null) { + fun pullChatMessages(lookIntoFuture: Boolean, setReadMarker: Boolean = true, xChatLastCommonRead: Int? = null) { if (!validSessionId()) { return } @@ -2277,7 +2277,12 @@ class ChatController(args: Bundle) : } fieldMap["limit"] = MESSAGE_PULL_LIMIT - fieldMap["setReadMarker"] = setReadMarker + + if (setReadMarker) { + fieldMap["setReadMarker"] = 1 + } else { + fieldMap["setReadMarker"] = 0 + } val lastKnown = if (lookIntoFuture) { globalLastKnownFutureMessageId @@ -2457,7 +2462,7 @@ class ChatController(args: Bundle) : adapter?.notifyDataSetChanged() if (validSessionId()) { - pullChatMessages(true, 1, xChatLastCommonRead) + pullChatMessages(true, true, xChatLastCommonRead) } } From 7065b18d07fe4c16fecc96cb1d3dcf72721977eb Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Mon, 6 Mar 2023 17:34:28 +0100 Subject: [PATCH 03/10] Refactor pullChatMessages request Signed-off-by: Marcel Hibbe --- .../talk/controllers/ChatController.kt | 93 +++++++------------ 1 file changed, 33 insertions(+), 60 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt index 61d9f9784..7f3de2a7d 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt @@ -2234,7 +2234,11 @@ class ChatController(args: Bundle) : } } - fun pullChatMessages(lookIntoFuture: Boolean, setReadMarker: Boolean = true, xChatLastCommonRead: Int? = null) { + fun pullChatMessages( + lookIntoFuture: Boolean, + setReadMarker: Boolean = true, + xChatLastCommonRead: Int? = null + ) { if (!validSessionId()) { return } @@ -2301,24 +2305,23 @@ class ChatController(args: Bundle) : apiVersion = ApiUtils.getChatApiVersion(conversationUser, intArrayOf(1)) } - if (lookIntoFuture) { - Log.d(TAG, "pullChatMessages - pullChatMessages[lookIntoFuture > 0] - calling") - ncApi.pullChatMessages( - credentials, - ApiUtils.getUrlForChat(apiVersion, conversationUser?.baseUrl, roomToken), - fieldMap - ) - ?.subscribeOn(Schedulers.io()) - ?.observeOn(AndroidSchedulers.mainThread()) - ?.subscribe(object : Observer> { - override fun onSubscribe(d: Disposable) { - disposables.add(d) - } + ncApi.pullChatMessages( + credentials, + ApiUtils.getUrlForChat(apiVersion, conversationUser?.baseUrl, roomToken), + fieldMap + ) + ?.subscribeOn(Schedulers.io()) + ?.observeOn(AndroidSchedulers.mainThread()) + ?.subscribe(object : Observer> { + override fun onSubscribe(d: Disposable) { + disposables.add(d) + } - @Suppress("Detekt.TooGenericExceptionCaught") - override fun onNext(response: Response<*>) { - Log.d(TAG, "pullChatMessages - pullChatMessages[lookIntoFuture > 0] - got response") - pullChatMessagesPending = false + @Suppress("Detekt.TooGenericExceptionCaught") + override fun onNext(response: Response<*>) { + pullChatMessagesPending = false + + if (lookIntoFuture) { if (response.code() == HTTP_CODE_NOT_MODIFIED) { Log.d(TAG, "pullChatMessages - quasi recursive call to pullChatMessages") pullChatMessages(true, setReadMarker, xChatLastCommonRead) @@ -2327,56 +2330,26 @@ class ChatController(args: Bundle) : } else { processMessagesResponse(response, true) } - - processExpiredMessages() - } - - override fun onError(e: Throwable) { - Log.e(TAG, "pullChatMessages - pullChatMessages[lookIntoFuture > 0] - ERROR", e) - pullChatMessagesPending = false - } - - override fun onComplete() { - pullChatMessagesPending = false - } - }) - } else { - Log.d(TAG, "pullChatMessages - pullChatMessages[lookIntoFuture <= 0] - calling") - ncApi.pullChatMessages( - credentials, - ApiUtils.getUrlForChat(apiVersion, conversationUser?.baseUrl, roomToken), - fieldMap - ) - ?.subscribeOn(Schedulers.io()) - ?.observeOn(AndroidSchedulers.mainThread()) - ?.subscribe(object : Observer> { - override fun onSubscribe(d: Disposable) { - disposables.add(d) - } - - @Suppress("Detekt.TooGenericExceptionCaught") - override fun onNext(response: Response<*>) { - Log.d(TAG, "pullChatMessages - pullChatMessages[lookIntoFuture <= 0] - got response") - pullChatMessagesPending = false + } else { if (response.code() == HTTP_CODE_PRECONDITION_FAILED) { pastPreconditionFailed = true } else { processMessagesResponse(response, false) } - - processExpiredMessages() } - override fun onError(e: Throwable) { - Log.e(TAG, "pullChatMessages - pullChatMessages[lookIntoFuture <= 0] - ERROR", e) - pullChatMessagesPending = false - } + processExpiredMessages() + } - override fun onComplete() { - pullChatMessagesPending = false - } - }) - } + override fun onError(e: Throwable) { + Log.e(TAG, "pullChatMessages - pullChatMessages ERROR", e) + pullChatMessagesPending = false + } + + override fun onComplete() { + pullChatMessagesPending = false + } + }) } private fun processExpiredMessages() { From 6f4a24b28f48b5daafbbe5f3331c55f620feaee0 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Mon, 6 Mar 2023 17:43:22 +0100 Subject: [PATCH 04/10] Refactor (delete lookingIntoFuture) Signed-off-by: Marcel Hibbe --- .../talk/controllers/ChatController.kt | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt index 7f3de2a7d..10e2d09dd 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt @@ -264,7 +264,6 @@ class ChatController(args: Bundle) : private var mentionAutocomplete: Autocomplete<*>? = null var layoutManager: LinearLayoutManager? = null var pullChatMessagesPending = false - private var lookingIntoFuture = false var newMessagesCount = 0 var startCallFromNotification: Boolean? = null var startCallFromRoomSwitch: Boolean = false @@ -2256,9 +2255,7 @@ class ChatController(args: Bundle) : val fieldMap = HashMap() fieldMap["includeLastKnown"] = 0 - if (lookIntoFuture) { - lookingIntoFuture = true - } else if (isFirstMessagesProcessing) { + if (!lookIntoFuture && isFirstMessagesProcessing) { if (currentConversation != null) { globalLastKnownFutureMessageId = currentConversation!!.lastReadMessage globalLastKnownPastMessageId = currentConversation!!.lastReadMessage @@ -2266,13 +2263,14 @@ class ChatController(args: Bundle) : } } - val timeout = if (lookingIntoFuture) { + val timeout = if (lookIntoFuture) { LOOKING_INTO_FUTURE_TIMEOUT } else { 0 } fieldMap["timeout"] = timeout + fieldMap["limit"] = MESSAGE_PULL_LIMIT if (lookIntoFuture) { fieldMap["lookIntoFuture"] = 1 @@ -2280,8 +2278,6 @@ class ChatController(args: Bundle) : fieldMap["lookIntoFuture"] = 0 } - fieldMap["limit"] = MESSAGE_PULL_LIMIT - if (setReadMarker) { fieldMap["setReadMarker"] = 1 } else { @@ -2376,7 +2372,10 @@ class ChatController(args: Bundle) : } } - private fun processMessagesResponse(response: Response<*>, isFromTheFuture: Boolean) { + private fun processMessagesResponse( + response: Response<*>, + isFromTheFuture: Boolean + ) { val xChatLastCommonRead = response.headers()["X-Chat-Last-Common-Read"]?.let { Integer.parseInt(it) } @@ -2398,9 +2397,7 @@ class ChatController(args: Bundle) : historyRead = true - if (!lookingIntoFuture && validSessionId()) { - pullChatMessages(true) - } + pullChatMessages(true) } } From 7f77cf8c8eddd0718e1d029295d56358eaa5ccaa Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 7 Mar 2023 12:01:43 +0100 Subject: [PATCH 05/10] Refactor pullChatMessages Signed-off-by: Marcel Hibbe --- .../talk/controllers/ChatController.kt | 138 ++++++++---------- 1 file changed, 60 insertions(+), 78 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt index 10e2d09dd..4b48de3e3 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt @@ -2313,24 +2313,70 @@ class ChatController(args: Bundle) : disposables.add(d) } + @SuppressLint("NotifyDataSetChanged") @Suppress("Detekt.TooGenericExceptionCaught") override fun onNext(response: Response<*>) { pullChatMessagesPending = false - if (lookIntoFuture) { - if (response.code() == HTTP_CODE_NOT_MODIFIED) { - Log.d(TAG, "pullChatMessages - quasi recursive call to pullChatMessages") - pullChatMessages(true, setReadMarker, xChatLastCommonRead) - } else if (response.code() == HTTP_CODE_PRECONDITION_FAILED) { - futurePreconditionFailed = true - } else { - processMessagesResponse(response, true) + if (isFirstMessagesProcessing) { + cancelNotificationsForCurrentConversation() + + isFirstMessagesProcessing = false + binding?.progressBar?.visibility = View.GONE + + binding?.messagesListView?.visibility = View.VISIBLE + } + + when (response.code()) { + HTTP_CODE_NOT_MODIFIED -> { + Log.d(TAG, "pullChatMessages - HTTP_CODE_NOT_MODIFIED") + + if (lookIntoFuture) { + Log.d(TAG, "recursive call to pullChatMessages") + pullChatMessages(true, setReadMarker, xChatLastCommonRead) + } else { + historyRead = true + pullChatMessages(true) + } } - } else { - if (response.code() == HTTP_CODE_PRECONDITION_FAILED) { - pastPreconditionFailed = true - } else { - processMessagesResponse(response, false) + HTTP_CODE_PRECONDITION_FAILED -> { + Log.d(TAG, "pullChatMessages - HTTP_CODE_PRECONDITION_FAILED") + + if (lookIntoFuture) { + futurePreconditionFailed = true + } else { + pastPreconditionFailed = true + } + } + HTTP_CODE_OK -> { + Log.d(TAG, "pullChatMessages - HTTP_CODE_OK") + + val chatOverall = response.body() as ChatOverall? + val chatMessageList = handleSystemMessages(chatOverall?.ocs!!.data!!) + + processHeaderChatLastGiven(response, lookIntoFuture) + + if (chatMessageList.isNotEmpty() && + ChatMessage.SystemMessageType.CLEARED_CHAT == chatMessageList[0].systemMessageType + ) { + adapter?.clear() + adapter?.notifyDataSetChanged() + } + + if (lookIntoFuture) { + processMessagesFromTheFuture(chatMessageList) + } else { + processMessagesNotFromTheFuture(chatMessageList) + } + + val xChatLastCommonRead = response.headers()["X-Chat-Last-Common-Read"]?.let { + Integer.parseInt(it) + } + + updateReadStatusOfAllMessages(xChatLastCommonRead) + adapter?.notifyDataSetChanged() + + pullChatMessages(true, true, xChatLastCommonRead) } } @@ -2372,70 +2418,6 @@ class ChatController(args: Bundle) : } } - private fun processMessagesResponse( - response: Response<*>, - isFromTheFuture: Boolean - ) { - val xChatLastCommonRead = response.headers()["X-Chat-Last-Common-Read"]?.let { - Integer.parseInt(it) - } - - processHeaderChatLastGiven(response, isFromTheFuture) - - if (response.code() == HTTP_CODE_OK) { - val chatOverall = response.body() as ChatOverall? - val chatMessageList = handleSystemMessages(chatOverall?.ocs!!.data!!) - - processMessages(chatMessageList, isFromTheFuture, xChatLastCommonRead) - } else if (response.code() == HTTP_CODE_NOT_MODIFIED && !isFromTheFuture) { - if (isFirstMessagesProcessing) { - cancelNotificationsForCurrentConversation() - - isFirstMessagesProcessing = false - binding?.progressBar?.visibility = View.GONE - } - - historyRead = true - - pullChatMessages(true) - } - } - - private fun processMessages( - chatMessageList: List, - isFromTheFuture: Boolean, - xChatLastCommonRead: Int? - ) { - if (chatMessageList.isNotEmpty() && - ChatMessage.SystemMessageType.CLEARED_CHAT == chatMessageList[0].systemMessageType - ) { - adapter?.clear() - adapter?.notifyDataSetChanged() - } - - if (isFirstMessagesProcessing) { - cancelNotificationsForCurrentConversation() - - isFirstMessagesProcessing = false - binding?.progressBar?.visibility = View.GONE - - binding?.messagesListView?.visibility = View.VISIBLE - } - - if (isFromTheFuture) { - processMessagesFromTheFuture(chatMessageList) - } else { - processMessagesNotFromTheFuture(chatMessageList) - } - - updateReadStatusOfAllMessages(xChatLastCommonRead) - adapter?.notifyDataSetChanged() - - if (validSessionId()) { - pullChatMessages(true, true, xChatLastCommonRead) - } - } - private fun updateReadStatusOfAllMessages(xChatLastCommonRead: Int?) { if (adapter != null) { for (message in adapter!!.items) { @@ -2624,7 +2606,7 @@ class ChatController(args: Bundle) : } override fun onLoadMore(page: Int, totalItemsCount: Int) { - if (!historyRead && validSessionId()) { + if (!historyRead) { pullChatMessages(false) } } From 67957996b01c7d734c5b24b71af72e6061788831 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 7 Mar 2023 14:54:55 +0100 Subject: [PATCH 06/10] Fix duplicated messages after history loading The solution was to avoid recursive call off pullChatMessages if lookIntoFuture is false. However the recursive call has to be made when fetching messages for the first time: if (isFirstMessagesProcessing || lookIntoFuture) { pullChatMessages(true,...) } For this, setting of isFirstMessagesProcessing had to be moved below this code. Furthermore in the commit: add logging & refactoring Signed-off-by: Marcel Hibbe --- .../talk/controllers/ChatController.kt | 62 +++++++++++-------- 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt index 4b48de3e3..0ccc90639 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt @@ -2242,6 +2242,8 @@ class ChatController(args: Bundle) : return } + Log.d(TAG, "pullChatMessages. lookIntoFuture= $lookIntoFuture") + if (pullChatMessagesPending) { // Sometimes pullChatMessages may be called before response to a previous call is received. // In such cases just ignore the second call. Message processing will continue when response to the @@ -2250,7 +2252,9 @@ class ChatController(args: Bundle) : Log.d(TAG, "pullChatMessages - pullChatMessagesPending is true, exiting") return } + pullChatMessagesPending = true + Log.d(TAG, "pullChatMessagesPending was set to true") val fieldMap = HashMap() fieldMap["includeLastKnown"] = 0 @@ -2263,6 +2267,17 @@ class ChatController(args: Bundle) : } } + val lastKnown = if (lookIntoFuture) { + globalLastKnownFutureMessageId + } else { + globalLastKnownPastMessageId + } + + fieldMap["lastKnownMessageId"] = lastKnown + xChatLastCommonRead?.let { + fieldMap["lastCommonReadId"] = it + } + val timeout = if (lookIntoFuture) { LOOKING_INTO_FUTURE_TIMEOUT } else { @@ -2284,17 +2299,6 @@ class ChatController(args: Bundle) : fieldMap["setReadMarker"] = 0 } - val lastKnown = if (lookIntoFuture) { - globalLastKnownFutureMessageId - } else { - globalLastKnownPastMessageId - } - - fieldMap["lastKnownMessageId"] = lastKnown - xChatLastCommonRead?.let { - fieldMap["lastCommonReadId"] = it - } - var apiVersion = 1 // FIXME this is a best guess, guests would need to get the capabilities themselves if (conversationUser != null) { @@ -2316,31 +2320,28 @@ class ChatController(args: Bundle) : @SuppressLint("NotifyDataSetChanged") @Suppress("Detekt.TooGenericExceptionCaught") override fun onNext(response: Response<*>) { + // when fetching history in between the regular polling, this is set to false!! -> bug? pullChatMessagesPending = false - - if (isFirstMessagesProcessing) { - cancelNotificationsForCurrentConversation() - - isFirstMessagesProcessing = false - binding?.progressBar?.visibility = View.GONE - - binding?.messagesListView?.visibility = View.VISIBLE - } + Log.d(TAG, "pullChatMessagesPending was set to false") when (response.code()) { HTTP_CODE_NOT_MODIFIED -> { - Log.d(TAG, "pullChatMessages - HTTP_CODE_NOT_MODIFIED") + Log.d(TAG, "pullChatMessages - HTTP_CODE_NOT_MODIFIED. lookIntoFuture=$lookIntoFuture") if (lookIntoFuture) { - Log.d(TAG, "recursive call to pullChatMessages") + Log.d(TAG, "recursive call to pullChatMessages. lookIntoFuture=$lookIntoFuture") pullChatMessages(true, setReadMarker, xChatLastCommonRead) } else { + Log.d(TAG, "when is this reached?") historyRead = true pullChatMessages(true) } } HTTP_CODE_PRECONDITION_FAILED -> { - Log.d(TAG, "pullChatMessages - HTTP_CODE_PRECONDITION_FAILED") + Log.d( + TAG, + "pullChatMessages - HTTP_CODE_PRECONDITION_FAILED. lookIntoFuture=$lookIntoFuture" + ) if (lookIntoFuture) { futurePreconditionFailed = true @@ -2349,7 +2350,7 @@ class ChatController(args: Bundle) : } } HTTP_CODE_OK -> { - Log.d(TAG, "pullChatMessages - HTTP_CODE_OK") + Log.d(TAG, "pullChatMessages - HTTP_CODE_OK. lookIntoFuture=$lookIntoFuture") val chatOverall = response.body() as ChatOverall? val chatMessageList = handleSystemMessages(chatOverall?.ocs!!.data!!) @@ -2376,11 +2377,21 @@ class ChatController(args: Bundle) : updateReadStatusOfAllMessages(xChatLastCommonRead) adapter?.notifyDataSetChanged() - pullChatMessages(true, true, xChatLastCommonRead) + if (isFirstMessagesProcessing || lookIntoFuture) { + Log.d(TAG, "recursive call to pullChatMessages. lookIntoFuture=$lookIntoFuture") + pullChatMessages(true, true, xChatLastCommonRead) + } } } processExpiredMessages() + + if (isFirstMessagesProcessing) { + cancelNotificationsForCurrentConversation() + isFirstMessagesProcessing = false + binding?.progressBar?.visibility = View.GONE + binding?.messagesListView?.visibility = View.VISIBLE + } } override fun onError(e: Throwable) { @@ -2607,6 +2618,7 @@ class ChatController(args: Bundle) : override fun onLoadMore(page: Int, totalItemsCount: Int) { if (!historyRead) { + Log.d(TAG, "requested onLoadMore to pullChatMessages with lookIntoFuture=false") pullChatMessages(false) } } From 82abb9d9bdb8808ed33784a64f998f04c31d1c02 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Tue, 7 Mar 2023 15:49:23 +0100 Subject: [PATCH 07/10] Fix to avoid duplicated messages for HTTP_CODE_NOT_MODIFIED the "historyRead" didn't make any sense to me, and deleting it solved the issue to avoid duplicated messages when the response was HTTP_CODE_NOT_MODIFIED Signed-off-by: Marcel Hibbe --- .../talk/controllers/ChatController.kt | 25 ++++++------------- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt index 0ccc90639..67c03be64 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt @@ -257,7 +257,6 @@ class ChatController(args: Bundle) : private val roomPassword: String var credentials: String? = null var currentConversation: Conversation? = null - private var historyRead = false private var globalLastKnownFutureMessageId = -1 private var globalLastKnownPastMessageId = -1 var adapter: TalkMessagesListAdapter? = null @@ -2320,28 +2319,20 @@ class ChatController(args: Bundle) : @SuppressLint("NotifyDataSetChanged") @Suppress("Detekt.TooGenericExceptionCaught") override fun onNext(response: Response<*>) { - // when fetching history in between the regular polling, this is set to false!! -> bug? pullChatMessagesPending = false Log.d(TAG, "pullChatMessagesPending was set to false") when (response.code()) { HTTP_CODE_NOT_MODIFIED -> { - Log.d(TAG, "pullChatMessages - HTTP_CODE_NOT_MODIFIED. lookIntoFuture=$lookIntoFuture") + Log.d(TAG, "pullChatMessages - HTTP_CODE_NOT_MODIFIED.") if (lookIntoFuture) { - Log.d(TAG, "recursive call to pullChatMessages. lookIntoFuture=$lookIntoFuture") + Log.d(TAG, "recursive call to pullChatMessages.") pullChatMessages(true, setReadMarker, xChatLastCommonRead) - } else { - Log.d(TAG, "when is this reached?") - historyRead = true - pullChatMessages(true) } } HTTP_CODE_PRECONDITION_FAILED -> { - Log.d( - TAG, - "pullChatMessages - HTTP_CODE_PRECONDITION_FAILED. lookIntoFuture=$lookIntoFuture" - ) + Log.d(TAG, "pullChatMessages - HTTP_CODE_PRECONDITION_FAILED.") if (lookIntoFuture) { futurePreconditionFailed = true @@ -2350,7 +2341,7 @@ class ChatController(args: Bundle) : } } HTTP_CODE_OK -> { - Log.d(TAG, "pullChatMessages - HTTP_CODE_OK. lookIntoFuture=$lookIntoFuture") + Log.d(TAG, "pullChatMessages - HTTP_CODE_OK.") val chatOverall = response.body() as ChatOverall? val chatMessageList = handleSystemMessages(chatOverall?.ocs!!.data!!) @@ -2378,7 +2369,7 @@ class ChatController(args: Bundle) : adapter?.notifyDataSetChanged() if (isFirstMessagesProcessing || lookIntoFuture) { - Log.d(TAG, "recursive call to pullChatMessages. lookIntoFuture=$lookIntoFuture") + Log.d(TAG, "recursive call to pullChatMessages") pullChatMessages(true, true, xChatLastCommonRead) } } @@ -2617,10 +2608,8 @@ class ChatController(args: Bundle) : } override fun onLoadMore(page: Int, totalItemsCount: Int) { - if (!historyRead) { - Log.d(TAG, "requested onLoadMore to pullChatMessages with lookIntoFuture=false") - pullChatMessages(false) - } + Log.d(TAG, "requested onLoadMore to pullChatMessages with lookIntoFuture=false") + pullChatMessages(false) } override fun format(date: Date): String { From 2a4ac69d894d95871429fc74268269e5989d9da5 Mon Sep 17 00:00:00 2001 From: github-actions Date: Tue, 7 Mar 2023 15:01:41 +0000 Subject: [PATCH 08/10] Analysis: update lint results to reflect reduced error/warning count Signed-off-by: github-actions --- scripts/analysis/lint-results.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/analysis/lint-results.txt b/scripts/analysis/lint-results.txt index 034225a6a..3694e6bcc 100644 --- a/scripts/analysis/lint-results.txt +++ b/scripts/analysis/lint-results.txt @@ -1,2 +1,2 @@ DO NOT TOUCH; GENERATED BY DRONE - Lint Report: 110 warnings + Lint Report: 108 warnings From 6efbbae8239de36f5885d41054c571179077eb24 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 9 Mar 2023 14:01:34 +0100 Subject: [PATCH 09/10] Extract creation of fieldMap for pullChatMessages Signed-off-by: Marcel Hibbe --- .../talk/controllers/ChatController.kt | 103 ++++++++++-------- 1 file changed, 56 insertions(+), 47 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt index 67c03be64..2c03e08cb 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt @@ -2251,52 +2251,13 @@ class ChatController(args: Bundle) : Log.d(TAG, "pullChatMessages - pullChatMessagesPending is true, exiting") return } - pullChatMessagesPending = true - Log.d(TAG, "pullChatMessagesPending was set to true") - val fieldMap = HashMap() - fieldMap["includeLastKnown"] = 0 - - if (!lookIntoFuture && isFirstMessagesProcessing) { - if (currentConversation != null) { - globalLastKnownFutureMessageId = currentConversation!!.lastReadMessage - globalLastKnownPastMessageId = currentConversation!!.lastReadMessage - fieldMap["includeLastKnown"] = 1 - } - } - - val lastKnown = if (lookIntoFuture) { - globalLastKnownFutureMessageId - } else { - globalLastKnownPastMessageId - } - - fieldMap["lastKnownMessageId"] = lastKnown - xChatLastCommonRead?.let { - fieldMap["lastCommonReadId"] = it - } - - val timeout = if (lookIntoFuture) { - LOOKING_INTO_FUTURE_TIMEOUT - } else { - 0 - } - - fieldMap["timeout"] = timeout - fieldMap["limit"] = MESSAGE_PULL_LIMIT - - if (lookIntoFuture) { - fieldMap["lookIntoFuture"] = 1 - } else { - fieldMap["lookIntoFuture"] = 0 - } - - if (setReadMarker) { - fieldMap["setReadMarker"] = 1 - } else { - fieldMap["setReadMarker"] = 0 - } + val pullChatMessagesFieldMap = setupFieldsForPullChatMessages( + lookIntoFuture, + xChatLastCommonRead, + setReadMarker + ) var apiVersion = 1 // FIXME this is a best guess, guests would need to get the capabilities themselves @@ -2307,7 +2268,7 @@ class ChatController(args: Bundle) : ncApi.pullChatMessages( credentials, ApiUtils.getUrlForChat(apiVersion, conversationUser?.baseUrl, roomToken), - fieldMap + pullChatMessagesFieldMap ) ?.subscribeOn(Schedulers.io()) ?.observeOn(AndroidSchedulers.mainThread()) @@ -2320,7 +2281,6 @@ class ChatController(args: Bundle) : @Suppress("Detekt.TooGenericExceptionCaught") override fun onNext(response: Response<*>) { pullChatMessagesPending = false - Log.d(TAG, "pullChatMessagesPending was set to false") when (response.code()) { HTTP_CODE_NOT_MODIFIED -> { @@ -2396,6 +2356,56 @@ class ChatController(args: Bundle) : }) } + private fun setupFieldsForPullChatMessages( + lookIntoFuture: Boolean, + xChatLastCommonRead: Int?, + setReadMarker: Boolean + ): HashMap { + val fieldMap = HashMap() + fieldMap["includeLastKnown"] = 0 + + if (!lookIntoFuture && isFirstMessagesProcessing) { + if (currentConversation != null) { + globalLastKnownFutureMessageId = currentConversation!!.lastReadMessage + globalLastKnownPastMessageId = currentConversation!!.lastReadMessage + fieldMap["includeLastKnown"] = 1 + } + } + + val lastKnown = if (lookIntoFuture) { + globalLastKnownFutureMessageId + } else { + globalLastKnownPastMessageId + } + + fieldMap["lastKnownMessageId"] = lastKnown + xChatLastCommonRead?.let { + fieldMap["lastCommonReadId"] = it + } + + val timeout = if (lookIntoFuture) { + LOOKING_INTO_FUTURE_TIMEOUT + } else { + 0 + } + + fieldMap["timeout"] = timeout + fieldMap["limit"] = MESSAGE_PULL_LIMIT + + if (lookIntoFuture) { + fieldMap["lookIntoFuture"] = 1 + } else { + fieldMap["lookIntoFuture"] = 0 + } + + if (setReadMarker) { + fieldMap["setReadMarker"] = 1 + } else { + fieldMap["setReadMarker"] = 0 + } + return fieldMap + } + private fun processExpiredMessages() { fun deleteExpiredMessages() { val messagesToDelete: ArrayList = ArrayList() @@ -2608,7 +2618,6 @@ class ChatController(args: Bundle) : } override fun onLoadMore(page: Int, totalItemsCount: Int) { - Log.d(TAG, "requested onLoadMore to pullChatMessages with lookIntoFuture=false") pullChatMessages(false) } From b10558eee8cc601a7761dc324d1b56c2acb08b76 Mon Sep 17 00:00:00 2001 From: Marcel Hibbe Date: Thu, 9 Mar 2023 14:05:54 +0100 Subject: [PATCH 10/10] Avoid shadowed warning for xChatLastCommonRead Signed-off-by: Marcel Hibbe --- .../java/com/nextcloud/talk/controllers/ChatController.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt index 2c03e08cb..4d0a7b9b4 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ChatController.kt @@ -2321,16 +2321,16 @@ class ChatController(args: Bundle) : processMessagesNotFromTheFuture(chatMessageList) } - val xChatLastCommonRead = response.headers()["X-Chat-Last-Common-Read"]?.let { + val newXChatLastCommonRead = response.headers()["X-Chat-Last-Common-Read"]?.let { Integer.parseInt(it) } - updateReadStatusOfAllMessages(xChatLastCommonRead) + updateReadStatusOfAllMessages(newXChatLastCommonRead) adapter?.notifyDataSetChanged() if (isFirstMessagesProcessing || lookIntoFuture) { Log.d(TAG, "recursive call to pullChatMessages") - pullChatMessages(true, true, xChatLastCommonRead) + pullChatMessages(true, true, newXChatLastCommonRead) } } }