From ef57e60df80cc6110e4e1b8f0f83b627d6be1609 Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Fri, 29 Apr 2022 15:29:03 +0200 Subject: [PATCH] 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 --- .../database/helper/ChunkEntityHelper.kt | 26 +++++++++++++++++++ .../sync/handler/room/RoomSyncHandler.kt | 10 +++++++ 2 files changed, 36 insertions(+) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/helper/ChunkEntityHelper.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/helper/ChunkEntityHelper.kt index 4fdedabd705..821e4b562a7 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/helper/ChunkEntityHelper.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/helper/ChunkEntityHelper.kt @@ -39,6 +39,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 @@ -165,6 +180,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 */ diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt index 05dad983dac..0e94a4c0cd0 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt @@ -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 @@ -364,6 +365,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) {