From b504dbe5f0205e1f2ee4d045d946f2ad290c5f25 Mon Sep 17 00:00:00 2001 From: Marco Romano Date: Thu, 29 Jun 2023 10:48:55 +0200 Subject: [PATCH] MatrixRoom API refinement (#719) - `syncUpdateFlow` becomes a `val` and always returns the same instance of the underlying `StateFlow` instead of different `Flow` instances to allow consumers not to remember the `Flow` and not to specify an unneeded initial value. - `timeline` becomes a `val` as it already always returns the same instance. - Amends calling code accordingly - Removes a few unneeded `val`s in `RustMatrixClient - Fixes a small bug in `MessagesPresenter` that allowed to sometime show a newly created room's name as "Empty room" (changes `LaunchedEffect(syncUpdateFlow)` to `LaunchedEffect(syncUpdateFlow.value)`) --- .../messages/impl/MessagesPresenter.kt | 4 +-- .../impl/timeline/TimelinePresenter.kt | 2 +- .../impl/edit/RoomDetailsEditPresenter.kt | 2 +- .../libraries/matrix/api/room/MatrixRoom.kt | 4 +-- .../libraries/matrix/impl/RustMatrixClient.kt | 4 +-- .../matrix/impl/room/RustMatrixRoom.kt | 28 ++++++++----------- .../matrix/test/room/FakeMatrixRoom.kt | 11 ++------ .../android/samples/minimal/RoomListScreen.kt | 2 +- 8 files changed, 23 insertions(+), 34 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 945c7cfd7c..6cd2ec62aa 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 @@ -99,7 +99,7 @@ class MessagesPresenter @AssistedInject constructor( val customReactionState = customReactionPresenter.present() val retryState = retrySendMenuPresenter.present() - val syncUpdateFlow = room.syncUpdateFlow().collectAsState(0L) + val syncUpdateFlow = room.syncUpdateFlow.collectAsState() val userHasPermissionToSendMessage by room.canSendEventAsState(type = MessageEventType.ROOM_MESSAGE, updateKey = syncUpdateFlow.value) val roomName: MutableState = rememberSaveable { mutableStateOf(null) @@ -112,7 +112,7 @@ class MessagesPresenter @AssistedInject constructor( val snackbarMessage by snackbarDispatcher.collectSnackbarMessageAsState() - LaunchedEffect(syncUpdateFlow) { + LaunchedEffect(syncUpdateFlow.value) { roomAvatar.value = AvatarData( id = room.roomId.value, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt index 26c0bedf01..f34bc284e1 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelinePresenter.kt @@ -43,7 +43,7 @@ class TimelinePresenter @Inject constructor( room: MatrixRoom, ) : Presenter { - private val timeline = room.timeline() + private val timeline = room.timeline @Composable override fun present(): TimelineState { diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt index d4d6643038..079708c560 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/edit/RoomDetailsEditPresenter.kt @@ -52,7 +52,7 @@ class RoomDetailsEditPresenter @Inject constructor( @Composable override fun present(): RoomDetailsEditState { - val roomSyncUpdateFlow = room.syncUpdateFlow().collectAsState(0L) + val roomSyncUpdateFlow = room.syncUpdateFlow.collectAsState() // Since there is no way to obtain the new avatar uri after uploading a new avatar, // just erase the local value when the room field has changed 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 de815119fe..57c7f27767 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 @@ -58,9 +58,9 @@ interface MatrixRoom : Closeable { */ suspend fun updateMembers(): Result - fun syncUpdateFlow(): Flow + val syncUpdateFlow: StateFlow - fun timeline(): MatrixTimeline + val timeline: MatrixTimeline fun open(): Result diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt index 060f8fc0a9..6cfa71d097 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt @@ -73,10 +73,10 @@ import org.matrix.rustcomponents.sdk.RoomVisibility as RustRoomVisibility class RustMatrixClient constructor( private val client: Client, private val sessionStore: SessionStore, - private val appCoroutineScope: CoroutineScope, + appCoroutineScope: CoroutineScope, private val dispatchers: CoroutineDispatchers, private val baseDirectory: File, - private val baseCacheDirectory: File, + baseCacheDirectory: File, private val clock: SystemClock, ) : MatrixClient { 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 30d339c0fa..b6ef6d6ea7 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 @@ -44,6 +44,7 @@ import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch @@ -71,15 +72,10 @@ class RustMatrixRoom( override val roomId = RoomId(innerRoom.id()) private val roomCoroutineScope = sessionCoroutineScope.childScope(coroutineDispatchers.main, "RoomScope-$roomId") - - override val membersStateFlow: StateFlow - get() = _membersStateFlow - - private var _membersStateFlow = MutableStateFlow(MatrixRoomMembersState.Unknown) + private val _membersStateFlow = MutableStateFlow(MatrixRoomMembersState.Unknown) private val isInit = MutableStateFlow(false) - private val syncUpdateFlow = MutableStateFlow(systemClock.epochMillis()) - - private val timeline by lazy { + private val _syncUpdateFlow = MutableStateFlow(0L) + private val _timeline by lazy { RustMatrixTimeline( matrixRoom = this, innerRoom = innerRoom, @@ -88,13 +84,11 @@ class RustMatrixRoom( ) } - override fun syncUpdateFlow(): Flow { - return syncUpdateFlow - } + override val membersStateFlow: StateFlow = _membersStateFlow.asStateFlow() - override fun timeline(): MatrixTimeline { - return timeline - } + override val syncUpdateFlow: StateFlow = _syncUpdateFlow.asStateFlow() + + override val timeline: MatrixTimeline = _timeline override fun open(): Result { if (isInit.value) return Result.failure(IllegalStateException("Listener already registered")) @@ -110,10 +104,10 @@ class RustMatrixRoom( roomListItem.subscribe(settings) roomCoroutineScope.launch(coroutineDispatchers.computation) { innerRoom.timelineDiffFlow { initialList -> - timeline.postItems(initialList) + _timeline.postItems(initialList) }.onEach { - syncUpdateFlow.value = systemClock.epochMillis() - timeline.postDiff(it) + _syncUpdateFlow.value = systemClock.epochMillis() + _timeline.postDiff(it) }.launchIn(this) fetchMembers() } 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 5a9f4bf356..2555f68226 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 @@ -34,9 +34,8 @@ import io.element.android.libraries.matrix.test.A_ROOM_ID import io.element.android.libraries.matrix.test.A_SESSION_ID import io.element.android.libraries.matrix.test.timeline.FakeMatrixTimeline import io.element.android.tests.testutils.simulateLongTask -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.emptyFlow +import kotlinx.coroutines.flow.StateFlow import java.io.File class FakeMatrixRoom( @@ -126,13 +125,9 @@ class FakeMatrixRoom( updateMembersResult } - override fun syncUpdateFlow(): Flow { - return emptyFlow() - } + override val syncUpdateFlow: StateFlow = MutableStateFlow(0L) - override fun timeline(): MatrixTimeline { - return matrixTimeline - } + override val timeline: MatrixTimeline = matrixTimeline override fun open(): Result { return Result.success(Unit) diff --git a/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/RoomListScreen.kt b/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/RoomListScreen.kt index abb9c6852a..c5b3911687 100644 --- a/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/RoomListScreen.kt +++ b/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/RoomListScreen.kt @@ -82,7 +82,7 @@ class RoomListScreen( withContext(coroutineDispatchers.io) { matrixClient.getRoom(roomId)!!.use { room -> room.open() - val timeline = room.timeline() + val timeline = room.timeline timeline.apply { // TODO This doesn't work reliably as initialize is asynchronous, and the timeline can't be used until it's finished paginateBackwards(20, 50)