From fe69d8e3fa6762ffab4f2717e0ae3c682930c536 Mon Sep 17 00:00:00 2001 From: ganfra Date: Wed, 5 Apr 2023 12:57:26 +0200 Subject: [PATCH] Fix multiple read receipts for the same user in timeline #7882 --- changelog.d/7882.bugfix | 1 + .../database/helper/ChunkEntityHelper.kt | 12 +++-- .../home/room/detail/TimelineFragment.kt | 2 +- .../timeline/TimelineEventController.kt | 25 +++------ .../readreceipts/DisplayReadReceiptItem.kt | 6 +-- .../DisplayReadReceiptsBottomSheet.kt | 6 +-- .../DisplayReadReceiptsController.kt | 6 +-- .../readreceipts/ReadReceiptsCache.kt | 54 +++++++++++++++++++ 8 files changed, 82 insertions(+), 30 deletions(-) create mode 100644 changelog.d/7882.bugfix rename vector/src/main/java/im/vector/app/features/home/room/detail/{ => timeline}/readreceipts/DisplayReadReceiptItem.kt (93%) rename vector/src/main/java/im/vector/app/features/home/room/detail/{ => timeline}/readreceipts/DisplayReadReceiptsBottomSheet.kt (95%) rename vector/src/main/java/im/vector/app/features/home/room/detail/{ => timeline}/readreceipts/DisplayReadReceiptsController.kt (92%) create mode 100644 vector/src/main/java/im/vector/app/features/home/room/detail/timeline/readreceipts/ReadReceiptsCache.kt diff --git a/changelog.d/7882.bugfix b/changelog.d/7882.bugfix new file mode 100644 index 0000000000..dea9d13795 --- /dev/null +++ b/changelog.d/7882.bugfix @@ -0,0 +1 @@ +Fix multiple read receipts for the same user in timeline. 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 43f84e771a..b48e71464c 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 @@ -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().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(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() diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt index 20e283f2c5..918d9316b3 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt +++ b/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 diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/TimelineEventController.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/TimelineEventController.kt index fcdbcd777c..8e8fcade4e 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/TimelineEventController.kt +++ b/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() private val timelineEventsGroups = TimelineEventsGroups() - private val receiptsByEvent = HashMap>() + private val readReceiptsCache = ReadReceiptsCache() private val modelCache = arrayListOf() private var currentSnapshot: List = 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) } } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/readreceipts/DisplayReadReceiptItem.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/readreceipts/DisplayReadReceiptItem.kt similarity index 93% rename from vector/src/main/java/im/vector/app/features/home/room/detail/readreceipts/DisplayReadReceiptItem.kt rename to vector/src/main/java/im/vector/app/features/home/room/detail/timeline/readreceipts/DisplayReadReceiptItem.kt index a489c1ec66..acc6d63d38 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/readreceipts/DisplayReadReceiptItem.kt +++ b/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 diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/readreceipts/DisplayReadReceiptsBottomSheet.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/readreceipts/DisplayReadReceiptsBottomSheet.kt similarity index 95% rename from vector/src/main/java/im/vector/app/features/home/room/detail/readreceipts/DisplayReadReceiptsBottomSheet.kt rename to vector/src/main/java/im/vector/app/features/home/room/detail/timeline/readreceipts/DisplayReadReceiptsBottomSheet.kt index dfb23d25c8..f3658cdf1d 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/readreceipts/DisplayReadReceiptsBottomSheet.kt +++ b/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 diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/readreceipts/DisplayReadReceiptsController.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/readreceipts/DisplayReadReceiptsController.kt similarity index 92% rename from vector/src/main/java/im/vector/app/features/home/room/detail/readreceipts/DisplayReadReceiptsController.kt rename to vector/src/main/java/im/vector/app/features/home/room/detail/timeline/readreceipts/DisplayReadReceiptsController.kt index 76f2d72456..565fd1b292 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/readreceipts/DisplayReadReceiptsController.kt +++ b/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 diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/readreceipts/ReadReceiptsCache.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/timeline/readreceipts/ReadReceiptsCache.kt new file mode 100644 index 0000000000..8d34a33779 --- /dev/null +++ b/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>() + + // Key is userId, Value is eventId + private val receiptEventIdByUserId = HashMap() + + fun receiptsByEvent(): Map> { + return receiptsByEventId + } + + fun addReceiptsOnEvent(receipts: List, 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() + } +}