Skip to content

Commit

Permalink
Decrease message limit for retries of message loading
Browse files Browse the repository at this point in the history 
make it possible to add any amount (up to 100) of messages to UI for initial loading.

add logging

Signed-off-by: Marcel Hibbe <[email protected]>
  • Loading branch information
@mahibi
mahibi committed Oct 8, 2024
1 parent 3451758 commit a89b8a7
Showing 1 changed file with 97 additions and 67 deletions.
164 changes: 97 additions & 67 deletions app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.launch
import java.util.concurrent.TimeUnit
import javax.inject.Inject

class OfflineFirstChatRepository @Inject constructor(
Expand Down Expand Up @@ -111,18 +110,20 @@ class OfflineFirstChatRepository @Inject constructor(
Log.d(TAG, "---- loadInitialMessages ------------")
Log.d(TAG, "conversationModel.internalId: " + conversationModel.internalId)


newXChatLastCommonRead = conversationModel.lastCommonReadMessage

Log.d(TAG, "conversationModel.lastReadMessage:" + conversationModel.lastReadMessage)

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")
}

if (conversationModel.lastReadMessage.toLong() != newestMessageId) {
Log.d(TAG, "An online request is made because chat is not up to date")

// set up field map to load the newest 100 messages
// set up field map to load the newest messages
val fieldMap = getFieldMap(
lookIntoFuture = false,
includeLastKnown = true,
Expand All @@ -132,9 +133,8 @@ class OfflineFirstChatRepository @Inject constructor(
withNetworkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap)
withNetworkParams.putString(BundleKeys.KEY_ROOM_TOKEN, conversationModel.token)

val chatMessageEntities = sync(withNetworkParams) // TODO: if sync fails, newestMessageId remains 0. ->
// needs error handling.
// Continuing with newestMessageId=0 loads the whole chat in longPolling loop
Log.d(TAG, "Starting online request for initial loading")
val chatMessageEntities = sync(withNetworkParams)
if (chatMessageEntities == null) {
Log.e(TAG, "initial loading of messages failed")
}
Expand All @@ -145,9 +145,26 @@ class OfflineFirstChatRepository @Inject constructor(
Log.d(TAG, "Initial online request is skipped because offline messages are up to date")
}

showLast100MessagesBeforeAndEqual(
val chatBlock = getBlockOfMessage(newestMessageId.toInt())

val amountBetween = chatDao.getCountBetweenMessageIds(
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
newestMessageId,
limit
)

// delay is a dirty workaround to make sure messages are added to adapter on initial load before dealing
Expand Down Expand Up @@ -195,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()
}

Expand All @@ -225,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)
Expand Down Expand Up @@ -260,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,
Expand All @@ -283,7 +302,8 @@ class OfflineFirstChatRepository @Inject constructor(
lookIntoFuture: Boolean,
includeLastKnown: Boolean,
setReadMarker: Boolean,
lastKnown: Int?
lastKnown: Int?,
limit: Int = DEFAULT_MESSAGES_LIMIT
): HashMap<String, Int> {
val fieldMap = HashMap<String, Int>()

Expand All @@ -298,8 +318,7 @@ class OfflineFirstChatRepository @Inject constructor(
}

fieldMap["timeout"] = if (lookIntoFuture) 30 else 0
fieldMap["limit"] = 100
// fieldMap["limit"] = 5
fieldMap["limit"] = limit
fieldMap["lookIntoFuture"] = if (lookIntoFuture) 1 else 0
fieldMap["setReadMarker"] = if (setReadMarker) 1 else 0

Expand All @@ -315,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)
Expand All @@ -329,59 +348,70 @@ class OfflineFirstChatRepository @Inject constructor(

@Suppress("UNCHECKED_CAST")
private fun getMessagesFromServer(bundle: Bundle): Pair<Int, List<ChatMessageJson>>? {
Log.d(TAG, "An online request is made!!!!!!!!!!!!!!!!!!!!")
val fieldMap = bundle.getSerializable(BundleKeys.KEY_FIELD_MAP) as HashMap<String, Int>

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<ChatMessageJson>()
)
}
return@map Pair(
HTTP_CODE_NOT_MODIFIED,
listOf<ChatMessageJson>()
)
}

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<ChatMessageJson>()
)
}
return@map Pair(
HTTP_CODE_PRECONDITION_FAILED,
listOf<ChatMessageJson>()
)
}

else -> {
return@map Pair(
HTTP_CODE_PRECONDITION_FAILED,
listOf<ChatMessageJson>()
)
else -> {
return@map Pair(
HTTP_CODE_PRECONDITION_FAILED,
listOf<ChatMessageJson>()
)
}
}
}
.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
}

Expand Down Expand Up @@ -497,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.
Expand All @@ -514,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
Expand Down Expand Up @@ -589,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,
Expand All @@ -606,7 +635,7 @@ class OfflineFirstChatRepository @Inject constructor(
val list = getMessagesBeforeAndEqual(
messageId,
internalConversationId,
100
limit
)

if (list.isNotEmpty()) {
Expand All @@ -615,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,
Expand All @@ -632,7 +661,7 @@ class OfflineFirstChatRepository @Inject constructor(
val list = getMessagesBefore(
messageId,
internalConversationId,
100
limit
)

if (list.isNotEmpty()) {
Expand Down Expand Up @@ -664,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
}
}

0 comments on commit a89b8a7

Please sign in to comment.