diff --git a/.github/workflows/kotlin.yml b/.github/workflows/kotlin.yml index 138a53ac9..7cbd907c3 100644 --- a/.github/workflows/kotlin.yml +++ b/.github/workflows/kotlin.yml @@ -180,6 +180,27 @@ jobs : build-root-directory : samples/tutorial restore-cache-key : main-build-artifacts + jvm-drainExclusive-runtime-test : + name : Conflate Stale Renderings Runtime JVM Tests + runs-on : ubuntu-latest + timeout-minutes : 20 + steps : + - name: Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + + - name : Check with Gradle + uses : ./.github/actions/gradle-task + with : + task : jvmTest --continue -Pworkflow.runtime=drainExclusive + restore-cache-key : main-build-artifacts + + # Report as GitHub Pull Request Check. + - name : Publish Test Report + uses : mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4 + if : always() # always run even if the previous step fails + with : + report_paths : '**/build/test-results/test/TEST-*.xml' + jvm-conflate-runtime-test : name : Conflate Stale Renderings Runtime JVM Tests runs-on : ubuntu-latest @@ -306,6 +327,111 @@ jobs : with: report_paths: '**/build/test-results/test/TEST-*.xml' + jvm-conflate-drainExclusive-runtime-test: + name: Conflate Stale Renderings Runtime JVM Tests + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + + - name: Check with Gradle + uses: ./.github/actions/gradle-task + with: + task: jvmTest --continue -Pworkflow.runtime=conflate-drainExclusive + restore-cache-key: main-build-artifacts + + # Report as GitHub Pull Request Check. + - name: Publish Test Report + uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4 + if: always() # always run even if the previous step fails + with: + report_paths: '**/build/test-results/test/TEST-*.xml' + + jvm-stateChange-drainExclusive-runtime-test: + name: Render On State Change Only Runtime JVM Tests + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + + - name: Check with Gradle + uses: ./.github/actions/gradle-task + with: + task: jvmTest --continue -Pworkflow.runtime=stateChange-drainExclusive + restore-cache-key: main-build-artifacts + + # Report as GitHub Pull Request Check. + - name: Publish Test Report + uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4 + if: always() # always run even if the previous step fails + with: + report_paths: '**/build/test-results/test/TEST-*.xml' + + jvm-partial-drainExclusive-runtime-test: + name: Partial Tree Rendering Only Runtime JVM Tests + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + + - name: Check with Gradle + uses: ./.github/actions/gradle-task + with: + task: jvmTest --continue -Pworkflow.runtime=partial-drainExclusive + restore-cache-key: main-build-artifacts + + # Report as GitHub Pull Request Check. + - name: Publish Test Report + uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4 + if: always() # always run even if the previous step fails + with: + report_paths: '**/build/test-results/test/TEST-*.xml' + + jvm-conflate-stateChange-drainExclusive-runtime-test: + name: Render On State Change Only and Conflate Stale Runtime JVM Tests + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + + - name: Check with Gradle + uses: ./.github/actions/gradle-task + with: + task: jvmTest --continue -Pworkflow.runtime=conflate-stateChange-drainExclusive + restore-cache-key: main-build-artifacts + + # Report as GitHub Pull Request Check. + - name: Publish Test Report + uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4 + if: always() # always run even if the previous step fails + with: + report_paths: '**/build/test-results/test/TEST-*.xml' + + jvm-conflate-partial-drainExclusive-runtime-test: + name: Render On State Change Only and Conflate Stale Runtime JVM Tests + runs-on: ubuntu-latest + timeout-minutes: 20 + steps: + - name: Checkout + uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 + + - name: Check with Gradle + uses: ./.github/actions/gradle-task + with: + task: jvmTest --continue -Pworkflow.runtime=conflate-partial-drainExclusive + restore-cache-key: main-build-artifacts + + # Report as GitHub Pull Request Check. + - name: Publish Test Report + uses: mikepenz/action-junit-report@5f47764eec0e1c1f19f40c8e60a5ba47e47015c5 # v4 + if: always() # always run even if the previous step fails + with: + report_paths: '**/build/test-results/test/TEST-*.xml' + ios-tests : name : iOS Tests runs-on : macos-latest @@ -412,7 +538,7 @@ jobs : ### shardNum: [ 1, 2, 3 ] ### - runtime : [ conflate, stateChange, conflate-stateChange, partial, conflate-partial, stable ] + runtime : [ conflate, stateChange, drainExclusive, conflate-stateChange, partial, conflate-partial, stable, conflate-drainExclusive, stateChange-drainExclusive, partial-drainExclusive, conflate-partial-drainExclusive ] steps : - name: Checkout uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1 diff --git a/workflow-config/config-android/src/main/java/com/squareup/workflow1/config/AndroidRuntimeConfigTools.kt b/workflow-config/config-android/src/main/java/com/squareup/workflow1/config/AndroidRuntimeConfigTools.kt index 05c3f32ce..56798220e 100644 --- a/workflow-config/config-android/src/main/java/com/squareup/workflow1/config/AndroidRuntimeConfigTools.kt +++ b/workflow-config/config-android/src/main/java/com/squareup/workflow1/config/AndroidRuntimeConfigTools.kt @@ -3,6 +3,7 @@ package com.squareup.workflow1.config import com.squareup.workflow1.RuntimeConfig import com.squareup.workflow1.RuntimeConfigOptions import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS +import com.squareup.workflow1.RuntimeConfigOptions.DRAIN_EXCLUSIVE_ACTIONS import com.squareup.workflow1.RuntimeConfigOptions.PARTIAL_TREE_RENDERING import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES import com.squareup.workflow1.RuntimeConfigOptions.STABLE_EVENT_HANDLERS @@ -33,6 +34,10 @@ public class AndroidRuntimeConfigTools { * * - `stable` Enables stable event handlers (changes the default value of the `remember` * parameter of `RenderContext.eventHandler` functions from `false` to `true`) + * + * - `drainExclusive` Enables draining exclusive actions. If we have more actions to process + * that are queued on nodes not affected by the last action application, then we will + * continue to process those actions before another render pass. */ @WorkflowExperimentalRuntime public fun getAppWorkflowRuntimeConfig(): RuntimeConfig { @@ -48,6 +53,7 @@ public class AndroidRuntimeConfigTools { "stateChange" -> config.add(RENDER_ONLY_WHEN_STATE_CHANGES) "partial" -> config.addAll(setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING)) "stable" -> config.add(STABLE_EVENT_HANDLERS) + "drainExclusive" -> config.add(DRAIN_EXCLUSIVE_ACTIONS) else -> throw IllegalArgumentException("Unrecognized runtime config option \"$it\"") } } diff --git a/workflow-config/config-jvm/src/main/java/com/squareup/workflow1/config/JvmTestRuntimeConfigTools.kt b/workflow-config/config-jvm/src/main/java/com/squareup/workflow1/config/JvmTestRuntimeConfigTools.kt index da64d2fc3..2d525ff38 100644 --- a/workflow-config/config-jvm/src/main/java/com/squareup/workflow1/config/JvmTestRuntimeConfigTools.kt +++ b/workflow-config/config-jvm/src/main/java/com/squareup/workflow1/config/JvmTestRuntimeConfigTools.kt @@ -3,6 +3,7 @@ package com.squareup.workflow1.config import com.squareup.workflow1.RuntimeConfig import com.squareup.workflow1.RuntimeConfigOptions import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS +import com.squareup.workflow1.RuntimeConfigOptions.DRAIN_EXCLUSIVE_ACTIONS import com.squareup.workflow1.RuntimeConfigOptions.PARTIAL_TREE_RENDERING import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES import com.squareup.workflow1.RuntimeConfigOptions.STABLE_EVENT_HANDLERS @@ -35,6 +36,10 @@ public class JvmTestRuntimeConfigTools { * * - `stable` Enables stable event handlers (changes the default value of the `remember` * parameter of `RenderContext.eventHandler` functions from `false` to `true`) + * + * - `drainExclusive` Enables draining exclusive actions. If we have more actions to process + * that are queued on nodes not affected by the last action application, then we will + * continue to process those actions before another render pass. */ @OptIn(WorkflowExperimentalRuntime::class) public fun getTestRuntimeConfig(): RuntimeConfig { @@ -50,6 +55,7 @@ public class JvmTestRuntimeConfigTools { "stateChange" -> config.add(RENDER_ONLY_WHEN_STATE_CHANGES) "partial" -> config.addAll(setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING)) "stable" -> config.add(STABLE_EVENT_HANDLERS) + "drainExclusive" -> config.add(DRAIN_EXCLUSIVE_ACTIONS) else -> throw IllegalArgumentException("Unrecognized runtime config option \"$it\"") } } diff --git a/workflow-core/api/workflow-core.api b/workflow-core/api/workflow-core.api index 7cddccb14..835a4dd19 100644 --- a/workflow-core/api/workflow-core.api +++ b/workflow-core/api/workflow-core.api @@ -164,6 +164,7 @@ public final class com/squareup/workflow1/PropsUpdated : com/squareup/workflow1/ public final class com/squareup/workflow1/RuntimeConfigOptions : java/lang/Enum { public static final field CONFLATE_STALE_RENDERINGS Lcom/squareup/workflow1/RuntimeConfigOptions; public static final field Companion Lcom/squareup/workflow1/RuntimeConfigOptions$Companion; + public static final field DRAIN_EXCLUSIVE_ACTIONS Lcom/squareup/workflow1/RuntimeConfigOptions; public static final field PARTIAL_TREE_RENDERING Lcom/squareup/workflow1/RuntimeConfigOptions; public static final field RENDER_ONLY_WHEN_STATE_CHANGES Lcom/squareup/workflow1/RuntimeConfigOptions; public static final field STABLE_EVENT_HANDLERS Lcom/squareup/workflow1/RuntimeConfigOptions; diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/RuntimeConfig.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/RuntimeConfig.kt index 44b0c3c5b..28ca69365 100644 --- a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/RuntimeConfig.kt +++ b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/RuntimeConfig.kt @@ -66,6 +66,14 @@ public enum class RuntimeConfigOptions { */ @WorkflowExperimentalRuntime STABLE_EVENT_HANDLERS, + + /** + * If we have more actions to process that are queued on nodes not affected by the last + * action application, then we will continue to process those actions before another render + * pass. + */ + @WorkflowExperimentalRuntime + DRAIN_EXCLUSIVE_ACTIONS, ; public companion object { diff --git a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/RenderWorkflow.kt b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/RenderWorkflow.kt index 78a25f097..db456d295 100644 --- a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/RenderWorkflow.kt +++ b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/RenderWorkflow.kt @@ -1,6 +1,7 @@ package com.squareup.workflow1 import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS +import com.squareup.workflow1.RuntimeConfigOptions.DRAIN_EXCLUSIVE_ACTIONS import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES import com.squareup.workflow1.WorkflowInterceptor.RenderPassSkipped import com.squareup.workflow1.WorkflowInterceptor.RenderPassesComplete @@ -14,6 +15,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.isActive import kotlinx.coroutines.launch +import kotlinx.coroutines.yield /** * Launches the [workflow] in a new coroutine in [scope] and returns a [StateFlow] of its @@ -167,13 +169,18 @@ public fun renderWorkflowIn( /** * If [runtimeConfig] contains [RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES] and * we have not changed state, then return true to short circuit the render loop. + * + * @param actionResult: Whether this action has changed state. + * @param actionChainingHasChangedState: Whether any of the chained actions has changed state. */ fun shouldShortCircuitForUnchangedState( actionResult: ActionProcessingResult, + actionChainingHasChangedState: Boolean = false ): Boolean { return runtimeConfig.contains(RENDER_ONLY_WHEN_STATE_CHANGES) && actionResult is ActionApplied<*> && - !actionResult.stateChanged + !actionResult.stateChanged && + !actionChainingHasChangedState } scope.launch { @@ -193,13 +200,53 @@ public fun renderWorkflowIn( // we don't surprise anyone with an unexpected rendering pass. Show's over, go home. if (!isActive) return@launch + var drainingActionResult = actionResult + var actionChainingHasChangedState = false + if (runtimeConfig.contains(DRAIN_EXCLUSIVE_ACTIONS)) { + drain@ while (isActive && + drainingActionResult is ActionApplied<*> && + drainingActionResult.output == null + ) { + actionChainingHasChangedState = + actionChainingHasChangedState || drainingActionResult.stateChanged + // We start by yielding, so that other coroutines can run if we are running on an + // immediate dispatcher with its own inner loop. + yield() + // We may have more mutually exclusive actions we can apply before a render pass. + drainingActionResult = + runner.processAction(waitForAnAction = false, skipChangedNodes = true) + + // If no mutually exclusive actions processed, then go ahead and do the render pass. + if (drainingActionResult == ActionsExhausted) break@drain + + // Now save last result if its not ActionsExhausted + actionResult = drainingActionResult + + // If no state changed, send any output and start outer loop again. + if (shouldShortCircuitForUnchangedState( + actionResult = drainingActionResult, + actionChainingHasChangedState = actionChainingHasChangedState + ) + ) { + chainedInterceptor.onRuntimeLoopTick(RenderPassSkipped(endOfTick = true)) + sendOutput(drainingActionResult, onOutput) + continue@outer + } + } + } + // Next Render Pass. var nextRenderAndSnapshot: RenderingAndSnapshot = runner.nextRendering() if (runtimeConfig.contains(CONFLATE_STALE_RENDERINGS)) { - var conflationHasChangedState = false - conflate@ while (isActive && actionResult is ActionApplied<*> && actionResult.output == null) { - conflationHasChangedState = conflationHasChangedState || actionResult.stateChanged + conflate@ while (isActive && + actionResult is ActionApplied<*> && + actionResult.output == null + ) { + actionChainingHasChangedState = actionChainingHasChangedState || actionResult.stateChanged + // We start by yielding, so that other coroutines can run if we are running on an + // immediate dispatcher with its own inner loop. + yield() // We may have more actions we can process, this rendering could be stale. actionResult = runner.processAction(waitForAnAction = false) @@ -211,7 +258,7 @@ public fun renderWorkflowIn( actionResult = actionResult, ) ) { - if (conflationHasChangedState) { + if (actionChainingHasChangedState) { chainedInterceptor.onRuntimeLoopTick(RenderPassSkipped(endOfTick = false)) // An earlier render changed state, so we need to pass that to the UI then we // can skip this render. diff --git a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/SubtreeManager.kt b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/SubtreeManager.kt index a150d5998..033f61d03 100644 --- a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/SubtreeManager.kt +++ b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/SubtreeManager.kt @@ -150,11 +150,14 @@ internal class SubtreeManager( * * @return [Boolean] whether or not the children action queues are empty. */ - fun onNextChildAction(selector: SelectBuilder): Boolean { + fun onNextChildAction( + selector: SelectBuilder, + skipChangedNodes: Boolean = false + ): Boolean { var empty = true children.forEachActive { child -> // Do this separately so the compiler doesn't avoid it if empty is already false. - val childEmpty = child.workflowNode.onNextAction(selector) + val childEmpty = child.workflowNode.onNextAction(selector, skipChangedNodes) empty = childEmpty && empty } return empty diff --git a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowNode.kt b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowNode.kt index 17dc3ff7d..bff2234c9 100644 --- a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowNode.kt +++ b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowNode.kt @@ -96,7 +96,16 @@ internal class WorkflowNode( private val eventActionsChannel = Channel>(capacity = UNLIMITED) private var state: StateT - private var subtreeStateDidChange: Boolean = true + + /** + * The state of this node or that of one of our descendants changed. + */ + private var subtreeStateDirty: Boolean = true + + /** + * The state of this node changed. + */ + private var selfStateDirty: Boolean = true private val baseRenderContext = RealRenderContext( renderer = subtreeManager, @@ -212,16 +221,29 @@ internal class WorkflowNode( * time of suspending. */ @OptIn(ExperimentalCoroutinesApi::class, DelicateCoroutinesApi::class) - fun onNextAction(selector: SelectBuilder): Boolean { - // Listen for any child workflow updates. - var empty = subtreeManager.onNextChildAction(selector) + fun onNextAction( + selector: SelectBuilder, + skipChangedNodes: Boolean = false + ): Boolean { + val shouldListenForEvents = !skipChangedNodes || !selfStateDirty + + var empty = if (shouldListenForEvents) { + // Listen for any child workflow events. + subtreeManager.onNextChildAction(selector, skipChangedNodes) + } else { + // Our state changed and we are skipping changed nodes, so our actions are empty from + // this node down. + true + } empty = empty && (eventActionsChannel.isEmpty || eventActionsChannel.isClosedForReceive) - // Listen for any events. - with(selector) { - eventActionsChannel.onReceive { action -> - return@onReceive applyAction(action) + if (shouldListenForEvents) { + // Listen for any events. + with(selector) { + eventActionsChannel.onReceive { action -> + return@onReceive applyAction(action) + } } } return empty @@ -267,7 +289,7 @@ internal class WorkflowNode( if (!runtimeConfig.contains(PARTIAL_TREE_RENDERING) || !lastRendering.isInitialized || - subtreeStateDidChange + subtreeStateDirty ) { // If we haven't already updated the cached instance, better do it now! maybeUpdateCachedWorkflowInstance(workflow) @@ -287,7 +309,8 @@ internal class WorkflowNode( } // After we have rendered this subtree, we need another action in order for us to be // considered dirty again. - subtreeStateDidChange = false + subtreeStateDirty = false + selfStateDirty = false } return lastRendering.getOrThrow() @@ -296,7 +319,7 @@ internal class WorkflowNode( /** * Update props if they have changed. If that happens, then check to see if we need * to update the cached workflow instance, then call [StatefulWorkflow.onPropsChanged] and - * update the state from that. We consider any change to props as [subtreeStateDidChange] because + * update the state from that. We consider any change to props as dirty because * the props themselves are used in [StatefulWorkflow.render] (they are the 'external' part of * the state) so we must re-render. */ @@ -308,7 +331,8 @@ internal class WorkflowNode( maybeUpdateCachedWorkflowInstance(workflow) val newState = interceptedWorkflowInstance.onPropsChanged(lastProps, newProps, state) state = newState - subtreeStateDidChange = true + subtreeStateDirty = true + selfStateDirty = true } lastProps = newProps } @@ -330,8 +354,10 @@ internal class WorkflowNode( // Changing state is sticky, we pass it up if it ever changed. stateChanged = actionApplied.stateChanged || (childResult?.stateChanged ?: false) ) + // Our state changed. + selfStateDirty = actionApplied.stateChanged // Our state changed or one of our children's state changed. - subtreeStateDidChange = aggregateActionApplied.stateChanged + subtreeStateDirty = aggregateActionApplied.stateChanged return if (actionApplied.output != null || runtimeConfig.contains(PARTIAL_TREE_RENDERING) ) { @@ -344,7 +370,7 @@ internal class WorkflowNode( // // However, the root and the path down to the changed nodes must always // re-render now, so this is the implementation detail of how we get - // subtreeStateDidChange = true on that entire path to the root. + // subtreeStateDirty = true on that entire path to the root. emitAppliedActionToParent(aggregateActionApplied) } else { aggregateActionApplied diff --git a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowRunner.kt b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowRunner.kt index 9eb66bb1b..b273effc9 100644 --- a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowRunner.kt +++ b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/WorkflowRunner.kt @@ -5,10 +5,8 @@ import com.squareup.workflow1.ActionsExhausted import com.squareup.workflow1.PropsUpdated import com.squareup.workflow1.RenderingAndSnapshot import com.squareup.workflow1.RuntimeConfig -import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS import com.squareup.workflow1.TreeSnapshot import com.squareup.workflow1.Workflow -import com.squareup.workflow1.WorkflowExperimentalRuntime import com.squareup.workflow1.WorkflowInterceptor import com.squareup.workflow1.WorkflowTracer import kotlinx.coroutines.CancellationException @@ -83,16 +81,18 @@ internal class WorkflowRunner( * and resume (breaking ties with order of declaration). Guarantees only continuing on the winning * coroutine and no others. */ - @OptIn(WorkflowExperimentalRuntime::class) - suspend fun processAction(waitForAnAction: Boolean = true): ActionProcessingResult { + suspend fun processAction( + waitForAnAction: Boolean = true, + skipChangedNodes: Boolean = false + ): ActionProcessingResult { // If waitForAction is true we block and wait until there is an action to process. return select { onPropsUpdated() // Have the workflow tree build the select to wait for an event/output from Worker. - val empty = rootNode.onNextAction(this) - if (!waitForAnAction && runtimeConfig.contains(CONFLATE_STALE_RENDERINGS) && empty) { - // With CONFLATE_STALE_RENDERINGS if there are no queued actions and we are not - // waiting for one, then return ActionsExhausted and pass the rendering on. + val empty = rootNode.onNextAction(this, skipChangedNodes) + if (!waitForAnAction && empty) { + // With CONFLATE_STALE_RENDERINGS && DRAIN_EXCLUSIVE_ACTIONS if there are no queued actions + // and we are not waiting for one, then return ActionsExhausted and pass the rendering on. onTimeout(timeMillis = 0) { // This will select synchronously since time is 0. ActionsExhausted diff --git a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt index 504203d52..b0b673fc8 100644 --- a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt +++ b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/RenderWorkflowInTest.kt @@ -1,6 +1,7 @@ package com.squareup.workflow1 import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS +import com.squareup.workflow1.RuntimeConfigOptions.DRAIN_EXCLUSIVE_ACTIONS import com.squareup.workflow1.RuntimeConfigOptions.PARTIAL_TREE_RENDERING import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES import com.squareup.workflow1.WorkflowInterceptor.RenderPassSkipped @@ -58,6 +59,17 @@ class RenderWorkflowInTest { setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES), setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING), setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING), + setOf(DRAIN_EXCLUSIVE_ACTIONS), + setOf(RENDER_ONLY_WHEN_STATE_CHANGES, DRAIN_EXCLUSIVE_ACTIONS), + setOf(CONFLATE_STALE_RENDERINGS, DRAIN_EXCLUSIVE_ACTIONS), + setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES, DRAIN_EXCLUSIVE_ACTIONS), + setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING, DRAIN_EXCLUSIVE_ACTIONS), + setOf( + CONFLATE_STALE_RENDERINGS, + RENDER_ONLY_WHEN_STATE_CHANGES, + PARTIAL_TREE_RENDERING, + DRAIN_EXCLUSIVE_ACTIONS + ), ) private val tracerOptions = setOf( @@ -1390,7 +1402,7 @@ class RenderWorkflowInTest { advanceIfStandard(dispatcher) sink.send("changing state") advanceIfStandard(dispatcher) - assertEquals(2, emitted.size) + assertEquals(2, emitted.size, "Expecting 2 renderings to be emitted.") collectionJob.cancel() } @@ -1529,10 +1541,16 @@ class RenderWorkflowInTest { ) {} advanceIfStandard(dispatcher) - trigger.emit("state 1") // different value than the child starts with. + launch { + trigger.emit("state 1") // different value than the child starts with. + } advanceIfStandard(dispatcher) - assertEquals(3, parentRenderCount) + if (runtimeConfig.contains(DRAIN_EXCLUSIVE_ACTIONS)) { + assertEquals(2, parentRenderCount) + } else { + assertEquals(3, parentRenderCount) + } // Parent needs to be rendered 3x, but child only 2x as the 3rd time its the same. assertEquals(2, childRenderCount) } @@ -1543,9 +1561,8 @@ class RenderWorkflowInTest { fun for_render_on_change_only_and_conflate_we_drain_action_but_do_not_render_no_state_changed() { runtimeTestRunner.runParametrizedTest( paramSource = runtimeOptions.filter { - it.first.contains(RENDER_ONLY_WHEN_STATE_CHANGES) && it.first.contains( - CONFLATE_STALE_RENDERINGS - ) + it.first.contains(RENDER_ONLY_WHEN_STATE_CHANGES) && + it.first.contains(CONFLATE_STALE_RENDERINGS) }, before = ::setup, ) { (runtimeConfig: RuntimeConfig, workflowTracer: WorkflowTracer?, dispatcher: TestDispatcher) -> @@ -1575,9 +1592,10 @@ class RenderWorkflowInTest { val workflow = Workflow.stateful( initialState = "unchanging state", render = { renderState -> - renderChild(childWorkflow) { _ -> + renderChild(childWorkflow) { childOutput -> action("childHandler") { childHandlerActionExecuted++ + setOutput(childOutput) } } runningWorker( @@ -1603,6 +1621,7 @@ class RenderWorkflowInTest { workflowTracer = workflowTracer, ) { outputSet.add(it) + yield() } advanceIfStandard(dispatcher) @@ -1615,7 +1634,7 @@ class RenderWorkflowInTest { assertEquals(2, renderCount) assertEquals(1, childHandlerActionExecuted) assertEquals(1, workerActionExecuted) - assertEquals(1, outputSet.size) + assertEquals(2, outputSet.size) assertEquals("changed state", outputSet[0]) } } @@ -1707,7 +1726,7 @@ class RenderWorkflowInTest { collectionJob.cancel() // 2 renderings (initial and then the update.) Not *3* renderings. - assertEquals(2, emitted.size) + assertEquals(2, emitted.size, "Expected only 2 total renderings.") assertEquals("changed state+update", emitted.last()) assertTrue(childHandlerActionExecuted) } @@ -1976,6 +1995,319 @@ class RenderWorkflowInTest { } } + @Test + fun for_drain_exclusive_we_handle_multiple_actions_in_one_render_or_not() { + runtimeTestRunner.runParametrizedTest( + paramSource = runtimeOptions, + before = ::setup, + ) { (runtimeConfig: RuntimeConfig, workflowTracer: WorkflowTracer?, dispatcher: TestDispatcher) -> + runTest(dispatcher) { + + var childActionAppliedCount = 0 + var parentRenderCount = 0 + val trigger = MutableSharedFlow() + + val childWorkflow = Workflow.stateful( + initialState = "unchanged state", + render = { renderState -> + runningWorker( + trigger.asWorker() + ) { + action("") { + state = it + childActionAppliedCount++ + } + } + renderState + } + ) + val workflow = Workflow.stateful( + initialState = "unchanging state", + render = { renderState -> + renderChild(childWorkflow, key = "key1") { _ -> + WorkflowAction.noAction() + } + renderChild(childWorkflow, key = "key2") { _ -> + WorkflowAction.noAction() + } + parentRenderCount++ + renderState + } + ) + val props = MutableStateFlow(Unit) + renderWorkflowIn( + workflow = workflow, + scope = backgroundScope, + props = props, + runtimeConfig = runtimeConfig, + workflowTracer = workflowTracer, + ) { } + advanceIfStandard(dispatcher) + + launch { + trigger.emit("changed state") + } + advanceIfStandard(dispatcher) + + // 2 child actions processed. + assertEquals(2, childActionAppliedCount, "Expecting 2 child actions to be applied.") + if (runtimeConfig.contains(DRAIN_EXCLUSIVE_ACTIONS)) { + // and 2 parent renders - 1 initial (synchronous) and then 1 additional. + assertEquals(2, parentRenderCount, "Expecting only 2 total renders.") + } else { + // and 3 parent renders - 1 initial (synchronous) and then 1 additional for each child. + assertEquals(3, parentRenderCount, "Expecting only 3 total renders.") + } + } + } + } + + @Test + fun `for_drain_exclusive_and_render_only_when_state_changes_we_handle_multiple_actions_in_one_render_but_do_not_render_if_no_state_change`() { + runtimeTestRunner.runParametrizedTest( + paramSource = runtimeOptions + .filter { + it.first.contains(DRAIN_EXCLUSIVE_ACTIONS) && + it.first.contains(RENDER_ONLY_WHEN_STATE_CHANGES) + }, + before = ::setup, + ) { (runtimeConfig: RuntimeConfig, workflowTracer: WorkflowTracer?, dispatcher: TestDispatcher) -> + runTest(dispatcher) { + check(runtimeConfig.contains(DRAIN_EXCLUSIVE_ACTIONS)) + check(runtimeConfig.contains(RENDER_ONLY_WHEN_STATE_CHANGES)) + + var childActionAppliedCount = 0 + var parentRenderCount = 0 + val trigger = MutableSharedFlow() + val receivedOutputs = mutableListOf() + + val childWorkflow = Workflow.stateful( + initialState = "unchanged state", + render = { renderState -> + runningWorker( + trigger.asWorker() + ) { + action("") { + // no state change! + childActionAppliedCount++ + setOutput(it) + } + } + renderState + } + ) + val workflow = Workflow.stateful( + initialState = "unchanging state", + render = { renderState -> + renderChild(childWorkflow, key = "key1") { _ -> + WorkflowAction.noAction() + } + renderChild(childWorkflow, key = "key2") { childOutput -> + action(name = "Child2Handler") { + // Second one sets output to test that we still send the output! + setOutput(childOutput) + } + } + parentRenderCount++ + renderState + } + ) + val props = MutableStateFlow(Unit) + renderWorkflowIn( + workflow = workflow, + scope = backgroundScope, + props = props, + runtimeConfig = runtimeConfig, + workflowTracer = workflowTracer, + ) { + receivedOutputs.add(it) + } + advanceIfStandard(dispatcher) + + launch { + trigger.emit("changed state") + } + advanceIfStandard(dispatcher) + + // 2 child actions processed and 1 parent render - only the initial one. + assertEquals(2, childActionAppliedCount, "Expected each child action applied.") + assertEquals(1, parentRenderCount, "Expected parent only rendered once.") + assertEquals(1, receivedOutputs.size, "Expected one output.") + assertEquals("changed state", receivedOutputs[0]) + } + } + } + + @Test + fun `for_drain_exclusive_and_render_only_when_state_changes_we_handle_multiple_actions_in_one_render_but_we_do_pass_rendering_if_state_changed_earlier`() { + runtimeTestRunner.runParametrizedTest( + paramSource = runtimeOptions + .filter { + it.first.contains(DRAIN_EXCLUSIVE_ACTIONS) && + it.first.contains(RENDER_ONLY_WHEN_STATE_CHANGES) + }, + before = ::setup, + ) { (runtimeConfig: RuntimeConfig, workflowTracer: WorkflowTracer?, dispatcher: TestDispatcher) -> + runTest(dispatcher) { + check(runtimeConfig.contains(DRAIN_EXCLUSIVE_ACTIONS)) + check(runtimeConfig.contains(RENDER_ONLY_WHEN_STATE_CHANGES)) + + var childActionAppliedCount = 0 + var parentRenderCount = 0 + val trigger = MutableSharedFlow() + val receivedOutputs = mutableListOf() + val emitted = mutableListOf() + + val childWorkflow = Workflow.stateful( + initialState = "unchanged state", + render = { renderState -> + runningWorker( + trigger.asWorker() + ) { + action("") { + if (childActionAppliedCount == 0) { + // change state on the first one. + state = "$it+update" + } else { + // no state change! + } + childActionAppliedCount++ + setOutput(it) + } + } + renderState + } + ) + val workflow = Workflow.stateful( + initialState = "unchanging state", + render = { renderState -> + renderChild(childWorkflow, key = "key1") { _ -> + WorkflowAction.noAction() + } + renderChild(childWorkflow, key = "key2") { childOutput -> + action(name = "Child2Handler") { + // Second one sets output to test that we still send the output! + setOutput(childOutput) + } + } + parentRenderCount++ + renderState + } + ) + val props = MutableStateFlow(Unit) + val renderings = renderWorkflowIn( + workflow = workflow, + scope = backgroundScope, + props = props, + runtimeConfig = runtimeConfig, + workflowTracer = workflowTracer, + ) { + receivedOutputs.add(it) + } + advanceIfStandard(dispatcher) + + val collectionJob = launch { + // Collect this unconfined so we can get all the renderings faster than actions can + // be processed. + renderings.collect { + emitted += it.rendering + } + } + advanceIfStandard(dispatcher) + + launch { + trigger.emit("changed state") + } + advanceIfStandard(dispatcher) + + collectionJob.cancel() + + // 2 renderings received always as the state on child changes! + assertEquals(2, emitted.size) + // 2 child actions processed and 1 (or 2) parent renders. + assertEquals(2, childActionAppliedCount, "Expected each child action applied.") + if (runtimeConfig.contains(PARTIAL_TREE_RENDERING)) { + assertEquals(1, parentRenderCount, "Expected parent only rendered once.") + } else { + assertEquals(2, parentRenderCount, "Expected parent rendered twice.") + } + assertEquals(1, receivedOutputs.size, "Expected one output.") + assertEquals("changed state", receivedOutputs[0]) + } + } + } + + @Test + fun for_drain_exclusive_we_do_not_handle_multiple_actions_in_one_render_if_not_exclusive() { + runtimeTestRunner.runParametrizedTest( + paramSource = runtimeOptions + .filter { + it.first.contains(DRAIN_EXCLUSIVE_ACTIONS) + }, + before = ::setup, + ) { (runtimeConfig: RuntimeConfig, workflowTracer: WorkflowTracer?, dispatcher: TestDispatcher) -> + runTest(dispatcher) { + check(runtimeConfig.contains(DRAIN_EXCLUSIVE_ACTIONS)) + + var childActionAppliedCount = 0 + var parentRenderCount = 0 + val trigger = MutableSharedFlow() + + val childWorkflow = Workflow.stateful( + initialState = "unchanged state", + render = { renderState -> + runningWorker( + trigger.asWorker() + ) { + action("") { + state = it + childActionAppliedCount++ + // set the output to dirty the parent node. + setOutput(it) + } + } + renderState + } + ) + val workflow = Workflow.stateful( + initialState = "unchanging state", + render = { renderState -> + renderChild(childWorkflow, key = "key1") { childOutput -> + action("childHandler1") { + state = childOutput + } + } + renderChild(childWorkflow, key = "key2") { childOutput -> + action("childHandler2") { + state = childOutput + } + } + parentRenderCount++ + renderState + } + ) + val props = MutableStateFlow(Unit) + renderWorkflowIn( + workflow = workflow, + scope = backgroundScope, + props = props, + runtimeConfig = runtimeConfig, + workflowTracer = workflowTracer, + ) { } + advanceIfStandard(dispatcher) + + launch { + trigger.emit("changed state") + } + advanceIfStandard(dispatcher) + + // 2 child actions processed and 3 parent renders + assertEquals(2, childActionAppliedCount, "Expecting 2 child actions applied") + assertEquals(3, parentRenderCount, "Expecting 3 parent renders") + } + } + } + private class ExpectedException : RuntimeException() private fun cartesianProduct( diff --git a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/internal/WorkflowRunnerTest.kt b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/internal/WorkflowRunnerTest.kt index 18e657ce4..4b455f9a4 100644 --- a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/internal/WorkflowRunnerTest.kt +++ b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/internal/WorkflowRunnerTest.kt @@ -5,6 +5,7 @@ import com.squareup.workflow1.NoopWorkflowInterceptor import com.squareup.workflow1.RuntimeConfig import com.squareup.workflow1.RuntimeConfigOptions import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS +import com.squareup.workflow1.RuntimeConfigOptions.DRAIN_EXCLUSIVE_ACTIONS import com.squareup.workflow1.RuntimeConfigOptions.PARTIAL_TREE_RENDERING import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES import com.squareup.workflow1.Worker @@ -38,6 +39,17 @@ internal class WorkflowRunnerTest { setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES), setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING), setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING), + setOf(DRAIN_EXCLUSIVE_ACTIONS), + setOf(RENDER_ONLY_WHEN_STATE_CHANGES, DRAIN_EXCLUSIVE_ACTIONS), + setOf(CONFLATE_STALE_RENDERINGS, DRAIN_EXCLUSIVE_ACTIONS), + setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES, DRAIN_EXCLUSIVE_ACTIONS), + setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING, DRAIN_EXCLUSIVE_ACTIONS), + setOf( + CONFLATE_STALE_RENDERINGS, + RENDER_ONLY_WHEN_STATE_CHANGES, + PARTIAL_TREE_RENDERING, + DRAIN_EXCLUSIVE_ACTIONS + ), ).asSequence() private fun setup() { diff --git a/workflow-testing/src/test/java/com/squareup/workflow1/WorkflowsLifecycleTests.kt b/workflow-testing/src/test/java/com/squareup/workflow1/WorkflowsLifecycleTests.kt index 5f3b6841b..bbafc05de 100644 --- a/workflow-testing/src/test/java/com/squareup/workflow1/WorkflowsLifecycleTests.kt +++ b/workflow-testing/src/test/java/com/squareup/workflow1/WorkflowsLifecycleTests.kt @@ -1,6 +1,7 @@ package com.squareup.workflow1 import com.squareup.workflow1.RuntimeConfigOptions.CONFLATE_STALE_RENDERINGS +import com.squareup.workflow1.RuntimeConfigOptions.DRAIN_EXCLUSIVE_ACTIONS import com.squareup.workflow1.RuntimeConfigOptions.PARTIAL_TREE_RENDERING import com.squareup.workflow1.RuntimeConfigOptions.RENDER_ONLY_WHEN_STATE_CHANGES import com.squareup.workflow1.testing.headlessIntegrationTest @@ -24,6 +25,17 @@ class WorkflowsLifecycleTests { setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES), setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING), setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING), + setOf(DRAIN_EXCLUSIVE_ACTIONS), + setOf(RENDER_ONLY_WHEN_STATE_CHANGES, DRAIN_EXCLUSIVE_ACTIONS), + setOf(CONFLATE_STALE_RENDERINGS, DRAIN_EXCLUSIVE_ACTIONS), + setOf(CONFLATE_STALE_RENDERINGS, RENDER_ONLY_WHEN_STATE_CHANGES, DRAIN_EXCLUSIVE_ACTIONS), + setOf(RENDER_ONLY_WHEN_STATE_CHANGES, PARTIAL_TREE_RENDERING, DRAIN_EXCLUSIVE_ACTIONS), + setOf( + CONFLATE_STALE_RENDERINGS, + RENDER_ONLY_WHEN_STATE_CHANGES, + PARTIAL_TREE_RENDERING, + DRAIN_EXCLUSIVE_ACTIONS + ), ).asSequence() private val runtimeTestRunner = ParameterizedTestRunner() @@ -229,7 +241,9 @@ class WorkflowsLifecycleTests { setState.invoke(1) setState.invoke(2) awaitNextRendering() - if (!runtimeConfig.contains(CONFLATE_STALE_RENDERINGS)) { + if (!runtimeConfig.contains(CONFLATE_STALE_RENDERINGS) && + !runtimeConfig.contains(DRAIN_EXCLUSIVE_ACTIONS) + ) { // 2 rendering or 1 depending on runtime config. awaitNextRendering() }