WIP Reimplemented Workers using side effects.

This fixes #82, which is the Kotlin half of square/workflow#1021.

Also fixes square/workflow#1197.

Author: zach-klippenstein

workflow-core/api/workflow-core.api

@@ -17,6 +17,7 @@ public abstract interface class com/squareup/workflow/ImpostorWorkflow {
 public abstract class com/squareup/workflow/LifecycleWorker : com/squareup/workflow/Worker {
 	public fun <init> ()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;
@@ -35,6 +36,7 @@ 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 fun runningWorker (Lcom/squareup/workflow/RenderContext;Lcom/squareup/workflow/Worker;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;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)V
 }
 
@@ -103,13 +105,15 @@ public abstract class com/squareup/workflow/StatelessWorkflow : com/squareup/wor
 public final class com/squareup/workflow/TypedWorker : com/squareup/workflow/Worker {
 	public fun <init> (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;
 }
 
 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;
 }
 
@@ -122,6 +126,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 {

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<Nothing> {
 
+  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<Nothing> {
   /**
    * 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
 }

workflow-core/src/main/java/com/squareup/workflow/RenderContext.kt

@@ -120,7 +120,10 @@ interface RenderContext<out PropsT, StateT, in OutputT> {
     worker: Worker<T>,
     key: String = "",
     handler: (T) -> WorkflowAction<PropsT, StateT, OutputT>
-  )
+  ) {
+    val workerWorkflow = WorkerWorkflow<T>(WorkerKType(worker), key)
+    renderChild(workerWorkflow, props = worker, key = key, handler = handler)
+  }
 
   /**
    * Ensures [sideEffect] is running with the given [key].

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<out OutputT> {
 
+  /**
+   * 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<out OutputT> {
    * 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<out OutputT> {
    * }
    * ```
    */
-  fun doesSameWorkAs(otherWorker: Worker<*>): Boolean = otherWorker::class == this::class
+  fun doesSameWorkAs(otherWorker: Worker<*>): Boolean = true
 
   companion object {
 
@@ -384,23 +396,23 @@ fun <T, R> Worker<T>.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<OutputT>(
-  private val type: KType,
+  override val outputType: KType,
   private val work: Flow<OutputT>
 ) : Worker<OutputT> {
 
   override fun run(): Flow<OutputT> = work
 
   override fun doesSameWorkAs(otherWorker: Worker<*>): Boolean =
-    otherWorker is TypedWorker && otherWorker.type == type
+    otherWorker is TypedWorker && otherWorker.outputType == outputType
 
-  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<Unit> {
@@ -412,17 +424,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<Nothing> {
   override fun run(): Flow<Nothing> = emptyFlow()
-  override fun doesSameWorkAs(otherWorker: Worker<*>): Boolean = otherWorker === FinishedWorker
   override fun toString(): String = "FinishedWorker"
 }
 
-private class WorkerWrapper<T, R>(
+private data class WorkerWrapper<T, R>(
   private val wrapped: Worker<T>,
   private val flow: Flow<R>
 ) : Worker<R> {

workflow-core/src/main/java/com/squareup/workflow/WorkerKType.kt

@@ -0,0 +1,44 @@
+/*
+ * 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 kotlin.reflect.KClassifier
+import kotlin.reflect.KType
+import kotlin.reflect.KTypeProjection
+import kotlin.reflect.KVariance.OUT
+
+/**
+ * An implementation of [KType] whose [classifier][KType.classifier] is the `KClass` of the worker
+ * itself, and which has a type parameter of the [Worker.outputType] of the worker.
+ */
+internal data class WorkerKType(
+  override val classifier: KClassifier,
+  val outputTypeProjection: KTypeProjection
+) : KType {
+  constructor(worker: Worker<*>) : this(worker::class, worker.outputTypeProjection)
+
+  override val arguments: List<KTypeProjection> get() = listOf(outputTypeProjection)
+  override val annotations: List<Annotation> get() = emptyList()
+  override val isMarkedNullable: Boolean get() = false
+
+  override fun toString(): String = "Worker<${outputTypeProjection.type}>"
+}
+
+private val Worker<*>.outputTypeProjection: KTypeProjection
+  get() = outputType?.let { KTypeProjection(OUT, it) } ?: KTypeProjection(null, null)

workflow-core/src/main/java/com/squareup/workflow/WorkerWorkflow.kt

@@ -0,0 +1,99 @@
+/*
+ * 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
+
+/**
+ * TODO write documentation
+ */
+@OptIn(ExperimentalWorkflowApi::class)
+internal class WorkerWorkflow<OutputT>(
+  workerType: WorkerKType,
+  private val key: String
+) :
+    StatefulWorkflow<Worker<OutputT>, Int, OutputT, Unit>(),
+    ImpostorWorkflow {
+
+  override val realIdentifier: WorkflowIdentifier = unsnapshottableIdentifier(workerType)
+
+  override fun initialState(
+    props: Worker<OutputT>,
+    snapshot: Snapshot?
+  ): Int = 0
+
+  override fun onPropsChanged(
+    old: Worker<OutputT>,
+    new: Worker<OutputT>,
+    state: Int
+  ): Int = if (!old.doesSameWorkAs(new)) state + 1 else state
+
+  override fun render(
+    props: Worker<OutputT>,
+    state: Int,
+    context: RenderContext<Worker<OutputT>, 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 <OutputT> runWorker(
+  worker: Worker<OutputT>,
+  renderKey: String,
+  actionSink: Sink<WorkflowAction<Worker<OutputT>, 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 <T> Worker<T>.runWithNullCheck(): Flow<T> =
+  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" }

workflow-core/src/test/java/com/squareup/workflow/NullFlowWorker.java

@@ -0,0 +1,51 @@
+/*
+ * 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;
+
+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
+ * mocking libraries.
+ *
+ * See <a href="https://github.com/square/workflow/issues/842">#842</a>.
+ */
+class NullFlowWorker implements Worker {
+
+  @Nullable @Override public KType getOutputType() {
+    return null;
+  }
+
+  @NotNull @Override public Flow run() {
+    //noinspection ConstantConditions
+    return null;
+  }
+
+  @Override public boolean doesSameWorkAs(@NotNull Worker otherWorker) {
+    //noinspection unchecked
+    return Worker.DefaultImpls.doesSameWorkAs(this, otherWorker);
+  }
+
+  /**
+   * Override this to make writing assertions on exception messages easier.
+   */
+  @Override public String toString() {
+    return "NullFlowWorker.toString";
+  }
+}