Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feature/4299/improve offline support #4300

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
+165 −85
Draft

Feature/4299/improve offline support #4300

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1b7c01e
only make initial request for chat messages when newest message from …
mahibi Oct 1, 2024
0af8190
load conversation from DB first, then update by request if connection…
mahibi Oct 1, 2024
fb76667
delay progress bar
mahibi Oct 2, 2024
4a41b13
add todo for error handling
mahibi Oct 2, 2024
3451758
check if sync succeeded
mahibi Oct 7, 2024
a89b8a7
Decrease message limit for retries of message loading
mahibi Oct 8, 2024
1243523
only do initial request if newestMessageIdFromDb is lower than lastRe…
mahibi Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
}
}
217 changes: 144 additions & 73 deletions app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,34 +108,86 @@ 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
// with them (otherwise there is a race condition).
delay(DELAY_TO_ENSURE_MESSAGES_ARE_ADDED)

updateUiForLastCommonRead()
updateUiForLastReadMessage(newestMessageId)
updateUiForLastReadMessage(newestMessageIdFromDb)

initMessagePolling()
}
Expand Down Expand Up @@ -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()
}

Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -263,7 +317,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 @@ -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

Expand All @@ -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)
Expand All @@ -308,59 +363,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 All @@ -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<ChatMessageEntity>? = null

val fieldMap = bundle.getSerializable(BundleKeys.KEY_FIELD_MAP) as HashMap<String, Int>
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -580,7 +650,7 @@ class OfflineFirstChatRepository @Inject constructor(
val list = getMessagesBeforeAndEqual(
messageId,
internalConversationId,
100
limit
)

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

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