From 883d8342849de8d0bbb45456aef0a416f4eaf85e Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Mon, 18 Mar 2024 15:57:25 +0100 Subject: [PATCH] Remove unnecessary `Room.updateMembers()` calls. (#2564) * Remove unnecessary `updateMembers` calls. Some of them can be directly removed since we have a way to automatically get member info updates based on membership changes. Others can be replaced by a simpler `getUpdatedMember` method. This might still need a full member sync, but it's quite unlikely. --- .../messages/impl/MessagesPresenter.kt | 2 -- .../roomdetails/impl/RoomDetailsPresenter.kt | 9 -------- .../impl/invite/RoomInviteMembersNode.kt | 2 -- .../details/RoomMemberDetailsPresenter.kt | 8 ++++--- .../RolesAndPermissionsNode.kt | 21 +++---------------- .../libraries/matrix/api/room/MatrixRoom.kt | 5 +++++ .../matrix/impl/room/RustMatrixRoom.kt | 6 ++++++ .../matrix/test/room/FakeMatrixRoom.kt | 10 ++++++--- 8 files changed, 26 insertions(+), 37 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt index b517d809b7..4c92585b0d 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt @@ -295,8 +295,6 @@ class MessagesPresenter @AssistedInject constructor( private fun CoroutineScope.reinviteOtherUser(inviteProgress: MutableState>) = launch(dispatchers.io) { inviteProgress.value = AsyncData.Loading() runCatching { - room.updateMembers() - val memberList = when (val memberState = room.membersStateFlow.value) { is MatrixRoomMembersState.Ready -> memberState.roomMembers is MatrixRoomMembersState.Error -> memberState.prevRoomMembers.orEmpty() diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt index ad7b4569c8..9e4030201a 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt @@ -26,7 +26,6 @@ import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.produceState import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope -import androidx.lifecycle.Lifecycle import im.vector.app.features.analytics.plan.Interaction import io.element.android.features.leaveroom.api.LeaveRoomEvent import io.element.android.features.leaveroom.api.LeaveRoomPresenter @@ -34,7 +33,6 @@ import io.element.android.features.roomdetails.impl.members.details.RoomMemberDe import io.element.android.libraries.architecture.Presenter import io.element.android.libraries.core.bool.orFalse import io.element.android.libraries.core.coroutine.CoroutineDispatchers -import io.element.android.libraries.designsystem.utils.OnLifecycleEvent import io.element.android.libraries.featureflag.api.FeatureFlagService import io.element.android.libraries.featureflag.api.FeatureFlags import io.element.android.libraries.matrix.api.MatrixClient @@ -92,13 +90,6 @@ class RoomDetailsPresenter @Inject constructor( } } - // Update room members only when first presenting the node - OnLifecycleEvent { _, event -> - if (event == Lifecycle.Event.ON_CREATE) { - scope.launch { room.updateMembers() } - } - } - val membersState by room.membersStateFlow.collectAsState() val canInvite by getCanInvite(membersState) val canEditName by getCanSendState(membersState, StateEventType.ROOM_NAME) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/invite/RoomInviteMembersNode.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/invite/RoomInviteMembersNode.kt index d5fe439ff3..34fe6273e7 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/invite/RoomInviteMembersNode.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/invite/RoomInviteMembersNode.kt @@ -80,8 +80,6 @@ class RoomInviteMembersNode @AssistedInject constructor( body = context.getString(CommonStrings.common_unable_to_invite_message), ) } - - room.updateMembers() } } ) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt index f3f3488018..cb749d2c80 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/details/RoomMemberDetailsPresenter.kt @@ -67,7 +67,9 @@ class RoomMemberDetailsPresenter @AssistedInject constructor( } } LaunchedEffect(Unit) { - room.updateMembers() + // Update room member info when opening this screen + // We don't need to assign the result as it will be automatically propagated by `room.getRoomMemberAsState` + room.getUpdatedMember(roomMemberId) } fun handleEvents(event: RoomMemberDetailsEvents) { @@ -133,7 +135,7 @@ class RoomMemberDetailsPresenter @AssistedInject constructor( .fold( onSuccess = { isBlockedState.value = AsyncData.Success(true) - room.updateMembers() + room.getUpdatedMember(userId) }, onFailure = { isBlockedState.value = AsyncData.Failure(it, false) @@ -147,7 +149,7 @@ class RoomMemberDetailsPresenter @AssistedInject constructor( .fold( onSuccess = { isBlockedState.value = AsyncData.Success(false) - room.updateMembers() + room.getUpdatedMember(userId) }, onFailure = { isBlockedState.value = AsyncData.Failure(it, true) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/rolesandpermissions/RolesAndPermissionsNode.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/rolesandpermissions/RolesAndPermissionsNode.kt index 0da156e54b..af04465c9f 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/rolesandpermissions/RolesAndPermissionsNode.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/rolesandpermissions/RolesAndPermissionsNode.kt @@ -19,9 +19,6 @@ package io.element.android.features.roomdetails.impl.rolesandpermissions import androidx.compose.runtime.Composable import androidx.compose.runtime.Stable import androidx.compose.ui.Modifier -import androidx.lifecycle.Lifecycle -import androidx.lifecycle.LifecycleEventObserver -import androidx.lifecycle.LifecycleOwner import androidx.lifecycle.lifecycleScope import com.bumble.appyx.core.modality.BuildContext import com.bumble.appyx.core.node.Node @@ -33,10 +30,8 @@ import io.element.android.anvilannotations.ContributesNode import io.element.android.libraries.di.RoomScope import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.RoomMember -import io.element.android.libraries.matrix.api.room.roomMembers import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.filter -import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.take import kotlinx.coroutines.launch @@ -69,22 +64,12 @@ class RolesAndPermissionsNode @AssistedInject constructor( override fun onBuilt() { super.onBuilt() - // Reload members when the user sees this screen - lifecycle.addObserver(object : LifecycleEventObserver { - override fun onStateChanged(source: LifecycleOwner, event: Lifecycle.Event) { - if (event == Lifecycle.Event.ON_RESUME) { - lifecycleScope.launch { room.updateMembers() } - } - } - }) - // If the user is not an admin anymore, exit this section since they won't have permissions to use it lifecycleScope.launch { - room.membersStateFlow - .map { state -> - state.roomMembers().orEmpty().find { it.userId == room.sessionId } + room.roomInfoFlow + .filter { info -> + info.userPowerLevels[room.sessionId] != RoomMember.Role.ADMIN.powerLevel } - .filter { it?.role != RoomMember.Role.ADMIN } .take(1) .onEach { navigateUp() } .collect() diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt index f049bd1deb..07b72e3c5f 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt @@ -86,6 +86,11 @@ interface MatrixRoom : Closeable { */ suspend fun updateMembers() + /** + * Will return an updated member or an error. + */ + suspend fun getUpdatedMember(userId: UserId): Result + suspend fun updateRoomNotificationSettings(): Result val syncUpdateFlow: StateFlow diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt index 9a1366a73e..1eb10bb4e8 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt @@ -230,6 +230,12 @@ class RustMatrixRoom( roomMemberListFetcher.fetchRoomMembers(source = source) } + override suspend fun getUpdatedMember(userId: UserId): Result = withContext(roomDispatcher) { + runCatching { + RoomMemberMapper.map(innerRoom.member(userId.value)) + } + } + override suspend fun userDisplayName(userId: UserId): Result = withContext(roomDispatcher) { runCatching { innerRoom.memberDisplayName(userId.value) diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt index 56d63fe1d8..5bab9e6a84 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt @@ -91,7 +91,7 @@ class FakeMatrixRoom( private var userDisplayNameResult = Result.success(null) private var userAvatarUrlResult = Result.success(null) private var userRoleResult = Result.success(RoomMember.Role.USER) - private var updateMembersResult: Result = Result.success(Unit) + private var getRoomMemberResult = Result.failure(IllegalStateException("Member not found")) private var joinRoomResult = Result.success(Unit) private var inviteUserResult = Result.success(Unit) private var canInviteResult = Result.success(true) @@ -195,6 +195,10 @@ class FakeMatrixRoom( override suspend fun updateMembers() = Unit + override suspend fun getUpdatedMember(userId: UserId): Result { + return getRoomMemberResult + } + override suspend fun updateRoomNotificationSettings(): Result = simulateLongTask { val notificationSettings = notificationSettingsService.getRoomNotificationSettings(roomId, isEncrypted, isOneToOne).getOrThrow() roomNotificationSettingsStateFlow.value = MatrixRoomNotificationSettingsState.Ready(notificationSettings) @@ -532,8 +536,8 @@ class FakeMatrixRoom( membersStateFlow.value = state } - fun givenUpdateMembersResult(result: Result) { - updateMembersResult = result + fun givenGetRoomMemberResult(result: Result) { + getRoomMemberResult = result } fun givenUserDisplayNameResult(displayName: Result) {