Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@ package org.matrix.android.sdk.api.session.room.model

data class ReadReceipt(
val roomMember: RoomMemberSummary,
val originServerTs: Long
val originServerTs: Long,
val threadId: String?
)
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ interface ReadService {
/**
* Force the read marker to be set on the latest event.
*/
suspend fun markAsRead(params: MarkAsReadParams = MarkAsReadParams.BOTH)
suspend fun markAsRead(params: MarkAsReadParams = MarkAsReadParams.BOTH, mainTimeLineOnly: Boolean)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add default value to the new parameters to avoid API breaks.
Also can you document API change in a .sdk file for towncrier? Thanks.


/**
* Set the read receipt on the event with provided eventId.
*/
suspend fun setReadReceipt(eventId: String)
suspend fun setReadReceipt(eventId: String, threadId: String)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add Kdoc for parameters, especially tell about the possible value THREAD_ID_MAIN for threadId.


/**
* Set the read marker on the event with provided eventId.
Expand All @@ -62,7 +62,7 @@ interface ReadService {
fun getMyReadReceiptLive(): LiveData<Optional<String>>

/**
* Get the eventId where the read receipt for the provided user is.
* Get the eventId from the main timeline where the read receipt for the provided user is.
* @param userId the id of the user to look for
*
* @return the eventId where the read receipt for the provided user is attached, or null if not found
Expand All @@ -74,4 +74,8 @@ interface ReadService {
* @param eventId the event
*/
fun getEventReadReceiptsLive(eventId: String): LiveData<List<ReadReceipt>>

companion object {
const val THREAD_ID_MAIN = "main"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ 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)
val readReceiptOfSender = ReadReceiptEntity.getOrCreate(realm, roomId = roomId, userId = senderId, threadId = eventEntity.rootThreadEventId)
// If the synced RR is older, update
if (timestampOfEvent > readReceiptOfSender.originServerTs) {
val previousReceiptsSummary = ReadReceiptsSummaryEntity.where(realm, eventId = readReceiptOfSender.eventId).findFirst()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import io.realm.RealmQuery
import io.realm.Sort
import org.matrix.android.sdk.api.session.events.model.UnsignedData
import org.matrix.android.sdk.api.session.events.model.isRedacted
import org.matrix.android.sdk.api.session.room.model.ReadReceipt
import org.matrix.android.sdk.api.session.room.read.ReadService
import org.matrix.android.sdk.api.session.room.timeline.TimelineEvent
import org.matrix.android.sdk.api.session.threads.ThreadNotificationState
import org.matrix.android.sdk.internal.database.mapper.asDomain
Expand Down Expand Up @@ -273,8 +275,8 @@ internal fun TimelineEventEntity.Companion.isUserMentionedInThread(realm: Realm,
/**
* Find the read receipt for the current user.
*/
internal fun findMyReadReceipt(realm: Realm, roomId: String, userId: String): String? =
ReadReceiptEntity.where(realm, roomId = roomId, userId = userId)
internal fun findMyReadReceipt(realm: Realm, roomId: String, userId: String, threadId: String?): String? =
ReadReceiptEntity.where(realm, roomId = roomId, userId = userId, threadId = threadId)
.findFirst()
?.eventId

Expand All @@ -294,7 +296,7 @@ internal fun isUserMentioned(currentUserId: String, timelineEventEntity: Timelin
* immediately so we should not display wrong notifications
*/
internal fun updateNotificationsNew(roomId: String, realm: Realm, currentUserId: String) {
val readReceipt = findMyReadReceipt(realm, roomId, currentUserId) ?: return
val readReceipt = findMyReadReceipt(realm, roomId, currentUserId, ReadService.THREAD_ID_MAIN) ?: return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use null instead of introducing const ReadService.THREAD_ID_MAIN. Not sure it's better though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null and main values for threadId in read receipt are different cases so we can't use null here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After looking deeper at this method I've realised that it needs to be changed, so it will use exact threadId instead of main


val readReceiptChunk = ChunkEntity
.findIncludingEvent(realm, readReceipt) ?: return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ internal class ReadReceiptsSummaryMapper @Inject constructor(
.mapNotNull {
val roomMember = RoomMemberSummaryEntity.where(realm, roomId = it.roomId, userId = it.userId).findFirst()
?: return@mapNotNull null
ReadReceipt(roomMember.asDomain(), it.originServerTs.toLong())
ReadReceipt(roomMember.asDomain(), it.originServerTs.toLong(), it.threadId)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ internal open class ReadReceiptEntity(
var eventId: String = "",
var roomId: String = "",
var userId: String = "",
var threadId: String? = null,
var originServerTs: Double = 0.0
) : RealmObject() {
companion object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import io.realm.RealmObject
import io.realm.RealmResults
import io.realm.annotations.Index
import io.realm.annotations.LinkingObjects
import org.matrix.android.sdk.api.session.room.read.ReadService
import org.matrix.android.sdk.internal.extensions.assertIsManaged

internal open class TimelineEventEntity(
Expand Down Expand Up @@ -52,3 +53,7 @@ internal fun TimelineEventEntity.deleteOnCascade(canDeleteRoot: Boolean) {
}
deleteFromRealm()
}

internal fun TimelineEventEntity.getThreadId(): String {
return root?.rootThreadEventId ?: ReadService.THREAD_ID_MAIN
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,14 @@ import org.matrix.android.sdk.internal.database.model.ChunkEntity
import org.matrix.android.sdk.internal.database.model.ReadMarkerEntity
import org.matrix.android.sdk.internal.database.model.ReadReceiptEntity
import org.matrix.android.sdk.internal.database.model.TimelineEventEntity
import org.matrix.android.sdk.internal.database.model.getThreadId

internal fun isEventRead(
realmConfiguration: RealmConfiguration,
userId: String?,
roomId: String?,
eventId: String?
eventId: String?,
shouldCheckIfReadInEventsThread: Boolean
): Boolean {
if (userId.isNullOrBlank() || roomId.isNullOrBlank() || eventId.isNullOrBlank()) {
return false
Expand All @@ -45,7 +47,8 @@ internal fun isEventRead(
eventToCheck.root?.sender == userId -> true
// If new event exists and the latest event is from ourselves we can infer the event is read
latestEventIsFromSelf(realm, roomId, userId) -> true
eventToCheck.isBeforeLatestReadReceipt(realm, roomId, userId) -> true
eventToCheck.isBeforeLatestReadReceipt(realm, roomId, userId, null) -> true
(shouldCheckIfReadInEventsThread && eventToCheck.isBeforeLatestReadReceipt(realm, roomId, userId, eventToCheck.getThreadId())) -> true
else -> false
}
}
Expand All @@ -54,11 +57,12 @@ internal fun isEventRead(
private fun latestEventIsFromSelf(realm: Realm, roomId: String, userId: String) = TimelineEventEntity.latestEvent(realm, roomId, true)
?.root?.sender == userId

private fun TimelineEventEntity.isBeforeLatestReadReceipt(realm: Realm, roomId: String, userId: String): Boolean {
return ReadReceiptEntity.where(realm, roomId, userId).findFirst()?.let { readReceipt ->
private fun TimelineEventEntity.isBeforeLatestReadReceipt(realm: Realm, roomId: String, userId: String, threadId: String?): Boolean {
val isMoreRecent = ReadReceiptEntity.where(realm, roomId, userId, threadId).findFirst()?.let { readReceipt ->
val readReceiptEvent = TimelineEventEntity.where(realm, roomId, readReceipt.eventId).findFirst()
readReceiptEvent?.isMoreRecentThan(this)
} ?: false
return isMoreRecent
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,26 @@ import io.realm.Realm
import io.realm.RealmQuery
import io.realm.kotlin.createObject
import io.realm.kotlin.where
import org.matrix.android.sdk.api.session.room.read.ReadService
import org.matrix.android.sdk.internal.database.model.ReadReceiptEntity
import org.matrix.android.sdk.internal.database.model.ReadReceiptEntityFields

internal fun ReadReceiptEntity.Companion.where(realm: Realm, roomId: String, userId: String, threadId: String?): RealmQuery<ReadReceiptEntity> {
return realm.where<ReadReceiptEntity>()
.equalTo(ReadReceiptEntityFields.PRIMARY_KEY, buildPrimaryKey(roomId, userId, threadId))
}

internal fun ReadReceiptEntity.Companion.forMainTimelineWhere(realm: Realm, roomId: String, userId: String): RealmQuery<ReadReceiptEntity> {
return realm.where<ReadReceiptEntity>()
.equalTo(ReadReceiptEntityFields.PRIMARY_KEY, buildPrimaryKey(roomId, userId, ReadService.THREAD_ID_MAIN))
.or()
.equalTo(ReadReceiptEntityFields.PRIMARY_KEY, buildPrimaryKey(roomId, userId, null))
}

internal fun ReadReceiptEntity.Companion.where(realm: Realm, roomId: String, userId: String): RealmQuery<ReadReceiptEntity> {
return realm.where<ReadReceiptEntity>()
.equalTo(ReadReceiptEntityFields.PRIMARY_KEY, buildPrimaryKey(roomId, userId))
.equalTo(ReadReceiptEntityFields.USER_ID, userId)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe make this fun private to avoid any mistake. I understand that only the 2 functions above have to be used now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've found some usages of this method, but after some closer look it appeared that these cases should use threadId also, so now there is no usages and I'll rather remove this method completely. Or is there a reason to keep it and make private even if it's not used?

.equalTo(ReadReceiptEntityFields.ROOM_ID, roomId)
}

internal fun ReadReceiptEntity.Companion.whereUserId(realm: Realm, userId: String): RealmQuery<ReadReceiptEntity> {
Expand All @@ -38,23 +52,25 @@ internal fun ReadReceiptEntity.Companion.whereRoomId(realm: Realm, roomId: Strin
.equalTo(ReadReceiptEntityFields.ROOM_ID, roomId)
}

internal fun ReadReceiptEntity.Companion.createUnmanaged(roomId: String, eventId: String, userId: String, originServerTs: Double): ReadReceiptEntity {
internal fun ReadReceiptEntity.Companion.createUnmanaged(roomId: String, eventId: String, userId: String, threadId: String?, originServerTs: Double): ReadReceiptEntity {
return ReadReceiptEntity().apply {
this.primaryKey = "${roomId}_$userId"
this.primaryKey = buildPrimaryKey(roomId, userId, threadId)
this.eventId = eventId
this.roomId = roomId
this.userId = userId
this.threadId = threadId
this.originServerTs = originServerTs
}
}

internal fun ReadReceiptEntity.Companion.getOrCreate(realm: Realm, roomId: String, userId: String): ReadReceiptEntity {
return ReadReceiptEntity.where(realm, roomId, userId).findFirst()
?: realm.createObject<ReadReceiptEntity>(buildPrimaryKey(roomId, userId))
internal fun ReadReceiptEntity.Companion.getOrCreate(realm: Realm, roomId: String, userId: String, threadId: String?): ReadReceiptEntity {
return ReadReceiptEntity.where(realm, roomId, userId, threadId).findFirst()
?: realm.createObject<ReadReceiptEntity>(buildPrimaryKey(roomId, userId, threadId))
.apply {
this.roomId = roomId
this.userId = userId
this.threadId = threadId
}
}

private fun buildPrimaryKey(roomId: String, userId: String) = "${roomId}_$userId"
private fun buildPrimaryKey(roomId: String, userId: String, threadId: String?) = "${roomId}_${userId}_${threadId ?: "null"}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For compatibility I would have written

Suggested change
private fun buildPrimaryKey(roomId: String, userId: String, threadId: String?) = "${roomId}_${userId}_${threadId ?: "null"}"
private fun buildPrimaryKey(roomId: String, userId: String, threadId: String?): String {
return if (threadId == null) {
"${roomId}_${userId}"
} else {
"${roomId}_${userId}_${threadId}"
}

Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import org.matrix.android.sdk.internal.session.room.membership.RoomMembersRespon
import org.matrix.android.sdk.internal.session.room.membership.admin.UserIdAndReason
import org.matrix.android.sdk.internal.session.room.membership.joining.InviteBody
import org.matrix.android.sdk.internal.session.room.membership.threepid.ThreePidInviteBody
import org.matrix.android.sdk.internal.session.room.read.ReadBody
import org.matrix.android.sdk.internal.session.room.relation.RelationsResponse
import org.matrix.android.sdk.internal.session.room.reporting.ReportContentBody
import org.matrix.android.sdk.internal.session.room.send.SendResponse
Expand Down Expand Up @@ -173,7 +174,7 @@ internal interface RoomAPI {
@Path("roomId") roomId: String,
@Path("receiptType") receiptType: String,
@Path("eventId") eventId: String,
@Body body: JsonDict = emptyMap()
@Body body: ReadBody
)

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.zhuinden.monarchy.Monarchy
import dagger.assisted.Assisted
import dagger.assisted.AssistedFactory
import dagger.assisted.AssistedInject
import org.matrix.android.sdk.api.session.homeserver.HomeServerCapabilitiesService
import org.matrix.android.sdk.api.session.room.model.ReadReceipt
import org.matrix.android.sdk.api.session.room.read.ReadService
import org.matrix.android.sdk.api.util.Optional
Expand All @@ -30,6 +31,7 @@ import org.matrix.android.sdk.internal.database.mapper.ReadReceiptsSummaryMapper
import org.matrix.android.sdk.internal.database.model.ReadMarkerEntity
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.forMainTimelineWhere
import org.matrix.android.sdk.internal.database.query.isEventRead
import org.matrix.android.sdk.internal.database.query.where
import org.matrix.android.sdk.internal.di.SessionDatabase
Expand All @@ -40,25 +42,37 @@ internal class DefaultReadService @AssistedInject constructor(
@SessionDatabase private val monarchy: Monarchy,
private val setReadMarkersTask: SetReadMarkersTask,
private val readReceiptsSummaryMapper: ReadReceiptsSummaryMapper,
@UserId private val userId: String
@UserId private val userId: String,
private val homeServerCapabilitiesService: HomeServerCapabilitiesService,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to inject HomeServerCapabilitiesDataSource here.

) : ReadService {

@AssistedFactory
interface Factory {
fun create(roomId: String): DefaultReadService
}

override suspend fun markAsRead(params: ReadService.MarkAsReadParams) {
override suspend fun markAsRead(params: ReadService.MarkAsReadParams, mainTimeLineOnly: Boolean) {
val readReceiptThreadId = if (homeServerCapabilitiesService.getHomeServerCapabilities().canUseThreadReadReceiptsAndNotifications) {
if (mainTimeLineOnly) ReadService.THREAD_ID_MAIN else null
} else {
null
}
val taskParams = SetReadMarkersTask.Params(
roomId = roomId,
forceReadMarker = params.forceReadMarker(),
forceReadReceipt = params.forceReadReceipt()
forceReadReceipt = params.forceReadReceipt(),
readReceiptThreadId = readReceiptThreadId
)
setReadMarkersTask.execute(taskParams)
}

override suspend fun setReadReceipt(eventId: String) {
val params = SetReadMarkersTask.Params(roomId, fullyReadEventId = null, readReceiptEventId = eventId)
override suspend fun setReadReceipt(eventId: String, threadId: String) {
val readReceiptThreadId = if (homeServerCapabilitiesService.getHomeServerCapabilities().canUseThreadReadReceiptsAndNotifications) {
threadId
} else {
null
}
val params = SetReadMarkersTask.Params(roomId, fullyReadEventId = null, readReceiptEventId = eventId, readReceiptThreadId = readReceiptThreadId)
setReadMarkersTask.execute(params)
}

Expand All @@ -68,7 +82,8 @@ internal class DefaultReadService @AssistedInject constructor(
}

override fun isEventRead(eventId: String): Boolean {
return isEventRead(monarchy.realmConfiguration, userId, roomId, eventId)
val shouldCheckIfReadInEventsThread = homeServerCapabilitiesService.getHomeServerCapabilities().canUseThreadReadReceiptsAndNotifications
return isEventRead(monarchy.realmConfiguration, userId, roomId, eventId, shouldCheckIfReadInEventsThread)
}

override fun getReadMarkerLive(): LiveData<Optional<String>> {
Expand All @@ -94,10 +109,11 @@ internal class DefaultReadService @AssistedInject constructor(
override fun getUserReadReceipt(userId: String): String? {
var eventId: String? = null
monarchy.doWithRealm {
eventId = ReadReceiptEntity.where(it, roomId = roomId, userId = userId)
eventId = ReadReceiptEntity.forMainTimelineWhere(it, roomId = roomId, userId = userId)
.findFirst()
?.eventId
}

return eventId
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright 2020 The Matrix.org Foundation C.I.C.
*
* 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 org.matrix.android.sdk.internal.session.room.read

import com.squareup.moshi.Json
import com.squareup.moshi.JsonClass

@JsonClass(generateAdapter = true)
internal data class ReadBody(
@Json(name = "thread_id") val threadId: String?,
)
Loading