diff --git a/library/ui-strings/src/main/res/values/strings_sc.xml b/library/ui-strings/src/main/res/values/strings_sc.xml index f4718909eb..01699e99ee 100644 --- a/library/ui-strings/src/main/res/values/strings_sc.xml +++ b/library/ui-strings/src/main/res/values/strings_sc.xml @@ -167,6 +167,7 @@ Read receipt debugging ViewPager debugging Show ViewPager debugging information + Duplicate read receipts Element features diff --git a/matrix-sdk-android/src/main/java/de/spiritcroc/matrixsdk/util/DbgUtil.kt b/matrix-sdk-android/src/main/java/de/spiritcroc/matrixsdk/util/DbgUtil.kt index 6825be65ac..fb27474fb6 100644 --- a/matrix-sdk-android/src/main/java/de/spiritcroc/matrixsdk/util/DbgUtil.kt +++ b/matrix-sdk-android/src/main/java/de/spiritcroc/matrixsdk/util/DbgUtil.kt @@ -13,6 +13,7 @@ object DbgUtil { const val DBG_SHOW_DISPLAY_INDEX = "DBG_SHOW_DISPLAY_INDEX" const val DBG_VIEW_PAGER = "DBG_VIEW_PAGER" const val DBG_VIEW_PAGER_VISUALS = "DBG_VIEW_PAGER_VISUALS" + const val DBG_SHOW_DUPLICATE_READ_RECEIPTS = "DBG_SHOW_DUPLICATE_READ_RECEIPTS" private val prefs = HashMap() @@ -24,6 +25,7 @@ object DbgUtil { DBG_SHOW_DISPLAY_INDEX, DBG_VIEW_PAGER, DBG_VIEW_PAGER_VISUALS, + DBG_SHOW_DUPLICATE_READ_RECEIPTS, ) fun load(context: Context) { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/read/ReadService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/read/ReadService.kt index 5ebabb093c..40dae18076 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/read/ReadService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/read/ReadService.kt @@ -89,5 +89,7 @@ interface ReadService { companion object { const val THREAD_ID_MAIN = "main" + // Artificial read marker, updated whenever someone sets the main or null one + const val THREAD_ID_MAIN_OR_NULL = "main_or_null" } } 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 5d7777480f..9a89eea00f 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 @@ -24,6 +24,7 @@ import org.matrix.android.sdk.api.session.events.model.content.EncryptedEventCon 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.Companion.THREAD_ID_MAIN +import org.matrix.android.sdk.api.session.room.read.ReadService.Companion.THREAD_ID_MAIN_OR_NULL 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 @@ -137,9 +138,13 @@ private fun handleReadReceipts(realm: Realm, roomId: String, eventEntity: EventE val originServerTs = eventEntity.originServerTs if (originServerTs != null) { val timestampOfEvent = originServerTs.toDouble() - // null receipts also update the main receipt - val receiptDestination = eventEntity.rootThreadEventId ?: THREAD_ID_MAIN - setOf(receiptDestination, eventEntity.rootThreadEventId).distinct().forEach { rootThreadEventId -> + // SC: fight duplicate read receipts in main timeline + val receiptDestinations = if (eventEntity.rootThreadEventId in listOf(null, THREAD_ID_MAIN)) { + setOf(eventEntity.rootThreadEventId, THREAD_ID_MAIN_OR_NULL) + } else { + setOf(eventEntity.rootThreadEventId) + } + receiptDestinations.forEach { rootThreadEventId -> val readReceiptOfSender = ReadReceiptEntity.getOrCreate(realm, roomId = roomId, userId = senderId, threadId = rootThreadEventId) // If the synced RR is older, update if (timestampOfEvent > readReceiptOfSender.originServerTs) { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/TimelineEventMapper.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/TimelineEventMapper.kt index e658622444..ccaa4c09c8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/TimelineEventMapper.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/mapper/TimelineEventMapper.kt @@ -49,7 +49,7 @@ internal class TimelineEventMapper @Inject constructor(private val readReceiptsS ownedByThreadChunk = timelineEventEntity.ownedByThreadChunk, readReceipts = readReceipts ?.distinctBy { - it.roomMember + Pair(it.roomMember, it.threadId) }?.sortedByDescending { it.originServerTs }.orEmpty() diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/ReadReceiptHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/ReadReceiptHandler.kt index 64343689ab..d34806f6c3 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/ReadReceiptHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/ReadReceiptHandler.kt @@ -19,8 +19,11 @@ package org.matrix.android.sdk.internal.session.sync.handler.room import de.spiritcroc.matrixsdk.util.DbgUtil import de.spiritcroc.matrixsdk.util.Dimber import io.realm.Realm +import org.matrix.android.sdk.api.extensions.orTrue import org.matrix.android.sdk.api.session.events.model.EventType import org.matrix.android.sdk.api.session.room.read.ReadService +import org.matrix.android.sdk.api.session.room.read.ReadService.Companion.THREAD_ID_MAIN +import org.matrix.android.sdk.api.session.room.read.ReadService.Companion.THREAD_ID_MAIN_OR_NULL import org.matrix.android.sdk.internal.database.model.ReadReceiptEntity import org.matrix.android.sdk.internal.database.model.ReadReceiptsSummaryEntity import org.matrix.android.sdk.internal.database.query.createUnmanaged @@ -104,6 +107,9 @@ internal class ReadReceiptHandler @Inject constructor( private fun initialSyncStrategy(realm: Realm, roomId: String, content: ReadReceiptContent) { val readReceiptSummaries = ArrayList() + // SC: fight duplicate read markers + val summariesByEventId = HashMap() + val mainReceiptByUserId = HashMap() for ((eventId, receiptDict) in content) { val userIdsDict = receiptDict[READ_KEY] ?: continue val readReceiptsSummary = ReadReceiptsSummaryEntity(eventId = eventId, roomId = roomId) @@ -114,8 +120,16 @@ internal class ReadReceiptHandler @Inject constructor( val receiptEntity = ReadReceiptEntity.createUnmanaged(roomId, eventId, userId, threadId, ts) rrDimber.i{"Handle initial sync RR $roomId / $userId thread $threadId: event $eventId"} readReceiptsSummary.readReceipts.add(receiptEntity) + // SC: fight duplicate read markers, by unifying main|null into the same marker + if (threadId in listOf(null, THREAD_ID_MAIN) && mainReceiptByUserId[userId]?.originServerTs?.let { it < ts }.orTrue()) { + mainReceiptByUserId[userId] = ReadReceiptEntity.createUnmanaged(roomId, eventId, userId, THREAD_ID_MAIN_OR_NULL, ts) + } } readReceiptSummaries.add(readReceiptsSummary) + summariesByEventId[eventId] = readReceiptsSummary + } + mainReceiptByUserId.forEach { + summariesByEventId[it.value.eventId]?.readReceipts?.add(it.value) } realm.insertOrUpdate(readReceiptSummaries) } @@ -147,8 +161,14 @@ internal class ReadReceiptHandler @Inject constructor( for ((userId, paramsDict) in userIdsDict) { val ts = paramsDict[TIMESTAMP_KEY] as? Double ?: 0.0 val syncedThreadId = paramsDict[THREAD_ID_KEY] as String? - val receiptDestination = syncedThreadId ?: ReadService.THREAD_ID_MAIN - setOf(receiptDestination, syncedThreadId).distinct().forEach { threadId -> + + // SC: fight duplicate read receipts in main timeline + val receiptDestinations = if (syncedThreadId in listOf(null, ReadService.THREAD_ID_MAIN)) { + setOf(syncedThreadId, ReadService.THREAD_ID_MAIN_OR_NULL) + } else { + setOf(syncedThreadId) + } + receiptDestinations.forEach { threadId -> val receiptEntity = ReadReceiptEntity.getOrCreate(realm, roomId, userId, threadId) // ensure new ts is superior to the previous one if (ts > receiptEntity.originServerTs) { 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 cfb99391d7..3be5816d2c 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 @@ -921,6 +921,7 @@ class TimelineFragment : menu.findItem(R.id.dev_membership_changes).isChecked = vectorPreferences.showJoinLeaveMessages() menu.findItem(R.id.dev_display_name_changes).isChecked = vectorPreferences.showAvatarDisplayNameChangeMessages() menu.findItem(R.id.dev_redacted).isChecked = vectorPreferences.showRedactedMessages() + menu.findItem(R.id.dev_duplicated_read_receipts).isChecked = DbgUtil.isDbgEnabled(DbgUtil.DBG_SHOW_DUPLICATE_READ_RECEIPTS) // Composer features menu.findItem(R.id.dev_composer_voice_message_button).isChecked = vectorPreferences.useVoiceMessage() @@ -1042,6 +1043,11 @@ class TimelineFragment : reloadTimeline() true } + R.id.dev_duplicated_read_receipts -> item.toggleExec { shouldShow -> + DbgUtil.onPreferenceChanged(requireContext(), DbgUtil.DBG_SHOW_DUPLICATE_READ_RECEIPTS, shouldShow) + reloadTimeline() + true + } R.id.dev_composer_voice_message_button -> item.toggleExec { enabled -> vectorPreferences.setVoiceMessageButtonEnabled(enabled) requireActivity().restart() 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 1f1b4a5fb4..887433b3ab 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 @@ -25,6 +25,7 @@ import androidx.recyclerview.widget.RecyclerView import com.airbnb.epoxy.EpoxyController import com.airbnb.epoxy.EpoxyModel import com.airbnb.epoxy.VisibilityState +import de.spiritcroc.matrixsdk.util.DbgUtil import im.vector.app.core.date.DateFormatKind import im.vector.app.core.date.VectorDateFormatter import im.vector.app.core.epoxy.LoadingItem_ @@ -59,7 +60,6 @@ import im.vector.app.features.home.room.detail.timeline.item.MessageInformationD import im.vector.app.features.home.room.detail.timeline.item.ReactionsSummaryEvents 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.reply.ReplyPreviewRetriever import im.vector.app.features.home.room.detail.timeline.url.PreviewUrlRetriever import im.vector.app.features.media.AttachmentData @@ -627,8 +627,10 @@ class TimelineEventController @Inject constructor( private fun ReadReceipt.isVisibleInThisThread(): Boolean { return if (partialState.isFromThreadTimeline()) { this.threadId == partialState.rootThreadEventId + } else if (DbgUtil.isDbgEnabled(DbgUtil.DBG_SHOW_DUPLICATE_READ_RECEIPTS)) { + this.threadId in listOf(null, ReadService.THREAD_ID_MAIN, ReadService.THREAD_ID_MAIN_OR_NULL) } else { - this.threadId == ReadService.THREAD_ID_MAIN + this.threadId == ReadService.THREAD_ID_MAIN_OR_NULL } } diff --git a/vector/src/main/java/im/vector/app/features/settings/VectorSettingsScDebuggingFragment.kt b/vector/src/main/java/im/vector/app/features/settings/VectorSettingsScDebuggingFragment.kt index f4f7c63c4a..48ea763812 100644 --- a/vector/src/main/java/im/vector/app/features/settings/VectorSettingsScDebuggingFragment.kt +++ b/vector/src/main/java/im/vector/app/features/settings/VectorSettingsScDebuggingFragment.kt @@ -31,6 +31,7 @@ class VectorSettingsScDebuggingFragment @Inject constructor( DbgPref(DbgUtil.DBG_SHOW_DISPLAY_INDEX, R.string.settings_sc_dbg_show_display_index), DbgPref(DbgUtil.DBG_SHOW_READ_TRACKING, R.string.settings_sc_dbg_show_read_tracking), DbgPref(DbgUtil.DBG_VIEW_PAGER_VISUALS, R.string.settings_sc_dbg_view_pager_visuals), + DbgPref(DbgUtil.DBG_SHOW_DUPLICATE_READ_RECEIPTS, R.string.settings_sc_dbg_show_duplicate_read_receipts), ) val dbgPrefs = dbgLoggingPrefs + dbgVisualsPrefs diff --git a/vector/src/main/res/menu/menu_timeline.xml b/vector/src/main/res/menu/menu_timeline.xml index f7ef814a5a..4a19e19065 100644 --- a/vector/src/main/res/menu/menu_timeline.xml +++ b/vector/src/main/res/menu/menu_timeline.xml @@ -185,6 +185,9 @@ +