Merge pull request #1378 from vector-im/feature/fga/fix_nav_restoration

DI: rework how components are created and provided

app/src/main/kotlin/io/element/android/x/MainActivity.kt

@@ -34,8 +34,8 @@ import com.bumble.appyx.core.integrationpoint.NodeComponentActivity
 import com.bumble.appyx.core.plugin.NodeReadyObserver
 import io.element.android.libraries.architecture.bindings
 import io.element.android.libraries.core.log.logger.LoggerTag
-import io.element.android.libraries.theme.ElementTheme
 import io.element.android.libraries.designsystem.utils.LocalSnackbarDispatcher
+import io.element.android.libraries.theme.ElementTheme
 import io.element.android.x.di.AppBindings
 import io.element.android.x.intent.SafeUriHandler
 import timber.log.Timber
@@ -74,7 +74,6 @@ class MainActivity : NodeComponentActivity() {
                     NodeHost(integrationPoint = appyxIntegrationPoint) {
                         MainNode(
                             it,
-                            appBindings.mainDaggerComponentOwner(),
                             plugins = listOf(
                                 object : NodeReadyObserver<MainNode> {
                                     override fun init(node: MainNode) {
@@ -83,7 +82,8 @@ class MainActivity : NodeComponentActivity() {
                                         mainNode.handleIntent(intent)
                                     }
                                 }
-                            )
+                            ),
+                            context = applicationContext
                         )
                     }
                 }

app/src/main/kotlin/io/element/android/x/MainNode.kt

@@ -16,6 +16,7 @@
 
 package io.element.android.x
 
+import android.content.Context
 import android.content.Intent
 import android.os.Parcelable
 import androidx.compose.runtime.Composable
@@ -27,24 +28,17 @@ import com.bumble.appyx.core.navigation.model.permanent.PermanentNavModel
 import com.bumble.appyx.core.node.Node
 import com.bumble.appyx.core.node.ParentNode
 import com.bumble.appyx.core.plugin.Plugin
-import io.element.android.appnav.LoggedInAppScopeFlowNode
 import io.element.android.appnav.RootFlowNode
-import io.element.android.appnav.room.RoomLoadedFlowNode
-import io.element.android.libraries.architecture.bindings
 import io.element.android.libraries.architecture.createNode
+import io.element.android.libraries.di.ApplicationContext
 import io.element.android.libraries.di.DaggerComponentOwner
