From 76d1c81495f0a37bfd77442a551e9e368e718d42 Mon Sep 17 00:00:00 2001 From: SpiritCroc Date: Sun, 2 Apr 2023 12:45:36 +0200 Subject: [PATCH] [TEST] Restructure RR deduplication logic and avoid threads We probably should add an extra check to only set main_or_null to events which actually exist in the main thread Change-Id: I8db0010e8717b6941917225fcce6646685814665 --- .../database/helper/ChunkEntityHelper.kt | 20 +++++++++++++------ .../sync/handler/room/ReadReceiptHandler.kt | 20 +++++++++++++------ 2 files changed, 28 insertions(+), 12 deletions(-) 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 03a71ab376..f3509dc8f7 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 @@ -147,20 +147,28 @@ private fun handleReadReceipts(realm: Realm, roomId: String, eventEntity: EventE receiptDestinations.forEach { rootThreadEventId -> val readReceiptOfSender = ReadReceiptEntity.getOrCreate(realm, roomId = roomId, userId = senderId, threadId = rootThreadEventId) val shouldForceMon: Boolean - val shouldSkipMon = if (rootThreadEventId == THREAD_ID_MAIN_OR_NULL) { + val shouldSkipMon: Boolean + if (rootThreadEventId == THREAD_ID_MAIN_OR_NULL) { val previousReceiptsSummary = ReadReceiptsSummaryEntity.where(realm, eventId = readReceiptOfSender.eventId).findFirst() val oldEventTs = previousReceiptsSummary?.let { EventEntity.where(realm, roomId, it.eventId).findFirst()?.originServerTs } - val newEventTs = EventEntity.where(realm, roomId, eventEntity.eventId).findFirst()?.originServerTs - shouldForceMon = oldEventTs != null && newEventTs != null && oldEventTs < newEventTs - oldEventTs != null && newEventTs != null && oldEventTs > newEventTs + val newEvent = EventEntity.where(realm, roomId, eventEntity.eventId).findFirst() + val newEventTs = newEvent?.originServerTs + // Avoid setting MAIN_OR_NULL to an event which only exists in the combined null-timeline, but not in the actual main-timeline + if (eventEntity.rootThreadEventId != THREAD_ID_MAIN && newEvent?.rootThreadEventId != null) { + shouldForceMon = false + shouldSkipMon = true + } else { + shouldForceMon = oldEventTs != null && newEventTs != null && oldEventTs < newEventTs + shouldSkipMon = oldEventTs != null && newEventTs != null && oldEventTs > newEventTs + } } else { shouldForceMon = false - false + shouldSkipMon = false } // If the synced RR is older, update if (shouldForceMon || (timestampOfEvent > readReceiptOfSender.originServerTs && !shouldSkipMon)) { val previousReceiptsSummary = ReadReceiptsSummaryEntity.where(realm, eventId = readReceiptOfSender.eventId).findFirst() - rrDimber.i { "Handle outdated chunk RR $roomId / $senderId thread $rootThreadEventId(${eventEntity.rootThreadEventId}): event ${readReceiptOfSender.eventId} -> ${eventEntity.eventId}" } + rrDimber.i { "Handle outdated chunk RR $roomId / $senderId thread $rootThreadEventId(${eventEntity.rootThreadEventId}): event ${readReceiptOfSender.eventId} -> ${eventEntity.eventId} || force $shouldForceMon legacy ${timestampOfEvent > readReceiptOfSender.originServerTs}" } readReceiptOfSender.eventId = eventEntity.eventId readReceiptOfSender.originServerTs = timestampOfEvent previousReceiptsSummary?.readReceipts?.remove(readReceiptOfSender) 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 d63cedb5c9..b45c278cea 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 @@ -173,18 +173,26 @@ internal class ReadReceiptHandler @Inject constructor( receiptDestinations.forEach { threadId -> val receiptEntity = ReadReceiptEntity.getOrCreate(realm, roomId, userId, threadId) val shouldForceMon: Boolean - val shouldSkipMon = if (threadId == THREAD_ID_MAIN_OR_NULL) { + val shouldSkipMon: Boolean + if (threadId == THREAD_ID_MAIN_OR_NULL) { val oldEventTs = EventEntity.where(realm, roomId, receiptEntity.eventId).findFirst()?.originServerTs - val newEventTs = EventEntity.where(realm, roomId, eventId).findFirst()?.originServerTs - shouldForceMon = oldEventTs != null && newEventTs != null && oldEventTs < newEventTs - oldEventTs != null && newEventTs != null && oldEventTs > newEventTs + val newEvent = EventEntity.where(realm, roomId, eventId).findFirst() + val newEventTs = newEvent?.originServerTs + // Avoid setting MAIN_OR_NULL to an event which only exists in the combined null-timeline, but not in the actual main-timeline + if (syncedThreadId != THREAD_ID_MAIN && newEvent?.rootThreadEventId != null) { + shouldForceMon = false + shouldSkipMon = true + } else { + shouldForceMon = oldEventTs != null && newEventTs != null && oldEventTs < newEventTs + shouldSkipMon = oldEventTs != null && newEventTs != null && oldEventTs > newEventTs + } } else { shouldForceMon = false - false + shouldSkipMon = false } // ensure new ts is superior to the previous one if (shouldForceMon || (ts > receiptEntity.originServerTs && !shouldSkipMon)) { - rrDimber.i { "Handle outdated sync RR $roomId / $userId thread $threadId($syncedThreadId): event ${receiptEntity.eventId} -> $eventId" } + rrDimber.i { "Handle outdated sync RR $roomId / $userId thread $threadId($syncedThreadId): event ${receiptEntity.eventId} -> $eventId || force $shouldForceMon legacy ${ts > receiptEntity.originServerTs}" } ReadReceiptsSummaryEntity.where(realm, receiptEntity.eventId).findFirst()?.also { it.readReceipts.remove(receiptEntity) }