Merge pull request #8310 from vector-im/feature/fga/fix_multiple_read_receipts

Fix multiple read receipts for the same user in timeline #7882

changelog.d/7882.bugfix

@@ -0,0 +1 @@
+Fix multiple read receipts for the same user in timeline.

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/helper/ChunkEntityHelper.kt

@@ -21,6 +21,7 @@ import io.realm.kotlin.createObject
 import org.matrix.android.sdk.api.session.events.model.content.EncryptedEventContent
 import org.matrix.android.sdk.api.session.events.model.toModel
 import org.matrix.android.sdk.api.session.room.model.RoomMemberContent
+import org.matrix.android.sdk.api.session.room.read.ReadService
 import org.matrix.android.sdk.internal.crypto.model.SessionInfo
 import org.matrix.android.sdk.internal.database.mapper.asDomain
 import org.matrix.android.sdk.internal.database.model.ChunkEntity
@@ -76,7 +77,7 @@ internal fun ChunkEntity.addTimelineEvent(
     val senderId = eventEntity.sender ?: ""
 
     // Update RR for the sender of a new message with a dummy one
-    val readReceiptsSummaryEntity = if (!ownedByThreadChunk) handleReadReceipts(realm, roomId, eventEntity, senderId) else null
+    val readReceiptsSummaryEntity = handleReadReceiptsOfSender(realm, roomId, eventEntity, senderId)
     val timelineEventEntity = realm.createObject<TimelineEventEntity>().apply {
         this.localId = localId
         this.root = eventEntity
@@ -124,7 +125,7 @@ internal fun computeIsUnique(
     }
 }
 
-private fun handleReadReceipts(realm: Realm, roomId: String, eventEntity: EventEntity, senderId: String): ReadReceiptsSummaryEntity {
+private fun handleReadReceiptsOfSender(realm: Realm, roomId: String, eventEntity: EventEntity, senderId: String): ReadReceiptsSummaryEntity {
     val readReceiptsSummaryEntity = ReadReceiptsSummaryEntity.where(realm, eventEntity.eventId).findFirst()
             ?: realm.createObject<ReadReceiptsSummaryEntity>(eventEntity.eventId).apply {
                 this.roomId = roomId
@@ -132,7 +133,12 @@ private fun handleReadReceipts(realm: Realm, roomId: String, eventEntity: EventE
     val originServerTs = eventEntity.originServerTs
     if (originServerTs != null) {
         val timestampOfEvent = originServerTs.toDouble()
-        val readReceiptOfSender = ReadReceiptEntity.getOrCreate(realm, roomId = roomId, userId = senderId, threadId = eventEntity.rootThreadEventId)
+        val readReceiptOfSender = ReadReceiptEntity.getOrCreate(
+                realm = realm,
+                roomId = roomId,
+                userId = senderId,
+                threadId = eventEntity.rootThreadEventId ?: ReadService.THREAD_ID_MAIN
+        )
         // If the synced RR is older, update
         if (timestampOfEvent > readReceiptOfSender.originServerTs) {
             val previousReceiptsSummary = ReadReceiptsSummaryEntity.where(realm, eventId = readReceiptOfSender.eventId).findFirst()

vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt

@@ -139,7 +139,6 @@ import im.vector.app.features.home.room.detail.composer.MessageComposerViewModel
 import im.vector.app.features.home.room.detail.composer.boolean
 import im.vector.app.features.home.room.detail.composer.voice.VoiceRecorderFragment
 import im.vector.app.features.home.room.detail.error.RoomNotFound
-import im.vector.app.features.home.room.detail.readreceipts.DisplayReadReceiptsBottomSheet
 import im.vector.app.features.home.room.detail.timeline.TimelineEventController
 import im.vector.app.features.home.room.detail.timeline.action.EventSharedAction
 import im.vector.app.features.home.room.detail.timeline.action.MessageActionsBottomSheet
@@ -156,6 +155,7 @@ import im.vector.app.features.home.room.detail.timeline.item.MessageTextItem
 import im.vector.app.features.home.room.detail.timeline.item.MessageVoiceItem
 import im.vector.app.features.home.room.detail.timeline.item.ReadReceiptData
 import im.vector.app.features.home.room.detail.timeline.reactions.ViewReactionsBottomSheet
+import im.vector.app.features.home.room.detail.timeline.readreceipts.DisplayReadReceiptsBottomSheet
 import im.vector.app.features.home.room.detail.timeline.url.PreviewUrlRetriever
 import im.vector.app.features.home.room.detail.upgrade.MigrateRoomBottomSheet
 import im.vector.app.features.home.room.detail.views.RoomDetailLazyLoadedViews

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/TimelineEventController.kt

@@ -58,6 +58,7 @@ import im.vector.app.features.home.room.detail.timeline.item.ReactionsSummaryEve
 import im.vector.app.features.home.room.detail.timeline.item.ReadReceiptData
 import im.vector.app.features.home.room.detail.timeline.item.ReadReceiptsItem
 import im.vector.app.features.home.room.detail.timeline.item.TypingItem_
+import im.vector.app.features.home.room.detail.timeline.readreceipts.ReadReceiptsCache
 import im.vector.app.features.home.room.detail.timeline.url.PreviewUrlRetriever
 import im.vector.app.features.media.AttachmentData
 import im.vector.app.features.media.ImageContentRenderer
@@ -74,7 +75,6 @@ import org.matrix.android.sdk.api.session.room.model.RoomSummary
 import org.matrix.android.sdk.api.session.room.model.message.MessageAudioContent
 import org.matrix.android.sdk.api.session.room.model.message.MessageImageInfoContent
 import org.matrix.android.sdk.api.session.room.model.message.MessageVideoContent
-import org.matrix.android.sdk.api.session.room.read.ReadService
 import org.matrix.android.sdk.api.session.room.timeline.Timeline
 import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
 import timber.log.Timber
@@ -201,7 +201,7 @@ class TimelineEventController @Inject constructor(
     // Map eventId to adapter position
     private val adapterPositionMapping = HashMap<String, Int>()
     private val timelineEventsGroups = TimelineEventsGroups()
-    private val receiptsByEvent = HashMap<String, MutableList<ReadReceipt>>()
+    private val readReceiptsCache = ReadReceiptsCache()
     private val modelCache = arrayListOf<CacheItemData?>()
     private var currentSnapshot: List<TimelineEvent> = emptyList()
     private var inSubmitList: Boolean = false
@@ -417,7 +417,7 @@ class TimelineEventController @Inject constructor(
         }
         Timber.v("Preprocess events took $preprocessEventsTiming ms")
         var numberOfEventsToBuild = 0
-        val lastSentEventWithoutReadReceipts = searchLastSentEventWithoutReadReceipts(receiptsByEvent)
+        val lastSentEventWithoutReadReceipts = searchLastSentEventWithoutReadReceipts(readReceiptsCache.receiptsByEvent())
         (0 until modelCache.size).forEach { position ->
             val event = currentSnapshot[position]
             val nextEvent = currentSnapshot.nextOrNull(position)
@@ -463,7 +463,7 @@ class TimelineEventController @Inject constructor(
             }
             val itemCachedData = modelCache[position] ?: return@forEach
             // Then update with additional models if needed
-            modelCache[position] = itemCachedData.enrichWithModels(event, nextEvent, position, receiptsByEvent)
+            modelCache[position] = itemCachedData.enrichWithModels(event, nextEvent, position, readReceiptsCache.receiptsByEvent())
         }
         Timber.v("Number of events to rebuild: $numberOfEventsToBuild on ${modelCache.size} total events")
     }
@@ -552,15 +552,15 @@ class TimelineEventController @Inject constructor(
     }
 
     private fun preprocessReverseEvents() {
-        receiptsByEvent.clear()
+        readReceiptsCache.clear()
         timelineEventsGroups.clear()
         val itr = currentSnapshot.listIterator(currentSnapshot.size)
         var lastShownEventId: String? = null
         while (itr.hasPrevious()) {
             val event = itr.previous()
             timelineEventsGroups.addOrIgnore(event)
-            val currentReadReceipts = ArrayList(event.readReceipts).filter {
-                it.roomMember.userId != session.myUserId && it.isVisibleInThisThread()
+            val currentReadReceipts = event.readReceipts.filter {
+                it.roomMember.userId != session.myUserId
             }
             if (timelineEventVisibilityHelper.shouldShowEvent(
                             timelineEvent = event,
@@ -573,16 +573,7 @@ class TimelineEventController @Inject constructor(
             if (lastShownEventId == null) {
                 continue
             }
-            val existingReceipts = receiptsByEvent.getOrPut(lastShownEventId) { ArrayList() }
-            existingReceipts.addAll(currentReadReceipts)
-        }
-    }
-
-    private fun ReadReceipt.isVisibleInThisThread(): Boolean {
-        return if (partialState.isFromThreadTimeline()) {
-            this.threadId == partialState.rootThreadEventId
-        } else {
-            this.threadId == null || this.threadId == ReadService.THREAD_ID_MAIN
+            readReceiptsCache.addReceiptsOnEvent(currentReadReceipts, lastShownEventId)
         }
     }
 

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/readreceipts/DisplayReadReceiptItem.kt

@@ -1,11 +1,11 @@
 /*
- * Copyright 2019 New Vector Ltd
+ * Copyright (c) 2023 New Vector Ltd
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- * http://www.apache.org/licenses/LICENSE-2.0
+ *     http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package im.vector.app.features.home.room.detail.readreceipts
+package im.vector.app.features.home.room.detail.timeline.readreceipts
 
 import android.widget.ImageView
 import android.widget.TextView

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/readreceipts/DisplayReadReceiptsBottomSheet.kt

@@ -1,11 +1,11 @@
 /*
- * Copyright 2019 New Vector Ltd
+ * Copyright (c) 2023 New Vector Ltd
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- * http://www.apache.org/licenses/LICENSE-2.0
+ *     http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package im.vector.app.features.home.room.detail.readreceipts
+package im.vector.app.features.home.room.detail.timeline.readreceipts
 
 import android.os.Bundle
 import android.os.Parcelable

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/readreceipts/DisplayReadReceiptsController.kt

@@ -1,11 +1,11 @@
 /*
- * Copyright 2019 New Vector Ltd
+ * Copyright (c) 2023 New Vector Ltd
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- * http://www.apache.org/licenses/LICENSE-2.0
+ *     http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-package im.vector.app.features.home.room.detail.readreceipts
+package im.vector.app.features.home.room.detail.timeline.readreceipts
 
 import com.airbnb.epoxy.TypedEpoxyController
 import im.vector.app.core.date.DateFormatKind

vector/src/main/java/im/vector/app/features/home/room/detail/timeline/readreceipts/ReadReceiptsCache.kt

@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2023 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.app.features.home.room.detail.timeline.readreceipts
+
+import im.vector.lib.core.utils.compat.removeIfCompat
+import org.matrix.android.sdk.api.session.room.model.ReadReceipt
+
+class ReadReceiptsCache {
+
+    private val receiptsByEventId = HashMap<String, MutableList<ReadReceipt>>()
+
+    // Key is userId, Value is eventId
+    private val receiptEventIdByUserId = HashMap<String, String>()
+
+    fun receiptsByEvent(): Map<String, List<ReadReceipt>> {
+        return receiptsByEventId
+    }
+
+    fun addReceiptsOnEvent(receipts: List<ReadReceipt>, eventId: String) {
+        val existingReceipts = receiptsByEventId.getOrPut(eventId) { ArrayList() }
+        receipts.forEach { readReceipt ->
+            val receiptUserId = readReceipt.roomMember.userId
+            val receiptEventId = receiptEventIdByUserId[receiptUserId]
+            // If we already have a read receipt for this user, move it so we only
+            // use the most recent. It can happen because of threaded read receipts.
+            if (receiptEventId != null) {
+                receiptsByEventId[receiptEventId]?.removeIfCompat {
+                    it.roomMember.userId == receiptUserId
+                }
+            }
+            receiptEventIdByUserId[receiptUserId] = eventId
+            existingReceipts.add(readReceipt)
+        }
+    }
+
+    fun clear() {
+        receiptsByEventId.clear()
+        receiptEventIdByUserId.clear()
+    }
+}