[TESTING] Fight duplicate read markers in main timeline, pt.2

- Make my read-marker experiments backwards-compatible, by introducing a
  new artificial marker
- Fix sometimes the marker going missing, by not deduplicating read
  markers, then filtering out the other one because duplicates didn't
  respect the thread id
Needs an initial sync to fully apply.

Change-Id: Id02ae19d03077016cbeb8d9a8fd5130d77931b2d

library/ui-strings/src/main/res/values/strings_sc.xml

@@ -167,6 +167,7 @@
     <string name="settings_sc_dbg_read_receipts">Read receipt debugging</string>
     <string name="settings_sc_dbg_view_pager">ViewPager debugging</string>
     <string name="settings_sc_dbg_view_pager_visuals">Show ViewPager debugging information</string>
+    <string name="settings_sc_dbg_show_duplicate_read_receipts">Duplicate read receipts</string>
 
     <!-- SC labs -->
     <string name="settings_upstream_labs">Element features</string>

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<String, Boolean>()
 
@@ -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) {

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"
     }
 }

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
+        // SC: fight duplicate read receipts in main timeline
-        val receiptDestination = eventEntity.rootThreadEventId ?: THREAD_ID_MAIN
+        val receiptDestinations = if (eventEntity.rootThreadEventId in listOf(null, THREAD_ID_MAIN)) {
-        setOf(receiptDestination, eventEntity.rootThreadEventId).distinct().forEach { rootThreadEventId ->
+            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) {

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()

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<ReadReceiptsSummaryEntity>()
+        // SC: fight duplicate read markers
+        val summariesByEventId = HashMap<String, ReadReceiptsSummaryEntity>()
+        val mainReceiptByUserId = HashMap<String, ReadReceiptEntity>()
         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) {

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()

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
         }
     }
 

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
 

vector/src/main/res/menu/menu_timeline.xml

@@ -185,6 +185,9 @@
                 <item
                     android:id="@+id/dev_redacted"
                     android:title="@string/dev_tools_menu_redacted" />
+                <item
+                    android:id="@+id/dev_duplicated_read_receipts"
+                    android:title="@string/settings_sc_dbg_show_duplicate_read_receipts" />
             </group>
         </menu>
     </item>