Skip to content

Commit

Permalink
Remove unnecessary Room.updateMembers() calls. (#2564)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
jmartinesp committed Mar 18, 2024
1 parent fde154a commit 883d834
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,6 @@ class MessagesPresenter @AssistedInject constructor(
private fun CoroutineScope.reinviteOtherUser(inviteProgress: MutableState<AsyncData<Unit>>) = 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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,13 @@ 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
import io.element.android.features.roomdetails.impl.members.details.RoomMemberDetailsPresenter
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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,6 @@ class RoomInviteMembersNode @AssistedInject constructor(
body = context.getString(CommonStrings.common_unable_to_invite_message),
)
}

room.updateMembers()
}
}
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ interface MatrixRoom : Closeable {
*/
suspend fun updateMembers()

/**
* Will return an updated member or an error.
*/
suspend fun getUpdatedMember(userId: UserId): Result<RoomMember>

suspend fun updateRoomNotificationSettings(): Result<Unit>

val syncUpdateFlow: StateFlow<Long>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,12 @@ class RustMatrixRoom(
roomMemberListFetcher.fetchRoomMembers(source = source)
}

override suspend fun getUpdatedMember(userId: UserId): Result<RoomMember> = withContext(roomDispatcher) {
runCatching {
RoomMemberMapper.map(innerRoom.member(userId.value))
}
}

override suspend fun userDisplayName(userId: UserId): Result<String?> = withContext(roomDispatcher) {
runCatching {
innerRoom.memberDisplayName(userId.value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class FakeMatrixRoom(
private var userDisplayNameResult = Result.success<String?>(null)
private var userAvatarUrlResult = Result.success<String?>(null)
private var userRoleResult = Result.success(RoomMember.Role.USER)
private var updateMembersResult: Result<Unit> = Result.success(Unit)
private var getRoomMemberResult = Result.failure<RoomMember>(IllegalStateException("Member not found"))
private var joinRoomResult = Result.success(Unit)
private var inviteUserResult = Result.success(Unit)
private var canInviteResult = Result.success(true)
Expand Down Expand Up @@ -195,6 +195,10 @@ class FakeMatrixRoom(

override suspend fun updateMembers() = Unit

override suspend fun getUpdatedMember(userId: UserId): Result<RoomMember> {
return getRoomMemberResult
}

override suspend fun updateRoomNotificationSettings(): Result<Unit> = simulateLongTask {
val notificationSettings = notificationSettingsService.getRoomNotificationSettings(roomId, isEncrypted, isOneToOne).getOrThrow()
roomNotificationSettingsStateFlow.value = MatrixRoomNotificationSettingsState.Ready(notificationSettings)
Expand Down Expand Up @@ -532,8 +536,8 @@ class FakeMatrixRoom(
membersStateFlow.value = state
}

fun givenUpdateMembersResult(result: Result<Unit>) {
updateMembersResult = result
fun givenGetRoomMemberResult(result: Result<RoomMember>) {
getRoomMemberResult = result
}

fun givenUserDisplayNameResult(displayName: Result<String?>) {
Expand Down

0 comments on commit 883d834

Please sign in to comment.