-import io.element.android.libraries.matrix.api.MatrixClient
-import io.element.android.libraries.matrix.api.room.MatrixRoom
-import io.element.android.x.di.MainDaggerComponentsOwner
-import io.element.android.x.di.RoomComponent
-import io.element.android.x.di.SessionComponent
 import kotlinx.coroutines.launch
 import kotlinx.parcelize.Parcelize
 
 class MainNode(
     buildContext: BuildContext,
-    private val mainDaggerComponentOwner: MainDaggerComponentsOwner,
     plugins: List<Plugin>,
+    @ApplicationContext context: Context,
 ) : ParentNode<MainNode.RootNavTarget>(
     navModel = PermanentNavModel(
         navTargets = setOf(RootNavTarget),
@@ -53,38 +47,12 @@ class MainNode(
     buildContext = buildContext,
     plugins = plugins,
 ),
-    DaggerComponentOwner by mainDaggerComponentOwner {
+    DaggerComponentOwner {
 
-    private val loggedInFlowNodeCallback = object : LoggedInAppScopeFlowNode.LifecycleCallback {
+    override val daggerComponent = (context as DaggerComponentOwner).daggerComponent
-        override fun onFlowCreated(identifier: String, client: MatrixClient) {
-            val component = bindings<SessionComponent.ParentBindings>().sessionComponentBuilder().client(client).build()
-            mainDaggerComponentOwner.addComponent(identifier, component)
-        }
-
-        override fun onFlowReleased(identifier: String, client: MatrixClient) {
-            mainDaggerComponentOwner.removeComponent(identifier)
-        }
-    }
-
-    private val roomFlowNodeCallback = object : RoomLoadedFlowNode.LifecycleCallback {
-        override fun onFlowCreated(identifier: String, room: MatrixRoom) {
-            val component = bindings<RoomComponent.ParentBindings>().roomComponentBuilder().room(room).build()
-            mainDaggerComponentOwner.addComponent(identifier, component)
-        }
-
-        override fun onFlowReleased(identifier: String, room: MatrixRoom) {
-            mainDaggerComponentOwner.removeComponent(identifier)
-        }
-    }
 
     override fun resolve(navTarget: RootNavTarget, buildContext: BuildContext): Node {
-        return createNode<RootFlowNode>(
+        return createNode<RootFlowNode>(context = buildContext)
-            context = buildContext,
-            plugins = listOf(
-                loggedInFlowNodeCallback,
-                roomFlowNodeCallback,
-            )
-        )
     }
 
     @Composable
@@ -100,4 +68,5 @@ class MainNode(
 
     @Parcelize
     object RootNavTarget : Parcelable
+
 }

app/src/main/kotlin/io/element/android/x/di/AppBindings.kt

@@ -24,7 +24,6 @@ import io.element.android.libraries.matrix.api.tracing.TracingService
 
 @ContributesTo(AppScope::class)
 interface AppBindings {
-    fun mainDaggerComponentOwner(): MainDaggerComponentsOwner
     fun snackbarDispatcher(): SnackbarDispatcher
     fun tracingService(): TracingService
     fun bugReporter(): BugReporter

app/src/main/kotlin/io/element/android/x/di/DefaultRoomComponentFactory.kt

@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2023 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.x.di
+
+import com.squareup.anvil.annotations.ContributesBinding
+import io.element.android.appnav.di.RoomComponentFactory
+import io.element.android.libraries.di.SessionScope
+import io.element.android.libraries.matrix.api.room.MatrixRoom
+import javax.inject.Inject
+
+@ContributesBinding(SessionScope::class)
+class DefaultRoomComponentFactory @Inject constructor(
+    private val roomComponentBuilder: RoomComponent.Builder
+) : RoomComponentFactory {
+
+    override fun create(room: MatrixRoom): Any {
+        return roomComponentBuilder.room(room).build()
+    }
+}

app/src/main/kotlin/io/element/android/x/di/DefaultSessionComponentFactory.kt

@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2023 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.x.di
+
+import com.squareup.anvil.annotations.ContributesBinding
+import io.element.android.appnav.di.SessionComponentFactory
+import io.element.android.libraries.di.AppScope
+import io.element.android.libraries.matrix.api.MatrixClient
+import javax.inject.Inject
+
+@ContributesBinding(AppScope::class)
+class DefaultSessionComponentFactory @Inject constructor(
+    private val sessionComponentBuilder: SessionComponent.Builder
+) : SessionComponentFactory {
+
+    override fun create(client: MatrixClient): Any {
+        return sessionComponentBuilder.client(client).build()
+    }
+}

app/src/main/kotlin/io/element/android/x/di/MainDaggerComponentsOwner.kt

@@ -1,47 +0,0 @@
-/*
- * Copyright (c) 2023 New Vector Ltd
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package io.element.android.x.di
-
-import android.content.Context
-import io.element.android.libraries.di.AppScope
-import io.element.android.libraries.di.ApplicationContext
-import io.element.android.libraries.di.DaggerComponentOwner
-import io.element.android.libraries.di.SingleIn
-import javax.inject.Inject
-
-@SingleIn(AppScope::class)
-class MainDaggerComponentsOwner @Inject constructor(@ApplicationContext context: Context) : DaggerComponentOwner {
-
-    private val daggerComponents = LinkedHashMap<String, Any>().apply {
-        put("app", (context as DaggerComponentOwner).daggerComponent)
-    }
-
-    fun addComponent(identifier: String, component: Any) {
-        daggerComponents[identifier] = component
-    }
-
-    fun removeComponent(identifier: String) {
-        daggerComponents.remove(identifier)
-    }
-
-    /**
-     * We expose the dagger components in the opposite order they arrived.
-     * So we pick the most recent component when searching with the [io.element.android.libraries.architecture.bindings] methods.
-     */
-    override val daggerComponent: Any
-        get() = daggerComponents.values.reversed()
-}

appnav/src/main/kotlin/io/element/android/appnav/LoggedInAppScopeFlowNode.kt

@@ -31,12 +31,13 @@ import com.bumble.appyx.core.plugin.plugins
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedInject
 import io.element.android.anvilannotations.ContributesNode
+import io.element.android.appnav.di.SessionComponentFactory
 import io.element.android.libraries.architecture.NodeInputs
 import io.element.android.libraries.architecture.bindings
 import io.element.android.libraries.architecture.createNode
 import io.element.android.libraries.architecture.inputs
-import io.element.android.libraries.architecture.waitForChildAttached
 import io.element.android.libraries.di.AppScope
+import io.element.android.libraries.di.DaggerComponentOwner
 import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.ui.di.MatrixUIBindings
 import kotlinx.parcelize.Parcelize
@@ -50,6 +51,7 @@ import kotlinx.parcelize.Parcelize
 class LoggedInAppScopeFlowNode @AssistedInject constructor(
     @Assisted buildContext: BuildContext,
     @Assisted plugins: List<Plugin>,
+    sessionComponentFactory: SessionComponentFactory,
 ) : ParentNode<LoggedInAppScopeFlowNode.NavTarget>(
     navModel = PermanentNavModel(
         navTargets = setOf(NavTarget),
@@ -57,7 +59,7 @@ class LoggedInAppScopeFlowNode @AssistedInject constructor(
     ),
     buildContext = buildContext,
     plugins = plugins
-) {
+), DaggerComponentOwner {
     interface Callback : Plugin {
         fun onOpenBugReport()
     }
@@ -65,29 +67,20 @@ class LoggedInAppScopeFlowNode @AssistedInject constructor(
     @Parcelize
     object NavTarget : Parcelable
 
-    interface LifecycleCallback : NodeLifecycleCallback {
-        fun onFlowCreated(identifier: String, client: MatrixClient)
-
-        fun onFlowReleased(identifier: String, client: MatrixClient)
-    }
-
     data class Inputs(
         val matrixClient: MatrixClient
     ) : NodeInputs
 
     private val inputs: Inputs = inputs()
+    override val daggerComponent = sessionComponentFactory.create(inputs.matrixClient)
 
     override fun onBuilt() {
         super.onBuilt()
         lifecycle.subscribe(
             onCreate = {
-                plugins<LifecycleCallback>().forEach { it.onFlowCreated(id, inputs.matrixClient) }
                 val imageLoaderFactory = bindings<MatrixUIBindings>().loggedInImageLoaderFactory()
                 Coil.setImageLoader(imageLoaderFactory)
             },
-            onDestroy = {
-                plugins<LifecycleCallback>().forEach { it.onFlowReleased(id, inputs.matrixClient) }
-            }
         )
     }
 
@@ -97,13 +90,10 @@ class LoggedInAppScopeFlowNode @AssistedInject constructor(
                 plugins<Callback>().forEach { it.onOpenBugReport() }
             }
         }
-        val nodeLifecycleCallbacks = plugins<NodeLifecycleCallback>()
+        return createNode<LoggedInFlowNode>(buildContext, listOf(callback))
-        return createNode<LoggedInFlowNode>(buildContext, nodeLifecycleCallbacks + callback)
     }
 
-    suspend fun attachSession(): LoggedInFlowNode {
+    suspend fun attachSession(): LoggedInFlowNode = waitForChildAttached()
-        return waitForChildAttached { _ -> true }
-    }
 
     @Composable
     override fun View(modifier: Modifier) {

appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt

@@ -227,14 +227,13 @@ class LoggedInFlowNode @AssistedInject constructor(
                     .build()
             }
             is NavTarget.Room -> {
-                val nodeLifecycleCallbacks = plugins<NodeLifecycleCallback>()
                 val callback = object : RoomLoadedFlowNode.Callback {
                     override fun onForwardedToSingleRoom(roomId: RoomId) {
                         coroutineScope.launch { attachRoom(roomId) }
                     }
                 }
                 val inputs = RoomFlowNode.Inputs(roomId = navTarget.roomId, initialElement = navTarget.initialElement)
-                createNode<RoomFlowNode>(buildContext, plugins = listOf(inputs, callback) + nodeLifecycleCallbacks)
+                createNode<RoomFlowNode>(buildContext, plugins = listOf(inputs, callback))
             }
             NavTarget.Settings -> {
                 val callback = object : PreferencesEntryPoint.Callback {

appnav/src/main/kotlin/io/element/android/appnav/RootFlowNode.kt

@@ -29,7 +29,6 @@ import com.bumble.appyx.core.modality.BuildContext
 import com.bumble.appyx.core.node.Node
 import com.bumble.appyx.core.node.node
 import com.bumble.appyx.core.plugin.Plugin
-import com.bumble.appyx.core.plugin.plugins
 import com.bumble.appyx.core.state.MutableSavedStateMap
 import com.bumble.appyx.navmodel.backstack.BackStack
 import com.bumble.appyx.navmodel.backstack.operation.pop
@@ -196,8 +195,7 @@ class RootFlowNode @AssistedInject constructor(
                         backstack.push(NavTarget.BugReport)
                     }
                 }
-                val nodeLifecycleCallbacks = plugins<NodeLifecycleCallback>()
+                createNode<LoggedInAppScopeFlowNode>(buildContext, plugins = listOf(inputs, callback))
-                createNode<LoggedInAppScopeFlowNode>(buildContext, plugins = listOf(inputs, callback) + nodeLifecycleCallbacks)
             }
             NavTarget.NotLoggedInFlow -> createNode<NotLoggedInFlowNode>(buildContext)
             NavTarget.SplashScreen -> splashNode(buildContext)

appnav/src/main/kotlin/io/element/android/appnav/di/RoomComponentFactory.kt

@@ -14,8 +14,10 @@
  * limitations under the License.
  */
 
-package io.element.android.appnav
+package io.element.android.appnav.di
 
-import com.bumble.appyx.core.plugin.Plugin
+import io.element.android.libraries.matrix.api.room.MatrixRoom
 
-interface NodeLifecycleCallback : Plugin
+interface RoomComponentFactory {
+    fun create(room: MatrixRoom): Any
+}

appnav/src/main/kotlin/io/element/android/appnav/di/SessionComponentFactory.kt

@@ -0,0 +1,23 @@
+/*
+ * Copyright (c) 2023 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.element.android.appnav.di
+
+import io.element.android.libraries.matrix.api.MatrixClient
+
+interface SessionComponentFactory {
+    fun create(client: MatrixClient): Any
+}

appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt

@@ -36,7 +36,6 @@ import com.bumble.appyx.navmodel.backstack.operation.newRoot
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedInject
 import io.element.android.anvilannotations.ContributesNode
-import io.element.android.appnav.NodeLifecycleCallback
 import io.element.android.features.networkmonitor.api.NetworkMonitor
 import io.element.android.features.networkmonitor.api.NetworkStatus
 import io.element.android.libraries.architecture.BackstackNode
@@ -103,12 +102,11 @@ class RoomFlowNode @AssistedInject constructor(
     override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node {
         return when (navTarget) {
             NavTarget.Loaded -> {
-                val nodeLifecycleCallbacks = plugins<NodeLifecycleCallback>()
                 val roomFlowNodeCallback = plugins<RoomLoadedFlowNode.Callback>()
                 val awaitRoomState = loadingRoomStateStateFlow.value
                 if (awaitRoomState is LoadingRoomState.Loaded) {
                     val inputs = RoomLoadedFlowNode.Inputs(awaitRoomState.room, initialElement = inputs.initialElement)
-                    createNode<RoomLoadedFlowNode>(buildContext, plugins = listOf(inputs) + roomFlowNodeCallback + nodeLifecycleCallbacks)
+                    createNode<RoomLoadedFlowNode>(buildContext, plugins = listOf(inputs) + roomFlowNodeCallback)
                 } else {
                     loadingNode(buildContext, this::navigateUp)
                 }

appnav/src/main/kotlin/io/element/android/appnav/room/RoomLoadedFlowNode.kt

@@ -27,19 +27,19 @@ 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
-import com.bumble.appyx.core.plugin.plugins
 import com.bumble.appyx.navmodel.backstack.BackStack
 import com.bumble.appyx.navmodel.backstack.operation.push
 import dagger.assisted.Assisted
 import dagger.assisted.AssistedInject
 import io.element.android.anvilannotations.ContributesNode
-import io.element.android.appnav.NodeLifecycleCallback
+import io.element.android.appnav.di.RoomComponentFactory
 import io.element.android.features.messages.api.MessagesEntryPoint
 import io.element.android.features.roomdetails.api.RoomDetailsEntryPoint
 import io.element.android.libraries.architecture.BackstackNode
 import io.element.android.libraries.architecture.NodeInputs
 import io.element.android.libraries.architecture.animation.rememberDefaultTransitionHandler
 import io.element.android.libraries.architecture.inputs
+import io.element.android.libraries.di.DaggerComponentOwner
 import io.element.android.libraries.di.SessionScope
 import io.element.android.libraries.matrix.api.core.RoomId
 import io.element.android.libraries.matrix.api.core.UserId
@@ -60,6 +60,7 @@ class RoomLoadedFlowNode @AssistedInject constructor(
     private val messagesEntryPoint: MessagesEntryPoint,
     private val roomDetailsEntryPoint: RoomDetailsEntryPoint,
     private val appNavigationStateService: AppNavigationStateService,
+    roomComponentFactory: RoomComponentFactory,
     roomMembershipObserver: RoomMembershipObserver,
 ) : BackstackNode<RoomLoadedFlowNode.NavTarget>(
     backstack = BackStack(
@@ -68,17 +69,12 @@ class RoomLoadedFlowNode @AssistedInject constructor(
     ),
     buildContext = buildContext,
     plugins = plugins,
-) {
+), DaggerComponentOwner {
 
     interface Callback : Plugin {
         fun onForwardedToSingleRoom(roomId: RoomId)
     }
 
-    interface LifecycleCallback : NodeLifecycleCallback {
-        fun onFlowCreated(identifier: String, room: MatrixRoom)
-        fun onFlowReleased(identifier: String, room: MatrixRoom)
-    }
-
     data class Inputs(
         val room: MatrixRoom,
         val initialElement: NavTarget = NavTarget.Messages,
@@ -86,18 +82,17 @@ class RoomLoadedFlowNode @AssistedInject constructor(
 
     private val inputs: Inputs = inputs()
     private val callbacks = plugins.filterIsInstance<Callback>()
+    override val daggerComponent = roomComponentFactory.create(inputs.room)
 
     init {
         lifecycle.subscribe(
             onCreate = {
-                Timber.v("OnCreate")
+                Timber.v("OnCreate => ${inputs.room.roomId}")
-                plugins<LifecycleCallback>().forEach { it.onFlowCreated(id, inputs.room) }
                 appNavigationStateService.onNavigateToRoom(id, inputs.room.roomId)
                 fetchRoomMembers()
             },
             onDestroy = {
                 Timber.v("OnDestroy")
-                plugins<LifecycleCallback>().forEach { it.onFlowReleased(id, inputs.room) }
                 appNavigationStateService.onLeavingRoom(id)
             }
         )

appnav/src/test/kotlin/io/element/android/appnav/RoomFlowNodeTest.kt

@@ -26,10 +26,12 @@ import com.bumble.appyx.navmodel.backstack.activeElement
 import com.bumble.appyx.testing.junit4.util.MainDispatcherRule
 import com.bumble.appyx.testing.unit.common.helper.parentNodeTestHelper
 import com.google.common.truth.Truth
+import io.element.android.appnav.di.RoomComponentFactory
 import io.element.android.appnav.room.RoomLoadedFlowNode
 import io.element.android.features.messages.api.MessagesEntryPoint
 import io.element.android.features.roomdetails.api.RoomDetailsEntryPoint
 import io.element.android.libraries.architecture.childNode
+import io.element.android.libraries.matrix.api.room.MatrixRoom
 import io.element.android.libraries.matrix.api.room.RoomMembershipObserver
 import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
 import io.element.android.services.appnavstate.test.FakeAppNavigationStateService
@@ -57,6 +59,12 @@ class RoomFlowNodeTest {
         }
     }
 
+    private class FakeRoomComponentFactory : RoomComponentFactory {
+        override fun create(room: MatrixRoom): Any {
+            return Unit
+        }
+    }
+
     private class FakeRoomDetailsEntryPoint : RoomDetailsEntryPoint {
 
         var nodeId: String? = null
@@ -83,7 +91,8 @@ class RoomFlowNodeTest {
         messagesEntryPoint = messagesEntryPoint,
         roomDetailsEntryPoint = roomDetailsEntryPoint,
         appNavigationStateService = FakeAppNavigationStateService(),
-        roomMembershipObserver = RoomMembershipObserver()
+        roomMembershipObserver = RoomMembershipObserver(),
+        roomComponentFactory = FakeRoomComponentFactory(),
     )
 
     @Test

changelog.d/1378.misc

@@ -0,0 +1 @@
+[DI] Rework how dagger components are created and provided.