Fix issues on JoinedRoom / BaseRoom (#4724)

* Import type

* Add test to cover an existing issue. roomCoroutineScope is not cancelled when the class is destroyed

* Cancel roomCoroutineScope when the class is destroyed

* Move `isOneToOne` to BaseRoom, we do not need a JoinedRoom for it.

* Let `roomInfoFlow` be implemented by RustBaseRoom.

It should fix a few issues where we rely on the room info to be live, and it was not the case on RustBaseRoom.

* Add more assertions.
The test would fail anyway if roomCoroutineScope was still active, but it's more explicit with these assertions.
This commit is contained in:
Benoit Marty 2025-05-15 18:08:58 +02:00 committed by GitHub
parent 0705ae0b3a
commit 3b5d365834
6 changed files with 77 additions and 26 deletions

View file

@ -55,6 +55,12 @@ interface BaseRoom : Closeable {
*/ */
fun info(): RoomInfo = roomInfoFlow.value fun info(): RoomInfo = roomInfoFlow.value
/**
* A one-to-one is a room with exactly 2 members.
* See [the Matrix spec](https://spec.matrix.org/latest/client-server-api/#default-underride-rules).
*/
val isOneToOne: Boolean get() = info().activeMembersCount == 2L
/** /**
* Try to load the room members and update the membersFlow. * Try to load the room members and update the membersFlow.
*/ */

View file

@ -50,12 +50,6 @@ interface JoinedRoom : BaseRoom {
*/ */
val knockRequestsFlow: Flow<List<KnockRequest>> val knockRequestsFlow: Flow<List<KnockRequest>>
/**
* A one-to-one is a room with exactly 2 members.
* See [the Matrix spec](https://spec.matrix.org/latest/client-server-api/#default-underride-rules).
*/
val isOneToOne: Boolean get() = info().activeMembersCount == 2L
/** /**
* The live timeline of the room. Must be used to send Event to a room. * The live timeline of the room. Must be used to send Event to a room.
*/ */

View file

@ -59,11 +59,8 @@ import io.element.android.libraries.matrix.impl.util.mxCallbackFlow
import io.element.android.libraries.matrix.impl.widget.RustWidgetDriver import io.element.android.libraries.matrix.impl.widget.RustWidgetDriver
import io.element.android.libraries.matrix.impl.widget.generateWidgetWebViewUrl import io.element.android.libraries.matrix.impl.widget.generateWidgetWebViewUrl
import io.element.android.services.toolbox.api.systemclock.SystemClock import io.element.android.services.toolbox.api.systemclock.SystemClock
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.drop
@ -71,12 +68,10 @@ import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.onStart
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
import org.matrix.rustcomponents.sdk.DateDividerMode import org.matrix.rustcomponents.sdk.DateDividerMode
import org.matrix.rustcomponents.sdk.IdentityStatusChangeListener import org.matrix.rustcomponents.sdk.IdentityStatusChangeListener
import org.matrix.rustcomponents.sdk.KnockRequestsListener import org.matrix.rustcomponents.sdk.KnockRequestsListener
import org.matrix.rustcomponents.sdk.RoomInfoListener
import org.matrix.rustcomponents.sdk.RoomMessageEventMessageType import org.matrix.rustcomponents.sdk.RoomMessageEventMessageType
import org.matrix.rustcomponents.sdk.TimelineConfiguration import org.matrix.rustcomponents.sdk.TimelineConfiguration
import org.matrix.rustcomponents.sdk.TimelineFilter import org.matrix.rustcomponents.sdk.TimelineFilter
@ -93,7 +88,6 @@ import java.io.File
import kotlin.coroutines.cancellation.CancellationException import kotlin.coroutines.cancellation.CancellationException
import org.matrix.rustcomponents.sdk.IdentityStatusChange as RustIdentityStateChange import org.matrix.rustcomponents.sdk.IdentityStatusChange as RustIdentityStateChange
import org.matrix.rustcomponents.sdk.KnockRequest as InnerKnockRequest import org.matrix.rustcomponents.sdk.KnockRequest as InnerKnockRequest
import org.matrix.rustcomponents.sdk.RoomInfo as InnerRoomInfo
import org.matrix.rustcomponents.sdk.Timeline as InnerTimeline import org.matrix.rustcomponents.sdk.Timeline as InnerTimeline
class JoinedRustRoom( class JoinedRustRoom(
@ -101,7 +95,6 @@ class JoinedRustRoom(
private val liveInnerTimeline: InnerTimeline, private val liveInnerTimeline: InnerTimeline,
private val notificationSettingsService: NotificationSettingsService, private val notificationSettingsService: NotificationSettingsService,
private val coroutineDispatchers: CoroutineDispatchers, private val coroutineDispatchers: CoroutineDispatchers,
private val roomInfoMapper: RoomInfoMapper,
private val systemClock: SystemClock, private val systemClock: SystemClock,
private val roomContentForwarder: RoomContentForwarder, private val roomContentForwarder: RoomContentForwarder,
private val featureFlagService: FeatureFlagService, private val featureFlagService: FeatureFlagService,
@ -112,14 +105,6 @@ class JoinedRustRoom(
override val syncUpdateFlow = MutableStateFlow(0L) override val syncUpdateFlow = MutableStateFlow(0L)
override val roomInfoFlow: StateFlow<io.element.android.libraries.matrix.api.room.RoomInfo> = mxCallbackFlow {
innerRoom.subscribeToRoomInfoUpdates(object : RoomInfoListener {
override fun call(roomInfo: InnerRoomInfo) {
channel.trySend(roomInfoMapper.map(roomInfo))
}
})
}.stateIn(roomCoroutineScope, started = SharingStarted.Lazily, initialValue = baseRoom.info())
override val roomTypingMembersFlow: Flow<List<UserId>> = mxCallbackFlow { override val roomTypingMembersFlow: Flow<List<UserId>> = mxCallbackFlow {
val initial = emptyList<UserId>() val initial = emptyList<UserId>()
channel.trySend(initial) channel.trySend(initial)
@ -645,7 +630,6 @@ class JoinedRustRoom(
override fun destroy() { override fun destroy() {
baseRoom.destroy() baseRoom.destroy()
liveInnerTimeline.destroy() liveInnerTimeline.destroy()
roomCoroutineScope.cancel()
} }
private fun InnerTimeline.map( private fun InnerTimeline.map(

View file

@ -31,10 +31,14 @@ import io.element.android.libraries.matrix.impl.room.member.RoomMemberMapper
import io.element.android.libraries.matrix.impl.room.powerlevels.RoomPowerLevelsMapper import io.element.android.libraries.matrix.impl.room.powerlevels.RoomPowerLevelsMapper
import io.element.android.libraries.matrix.impl.roomdirectory.map import io.element.android.libraries.matrix.impl.roomdirectory.map
import io.element.android.libraries.matrix.impl.timeline.toRustReceiptType import io.element.android.libraries.matrix.impl.timeline.toRustReceiptType
import io.element.android.libraries.matrix.impl.util.mxCallbackFlow
import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.stateIn
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
import org.matrix.rustcomponents.sdk.RoomInfoListener
import org.matrix.rustcomponents.sdk.use import org.matrix.rustcomponents.sdk.use
import timber.log.Timber import timber.log.Timber
import uniffi.matrix_sdk_base.EncryptionState import uniffi.matrix_sdk_base.EncryptionState
@ -48,6 +52,7 @@ class RustBaseRoom(
private val roomSyncSubscriber: RoomSyncSubscriber, private val roomSyncSubscriber: RoomSyncSubscriber,
private val roomMembershipObserver: RoomMembershipObserver, private val roomMembershipObserver: RoomMembershipObserver,
sessionCoroutineScope: CoroutineScope, sessionCoroutineScope: CoroutineScope,
roomInfoMapper: RoomInfoMapper,
initialRoomInfo: RoomInfo, initialRoomInfo: RoomInfo,
) : BaseRoom { ) : BaseRoom {
override val roomId = RoomId(innerRoom.id()) override val roomId = RoomId(innerRoom.id())
@ -62,10 +67,16 @@ class RustBaseRoom(
override val membersStateFlow: StateFlow<RoomMembersState> = roomMemberListFetcher.membersFlow override val membersStateFlow: StateFlow<RoomMembersState> = roomMemberListFetcher.membersFlow
override val roomInfoFlow: StateFlow<RoomInfo> = MutableStateFlow(initialRoomInfo)
override val roomCoroutineScope = sessionCoroutineScope.childScope(coroutineDispatchers.main, "RoomScope-$roomId") override val roomCoroutineScope = sessionCoroutineScope.childScope(coroutineDispatchers.main, "RoomScope-$roomId")
override val roomInfoFlow: StateFlow<RoomInfo> = mxCallbackFlow {
innerRoom.subscribeToRoomInfoUpdates(object : RoomInfoListener {
override fun call(roomInfo: org.matrix.rustcomponents.sdk.RoomInfo) {
channel.trySend(roomInfoMapper.map(roomInfo))
}
})
}.stateIn(roomCoroutineScope, started = SharingStarted.Lazily, initialValue = initialRoomInfo)
override suspend fun subscribeToSync() = roomSyncSubscriber.subscribe(roomId) override suspend fun subscribeToSync() = roomSyncSubscriber.subscribe(roomId)
override suspend fun updateMembers() { override suspend fun updateMembers() {
@ -98,6 +109,7 @@ class RustBaseRoom(
override fun destroy() { override fun destroy() {
innerRoom.destroy() innerRoom.destroy()
roomCoroutineScope.cancel()
} }
override suspend fun userDisplayName(userId: UserId): Result<String?> = withContext(roomDispatcher) { override suspend fun userDisplayName(userId: UserId): Result<String?> = withContext(roomDispatcher) {

View file

@ -98,6 +98,7 @@ class RustRoomFactory(
coroutineDispatchers = dispatchers, coroutineDispatchers = dispatchers,
roomSyncSubscriber = roomSyncSubscriber, roomSyncSubscriber = roomSyncSubscriber,
roomMembershipObserver = roomMembershipObserver, roomMembershipObserver = roomMembershipObserver,
roomInfoMapper = roomInfoMapper,
initialRoomInfo = roomInfoMapper.map(initialRoomInfo), initialRoomInfo = roomInfoMapper.map(initialRoomInfo),
sessionCoroutineScope = sessionCoroutineScope, sessionCoroutineScope = sessionCoroutineScope,
) )
@ -127,7 +128,6 @@ class RustRoomFactory(
liveInnerTimeline = roomReferences.room.timeline(), liveInnerTimeline = roomReferences.room.timeline(),
coroutineDispatchers = dispatchers, coroutineDispatchers = dispatchers,
systemClock = systemClock, systemClock = systemClock,
roomInfoMapper = roomInfoMapper,
featureFlagService = featureFlagService, featureFlagService = featureFlagService,
) )
) )

View file

@ -0,0 +1,55 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/
package io.element.android.libraries.matrix.impl.room
import com.google.common.truth.Truth.assertThat
import io.element.android.libraries.matrix.api.room.RoomMembershipObserver
import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeRustRoom
import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeRustRoomListService
import io.element.android.libraries.matrix.test.A_DEVICE_ID
import io.element.android.libraries.matrix.test.A_SESSION_ID
import io.element.android.libraries.matrix.test.room.aRoomInfo
import io.element.android.tests.testutils.testCoroutineDispatchers
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.isActive
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.runTest
import org.junit.Test
class RustBaseRoomTest {
@Test
fun `RustBaseRoom should cancel the room coroutine scope when it is destroyed`() = runTest {
val rustBaseRoom = createRustBaseRoom(
// Not using backgroundScope here, but the test scope
sessionCoroutineScope = this
)
assertThat(rustBaseRoom.roomCoroutineScope.isActive).isTrue()
rustBaseRoom.destroy()
assertThat(rustBaseRoom.roomCoroutineScope.isActive).isFalse()
}
private fun TestScope.createRustBaseRoom(
sessionCoroutineScope: CoroutineScope,
): RustBaseRoom {
val dispatchers = testCoroutineDispatchers()
return RustBaseRoom(
sessionId = A_SESSION_ID,
deviceId = A_DEVICE_ID,
innerRoom = FakeRustRoom(),
coroutineDispatchers = dispatchers,
roomSyncSubscriber = RoomSyncSubscriber(
roomListService = FakeRustRoomListService(),
dispatchers = dispatchers,
),
roomMembershipObserver = RoomMembershipObserver(),
sessionCoroutineScope = sessionCoroutineScope,
roomInfoMapper = RoomInfoMapper(),
initialRoomInfo = aRoomInfo(),
)
}
}