From 13f7a9fc10df57b668f92607c5a02bfaef3946c4 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 22 Aug 2022 17:44:07 +0200 Subject: [PATCH 01/11] Performance: invoke UpdateTrustWorker only once per incremental sync. --- .../session/room/summary/RoomSummaryUpdater.kt | 14 +++++++++++--- .../sync/SyncResponsePostTreatmentAggregator.kt | 3 +++ ...yncResponsePostTreatmentAggregatorHandler.kt | 7 +++++++ .../sync/handler/room/RoomSyncHandler.kt | 17 ++++++++++------- 4 files changed, 31 insertions(+), 10 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt index 7e064a84ec..f82aa7372e 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt @@ -63,6 +63,7 @@ import org.matrix.android.sdk.internal.session.room.accountdata.RoomAccountDataD import org.matrix.android.sdk.internal.session.room.membership.RoomDisplayNameResolver import org.matrix.android.sdk.internal.session.room.membership.RoomMemberHelper import org.matrix.android.sdk.internal.session.room.relationship.RoomChildRelationInfo +import org.matrix.android.sdk.internal.session.sync.SyncResponsePostTreatmentAggregator import timber.log.Timber import javax.inject.Inject import kotlin.system.measureTimeMillis @@ -91,7 +92,8 @@ internal class RoomSummaryUpdater @Inject constructor( roomSummary: RoomSyncSummary? = null, unreadNotifications: RoomSyncUnreadNotifications? = null, updateMembers: Boolean = false, - inviterId: String? = null + inviterId: String? = null, + aggregator: SyncResponsePostTreatmentAggregator? = null ) { val roomSummaryEntity = RoomSummaryEntity.getOrCreate(realm, roomId) if (roomSummary != null) { @@ -180,8 +182,14 @@ internal class RoomSummaryUpdater @Inject constructor( roomSummaryEntity.otherMemberIds.clear() roomSummaryEntity.otherMemberIds.addAll(otherRoomMembers) if (roomSummaryEntity.isEncrypted && otherRoomMembers.isNotEmpty()) { - // mmm maybe we could only refresh shield instead of checking trust also? - crossSigningService.onUsersDeviceUpdate(otherRoomMembers) + if (aggregator == null) { + // Do it now + // mmm maybe we could only refresh shield instead of checking trust also? + crossSigningService.onUsersDeviceUpdate(otherRoomMembers) // This is very long and could maybe be done once per sync response. + } else { + // Schedule it + aggregator.userIdsWithDeviceUpdate.addAll(otherRoomMembers) + } } } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt index e9452c59fc..14f89e8a34 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt @@ -25,4 +25,7 @@ internal class SyncResponsePostTreatmentAggregator { // List of userIds to fetch and update at the end of incremental syncs val userIdsToFetch = mutableListOf() + + // Set of users to call `crossSigningService.onUsersDeviceUpdate` once per sync + val userIdsWithDeviceUpdate = mutableSetOf() } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt index c638ed4f80..9fb37994a0 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt @@ -20,6 +20,7 @@ import com.zhuinden.monarchy.Monarchy import org.matrix.android.sdk.api.MatrixPatterns import org.matrix.android.sdk.api.extensions.tryOrNull import org.matrix.android.sdk.api.session.user.model.User +import org.matrix.android.sdk.internal.crypto.crosssigning.DefaultCrossSigningService import org.matrix.android.sdk.internal.di.SessionDatabase import org.matrix.android.sdk.internal.session.profile.GetProfileInfoTask import org.matrix.android.sdk.internal.session.sync.RoomSyncEphemeralTemporaryStore @@ -36,12 +37,14 @@ internal class SyncResponsePostTreatmentAggregatorHandler @Inject constructor( private val ephemeralTemporaryStore: RoomSyncEphemeralTemporaryStore, private val updateUserAccountDataTask: UpdateUserAccountDataTask, private val getProfileInfoTask: GetProfileInfoTask, + private val crossSigningService: DefaultCrossSigningService, @SessionDatabase private val monarchy: Monarchy, ) { suspend fun handle(aggregator: SyncResponsePostTreatmentAggregator) { cleanupEphemeralFiles(aggregator.ephemeralFilesToDelete) updateDirectUserIds(aggregator.directChatsToCheck) fetchAndUpdateUsers(aggregator.userIdsToFetch) + handleUserIdsWithDeviceUpdate(aggregator.userIdsWithDeviceUpdate) } private fun cleanupEphemeralFiles(ephemeralFilesToDelete: List) { @@ -92,6 +95,10 @@ internal class SyncResponsePostTreatmentAggregatorHandler @Inject constructor( } } + private fun handleUserIdsWithDeviceUpdate(userIdsWithDeviceUpdate: Iterable) { + crossSigningService.onUsersDeviceUpdate(userIdsWithDeviceUpdate.toList()) + } + private suspend fun List.saveLocally() { val userEntities = map { user -> UserEntityFactory.create(user) } monarchy.awaitTransaction { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt index 30e1ec6679..bc91ca205d 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/room/RoomSyncHandler.kt @@ -154,12 +154,12 @@ internal class RoomSyncHandler @Inject constructor( } is HandlingStrategy.INVITED -> handlingStrategy.data.mapWithProgress(reporter, InitialSyncStep.ImportingAccountInvitedRooms, 0.1f) { - handleInvitedRoom(realm, it.key, it.value, insertType, syncLocalTimeStampMillis) + handleInvitedRoom(realm, it.key, it.value, insertType, syncLocalTimeStampMillis, aggregator) } is HandlingStrategy.LEFT -> { handlingStrategy.data.mapWithProgress(reporter, InitialSyncStep.ImportingAccountLeftRooms, 0.3f) { - handleLeftRoom(realm, it.key, it.value, insertType, syncLocalTimeStampMillis) + handleLeftRoom(realm, it.key, it.value, insertType, syncLocalTimeStampMillis, aggregator) } } } @@ -285,7 +285,8 @@ internal class RoomSyncHandler @Inject constructor( Membership.JOIN, roomSync.summary, roomSync.unreadNotifications, - updateMembers = hasRoomMember + updateMembers = hasRoomMember, + aggregator = aggregator ) return roomEntity } @@ -295,7 +296,8 @@ internal class RoomSyncHandler @Inject constructor( roomId: String, roomSync: InvitedRoomSync, insertType: EventInsertType, - syncLocalTimestampMillis: Long + syncLocalTimestampMillis: Long, + aggregator: SyncResponsePostTreatmentAggregator ): RoomEntity { Timber.v("Handle invited sync for room $roomId") val isInitialSync = insertType == EventInsertType.INITIAL_SYNC @@ -319,7 +321,7 @@ internal class RoomSyncHandler @Inject constructor( it.type == EventType.STATE_ROOM_MEMBER } roomChangeMembershipStateDataSource.setMembershipFromSync(roomId, Membership.INVITE) - roomSummaryUpdater.update(realm, roomId, Membership.INVITE, updateMembers = true, inviterId = inviterEvent?.senderId) + roomSummaryUpdater.update(realm, roomId, Membership.INVITE, updateMembers = true, inviterId = inviterEvent?.senderId, aggregator = aggregator) return roomEntity } @@ -328,7 +330,8 @@ internal class RoomSyncHandler @Inject constructor( roomId: String, roomSync: RoomSync, insertType: EventInsertType, - syncLocalTimestampMillis: Long + syncLocalTimestampMillis: Long, + aggregator: SyncResponsePostTreatmentAggregator ): RoomEntity { val isInitialSync = insertType == EventInsertType.INITIAL_SYNC val roomEntity = RoomEntity.getOrCreate(realm, roomId) @@ -366,7 +369,7 @@ internal class RoomSyncHandler @Inject constructor( roomEntity.chunks.clearWith { it.deleteOnCascade(deleteStateEvents = true, canDeleteRoot = true) } roomTypingUsersHandler.handle(realm, roomId, null) roomChangeMembershipStateDataSource.setMembershipFromSync(roomId, Membership.LEAVE) - roomSummaryUpdater.update(realm, roomId, membership, roomSync.summary, roomSync.unreadNotifications) + roomSummaryUpdater.update(realm, roomId, membership, roomSync.summary, roomSync.unreadNotifications, aggregator = aggregator) return roomEntity } From 9a0ea7bc2e02664a4377641bbf8ae22dd2b730e4 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 22 Aug 2022 17:33:03 +0200 Subject: [PATCH 02/11] Add some log for further investigation --- .../crypto/crosssigning/UpdateTrustWorker.kt | 2 ++ .../session/sync/SyncResponseHandler.kt | 28 +++++++++++++------ ...cResponsePostTreatmentAggregatorHandler.kt | 4 +++ 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt index f1dc060e10..6d845ec59e 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/UpdateTrustWorker.kt @@ -207,6 +207,7 @@ internal class UpdateTrustWorker(context: Context, params: WorkerParameters, ses private suspend fun updateTrustStep2(userList: List, myCrossSigningInfo: MXCrossSigningInfo?) { Timber.d("## CrossSigning - Updating shields for impacted rooms...") awaitTransaction(sessionRealmConfiguration) { sessionRealm -> + Timber.d("## CrossSigning - Updating shields for impacted rooms - in transaction") Realm.getInstance(cryptoRealmConfiguration).use { cryptoRealm -> sessionRealm.where(RoomMemberSummaryEntity::class.java) .`in`(RoomMemberSummaryEntityFields.USER_ID, userList.toTypedArray()) @@ -239,6 +240,7 @@ internal class UpdateTrustWorker(context: Context, params: WorkerParameters, ses } } } + Timber.d("## CrossSigning - Updating shields for impacted rooms - END") } private fun getCrossSigningInfo(cryptoRealm: Realm, userId: String): MXCrossSigningInfo? { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt index 392c73bd83..53456d87ff 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt @@ -126,21 +126,33 @@ internal class SyncResponseHandler @Inject constructor( } // Everything else we need to do outside the transaction - aggregatorHandler.handle(aggregator) - - syncResponse.rooms?.let { - checkPushRules(it, isInitialSync) - userAccountDataSyncHandler.synchronizeWithServerIfNeeded(it.invite) - dispatchInvitedRoom(it) + measureTimeMillis { + aggregatorHandler.handle(aggregator) + }.also { + Timber.v("Aggregator management took $it ms") } - Timber.v("On sync completed") - cryptoSyncHandler.onSyncCompleted(syncResponse) + measureTimeMillis { + syncResponse.rooms?.let { + checkPushRules(it, isInitialSync) + userAccountDataSyncHandler.synchronizeWithServerIfNeeded(it.invite) + dispatchInvitedRoom(it) + }.also { + Timber.v("SyncResponse.rooms post treatment took $it ms") + } + } + + measureTimeMillis { + cryptoSyncHandler.onSyncCompleted(syncResponse) + }.also { + Timber.v("cryptoSyncHandler.onSyncCompleted took $it ms") + } // post sync stuffs monarchy.writeAsync { roomSyncHandler.postSyncSpaceHierarchyHandle(it) } + Timber.v("On sync completed") } private fun dispatchInvitedRoom(roomsSyncResponse: RoomsSyncResponse) { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt index 9fb37994a0..fb7b7c60a0 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt @@ -30,6 +30,7 @@ import org.matrix.android.sdk.internal.session.user.UserEntityFactory import org.matrix.android.sdk.internal.session.user.accountdata.DirectChatsHelper import org.matrix.android.sdk.internal.session.user.accountdata.UpdateUserAccountDataTask import org.matrix.android.sdk.internal.util.awaitTransaction +import timber.log.Timber import javax.inject.Inject internal class SyncResponsePostTreatmentAggregatorHandler @Inject constructor( @@ -101,8 +102,11 @@ internal class SyncResponsePostTreatmentAggregatorHandler @Inject constructor( private suspend fun List.saveLocally() { val userEntities = map { user -> UserEntityFactory.create(user) } + Timber.d("## saveLocally()") monarchy.awaitTransaction { + Timber.d("## saveLocally() - in transaction") it.insertOrUpdate(userEntities) } + Timber.d("## saveLocally() - END") } } From a7666e2112ee6ec957d1972def10b83f1d4e3662 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 22 Aug 2022 17:37:29 +0200 Subject: [PATCH 03/11] Set instead of List, to avoid duplication. --- .../session/sync/SyncResponsePostTreatmentAggregator.kt | 2 +- .../handler/SyncResponsePostTreatmentAggregatorHandler.kt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt index 14f89e8a34..2e1863a034 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt @@ -24,7 +24,7 @@ internal class SyncResponsePostTreatmentAggregator { val directChatsToCheck = mutableMapOf() // List of userIds to fetch and update at the end of incremental syncs - val userIdsToFetch = mutableListOf() + val userIdsToFetch = mutableSetOf() // Set of users to call `crossSigningService.onUsersDeviceUpdate` once per sync val userIdsWithDeviceUpdate = mutableSetOf() diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt index fb7b7c60a0..b7806d154b 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt @@ -83,13 +83,13 @@ internal class SyncResponsePostTreatmentAggregatorHandler @Inject constructor( } } - private suspend fun fetchAndUpdateUsers(userIdsToFetch: List) { + private suspend fun fetchAndUpdateUsers(userIdsToFetch: Collection) { fetchUsers(userIdsToFetch) .takeIf { it.isNotEmpty() } ?.saveLocally() } - private suspend fun fetchUsers(userIdsToFetch: List) = userIdsToFetch.mapNotNull { + private suspend fun fetchUsers(userIdsToFetch: Collection) = userIdsToFetch.mapNotNull { tryOrNull { val profileJson = getProfileInfoTask.execute(GetProfileInfoTask.Params(it)) User.fromJson(it, profileJson) From 94a87744ac73502886b951d8f4f53af90daf1699 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 23 Aug 2022 09:50:35 +0200 Subject: [PATCH 04/11] Defer the treatment of updating the User profiles to a background Worker. --- .../sdk/internal/session/SessionComponent.kt | 3 + ...cResponsePostTreatmentAggregatorHandler.kt | 57 ++++++----- .../session/sync/handler/UpdateUserWorker.kt | 99 +++++++++++++++++++ .../internal/worker/MatrixWorkerFactory.kt | 3 + 4 files changed, 133 insertions(+), 29 deletions(-) create mode 100644 matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/UpdateUserWorker.kt diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/SessionComponent.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/SessionComponent.kt index a79f35bcb6..a7572035df 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/SessionComponent.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/SessionComponent.kt @@ -55,6 +55,7 @@ import org.matrix.android.sdk.internal.session.space.SpaceModule import org.matrix.android.sdk.internal.session.sync.SyncModule import org.matrix.android.sdk.internal.session.sync.SyncTask import org.matrix.android.sdk.internal.session.sync.SyncTokenStore +import org.matrix.android.sdk.internal.session.sync.handler.UpdateUserWorker import org.matrix.android.sdk.internal.session.sync.job.SyncWorker import org.matrix.android.sdk.internal.session.terms.TermsModule import org.matrix.android.sdk.internal.session.thirdparty.ThirdPartyModule @@ -128,6 +129,8 @@ internal interface SessionComponent { fun inject(worker: UpdateTrustWorker) + fun inject(worker: UpdateUserWorker) + fun inject(worker: DeactivateLiveLocationShareWorker) @Component.Factory diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt index b7806d154b..2d8339c901 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt @@ -16,30 +16,33 @@ package org.matrix.android.sdk.internal.session.sync.handler -import com.zhuinden.monarchy.Monarchy +import androidx.work.BackoffPolicy +import androidx.work.ExistingWorkPolicy import org.matrix.android.sdk.api.MatrixPatterns -import org.matrix.android.sdk.api.extensions.tryOrNull -import org.matrix.android.sdk.api.session.user.model.User import org.matrix.android.sdk.internal.crypto.crosssigning.DefaultCrossSigningService -import org.matrix.android.sdk.internal.di.SessionDatabase -import org.matrix.android.sdk.internal.session.profile.GetProfileInfoTask +import org.matrix.android.sdk.internal.crypto.crosssigning.UpdateTrustWorker +import org.matrix.android.sdk.internal.crypto.crosssigning.UpdateTrustWorkerDataRepository +import org.matrix.android.sdk.internal.di.SessionId +import org.matrix.android.sdk.internal.di.WorkManagerProvider import org.matrix.android.sdk.internal.session.sync.RoomSyncEphemeralTemporaryStore import org.matrix.android.sdk.internal.session.sync.SyncResponsePostTreatmentAggregator import org.matrix.android.sdk.internal.session.sync.model.accountdata.toMutable -import org.matrix.android.sdk.internal.session.user.UserEntityFactory import org.matrix.android.sdk.internal.session.user.accountdata.DirectChatsHelper import org.matrix.android.sdk.internal.session.user.accountdata.UpdateUserAccountDataTask -import org.matrix.android.sdk.internal.util.awaitTransaction +import org.matrix.android.sdk.internal.util.logLimit +import org.matrix.android.sdk.internal.worker.WorkerParamsFactory import timber.log.Timber +import java.util.concurrent.TimeUnit import javax.inject.Inject internal class SyncResponsePostTreatmentAggregatorHandler @Inject constructor( private val directChatsHelper: DirectChatsHelper, private val ephemeralTemporaryStore: RoomSyncEphemeralTemporaryStore, private val updateUserAccountDataTask: UpdateUserAccountDataTask, - private val getProfileInfoTask: GetProfileInfoTask, private val crossSigningService: DefaultCrossSigningService, - @SessionDatabase private val monarchy: Monarchy, + private val updateTrustWorkerDataRepository: UpdateTrustWorkerDataRepository, + private val workManagerProvider: WorkManagerProvider, + @SessionId private val sessionId: String, ) { suspend fun handle(aggregator: SyncResponsePostTreatmentAggregator) { cleanupEphemeralFiles(aggregator.ephemeralFilesToDelete) @@ -83,30 +86,26 @@ internal class SyncResponsePostTreatmentAggregatorHandler @Inject constructor( } } - private suspend fun fetchAndUpdateUsers(userIdsToFetch: Collection) { - fetchUsers(userIdsToFetch) - .takeIf { it.isNotEmpty() } - ?.saveLocally() - } + private fun fetchAndUpdateUsers(userIdsToFetch: Collection) { + if (userIdsToFetch.isEmpty()) return + Timber.d("## Configure Worker to update users: ${userIdsToFetch.logLimit()}") + val workerParams = UpdateTrustWorker.Params( + sessionId = sessionId, + filename = updateTrustWorkerDataRepository.createParam(userIdsToFetch.toList()) + ) + val workerData = WorkerParamsFactory.toData(workerParams) - private suspend fun fetchUsers(userIdsToFetch: Collection) = userIdsToFetch.mapNotNull { - tryOrNull { - val profileJson = getProfileInfoTask.execute(GetProfileInfoTask.Params(it)) - User.fromJson(it, profileJson) - } + val workRequest = workManagerProvider.matrixOneTimeWorkRequestBuilder() + .setInputData(workerData) + .setBackoffCriteria(BackoffPolicy.LINEAR, WorkManagerProvider.BACKOFF_DELAY_MILLIS, TimeUnit.MILLISECONDS) + .build() + + workManagerProvider.workManager + .beginUniqueWork("USER_UPDATE_QUEUE", ExistingWorkPolicy.APPEND_OR_REPLACE, workRequest) + .enqueue() } private fun handleUserIdsWithDeviceUpdate(userIdsWithDeviceUpdate: Iterable) { crossSigningService.onUsersDeviceUpdate(userIdsWithDeviceUpdate.toList()) } - - private suspend fun List.saveLocally() { - val userEntities = map { user -> UserEntityFactory.create(user) } - Timber.d("## saveLocally()") - monarchy.awaitTransaction { - Timber.d("## saveLocally() - in transaction") - it.insertOrUpdate(userEntities) - } - Timber.d("## saveLocally() - END") - } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/UpdateUserWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/UpdateUserWorker.kt new file mode 100644 index 0000000000..646e09f30c --- /dev/null +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/UpdateUserWorker.kt @@ -0,0 +1,99 @@ +/* + * Copyright 2022 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.sync.handler + +import android.content.Context +import androidx.work.WorkerParameters +import com.zhuinden.monarchy.Monarchy +import org.matrix.android.sdk.api.extensions.tryOrNull +import org.matrix.android.sdk.api.session.user.model.User +import org.matrix.android.sdk.internal.SessionManager +import org.matrix.android.sdk.internal.crypto.crosssigning.UpdateTrustWorker +import org.matrix.android.sdk.internal.crypto.crosssigning.UpdateTrustWorkerDataRepository +import org.matrix.android.sdk.internal.di.SessionDatabase +import org.matrix.android.sdk.internal.session.SessionComponent +import org.matrix.android.sdk.internal.session.profile.GetProfileInfoTask +import org.matrix.android.sdk.internal.session.user.UserEntityFactory +import org.matrix.android.sdk.internal.util.awaitTransaction +import org.matrix.android.sdk.internal.util.logLimit +import org.matrix.android.sdk.internal.worker.SessionSafeCoroutineWorker +import timber.log.Timber +import javax.inject.Inject + +/** + * Note: We reuse the same type [UpdateTrustWorker.Params], since the inout data are the same. + */ +internal class UpdateUserWorker(context: Context, params: WorkerParameters, sessionManager: SessionManager) : + SessionSafeCoroutineWorker(context, params, sessionManager, UpdateTrustWorker.Params::class.java) { + + @SessionDatabase + @Inject lateinit var monarchy: Monarchy + @Inject lateinit var updateTrustWorkerDataRepository: UpdateTrustWorkerDataRepository + @Inject lateinit var getProfileInfoTask: GetProfileInfoTask + + override fun injectWith(injector: SessionComponent) { + injector.inject(this) + } + + override suspend fun doSafeWork(params: UpdateTrustWorker.Params): Result { + val userList = params.filename + ?.let { updateTrustWorkerDataRepository.getParam(it) } + ?.userIds + ?: params.updatedUserIds.orEmpty() + + // List should not be empty, but let's avoid go further in case of empty list + if (userList.isNotEmpty()) { + Timber.v("## UpdateUserWorker - updating users: ${userList.logLimit()}") + fetchAndUpdateUsers(userList) + } + + cleanup(params) + return Result.success() + } + + private suspend fun fetchAndUpdateUsers(userIdsToFetch: Collection) { + fetchUsers(userIdsToFetch) + .takeIf { it.isNotEmpty() } + ?.saveLocally() + } + + private suspend fun fetchUsers(userIdsToFetch: Collection) = userIdsToFetch.mapNotNull { + tryOrNull { + val profileJson = getProfileInfoTask.execute(GetProfileInfoTask.Params(it)) + User.fromJson(it, profileJson) + } + } + + private suspend fun List.saveLocally() { + val userEntities = map { user -> UserEntityFactory.create(user) } + Timber.d("## saveLocally()") + monarchy.awaitTransaction { + Timber.d("## saveLocally() - in transaction") + it.insertOrUpdate(userEntities) + } + Timber.d("## saveLocally() - END") + } + + private fun cleanup(params: UpdateTrustWorker.Params) { + params.filename + ?.let { updateTrustWorkerDataRepository.delete(it) } + } + + override fun buildErrorParams(params: UpdateTrustWorker.Params, message: String): UpdateTrustWorker.Params { + return params.copy(lastFailureMessage = params.lastFailureMessage ?: message) + } +} diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/MatrixWorkerFactory.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/MatrixWorkerFactory.kt index 83f9532870..80bbbb7938 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/MatrixWorkerFactory.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/worker/MatrixWorkerFactory.kt @@ -30,6 +30,7 @@ import org.matrix.android.sdk.internal.session.room.aggregation.livelocation.Dea import org.matrix.android.sdk.internal.session.room.send.MultipleEventSendingDispatcherWorker import org.matrix.android.sdk.internal.session.room.send.RedactEventWorker import org.matrix.android.sdk.internal.session.room.send.SendEventWorker +import org.matrix.android.sdk.internal.session.sync.handler.UpdateUserWorker import org.matrix.android.sdk.internal.session.sync.job.SyncWorker import timber.log.Timber import javax.inject.Inject @@ -62,6 +63,8 @@ internal class MatrixWorkerFactory @Inject constructor(private val sessionManage SyncWorker(appContext, workerParameters, sessionManager) UpdateTrustWorker::class.java.name -> UpdateTrustWorker(appContext, workerParameters, sessionManager) + UpdateUserWorker::class.java.name -> + UpdateUserWorker(appContext, workerParameters, sessionManager) UploadContentWorker::class.java.name -> UploadContentWorker(appContext, workerParameters, sessionManager) DeactivateLiveLocationShareWorker::class.java.name -> From e3f5d15eafdef3f6418ecb45e2a027e09c059e62 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 23 Aug 2022 11:19:14 +0200 Subject: [PATCH 05/11] Do not fetch user if we do not have the previous content. --- .../internal/session/room/membership/RoomMemberEventHandler.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/RoomMemberEventHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/RoomMemberEventHandler.kt index fd6552525e..cb7bbf07fc 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/RoomMemberEventHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/membership/RoomMemberEventHandler.kt @@ -140,7 +140,8 @@ internal class RoomMemberEventHandler @Inject constructor( val previousDisplayName = prevContent?.get("displayname") as? String val previousAvatar = prevContent?.get("avatar_url") as? String - if (previousDisplayName != roomMember.displayName || previousAvatar != roomMember.avatarUrl) { + if ((previousDisplayName != null && previousDisplayName != roomMember.displayName) || + (previousAvatar != null && previousAvatar != roomMember.avatarUrl)) { aggregator.userIdsToFetch.add(eventUserId) } } From c9e76f5f9738b26a0c690f75de51a746b6c6cc14 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 23 Aug 2022 11:58:05 +0200 Subject: [PATCH 06/11] changelog --- changelog.d/6917.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6917.bugfix diff --git a/changelog.d/6917.bugfix b/changelog.d/6917.bugfix new file mode 100644 index 0000000000..7ddcc16d9a --- /dev/null +++ b/changelog.d/6917.bugfix @@ -0,0 +1 @@ +Fix long incremental sync. From 5c02290ad45099397f24f30237c6f1d6a89a4776 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 23 Aug 2022 12:00:27 +0200 Subject: [PATCH 07/11] Fix logging issue --- .../android/sdk/internal/session/sync/SyncResponseHandler.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt index 53456d87ff..05216d1de1 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt @@ -137,9 +137,9 @@ internal class SyncResponseHandler @Inject constructor( checkPushRules(it, isInitialSync) userAccountDataSyncHandler.synchronizeWithServerIfNeeded(it.invite) dispatchInvitedRoom(it) - }.also { - Timber.v("SyncResponse.rooms post treatment took $it ms") } + }.also { + Timber.v("SyncResponse.rooms post treatment took $it ms") } measureTimeMillis { From 1a79828aa58deb361f88ccc7919b94a0a70949d0 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 23 Aug 2022 12:01:43 +0200 Subject: [PATCH 08/11] Update comment --- .../session/sync/SyncResponsePostTreatmentAggregator.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt index 2e1863a034..ab6907d9e8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt @@ -23,7 +23,7 @@ internal class SyncResponsePostTreatmentAggregator { // Map of roomId to directUserId val directChatsToCheck = mutableMapOf() - // List of userIds to fetch and update at the end of incremental syncs + // Set of userIds to fetch and update at the end of incremental syncs val userIdsToFetch = mutableSetOf() // Set of users to call `crossSigningService.onUsersDeviceUpdate` once per sync From aa750cccbfdbca7c3f3a4ca5fa3e73e570a785f0 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Tue, 23 Aug 2022 12:03:25 +0200 Subject: [PATCH 09/11] typo --- .../sdk/internal/session/sync/handler/UpdateUserWorker.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/UpdateUserWorker.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/UpdateUserWorker.kt index 646e09f30c..1f840a82d5 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/UpdateUserWorker.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/UpdateUserWorker.kt @@ -35,7 +35,7 @@ import timber.log.Timber import javax.inject.Inject /** - * Note: We reuse the same type [UpdateTrustWorker.Params], since the inout data are the same. + * Note: We reuse the same type [UpdateTrustWorker.Params], since the input data are the same. */ internal class UpdateUserWorker(context: Context, params: WorkerParameters, sessionManager: SessionManager) : SessionSafeCoroutineWorker(context, params, sessionManager, UpdateTrustWorker.Params::class.java) { From f668be5266c60c7eb6beb1340fd4362bfdfad5c3 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 29 Aug 2022 12:39:58 +0200 Subject: [PATCH 10/11] Remove tmp comment --- .../sdk/internal/session/room/summary/RoomSummaryUpdater.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt index f82aa7372e..33ca0ca777 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt @@ -185,7 +185,7 @@ internal class RoomSummaryUpdater @Inject constructor( if (aggregator == null) { // Do it now // mmm maybe we could only refresh shield instead of checking trust also? - crossSigningService.onUsersDeviceUpdate(otherRoomMembers) // This is very long and could maybe be done once per sync response. + crossSigningService.onUsersDeviceUpdate(otherRoomMembers) } else { // Schedule it aggregator.userIdsWithDeviceUpdate.addAll(otherRoomMembers) From a8eb7d95acdc4c4cc009afd9790c2bef462dd36b Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 29 Aug 2022 14:27:26 +0200 Subject: [PATCH 11/11] Create a new `fun` for code clarity --- .../crypto/crosssigning/DefaultCrossSigningService.kt | 5 +++++ .../sdk/internal/session/room/summary/RoomSummaryUpdater.kt | 4 ++-- .../session/sync/SyncResponsePostTreatmentAggregator.kt | 4 ++-- .../handler/SyncResponsePostTreatmentAggregatorHandler.kt | 6 +++--- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/DefaultCrossSigningService.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/DefaultCrossSigningService.kt index e466def1a1..d405bdce27 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/DefaultCrossSigningService.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/crosssigning/DefaultCrossSigningService.kt @@ -779,6 +779,11 @@ internal class DefaultCrossSigningService @Inject constructor( override fun onUsersDeviceUpdate(userIds: List) { Timber.d("## CrossSigning - onUsersDeviceUpdate for users: ${userIds.logLimit()}") + checkTrustAndAffectedRoomShields(userIds) + } + + fun checkTrustAndAffectedRoomShields(userIds: List) { + Timber.d("## CrossSigning - checkTrustAndAffectedRoomShields for users: ${userIds.logLimit()}") val workerParams = UpdateTrustWorker.Params( sessionId = sessionId, filename = updateTrustWorkerDataRepository.createParam(userIds) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt index 33ca0ca777..6979d42827 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/summary/RoomSummaryUpdater.kt @@ -185,10 +185,10 @@ internal class RoomSummaryUpdater @Inject constructor( if (aggregator == null) { // Do it now // mmm maybe we could only refresh shield instead of checking trust also? - crossSigningService.onUsersDeviceUpdate(otherRoomMembers) + crossSigningService.checkTrustAndAffectedRoomShields(otherRoomMembers) } else { // Schedule it - aggregator.userIdsWithDeviceUpdate.addAll(otherRoomMembers) + aggregator.userIdsForCheckingTrustAndAffectedRoomShields.addAll(otherRoomMembers) } } } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt index ab6907d9e8..2b7f936fa8 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponsePostTreatmentAggregator.kt @@ -26,6 +26,6 @@ internal class SyncResponsePostTreatmentAggregator { // Set of userIds to fetch and update at the end of incremental syncs val userIdsToFetch = mutableSetOf() - // Set of users to call `crossSigningService.onUsersDeviceUpdate` once per sync - val userIdsWithDeviceUpdate = mutableSetOf() + // Set of users to call `crossSigningService.checkTrustAndAffectedRoomShields` once per sync + val userIdsForCheckingTrustAndAffectedRoomShields = mutableSetOf() } diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt index 2d8339c901..c749f77fff 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/handler/SyncResponsePostTreatmentAggregatorHandler.kt @@ -48,7 +48,7 @@ internal class SyncResponsePostTreatmentAggregatorHandler @Inject constructor( cleanupEphemeralFiles(aggregator.ephemeralFilesToDelete) updateDirectUserIds(aggregator.directChatsToCheck) fetchAndUpdateUsers(aggregator.userIdsToFetch) - handleUserIdsWithDeviceUpdate(aggregator.userIdsWithDeviceUpdate) + handleUserIdsForCheckingTrustAndAffectedRoomShields(aggregator.userIdsForCheckingTrustAndAffectedRoomShields) } private fun cleanupEphemeralFiles(ephemeralFilesToDelete: List) { @@ -105,7 +105,7 @@ internal class SyncResponsePostTreatmentAggregatorHandler @Inject constructor( .enqueue() } - private fun handleUserIdsWithDeviceUpdate(userIdsWithDeviceUpdate: Iterable) { - crossSigningService.onUsersDeviceUpdate(userIdsWithDeviceUpdate.toList()) + private fun handleUserIdsForCheckingTrustAndAffectedRoomShields(userIdsWithDeviceUpdate: Iterable) { + crossSigningService.checkTrustAndAffectedRoomShields(userIdsWithDeviceUpdate.toList()) } }