Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update Account Data with user matrix id for invited user by email #3743

Merged
merged 4 commits into from
Aug 23, 2021
Merged

Update Account Data with user matrix id for invited user by email #3743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6d47fdf
Rename getLocalUserAccount to getLocalDirectMessages
Frenaud14 Jul 26, 2021
ac56b1e
Update account data for invited users by email
Frenaud14 Jul 28, 2021
a399747
change
Frenaud14 Jul 29, 2021
0a08746
Set DirectMessagesContent immutable
Frenaud14 Jul 30, 2021
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
1 change: 1 addition & 0 deletions changelog.d/3743.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Update the AccountData with the users' matrix Id instead of their email for those invited by email in a direct chat
2 changes: 1 addition & 1 deletion ...droid/src/main/java/org/matrix/android/sdk/internal/session/room/create/CreateRoomTask.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ internal class DefaultCreateRoomTask @Inject constructor(
this.isDirect = true
}
}
val directChats = directChatsHelper.getLocalUserAccount()
val directChats = directChatsHelper.getLocalDirectMessages()
updateUserAccountDataTask.execute(UpdateUserAccountDataTask.DirectChatParams(directMessages = directChats))
}

Expand Down
24 changes: 20 additions & 4 deletions ...in/java/org/matrix/android/sdk/internal/session/room/membership/RoomMemberEventHandler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,30 @@ import io.realm.Realm
import org.matrix.android.sdk.api.session.events.model.Event
import org.matrix.android.sdk.api.session.events.model.EventType
import org.matrix.android.sdk.api.session.room.model.RoomMemberContent
import org.matrix.android.sdk.internal.di.UserId
import org.matrix.android.sdk.internal.session.events.getFixedRoomMemberContent
import org.matrix.android.sdk.internal.session.sync.SyncResponsePostTreatmentAggregator
import org.matrix.android.sdk.internal.session.user.UserEntityFactory
import javax.inject.Inject

