fix to display newest messages while not showing duplicated first mes…

…sage of chat

+ remove some comments

Signed-off-by: Marcel Hibbe <dev@mhibbe.de>

app/src/androidTest/java/com/nextcloud/talk/data/database/dao/ChatMessagesDaoTest.kt

@@ -144,7 +144,7 @@ class ChatMessagesDaoTest {
  assertEquals("messages", chatMessagesConv1Since.first()[2].message)
 
  val chatMessagesConv1To =
- chatMessagesDao.getMessagesForConversationBefore(
+ chatMessagesDao.getMessagesForConversationBeforeAndEqual(
  conversation1.internalId,
  conv1chatMessage3.id,
  3

app/src/main/java/com/nextcloud/talk/chat/data/network/OfflineFirstChatRepository.kt

@@ -82,11 +82,6 @@ class OfflineFirstChatRepository @Inject constructor(
  internalConversationId = userProvider.currentUser.blockingGet().id!!.toString() + "@" + conversationModel
  .token
 
- // TODO: think about when getting id from conv list makes sense or when from datastore
- // val lastKnown = conversationModel.lastMessageViaConversationList!!.id
-
- // Log.d(TAG, "lastKnown from datastore: " + datastore.getLastKnownId(internalConversationId, 0).toLong())
-
  val fieldMap = getFieldMap(
  lookIntoFuture = false,
  includeLastKnown = true,
@@ -96,45 +91,16 @@ class OfflineFirstChatRepository @Inject constructor(
  withNetworkParams.putSerializable(BundleKeys.KEY_FIELD_MAP, fieldMap)
  withNetworkParams.putString(BundleKeys.KEY_ROOM_TOKEN, conversationModel.token)
 
- // this@OfflineFirstChatRepository.sync(withNetworkParams)
  sync(withNetworkParams)
 
  Log.d(TAG, "newestMessageId after sync: " + chatDao.getNewestMessageId(internalConversationId))
 
- showLast100MessagesUntil(
+ showLast100MessagesBeforeAndEqual(
  internalConversationId,
  chatDao.getNewestMessageId(internalConversationId)
  )
  }
 
- private suspend fun showLast100MessagesUntil(internalConversationId: String, messageId: Long) {
- Log.d(TAG, "showLast100MessagesUntil messageId $messageId")
- // if (messageId > 0) {
- val list = getMessagesBefore(
- messageId,
- internalConversationId,
- 100 // TODO 100 or 101 ??????? dep on includeLastKnown ?????
- )
-
- if (list.isNotEmpty()) {
-
- // val placeholderMessage = ChatMessage()
- // placeholderMessage.message = "MESSAGES MAY BE MISSING!!!!"
- // placeholderMessage.systemMessageType = ChatMessage.SystemMessageType.LOBBY_NONE
- //
- // val mutableList = list.toMutableList()
- // mutableList.add(list.size, placeholderMessage)
- val pair = Pair(false, list)
- _messageFlow.emit(pair)
- } else if (!monitor.isOnline.first()) {
- // TODO: let user now that device is offline and no messages are stored offline!
- Log.d(TAG, "device is offline and no messages are stored offline")
- }
- // }
- }
-
- // TODO: right now this only handles scrolling into past. Must also handle scrolling towards future, e.g. when
- // beforehand jumped to some original message of an answer or searched for a message.
  override fun loadMoreMessages(
  beforeMessageId: Long,
  roomToken: String,
@@ -174,7 +140,7 @@ class OfflineFirstChatRepository @Inject constructor(
  }
  }
 
- showLast100MessagesUntil(internalConversationId, beforeMessageId)
+ showLast100MessagesBefore(internalConversationId, beforeMessageId)
  }
 
  private suspend fun hasToLoadPreviousMessagesFromServer(
@@ -186,7 +152,6 @@ class OfflineFirstChatRepository @Inject constructor(
  internalConversationId,
  beforeMessageId
  ).first().first()
- // val blockForMessage = updateBlocks(chatBlock) // only get block without to update?!
 
  if (blockForMessage == null) {
  Log.d(TAG, "No blocks for this message were found so we have to ask server")
@@ -289,12 +254,12 @@ class OfflineFirstChatRepository @Inject constructor(
  return fieldMap
  }
 
- private suspend fun getMessagesBefore(
+ private suspend fun getMessagesBeforeAndEqual(
  messageId: Long,
  internalConversationId: String,
  messageLimit: Int
  ): List<ChatMessage> =
- chatDao.getMessagesForConversationBefore(
+ chatDao.getMessagesForConversationBeforeAndEqual(
  internalConversationId,
  messageId,
  messageLimit
@@ -331,10 +296,6 @@ class OfflineFirstChatRepository @Inject constructor(
  .map(ChatMessageEntity::asModel)
  }
 
- // TODO: move this code to getMessagesFromServer ? Do not use global variables here?!
- private fun process(response: Response<*>, roomToken: String) {
- }
-
  @Suppress("UNCHECKED_CAST")
  private fun getMessagesFromServer(bundle: Bundle): Pair<Int, List<ChatMessageJson>>? {
  Log.d(TAG, "An online request is made!!!!!!!!!!!!!!!!!!!!")
@@ -347,7 +308,6 @@ class OfflineFirstChatRepository @Inject constructor(
  val result = network.pullChatMessages(credentials!!, url!!, fieldMap)
  .subscribeOn(Schedulers.io())
  .observeOn(AndroidSchedulers.mainThread())
- // .filter { it.body() != null }
  .map {
  // process(it, roomToken)
 
@@ -457,8 +417,6 @@ class OfflineFirstChatRepository @Inject constructor(
 
  Log.d(TAG, "newestIdForNewChatBlock: $newestIdForNewChatBlock")
 
- // insert new chat block without to analyze if other chatBlocks cover this messageId. This will be
- // done in following steps
  val newChatBlock = ChatBlockEntity(
  internalConversationId = internalConversationId,
  oldestMessageId = oldestIdFromSync,
@@ -473,42 +431,6 @@ class OfflineFirstChatRepository @Inject constructor(
  }
  }
 
- // override suspend fun syncWith(bundle: Bundle, synchronizer: Synchronizer): Boolean =
- // synchronizer.changeListSync(
- // modelFetcher = {
- // return@changeListSync getMessagesFromServer(bundle)!!
- // },
- // // not needed
- // versionUpdater = {},
- // // not needed
- // modelDeleter = {},
- // modelUpdater = { models ->
- // newMessageIds = models.map { it.id }
- // val list = models.filterIsInstance<ChatMessageJson>().map {
- // it.asEntity(currentUser.id!!)
- // }
- // chatDao.upsertChatMessages(list)
- //
- // val oldestIdFromSync = list.minByOrNull { it.id }!!.id
- // val newestIdFromSync = list.maxByOrNull { it.id }!!.id
- //
- // Log.d(TAG, "oldestIdFromSync: $oldestIdFromSync")
- // Log.d(TAG, "newestIdFromSync: $newestIdFromSync")
- //
- // // insert new chat block without to analyze if other chatBlocks cover this messageId. This will be
- // // done in following steps
- // val newChatBlock = ChatBlockEntity(
- // internalConversationId = internalConversationId,
- // oldestMessageId = oldestIdFromSync,
- // newestMessageId = newestIdFromSync,
- // hasHistory = false
- // )
- // chatBlocksDao.upsertChatBlock(newChatBlock)
- //
- // updateBlocksForMessageId(oldestIdFromSync)
- // }
- // )
-
  /**
  Updates all chat blocks to handle the messageId
  - If one chat block contains the messageId: Nothing is done. return this chatBlock
@@ -561,6 +483,58 @@ class OfflineFirstChatRepository @Inject constructor(
  }
  }
 
+ private suspend fun showLast100MessagesBeforeAndEqual(internalConversationId: String, messageId: Long) {
+ suspend fun getMessagesBeforeAndEqual(
+ messageId: Long,
+ internalConversationId: String,
+ messageLimit: Int
+ ): List<ChatMessage> =
+ chatDao.getMessagesForConversationBeforeAndEqual(
+ internalConversationId,
+ messageId,
+ messageLimit
+ ).map {
+ it.map(ChatMessageEntity::asModel)
+ }.first()
+
+ val list = getMessagesBeforeAndEqual(
+ messageId,
+ internalConversationId,
+ 100
+ )
+
+ if (list.isNotEmpty()) {
+ val pair = Pair(false, list)
+ _messageFlow.emit(pair)
+ }
+ }
+
+ private suspend fun showLast100MessagesBefore(internalConversationId: String, messageId: Long) {
+ suspend fun getMessagesBefore(
+ messageId: Long,
+ internalConversationId: String,
+ messageLimit: Int
+ ): List<ChatMessage> =
+ chatDao.getMessagesForConversationBefore(
+ internalConversationId,
+ messageId,
+ messageLimit
+ ).map {
+ it.map(ChatMessageEntity::asModel)
+ }.first()
+
+ val list = getMessagesBefore(
+ messageId,
+ internalConversationId,
+ 100
+ )
+
+ if (list.isNotEmpty()) {
+ val pair = Pair(false, list)
+ _messageFlow.emit(pair)
+ }
+ }
+
  override fun handleOnPause() {
  itIsPaused = true
  }

app/src/main/java/com/nextcloud/talk/data/database/dao/ChatMessagesDao.kt

@@ -95,6 +95,22 @@ interface ChatMessagesDao {
  limit: Int
  ): Flow<List<ChatMessageEntity>>
 
+ @Query(
+ """
+ SELECT *
+ FROM ChatMessages
+ WHERE internalConversationId = :internalConversationId 
+ AND id <= :messageId
+ ORDER BY timestamp DESC, id DESC
+ LIMIT :limit
+ """
+ )
+ fun getMessagesForConversationBeforeAndEqual(
+ internalConversationId: String,
+ messageId: Long,
+ limit: Int
+ ): Flow<List<ChatMessageEntity>>
+
  @Query(
  """
  SELECT COUNT(*)