From 76065ac4fc981708942a73088c2ad8be9ad58361 Mon Sep 17 00:00:00 2001 From: ganfra Date: Wed, 22 Jan 2020 14:43:39 +0100 Subject: [PATCH] Read: allow setting read marker and read receipt to latest known event independently --- .../api/session/room/read/ReadService.kt | 8 ++- .../session/room/create/CreateRoomTask.kt | 2 +- .../room/membership/joining/JoinRoomTask.kt | 2 +- .../session/room/read/DefaultReadService.kt | 19 ++++++- .../session/room/read/MarkAllRoomsReadTask.kt | 2 +- .../session/room/read/SetReadMarkersTask.kt | 57 +++++++++++-------- .../home/room/detail/RoomDetailViewModel.kt | 4 +- .../NotificationBroadcastReceiver.kt | 3 +- 8 files changed, 64 insertions(+), 33 deletions(-) diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/read/ReadService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/read/ReadService.kt index c9bb6fbf9b..a7995835c5 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/read/ReadService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/api/session/room/read/ReadService.kt @@ -26,10 +26,16 @@ import im.vector.matrix.android.api.util.Optional */ interface ReadService { + enum class MarkAsReadParams{ + READ_RECEIPT, + READ_MARKER, + BOTH + } + /** * Force the read marker to be set on the latest event. */ - fun markAllAsRead(callback: MatrixCallback) + fun markAsRead(params: MarkAsReadParams = MarkAsReadParams.BOTH, callback: MatrixCallback) /** * Set the read receipt on the event with provided eventId. diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/create/CreateRoomTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/create/CreateRoomTask.kt index 6567b7ad97..b4f7bdcd55 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/create/CreateRoomTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/create/CreateRoomTask.kt @@ -88,7 +88,7 @@ internal class DefaultCreateRoomTask @Inject constructor( } private suspend fun setReadMarkers(roomId: String) { - val setReadMarkerParams = SetReadMarkersTask.Params(roomId, markAllAsRead = true) + val setReadMarkerParams = SetReadMarkersTask.Params(roomId, forceReadReceipt = true, forceReadMarker = true) return readMarkersTask.execute(setReadMarkerParams) } } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/membership/joining/JoinRoomTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/membership/joining/JoinRoomTask.kt index d4341951eb..0153930226 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/membership/joining/JoinRoomTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/membership/joining/JoinRoomTask.kt @@ -64,7 +64,7 @@ internal class DefaultJoinRoomTask @Inject constructor( } private suspend fun setReadMarkers(roomId: String) { - val setReadMarkerParams = SetReadMarkersTask.Params(roomId, markAllAsRead = true) + val setReadMarkerParams = SetReadMarkersTask.Params(roomId, forceReadMarker = true, forceReadReceipt = true) readMarkersTask.execute(setReadMarkerParams) } } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/DefaultReadService.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/DefaultReadService.kt index a9a0f60083..36ad2f168f 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/DefaultReadService.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/DefaultReadService.kt @@ -50,10 +50,14 @@ internal class DefaultReadService @AssistedInject constructor( fun create(roomId: String): ReadService } - override fun markAllAsRead(callback: MatrixCallback) { - val params = SetReadMarkersTask.Params(roomId, markAllAsRead = true) + override fun markAsRead(params: ReadService.MarkAsReadParams, callback: MatrixCallback) { + val taskParams = SetReadMarkersTask.Params( + roomId = roomId, + forceReadMarker = params.forceReadMarker(), + forceReadReceipt = params.forceReadReceipt() + ) setReadMarkersTask - .configureWith(params) { + .configureWith(taskParams) { this.callback = callback } .executeBy(taskExecutor) @@ -110,4 +114,13 @@ internal class DefaultReadService @AssistedInject constructor( it.firstOrNull() ?: emptyList() } } + + private fun ReadService.MarkAsReadParams.forceReadMarker(): Boolean { + return this == ReadService.MarkAsReadParams.READ_MARKER || this == ReadService.MarkAsReadParams.BOTH + } + + private fun ReadService.MarkAsReadParams.forceReadReceipt(): Boolean { + return this == ReadService.MarkAsReadParams.READ_RECEIPT || this == ReadService.MarkAsReadParams.BOTH + } + } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/MarkAllRoomsReadTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/MarkAllRoomsReadTask.kt index 99376a981a..b7b63dcdf9 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/MarkAllRoomsReadTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/MarkAllRoomsReadTask.kt @@ -29,7 +29,7 @@ internal class DefaultMarkAllRoomsReadTask @Inject constructor(private val readM override suspend fun execute(params: MarkAllRoomsReadTask.Params) { params.roomIds.forEach { roomId -> - readMarkersTask.execute(SetReadMarkersTask.Params(roomId, markAllAsRead = true)) + readMarkersTask.execute(SetReadMarkersTask.Params(roomId, forceReadMarker = true, forceReadReceipt = true)) } } } diff --git a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/SetReadMarkersTask.kt b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/SetReadMarkersTask.kt index 6a0d04193d..68a5e30a3f 100644 --- a/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/SetReadMarkersTask.kt +++ b/matrix-sdk-android/src/main/java/im/vector/matrix/android/internal/session/room/read/SetReadMarkersTask.kt @@ -25,6 +25,7 @@ import im.vector.matrix.android.internal.database.query.isReadMarkerMoreRecent import im.vector.matrix.android.internal.database.query.latestEvent import im.vector.matrix.android.internal.database.query.where import im.vector.matrix.android.internal.di.UserId +import im.vector.matrix.android.internal.network.NetworkConnectivityChecker import im.vector.matrix.android.internal.network.executeRequest import im.vector.matrix.android.internal.session.room.RoomAPI import im.vector.matrix.android.internal.session.sync.ReadReceiptHandler @@ -35,16 +36,16 @@ import io.realm.Realm import org.greenrobot.eventbus.EventBus import timber.log.Timber import javax.inject.Inject -import kotlin.collections.HashMap import kotlin.collections.set internal interface SetReadMarkersTask : Task { data class Params( val roomId: String, - val markAllAsRead: Boolean = false, val fullyReadEventId: String? = null, - val readReceiptEventId: String? = null + val readReceiptEventId: String? = null, + val forceReadReceipt: Boolean = false, + val forceReadMarker: Boolean = false ) } @@ -57,22 +58,24 @@ internal class DefaultSetReadMarkersTask @Inject constructor( private val roomFullyReadHandler: RoomFullyReadHandler, private val readReceiptHandler: ReadReceiptHandler, @UserId private val userId: String, - private val eventBus: EventBus + private val eventBus: EventBus, + private val networkConnectivityChecker: NetworkConnectivityChecker ) : SetReadMarkersTask { override suspend fun execute(params: SetReadMarkersTask.Params) { val markers = HashMap() - Timber.v("Execute set read marker with params: $params") - val (fullyReadEventId, readReceiptEventId) = if (params.markAllAsRead) { - val latestSyncedEventId = Realm.getInstance(monarchy.realmConfiguration).use { realm -> - TimelineEventEntity.latestEvent(realm, roomId = params.roomId, includesSending = false)?.eventId - } - Pair(latestSyncedEventId, latestSyncedEventId) - } else { - Pair(params.fullyReadEventId, params.readReceiptEventId) + val latestSyncedEventId = latestSyncedEventId(params.roomId) + val fullyReadEventId = if(params.forceReadMarker){ + latestSyncedEventId + }else { + params.fullyReadEventId + } + val readReceiptEventId = if(params.forceReadReceipt){ + latestSyncedEventId + }else { + params.readReceiptEventId } - if (fullyReadEventId != null && !isReadMarkerMoreRecent(monarchy, params.roomId, fullyReadEventId)) { if (LocalEcho.isLocalEchoId(fullyReadEventId)) { Timber.w("Can't set read marker for local event $fullyReadEventId") @@ -80,7 +83,6 @@ internal class DefaultSetReadMarkersTask @Inject constructor( markers[READ_MARKER] = fullyReadEventId } } - if (readReceiptEventId != null && !isEventRead(monarchy, userId, params.roomId, readReceiptEventId)) { if (LocalEcho.isLocalEchoId(readReceiptEventId)) { @@ -89,16 +91,24 @@ internal class DefaultSetReadMarkersTask @Inject constructor( markers[READ_RECEIPT] = readReceiptEventId } } + + val shouldUpdateRoomSummary = readReceiptEventId != null && readReceiptEventId == latestSyncedEventId + updateDatabase(params.roomId, markers, shouldUpdateRoomSummary) if (markers.isEmpty()) { return } - updateDatabase(params.roomId, markers) + networkConnectivityChecker.waitUntilConnected() executeRequest(eventBus) { apiCall = roomAPI.sendReadMarker(params.roomId, markers) } } - private suspend fun updateDatabase(roomId: String, markers: HashMap) { + private fun latestSyncedEventId(roomId: String): String? = + Realm.getInstance(monarchy.realmConfiguration).use { realm -> + TimelineEventEntity.latestEvent(realm, roomId = roomId, includesSending = false)?.eventId + } + + private suspend fun updateDatabase(roomId: String, markers: HashMap, shouldUpdateRoomSummary: Boolean) { monarchy.awaitTransaction { realm -> val readMarkerId = markers[READ_MARKER] val readReceiptId = markers[READ_RECEIPT] @@ -108,14 +118,13 @@ internal class DefaultSetReadMarkersTask @Inject constructor( if (readReceiptId != null) { val readReceiptContent = ReadReceiptHandler.createContent(userId, readReceiptId) readReceiptHandler.handle(realm, roomId, readReceiptContent, false) - val isLatestReceived = TimelineEventEntity.latestEvent(realm, roomId = roomId, includesSending = false)?.eventId == readReceiptId - if (isLatestReceived) { - val roomSummary = RoomSummaryEntity.where(realm, roomId).findFirst() - ?: return@awaitTransaction - roomSummary.notificationCount = 0 - roomSummary.highlightCount = 0 - roomSummary.hasUnreadMessages = false - } + } + if(shouldUpdateRoomSummary){ + val roomSummary = RoomSummaryEntity.where(realm, roomId).findFirst() + ?: return@awaitTransaction + roomSummary.notificationCount = 0 + roomSummary.highlightCount = 0 + roomSummary.hasUnreadMessages = false } } } diff --git a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt index db938c14e5..7d48124c5a 100644 --- a/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt +++ b/vector/src/main/java/im/vector/riotx/features/home/room/detail/RoomDetailViewModel.kt @@ -41,6 +41,7 @@ import im.vector.matrix.android.api.session.room.model.message.MessageContent import im.vector.matrix.android.api.session.room.model.message.MessageType import im.vector.matrix.android.api.session.room.model.message.getFileUrl import im.vector.matrix.android.api.session.room.model.tombstone.RoomTombstoneContent +import im.vector.matrix.android.api.session.room.read.ReadService import im.vector.matrix.android.api.session.room.send.UserDraft import im.vector.matrix.android.api.session.room.timeline.Timeline import im.vector.matrix.android.api.session.room.timeline.TimelineEvent @@ -149,6 +150,7 @@ class RoomDetailViewModel @AssistedInject constructor(@Assisted initialState: Ro observeUnreadState() room.getRoomSummaryLive() room.rx().loadRoomMembersIfNeeded().subscribeLogError().disposeOnClear() + room.markAsRead(ReadService.MarkAsReadParams.READ_RECEIPT, object : MatrixCallback {}) // Inform the SDK that the room is displayed session.onRoomDisplayed(initialState.roomId) } @@ -753,7 +755,7 @@ class RoomDetailViewModel @AssistedInject constructor(@Assisted initialState: Ro } private fun handleMarkAllAsRead() { - room.markAllAsRead(object : MatrixCallback {}) + room.markAsRead(ReadService.MarkAsReadParams.BOTH, object : MatrixCallback {}) } private fun handleReportContent(action: RoomDetailAction.ReportContent) { diff --git a/vector/src/main/java/im/vector/riotx/features/notifications/NotificationBroadcastReceiver.kt b/vector/src/main/java/im/vector/riotx/features/notifications/NotificationBroadcastReceiver.kt index c9dc131b42..816e0dc0ad 100644 --- a/vector/src/main/java/im/vector/riotx/features/notifications/NotificationBroadcastReceiver.kt +++ b/vector/src/main/java/im/vector/riotx/features/notifications/NotificationBroadcastReceiver.kt @@ -23,6 +23,7 @@ import androidx.core.app.RemoteInput import im.vector.matrix.android.api.MatrixCallback import im.vector.matrix.android.api.session.Session import im.vector.matrix.android.api.session.room.Room +import im.vector.matrix.android.api.session.room.read.ReadService import im.vector.riotx.R import im.vector.riotx.core.di.ActiveSessionHolder import im.vector.riotx.core.extensions.vectorComponent @@ -88,7 +89,7 @@ class NotificationBroadcastReceiver : BroadcastReceiver() { private fun handleMarkAsRead(roomId: String) { activeSessionHolder.getActiveSession().let { session -> session.getRoom(roomId) - ?.markAllAsRead(object : MatrixCallback {}) + ?.markAsRead(ReadService.MarkAsReadParams.READ_RECEIPT, object : MatrixCallback {}) } }