Update forwarded result handling to resolve several bugs with behavio…

…ur when result handling was occurring outside of the same container

CHANGELOG.md

@@ -7,6 +7,8 @@
 * Added extensions for `findContext` and `findActiveContext` to `NavigationContext` to allow for finding other NavigationContexts from a context reference
 * Updated `NavigationContainer` to add `getChildContext` which allows finding specific Active/ActivePushed/ActivePresented/Specific contexts from a container reference
 * Added `instruction` property to `NavigationContext`, and marked `NavigationContext` as `@AdvancedEnroApi`
+* Updated `NavigationContext` and `NavigationHandle` to bind each other to allow for easier access to the other from either reference, and to ensure the lazy references are still available while the context is being referenced
+* Updated result handling for forwarding results to fix several bugs and improve behaviour (including correctly handling forwarded results through Activities)
 
 ## 2.3.0
 * Updated NavigationFlow to return from `next` after `onCompleted` is called, rather than continuing to set the backstack from the flow

enro-core/src/main/java/dev/enro/core/NavigationContext.kt

@@ -26,6 +26,14 @@ import dev.enro.core.container.NavigationContainerManager
 import dev.enro.core.controller.NavigationController
 import dev.enro.core.internal.handle.getNavigationHandleViewModel
 
+/**
+ * NavigationContext represents a context in which navigation can occur. In Android, this may be a Fragment, Activity, or Composable.
+ *
+ * When constructing a NavigationContext, the contextReference is the actual object that the NavigationContext represents
+ * (e.g. a Fragment, Activity or Composable), and the other parameters are functions that can be used to retrieve information
+ * about the context. The get functions are invoked lazily, either when the are accessed for the first time,
+ * or once the NavigationContext is bound to a NavigationHandle.
+ */
 public class NavigationContext<ContextType : Any> internal constructor(
     public val contextReference: ContextType,
     private val getController: () -> NavigationController,
@@ -35,8 +43,8 @@ public class NavigationContext<ContextType : Any> internal constructor(
     private val getSavedStateRegistryOwner: () -> SavedStateRegistryOwner,
     private val getLifecycleOwner: () -> LifecycleOwner,
 ) {
-    public val controller: NavigationController get() = getController()
-    public val parentContext: NavigationContext<*>? get() = getParentContext()
+    public val controller: NavigationController by lazy { getController() }
+    public val parentContext: NavigationContext<*>? by lazy { getParentContext() }
 
     /**
      * The arguments provided to this NavigationContext. It is possible to read the open instruction from these arguments,
@@ -47,15 +55,34 @@ public class NavigationContext<ContextType : Any> internal constructor(
      * Generally it should be preferred to read the instruction property, rather than read the instruction from the arguments.
      */
     @AdvancedEnroApi
-    public val arguments: Bundle get() = getArguments()
+    public val arguments: Bundle by lazy { getArguments() }
 
-    public val instruction: NavigationInstruction.Open<*> by lazy { getNavigationHandle().instruction }
-    public val viewModelStoreOwner: ViewModelStoreOwner get() = getViewModelStoreOwner()
-    public val savedStateRegistryOwner: SavedStateRegistryOwner get() = getSavedStateRegistryOwner()
-    public val lifecycleOwner: LifecycleOwner get() = getLifecycleOwner()
+    private lateinit var _instruction: NavigationInstruction.Open<*>
+    public val instruction: NavigationInstruction.Open<*> get() = _instruction
+
+    public val viewModelStoreOwner: ViewModelStoreOwner by lazy { getViewModelStoreOwner() }
+    public val savedStateRegistryOwner: SavedStateRegistryOwner by lazy { getSavedStateRegistryOwner() }
+    public val lifecycleOwner: LifecycleOwner by lazy { getLifecycleOwner() }
     public val lifecycle: Lifecycle get() = lifecycleOwner.lifecycle
 
     public val containerManager: NavigationContainerManager = NavigationContainerManager()
+
+    private var _navigationHandle: NavigationHandle? = null
+    public val navigationHandle: NavigationHandle get() = requireNotNull(_navigationHandle)
+
+    internal fun bind(navigationHandle: NavigationHandle) {
+        _navigationHandle = navigationHandle
+        _instruction = navigationHandle.instruction
+
+        // Invoke hashcode on all lazy items to ensure they are initialized
+
+        controller.hashCode()
+        parentContext.hashCode()
+        arguments.hashCode()
+        viewModelStoreOwner.hashCode()
+        savedStateRegistryOwner.hashCode()
+        lifecycleOwner.hashCode()
+    }
 }
 
 public val NavigationContext<out Fragment>.fragment: Fragment get() = contextReference

enro-core/src/main/java/dev/enro/core/container/NavigationBackstack.kt

@@ -4,13 +4,20 @@ import android.os.Parcelable
 import dev.enro.core.AnyOpenInstruction
 import dev.enro.core.EnroException
 import dev.enro.core.NavigationContext
+import dev.enro.core.NavigationDirection
 import dev.enro.core.controller.interceptor.InstructionOpenedByInterceptor
 import kotlinx.parcelize.Parcelize
 
 @JvmInline
 @Parcelize
 public value class NavigationBackstack(private val backstack: List<AnyOpenInstruction>) : List<AnyOpenInstruction> by backstack, Parcelable {
     public val active: AnyOpenInstruction? get() = lastOrNull()
+
+    public val activePushed: AnyOpenInstruction? get() = lastOrNull { it.navigationDirection == NavigationDirection.Push }
+
+    public val activePresented: AnyOpenInstruction? get() = takeWhile { it.navigationDirection != NavigationDirection.Push }
+        .lastOrNull { it.navigationDirection == NavigationDirection.Push }
+
     internal val identity get() = System.identityHashCode(backstack)
 }
 

enro-core/src/main/java/dev/enro/core/container/NavigationContainer.kt

@@ -34,6 +34,7 @@ import dev.enro.core.getNavigationHandle
 import dev.enro.core.leafContext
 import dev.enro.core.parentContainer
 import dev.enro.core.requestClose
+import dev.enro.core.result.EnroResult
 import dev.enro.core.rootContext
 import dev.enro.extensions.getParcelableListCompat
 import kotlinx.coroutines.Job
@@ -126,13 +127,14 @@ public abstract class NavigationContainer(
         if (Looper.myLooper() != Looper.getMainLooper()) throw EnroException.NavigationContainerWrongThread(
             "A NavigationContainer's setBackstack method must only be called from the main thread"
         )
-        if (backstack == backstackFlow.value) return@synchronized
         renderJob?.cancel()
         val processedBackstack = Compatibility.NavigationContainer
             .processBackstackForDeprecatedInstructionTypes(backstack, instructionFilter)
+            .filterBackstackForForwardedResults()
             .ensureOpeningTypeIsSet(context)
             .processBackstackForPreviouslyActiveContainer()
 
+        if (processedBackstack == backstackFlow.value) return@synchronized
         if (handleEmptyBehaviour(processedBackstack)) return
         val lastBackstack = mutableBackstack
         mutableBackstack = processedBackstack
@@ -263,6 +265,16 @@ public abstract class NavigationContainer(
         }.toBackstack()
     }
 
+    // When using result forwarding, a NavigationContainer can be restored (or otherwise have the backstack set) for
+    // instructions that have a pending result already applied in the EnroResult result manager. In these cases,
+    // we want to filter out the instructions that already have a result applied. This is to ensure that when result
+    // forwarding happens across multiple containers, all destinations providing the result are closed, even if those
+    // destinations aren't visible/active when the forwarded result is added to EnroResult.
+    private fun NavigationBackstack.filterBackstackForForwardedResults(): NavigationBackstack {
+        val enroResult = EnroResult.from(context.controller)
+        return filter { !enroResult.hasPendingResultFrom(it) }.toBackstack()
+    }
+
     public sealed class ContextFilter {
         public data object Active : ContextFilter()
         public data object ActivePresented : ContextFilter()

enro-core/src/main/java/dev/enro/core/container/NavigationInstructionFilter.kt

@@ -21,6 +21,14 @@ public class NavigationContainerFilterBuilder internal constructor() {
         filters.add(NavigationInstructionFilter { predicate(it.navigationKey) })
     }
 
+    public fun key(key: NavigationKey) {
+        key { it == key }
+    }
+
+    public inline fun <reified T: NavigationKey> key() {
+        key { it is T }
+    }
+
     public fun instruction(predicate: (NavigationInstruction.Open<*>) -> Boolean) {
         filters.add(NavigationInstructionFilter(predicate))
     }
@@ -32,14 +40,6 @@ public class NavigationContainerFilterBuilder internal constructor() {
     }
 }
 
-public fun NavigationContainerFilterBuilder.key(key: NavigationKey) {
-    key { it == key }
-}
-
-public inline fun <reified T: NavigationKey> NavigationContainerFilterBuilder.key() {
-    key { it is T }
-}
-
 /**
  * A [NavigationInstructionFilter] that accepts all [NavigationInstruction.Open] instructions.
  */

enro-core/src/main/java/dev/enro/core/controller/usecase/AddPendingResult.kt

@@ -4,7 +4,6 @@ import dev.enro.core.NavigationContext
 import dev.enro.core.NavigationInstruction
 import dev.enro.core.NavigationKey
 import dev.enro.core.controller.NavigationController
-import dev.enro.core.readOpenInstruction
 import dev.enro.core.result.AdvancedResultExtensions
 import dev.enro.core.result.EnroResult
 import dev.enro.core.result.internal.PendingResult
@@ -18,7 +17,7 @@ internal class AddPendingResult(
         navigationContext: NavigationContext<*>,
         instruction: NavigationInstruction.Close
     ) {
-        val openInstruction = navigationContext.arguments.readOpenInstruction() ?: return
+        val openInstruction = navigationContext.instruction
         val navigationKey = openInstruction.internal.resultKey
             ?: openInstruction.navigationKey
 

enro-core/src/main/java/dev/enro/core/controller/usecase/ExecuteContainerOperationInstruction.kt

@@ -4,7 +4,6 @@ import dev.enro.core.NavigationContext
 import dev.enro.core.NavigationInstruction
 import dev.enro.core.findContainer
 import dev.enro.core.parentContainer
-import dev.enro.core.readOpenInstruction
 
 internal interface ExecuteContainerOperationInstruction {
     operator fun invoke(
@@ -29,7 +28,7 @@ internal class ExecuteContainerOperationInstructionImpl(): ExecuteContainerOpera
                 NavigationInstruction.ContainerOperation.Target.ActiveContainer -> "ActiveContainer"
                 is NavigationInstruction.ContainerOperation.Target.TargetContainer -> "TargetContainer(${instruction.target.key})"
             }
-            val contextKeyName = navigationContext.arguments.readOpenInstruction()!!.navigationKey::class.java.simpleName
+            val contextKeyName = navigationContext.instruction.navigationKey::class.java.simpleName
             "Failed to perform container instruction for $targetName in context with key $contextKeyName: Could not find valid container to perform instruction on"
         }
         instruction.operation.invoke(container)

enro-core/src/main/java/dev/enro/core/internal/handle/NavigationHandleViewModel.kt

@@ -4,8 +4,18 @@ import android.annotation.SuppressLint
 import android.os.Looper
 import androidx.activity.ComponentActivity
 import androidx.fragment.app.Fragment
-import androidx.lifecycle.*
-import dev.enro.core.*
+import androidx.lifecycle.Lifecycle
+import androidx.lifecycle.LifecycleEventObserver
+import androidx.lifecycle.LifecycleRegistry
+import androidx.lifecycle.ViewModel
+import androidx.lifecycle.lifecycleScope
+import androidx.lifecycle.withStarted
+import dev.enro.core.AnyOpenInstruction
+import dev.enro.core.NavigationContext
+import dev.enro.core.NavigationHandle
+import dev.enro.core.NavigationInstruction
+import dev.enro.core.NavigationKey
+import dev.enro.core.close
 import dev.enro.core.compose.ComposableDestination
 import dev.enro.core.controller.usecase.ExecuteCloseInstruction
 import dev.enro.core.controller.usecase.ExecuteContainerOperationInstruction
@@ -44,6 +54,7 @@ internal open class NavigationHandleViewModel(
             field = value
             if (value == null) return
 
+            value.bind(this)
             registerLifecycleObservers(value)
             executePendingInstruction()
 

enro-core/src/main/java/dev/enro/core/result/EnroResult.kt

@@ -1,5 +1,6 @@
 package dev.enro.core.result
 
+import dev.enro.core.AnyOpenInstruction
 import dev.enro.core.EnroException
 import dev.enro.core.NavigationHandle
 import dev.enro.core.controller.NavigationController
@@ -38,6 +39,9 @@ internal class EnroResult: EnroPlugin() {
         }
     }
 
+    internal fun hasPendingResultFrom(instruction: AnyOpenInstruction): Boolean {
+        return pendingResults[instruction.internal.resultId] != null
+    }
 
     private fun consumePendingResult(resultChannelId: ResultChannelId): PendingResult? {
         val result = pendingResults[resultChannelId] ?: return null

enro-core/src/main/java/dev/enro/core/result/ForwardingResultInterceptor.kt

@@ -1,10 +1,17 @@
 package dev.enro.core.result
 
-import dev.enro.core.*
+import dev.enro.core.NavigationContext
+import dev.enro.core.NavigationInstruction
+import dev.enro.core.NavigationKey
+import dev.enro.core.activity
+import dev.enro.core.activity.ActivityNavigationContainer
 import dev.enro.core.container.toBackstack
 import dev.enro.core.controller.get
 import dev.enro.core.controller.interceptor.NavigationInstructionInterceptor
 import dev.enro.core.controller.usecase.AddPendingResult
+import dev.enro.core.navigationContext
+import dev.enro.core.readOpenInstruction
+import dev.enro.core.rootContext
 
 internal object ForwardingResultInterceptor  : NavigationInstructionInterceptor {
     override fun intercept(
@@ -23,6 +30,7 @@ internal object ForwardingResultInterceptor  : NavigationInstructionInterceptor
         val containers = context.rootContext()
             .containerManager
             .containers
+            .plus(ActivityNavigationContainer(context.activity.navigationContext))
             .toMutableList()
 
         while (containers.isNotEmpty()) {

enro-core/src/main/java/dev/enro/destination/activity/ActivityNavigationContainer.kt

@@ -20,6 +20,7 @@ import dev.enro.core.container.backstackOf
 import dev.enro.core.controller.get
 import dev.enro.core.controller.usecase.GetNavigationBinding
 import dev.enro.core.controller.usecase.HostInstructionAs
+import dev.enro.core.result.EnroResult
 
 internal class ActivityNavigationContainer internal constructor(
     activityContext: NavigationContext<out ComponentActivity>,
@@ -47,6 +48,14 @@ internal class ActivityNavigationContainer internal constructor(
     }
 
     override fun onBackstackUpdated(transition: NavigationBackstackTransition): Boolean {
+        // When the backstack is updated, we need to check if there are pending results and close
+        // immediately to ensure forwarding results work correctly
+        val result = EnroResult.from(context.controller)
+        if (result.hasPendingResultFrom(context.instruction)) {
+            context.activity.finish()
+            return true
+        }
+
         if (transition.activeBackstack.singleOrNull()?.instructionId == rootInstruction.instructionId) return true
         val childContext = requireNotNull(childContext)
         setBackstack(backstackOf(rootInstruction))

enro-core/src/main/java/dev/enro/destination/compose/ComposableNavigationResult.kt

@@ -5,10 +5,12 @@ import androidx.compose.runtime.DisallowComposableCalls
 import androidx.compose.runtime.DisposableEffect
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.saveable.rememberSaveable
+import androidx.lifecycle.Lifecycle
+import androidx.lifecycle.LifecycleEventObserver
 import dev.enro.core.NavigationKey
 import dev.enro.core.controller.usecase.createResultChannel
 import dev.enro.core.result.NavigationResultChannel
-import java.util.*
+import java.util.UUID
 
 
 @Composable
@@ -38,8 +40,25 @@ public inline fun <reified T : Any> registerForNavigationResult(
     }
 
     DisposableEffect(true) {
-        resultChannel.attach()
+        // In some cases, particularly with navigation to Activities,
+        // Composables aren't actually called through to onDispose, meaning the
+        // result channel sticks around as being "active" even though the associated
+        // activity is not started. We're adding a lifecycle observer here to ensure this
+        // is managed correctly.
+        val observer = LifecycleEventObserver { _, event ->
+            if (event == Lifecycle.Event.ON_START) {
+                resultChannel.attach()
+            }
+            if (event == Lifecycle.Event.ON_STOP) {
+                resultChannel.detach()
+            }
+        }
+        navigationHandle.lifecycle.addObserver(observer)
+        if (navigationHandle.lifecycle.currentState.isAtLeast(Lifecycle.State.STARTED)) {
+            resultChannel.attach()
+        }
         onDispose {
+            navigationHandle.lifecycle.removeObserver(observer)
             resultChannel.detach()
         }
     }