Skip to content

Commit

Permalink
Do not link in pagination direction for events at start of chunk
Browse files Browse the repository at this point in the history
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
  • Loading branch information
SpiritCroc committed Apr 30, 2022
1 parent ef57e60 commit 638e18e
Showing 1 changed file with 12 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down

0 comments on commit 638e18e

Please sign in to comment.