From 638e18eaab475744b3a23742bceb9c2f2319e66c Mon Sep 17 00:00:00 2001
From: SpiritCroc <dev@spiritcroc.de>
Date: Fri, 29 Apr 2022 15:29:36 +0200
Subject: [PATCH] Do not link in pagination direction for events at start of
 chunk

If we link chunks in pagination direction, and discard all events after
that, we assume that we reached a point in the chunk that is already
covered by a different chunk.
If we however haven't seen any new events in that chunk yet, chances are
this is the wrong direction we are linking. So in this case, better just
skip related events and continue processing later events - making sure
we don't lose new events and don't link in the wrong direction.

Note we could also enforce links into the opposite direction in this case.
Since in the cases I observed so far, such link already existed, so I
think this is probably not necessary.

Change-Id: Ia4d2fd87188b9757ed68416e883c3fb489cdfa6e
---
 .../room/timeline/TokenChunkEventPersistor.kt        | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TokenChunkEventPersistor.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TokenChunkEventPersistor.kt
index d3f24a8568a..a7bb53883b9 100644
--- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TokenChunkEventPersistor.kt
+++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TokenChunkEventPersistor.kt
@@ -177,6 +177,7 @@ internal class TokenChunkEventPersistor @Inject constructor(
             }
         }
         val optimizedThreadSummaryMap = hashMapOf<String, EventEntity>()
+        var hasNewEvents = false
         run processTimelineEvents@{
             eventList.forEach { event ->
                 if (event.eventId == null || event.senderId == null) {
@@ -191,6 +192,13 @@ internal class TokenChunkEventPersistor @Inject constructor(
                 // If it exists, we want to stop here, just link the prevChunk
                 val existingChunk = existingTimelineEvent?.chunk?.firstOrNull()
                 if (existingChunk != null) {
+                    // If we haven't found a single new event yet, we don't want to link in the pagination direction, as that might cause a
+                    // timeline loop if the other chunk is in the other direction.
+                    if (!hasNewEvents) {
+                        Timber.i("Skip adding event $eventId, already exists")
+                        // Only skip this event, but still process other events
+                        return@forEach
+                    }
                     when (direction) {
                         PaginationDirection.BACKWARDS -> {
                             if (currentChunk.nextChunk == existingChunk) {
@@ -212,6 +220,10 @@ internal class TokenChunkEventPersistor @Inject constructor(
                     // Stop processing here
                     return@processTimelineEvents
                 }
+
+                // existingChunk == null => this is a new event we haven't seen before
+                hasNewEvents = true
+
                 val ageLocalTs = event.unsignedData?.age?.let { now - it }
                 var eventEntity = event.toEntity(roomId, SendState.SYNCED, ageLocalTs).copyToRealmOrIgnore(realm, EventInsertType.PAGINATION)
                 if (event.type == EventType.STATE_ROOM_MEMBER && event.stateKey != null) {