Do not delete events from the last forward chunk

We get end up with missing messages by the combination of
- deleting the last forward chunk when receiving a new one
- not adding events to a chunk that are already found in another chunk

Accordingly, when using chunk tokens to load more messages, those
messages that were not added to a chunk due to a /sync chunk will get
lost. More thorough steps to reproduce:

- Receive e.g. 30 new messages while offline
- Use /sync in the room overview, this will fetch the latest 10 events
- Open a chat in the past before the latest unread messages
- Scroll down a little, in order to fill the message gap and load all
  unread messages
- Close the chat
- Receive another e.g. 60 messages while offline
- Re-open the chat at some time in the past, before the latest 70
  messages
  => messages from the old /sync chunk will be missing

Change-Id: Ia3f2d2715a3edfd0b3fe5c3d48a02ade4ea49c4d
This commit is contained in:
SpiritCroc 2022-04-29 15:29:03 +02:00
parent 99053e8467
commit bda09aa03f
2 changed files with 36 additions and 0 deletions

View file

@ -40,6 +40,21 @@ import org.matrix.android.sdk.internal.extensions.assertIsManaged
import org.matrix.android.sdk.internal.session.room.timeline.PaginationDirection
import timber.log.Timber
internal fun ChunkEntity.moveEventsFrom(chunkToMerge: ChunkEntity, direction: PaginationDirection) {
assertIsManaged()
val localRealm = this.realm
val eventsToMerge = if (direction == PaginationDirection.FORWARDS) {
chunkToMerge.timelineEvents.sort(TimelineEventEntityFields.DISPLAY_INDEX, Sort.ASCENDING)
} else {
chunkToMerge.timelineEvents.sort(TimelineEventEntityFields.DISPLAY_INDEX, Sort.DESCENDING)
}
eventsToMerge.forEach {
if (addTimelineEventFromMove(localRealm, it, direction)) {
chunkToMerge.timelineEvents.remove(it)
}
}
}
internal fun ChunkEntity.merge(roomId: String, chunkToMerge: ChunkEntity, direction: PaginationDirection) {
assertIsManaged()
val localRealm = this.realm
@ -166,6 +181,17 @@ private fun ChunkEntity.addTimelineEventFromMerge(realm: Realm, timelineEventEnt
timelineEvents.add(copied)
}
private fun ChunkEntity.addTimelineEventFromMove(realm: Realm, event: TimelineEventEntity, direction: PaginationDirection): Boolean {
val eventId = event.eventId
if (timelineEvents.find(eventId) != null) {
return false
}
event.displayIndex = nextDisplayIndex(direction)
handleThreadSummary(realm, eventId, event)
timelineEvents.add(event)
return true
}
/**
* Upon copy of the timeline events we should update the latestMessage TimelineEventEntity with the new one
*/

View file

@ -43,6 +43,7 @@ import org.matrix.android.sdk.internal.crypto.DefaultCryptoService
import org.matrix.android.sdk.internal.database.helper.addIfNecessary
import org.matrix.android.sdk.internal.database.helper.addTimelineEvent
import org.matrix.android.sdk.internal.database.helper.createOrUpdate
import org.matrix.android.sdk.internal.database.helper.moveEventsFrom
import org.matrix.android.sdk.internal.database.helper.updateThreadSummaryIfNeeded
import org.matrix.android.sdk.internal.database.mapper.asDomain
import org.matrix.android.sdk.internal.database.mapper.toEntity
@ -365,6 +366,15 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle
aggregator: SyncResponsePostTreatmentAggregator): ChunkEntity {
val lastChunk = ChunkEntity.findLastForwardChunkOfRoom(realm, roomEntity.roomId)
if (isLimited && lastChunk != null) {
Timber.i("Deleting last forward chunk (${lastChunk.identifier()})")
// Add events that oldPrev may have dropped since they were already in lastChunk
val oldPrev = lastChunk.prevChunk
if (oldPrev != null && oldPrev.nextToken != lastChunk.prevToken) {
// If the tokens mismatch, this means we have chained them due to duplicated events.
// In this case, we need to make sure to re-add possibly dropped events (which would have
// been duplicates otherwise)
oldPrev.moveEventsFrom(lastChunk, PaginationDirection.FORWARDS)
}
lastChunk.deleteOnCascade(deleteStateEvents = false, canDeleteRoot = true)
}
val chunkEntity = if (!isLimited && lastChunk != null) {