Clean up the AbstractWorkflowNode refactor.

Author: zach-klippenstein

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/AbstractWorkflowNode.kt

@@ -8,13 +8,11 @@ import com.squareup.workflow1.RuntimeConfigOptions
 import com.squareup.workflow1.TreeSnapshot
 import com.squareup.workflow1.Workflow
 import com.squareup.workflow1.WorkflowExperimentalApi
-import com.squareup.workflow1.WorkflowIdentifier
 import com.squareup.workflow1.WorkflowInterceptor
 import com.squareup.workflow1.WorkflowInterceptor.WorkflowSession
 import com.squareup.workflow1.WorkflowTracer
 import com.squareup.workflow1.compose.ComposeWorkflow
 import com.squareup.workflow1.internal.compose.ComposeWorkflowNodeAdapter
-import com.squareup.workflow1.workflowSessionToString
 import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.CoroutineName
 import kotlinx.coroutines.CoroutineScope
@@ -40,7 +38,6 @@ internal fun <PropsT, OutputT, RenderingT> createWorkflowNode(
 ): AbstractWorkflowNode<PropsT, OutputT, RenderingT> = when (workflow) {
   is ComposeWorkflow<*, *, *> -> ComposeWorkflowNodeAdapter(
     id = id,
-    workflow = workflow as ComposeWorkflow,
     initialProps = initialProps,
     snapshot = snapshot,
     baseContext = baseContext,
@@ -69,15 +66,15 @@ internal fun <PropsT, OutputT, RenderingT> createWorkflowNode(
 
 internal abstract class AbstractWorkflowNode<PropsT, OutputT, RenderingT>(
   val id: WorkflowNodeId,
-  final override val parent: WorkflowSession?,
-  final override val workflowTracer: WorkflowTracer?,
-  final override val runtimeConfig: RuntimeConfig,
   protected val interceptor: WorkflowInterceptor,
   protected val emitAppliedActionToParent: (ActionApplied<OutputT>) -> ActionProcessingResult,
   baseContext: CoroutineContext,
-  idCounter: IdCounter?,
-) : WorkflowSession,
-  CoroutineScope {
+) : CoroutineScope {
+
+  /**
+   * The [WorkflowSession] that represents this node to [WorkflowInterceptor]s.
+   */
+  abstract val session: WorkflowSession
 
   /**
    * Context that has a job that will live as long as this node.
@@ -87,13 +84,6 @@ internal abstract class AbstractWorkflowNode<PropsT, OutputT, RenderingT>(
     Job(baseContext[Job]) +
     CoroutineName(id.toString())
 
-  // WorkflowSession properties
-  final override val identifier: WorkflowIdentifier get() = id.identifier
-  final override val renderKey: String get() = id.name
-  final override val sessionId: Long = idCounter.createId()
-
-  final override fun toString(): String = workflowSessionToString()
-
   /**
    * Walk the tree of workflows, rendering each one and using
    * [RenderContext][com.squareup.workflow1.BaseRenderContext] to give its children a chance to
@@ -117,9 +107,9 @@ internal abstract class AbstractWorkflowNode<PropsT, OutputT, RenderingT>(
    * Gets the next [result][ActionProcessingResult] from the state machine. This will be an
    * [OutputT] or null.
    *
-   * Walk the tree of state machines, asking each one to wait for its next event. If something happen
-   * that results in an output, that output is returned. Null means something happened that requires
-   * a re-render, e.g. my state changed or a child state changed.
+   * Walk the tree of state machines, asking each one to wait for its next event. If something
+   * happen that results in an output, that output is returned. Null means something happened that
+   * requires a re-render, e.g. my state changed or a child state changed.
    *
    * It is an error to call this method after calling [cancel].
    *
@@ -131,8 +121,8 @@ internal abstract class AbstractWorkflowNode<PropsT, OutputT, RenderingT>(
   /**
    * Cancels this state machine host, and any coroutines started as children of it.
    *
-   * This must be called when the caller will no longer call [onNextAction]. It is an error to call [onNextAction]
-   * after calling this method.
+   * This must be called when the caller will no longer call [onNextAction]. It is an error to call
+   * [onNextAction] after calling this method.
    */
   open fun cancel(cause: CancellationException? = null) {
     coroutineContext.cancel(cause)

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/StatefulWorkflowNode.kt

@@ -14,6 +14,7 @@ import com.squareup.workflow1.Workflow
 import com.squareup.workflow1.WorkflowAction
 import com.squareup.workflow1.WorkflowExperimentalApi
 import com.squareup.workflow1.WorkflowExperimentalRuntime
+import com.squareup.workflow1.WorkflowIdentifier
 import com.squareup.workflow1.WorkflowInterceptor
 import com.squareup.workflow1.WorkflowInterceptor.WorkflowSession
 import com.squareup.workflow1.WorkflowTracer
@@ -22,6 +23,7 @@ import com.squareup.workflow1.intercept
 import com.squareup.workflow1.internal.RealRenderContext.RememberStore
 import com.squareup.workflow1.internal.RealRenderContext.SideEffectRunner
 import com.squareup.workflow1.trace
+import com.squareup.workflow1.workflowSessionToString
 import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.CoroutineName
 import kotlinx.coroutines.CoroutineScope
@@ -56,28 +58,32 @@ internal class StatefulWorkflowNode<PropsT, StateT, OutputT, RenderingT>(
   snapshot: TreeSnapshot?,
   baseContext: CoroutineContext,
   // Providing default value so we don't need to specify in test.
-  runtimeConfig: RuntimeConfig = RuntimeConfigOptions.DEFAULT_CONFIG,
-  workflowTracer: WorkflowTracer? = null,
+  override val runtimeConfig: RuntimeConfig = RuntimeConfigOptions.DEFAULT_CONFIG,
+  override val workflowTracer: WorkflowTracer? = null,
   emitAppliedActionToParent: (ActionApplied<OutputT>) -> ActionProcessingResult = { it },
-  parent: WorkflowSession? = null,
+  override val parent: WorkflowSession? = null,
   interceptor: WorkflowInterceptor = NoopWorkflowInterceptor,
   idCounter: IdCounter? = null
 ) : AbstractWorkflowNode<PropsT, OutputT, RenderingT>(
   id = id,
-  runtimeConfig = runtimeConfig,
-  workflowTracer = workflowTracer,
-  parent = parent,
   baseContext = baseContext,
-  idCounter = idCounter,
   interceptor = interceptor,
   emitAppliedActionToParent = emitAppliedActionToParent,
 ),
   SideEffectRunner,
-  RememberStore {
+  RememberStore,
+  WorkflowSession {
 
+  // WorkflowSession properties
+  override val identifier: WorkflowIdentifier get() = id.identifier
+  override val renderKey: String get() = id.name
+  override val sessionId: Long = idCounter.createId()
   private var cachedWorkflowInstance: StatefulWorkflow<PropsT, StateT, OutputT, RenderingT>
   private var interceptedWorkflowInstance: StatefulWorkflow<PropsT, StateT, OutputT, RenderingT>
 
+  override val session: WorkflowSession
+    get() = this
+
   private val subtreeManager = SubtreeManager(
     snapshotCache = snapshot?.childTreeSnapshots,
     contextForChildren = coroutineContext,
@@ -115,6 +121,8 @@ internal class StatefulWorkflowNode<PropsT, StateT, OutputT, RenderingT>(
     state = interceptedWorkflowInstance.initialState(initialProps, snapshot?.workflowSnapshot, this)
   }
 
+  override fun toString(): String = workflowSessionToString()
+
   /**
    * Walk the tree of workflows, rendering each one and using
    * [RenderContext][com.squareup.workflow1.BaseRenderContext] to give its children a chance to

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowRunner.kt

@@ -69,11 +69,15 @@ internal class WorkflowRunner<PropsT, OutputT, RenderingT>(
    * between every subsequent call to [processAction].
    */
   fun nextRendering(): RenderingAndSnapshot<RenderingT> {
-    return interceptor.onRenderAndSnapshot(currentProps, { props ->
-      val rendering = rootNode.render(workflow, props)
-      val snapshot = rootNode.snapshot()
-      RenderingAndSnapshot(rendering, snapshot)
-    }, rootNode)
+    return interceptor.onRenderAndSnapshot(
+      renderProps = currentProps,
+      proceed = { props ->
+        val rendering = rootNode.render(workflow, props)
+        val snapshot = rootNode.snapshot()
+        RenderingAndSnapshot(rendering, snapshot)
+      },
+      session = rootNode.session,
+    )
   }
 
   /**

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/compose/ComposeWorkflowChildNode.kt

@@ -56,7 +56,10 @@ internal class ComposeWorkflowChildNode<PropsT, OutputT, RenderingT>(
   override val id: WorkflowNodeId,
   initialProps: PropsT,
   snapshot: TreeSnapshot?,
-  baseContext: CoroutineContext,
+  // We don't need to create our own job because unlike for WorkflowNode, the baseContext already
+  // has a dedicated job: either from the adapter (for root compose workflow), or from
+  // rememberCoroutineScope().
+  override val coroutineContext: CoroutineContext,
   override val parent: WorkflowSession?,
   override val workflowTracer: WorkflowTracer?,
   override val runtimeConfig: RuntimeConfig,
@@ -69,12 +72,6 @@ internal class ComposeWorkflowChildNode<PropsT, OutputT, RenderingT>(
   WorkflowComposableRenderer,
   CoroutineScope {
 
-  // We don't need to create our own job because unlike for WorkflowNode, the baseContext already
-  // has a dedicated job: either from the adapter (for root compose workflow), or from
-  // rememberCoroutineScope().
-  override val coroutineContext: CoroutineContext = baseContext +
-    CoroutineName(id.toString())
-
   // WorkflowSession properties
   override val identifier: WorkflowIdentifier get() = id.identifier
   override val renderKey: String get() = id.name
@@ -289,7 +286,7 @@ internal class ComposeWorkflowChildNode<PropsT, OutputT, RenderingT>(
           id = childId,
           initialProps = initialProps,
           snapshot = childSnapshot,
-          baseContext = childCoroutineScope.coroutineContext,
+          coroutineContext = childCoroutineScope.coroutineContext + CoroutineName(id.toString()),
           parent = this,
           workflowTracer = workflowTracer,
           runtimeConfig = runtimeConfig,

workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/compose/ComposeWorkflowNodeAdapter.kt

@@ -8,7 +8,6 @@ import com.squareup.workflow1.RuntimeConfig
 import com.squareup.workflow1.RuntimeConfigOptions
 import com.squareup.workflow1.TreeSnapshot
 import com.squareup.workflow1.Workflow
-import com.squareup.workflow1.WorkflowExperimentalApi
 import com.squareup.workflow1.WorkflowInterceptor
 import com.squareup.workflow1.WorkflowInterceptor.WorkflowSession
 import com.squareup.workflow1.WorkflowTracer
@@ -34,10 +33,8 @@ internal fun log(message: String) = message.lines().forEach {
  * [ComposeWorkflowChildNode]. [ComposeWorkflow]s nested directly under this one do not have their
  * own composition, they share this one.
  */
-@OptIn(WorkflowExperimentalApi::class)
 internal class ComposeWorkflowNodeAdapter<PropsT, OutputT, RenderingT>(
   id: WorkflowNodeId,
-  workflow: ComposeWorkflow<PropsT, OutputT, RenderingT>,
   initialProps: PropsT,
   snapshot: TreeSnapshot?,
   baseContext: CoroutineContext,
@@ -48,15 +45,9 @@ internal class ComposeWorkflowNodeAdapter<PropsT, OutputT, RenderingT>(
   parent: WorkflowSession? = null,
   interceptor: WorkflowInterceptor = NoopWorkflowInterceptor,
   idCounter: IdCounter? = null
-  // TODO AbstractWorkflowNode should not implement WorkflowSession, since only StatefulWorkflowNode
-  //  needs that. The composable session is implemented by ComposeWorkflowChildNode.
 ) : AbstractWorkflowNode<PropsT, OutputT, RenderingT>(
   id = id,
-  runtimeConfig = runtimeConfig,
-  workflowTracer = workflowTracer,
-  parent = parent,
   baseContext = baseContext,
-  idCounter = idCounter,
   interceptor = interceptor,
   emitAppliedActionToParent = emitAppliedActionToParent,
 ) {
@@ -70,7 +61,7 @@ internal class ComposeWorkflowNodeAdapter<PropsT, OutputT, RenderingT>(
     id = id,
     initialProps = initialProps,
     snapshot = snapshot,
-    baseContext = coroutineContext,
+    coroutineContext = coroutineContext,
     parent = parent,
     workflowTracer = workflowTracer,
     runtimeConfig = runtimeConfig,
@@ -109,6 +100,9 @@ internal class ComposeWorkflowNodeAdapter<PropsT, OutputT, RenderingT>(
     }
   )
 
+  override val session: WorkflowSession
+    get() = childNode
+
   /**
    * Function invoked when [onNextAction] receives a recompose request.
    * This handles the case where some state read by the composition is changed but emitOutput is