misc (space) : ensure SpaceRoomList is destroyed

2025-10-01 10:49:35 +02:00
parent 250667b5b5
commit 388cb31700
12 changed files with 120 additions and 66 deletions
--- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/SpaceFlowNode.kt
+++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/SpaceFlowNode.kt
@@ -13,6 +13,7 @@ import android.os.Parcelable
 import androidx.compose.material3.ExperimentalMaterial3Api
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
+import com.bumble.appyx.core.lifecycle.subscribe
 import com.bumble.appyx.core.modality.BuildContext
 import com.bumble.appyx.core.node.Node
 import com.bumble.appyx.core.plugin.Plugin
@@ -22,13 +23,16 @@ import dev.zacsweers.metro.Assisted
 import dev.zacsweers.metro.AssistedInject
 import io.element.android.annotations.ContributesNode
 import io.element.android.features.space.api.SpaceEntryPoint
+import io.element.android.features.space.impl.di.SpaceFlowGraph
 import io.element.android.features.space.impl.leave.LeaveSpaceNode
 import io.element.android.features.space.impl.root.SpaceNode
 import io.element.android.libraries.architecture.BackstackView
 import io.element.android.libraries.architecture.BaseFlowNode
 import io.element.android.libraries.architecture.createNode
 import io.element.android.libraries.architecture.inputs
+import io.element.android.libraries.di.DependencyInjectionGraphOwner
 import io.element.android.libraries.di.SessionScope
+import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.core.RoomId
 import kotlinx.parcelize.Parcelize

