From cd45248f402699ab7970520123468555d7918ee0 Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Sat, 12 Mar 2022 09:32:25 +0100 Subject: [PATCH 1/4] Fix modifying the wrong events in TimelineChunk I was observing cases where builtEvents[modificationIndex] was not having the same eventId as the udpatedEntity in handleDatabaseChangeSet. In particular, I observed both cases that - there was no item in the list yet with the same eventId as the updated one - there was an item with the same eventId already in the list, but at a different position. Whenever this happened, the timeline would render missing, duplicated, or swapped messages in the timeline. Instead of relying on the modificationIndex to be the same for both the change set and builtEvents, look up the proper index by eventId. --- changelog.d/5528.bugfix | 1 + .../sdk/internal/session/room/timeline/TimelineChunk.kt | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 changelog.d/5528.bugfix diff --git a/changelog.d/5528.bugfix b/changelog.d/5528.bugfix new file mode 100644 index 0000000000..b0dc759ab0 --- /dev/null +++ b/changelog.d/5528.bugfix @@ -0,0 +1 @@ +Fix cases of missing, swapped, or duplicated messages diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt index 77f210aa9a..472cda39f8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt @@ -447,8 +447,12 @@ internal class TimelineChunk(private val chunkEntity: ChunkEntity, for (range in modifications) { for (modificationIndex in (range.startIndex until range.startIndex + range.length)) { val updatedEntity = results[modificationIndex] ?: continue + val displayIndex = builtEventsIndexes.getOrDefault(updatedEntity.eventId, null) + if (displayIndex == null) { + continue + } try { - builtEvents[modificationIndex] = updatedEntity.buildAndDecryptIfNeeded() + builtEvents[displayIndex] = updatedEntity.buildAndDecryptIfNeeded() } catch (failure: Throwable) { Timber.v("Fail to update items at index: $modificationIndex") } From 626395304db6f0818cbfa337ed469a47e4684642 Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Sat, 19 Mar 2022 11:51:19 +0100 Subject: [PATCH 2/4] Fix crash on Android 6 --- .../android/sdk/internal/session/room/timeline/TimelineChunk.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt index 472cda39f8..368979d73e 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt @@ -447,7 +447,7 @@ internal class TimelineChunk(private val chunkEntity: ChunkEntity, for (range in modifications) { for (modificationIndex in (range.startIndex until range.startIndex + range.length)) { val updatedEntity = results[modificationIndex] ?: continue - val displayIndex = builtEventsIndexes.getOrDefault(updatedEntity.eventId, null) + val displayIndex = builtEventsIndexes[updatedEntity.eventId] if (displayIndex == null) { continue } From 6a8230239b13b52d6b2622afdffd4d56726964c3 Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Mon, 21 Mar 2022 10:46:59 +0100 Subject: [PATCH 3/4] Avoid inconsistent timelines by db insertions before fully loaded chunk --- .../session/room/timeline/TimelineChunk.kt | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt index 368979d73e..1bd541c2a9 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt @@ -430,6 +430,31 @@ internal class TimelineChunk(private val chunkEntity: ChunkEntity, private fun handleDatabaseChangeSet(results: RealmResults, changeSet: OrderedCollectionChangeSet) { val insertions = changeSet.insertionRanges for (range in insertions) { + // Check if the insertion's displayIndices match our expectations - or skip this insertion. + // Inconsistencies (missing messages) can happen otherwise if we get insertions before having loaded all timeline events of the chunk. + if (builtEvents.isNotEmpty()) { + // Check consistency to item before insertions + if (range.startIndex > 0) { + val firstInsertion = results[range.startIndex]!! + val lastBeforeInsertion = builtEvents[range.startIndex-1] + if (firstInsertion.displayIndex+1 != lastBeforeInsertion.displayIndex) { + Timber.i("handleDatabaseChangeSet: skip insertion at ${range.startIndex}/${builtEvents.size}, " + + "displayIndex mismatch at ${range.startIndex}: ${firstInsertion.displayIndex} -> ${lastBeforeInsertion.displayIndex}") + continue + } + } + // Check consistency to item after insertions + if (range.startIndex < builtEvents.size) { + val lastInsertion = results[range.startIndex+range.length-1]!! + val firstAfterInsertion = builtEvents[range.startIndex] + if (firstAfterInsertion.displayIndex+1 != lastInsertion.displayIndex) { + Timber.i("handleDatabaseChangeSet: skip insertion at ${range.startIndex}/${builtEvents.size}, " + + "displayIndex mismatch at ${range.startIndex+range.length}: " + + "${firstAfterInsertion.displayIndex} -> ${lastInsertion.displayIndex}") + continue + } + } + } val newItems = results .subList(range.startIndex, range.startIndex + range.length) .map { it.buildAndDecryptIfNeeded() } From 7c0cd1dc52e3ebb6f7b8be65a8da93f837b175f0 Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Tue, 12 Apr 2022 19:16:48 +0200 Subject: [PATCH 4/4] Fix ktLint / op-spacing --- .../internal/session/room/timeline/TimelineChunk.kt | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt index 1bd541c2a9..db90343422 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/timeline/TimelineChunk.kt @@ -436,8 +436,8 @@ internal class TimelineChunk(private val chunkEntity: ChunkEntity, // Check consistency to item before insertions if (range.startIndex > 0) { val firstInsertion = results[range.startIndex]!! - val lastBeforeInsertion = builtEvents[range.startIndex-1] - if (firstInsertion.displayIndex+1 != lastBeforeInsertion.displayIndex) { + val lastBeforeInsertion = builtEvents[range.startIndex - 1] + if (firstInsertion.displayIndex + 1 != lastBeforeInsertion.displayIndex) { Timber.i("handleDatabaseChangeSet: skip insertion at ${range.startIndex}/${builtEvents.size}, " + "displayIndex mismatch at ${range.startIndex}: ${firstInsertion.displayIndex} -> ${lastBeforeInsertion.displayIndex}") continue @@ -445,11 +445,11 @@ internal class TimelineChunk(private val chunkEntity: ChunkEntity, } // Check consistency to item after insertions if (range.startIndex < builtEvents.size) { - val lastInsertion = results[range.startIndex+range.length-1]!! + val lastInsertion = results[range.startIndex + range.length - 1]!! val firstAfterInsertion = builtEvents[range.startIndex] - if (firstAfterInsertion.displayIndex+1 != lastInsertion.displayIndex) { + if (firstAfterInsertion.displayIndex + 1 != lastInsertion.displayIndex) { Timber.i("handleDatabaseChangeSet: skip insertion at ${range.startIndex}/${builtEvents.size}, " + - "displayIndex mismatch at ${range.startIndex+range.length}: " + + "displayIndex mismatch at ${range.startIndex + range.length}: " + "${firstAfterInsertion.displayIndex} -> ${lastInsertion.displayIndex}") continue }