From e328ea521cc7a7481936ca514743c2c6d6097e6b Mon Sep 17 00:00:00 2001 From: Zach Klippenstein Date: Wed, 24 Jun 2020 19:33:27 -0700 Subject: [PATCH] WIP Reimplemented Workers using side effects. Issues: - Fixes #82, which is the Kotlin half of https://github.com/square/workflow/issues/1021. - Fixes #92. - Fixes https://github.com/square/workflow/issues/1197. --- .../sample/dungeon/DungeonAppWorkflow.kt | 1 + .../sample/dungeon/GameSessionWorkflow.kt | 1 + .../com/squareup/sample/dungeon/AiWorkflow.kt | 1 + .../squareup/sample/dungeon/GameWorkflow.kt | 1 + .../shakeable/ShakeableTimeMachineWorkflow.kt | 1 + .../helloterminal/BlinkingCursorWorkflow.kt | 1 + .../helloterminal/HelloTerminalWorkflow.kt | 1 + .../sample/authworkflow/AuthWorkflow.kt | 1 + .../sample/gameworkflow/RunGameWorkflow.kt | 1 + workflow-core/api/workflow-core.api | 9 +- .../com/squareup/workflow/LifecycleWorker.kt | 6 +- .../com/squareup/workflow/RenderContext.kt | 61 +++- .../main/java/com/squareup/workflow/Worker.kt | 36 +-- .../com/squareup/workflow/WorkerWorkflow.kt | 103 +++++++ .../squareup/workflow/WorkflowIdentifier.kt | 8 +- .../squareup/workflow}/NullFlowWorker.java | 10 +- .../java/com/squareup/workflow/SinkTest.kt | 59 +++- .../java/com/squareup/workflow/WorkerTest.kt | 30 +- .../squareup/workflow/WorkerWorkflowTest.kt | 83 ++++++ workflow-runtime/api/workflow-runtime.api | 4 +- .../com/squareup/workflow/RenderWorkflow.kt | 25 +- .../workflow/internal/RealRenderContext.kt | 19 -- .../workflow/internal/SubtreeManager.kt | 7 +- .../workflow/internal/WorkerChildNode.kt | 66 ----- .../com/squareup/workflow/internal/Workers.kt | 115 -------- .../workflow/internal/WorkflowNode.kt | 64 +---- .../workflow/internal/WorkflowRunner.kt | 6 +- .../squareup/workflow/RenderWorkflowInTest.kt | 102 +++++-- .../workflow/WorkflowInterceptorTest.kt | 6 - .../workflow/WorkflowOperatorsTest.kt | 2 +- .../ChainedWorkflowInterceptorTest.kt | 9 - .../internal/RealRenderContextTest.kt | 30 +- .../squareup/workflow/internal/WorkersTest.kt | 220 --------------- .../workflow/internal/WorkflowNodeTest.kt | 267 +----------------- .../workflow/internal/WorkflowRunnerTest.kt | 3 - workflow-rx2/api/workflow-rx2.api | 1 + .../workflow/rx2/PublisherWorkerTest.kt | 1 + workflow-testing/api/workflow-testing.api | 49 +--- .../workflow/testing/RealRenderTester.kt | 39 --- .../testing/RenderIdempotencyChecker.kt | 12 - .../squareup/workflow/testing/RenderTester.kt | 154 +++++++--- .../workflow/testing/WorkflowTestRuntime.kt | 3 - .../WorkerCompositionIntegrationTest.kt | 17 +- .../workflow/testing/RealRenderTesterTest.kt | 110 +------- .../testing/WorkerRenderExpectationsTest.kt | 261 +++++++++++++++++ .../workflow/testing/WorkerSinkTest.kt | 25 ++ .../testing/WorkflowTestRuntimeTest.kt | 1 + .../tracing/TracingWorkflowInterceptor.kt | 123 +------- .../tracing/TracingWorkflowInterceptorTest.kt | 1 + 49 files changed, 891 insertions(+), 1265 deletions(-) create mode 100644 workflow-core/src/main/java/com/squareup/workflow/WorkerWorkflow.kt rename {workflow-runtime/src/test/java/com/squareup/workflow/internal => workflow-core/src/test/java/com/squareup/workflow}/NullFlowWorker.java (81%) create mode 100644 workflow-core/src/test/java/com/squareup/workflow/WorkerWorkflowTest.kt delete mode 100644 workflow-runtime/src/main/java/com/squareup/workflow/internal/WorkerChildNode.kt delete mode 100644 workflow-runtime/src/main/java/com/squareup/workflow/internal/Workers.kt delete mode 100644 workflow-runtime/src/test/java/com/squareup/workflow/internal/WorkersTest.kt create mode 100644 workflow-testing/src/test/java/com/squareup/workflow/testing/WorkerRenderExpectationsTest.kt diff --git a/samples/dungeon/app/src/main/java/com/squareup/sample/dungeon/DungeonAppWorkflow.kt b/samples/dungeon/app/src/main/java/com/squareup/sample/dungeon/DungeonAppWorkflow.kt index abd4824d90..669229bae6 100644 --- a/samples/dungeon/app/src/main/java/com/squareup/sample/dungeon/DungeonAppWorkflow.kt +++ b/samples/dungeon/app/src/main/java/com/squareup/sample/dungeon/DungeonAppWorkflow.kt @@ -26,6 +26,7 @@ import com.squareup.workflow.Snapshot import com.squareup.workflow.StatefulWorkflow import com.squareup.workflow.action import com.squareup.workflow.renderChild +import com.squareup.workflow.runningWorker import com.squareup.workflow.ui.WorkflowUiExperimentalApi import com.squareup.workflow.ui.modal.AlertContainerScreen diff --git a/samples/dungeon/app/src/main/java/com/squareup/sample/dungeon/GameSessionWorkflow.kt b/samples/dungeon/app/src/main/java/com/squareup/sample/dungeon/GameSessionWorkflow.kt index 2586c08908..dc857986bf 100644 --- a/samples/dungeon/app/src/main/java/com/squareup/sample/dungeon/GameSessionWorkflow.kt +++ b/samples/dungeon/app/src/main/java/com/squareup/sample/dungeon/GameSessionWorkflow.kt @@ -31,6 +31,7 @@ import com.squareup.workflow.WorkflowAction import com.squareup.workflow.WorkflowAction.Companion.noAction import com.squareup.workflow.WorkflowAction.Updater import com.squareup.workflow.action +import com.squareup.workflow.runningWorker import com.squareup.workflow.ui.WorkflowUiExperimentalApi import com.squareup.workflow.ui.modal.AlertContainerScreen import com.squareup.workflow.ui.modal.AlertScreen diff --git a/samples/dungeon/common/src/main/java/com/squareup/sample/dungeon/AiWorkflow.kt b/samples/dungeon/common/src/main/java/com/squareup/sample/dungeon/AiWorkflow.kt index d6b9289cf1..5cfb3ee12e 100644 --- a/samples/dungeon/common/src/main/java/com/squareup/sample/dungeon/AiWorkflow.kt +++ b/samples/dungeon/common/src/main/java/com/squareup/sample/dungeon/AiWorkflow.kt @@ -28,6 +28,7 @@ import com.squareup.workflow.Snapshot import com.squareup.workflow.StatefulWorkflow import com.squareup.workflow.Worker import com.squareup.workflow.action +import com.squareup.workflow.runningWorker import com.squareup.workflow.transform import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.transform diff --git a/samples/dungeon/common/src/main/java/com/squareup/sample/dungeon/GameWorkflow.kt b/samples/dungeon/common/src/main/java/com/squareup/sample/dungeon/GameWorkflow.kt index bf9f9c923b..feccecd2b2 100644 --- a/samples/dungeon/common/src/main/java/com/squareup/sample/dungeon/GameWorkflow.kt +++ b/samples/dungeon/common/src/main/java/com/squareup/sample/dungeon/GameWorkflow.kt @@ -36,6 +36,7 @@ import com.squareup.workflow.StatefulWorkflow import com.squareup.workflow.Worker import com.squareup.workflow.action import com.squareup.workflow.renderChild +import com.squareup.workflow.runningWorker import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flow diff --git a/samples/dungeon/timemachine-shakeable/src/main/java/com/squareup/sample/timemachine/shakeable/ShakeableTimeMachineWorkflow.kt b/samples/dungeon/timemachine-shakeable/src/main/java/com/squareup/sample/timemachine/shakeable/ShakeableTimeMachineWorkflow.kt index b51b0c5fdd..6806320e4a 100644 --- a/samples/dungeon/timemachine-shakeable/src/main/java/com/squareup/sample/timemachine/shakeable/ShakeableTimeMachineWorkflow.kt +++ b/samples/dungeon/timemachine-shakeable/src/main/java/com/squareup/sample/timemachine/shakeable/ShakeableTimeMachineWorkflow.kt @@ -28,6 +28,7 @@ import com.squareup.workflow.StatefulWorkflow import com.squareup.workflow.WorkflowAction import com.squareup.workflow.WorkflowAction.Updater import com.squareup.workflow.action +import com.squareup.workflow.runningWorker import kotlin.time.Duration import kotlin.time.ExperimentalTime diff --git a/samples/hello-terminal/hello-terminal-app/src/main/java/com/squareup/sample/helloterminal/BlinkingCursorWorkflow.kt b/samples/hello-terminal/hello-terminal-app/src/main/java/com/squareup/sample/helloterminal/BlinkingCursorWorkflow.kt index b53d9e4e96..aec54e6748 100644 --- a/samples/hello-terminal/hello-terminal-app/src/main/java/com/squareup/sample/helloterminal/BlinkingCursorWorkflow.kt +++ b/samples/hello-terminal/hello-terminal-app/src/main/java/com/squareup/sample/helloterminal/BlinkingCursorWorkflow.kt @@ -20,6 +20,7 @@ import com.squareup.workflow.Snapshot import com.squareup.workflow.StatefulWorkflow import com.squareup.workflow.Worker import com.squareup.workflow.action +import com.squareup.workflow.runningWorker import kotlinx.coroutines.delay /** diff --git a/samples/hello-terminal/hello-terminal-app/src/main/java/com/squareup/sample/helloterminal/HelloTerminalWorkflow.kt b/samples/hello-terminal/hello-terminal-app/src/main/java/com/squareup/sample/helloterminal/HelloTerminalWorkflow.kt index b025100f4f..bfd90736e7 100644 --- a/samples/hello-terminal/hello-terminal-app/src/main/java/com/squareup/sample/helloterminal/HelloTerminalWorkflow.kt +++ b/samples/hello-terminal/hello-terminal-app/src/main/java/com/squareup/sample/helloterminal/HelloTerminalWorkflow.kt @@ -29,6 +29,7 @@ import com.squareup.workflow.StatefulWorkflow import com.squareup.workflow.WorkflowAction import com.squareup.workflow.action import com.squareup.workflow.renderChild +import com.squareup.workflow.runningWorker private typealias HelloTerminalAction = WorkflowAction diff --git a/samples/tictactoe/common/src/main/java/com/squareup/sample/authworkflow/AuthWorkflow.kt b/samples/tictactoe/common/src/main/java/com/squareup/sample/authworkflow/AuthWorkflow.kt index 8a86c02b73..d71e1a3e36 100644 --- a/samples/tictactoe/common/src/main/java/com/squareup/sample/authworkflow/AuthWorkflow.kt +++ b/samples/tictactoe/common/src/main/java/com/squareup/sample/authworkflow/AuthWorkflow.kt @@ -36,6 +36,7 @@ import com.squareup.workflow.StatefulWorkflow import com.squareup.workflow.Workflow import com.squareup.workflow.WorkflowAction import com.squareup.workflow.WorkflowAction.Updater +import com.squareup.workflow.runningWorker import com.squareup.workflow.rx2.asWorker import com.squareup.workflow.ui.WorkflowUiExperimentalApi import com.squareup.workflow.ui.backstack.BackStackScreen diff --git a/samples/tictactoe/common/src/main/java/com/squareup/sample/gameworkflow/RunGameWorkflow.kt b/samples/tictactoe/common/src/main/java/com/squareup/sample/gameworkflow/RunGameWorkflow.kt index 2edb5c0584..b8f1ef4310 100644 --- a/samples/tictactoe/common/src/main/java/com/squareup/sample/gameworkflow/RunGameWorkflow.kt +++ b/samples/tictactoe/common/src/main/java/com/squareup/sample/gameworkflow/RunGameWorkflow.kt @@ -46,6 +46,7 @@ import com.squareup.workflow.StatefulWorkflow import com.squareup.workflow.Workflow import com.squareup.workflow.WorkflowAction import com.squareup.workflow.WorkflowAction.Updater +import com.squareup.workflow.runningWorker import com.squareup.workflow.rx2.asWorker import com.squareup.workflow.ui.WorkflowUiExperimentalApi import com.squareup.workflow.ui.modal.AlertContainerScreen diff --git a/workflow-core/api/workflow-core.api b/workflow-core/api/workflow-core.api index 491faa5e4a..767409c1d6 100644 --- a/workflow-core/api/workflow-core.api +++ b/workflow-core/api/workflow-core.api @@ -22,6 +22,7 @@ public final class com/squareup/workflow/ImpostorWorkflow$DefaultImpls { public abstract class com/squareup/workflow/LifecycleWorker : com/squareup/workflow/Worker { public fun ()V public fun doesSameWorkAs (Lcom/squareup/workflow/Worker;)Z + public final fun getOutputType ()Lkotlin/reflect/KType; public fun onStarted ()V public fun onStopped ()V public final fun run ()Lkotlinx/coroutines/flow/Flow; @@ -33,14 +34,12 @@ public abstract interface class com/squareup/workflow/RenderContext { public abstract fun onEvent (Lkotlin/jvm/functions/Function1;)Lkotlin/jvm/functions/Function1; public abstract fun renderChild (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)Ljava/lang/Object; public abstract fun runningSideEffect (Ljava/lang/String;Lkotlin/jvm/functions/Function1;)V - public abstract fun runningWorker (Lcom/squareup/workflow/Worker;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)V } public final class com/squareup/workflow/RenderContext$DefaultImpls { public static fun makeActionSink (Lcom/squareup/workflow/RenderContext;)Lcom/squareup/workflow/Sink; public static fun onEvent (Lcom/squareup/workflow/RenderContext;Lkotlin/jvm/functions/Function1;)Lkotlin/jvm/functions/Function1; public static synthetic fun renderChild$default (Lcom/squareup/workflow/RenderContext;Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Ljava/lang/Object; - public static synthetic fun runningWorker$default (Lcom/squareup/workflow/RenderContext;Lcom/squareup/workflow/Worker;Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)V } public abstract interface class com/squareup/workflow/Sink { @@ -108,6 +107,7 @@ public abstract class com/squareup/workflow/StatelessWorkflow : com/squareup/wor public final class com/squareup/workflow/TypedWorker : com/squareup/workflow/Worker { public fun (Lkotlin/reflect/KType;Lkotlinx/coroutines/flow/Flow;)V public fun doesSameWorkAs (Lcom/squareup/workflow/Worker;)Z + public fun getOutputType ()Lkotlin/reflect/KType; public fun run ()Lkotlinx/coroutines/flow/Flow; public fun toString ()Ljava/lang/String; } @@ -115,6 +115,7 @@ public final class com/squareup/workflow/TypedWorker : com/squareup/workflow/Wor public abstract interface class com/squareup/workflow/Worker { public static final field Companion Lcom/squareup/workflow/Worker$Companion; public abstract fun doesSameWorkAs (Lcom/squareup/workflow/Worker;)Z + public abstract fun getOutputType ()Lkotlin/reflect/KType; public abstract fun run ()Lkotlinx/coroutines/flow/Flow; } @@ -127,6 +128,7 @@ public final class com/squareup/workflow/Worker$Companion { public final class com/squareup/workflow/Worker$DefaultImpls { public static fun doesSameWorkAs (Lcom/squareup/workflow/Worker;Lcom/squareup/workflow/Worker;)Z + public static fun getOutputType (Lcom/squareup/workflow/Worker;)Lkotlin/reflect/KType; } public abstract interface class com/squareup/workflow/Workflow { @@ -217,8 +219,6 @@ public final class com/squareup/workflow/Workflows { public static final fun invoke (Lcom/squareup/workflow/EventHandler;)V public static final fun makeEventSink (Lcom/squareup/workflow/RenderContext;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow/Sink; public static final fun mapRendering (Lcom/squareup/workflow/Workflow;Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow/Workflow; - public static final fun onWorkerOutput (Lcom/squareup/workflow/RenderContext;Lcom/squareup/workflow/Worker;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)V - public static synthetic fun onWorkerOutput$default (Lcom/squareup/workflow/RenderContext;Lcom/squareup/workflow/Worker;Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)V public static final fun renderChild (Lcom/squareup/workflow/RenderContext;Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Ljava/lang/String;)Ljava/lang/Object; public static final fun renderChild (Lcom/squareup/workflow/RenderContext;Lcom/squareup/workflow/Workflow;Ljava/lang/String;)Ljava/lang/Object; public static final fun renderChild (Lcom/squareup/workflow/RenderContext;Lcom/squareup/workflow/Workflow;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)Ljava/lang/Object; @@ -227,6 +227,7 @@ public final class com/squareup/workflow/Workflows { public static synthetic fun renderChild$default (Lcom/squareup/workflow/RenderContext;Lcom/squareup/workflow/Workflow;Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Ljava/lang/Object; public static final fun rendering (Lcom/squareup/workflow/Workflow$Companion;Ljava/lang/Object;)Lcom/squareup/workflow/Workflow; public static final fun runningWorker (Lcom/squareup/workflow/RenderContext;Lcom/squareup/workflow/Worker;Ljava/lang/String;)V + public static final fun runningWorker (Lcom/squareup/workflow/RenderContext;Lcom/squareup/workflow/Worker;Lkotlin/reflect/KType;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)V public static synthetic fun runningWorker$default (Lcom/squareup/workflow/RenderContext;Lcom/squareup/workflow/Worker;Ljava/lang/String;ILjava/lang/Object;)V public static final fun sendAndAwaitApplication (Lcom/squareup/workflow/Sink;Lcom/squareup/workflow/WorkflowAction;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public static final fun stateful (Lcom/squareup/workflow/Workflow$Companion;Ljava/lang/Object;Lkotlin/jvm/functions/Function2;)Lcom/squareup/workflow/StatefulWorkflow; diff --git a/workflow-core/src/main/java/com/squareup/workflow/LifecycleWorker.kt b/workflow-core/src/main/java/com/squareup/workflow/LifecycleWorker.kt index 921484fcb1..33d1e03b75 100644 --- a/workflow-core/src/main/java/com/squareup/workflow/LifecycleWorker.kt +++ b/workflow-core/src/main/java/com/squareup/workflow/LifecycleWorker.kt @@ -21,6 +21,7 @@ package com.squareup.workflow import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flow import kotlinx.coroutines.suspendCancellableCoroutine +import kotlin.reflect.KType /** * [Worker] that performs some action when the worker is started and/or stopped. @@ -30,6 +31,8 @@ import kotlinx.coroutines.suspendCancellableCoroutine */ abstract class LifecycleWorker : Worker { + final override val outputType: KType? get() = null + /** * Called when this worker is started. It is executed concurrently with the parent workflow – * the first render pass that starts this worker *will not* wait for this method to return, and @@ -73,6 +76,5 @@ abstract class LifecycleWorker : Worker { /** * Equates [LifecycleWorker]s that have the same concrete class. */ - override fun doesSameWorkAs(otherWorker: Worker<*>): Boolean = - this::class == otherWorker::class + override fun doesSameWorkAs(otherWorker: Worker<*>): Boolean = true } diff --git a/workflow-core/src/main/java/com/squareup/workflow/RenderContext.kt b/workflow-core/src/main/java/com/squareup/workflow/RenderContext.kt index d515cca15a..fb73525f33 100644 --- a/workflow-core/src/main/java/com/squareup/workflow/RenderContext.kt +++ b/workflow-core/src/main/java/com/squareup/workflow/RenderContext.kt @@ -21,6 +21,8 @@ package com.squareup.workflow import com.squareup.workflow.WorkflowAction.Companion.noAction import com.squareup.workflow.WorkflowAction.Updater +import kotlin.reflect.KType +import kotlin.reflect.typeOf /** * Facilities for a [Workflow] to interact with other [Workflow]s and the outside world from inside @@ -109,19 +111,6 @@ interface RenderContext { handler: (ChildOutputT) -> WorkflowAction ): ChildRenderingT - /** - * Ensures [worker] is running. When the [Worker] emits an output, [handler] is called - * to determine the [WorkflowAction] to take. When the worker finishes, nothing happens (although - * another render pass may be triggered). - * - * @param key An optional string key that is used to distinguish between identical [Worker]s. - */ - fun runningWorker( - worker: Worker, - key: String = "", - handler: (T) -> WorkflowAction - ) - /** * Ensures [sideEffect] is running with the given [key]. * @@ -202,6 +191,48 @@ fun RenderContext.runningWork } } +/** + * Ensures [worker] is running. When the [Worker] emits an output, [handler] is called + * to determine the [WorkflowAction] to take. When the worker finishes, nothing happens (although + * another render pass may be triggered). + * + * @param key An optional string key that is used to distinguish between identical [Worker]s. + */ +@OptIn(ExperimentalStdlibApi::class) +/* ktlint-disable parameter-list-wrapping */ +inline fun , PropsT, StateT, OutputT> + RenderContext.runningWorker( + worker: W, + key: String = "", + noinline handler: (T) -> WorkflowAction +) { +/* ktlint-enable parameter-list-wrapping */ + runningWorker(worker, typeOf(), key, handler) +} + +/** + * Ensures [worker] is running. When the [Worker] emits an output, [handler] is called + * to determine the [WorkflowAction] to take. When the worker finishes, nothing happens (although + * another render pass may be triggered). + * + * @param workerType `typeOf()` + * @param key An optional string key that is used to distinguish between identical [Worker]s. + */ +@OptIn(ExperimentalStdlibApi::class) +@PublishedApi +/* ktlint-disable parameter-list-wrapping */ +internal fun + RenderContext.runningWorker( + worker: Worker, + workerType: KType, + key: String = "", + handler: (T) -> WorkflowAction +) { +/* ktlint-enable parameter-list-wrapping */ + val workerWorkflow = WorkerWorkflow(workerType, key) + renderChild(workerWorkflow, props = worker, key = key, handler = handler) +} + /** * Alternative to [RenderContext.actionSink] that allows externally defined * event types to be mapped to anonymous [WorkflowAction]s. @@ -223,8 +254,8 @@ fun RenderContext.mak "Use runningWorker", ReplaceWith("runningWorker(worker, key, handler)", "com.squareup.workflow.runningWorker") ) -fun RenderContext.onWorkerOutput( +inline fun RenderContext.onWorkerOutput( worker: Worker, key: String = "", - handler: (T) -> WorkflowAction + noinline handler: (T) -> WorkflowAction ) = runningWorker(worker, key, handler) diff --git a/workflow-core/src/main/java/com/squareup/workflow/Worker.kt b/workflow-core/src/main/java/com/squareup/workflow/Worker.kt index 67c26adebd..6530af0cd9 100644 --- a/workflow-core/src/main/java/com/squareup/workflow/Worker.kt +++ b/workflow-core/src/main/java/com/squareup/workflow/Worker.kt @@ -52,6 +52,11 @@ import kotlin.reflect.typeOf * See the documentation on [doesSameWorkAs] for more details on how and when workers are compared * and the worker lifecycle. * + * Implementations of this interface that are themselves parameterized on a type should override + * [outputType] to return the [KType] of their type parameter. This allows the runtime to compare + * workers by the output type as well as the concrete type. If [outputType] returns null, all + * workers of the same concrete type will be considered equivalent. + * * ## Example: Network request * * Let's say you have a network service with an API that returns a number, and you want to @@ -117,6 +122,14 @@ import kotlin.reflect.typeOf */ interface Worker { + /** + * Should be overridden in subclasses that have their own type parameters, to allow the runtime to + * make use of the value of those type parameters to compare workers. Two workers of the same + * concrete [Worker] class will be considered equivalent if and only if their [outputType]s are + * also equivalent. If this property returns null, the worker will be treated as a `Worker<*>`. + */ + val outputType: KType? get() = null + /** * Returns a [Flow] to execute the work represented by this worker. * @@ -165,8 +178,7 @@ interface Worker { * that performs a network request might check that two workers are requests to the same endpoint * and have the same request data. * - * Most implementations of this method will check for concrete type equality, and then match - * on constructor parameters. + * Most implementations of this method should compare constructor parameters. * * E.g: * @@ -179,7 +191,7 @@ interface Worker { * } * ``` */ - fun doesSameWorkAs(otherWorker: Worker<*>): Boolean = otherWorker::class == this::class + fun doesSameWorkAs(otherWorker: Worker<*>): Boolean = true companion object { @@ -384,23 +396,18 @@ fun Worker.transform( /** * A generic [Worker] implementation that defines equivalent workers as those having equivalent - * [type]s. This is used by all the [Worker] builder functions. + * [outputType]s. This is used by all the [Worker] builder functions. */ @PublishedApi internal class TypedWorker( - private val type: KType, + override val outputType: KType, private val work: Flow ) : Worker { - override fun run(): Flow = work - - override fun doesSameWorkAs(otherWorker: Worker<*>): Boolean = - otherWorker is TypedWorker && otherWorker.type == type - - override fun toString(): String = "TypedWorker($type)" + override fun toString(): String = "TypedWorker($outputType)" } -private class TimerWorker( +private data class TimerWorker( private val delayMs: Long, private val key: String ) : Worker { @@ -412,17 +419,14 @@ private class TimerWorker( override fun doesSameWorkAs(otherWorker: Worker<*>): Boolean = otherWorker is TimerWorker && otherWorker.key == key - - override fun toString(): String = "TimerWorker(delayMs=$delayMs)" } private object FinishedWorker : Worker { override fun run(): Flow = emptyFlow() - override fun doesSameWorkAs(otherWorker: Worker<*>): Boolean = otherWorker === FinishedWorker override fun toString(): String = "FinishedWorker" } -private class WorkerWrapper( +private data class WorkerWrapper( private val wrapped: Worker, private val flow: Flow ) : Worker { diff --git a/workflow-core/src/main/java/com/squareup/workflow/WorkerWorkflow.kt b/workflow-core/src/main/java/com/squareup/workflow/WorkerWorkflow.kt new file mode 100644 index 0000000000..199c779da3 --- /dev/null +++ b/workflow-core/src/main/java/com/squareup/workflow/WorkerWorkflow.kt @@ -0,0 +1,103 @@ +/* + * Copyright 2020 Square Inc. + * + * 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. + */ +@file:JvmMultifileClass +@file:JvmName("Workflows") + +package com.squareup.workflow + +import kotlinx.coroutines.CoroutineName +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.withContext +import kotlin.reflect.KType + +/** + * TODO write documentation + * + * @param workerType The [KType] representing the particular type of `Worker`. + * @param key The key used to render this workflow, as passed to [RenderContext.runningWorker]. + */ +@OptIn(ExperimentalWorkflowApi::class) +internal class WorkerWorkflow( + private val workerType: KType, + private val key: String +) : StatefulWorkflow, Int, OutputT, Unit>(), + ImpostorWorkflow { + + override val realIdentifier: WorkflowIdentifier = unsnapshottableIdentifier(workerType) + override fun describeRealIdentifier(): String? = "worker $workerType" + + override fun initialState( + props: Worker, + snapshot: Snapshot? + ): Int = 0 + + override fun onPropsChanged( + old: Worker, + new: Worker, + state: Int + ): Int = if (!old.doesSameWorkAs(new)) state + 1 else state + + override fun render( + props: Worker, + state: Int, + context: RenderContext, Int, OutputT> + ) { + context.runningSideEffect(state.toString()) { + runWorker(props, key, context.actionSink) + } + } + + override fun snapshotState(state: Int): Snapshot = Snapshot.EMPTY +} + +/** + * TODO write kdoc + * + * Visible for testing. + */ +@OptIn(ExperimentalWorkflowApi::class) +internal suspend fun runWorker( + worker: Worker, + renderKey: String, + actionSink: Sink, Int, OutputT>> +) { + withContext(CoroutineName(worker.debugName(renderKey))) { + worker.runWithNullCheck() + .collectToSink(actionSink) { output -> + action { setOutput(output) } + } + } +} + +/** + * In unit tests, if you use a mocking library to create a Worker, the run method will return null + * even though the return type is non-nullable in Kotlin. Kotlin helps out with this by throwing an + * NPE before before any kotlin code gets the null, but the NPE that it throws includes an almost + * completely useless stacktrace and no other details. + * + * This method does an explicit null check and throws an exception with a more helpful message. + * + * See [#842](https://github.com/square/workflow/issues/842). + */ +@Suppress("USELESS_ELVIS") +private fun Worker.runWithNullCheck(): Flow = + run() ?: throw NullPointerException( + "Worker $this returned a null Flow. " + + "If this is a test mock, make sure you mock the run() method!" + ) + +private fun Worker<*>.debugName(key: String) = + toString().let { if (key.isBlank()) it else "$it:$key" } diff --git a/workflow-core/src/main/java/com/squareup/workflow/WorkflowIdentifier.kt b/workflow-core/src/main/java/com/squareup/workflow/WorkflowIdentifier.kt index 61e99a7830..1ebc9191e8 100644 --- a/workflow-core/src/main/java/com/squareup/workflow/WorkflowIdentifier.kt +++ b/workflow-core/src/main/java/com/squareup/workflow/WorkflowIdentifier.kt @@ -212,12 +212,14 @@ fun unsnapshottableIdentifier(type: KType): WorkflowIdentifier = WorkflowIdentif val KClass>.workflowIdentifier: WorkflowIdentifier get() { val workflowClass = this@workflowIdentifier - require( - !ImpostorWorkflow::class.java.isAssignableFrom(workflowClass.java) - ) { "Cannot create WorkflowIdentifier from a KClass of ImpostorWorkflow: ${workflowClass.qualifiedName}" } + require(!ImpostorWorkflow::class.java.isAssignableFrom(workflowClass.java)) { + "Cannot create WorkflowIdentifier from a KClass of ImpostorWorkflow: " + + workflowClass.qualifiedName.toString() + } return WorkflowIdentifier(type = workflowClass) } +// TODO delete this, it's not used /** * Creates a [WorkflowIdentifier] that identifies the [ImpostorWorkflow] this [KClass] represents. * diff --git a/workflow-runtime/src/test/java/com/squareup/workflow/internal/NullFlowWorker.java b/workflow-core/src/test/java/com/squareup/workflow/NullFlowWorker.java similarity index 81% rename from workflow-runtime/src/test/java/com/squareup/workflow/internal/NullFlowWorker.java rename to workflow-core/src/test/java/com/squareup/workflow/NullFlowWorker.java index 7aab1dbc28..9a54194cf8 100644 --- a/workflow-runtime/src/test/java/com/squareup/workflow/internal/NullFlowWorker.java +++ b/workflow-core/src/test/java/com/squareup/workflow/NullFlowWorker.java @@ -1,8 +1,9 @@ -package com.squareup.workflow.internal; +package com.squareup.workflow; -import com.squareup.workflow.Worker; +import kotlin.reflect.KType; import kotlinx.coroutines.flow.Flow; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; /** * Worker that incorrectly returns null from {@link #run}, to simulate the default behavior of some @@ -11,6 +12,11 @@ * See #842. */ class NullFlowWorker implements Worker { + + @Nullable @Override public KType getOutputType() { + return null; + } + @NotNull @Override public Flow run() { //noinspection ConstantConditions return null; diff --git a/workflow-core/src/test/java/com/squareup/workflow/SinkTest.kt b/workflow-core/src/test/java/com/squareup/workflow/SinkTest.kt index 68fffae7f3..ecb8e7dea0 100644 --- a/workflow-core/src/test/java/com/squareup/workflow/SinkTest.kt +++ b/workflow-core/src/test/java/com/squareup/workflow/SinkTest.kt @@ -15,10 +15,14 @@ */ package com.squareup.workflow +import kotlinx.coroutines.CoroutineStart.UNDISPATCHED import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.consumeAsFlow import kotlinx.coroutines.launch import kotlinx.coroutines.test.runBlockingTest +import java.util.concurrent.atomic.AtomicInteger import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -35,7 +39,7 @@ class SinkTest { private val sink = RecordingSink() - @Test fun `collectToActionSink sends action`() { + @Test fun `collectToSink sends action`() { runBlockingTest { val flow = MutableStateFlow(1) val collector = launch { @@ -71,6 +75,59 @@ class SinkTest { } } + @Test fun `collectToSink propagates backpressure`() { + val channel = Channel() + val flow = channel.consumeAsFlow() + // Used to assert ordering. + val counter = AtomicInteger(0) + val sentActions = mutableListOf>() + val sink = object : Sink> { + override fun send(value: WorkflowAction) { + sentActions += value + } + } + + runBlockingTest { + val collectJob = launch { + flow.collectToSink(sink) { action { setOutput(it) } } + } + + val sendJob = launch(start = UNDISPATCHED) { + assertEquals(0, counter.getAndIncrement()) + channel.send("a") + assertEquals(1, counter.getAndIncrement()) + channel.send("b") + assertEquals(4, counter.getAndIncrement()) + channel.close() + assertEquals(5, counter.getAndIncrement()) + } + advanceUntilIdle() + assertEquals(2, counter.getAndIncrement()) + + sentActions.removeFirst() + .also { + advanceUntilIdle() + // Sender won't resume until we've _applied_ the action. + assertEquals(3, counter.getAndIncrement()) + } + .applyTo(Unit, Unit) + .let { (_, output) -> + assertEquals(6, counter.getAndIncrement()) + assertEquals("a", output?.value) + } + + sentActions.removeFirst() + .applyTo(Unit, Unit) + .let { (_, output) -> + assertEquals(7, counter.getAndIncrement()) + assertEquals("b", output?.value) + } + + collectJob.cancel() + sendJob.cancel() + } + } + @Test fun `sendAndAwaitApplication applies action`() { var applications = 0 val action = action { diff --git a/workflow-core/src/test/java/com/squareup/workflow/WorkerTest.kt b/workflow-core/src/test/java/com/squareup/workflow/WorkerTest.kt index 3e1e24840b..8de4d2ab53 100644 --- a/workflow-core/src/test/java/com/squareup/workflow/WorkerTest.kt +++ b/workflow-core/src/test/java/com/squareup/workflow/WorkerTest.kt @@ -1,26 +1,26 @@ +/* + * Copyright 2020 Square Inc. + * + * 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 com.squareup.workflow -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.emptyFlow import kotlin.test.Test import kotlin.test.assertFalse import kotlin.test.assertTrue class WorkerTest { - @Test fun `default Worker#doesSameWorkAs implementation compares by concrete type`() { - class Worker1 : Worker { - override fun run(): Flow = emptyFlow() - } - - class Worker2 : Worker { - override fun run(): Flow = emptyFlow() - } - - assertTrue(Worker1().doesSameWorkAs(Worker1())) - assertFalse(Worker1().doesSameWorkAs(Worker2())) - } - @Test fun `createSideEffect workers are equivalent`() { val worker1 = Worker.createSideEffect {} val worker2 = Worker.createSideEffect {} diff --git a/workflow-core/src/test/java/com/squareup/workflow/WorkerWorkflowTest.kt b/workflow-core/src/test/java/com/squareup/workflow/WorkerWorkflowTest.kt new file mode 100644 index 0000000000..1afdf21143 --- /dev/null +++ b/workflow-core/src/test/java/com/squareup/workflow/WorkerWorkflowTest.kt @@ -0,0 +1,83 @@ +/* + * Copyright 2020 Square Inc. + * + * 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 com.squareup.workflow + +import kotlinx.coroutines.CoroutineName +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.runBlocking +import kotlin.coroutines.coroutineContext +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith + +class WorkerWorkflowTest { + + /** + * This should be impossible, since the return type is non-nullable. However it is very easy to + * accidentally create a mock using libraries like Mockito in unit tests that return null Flows. + */ + @Test fun `runWorker throws when flow is null`() { + val nullFlowWorker = NullFlowWorker() + + val error = runBlocking { + assertFailsWith { + runWorker(nullFlowWorker, "", NoopSink) + } + } + + assertEquals( + "Worker NullFlowWorker.toString returned a null Flow. " + + "If this is a test mock, make sure you mock the run() method!", + error.message + ) + } + + @Test fun `runWorker coroutine is named without key`() { + val worker = CoroutineNameWorker() + runBlocking { + runWorker(worker, renderKey = "", actionSink = NoopSink) + } + + assertEquals("CoroutineNameWorker.toString", worker.recordedName) + } + + @Test fun `runWorker coroutine is named with key`() { + val worker = CoroutineNameWorker() + runBlocking { + runWorker(worker, renderKey = "foo", actionSink = NoopSink) + } + + assertEquals("CoroutineNameWorker.toString:foo", worker.recordedName) + } + + private object NoopSink : Sink { + override fun send(value: Any?) { + // Noop + } + } + + private class CoroutineNameWorker : Worker { + var recordedName: String? = null + private set + + override fun run(): Flow = flow { + recordedName = (coroutineContext[CoroutineName] as CoroutineName).name + } + + override fun toString(): String = "CoroutineNameWorker.toString" + } +} diff --git a/workflow-runtime/api/workflow-runtime.api b/workflow-runtime/api/workflow-runtime.api index 9a83cdbe5c..f71077bf03 100644 --- a/workflow-runtime/api/workflow-runtime.api +++ b/workflow-runtime/api/workflow-runtime.api @@ -8,8 +8,8 @@ public final class com/squareup/workflow/NoopWorkflowInterceptor : com/squareup/ } public final class com/squareup/workflow/RenderWorkflowKt { - public static final fun renderWorkflowIn (Lcom/squareup/workflow/Workflow;Lkotlinx/coroutines/CoroutineScope;Lkotlinx/coroutines/flow/StateFlow;Lcom/squareup/workflow/TreeSnapshot;Ljava/util/List;Lkotlinx/coroutines/CoroutineDispatcher;Lkotlin/jvm/functions/Function2;)Lkotlinx/coroutines/flow/StateFlow; - public static synthetic fun renderWorkflowIn$default (Lcom/squareup/workflow/Workflow;Lkotlinx/coroutines/CoroutineScope;Lkotlinx/coroutines/flow/StateFlow;Lcom/squareup/workflow/TreeSnapshot;Ljava/util/List;Lkotlinx/coroutines/CoroutineDispatcher;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)Lkotlinx/coroutines/flow/StateFlow; + public static final fun renderWorkflowIn (Lcom/squareup/workflow/Workflow;Lkotlinx/coroutines/CoroutineScope;Lkotlinx/coroutines/flow/StateFlow;Lcom/squareup/workflow/TreeSnapshot;Ljava/util/List;Lkotlin/jvm/functions/Function2;)Lkotlinx/coroutines/flow/StateFlow; + public static synthetic fun renderWorkflowIn$default (Lcom/squareup/workflow/Workflow;Lkotlinx/coroutines/CoroutineScope;Lkotlinx/coroutines/flow/StateFlow;Lcom/squareup/workflow/TreeSnapshot;Ljava/util/List;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)Lkotlinx/coroutines/flow/StateFlow; } public final class com/squareup/workflow/RenderingAndSnapshot { diff --git a/workflow-runtime/src/main/java/com/squareup/workflow/RenderWorkflow.kt b/workflow-runtime/src/main/java/com/squareup/workflow/RenderWorkflow.kt index 179dae4c51..2ee2e0ab50 100644 --- a/workflow-runtime/src/main/java/com/squareup/workflow/RenderWorkflow.kt +++ b/workflow-runtime/src/main/java/com/squareup/workflow/RenderWorkflow.kt @@ -18,7 +18,6 @@ package com.squareup.workflow import com.squareup.workflow.internal.WorkflowRunner import com.squareup.workflow.internal.chained import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.CoroutineStart.ATOMIC import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -28,7 +27,6 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.isActive import kotlinx.coroutines.launch -import kotlin.coroutines.EmptyCoroutineContext /** * Launches the [workflow] in a new coroutine in [scope] and returns a [StateFlow] of its @@ -123,16 +121,11 @@ fun renderWorkflowIn( props: StateFlow, initialSnapshot: TreeSnapshot = TreeSnapshot.NONE, interceptors: List = emptyList(), - workerDispatcher: CoroutineDispatcher? = null, onOutput: suspend (OutputT) -> Unit ): StateFlow> { val chainedInterceptor = interceptors.chained() - val runner = - WorkflowRunner( - scope, workflow, props, initialSnapshot, chainedInterceptor, - workerDispatcher ?: EmptyCoroutineContext - ) + val runner = WorkflowRunner(scope, workflow, props, initialSnapshot, chainedInterceptor) // Rendering is synchronous, so we can run the first render pass before launching the runtime // coroutine to calculate the initial rendering. @@ -154,15 +147,15 @@ fun renderWorkflowIn( // Launch atomically so the finally block is run even if the scope is cancelled before the // coroutine starts executing. scope.launch(start = ATOMIC) { - while (isActive) { - // It might look weird to start by consuming the output before getting the rendering below, - // but remember the first render pass already occurred above, before this coroutine was even - // launched. - runner.nextOutput() - ?.let { onOutput(it.value) } + while (isActive) { + // It might look weird to start by consuming the output before getting the rendering below, + // but remember the first render pass already occurred above, before this coroutine was even + // launched. + runner.nextOutput() + ?.let { onOutput(it.value) } - renderingsAndSnapshots.value = runner.nextRendering() - } + renderingsAndSnapshots.value = runner.nextRendering() + } } return renderingsAndSnapshots diff --git a/workflow-runtime/src/main/java/com/squareup/workflow/internal/RealRenderContext.kt b/workflow-runtime/src/main/java/com/squareup/workflow/internal/RealRenderContext.kt index 12085baa3d..f0a2e752eb 100644 --- a/workflow-runtime/src/main/java/com/squareup/workflow/internal/RealRenderContext.kt +++ b/workflow-runtime/src/main/java/com/squareup/workflow/internal/RealRenderContext.kt @@ -19,14 +19,12 @@ package com.squareup.workflow.internal import com.squareup.workflow.RenderContext import com.squareup.workflow.Sink -import com.squareup.workflow.Worker import com.squareup.workflow.Workflow import com.squareup.workflow.WorkflowAction import kotlinx.coroutines.channels.SendChannel internal class RealRenderContext( private val renderer: Renderer, - private val workerRunner: WorkerRunner, private val sideEffectRunner: SideEffectRunner, private val eventActionsChannel: SendChannel> ) : RenderContext, Sink> { @@ -40,14 +38,6 @@ internal class RealRenderContext( ): ChildRenderingT } - interface WorkerRunner { - fun runningWorker( - worker: Worker, - key: String, - handler: (T) -> WorkflowAction - ) - } - interface SideEffectRunner { fun runningSideEffect( key: String, @@ -85,15 +75,6 @@ internal class RealRenderContext( return renderer.render(child, props, key, handler) } - override fun runningWorker( - worker: Worker, - key: String, - handler: (T) -> WorkflowAction - ) { - checkNotFrozen() - workerRunner.runningWorker(worker, key, handler) - } - override fun runningSideEffect( key: String, sideEffect: suspend () -> Unit diff --git a/workflow-runtime/src/main/java/com/squareup/workflow/internal/SubtreeManager.kt b/workflow-runtime/src/main/java/com/squareup/workflow/internal/SubtreeManager.kt index 1bd4a269f2..5c40328d89 100644 --- a/workflow-runtime/src/main/java/com/squareup/workflow/internal/SubtreeManager.kt +++ b/workflow-runtime/src/main/java/com/squareup/workflow/internal/SubtreeManager.kt @@ -25,7 +25,6 @@ import com.squareup.workflow.WorkflowInterceptor.WorkflowSession import com.squareup.workflow.WorkflowOutput import kotlinx.coroutines.selects.SelectBuilder import kotlin.coroutines.CoroutineContext -import kotlin.coroutines.EmptyCoroutineContext /** * Responsible for tracking child workflows, starting them and tearing them down when necessary. @@ -102,8 +101,7 @@ internal class SubtreeManager( private val emitActionToParent: (WorkflowAction) -> Any?, private val workflowSession: WorkflowSession? = null, private val interceptor: WorkflowInterceptor = NoopWorkflowInterceptor, - private val idCounter: IdCounter? = null, - private val workerContext: CoroutineContext = EmptyCoroutineContext + private val idCounter: IdCounter? = null ) : RealRenderContext.Renderer { /** @@ -199,8 +197,7 @@ internal class SubtreeManager( ::acceptChildOutput, workflowSession, interceptor, - idCounter = idCounter, - workerContext = workerContext + idCounter = idCounter ) return WorkflowChildNode(child, handler, workflowNode) .also { node = it } diff --git a/workflow-runtime/src/main/java/com/squareup/workflow/internal/WorkerChildNode.kt b/workflow-runtime/src/main/java/com/squareup/workflow/internal/WorkerChildNode.kt deleted file mode 100644 index 7ceb36655b..0000000000 --- a/workflow-runtime/src/main/java/com/squareup/workflow/internal/WorkerChildNode.kt +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Copyright 2020 Square Inc. - * - * 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 com.squareup.workflow.internal - -import com.squareup.workflow.Worker -import com.squareup.workflow.WorkflowAction -import com.squareup.workflow.internal.InlineLinkedList.InlineListNode -import kotlinx.coroutines.channels.ReceiveChannel - -/** - * Holds the channel representing the outputs of a worker, as well as a tombstone flag that is - * true after the worker has finished and we've reported that fact to the workflow. This is to - * prevent the workflow from entering an infinite loop of getting `Finished` events if it - * continues to listen to the worker after it finishes. - * - * @param worker The first instance of the worker that was used to start this worker. This instance - * will be the receiver for the [Worker.doesSameWorkAs] call in future render passes. - * @param channel A [ReceiveChannel] that represents the subscription to the worker's flow. - * @param tombstone Mutable flag that starts as false and is set to true once the channel has been - * closed (i.e. the flow completes successfully). [WorkerChildNode]s continue to be tracked even - * after the worker finishes so that they aren't immediately restarted on the next render pass. This - * flag indicates that the channel should not be polled on the next tick. - */ -internal class WorkerChildNode( - val worker: Worker, - val key: String, - val channel: ReceiveChannel>, - var tombstone: Boolean = false, - private var handler: (T) -> WorkflowAction -) : InlineListNode> { - - override var nextListNode: WorkerChildNode<*, *, *, *>? = null - - /** - * Updates the handler function that will be invoked by [acceptUpdate]. - */ - fun setHandler(newHandler: (T2) -> WorkflowAction) { - @Suppress("UNCHECKED_CAST") - handler = newHandler as (T) -> WorkflowAction - } - - @Suppress("UNCHECKED_CAST") - fun acceptUpdate(value: Any?): WorkflowAction = - handler(value as T) - - /** - * Returns true if this worker does the same work as [otherWorker] and has the same key. - */ - fun matches( - otherWorker: Worker<*>, - key: String - ): Boolean = worker.doesSameWorkAs(otherWorker) && this.key == key -} diff --git a/workflow-runtime/src/main/java/com/squareup/workflow/internal/Workers.kt b/workflow-runtime/src/main/java/com/squareup/workflow/internal/Workers.kt deleted file mode 100644 index 1e08ef54f1..0000000000 --- a/workflow-runtime/src/main/java/com/squareup/workflow/internal/Workers.kt +++ /dev/null @@ -1,115 +0,0 @@ -/* - * Copyright 2019 Square Inc. - * - * 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 com.squareup.workflow.internal - -import com.squareup.workflow.Worker -import kotlinx.coroutines.CoroutineName -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers.Unconfined -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.FlowPreview -import kotlinx.coroutines.channels.Channel.Factory.RENDEZVOUS -import kotlinx.coroutines.channels.ReceiveChannel -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.buffer -import kotlinx.coroutines.flow.catch -import kotlinx.coroutines.flow.collect -import kotlinx.coroutines.flow.flow -import kotlinx.coroutines.flow.produceIn -import kotlinx.coroutines.plus -import kotlin.coroutines.CoroutineContext - -/** - * Launches a new coroutine that is a child of this node's scope, and calls - * [com.squareup.workflow.Worker.run] from that coroutine. Returns a [ReceiveChannel] that - * will emit everything from the worker. The channel will be closed when the flow completes. - */ -@OptIn(FlowPreview::class, ExperimentalCoroutinesApi::class) -internal fun CoroutineScope.launchWorker( - worker: Worker, - key: String, - workerContext: CoroutineContext -): ReceiveChannel> = worker.runWithNullCheck() - .transformToValueOrDone() - .catch { e -> - // Workers that failed (as opposed to just cancelled) should have their failure reason - // re-thrown from the workflow runtime. If we don't unwrap the cause here, they'll just - // cause the runtime to cancel. - val cancellationCause = e.unwrapCancellationCause() - throw cancellationCause ?: e - } - // produceIn implicitly creates a buffer (it uses a Channel to bridge between contexts). This - // operator is required to override the default buffer size. - .buffer(RENDEZVOUS) - .produceIn(createWorkerScope(worker, key, workerContext)) - -/** - * In unit tests, if you use a mocking library to create a Worker, the run method will return null - * even though the return type is non-nullable in Kotlin. Kotlin helps out with this by throwing an - * NPE before before any kotlin code gets the null, but the NPE that it throws includes an almost - * completely useless stacktrace and no other details. - * - * This method does an explicit null check and throws an exception with a more helpful message. - * - * See [#842](https://github.com/square/workflow/issues/842). - */ -@Suppress("USELESS_ELVIS") -private fun Worker.runWithNullCheck(): Flow = - run() ?: throw NullPointerException( - "Worker $this returned a null Flow. " + - "If this is a test mock, make sure you mock the run() method!" - ) - -/** - * Pretend we can use ReceiveChannel.onReceiveOrClosed. - * - * See https://github.com/Kotlin/kotlinx.coroutines/issues/1584 and - * https://github.com/square/workflow/issues/626. - */ -internal class ValueOrDone private constructor(private val _value: Any?) { - - val isDone: Boolean get() = this === Done - - @Suppress("UNCHECKED_CAST") - val value: T - get() { - check(!isDone) - return _value as T - } - - companion object { - private val Done = ValueOrDone(null) - - fun value(value: T): ValueOrDone = ValueOrDone(value) - fun done(): ValueOrDone = Done - } -} - -private fun Flow.transformToValueOrDone(): Flow> = flow { - collect { - emit(ValueOrDone.value(it)) - } - emit(ValueOrDone.done()) -} - -private fun CoroutineScope.createWorkerScope( - worker: Worker<*>, - key: String, - workerContext: CoroutineContext -): CoroutineScope = this + CoroutineName(worker.debugName(key)) + Unconfined + workerContext - -private fun Worker<*>.debugName(key: String) = - toString().let { if (key.isBlank()) it else "$it:$key" } diff --git a/workflow-runtime/src/main/java/com/squareup/workflow/internal/WorkflowNode.kt b/workflow-runtime/src/main/java/com/squareup/workflow/internal/WorkflowNode.kt index cb277d103a..d7c13e1c89 100644 --- a/workflow-runtime/src/main/java/com/squareup/workflow/internal/WorkflowNode.kt +++ b/workflow-runtime/src/main/java/com/squareup/workflow/internal/WorkflowNode.kt @@ -19,7 +19,6 @@ import com.squareup.workflow.ExperimentalWorkflowApi import com.squareup.workflow.NoopWorkflowInterceptor import com.squareup.workflow.StatefulWorkflow import com.squareup.workflow.TreeSnapshot -import com.squareup.workflow.Worker import com.squareup.workflow.Workflow import com.squareup.workflow.WorkflowAction import com.squareup.workflow.WorkflowIdentifier @@ -29,7 +28,6 @@ import com.squareup.workflow.WorkflowOutput import com.squareup.workflow.applyTo import com.squareup.workflow.intercept import com.squareup.workflow.internal.RealRenderContext.SideEffectRunner -import com.squareup.workflow.internal.RealRenderContext.WorkerRunner import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.CoroutineScope @@ -42,7 +40,6 @@ import kotlinx.coroutines.launch import kotlinx.coroutines.plus import kotlinx.coroutines.selects.SelectBuilder import kotlin.coroutines.CoroutineContext -import kotlin.coroutines.EmptyCoroutineContext /** * A node in a state machine tree. Manages the actual state for a given [Workflow]. @@ -64,9 +61,8 @@ internal class WorkflowNode( private val emitOutputToParent: (OutputT) -> Any? = { WorkflowOutput(it) }, override val parent: WorkflowSession? = null, private val interceptor: WorkflowInterceptor = NoopWorkflowInterceptor, - idCounter: IdCounter? = null, - private val workerContext: CoroutineContext = EmptyCoroutineContext -) : CoroutineScope, WorkerRunner, SideEffectRunner, WorkflowSession { + idCounter: IdCounter? = null +) : CoroutineScope, SideEffectRunner, WorkflowSession { /** * Context that has a job that will live as long as this node. @@ -85,10 +81,8 @@ internal class WorkflowNode( emitActionToParent = ::applyAction, workflowSession = this, interceptor = interceptor, - idCounter = idCounter, - workerContext = workerContext + idCounter = idCounter ) - private val workers = ActiveStagingList>() private val sideEffects = ActiveStagingList() private var lastProps: PropsT = initialProps private val eventActionsChannel = @@ -141,26 +135,6 @@ internal class WorkflowNode( ) } - override fun runningWorker( - worker: Worker, - key: String, - handler: (T) -> WorkflowAction - ) { - // Prevent duplicate workers with the same key. - workers.forEachStaging { - require(!(it.matches(worker, key))) { - "Expected keys to be unique for $worker: key=$key" - } - } - - // Start tracking this case so we can be ready to render it. - val stagedWorker = workers.retainOrCreate( - predicate = { it.matches(worker, key) }, - create = { createWorkerNode(worker, key, handler) } - ) - stagedWorker.setHandler(handler) - } - override fun runningSideEffect( key: String, sideEffect: suspend () -> Unit @@ -189,27 +163,6 @@ internal class WorkflowNode( // Listen for any child workflow updates. subtreeManager.tickChildren(selector) - // Listen for any subscription updates. - workers.forEachActive { child -> - // Skip children that have finished but are still being run by the workflow. - if (child.tombstone) return@forEachActive - - with(selector) { - child.channel.onReceive { valueOrDone -> - if (valueOrDone.isDone) { - // Set the tombstone flag so we don't continue to listen to the subscription. - child.tombstone = true - // Nothing to do on close other than update the session, so don't emit any output. - return@onReceive null - } else { - val update = child.acceptUpdate(valueOrDone.value) - @Suppress("UNCHECKED_CAST") - return@onReceive applyAction(update as WorkflowAction) - } - } - } - } - // Listen for any events. with(selector) { eventActionsChannel.onReceive { action -> @@ -244,7 +197,6 @@ internal class WorkflowNode( val context = RealRenderContext( renderer = subtreeManager, - workerRunner = this, sideEffectRunner = this, eventActionsChannel = eventActionsChannel ) @@ -254,7 +206,6 @@ internal class WorkflowNode( // Tear down workflows and workers that are obsolete. subtreeManager.commitRenderedChildren() - workers.commitStaging { it.channel.cancel() } // Side effect jobs are launched lazily, since they can send actions to the sink, and can only // be started after context is frozen. sideEffects.forEachStaging { it.job.start() } @@ -286,15 +237,6 @@ internal class WorkflowNode( return tickResult?.let { emitOutputToParent(it.value) } as T? } - private fun createWorkerNode( - worker: Worker, - key: String, - handler: (T) -> WorkflowAction - ): WorkerChildNode { - val workerChannel = launchWorker(worker, key, workerContext) - return WorkerChildNode(worker, key, workerChannel, handler = handler) - } - private fun createSideEffectNode( key: String, sideEffect: suspend () -> Unit diff --git a/workflow-runtime/src/main/java/com/squareup/workflow/internal/WorkflowRunner.kt b/workflow-runtime/src/main/java/com/squareup/workflow/internal/WorkflowRunner.kt index 4d406496ff..6c6fb10982 100644 --- a/workflow-runtime/src/main/java/com/squareup/workflow/internal/WorkflowRunner.kt +++ b/workflow-runtime/src/main/java/com/squareup/workflow/internal/WorkflowRunner.kt @@ -29,8 +29,6 @@ import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.dropWhile import kotlinx.coroutines.flow.produceIn import kotlinx.coroutines.selects.select -import kotlin.coroutines.CoroutineContext -import kotlin.coroutines.EmptyCoroutineContext @OptIn(ExperimentalCoroutinesApi::class, ExperimentalWorkflowApi::class) internal class WorkflowRunner( @@ -38,8 +36,7 @@ internal class WorkflowRunner( protoWorkflow: Workflow, props: StateFlow, snapshot: TreeSnapshot, - interceptor: WorkflowInterceptor, - workerContext: CoroutineContext = EmptyCoroutineContext + interceptor: WorkflowInterceptor ) { private val workflow = protoWorkflow.asStatefulWorkflow() private val idCounter = IdCounter() @@ -65,7 +62,6 @@ internal class WorkflowRunner( initialProps = currentProps, snapshot = snapshot, baseContext = scope.coroutineContext, - workerContext = workerContext, interceptor = interceptor, idCounter = idCounter ) diff --git a/workflow-runtime/src/test/java/com/squareup/workflow/RenderWorkflowInTest.kt b/workflow-runtime/src/test/java/com/squareup/workflow/RenderWorkflowInTest.kt index d4c6fbb3ae..5c08b2f344 100644 --- a/workflow-runtime/src/test/java/com/squareup/workflow/RenderWorkflowInTest.kt +++ b/workflow-runtime/src/test/java/com/squareup/workflow/RenderWorkflowInTest.kt @@ -32,7 +32,6 @@ import kotlinx.coroutines.isActive import kotlinx.coroutines.launch import kotlinx.coroutines.suspendCancellableCoroutine import kotlinx.coroutines.test.TestCoroutineScope -import org.junit.Ignore import org.junit.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith @@ -66,27 +65,39 @@ class RenderWorkflowInTest { assertEquals("props: foo", renderings.value.rendering) } - @Ignore("https://github.com/square/workflow/issues/1197") - @Test fun `workers from initial rendering are never started when scope cancelled before start`() { - var workerWasRan = false - var cancellationException: Throwable? = null + @Test + fun `side effects from initial rendering in root workflow are never started when scope cancelled before start`() { + var sideEffectWasRan = false val workflow = Workflow.stateless { - runningWorker(Worker.createSideEffect { - workerWasRan = true - suspendCancellableCoroutine { continuation -> - continuation.invokeOnCancellation { cause -> - cancellationException = cause - } - } - }) + runningSideEffect("test") { + sideEffectWasRan = true + } } scope.cancel() renderWorkflowIn(workflow, scope, MutableStateFlow(Unit)) {} scope.advanceUntilIdle() - assertFalse(workerWasRan) - assertNull(cancellationException) + assertFalse(sideEffectWasRan) + } + + @Test + fun `side effects from initial rendering in non-root workflow are never started when scope cancelled before start`() { + var sideEffectWasRan = false + val childWorkflow = Workflow.stateless { + runningSideEffect("test") { + sideEffectWasRan = true + } + } + val workflow = Workflow.stateless { + renderChild(childWorkflow) + } + + scope.cancel() + renderWorkflowIn(workflow, scope, MutableStateFlow(Unit)) {} + + scope.advanceUntilIdle() + assertFalse(sideEffectWasRan) } @Test fun `new renderings are emitted on update`() { @@ -197,34 +208,71 @@ class RenderWorkflowInTest { assertTrue(scope.isActive) } - /** - * The initial render pass can start workers, but if it fails afterwards, those workers should - * still be cancelled. - */ - @Test fun `exception from initial render cancels runtime`() { - var workerWasRan = false - var cancellationException: Throwable? = null + @Test + fun `side effects from initial rendering in root workflow are never started when initial render of root workflow fails`() { + var sideEffectWasRan = false val workflow = Workflow.stateless { - runningWorker(Worker.createSideEffect { - workerWasRan = true + runningSideEffect("test") { + sideEffectWasRan = true + } + throw ExpectedException() + } + + assertFailsWith { + renderWorkflowIn(workflow, scope, MutableStateFlow(Unit)) {} + } + scope.advanceUntilIdle() + assertFalse(sideEffectWasRan) + } + + @Test + fun `side effects from initial rendering in non-root workflow are cancelled when initial render of root workflow fails`() { + var sideEffectWasRan = false + var cancellationException: Throwable? = null + val childWorkflow = Workflow.stateless { + runningSideEffect("test") { + sideEffectWasRan = true suspendCancellableCoroutine { continuation -> continuation.invokeOnCancellation { cause -> cancellationException = cause } } - }) + } + } + val workflow = Workflow.stateless { + renderChild(childWorkflow) throw ExpectedException() } - scope.pauseDispatcher() assertFailsWith { renderWorkflowIn(workflow, scope, MutableStateFlow(Unit)) {} } - assertTrue(workerWasRan) + scope.advanceUntilIdle() + assertTrue(sideEffectWasRan) assertNotNull(cancellationException) val realCause = generateSequence(cancellationException) { it.cause } .firstOrNull { it !is CancellationException } assertTrue(realCause is ExpectedException) } + @Test + fun `side effects from initial rendering in non-root workflow are never started when initial render of non-root workflow fails`() { + var sideEffectWasRan = false + val childWorkflow = Workflow.stateless { + runningSideEffect("test") { + sideEffectWasRan = true + } + throw ExpectedException() + } + val workflow = Workflow.stateless { + renderChild(childWorkflow) + } + + assertFailsWith { + renderWorkflowIn(workflow, scope, MutableStateFlow(Unit)) {} + } + scope.advanceUntilIdle() + assertFalse(sideEffectWasRan) + } + @Test fun `exception from non-initial render fails parent scope`() { val trigger = CompletableDeferred() // Throws an exception when trigger is completed. diff --git a/workflow-runtime/src/test/java/com/squareup/workflow/WorkflowInterceptorTest.kt b/workflow-runtime/src/test/java/com/squareup/workflow/WorkflowInterceptorTest.kt index 106b853cbd..f1f10caa6b 100644 --- a/workflow-runtime/src/test/java/com/squareup/workflow/WorkflowInterceptorTest.kt +++ b/workflow-runtime/src/test/java/com/squareup/workflow/WorkflowInterceptorTest.kt @@ -67,12 +67,6 @@ class WorkflowInterceptorTest { handler: (ChildOutputT) -> WorkflowAction ): ChildRenderingT = fail() - override fun runningWorker( - worker: Worker, - key: String, - handler: (T) -> WorkflowAction - ): Unit = fail() - override fun runningSideEffect( key: String, sideEffect: suspend () -> Unit diff --git a/workflow-runtime/src/test/java/com/squareup/workflow/WorkflowOperatorsTest.kt b/workflow-runtime/src/test/java/com/squareup/workflow/WorkflowOperatorsTest.kt index c163a9de5a..a417b8e481 100644 --- a/workflow-runtime/src/test/java/com/squareup/workflow/WorkflowOperatorsTest.kt +++ b/workflow-runtime/src/test/java/com/squareup/workflow/WorkflowOperatorsTest.kt @@ -233,7 +233,7 @@ class WorkflowOperatorsTest { context: RenderContext ): T { // Listen to the flow to trigger a re-render when it updates. - context.runningWorker(rerenderWorker) { WorkflowAction.noAction() } + context.runningWorker(rerenderWorker as Worker) { WorkflowAction.noAction() } return flow.value } diff --git a/workflow-runtime/src/test/java/com/squareup/workflow/internal/ChainedWorkflowInterceptorTest.kt b/workflow-runtime/src/test/java/com/squareup/workflow/internal/ChainedWorkflowInterceptorTest.kt index 82d9bb8f35..0ece908d1b 100644 --- a/workflow-runtime/src/test/java/com/squareup/workflow/internal/ChainedWorkflowInterceptorTest.kt +++ b/workflow-runtime/src/test/java/com/squareup/workflow/internal/ChainedWorkflowInterceptorTest.kt @@ -22,7 +22,6 @@ import com.squareup.workflow.NoopWorkflowInterceptor import com.squareup.workflow.RenderContext import com.squareup.workflow.Sink import com.squareup.workflow.Snapshot -import com.squareup.workflow.Worker import com.squareup.workflow.Workflow import com.squareup.workflow.WorkflowAction import com.squareup.workflow.WorkflowIdentifier @@ -270,14 +269,6 @@ class ChainedWorkflowInterceptorTest { fail() } - override fun runningWorker( - worker: Worker, - key: String, - handler: (T) -> WorkflowAction - ) { - fail() - } - override fun runningSideEffect( key: String, sideEffect: suspend () -> Unit diff --git a/workflow-runtime/src/test/java/com/squareup/workflow/internal/RealRenderContextTest.kt b/workflow-runtime/src/test/java/com/squareup/workflow/internal/RealRenderContextTest.kt index 1e2de2819d..3f1c10043d 100644 --- a/workflow-runtime/src/test/java/com/squareup/workflow/internal/RealRenderContextTest.kt +++ b/workflow-runtime/src/test/java/com/squareup/workflow/internal/RealRenderContextTest.kt @@ -22,7 +22,6 @@ import com.squareup.workflow.RenderContext import com.squareup.workflow.Sink import com.squareup.workflow.Snapshot import com.squareup.workflow.StatefulWorkflow -import com.squareup.workflow.Worker import com.squareup.workflow.Workflow import com.squareup.workflow.WorkflowAction import com.squareup.workflow.WorkflowAction.Companion.noAction @@ -31,7 +30,6 @@ import com.squareup.workflow.action import com.squareup.workflow.applyTo import com.squareup.workflow.internal.RealRenderContext.Renderer import com.squareup.workflow.internal.RealRenderContext.SideEffectRunner -import com.squareup.workflow.internal.RealRenderContext.WorkerRunner import com.squareup.workflow.internal.RealRenderContextTest.TestRenderer.Rendering import com.squareup.workflow.makeEventSink import com.squareup.workflow.renderChild @@ -72,15 +70,7 @@ class RealRenderContextTest { ) as ChildRenderingT } - private class TestRunner : WorkerRunner, SideEffectRunner { - override fun runningWorker( - worker: Worker, - key: String, - handler: (T) -> WorkflowAction - ) { - // No-op - } - + private class TestRunner : SideEffectRunner { override fun runningSideEffect( key: String, sideEffect: suspend () -> Unit @@ -115,15 +105,7 @@ class RealRenderContextTest { ): ChildRenderingT = fail() } - private class PoisonRunner : WorkerRunner, SideEffectRunner { - override fun runningWorker( - worker: Worker, - key: String, - handler: (T) -> WorkflowAction - ) { - fail() - } - + private class PoisonRunner : SideEffectRunner { override fun runningSideEffect( key: String, sideEffect: suspend () -> Unit @@ -255,18 +237,16 @@ class RealRenderContextTest { val child = Workflow.stateless { fail() } assertFailsWith { context.renderChild(child) } - val worker = Worker.from { Unit } - assertFailsWith { context.runningWorker(worker) { fail() } } assertFailsWith { context.freeze() } } private fun createdPoisonedContext(): RealRenderContext { - val workerRunner = PoisonRunner() - return RealRenderContext(PoisonRenderer(), workerRunner, workerRunner, eventActionsChannel) + val workerRunner = PoisonRunner() + return RealRenderContext(PoisonRenderer(), workerRunner, eventActionsChannel) } private fun createTestContext(): RealRenderContext { val workerRunner = TestRunner() - return RealRenderContext(TestRenderer(), workerRunner, workerRunner, eventActionsChannel) + return RealRenderContext(TestRenderer(), workerRunner, eventActionsChannel) } } diff --git a/workflow-runtime/src/test/java/com/squareup/workflow/internal/WorkersTest.kt b/workflow-runtime/src/test/java/com/squareup/workflow/internal/WorkersTest.kt deleted file mode 100644 index 479a385a0b..0000000000 --- a/workflow-runtime/src/test/java/com/squareup/workflow/internal/WorkersTest.kt +++ /dev/null @@ -1,220 +0,0 @@ -/* - * Copyright 2019 Square Inc. - * - * 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. - */ -@file:Suppress("EXPERIMENTAL_API_USAGE") - -package com.squareup.workflow.internal - -import com.squareup.workflow.Worker -import com.squareup.workflow.asWorker -import kotlinx.coroutines.CoroutineName -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.CoroutineStart.UNDISPATCHED -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.channels.Channel -import kotlinx.coroutines.channels.consume -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.consumeAsFlow -import kotlinx.coroutines.flow.flow -import kotlinx.coroutines.launch -import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.supervisorScope -import kotlinx.coroutines.yield -import java.util.concurrent.atomic.AtomicInteger -import kotlin.coroutines.ContinuationInterceptor -import kotlin.coroutines.EmptyCoroutineContext -import kotlin.coroutines.coroutineContext -import kotlin.test.Test -import kotlin.test.assertEquals -import kotlin.test.assertFailsWith -import kotlin.test.assertSame -import kotlin.test.assertTrue - -class WorkersTest { - - @Test fun `launchWorker propagates backpressure`() { - val channel = Channel() - val worker = channel.consumeAsFlow() - .asWorker() - // Used to assert ordering. - val counter = AtomicInteger(0) - - runBlocking { - val workerOutputs = launchWorker(worker) - - launch(start = UNDISPATCHED) { - assertEquals(0, counter.getAndIncrement()) - channel.send("a") - assertEquals(1, counter.getAndIncrement()) - channel.send("b") - assertEquals(3, counter.getAndIncrement()) - channel.close() - assertEquals(4, counter.getAndIncrement()) - } - yield() - assertEquals(2, counter.getAndIncrement()) - - assertEquals("a", workerOutputs.poll()!!.value) - yield() - assertEquals(5, counter.getAndIncrement()) - - assertEquals("b", workerOutputs.poll()!!.value) - yield() - assertEquals(6, counter.getAndIncrement()) - - // Cancel the worker so we can exit this loop. - workerOutputs.cancel() - } - } - - @Test fun `launchWorker emits done when complete immediately`() { - val channel = Channel(capacity = 1) - - runBlocking { - val workerOutputs = launchWorker( - channel.consumeAsFlow() - .asWorker() - ) - assertTrue(workerOutputs.isEmpty) - - channel.close() - assertTrue(workerOutputs.receive().isDone) - } - } - - @Test fun `launchWorker emits done when complete after sending`() { - val channel = Channel(capacity = 1) - - runBlocking { - val workerOutputs = launchWorker( - channel.consumeAsFlow() - .asWorker() - ) - assertTrue(workerOutputs.isEmpty) - - channel.send("foo") - assertEquals("foo", workerOutputs.receive().value) - - channel.close() - assertTrue(workerOutputs.receive().isDone) - } - } - - @Test fun `launchWorker does not emit done when failed`() { - val channel = Channel(capacity = 1) - - runBlocking { - // Needed so that cancelling the channel doesn't cancel our job, which means receive will - // throw the JobCancellationException instead of the actual channel failure. - supervisorScope { - val workerOutputs = launchWorker( - channel.consumeAsFlow() - .asWorker() - ) - assertTrue(workerOutputs.isEmpty) - - channel.close(ExpectedException()) - assertFailsWith { workerOutputs.receive() } - } - } - } - - @Test fun `launchWorker completes after emitting done`() { - val channel = Channel(capacity = 1) - - runBlocking { - val workerOutputs = launchWorker( - channel.consumeAsFlow() - .asWorker() - ) - channel.close() - assertTrue(workerOutputs.receive().isDone) - - assertTrue(channel.isClosedForReceive) - } - } - - /** - * This should be impossible, since the return type is non-nullable. However it is very easy to - * accidentally create a mock using libraries like Mockito in unit tests that return null Flows. - */ - @Test fun `launchWorker throws when flow is null`() { - val nullFlowWorker = NullFlowWorker() - - val error = runBlocking { - assertFailsWith { - launchWorker(nullFlowWorker) - } - } - - assertEquals( - "Worker NullFlowWorker.toString returned a null Flow. " + - "If this is a test mock, make sure you mock the run() method!", - error.message - ) - } - - @Test fun `launchWorker coroutine is named without key`() { - val output = runBlocking { - launchWorker(CoroutineNameWorker) - .consume { receive() } - .value - } - - assertEquals("CoroutineNameWorker.toString", output) - } - - @Test fun `launchWorker coroutine is named with key`() { - val output = runBlocking { - launchWorker(CoroutineNameWorker, key = "foo") - .consume { receive() } - .value - } - - assertEquals("CoroutineNameWorker.toString:foo", output) - } - - @Test fun `launchWorker dispatcher is unconfined`() { - val worker = Worker.from { coroutineContext[ContinuationInterceptor] } - - runBlocking { - val interceptor = launchWorker(worker) - .consume { receive() } - .value - - assertSame(Dispatchers.Unconfined, interceptor) - } - } - - private fun CoroutineScope.launchWorker( - worker: Worker, - key: String = "" - ) = launchWorker( - worker, - key = key, - workerContext = EmptyCoroutineContext - ) - - private class ExpectedException : RuntimeException() - - private object CoroutineNameWorker : Worker { - override fun run(): Flow = flow { - val nameElement = coroutineContext[CoroutineName] as CoroutineName - emit(nameElement.name) - } - - override fun toString(): String = "CoroutineNameWorker.toString" - } -} diff --git a/workflow-runtime/src/test/java/com/squareup/workflow/internal/WorkflowNodeTest.kt b/workflow-runtime/src/test/java/com/squareup/workflow/internal/WorkflowNodeTest.kt index be0b1efddd..ae54fd1790 100644 --- a/workflow-runtime/src/test/java/com/squareup/workflow/internal/WorkflowNodeTest.kt +++ b/workflow-runtime/src/test/java/com/squareup/workflow/internal/WorkflowNodeTest.kt @@ -23,7 +23,6 @@ import com.squareup.workflow.Sink import com.squareup.workflow.Snapshot import com.squareup.workflow.StatefulWorkflow import com.squareup.workflow.TreeSnapshot -import com.squareup.workflow.Worker import com.squareup.workflow.Workflow import com.squareup.workflow.WorkflowAction import com.squareup.workflow.WorkflowAction.Companion.emitOutput @@ -33,7 +32,6 @@ import com.squareup.workflow.WorkflowInterceptor import com.squareup.workflow.WorkflowInterceptor.WorkflowSession import com.squareup.workflow.WorkflowOutput import com.squareup.workflow.action -import com.squareup.workflow.asWorker import com.squareup.workflow.contraMap import com.squareup.workflow.identifier import com.squareup.workflow.makeEventSink @@ -45,21 +43,16 @@ import com.squareup.workflow.stateful import com.squareup.workflow.stateless import com.squareup.workflow.writeUtf8WithLength import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineName import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers.Unconfined import kotlinx.coroutines.Job -import kotlinx.coroutines.Runnable -import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.cancel -import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.runBlocking import kotlinx.coroutines.selects.select import kotlinx.coroutines.suspendCancellableCoroutine import kotlinx.coroutines.withTimeout -import kotlin.coroutines.ContinuationInterceptor import kotlin.coroutines.CoroutineContext import kotlin.coroutines.coroutineContext import kotlin.test.AfterTest @@ -275,122 +268,6 @@ class WorkflowNodeTest { sink.send(emitOutput("event2")) } - @Test fun `worker gets value`() { - val channel = Channel(capacity = 1) - var update: String? = null - val workflow = object : StringWorkflow() { - override fun initialState( - props: String, - snapshot: Snapshot? - ): String { - assertNull(snapshot) - return props - } - - override fun render( - props: String, - state: String, - context: RenderContext - ): String { - context.runningWorker(channel.asWorker()) { - check(update == null) - update = it - action { setOutput("update:$it") } - } - return "" - } - } - val node = WorkflowNode(workflow.id(), workflow, "", TreeSnapshot.NONE, context) - - assertEquals(null, update) - node.render(workflow, "") - assertEquals(null, update) - - // Shouldn't have the update yet, since we haven't sent anything. - val output = runBlocking { - try { - withTimeout(1) { - select?> { - node.tick(this) - } - } - fail("Expected exception") - } catch (e: TimeoutCancellationException) { - // Expected. - } - - channel.send("element") - - withTimeout(1) { - select?> { - node.tick(this) - } - } - } - - assertEquals("element", update) - assertEquals("update:element", output?.value) - } - - @Test fun `worker is cancelled`() { - val channel = Channel(capacity = 0) - lateinit var doClose: () -> Unit - val workflow = object : StringWorkflow() { - override fun initialState( - props: String, - snapshot: Snapshot? - ): String { - assertNull(snapshot) - return props - } - - fun update(value: String) = action { - setOutput("update:$value") - } - - val finish = action { - state = "finished" - } - - override fun render( - props: String, - state: String, - context: RenderContext - ): String { - when (state) { - "listen" -> { - context.runningWorker(channel.asWorker(closeOnCancel = true)) { - update(it) - } - doClose = { context.actionSink.send(finish) } - } - } - return "" - } - } - val node = WorkflowNode(workflow.id(), workflow, "listen", TreeSnapshot.NONE, context) - - runBlocking { - node.render(workflow, "listen") - assertFalse(channel.isClosedForSend) - doClose() - - // This tick will process the event handler, it won't close the channel yet. - withTimeout(1) { - select?> { - node.tick(this) - } - } - - assertFalse(channel.isClosedForSend) - - // This should close the channel. - node.render(workflow, "") - - assertTrue(channel.isClosedForSend) - } - } - @Test fun `sideEffect is not started until after render completes`() { var started = false val workflow = Workflow.stateless { @@ -410,36 +287,6 @@ class WorkflowNodeTest { } } - @Test fun `sideEffect is launched with dispatcher from workflow context`() { - class TestDispatcher : CoroutineDispatcher() { - override fun dispatch( - context: CoroutineContext, - block: Runnable - ) = Unconfined.dispatch(context, block) - - override fun isDispatchNeeded(context: CoroutineContext): Boolean = - Unconfined.isDispatchNeeded(context) - } - - val baseDispatcher = TestDispatcher() - val sideEffectDispatcher = TestDispatcher() - var contextFromWorker: CoroutineContext? = null - val workflow = Workflow.stateless { - runningSideEffect("the key") { - contextFromWorker = coroutineContext - } - } - val node = WorkflowNode( - workflow.id(), workflow.asStatefulWorkflow(), initialProps = Unit, - snapshot = TreeSnapshot.NONE, baseContext = context + baseDispatcher, - workerContext = context + sideEffectDispatcher - ) - - node.render(workflow.asStatefulWorkflow(), Unit) - assertSame(baseDispatcher, node.coroutineContext[ContinuationInterceptor]) - assertSame(baseDispatcher, contextFromWorker!![ContinuationInterceptor]) - } - @Test fun `sideEffect coroutine is named`() { var contextFromWorker: CoroutineContext? = null val workflow = Workflow.stateless { @@ -460,27 +307,6 @@ class WorkflowNodeTest { ) } - @Test fun `sideEffect ignores name from worker context`() { - var contextFromWorker: CoroutineContext? = null - val workflow = Workflow.stateless { - runningSideEffect("the key") { - contextFromWorker = coroutineContext - } - } - val node = WorkflowNode( - workflow.id(), workflow.asStatefulWorkflow(), initialProps = Unit, - snapshot = TreeSnapshot.NONE, baseContext = context, - workerContext = context + CoroutineName("ignored name") - ) - - node.render(workflow.asStatefulWorkflow(), Unit) - assertEquals(WorkflowNodeId(workflow).toString(), node.coroutineContext[CoroutineName]!!.name) - assertEquals( - "sideEffect[the key] for ${WorkflowNodeId(workflow)}", - contextFromWorker!![CoroutineName]!!.name - ) - } - @Test fun `sideEffect can send to actionSink`() { val workflow = Workflow.stateless { runningSideEffect("key") { @@ -1351,84 +1177,13 @@ class WorkflowNodeTest { assertNull(output?.value) } - @Test fun `worker action changes state`() { - val workflow = Workflow.stateful( - initialState = { "initial" }, - render = { _, state -> - runningWorker(Worker.from { "hello" }) { action { this.state = "${this.state}->$it" } } - return@stateful state - } - ) - val node = WorkflowNode( - workflow.id(), - workflow.asStatefulWorkflow(), - initialProps = Unit, - snapshot = TreeSnapshot.NONE, - baseContext = Unconfined - ) - node.render(workflow.asStatefulWorkflow(), Unit) - - runBlocking { - select?> { - node.tick(this) - } - } - - val state = node.render(workflow.asStatefulWorkflow(), Unit) - assertEquals("initial->hello", state) - } - - @Test fun `worker action emits output`() { - val workflow = Worker.from { "hello" } - .asWorkflow() - val node = WorkflowNode( - workflow.id(), - workflow.asStatefulWorkflow(), - initialProps = Unit, - snapshot = TreeSnapshot.NONE, - baseContext = Unconfined, - emitOutputToParent = { WorkflowOutput("output:$it") } - ) - node.render(workflow.asStatefulWorkflow(), Unit) - - val output = runBlocking { - select?> { - node.tick(this) - } - } - - assertEquals("output:hello", output?.value) - } - - @Test fun `worker action allows null output`() { - val workflow = Worker.from { null } - .asWorkflow() - val node = WorkflowNode( - workflow.id(), - workflow.asStatefulWorkflow(), - initialProps = Unit, - snapshot = TreeSnapshot.NONE, - baseContext = Unconfined, - emitOutputToParent = { WorkflowOutput(it) } - ) - node.render(workflow.asStatefulWorkflow(), Unit) - - val output = runBlocking { - select?> { - node.tick(this) - } - } - - assertNull(output?.value) - } - @Test fun `child action changes state`() { - val child = Worker.from { "hello" } - .asWorkflow() val workflow = Workflow.stateful( initialState = { "initial" }, render = { _, state -> - renderChild(child) { action { this.state = "${this.state}->$it" } } + runningSideEffect("test") { + actionSink.send(action { this.state = "${this.state}->hello" }) + } return@stateful state } ) @@ -1452,10 +1207,10 @@ class WorkflowNodeTest { } @Test fun `child action emits output`() { - val child = Worker.from { "hello" } - .asWorkflow() val workflow = Workflow.stateless { - renderChild(child) { action { setOutput("child:$it") } } + runningSideEffect("test") { + actionSink.send(action { setOutput("child:hello") }) + } } val node = WorkflowNode( workflow.id(), @@ -1477,10 +1232,10 @@ class WorkflowNodeTest { } @Test fun `child action allows null output`() { - val child = Worker.from { null } - .asWorkflow() val workflow = Workflow.stateless { - renderChild(child) { action { setOutput(null) } } + runningSideEffect("test") { + actionSink.send(action { setOutput(null) }) + } } val node = WorkflowNode( workflow.id(), @@ -1506,8 +1261,4 @@ class WorkflowNodeTest { override val renderKey: String = "" override val parent: WorkflowSession? = null } - - private fun Worker.asWorkflow() = Workflow.stateless { - runningWorker(this@asWorkflow) { action { setOutput(it) } } - } } diff --git a/workflow-runtime/src/test/java/com/squareup/workflow/internal/WorkflowRunnerTest.kt b/workflow-runtime/src/test/java/com/squareup/workflow/internal/WorkflowRunnerTest.kt index 5df727fa0d..63653f3a99 100644 --- a/workflow-runtime/src/test/java/com/squareup/workflow/internal/WorkflowRunnerTest.kt +++ b/workflow-runtime/src/test/java/com/squareup/workflow/internal/WorkflowRunnerTest.kt @@ -197,16 +197,13 @@ class WorkflowRunnerTest { } val runner = WorkflowRunner(workflow, MutableStateFlow(Unit)) runner.nextRendering() - val output = scope.async { runner.nextOutput() } dispatcher.resumeDispatcher() dispatcher.advanceUntilIdle() - assertTrue(output.isActive) assertNull(cancellationException) runner.cancelRuntime() dispatcher.advanceUntilIdle() - assertTrue(output.isCancelled) assertNotNull(cancellationException) val causes = generateSequence(cancellationException) { it.cause } assertTrue(causes.all { it is CancellationException }) diff --git a/workflow-rx2/api/workflow-rx2.api b/workflow-rx2/api/workflow-rx2.api index b31548ce88..d0ad74725e 100644 --- a/workflow-rx2/api/workflow-rx2.api +++ b/workflow-rx2/api/workflow-rx2.api @@ -1,6 +1,7 @@ public abstract class com/squareup/workflow/rx2/PublisherWorker : com/squareup/workflow/Worker { public fun ()V public fun doesSameWorkAs (Lcom/squareup/workflow/Worker;)Z + public fun getOutputType ()Lkotlin/reflect/KType; public final fun run ()Lkotlinx/coroutines/flow/Flow; public abstract fun runPublisher ()Lorg/reactivestreams/Publisher; } diff --git a/workflow-rx2/src/test/java/com/squareup/workflow/rx2/PublisherWorkerTest.kt b/workflow-rx2/src/test/java/com/squareup/workflow/rx2/PublisherWorkerTest.kt index 48b068b9ac..db45760861 100644 --- a/workflow-rx2/src/test/java/com/squareup/workflow/rx2/PublisherWorkerTest.kt +++ b/workflow-rx2/src/test/java/com/squareup/workflow/rx2/PublisherWorkerTest.kt @@ -18,6 +18,7 @@ package com.squareup.workflow.rx2 import com.squareup.workflow.Worker import com.squareup.workflow.Workflow import com.squareup.workflow.action +import com.squareup.workflow.runningWorker import com.squareup.workflow.stateless import com.squareup.workflow.testing.launchForTestingFromStartWith import io.reactivex.BackpressureStrategy.BUFFER diff --git a/workflow-testing/api/workflow-testing.api b/workflow-testing/api/workflow-testing.api index 8937dbf73f..2e896f3bb1 100644 --- a/workflow-testing/api/workflow-testing.api +++ b/workflow-testing/api/workflow-testing.api @@ -13,59 +13,27 @@ public abstract interface class com/squareup/workflow/testing/RenderTestResult { } public abstract interface class com/squareup/workflow/testing/RenderTester { - public abstract fun expectSideEffect (Ljava/lang/String;ZLkotlin/jvm/functions/Function1;)Lcom/squareup/workflow/testing/RenderTester; - public abstract fun expectWorker (Lkotlin/jvm/functions/Function1;Ljava/lang/String;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;)Lcom/squareup/workflow/testing/RenderTester; + public abstract fun disallowUnexpectedChildren ()Lcom/squareup/workflow/testing/RenderTester; + public abstract fun expectSideEffect (Ljava/lang/String;)Lcom/squareup/workflow/testing/RenderTester; public abstract fun expectWorkflow (Lcom/squareup/workflow/WorkflowIdentifier;Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;)Lcom/squareup/workflow/testing/RenderTester; - public abstract fun expectWorkflow (Ljava/lang/String;ZLkotlin/jvm/functions/Function1;)Lcom/squareup/workflow/testing/RenderTester; public abstract fun expectWorkflow (Lkotlin/reflect/KClass;Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;)Lcom/squareup/workflow/testing/RenderTester; public abstract fun render (Lkotlin/jvm/functions/Function1;)Lcom/squareup/workflow/testing/RenderTestResult; } -public abstract class com/squareup/workflow/testing/RenderTester$ChildWorkflowMatch { -} - -public final class com/squareup/workflow/testing/RenderTester$ChildWorkflowMatch$Matched : com/squareup/workflow/testing/RenderTester$ChildWorkflowMatch { - public fun (Ljava/lang/Object;Lcom/squareup/workflow/WorkflowOutput;)V - public synthetic fun (Ljava/lang/Object;Lcom/squareup/workflow/WorkflowOutput;ILkotlin/jvm/internal/DefaultConstructorMarker;)V - public final fun getChildRendering ()Ljava/lang/Object; - public final fun getOutput ()Lcom/squareup/workflow/WorkflowOutput; -} - -public final class com/squareup/workflow/testing/RenderTester$ChildWorkflowMatch$NotMatched : com/squareup/workflow/testing/RenderTester$ChildWorkflowMatch { - public static final field INSTANCE Lcom/squareup/workflow/testing/RenderTester$ChildWorkflowMatch$NotMatched; -} - public final class com/squareup/workflow/testing/RenderTester$DefaultImpls { - public static synthetic fun expectSideEffect$default (Lcom/squareup/workflow/testing/RenderTester;Ljava/lang/String;ZLkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow/testing/RenderTester; - public static synthetic fun expectWorker$default (Lcom/squareup/workflow/testing/RenderTester;Lkotlin/jvm/functions/Function1;Ljava/lang/String;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;ILjava/lang/Object;)Lcom/squareup/workflow/testing/RenderTester; - public static fun expectWorkflow (Lcom/squareup/workflow/testing/RenderTester;Lcom/squareup/workflow/WorkflowIdentifier;Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;)Lcom/squareup/workflow/testing/RenderTester; public static fun expectWorkflow (Lcom/squareup/workflow/testing/RenderTester;Lkotlin/reflect/KClass;Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;)Lcom/squareup/workflow/testing/RenderTester; public static synthetic fun expectWorkflow$default (Lcom/squareup/workflow/testing/RenderTester;Lcom/squareup/workflow/WorkflowIdentifier;Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;ILjava/lang/Object;)Lcom/squareup/workflow/testing/RenderTester; - public static synthetic fun expectWorkflow$default (Lcom/squareup/workflow/testing/RenderTester;Ljava/lang/String;ZLkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow/testing/RenderTester; public static synthetic fun expectWorkflow$default (Lcom/squareup/workflow/testing/RenderTester;Lkotlin/reflect/KClass;Ljava/lang/Object;Ljava/lang/String;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;ILjava/lang/Object;)Lcom/squareup/workflow/testing/RenderTester; public static synthetic fun render$default (Lcom/squareup/workflow/testing/RenderTester;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/squareup/workflow/testing/RenderTestResult; } -public final class com/squareup/workflow/testing/RenderTester$RenderChildInvocation { - public fun (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;Lkotlin/reflect/KTypeProjection;Lkotlin/reflect/KTypeProjection;Ljava/lang/String;)V - public final fun getOutputType ()Lkotlin/reflect/KTypeProjection; - public final fun getProps ()Ljava/lang/Object; - public final fun getRenderKey ()Ljava/lang/String; - public final fun getRenderingType ()Lkotlin/reflect/KTypeProjection; - public final fun getWorkflow ()Lcom/squareup/workflow/Workflow; -} - -public final class com/squareup/workflow/testing/RenderTester$SideEffectMatch : java/lang/Enum { - public static final field MATCH Lcom/squareup/workflow/testing/RenderTester$SideEffectMatch; - public static final field NOT_MATCHED Lcom/squareup/workflow/testing/RenderTester$SideEffectMatch; - public static fun valueOf (Ljava/lang/String;)Lcom/squareup/workflow/testing/RenderTester$SideEffectMatch; - public static fun values ()[Lcom/squareup/workflow/testing/RenderTester$SideEffectMatch; -} - public final class com/squareup/workflow/testing/RenderTesterKt { - public static final fun expectSideEffect (Lcom/squareup/workflow/testing/RenderTester;Ljava/lang/String;)Lcom/squareup/workflow/testing/RenderTester; - public static final fun expectWorker (Lcom/squareup/workflow/testing/RenderTester;Lcom/squareup/workflow/Worker;Ljava/lang/String;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;)Lcom/squareup/workflow/testing/RenderTester; - public static synthetic fun expectWorker$default (Lcom/squareup/workflow/testing/RenderTester;Lcom/squareup/workflow/Worker;Ljava/lang/String;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;ILjava/lang/Object;)Lcom/squareup/workflow/testing/RenderTester; + public static final fun expectAnyWorkerOutputting (Lcom/squareup/workflow/testing/RenderTester;Lkotlin/reflect/KType;Ljava/lang/String;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;)Lcom/squareup/workflow/testing/RenderTester; + public static synthetic fun expectAnyWorkerOutputting$default (Lcom/squareup/workflow/testing/RenderTester;Lkotlin/reflect/KType;Ljava/lang/String;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;ILjava/lang/Object;)Lcom/squareup/workflow/testing/RenderTester; + public static final fun expectWorker (Lcom/squareup/workflow/testing/RenderTester;Lkotlin/reflect/KClass;Ljava/util/List;Ljava/lang/String;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;)Lcom/squareup/workflow/testing/RenderTester; + public static final fun expectWorker (Lcom/squareup/workflow/testing/RenderTester;Lkotlin/reflect/KType;Ljava/lang/String;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;)Lcom/squareup/workflow/testing/RenderTester; + public static synthetic fun expectWorker$default (Lcom/squareup/workflow/testing/RenderTester;Lkotlin/reflect/KClass;Ljava/util/List;Ljava/lang/String;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;ILjava/lang/Object;)Lcom/squareup/workflow/testing/RenderTester; + public static synthetic fun expectWorker$default (Lcom/squareup/workflow/testing/RenderTester;Lkotlin/reflect/KType;Ljava/lang/String;Lkotlin/jvm/functions/Function1;Lcom/squareup/workflow/WorkflowOutput;Ljava/lang/String;ILjava/lang/Object;)Lcom/squareup/workflow/testing/RenderTester; public static final fun renderTester (Lcom/squareup/workflow/StatefulWorkflow;Ljava/lang/Object;Ljava/lang/Object;)Lcom/squareup/workflow/testing/RenderTester; public static final fun renderTester (Lcom/squareup/workflow/Workflow;Ljava/lang/Object;)Lcom/squareup/workflow/testing/RenderTester; public static final fun testRender (Lcom/squareup/workflow/StatefulWorkflow;Ljava/lang/Object;Ljava/lang/Object;)Lcom/squareup/workflow/testing/RenderTester; @@ -75,6 +43,7 @@ public final class com/squareup/workflow/testing/RenderTesterKt { public final class com/squareup/workflow/testing/WorkerSink : com/squareup/workflow/Worker { public fun (Ljava/lang/String;Lkotlin/reflect/KClass;)V public fun doesSameWorkAs (Lcom/squareup/workflow/Worker;)Z + public fun getOutputType ()Lkotlin/reflect/KType; public fun run ()Lkotlinx/coroutines/flow/Flow; public final fun send (Ljava/lang/Object;)V public fun toString ()Ljava/lang/String; diff --git a/workflow-testing/src/main/java/com/squareup/workflow/testing/RealRenderTester.kt b/workflow-testing/src/main/java/com/squareup/workflow/testing/RealRenderTester.kt index 334381f08a..e5c63a2584 100644 --- a/workflow-testing/src/main/java/com/squareup/workflow/testing/RealRenderTester.kt +++ b/workflow-testing/src/main/java/com/squareup/workflow/testing/RealRenderTester.kt @@ -98,21 +98,6 @@ internal class RealRenderTester( expectations += ExpectedWorkflow(matcher, exactMatch, description) } - override fun expectWorker( - matchesWhen: (otherWorker: Worker<*>) -> Boolean, - key: String, - output: WorkflowOutput?, - description: String - ): RenderTester { - val expectedWorker = ExpectedWorker(matchesWhen, key, output, description) - if (output != null) { - checkNoOutputs(expectedWorker) - childWillEmitOutput = true - } - expectations += expectedWorker - return this - } - override fun expectSideEffect( description: String, exactMatch: Boolean, @@ -203,30 +188,6 @@ internal class RealRenderTester( return match.childRendering as ChildRenderingT } - override fun runningWorker( - worker: Worker, - key: String, - handler: (T) -> WorkflowAction - ) { - val description = "worker $worker" + - key.takeUnless { it.isEmpty() } - ?.let { " with key \"$it\"" } - .orEmpty() - val expected = consumeExpectedWorker>( - predicate = { it.matchesWhen(worker) && it.key == key }, - description = { description } - ) - - if (expected?.output != null) { - check(processedAction == null) { - "Expected only one output to be expected: $description expected to emit " + - "${expected.output.value} but $processedAction was already processed." - } - @Suppress("UNCHECKED_CAST") - processedAction = handler(expected.output.value as T) - } - } - override fun runningSideEffect( key: String, sideEffect: suspend () -> Unit diff --git a/workflow-testing/src/main/java/com/squareup/workflow/testing/RenderIdempotencyChecker.kt b/workflow-testing/src/main/java/com/squareup/workflow/testing/RenderIdempotencyChecker.kt index 776fea3d9c..d17c418550 100644 --- a/workflow-testing/src/main/java/com/squareup/workflow/testing/RenderIdempotencyChecker.kt +++ b/workflow-testing/src/main/java/com/squareup/workflow/testing/RenderIdempotencyChecker.kt @@ -18,7 +18,6 @@ package com.squareup.workflow.testing import com.squareup.workflow.ExperimentalWorkflowApi import com.squareup.workflow.RenderContext import com.squareup.workflow.Sink -import com.squareup.workflow.Worker import com.squareup.workflow.Workflow import com.squareup.workflow.WorkflowAction import com.squareup.workflow.WorkflowInterceptor @@ -99,17 +98,6 @@ private class RecordingRenderContext( childRenderings.removeLast() as ChildRenderingT } - override fun runningWorker( - worker: Worker, - key: String, - handler: (T) -> WorkflowAction - ) { - if (!replaying) { - delegate.runningWorker(worker, key, handler) - } - // Else noop. - } - override fun runningSideEffect( key: String, sideEffect: suspend () -> Unit diff --git a/workflow-testing/src/main/java/com/squareup/workflow/testing/RenderTester.kt b/workflow-testing/src/main/java/com/squareup/workflow/testing/RenderTester.kt index 3c8900efeb..4363564cb2 100644 --- a/workflow-testing/src/main/java/com/squareup/workflow/testing/RenderTester.kt +++ b/workflow-testing/src/main/java/com/squareup/workflow/testing/RenderTester.kt @@ -22,11 +22,15 @@ import com.squareup.workflow.Workflow import com.squareup.workflow.WorkflowAction import com.squareup.workflow.WorkflowIdentifier import com.squareup.workflow.WorkflowOutput +import com.squareup.workflow.unsnapshottableIdentifier import com.squareup.workflow.identifier import com.squareup.workflow.testing.RenderTester.SideEffectMatch.MATCH import com.squareup.workflow.testing.RenderTester.SideEffectMatch.NOT_MATCHED import com.squareup.workflow.workflowIdentifier import kotlin.reflect.KClass +import kotlin.reflect.KTypeProjection.Companion.STAR +import kotlin.reflect.full.createType +import kotlin.reflect.typeOf import kotlin.reflect.KType import kotlin.reflect.KTypeProjection @@ -362,43 +366,6 @@ interface RenderTester { assertProps(it as ChildPropsT) }) - /** - * Specifies that this render pass is expected to render a particular child workflow. - * - * @param description String that will be used to describe this expectation in error messages. - * @param exactMatch If true, then the test will fail if any other matching expectations are also - * exact matches, and the expectation will only be allowed to match a single child workflow. - * If false, the match will only be used if no other expectations return exclusive matches (in - * which case the first match will be used), and the expectation may match multiple children. - * @param matcher A function that determines whether a given [RenderChildInvocation] matches this - * expectation by returning a [ChildWorkflowMatch]. If the expectation matches, the function - * must include the rendering and optional output for the child workflow. - */ - fun expectWorkflow( - description: String, - exactMatch: Boolean = true, - matcher: (RenderChildInvocation) -> ChildWorkflowMatch - ): RenderTester - - /** - * Specifies that this render pass is expected to run a particular worker. - * - * @param matchesWhen Predicate used to determine if this matches the worker being ran. - * @param key The key passed to [runningWorker][com.squareup.workflow.RenderContext.runningWorker] - * when rendering this workflow. - * @param output If non-null, [WorkflowOutput.value] will be emitted when this worker is ran. - * The [WorkflowAction] used to handle this output can be verified using methods on - * [RenderTestResult]. - * @param description Optional string that will be used to describe this expectation in error - * messages. - */ - fun expectWorker( - matchesWhen: (otherWorker: Worker<*>) -> Boolean, - key: String = "", - output: WorkflowOutput? = null, - description: String = "" - ): RenderTester - /** * Specifies that this render pass is expected to run a side effect with a key that satisfies * [matcher]. This expectation is strict, and will fail if multiple side effects match. @@ -490,11 +457,6 @@ interface RenderTester { /** * Specifies that this render pass is expected to run a particular worker. * - * @param doesSameWorkAs Worker passed to the actual worker's - * [doesSameWorkAs][Worker.doesSameWorkAs] method to identify the worker. Note that the actual - * method is called on the worker instance given by the workflow-under-test, and the value of this - * argument is passed to that method – if you need custom comparison logic for some reason, use - * the overload of this method that takes a `matchesWhen` parameter. * @param key The key passed to [runningWorker][com.squareup.workflow.RenderContext.runningWorker] * when rendering this workflow. * @param output If non-null, [WorkflowOutput.value] will be emitted when this worker is ran. @@ -503,19 +465,119 @@ interface RenderTester { * @param description Optional string that will be used to describe this expectation in error * messages. */ +// worker parameter is required for type inference. +@Suppress("UNUSED_PARAMETER") +@OptIn(ExperimentalStdlibApi::class, ExperimentalWorkflowApi::class) +/* ktlint-disable parameter-list-wrapping */ +inline fun , PropsT, StateT, OutputT, RenderingT> + RenderTester.expectWorker( + worker: W, + key: String = "", + crossinline matchesWhen: (W) -> Boolean = { true }, + output: WorkflowOutput? = null, + description: String = "" +): RenderTester = +/* ktlint-enable parameter-list-wrapping */ + expectWorker( + workerType = typeOf(), + key = key, + matchesWhen = { matchesWhen(it as W) }, + output = output, + description = description + ) + +/** + * Specifies that this render pass is expected to run a particular worker. + * + * @param key The key passed to [runningWorker][com.squareup.workflow.RenderContext.runningWorker] + * when rendering this workflow. + * @param output If non-null, [WorkflowOutput.value] will be emitted when this worker is ran. + * The [WorkflowAction] used to handle this output can be verified using methods on + * [RenderTestResult]. + * @param description Optional string that will be used to describe this expectation in error + * messages. + */ +@OptIn(ExperimentalStdlibApi::class, ExperimentalWorkflowApi::class) +/* ktlint-disable parameter-list-wrapping */ +fun + RenderTester.expectAnyWorkerOutputting( + outputType: KType, + key: String = "", + output: WorkflowOutput? = null, + description: String = "" +): RenderTester = + expectWorker(Worker::class, listOf(outputType), key, output = output, description = description) +/* ktlint-enable parameter-list-wrapping */ + +/** + * Specifies that this render pass is expected to run a particular worker. + * + * @param key The key passed to [runningWorker][com.squareup.workflow.RenderContext.runningWorker] + * when rendering this workflow. + * @param output If non-null, [WorkflowOutput.value] will be emitted when this worker is ran. + * The [WorkflowAction] used to handle this output can be verified using methods on + * [RenderTestResult]. + * @param description Optional string that will be used to describe this expectation in error + * messages. + */ +@OptIn(ExperimentalStdlibApi::class, ExperimentalWorkflowApi::class) +/* ktlint-disable parameter-list-wrapping */ +fun , PropsT, StateT, OutputT, RenderingT> + RenderTester.expectWorker( + workerClass: KClass, + typeArgs: List = emptyList(), + key: String = "", + matchesWhen: (W) -> Boolean = { true }, + output: WorkflowOutput? = null, + description: String = "" +): RenderTester +/* ktlint-enable parameter-list-wrapping */ { + val typeProjections = (typeArgs.asSequence() + generateSequence { Unit }.map { null }) + .zip(workerClass.typeParameters.asSequence()) { typeArg, typeParam -> + if (typeArg == null) STAR else KTypeProjection(typeParam.variance, typeArg) + } + .toList() + val workerType = workerClass.createType(typeProjections) + + @Suppress("UNCHECKED_CAST") + return expectWorker( + workerType = workerType, + key = key, + matchesWhen = { matchesWhen(it as W) }, + output = output, + description = description + ) +} + +/** + * Specifies that this render pass is expected to run a particular worker. + * + * @param workerType TODO + * @param key The key passed to [runningWorker][com.squareup.workflow.RenderContext.runningWorker] + * when rendering this workflow. + * @param output If non-null, [WorkflowOutput.value] will be emitted when this worker is ran. + * The [WorkflowAction] used to handle this output can be verified using methods on + * [RenderTestResult]. + * @param description Optional string that will be used to describe this expectation in error + * messages. + */ +@OptIn(ExperimentalStdlibApi::class, ExperimentalWorkflowApi::class) /* ktlint-disable parameter-list-wrapping */ fun RenderTester.expectWorker( - doesSameWorkAs: Worker<*>, + workerType: KType, key: String = "", + matchesWhen: (Worker<*>) -> Boolean = { true }, output: WorkflowOutput? = null, description: String = "" -): RenderTester = expectWorker( +): RenderTester = expectWorkflow( /* ktlint-enable parameter-list-wrapping */ - matchesWhen = { it.doesSameWorkAs(doesSameWorkAs) }, + identifier = unsnapshottableIdentifier(workerType), + rendering = Unit, key = key, + assertProps = { matchesWhen(it as Worker<*>) }, output = output, - description = description.ifBlank { doesSameWorkAs.toString() } + description = description.ifBlank { "worker $workerType" } ) /** diff --git a/workflow-testing/src/main/java/com/squareup/workflow/testing/WorkflowTestRuntime.kt b/workflow-testing/src/main/java/com/squareup/workflow/testing/WorkflowTestRuntime.kt index dd9d6d6865..dc9b930439 100644 --- a/workflow-testing/src/main/java/com/squareup/workflow/testing/WorkflowTestRuntime.kt +++ b/workflow-testing/src/main/java/com/squareup/workflow/testing/WorkflowTestRuntime.kt @@ -32,7 +32,6 @@ import com.squareup.workflow.testing.WorkflowTestParams.StartMode.StartFromCompl import com.squareup.workflow.testing.WorkflowTestParams.StartMode.StartFromState import com.squareup.workflow.testing.WorkflowTestParams.StartMode.StartFromWorkflowSnapshot import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineExceptionHandler import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers.Unconfined @@ -51,7 +50,6 @@ import kotlinx.coroutines.plus import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout import org.jetbrains.annotations.TestOnly -import kotlin.coroutines.ContinuationInterceptor import kotlin.coroutines.CoroutineContext import kotlin.coroutines.EmptyCoroutineContext @@ -379,7 +377,6 @@ fun workflow = this@launchForTestingWith, scope = workflowScope, props = propsFlow, - workerDispatcher = context[ContinuationInterceptor] as? CoroutineDispatcher, initialSnapshot = snapshot, interceptors = interceptors ) { output -> outputs.send(output) } diff --git a/workflow-testing/src/test/java/com/squareup/workflow/WorkerCompositionIntegrationTest.kt b/workflow-testing/src/test/java/com/squareup/workflow/WorkerCompositionIntegrationTest.kt index 438c7830d2..95f4285cae 100644 --- a/workflow-testing/src/test/java/com/squareup/workflow/WorkerCompositionIntegrationTest.kt +++ b/workflow-testing/src/test/java/com/squareup/workflow/WorkerCompositionIntegrationTest.kt @@ -20,7 +20,6 @@ package com.squareup.workflow import com.squareup.workflow.WorkflowAction.Companion.noAction import com.squareup.workflow.testing.WorkerSink import com.squareup.workflow.testing.launchForTestingFromStartWith -import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers.Unconfined import kotlinx.coroutines.Job @@ -66,7 +65,7 @@ class WorkerCompositionIntegrationTest { } } val workflow = Workflow.stateless { props -> - if (props) runningWorker(worker) { noAction() } + if (props) runningWorker(worker) } workflow.launchForTestingFromStartWith(true) { @@ -88,7 +87,7 @@ class WorkerCompositionIntegrationTest { stops++ } } - val workflow = Workflow.stateless { runningWorker(worker) { noAction() } } + val workflow = Workflow.stateless { runningWorker(worker) } workflow.launchForTestingFromStartWith { assertEquals(1, starts) @@ -117,7 +116,7 @@ class WorkerCompositionIntegrationTest { } } val workflow = Workflow.stateless { props -> - if (props) runningWorker(worker) { noAction() } + if (props) runningWorker(worker) } workflow.launchForTestingFromStartWith(false) { @@ -154,20 +153,14 @@ class WorkerCompositionIntegrationTest { } @Test fun `runningWorker gets error`() { - val channel = Channel() - val workflow = Workflow.stateless { - runningWorker( - channel.consumeAsFlow() - .asWorker() - ) { action { setOutput(it) } } + val workflow = Workflow.stateless { + runningWorker(Worker.createSideEffect { throw ExpectedException() }) } assertFailsWith { workflow.launchForTestingFromStartWith { assertFalse(this.hasOutput) - channel.cancel(CancellationException(null, ExpectedException())) - awaitNextOutput() } } diff --git a/workflow-testing/src/test/java/com/squareup/workflow/testing/RealRenderTesterTest.kt b/workflow-testing/src/test/java/com/squareup/workflow/testing/RealRenderTesterTest.kt index d00510367f..affeea65ff 100644 --- a/workflow-testing/src/test/java/com/squareup/workflow/testing/RealRenderTesterTest.kt +++ b/workflow-testing/src/test/java/com/squareup/workflow/testing/RealRenderTesterTest.kt @@ -89,7 +89,7 @@ class RealRenderTesterTest { } val tester = workflow.testRender(Unit) .expectWorkflow(child.identifier, rendering = Unit, output = WorkflowOutput(Unit)) - .expectWorker(matchesWhen = { true }, output = WorkflowOutput(Unit)) + .expectWorker(typeOf>(), output = WorkflowOutput(Unit)) val failure = assertFailsWith { tester.render() @@ -115,95 +115,17 @@ class RealRenderTesterTest { tester.expectWorkflow(workflow::class, rendering = Unit) } - @Test fun `expectWorker with output throws when already expecting worker output`() { - // Don't need an implementation, the test should fail before even calling render. - val workflow = Workflow.stateless {} - val tester = workflow.testRender(Unit) - .expectWorker(matchesWhen = { true }, output = WorkflowOutput(Unit)) - - val failure = assertFailsWith { - tester.expectWorker(matchesWhen = { true }, output = WorkflowOutput(Unit)) - } - - val failureMessage = failure.message!! - assertTrue(failureMessage.startsWith("Expected only one child to emit an output:")) - assertEquals( - 3, failureMessage.lines().size, - "Expected error message to have 3 lines, but was ${failureMessage.lines().size}" - ) - assertEquals(2, failureMessage.lines() - .count { "ExpectedWorker" in it }) - } - - @Test fun `renderChild throws when already expecting worker output`() { - val child = Workflow.stateless {} - val worker = Worker.finished() - val workflow = Workflow.stateless { - runningWorker(worker) { noAction() } - renderChild(child) { noAction() } - } - val tester = workflow.testRender(Unit) - .expectWorkflow(child.identifier, rendering = Unit, output = WorkflowOutput(Unit)) - .expectWorker(matchesWhen = { true }, output = WorkflowOutput(Unit)) - - val failure = assertFailsWith { - tester.render() - } - - assertEquals( - "Expected only one output to be expected: child workflow ${child.identifier} " + - "expected to emit kotlin.Unit but WorkflowAction.noAction() was already processed.", - failure.message - ) - } - - @Test fun `expectWorker without output doesn't throw when already expecting output`() { - // Don't need an implementation, the test should fail before even calling render. - val workflow = Workflow.stateless {} - val tester = workflow.testRender(Unit) - .expectWorker(matchesWhen = { true }, output = WorkflowOutput(Unit)) - - // Doesn't throw. - tester.expectWorker(matchesWhen = { true }) - } - - @Test fun `expectWorker taking Worker matches`() { - val worker = object : Worker { - override fun doesSameWorkAs(otherWorker: Worker<*>) = otherWorker === this - override fun run() = emptyFlow() - } - val workflow = Workflow.stateless { - runningWorker(worker) { noAction() } - } - - workflow.testRender(Unit) - .expectWorker(worker) - .render() - } - - @Test fun `expectWorker taking Worker doesn't match`() { - val worker1 = object : Worker { - override fun doesSameWorkAs(otherWorker: Worker<*>) = otherWorker === this - override fun toString(): String = "Worker1" - override fun run() = emptyFlow() - } - val worker2 = object : Worker { - override fun doesSameWorkAs(otherWorker: Worker<*>) = otherWorker === this - override fun toString(): String = "Worker2" - override fun run() = emptyFlow() - } + @Test fun `expectSideEffect throws when already expecting side effect`() { val workflow = Workflow.stateless { - runningWorker(worker1) { noAction() } + runningSideEffect("the key") {} } val tester = workflow.testRender(Unit) - .expectWorker(worker2) + .expectSideEffect("the key") val error = assertFailsWith { - tester.render() + tester.expectSideEffect("the key") } - assertTrue( - error.message!!.startsWith("Expected 1 more workflows, workers, or side effects to be ran:") - ) + assertEquals("Already expecting side effect with key \"the key\".", error.message) } @Test fun `sideEffect matches on key`() { @@ -277,7 +199,7 @@ class RealRenderTesterTest { ) workflow.testRender(Unit) - .expectWorker(matchesWhen = { true }, output = WorkflowOutput(Unit)) + .expectWorker(typeOf>(), output = WorkflowOutput(Unit)) .render { sink -> val error = assertFailsWith { sink.send(TestAction()) @@ -541,7 +463,7 @@ class RealRenderTesterTest { runningWorker(worker) } val tester = workflow.testRender(Unit) - .expectWorker(matchesWhen = { false }) + .expectWorker(typeOf>()) val error = assertFailsWith { tester.render() @@ -578,7 +500,7 @@ class RealRenderTesterTest { runningWorker(worker, key = "key") } val tester = workflow.testRender(Unit) - .expectWorker(matchesWhen = { true }) + .expectWorker(typeOf>()) val error = assertFailsWith { tester.render() @@ -600,7 +522,7 @@ class RealRenderTesterTest { } val tester = workflow.testRender(Unit) .expectWorker( - matchesWhen = { true }, + typeOf>(), key = "wrong key" ) @@ -622,8 +544,8 @@ class RealRenderTesterTest { runningWorker(worker) } val tester = workflow.testRender(Unit) - .expectWorker(matchesWhen = { true }) - .expectWorker(matchesWhen = { true }) + .expectWorker(worker) + .expectWorker(worker) val error = assertFailsWith { tester.render() @@ -658,8 +580,9 @@ class RealRenderTesterTest { // Do nothing. } val tester = workflow.testRender(Unit) - .expectWorker(matchesWhen = { true }) + .expectWorker(typeOf>()) + tester.render() val error = assertFailsWith { tester.render() } @@ -841,10 +764,7 @@ class RealRenderTesterTest { runningWorker(worker) { TestAction(it) } } val testResult = workflow.testRender(Unit) - .expectWorker( - matchesWhen = { true }, - output = WorkflowOutput("output") - ) + .expectWorker(worker, output = WorkflowOutput("output")) .render() testResult.verifyAction { diff --git a/workflow-testing/src/test/java/com/squareup/workflow/testing/WorkerRenderExpectationsTest.kt b/workflow-testing/src/test/java/com/squareup/workflow/testing/WorkerRenderExpectationsTest.kt new file mode 100644 index 0000000000..9324753a10 --- /dev/null +++ b/workflow-testing/src/test/java/com/squareup/workflow/testing/WorkerRenderExpectationsTest.kt @@ -0,0 +1,261 @@ +package com.squareup.workflow.testing + +import com.squareup.workflow.Worker +import com.squareup.workflow.Workflow +import com.squareup.workflow.WorkflowOutput +import com.squareup.workflow.action +import com.squareup.workflow.runningWorker +import com.squareup.workflow.stateless +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.emptyFlow +import kotlin.reflect.typeOf +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith + +// TODO add more failure tests +@OptIn(ExperimentalStdlibApi::class) +class WorkerRenderExpectationsTest { + + @Test fun `expectAnyWorkerOutputting() works`() { + val stringWorker = object : Worker { + override fun run(): Flow = emptyFlow() + } + val intWorker = object : Worker { + override fun run(): Flow = emptyFlow() + } + val workflow = Workflow.stateless { + runningWorker(stringWorker) { action { setOutput(it) } } + runningWorker(intWorker) { action { setOutput(it.toString()) } } + } + + // Exact string match + workflow.testRender(Unit) + .expectAnyWorkerOutputting(typeOf()) + .render() + workflow.testRender(Unit) + .expectAnyWorkerOutputting(typeOf(), output = WorkflowOutput("foo")) + .render() + .verifyActionResult { _, output -> assertEquals("foo", output?.value) } + + // Supertype match + workflow.testRender(Unit) + .expectAnyWorkerOutputting(typeOf()) + .render() + workflow.testRender(Unit) + .expectAnyWorkerOutputting(typeOf(), output = WorkflowOutput("foo")) + .render() + .verifyActionResult { _, output -> assertEquals("foo", output?.value) } + + // Other type match + workflow.testRender(Unit) + .expectAnyWorkerOutputting(typeOf()) + .render() + workflow.testRender(Unit) + .expectAnyWorkerOutputting(typeOf(), output = WorkflowOutput(42)) + .render() + .verifyActionResult { _, output -> assertEquals("42", output?.value) } + } + + @Test fun `expectWorker(worker) works`() { + val stringWorker = object : Worker { + override fun run(): Flow = emptyFlow() + } + val intWorker = object : Worker { + override fun run(): Flow = emptyFlow() + } + val workflow = Workflow.stateless { + runningWorker(stringWorker) { action { setOutput(it) } } + runningWorker(intWorker) { action { setOutput(it.toString()) } } + } + + // Exact string match + workflow.testRender(Unit) + .expectWorker(stringWorker) + .render() + workflow.testRender(Unit) + .expectWorker(stringWorker, output = WorkflowOutput("foo")) + .render() + .verifyActionResult { _, output -> assertEquals("foo", output?.value) } + + // Other type match + workflow.testRender(Unit) + .expectWorker(intWorker) + .render() + workflow.testRender(Unit) + .expectWorker(intWorker, output = WorkflowOutput(42)) + .render() + .verifyActionResult { _, output -> assertEquals("42", output?.value) } + } + + @Test fun `expectWorker(worker class, type args) works`() { + abstract class EmptyWorkerCovariant : Worker { + final override fun run(): Flow = emptyFlow() + } + + abstract class EmptyWorker : EmptyWorkerCovariant() + + class EmptyStringWorker : EmptyWorker() + class EmptyIntWorker : EmptyWorker() + + val workflow = Workflow.stateless { + runningWorker(EmptyStringWorker()) { action { setOutput(it) } } + runningWorker(EmptyIntWorker()) { action { setOutput(it.toString()) } } + } + + // Exact string match + workflow.testRender(Unit) + .expectWorker(EmptyStringWorker::class) + .render() + workflow.testRender(Unit) + .expectWorker(EmptyStringWorker::class, output = WorkflowOutput("foo")) + .render() + .verifyActionResult { _, output -> assertEquals("foo", output?.value) } + + // Supertype match without type args + workflow.testRender(Unit) + .expectWorker(EmptyWorker::class) + .render() + workflow.testRender(Unit) + .expectWorker(EmptyWorker::class, output = WorkflowOutput("foo")) + .render() + .verifyActionResult { _, output -> assertEquals("foo", output?.value) } + + // Invariant supertype match with exact type args + workflow.testRender(Unit) + .expectWorker(EmptyWorker::class, listOf(typeOf())) + .render() + workflow.testRender(Unit) + .expectWorker(EmptyWorker::class, listOf(typeOf()), output = WorkflowOutput("foo")) + .render() + .verifyActionResult { _, output -> assertEquals("foo", output?.value) } + + // Invariant supertype match with supertype args + workflow.testRender(Unit) + .expectWorker(EmptyWorker::class, listOf(typeOf())) + .expectWorker(EmptyIntWorker::class) + .disallowUnexpectedChildren() + .apply { + val error = assertFailsWith { render() } + assertEquals( + "Tried to render unexpected child worker ${typeOf()}", + error.message + ) + } + + // Covariant supertype match with exact type args + workflow.testRender(Unit) + .expectWorker(EmptyWorkerCovariant::class, listOf(typeOf())) + .render() + workflow.testRender(Unit) + .expectWorker( + EmptyWorkerCovariant::class, listOf(typeOf()), output = WorkflowOutput("foo") + ) + .render() + .verifyActionResult { _, output -> assertEquals("foo", output?.value) } + + // Covariant supertype match with supertype args + workflow.testRender(Unit) + .expectWorker(EmptyWorkerCovariant::class, listOf(typeOf())) + .render() + workflow.testRender(Unit) + .expectWorker( + EmptyWorkerCovariant::class, listOf(typeOf()), + output = WorkflowOutput("foo") + ) + .render() + .verifyActionResult { _, output -> assertEquals("foo", output?.value) } + + // Other type match + workflow.testRender(Unit) + .expectWorker(EmptyIntWorker::class) + .render() + workflow.testRender(Unit) + .expectWorker(EmptyIntWorker::class, output = WorkflowOutput(42)) + .render() + .verifyActionResult { _, output -> assertEquals("42", output?.value) } + } + + @Test fun `expectWorker(worker KType) works`() { + abstract class EmptyWorkerCovariant : Worker { + final override fun run(): Flow = emptyFlow() + } + + abstract class EmptyWorker : EmptyWorkerCovariant() + + class EmptyStringWorker : EmptyWorker() + class EmptyIntWorker : EmptyWorker() + + val workflow = Workflow.stateless { + runningWorker(EmptyStringWorker()) { action { setOutput(it) } } + runningWorker(EmptyIntWorker()) { action { setOutput(it.toString()) } } + } + + // Exact string match + workflow.testRender(Unit) + .expectWorker(typeOf()) + .render() + workflow.testRender(Unit) + .expectWorker(typeOf(), output = WorkflowOutput("foo")) + .render() + .verifyActionResult { _, output -> assertEquals("foo", output?.value) } + + // Supertype match without type args + workflow.testRender(Unit) + .expectWorker(typeOf>()) + .render() + workflow.testRender(Unit) + .expectWorker(typeOf>(), output = WorkflowOutput("foo")) + .render() + .verifyActionResult { _, output -> assertEquals("foo", output?.value) } + + // Invariant supertype match with exact type args + workflow.testRender(Unit) + .expectWorker(typeOf>()) + .render() + workflow.testRender(Unit) + .expectWorker(typeOf>(), output = WorkflowOutput("foo")) + .render() + .verifyActionResult { _, output -> assertEquals("foo", output?.value) } + + // Invariant supertype match with supertype args + workflow.testRender(Unit) + .expectWorker(typeOf>()) + .expectWorker(EmptyIntWorker::class) + .disallowUnexpectedChildren() + .apply { + val error = assertFailsWith { render() } + assertEquals( + "Tried to render unexpected child worker ${typeOf()}", + error.message + ) + } + + // Covariant supertype match with exact type args + workflow.testRender(Unit) + .expectWorker(typeOf>()) + .render() + workflow.testRender(Unit) + .expectWorker(typeOf>(), output = WorkflowOutput("foo")) + .render() + .verifyActionResult { _, output -> assertEquals("foo", output?.value) } + + // Covariant supertype match with supertype args + workflow.testRender(Unit) + .expectWorker(typeOf>()) + .render() + workflow.testRender(Unit) + .expectWorker(typeOf>(), output = WorkflowOutput("foo")) + .render() + .verifyActionResult { _, output -> assertEquals("foo", output?.value) } + + // Other type match + workflow.testRender(Unit) + .expectWorker(typeOf()) + .render() + workflow.testRender(Unit) + .expectWorker(typeOf(), output = WorkflowOutput(42)) + .render() + .verifyActionResult { _, output -> assertEquals("42", output?.value) } + } +} diff --git a/workflow-testing/src/test/java/com/squareup/workflow/testing/WorkerSinkTest.kt b/workflow-testing/src/test/java/com/squareup/workflow/testing/WorkerSinkTest.kt index 741a38eaee..f23ddec3f1 100644 --- a/workflow-testing/src/test/java/com/squareup/workflow/testing/WorkerSinkTest.kt +++ b/workflow-testing/src/test/java/com/squareup/workflow/testing/WorkerSinkTest.kt @@ -15,17 +15,22 @@ */ package com.squareup.workflow.testing +import com.squareup.workflow.Worker import kotlinx.coroutines.CoroutineStart.UNDISPATCHED import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.async import kotlinx.coroutines.cancelChildren +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.take import kotlinx.coroutines.flow.toList import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking import kotlinx.coroutines.yield +import kotlin.reflect.full.isSupertypeOf +import kotlin.reflect.typeOf import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith @@ -34,6 +39,26 @@ import kotlin.test.assertTrue class WorkerSinkTest { + @OptIn(ExperimentalStdlibApi::class) + @Test fun types() { + abstract class IntermediateWorker : Worker + class MyWorker : IntermediateWorker() { + override fun run(): Flow = emptyFlow() + } + + assertTrue(typeOf>().isSupertypeOf(typeOf())) + assertTrue(typeOf>().isSupertypeOf(typeOf())) + assertTrue(typeOf>().isSupertypeOf(typeOf())) + + assertTrue(typeOf>().isSupertypeOf(typeOf>())) + assertTrue(typeOf>().isSupertypeOf(typeOf>())) + assertTrue(typeOf>().isSupertypeOf(typeOf>())) + + assertTrue(typeOf>().isSupertypeOf(typeOf())) + assertTrue(typeOf>().isSupertypeOf(typeOf())) + assertTrue(typeOf>().isSupertypeOf(typeOf())) + } + @Test fun `workers are equivalent with matching type and name`() { val fooWorker = WorkerSink("foo") val otherFooWorker = WorkerSink("foo") diff --git a/workflow-testing/src/test/java/com/squareup/workflow/testing/WorkflowTestRuntimeTest.kt b/workflow-testing/src/test/java/com/squareup/workflow/testing/WorkflowTestRuntimeTest.kt index f3d722c224..c6abcb508e 100644 --- a/workflow-testing/src/test/java/com/squareup/workflow/testing/WorkflowTestRuntimeTest.kt +++ b/workflow-testing/src/test/java/com/squareup/workflow/testing/WorkflowTestRuntimeTest.kt @@ -21,6 +21,7 @@ import com.squareup.workflow.Snapshot import com.squareup.workflow.Worker import com.squareup.workflow.Workflow import com.squareup.workflow.internal.util.rethrowingUncaughtExceptions +import com.squareup.workflow.runningWorker import com.squareup.workflow.stateful import com.squareup.workflow.stateless import com.squareup.workflow.testing.WorkflowTestParams.StartMode.StartFromWorkflowSnapshot diff --git a/workflow-tracing/src/main/java/com/squareup/workflow/diagnostic/tracing/TracingWorkflowInterceptor.kt b/workflow-tracing/src/main/java/com/squareup/workflow/diagnostic/tracing/TracingWorkflowInterceptor.kt index 548fff5eaf..4946745470 100644 --- a/workflow-tracing/src/main/java/com/squareup/workflow/diagnostic/tracing/TracingWorkflowInterceptor.kt +++ b/workflow-tracing/src/main/java/com/squareup/workflow/diagnostic/tracing/TracingWorkflowInterceptor.kt @@ -32,7 +32,6 @@ import com.squareup.workflow.ExperimentalWorkflowApi import com.squareup.workflow.RenderContext import com.squareup.workflow.Sink import com.squareup.workflow.Snapshot -import com.squareup.workflow.Worker import com.squareup.workflow.WorkflowAction import com.squareup.workflow.WorkflowAction.Updater import com.squareup.workflow.WorkflowInterceptor @@ -41,9 +40,6 @@ import com.squareup.workflow.WorkflowOutput import com.squareup.workflow.applyTo import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Job -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.collect -import kotlinx.coroutines.flow.flow import okio.buffer import okio.sink import java.io.File @@ -141,12 +137,6 @@ class TracingWorkflowInterceptor internal constructor( private var gcDetector: GcDetector? = null private val workflowNamesById = mutableMapOf() - private val workerDescriptionsById = mutableMapOf() - - private var workerIdCounter = 1 - - /** Used to look up workers to perform doesSameWorkAs checks until Workers go away. */ - private val workersById = mutableMapOf>() override fun onSessionStarted( workflowScope: CoroutineScope, @@ -179,7 +169,7 @@ class TracingWorkflowInterceptor internal constructor( onWorkflowStarted( workflowId = session.sessionId, parentId = session.parent?.sessionId, - workflowType = session.identifier.toString(), + workflowType = session.identifier.let { it.describeRealIdentifier() ?: it.toString() }, key = session.renderKey, initialProps = props, initialState = initialState, @@ -364,41 +354,6 @@ class TracingWorkflowInterceptor internal constructor( workflowNamesById -= workflowId } - private fun onWorkerStarted( - workerId: Long, - parentWorkflowId: Long, - key: String, - description: String - ) { - val parentName = workflowNamesById.getValue(parentWorkflowId) - workerDescriptionsById[workerId] = description - logger?.log( - AsyncDurationBegin( - id = "workflow", - name = "Worker: ${workerId.toHex()}", - category = "workflow", - args = mapOf( - "parent" to parentName, - "key" to key, - "description" to description - ) - ) - ) - } - - private fun onWorkerStopped(workerId: Long) { - val description = workerDescriptionsById.getValue(workerId) - logger?.log( - AsyncDurationEnd( - id = "workflow", - name = "Worker: ${workerId.toHex()}", - category = "workflow", - args = mapOf("description" to description) - ) - ) - workerDescriptionsById -= workerId - } - private fun onBeforeWorkflowRendered( workflowId: Long, props: Any?, @@ -454,26 +409,6 @@ class TracingWorkflowInterceptor internal constructor( ) } - private fun onWorkerOutput( - workerId: Long, - parentWorkflowId: Long, - output: Any - ) { - val parentName = workflowNamesById.getValue(parentWorkflowId) - val description = workerDescriptionsById.getValue(workerId) - logger?.log( - Instant( - name = "Worker output: $parentName", - category = "update", - args = mapOf( - "workerId" to workerId.toHex(), - "description" to description, - "output" to output.toString() - ) - ) - ) - } - private fun onPropsChanged( workflowId: Long?, oldProps: Any?, @@ -540,15 +475,6 @@ class TracingWorkflowInterceptor internal constructor( ) } - /** - * Creates a unique ID for a worker by incrementing the worker counter int, then storing the - * previous counter value in the most-significant half of the workflow ID's bits (since those - * bits are unused by workflow IDs in the real world). - */ - private fun createWorkerId(session: WorkflowSession) = - workerIdCounter++.toLong() - .shl(32) xor session.sessionId - private inner class TracingRenderContext( private val delegate: RenderContext, private val session: WorkflowSession @@ -560,17 +486,6 @@ class TracingWorkflowInterceptor internal constructor( val wrapperAction = TracingAction(value, session) delegate.actionSink.send(wrapperAction) } - - override fun runningWorker( - worker: Worker, - key: String, - handler: (T) -> WorkflowAction - ) { - val wrappedWorker = TracingWorker(session, worker, key) - delegate.runningWorker(wrappedWorker, key) { output -> - TracingAction(handler(output), session) - } - } } private inner class TracingAction( @@ -591,42 +506,6 @@ class TracingWorkflowInterceptor internal constructor( ) } } - - private inner class TracingWorker( - private val session: WorkflowSession, - private val delegate: Worker, - private val key: String - ) : Worker { - - val workerId: Long - - init { - val existingWorker = workersById.filterValues { it.doesSameWorkAs(this) } - .entries - .singleOrNull() - workerId = existingWorker?.key ?: (createWorkerId(session).also { workerId -> - onWorkerStarted(workerId, session.sessionId, key, delegate.toString()) - workersById[workerId] = this - }) - } - - override fun doesSameWorkAs(otherWorker: Worker<*>): Boolean = - otherWorker is TracingWorker<*> && - session == otherWorker.session && - delegate.doesSameWorkAs(otherWorker.delegate) - - override fun run(): Flow = flow { - try { - delegate.run() - .collect { output -> - onWorkerOutput(workerId, session.sessionId, output ?: "{null}") - emit(output) - } - } finally { - onWorkerStopped(workerId) - } - } - } } @Suppress("NOTHING_TO_INLINE") diff --git a/workflow-tracing/src/test/java/com/squareup/workflow/diagnostic/tracing/TracingWorkflowInterceptorTest.kt b/workflow-tracing/src/test/java/com/squareup/workflow/diagnostic/tracing/TracingWorkflowInterceptorTest.kt index 982f14559f..bdb09572f8 100644 --- a/workflow-tracing/src/test/java/com/squareup/workflow/diagnostic/tracing/TracingWorkflowInterceptorTest.kt +++ b/workflow-tracing/src/test/java/com/squareup/workflow/diagnostic/tracing/TracingWorkflowInterceptorTest.kt @@ -23,6 +23,7 @@ import com.squareup.workflow.StatefulWorkflow import com.squareup.workflow.action import com.squareup.workflow.asWorker import com.squareup.workflow.renderWorkflowIn +import com.squareup.workflow.runningWorker import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers.Unconfined