@@ -37,6 +41,8 @@ import kotlinx.parcelize.Parcelize
 class SpaceFlowNode(
    @Assisted val buildContext: BuildContext,
    @Assisted plugins: List<Plugin>,
+    matrixClient: MatrixClient,
+    graphFactory: SpaceFlowGraph.Factory,
 ) : BaseFlowNode<SpaceFlowNode.NavTarget>(
    backstack = BackStack(
        initialElement = NavTarget.Root,
@@ -44,9 +50,11 @@ class SpaceFlowNode(
    ),
    buildContext = buildContext,
    plugins = plugins,
-) {
+), DependencyInjectionGraphOwner {
    private val inputs: SpaceEntryPoint.Inputs = inputs()
    private val callback = plugins.filterIsInstance<SpaceEntryPoint.Callback>().single()
+    private val spaceRoomList = matrixClient.spaceService.spaceRoomList(inputs.roomId)
+    override val graph = graphFactory.create(spaceRoomList)

    sealed interface NavTarget : Parcelable {
        @Parcelize
@@ -56,6 +64,15 @@ class SpaceFlowNode(
        data object Leave : NavTarget
    }

+    override fun onBuilt() {
+        super.onBuilt()
+        lifecycle.subscribe(
+            onDestroy = {
+                spaceRoomList.destroy()
+            }
+        )
+    }
+
    override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
        return when (navTarget) {
            NavTarget.Leave -> {
--- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/di/SpaceFlowGraph.kt
+++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/di/SpaceFlowGraph.kt
@@ -0,0 +1,24 @@
+/*
+ * Copyright 2024 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.features.space.impl.di
+
+import dev.zacsweers.metro.ContributesTo
+import dev.zacsweers.metro.GraphExtension
+import dev.zacsweers.metro.Provides
+import io.element.android.libraries.architecture.NodeFactoriesBindings
+import io.element.android.libraries.di.SessionScope
+import io.element.android.libraries.matrix.api.spaces.SpaceRoomList
+
+@GraphExtension(SpaceFlowScope::class)
+interface SpaceFlowGraph : NodeFactoriesBindings {
+    @ContributesTo(SessionScope::class)
+    @GraphExtension.Factory
+    interface Factory {
+        fun create(@Provides spaceRoomList: SpaceRoomList): SpaceFlowGraph
+    }
+}
--- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/di/SpaceFlowScope.kt
+++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/di/SpaceFlowScope.kt
@@ -0,0 +1,10 @@
+/*
+ * Copyright 2024 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.features.space.impl.di
+
+abstract class SpaceFlowScope private constructor()
--- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/leave/LeaveSpaceNode.kt
+++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/leave/LeaveSpaceNode.kt
@@ -15,20 +15,15 @@ import com.bumble.appyx.core.plugin.Plugin
 import dev.zacsweers.metro.Assisted
 import dev.zacsweers.metro.AssistedInject
 import io.element.android.annotations.ContributesNode
-import io.element.android.features.space.api.SpaceEntryPoint
-import io.element.android.libraries.architecture.inputs
-import io.element.android.libraries.di.SessionScope
+import io.element.android.features.space.impl.di.SpaceFlowScope

-@ContributesNode(SessionScope::class)
+@ContributesNode(SpaceFlowScope::class)
@AssistedInject
 class LeaveSpaceNode(
    @Assisted buildContext: BuildContext,
    @Assisted plugins: List<Plugin>,
-    presenterFactory: LeaveSpacePresenter.Factory,
+    private val presenter: LeaveSpacePresenter,
 ) : Node(buildContext, plugins = plugins) {
-    private val inputs: SpaceEntryPoint.Inputs = inputs()
-    private val presenter = presenterFactory.create(inputs)
-
    @Composable
    override fun View(modifier: Modifier) {
        val state = presenter.present()
--- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/leave/LeaveSpacePresenter.kt
+++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/leave/LeaveSpacePresenter.kt
@@ -15,17 +15,14 @@ import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.produceState
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
-import dev.zacsweers.metro.Assisted
-import dev.zacsweers.metro.AssistedFactory
-import dev.zacsweers.metro.AssistedInject
-import io.element.android.features.space.api.SpaceEntryPoint
+import dev.zacsweers.metro.Inject
 import io.element.android.libraries.architecture.AsyncAction
 import io.element.android.libraries.architecture.AsyncData
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.architecture.runUpdatingState
-import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.core.RoomId
 import io.element.android.libraries.matrix.api.spaces.SpaceRoom
+import io.element.android.libraries.matrix.api.spaces.SpaceRoomList
 import kotlinx.collections.immutable.ImmutableList
 import kotlinx.collections.immutable.ImmutableSet
 import kotlinx.collections.immutable.persistentSetOf
@@ -35,18 +32,10 @@ import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.launch
 import kotlin.jvm.optionals.getOrNull

-@AssistedInject
+@Inject
 class LeaveSpacePresenter(
-    @Assisted private val inputs: SpaceEntryPoint.Inputs,
-    matrixClient: MatrixClient,
+    private val spaceRoomList: SpaceRoomList,
 ) : Presenter<LeaveSpaceState> {
-    @AssistedFactory
-    fun interface Factory {
-        fun create(inputs: SpaceEntryPoint.Inputs): LeaveSpacePresenter
-    }
-
-    private val spaceRoomList = matrixClient.spaceService.spaceRoomList(inputs.roomId)
-
    @Composable
    override fun present(): LeaveSpaceState {
        val coroutineScope = rememberCoroutineScope()
--- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpaceNode.kt
+++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpaceNode.kt
@@ -18,36 +18,33 @@ import com.bumble.appyx.core.plugin.Plugin
 import dev.zacsweers.metro.Assisted
 import dev.zacsweers.metro.AssistedInject
 import io.element.android.annotations.ContributesNode
-import io.element.android.features.space.api.SpaceEntryPoint
+import io.element.android.features.space.impl.di.SpaceFlowScope
 import io.element.android.libraries.androidutils.R
 import io.element.android.libraries.androidutils.system.startSharePlainTextIntent
-import io.element.android.libraries.architecture.inputs
-import io.element.android.libraries.di.SessionScope
 import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.core.RoomId
+import io.element.android.libraries.matrix.api.spaces.SpaceRoomList
 import io.element.android.libraries.ui.strings.CommonStrings
 import kotlinx.coroutines.launch
 import timber.log.Timber

-@ContributesNode(SessionScope::class)
+@ContributesNode(SpaceFlowScope::class)
@AssistedInject
 class SpaceNode(
    @Assisted buildContext: BuildContext,
    @Assisted plugins: List<Plugin>,
-    presenterFactory: SpacePresenter.Factory,
+    private val presenter: SpacePresenter,
    private val matrixClient: MatrixClient,
+    private val spaceRoomList: SpaceRoomList,
 ) : Node(buildContext, plugins = plugins) {
    interface Callback : Plugin {
        fun onOpenRoom(roomId: RoomId, viaParameters: List<String>)
        fun onLeaveSpace()
    }
-
-    private val inputs: SpaceEntryPoint.Inputs = inputs()
    private val callback = plugins.filterIsInstance<Callback>().single()
-    private val presenter = presenterFactory.create(inputs)

    private fun onShareRoom(context: Context) = lifecycleScope.launch {
-        matrixClient.getRoom(inputs.roomId)?.use { room ->
+        matrixClient.getRoom(spaceRoomList.roomId)?.use { room ->
            room.getPermalink()
                .onSuccess { permalink ->
                    context.startSharePlainTextIntent(
--- a/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpacePresenter.kt
+++ b/features/space/impl/src/main/kotlin/io/element/android/features/space/impl/root/SpacePresenter.kt
@@ -13,11 +13,8 @@ import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
-import dev.zacsweers.metro.Assisted
-import dev.zacsweers.metro.AssistedFactory
-import dev.zacsweers.metro.AssistedInject
+import dev.zacsweers.metro.Inject
 import io.element.android.features.invite.api.SeenInvitesStore
-import io.element.android.features.space.api.SpaceEntryPoint
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.core.coroutine.mapState
 import io.element.android.libraries.matrix.api.MatrixClient
@@ -31,19 +28,12 @@ import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.launch
 import kotlin.jvm.optionals.getOrNull

-@AssistedInject
+@Inject
 class SpacePresenter(
-    @Assisted private val inputs: SpaceEntryPoint.Inputs,
+    private val spaceRoomList: SpaceRoomList,
    private val client: MatrixClient,
    private val seenInvitesStore: SeenInvitesStore,
 ) : Presenter<SpaceState> {
-    @AssistedFactory
-    fun interface Factory {
-        fun create(inputs: SpaceEntryPoint.Inputs): SpacePresenter
-    }
-
-    private val spaceRoomList = client.spaceService.spaceRoomList(inputs.roomId)
-
    @Composable
    override fun present(): SpaceState {
        LaunchedEffect(Unit) {
--- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/spaces/SpaceRoomList.kt
+++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/spaces/SpaceRoomList.kt
@@ -7,6 +7,7 @@

 package io.element.android.libraries.matrix.api.spaces

+import io.element.android.libraries.matrix.api.core.RoomId
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.StateFlow
 import java.util.Optional
@@ -17,9 +18,13 @@ interface SpaceRoomList {
        data class Idle(val hasMoreToLoad: Boolean) : PaginationStatus
    }

+    val roomId: RoomId
+
    val currentSpaceFlow: StateFlow<Optional<SpaceRoom>>

    val spaceRoomsFlow: Flow<List<SpaceRoom>>
    val paginationStatusFlow: StateFlow<PaginationStatus>
    suspend fun paginate(): Result<Unit>
+
+    fun destroy()
 }
--- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceRoomList.kt
+++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceRoomList.kt
@@ -8,26 +8,31 @@
 package io.element.android.libraries.matrix.impl.spaces

 import io.element.android.libraries.core.extensions.runCatchingExceptions
+import io.element.android.libraries.matrix.api.core.RoomId
 import io.element.android.libraries.matrix.api.spaces.SpaceRoom
 import io.element.android.libraries.matrix.api.spaces.SpaceRoomList
 import kotlinx.coroutines.CompletableDeferred
 import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.cancel
 import kotlinx.coroutines.flow.MutableSharedFlow
 import kotlinx.coroutines.flow.MutableStateFlow
-import kotlinx.coroutines.flow.collect
+import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
+import timber.log.Timber
 import uniffi.matrix_sdk_ui.SpaceRoomListPaginationState
 import java.util.Optional
 import org.matrix.rustcomponents.sdk.SpaceRoomList as InnerSpaceRoomList

 class RustSpaceRoomList(
+    override val roomId: RoomId,
    private val innerProvider: suspend () -> InnerSpaceRoomList,
-    sessionCoroutineScope: CoroutineScope,
+    private val coroutineScope: CoroutineScope,
    spaceRoomMapper: SpaceRoomMapper,
 ) : SpaceRoomList {
-    private val inner = CompletableDeferred<InnerSpaceRoomList>()
+    private val innerCompletable = CompletableDeferred<InnerSpaceRoomList>()

    override val currentSpaceFlow = MutableStateFlow<Optional<SpaceRoom>>(Optional.empty())

@@ -41,37 +46,45 @@ class RustSpaceRoomList(
    )

    init {
-        sessionCoroutineScope.launch {
-            inner.complete(innerProvider())
-        }
-        sessionCoroutineScope.launch {
-            inner.await().paginationStateFlow()
+        coroutineScope.launch {
+            val inner = innerProvider()
+            innerCompletable.complete(inner)
+
+            inner.paginationStateFlow()
                .onEach { paginationStatus ->
                    paginationStatusFlow.emit(paginationStatus.into())
                }
-                .collect()
-        }
+                .launchIn(this)

-        sessionCoroutineScope.launch {
-            inner.await().spaceListUpdateFlow()
+            inner.spaceListUpdateFlow()
                .onEach { updates ->
                    spaceListUpdateProcessor.postUpdates(updates)
                }
-                .collect()
-        }
-        sessionCoroutineScope.launch {
-            inner.await().spaceUpdateFlow()
+                .launchIn(this)
+
+            inner.spaceUpdateFlow()
                .map { space -> space.map(spaceRoomMapper::map) }
                .onEach { space ->
                    currentSpaceFlow.emit(space)
                }
-                .collect()
+                .launchIn(this)
        }
    }

    override suspend fun paginate(): Result<Unit> {
        return runCatchingExceptions {
-            inner.await().paginate()
+            innerCompletable.await().paginate()
+        }
+    }
+
+    @OptIn(ExperimentalCoroutinesApi::class)
+    override fun destroy() {
+        Timber.d("Destroying SpaceRoomList $roomId")
+        coroutineScope.cancel()
+        try {
+            innerCompletable.getCompleted().destroy()
+        } catch (_: Exception) {
+            // Ignore, we just want to make sure it's completed
        }
    }

--- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceService.kt
+++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceService.kt
@@ -7,6 +7,7 @@

 package io.element.android.libraries.matrix.impl.spaces

+import io.element.android.libraries.core.coroutine.childScope
 import io.element.android.libraries.core.extensions.runCatchingExceptions
 import io.element.android.libraries.matrix.api.core.RoomId
 import io.element.android.libraries.matrix.api.spaces.SpaceRoom
@@ -54,9 +55,11 @@ class RustSpaceService(
    }

    override fun spaceRoomList(id: RoomId): SpaceRoomList {
+        val childCoroutineScope = sessionCoroutineScope.childScope(sessionDispatcher, "SpaceRoomListScope-$this")
        return RustSpaceRoomList(
+            roomId = id,
            innerProvider = { innerSpaceService.spaceRoomList(id.value) },
-            sessionCoroutineScope = sessionCoroutineScope,
+            coroutineScope = childCoroutineScope,
            spaceRoomMapper = spaceRoomMapper,
        )
    }
--- a/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceRoomListTest.kt
+++ b/libraries/matrix/impl/src/test/kotlin/io/element/android/libraries/matrix/impl/spaces/RustSpaceRoomListTest.kt
@@ -11,9 +11,11 @@ package io.element.android.libraries.matrix.impl.spaces

 import app.cash.turbine.test
 import com.google.common.truth.Truth.assertThat
+import io.element.android.libraries.matrix.api.core.RoomId
 import io.element.android.libraries.matrix.api.spaces.SpaceRoomList
 import io.element.android.libraries.matrix.impl.fixtures.factories.aRustSpaceRoom
 import io.element.android.libraries.matrix.impl.fixtures.fakes.FakeFfiSpaceRoomList
+import io.element.android.libraries.matrix.test.A_ROOM_ID
 import io.element.android.libraries.matrix.test.A_ROOM_ID_2
 import io.element.android.tests.testutils.lambda.lambdaRecorder
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -84,13 +86,15 @@ class RustSpaceRoomListTest {
    }

    private fun TestScope.createRustSpaceRoomList(
+        roomId: RoomId = A_ROOM_ID,
        innerSpaceRoomList: InnerSpaceRoomList = FakeFfiSpaceRoomList(),
        innerProvider: suspend () -> InnerSpaceRoomList = { innerSpaceRoomList },
        spaceRoomMapper: SpaceRoomMapper = SpaceRoomMapper(),
    ): RustSpaceRoomList {
        return RustSpaceRoomList(
+            roomId = roomId,
            innerProvider = innerProvider,
-            sessionCoroutineScope = backgroundScope,
+            coroutineScope = backgroundScope,
            spaceRoomMapper = spaceRoomMapper,
        )
    }
--- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/spaces/FakeSpaceRoomList.kt
+++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/spaces/FakeSpaceRoomList.kt
@@ -7,8 +7,10 @@

 package io.element.android.libraries.matrix.test.spaces

+import io.element.android.libraries.matrix.api.core.RoomId
 import io.element.android.libraries.matrix.api.spaces.SpaceRoom
 import io.element.android.libraries.matrix.api.spaces.SpaceRoomList
+import io.element.android.libraries.matrix.test.A_ROOM_ID
 import io.element.android.tests.testutils.lambda.lambdaError
 import io.element.android.tests.testutils.simulateLongTask
 import kotlinx.coroutines.flow.Flow
@@ -18,6 +20,7 @@ import kotlinx.coroutines.flow.asStateFlow
 import java.util.Optional

 class FakeSpaceRoomList(
+    override val roomId: RoomId = A_ROOM_ID,
    initialSpaceFlowValue: SpaceRoom? = null,
    initialSpaceRoomsValue: List<SpaceRoom> = emptyList(),
    initialSpaceRoomList: SpaceRoomList.PaginationStatus = SpaceRoomList.PaginationStatus.Loading,
@@ -47,4 +50,8 @@ class FakeSpaceRoomList(
    override suspend fun paginate(): Result<Unit> = simulateLongTask {
        paginateResult()
    }
+
+    override fun destroy() {
+        // No op
+    }
 }