internal class RoomMemberEventHandler @Inject constructor() {
internal class RoomMemberEventHandler @Inject constructor(
@UserId private val myUserId: String
) {

fun handle(realm: Realm, roomId: String, event: Event): Boolean {
fun handle(realm: Realm, roomId: String, event: Event, aggregator: SyncResponsePostTreatmentAggregator? = null): Boolean {
if (event.type != EventType.STATE_ROOM_MEMBER) {
return false
}
val userId = event.stateKey ?: return false
val roomMember = event.getFixedRoomMemberContent()
return handle(realm, roomId, userId, roomMember)
return handle(realm, roomId, userId, roomMember, aggregator)
}

fun handle(realm: Realm, roomId: String, userId: String, roomMember: RoomMemberContent?): Boolean {
fun handle(realm: Realm,
roomId: String,
userId: String,
roomMember: RoomMemberContent?,
aggregator: SyncResponsePostTreatmentAggregator? = null): Boolean {
if (roomMember == null) {
return false
}
Expand All @@ -45,6 +53,14 @@ internal class RoomMemberEventHandler @Inject constructor() {
val userEntity = UserEntityFactory.create(userId, roomMember)
realm.insertOrUpdate(userEntity)
}

// check whether this new room member event may be used to update the directs dictionary in account data
// this is required to handle correctly invite by email in DM
val mxId = roomMember.thirdPartyInvite?.signed?.mxid
if (mxId != null && mxId != myUserId) {
aggregator?.directChatsToCheck?.put(roomId, mxId)
}

return true
}
}
10 changes: 6 additions & 4 deletions ...sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/RoomSyncHandler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle
}
// Give info to crypto module
cryptoService.onStateEvent(roomId, event)
roomMemberEventHandler.handle(realm, roomId, event)
roomMemberEventHandler.handle(realm, roomId, event, aggregator)
}
}
if (roomSync.timeline?.events?.isNotEmpty() == true) {
Expand All @@ -233,7 +233,8 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle
roomSync.timeline.prevToken,
roomSync.timeline.limited,
insertType,
syncLocalTimestampMillis
syncLocalTimestampMillis,
aggregator
)
roomEntity.addIfNecessary(chunkEntity)
}
Expand Down Expand Up @@ -337,7 +338,8 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle
prevToken: String? = null,
isLimited: Boolean = true,
insertType: EventInsertType,
syncLocalTimestampMillis: Long): ChunkEntity {
syncLocalTimestampMillis: Long,
aggregator: SyncResponsePostTreatmentAggregator): ChunkEntity {
val lastChunk = ChunkEntity.findLastForwardChunkOfRoom(realm, roomEntity.roomId)
val chunkEntity = if (!isLimited && lastChunk != null) {
lastChunk
Expand Down Expand Up @@ -371,7 +373,7 @@ internal class RoomSyncHandler @Inject constructor(private val readReceiptHandle
if (event.type == EventType.STATE_ROOM_MEMBER) {
val fixedContent = event.getFixedRoomMemberContent()
roomMemberContentsByUser[event.stateKey] = fixedContent
roomMemberEventHandler.handle(realm, roomEntity.roomId, event.stateKey, fixedContent)
roomMemberEventHandler.handle(realm, roomEntity.roomId, event.stateKey, fixedContent, aggregator)
}
}
roomMemberContentsByUser.getOrPut(event.senderId) {
Expand Down
2 changes: 2 additions & 0 deletions .../java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,6 @@ package org.matrix.android.sdk.internal.session.sync
internal class SyncResponsePostTreatmentAggregator {
// List of RoomId
val ephemeralFilesToDelete = mutableListOf<String>()
// Map of roomId to directUserId
val directChatsToCheck = mutableMapOf<String, String>()
}
40 changes: 38 additions & 2 deletions ...rg/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregatorHandler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,54 @@

package org.matrix.android.sdk.internal.session.sync

import org.matrix.android.sdk.api.MatrixPatterns
import org.matrix.android.sdk.internal.session.sync.model.accountdata.toMutable
import org.matrix.android.sdk.internal.session.user.accountdata.DirectChatsHelper
import org.matrix.android.sdk.internal.session.user.accountdata.UpdateUserAccountDataTask
import javax.inject.Inject

internal class SyncResponsePostTreatmentAggregatorHandler @Inject constructor(
private val ephemeralTemporaryStore: RoomSyncEphemeralTemporaryStore
private val directChatsHelper: DirectChatsHelper,
private val ephemeralTemporaryStore: RoomSyncEphemeralTemporaryStore,
private val updateUserAccountDataTask: UpdateUserAccountDataTask
) {
fun handle(synResHaResponsePostTreatmentAggregator: SyncResponsePostTreatmentAggregator) {
suspend fun handle(synResHaResponsePostTreatmentAggregator: SyncResponsePostTreatmentAggregator) {
cleanupEphemeralFiles(synResHaResponsePostTreatmentAggregator.ephemeralFilesToDelete)
updateDirectUserIds(synResHaResponsePostTreatmentAggregator.directChatsToCheck)
}

private fun cleanupEphemeralFiles(ephemeralFilesToDelete: List<String>) {
ephemeralFilesToDelete.forEach {
ephemeralTemporaryStore.delete(it)
}
}

private suspend fun updateDirectUserIds(directUserIdsToUpdate: Map<String, String>) {
val directChats = directChatsHelper.getLocalDirectMessages().toMutable()
var hasUpdate = false
directUserIdsToUpdate.forEach { (roomId, candidateUserId) ->
// consider room is a DM if referenced in the DM dictionary
val currentDirectUserId = directChats.firstNotNullOfOrNull { (userId, roomIds) -> userId.takeIf { roomId in roomIds } }
// update directUserId with the given candidateUserId if it mismatches the current one
if (currentDirectUserId != null && !MatrixPatterns.isUserId(currentDirectUserId)) {
// link roomId with the matrix id
directChats
.getOrPut(candidateUserId) { arrayListOf() }
.apply {
if (!contains(roomId)) {
hasUpdate = true
add(roomId)
}
}

// remove roomId from currentDirectUserId entry
hasUpdate = hasUpdate or(directChats[currentDirectUserId]?.remove(roomId) == true)
Copy link
Member

@bmarty bmarty Jul 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

is it necessary to assign hasUpdate 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.

Yes, the remove method will modify the direct chat dictionary if the given item has been found and removed, so I want to edit this flag in that case.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, but I was wondering if hasUpdate was not already equal to true in this case. It can maybe add some cleanup on some existing account data.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I think I understand 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'm not sure to understand, I use the or function which will always call the second part of the operation (whether hasUpdate is true or not). The flag should be updated for each modification in the direct chats dictionary. In our case, the cleanup will be performed only for members related to the given directUserIdsToUpdate dictionary.

// remove currentDirectUserId entry if there is no attached room anymore
hasUpdate = hasUpdate or(directChats.takeIf { it[currentDirectUserId].isNullOrEmpty() }?.remove(currentDirectUserId) != null)
Copy link
Member

Choose a reason for hiding this comment

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

Same remark

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same answer, If I remove a key from the directChats dictionary, I also want to edit this flag in order to update the accountData with the updated dictionary

Copy link
Member

Choose a reason for hiding this comment

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

!= null could be replaced by == true, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nop, in that case, remove returns the removed object, if any or null otherwise

}
}
if (hasUpdate) {
updateUserAccountDataTask.execute(UpdateUserAccountDataTask.DirectChatParams(directMessages = directChats))
}
}
}
3 changes: 2 additions & 1 deletion .../src/main/java/org/matrix/android/sdk/internal/session/sync/UserAccountDataSyncHandler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import org.matrix.android.sdk.internal.session.sync.model.accountdata.Breadcrumb
import org.matrix.android.sdk.internal.session.sync.model.accountdata.DirectMessagesContent
import org.matrix.android.sdk.internal.session.sync.model.accountdata.IgnoredUsersContent
import org.matrix.android.sdk.internal.session.sync.model.accountdata.UserAccountDataSync
import org.matrix.android.sdk.internal.session.sync.model.accountdata.toMutable
import org.matrix.android.sdk.internal.session.user.accountdata.DirectChatsHelper
import org.matrix.android.sdk.internal.session.user.accountdata.UpdateUserAccountDataTask
import timber.log.Timber
Expand Down Expand Up @@ -83,7 +84,7 @@ internal class UserAccountDataSyncHandler @Inject constructor(
// If we get some direct chat invites, we synchronize the user account data including those.
suspend fun synchronizeWithServerIfNeeded(invites: Map<String, InvitedRoomSync>) {
if (invites.isNullOrEmpty()) return
val directChats = directChatsHelper.getLocalUserAccount()
val directChats = directChatsHelper.getLocalDirectMessages().toMutable()
var hasUpdate = false
monarchy.doWithRealm { realm ->
invites.forEach { (roomId, _) ->
Expand Down
9 changes: 9 additions & 0 deletions ...a/org/matrix/android/sdk/internal/session/sync/model/accountdata/DirectMessagesContent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,12 @@ package org.matrix.android.sdk.internal.session.sync.model.accountdata
* Keys are userIds, values are list of roomIds
*/
internal typealias DirectMessagesContent = Map<String, List<String>>

/**
* Returns a new [MutableMap] with all elements of this collection.
*/
internal fun DirectMessagesContent.toMutable(): MutableMap<String, MutableList<String>> {
return map { it.key to it.value.toMutableList() }
.toMap()
.toMutableMap()
}
3 changes: 2 additions & 1 deletion ...c/main/java/org/matrix/android/sdk/internal/session/user/accountdata/DirectChatsHelper.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import org.matrix.android.sdk.internal.database.query.getDirectRooms
import org.matrix.android.sdk.internal.di.SessionDatabase
import io.realm.Realm
import io.realm.RealmConfiguration
import org.matrix.android.sdk.internal.session.sync.model.accountdata.DirectMessagesContent
import javax.inject.Inject

internal class DirectChatsHelper @Inject constructor(@SessionDatabase
Expand All @@ -29,7 +30,7 @@ internal class DirectChatsHelper @Inject constructor(@SessionDatabase
/**
* @return a map of userId <-> list of roomId
*/
fun getLocalUserAccount(filterRoomId: String? = null): MutableMap<String, MutableList<String>> {
fun getLocalDirectMessages(filterRoomId: String? = null): DirectMessagesContent {
return Realm.getInstance(realmConfiguration).use { realm ->
// Makes sure we have the latest realm updates, this is important as we sent this information to the server.
realm.refresh()
Expand Down