Merge pull request #1139 from square/sedwards/session-docs

Undeprecate runningWorker for LifecyleWorker; Docs for SessionWorkflow

Author: steve-the-edwards

workflow-core/src/commonMain/kotlin/com/squareup/workflow1/BaseRenderContext.kt

@@ -308,6 +308,28 @@ public fun <PropsT, StateT, OutputT, ChildRenderingT>
     key: String = ""
   ): ChildRenderingT = renderChild(child, Unit, key) { noAction() }
 
+/**
+ * Ensures a [LifecycleWorker] is running. Since [worker] can't emit anything,
+ * it can't trigger any [WorkflowAction]s.
+ *
+ * You may want to consider using [SessionWorkflow]. See note on [LifecycleWorker] and the docs
+ * for [SessionWorkflow].
+ *
+ * @param key An optional string key that is used to distinguish between identical [Worker]s.
+ */
+public inline fun <reified W : LifecycleWorker, PropsT, StateT, OutputT>
+  BaseRenderContext<PropsT, StateT, OutputT>.runningWorker(
+    worker: W,
+    key: String = ""
+  ) {
+  runningWorker(worker, key) {
+    // The compiler thinks this code is unreachable, and it is correct. But we have to pass a lambda
+    // here so we might as well check at runtime as well.
+    @Suppress("UNREACHABLE_CODE")
+    throw AssertionError("Worker<Nothing> emitted $it")
+  }
+}
+
 /**
  * Ensures a [Worker] that never emits anything is running. Since [worker] can't emit anything,
  * it can't trigger any [WorkflowAction]s.

workflow-core/src/commonMain/kotlin/com/squareup/workflow1/LifecycleWorker.kt

@@ -18,6 +18,11 @@ import kotlin.jvm.JvmName
  * Note that there is currently an [issue](https://github.com/square/workflow-kotlin/issues/1093)
  * which can effect whether a [LifecycleWorker] is ever executed.
  * See more details at [BaseRenderContext.runningSideEffect].
+ *
+ * Also note that [LifecycleWorker] is inherently racy with other Workers. There is no guarantee
+ * this will run first or last compared to other workers and side effects. Ideally setup and
+ * teardown is handled by each Worker or sideEffect itself. Consider using a try { } finally { }
+ * or [Flow.onCompletion][kotlinx.coroutines.flow.onCompletion] to handle that.
  */
 public abstract class LifecycleWorker : Worker<Nothing> {
 

workflow-core/src/commonMain/kotlin/com/squareup/workflow1/SessionWorkflow.kt

@@ -35,8 +35,6 @@ public abstract class SessionWorkflow<
    *
    * This [CoroutineScope] can be used to:
    *
-   *  - set reliable teardown hooks, e.g. via [Job.invokeOnCompletion][kotlinx.coroutines.Job.invokeOnCompletion].
-   *
    *  -  own the transforms on a [StateFlow][kotlinx.coroutines.flow.StateFlow],
    *     linking them to the lifetime of a Workflow session. For example,
    *     here is how you might safely combine two `StateFlow`s:
@@ -60,6 +58,20 @@ public abstract class SessionWorkflow<
    *       )
    *     }
    *
+   *   - set reliable teardown hooks, e.g. via [Job.invokeOnCompletion][kotlinx.coroutines.Job.invokeOnCompletion].
+   *     Note however, that while these are reliable in the sense of being guaranteed to be executed
+   *     regardless of the lifetime of this workflow session, they are not reliable in that a
+   *     completion handler on the Job is not thread-safe and will be executed on whatever the last
+   *     dispatcher was used when the Job completes. See more on the [Job.invokeOnCompletion][kotlinx.coroutines.Job.invokeOnCompletion]
+   *     kdoc.
+   *
+   *     So what do you do? Well, cleanup and lifecycle matters should be handled by each individual
+   *     Worker and sideEffect. Consider using a try { } finally { cleanup() }
+   *     or [Flow.onCompletion][kotlinx.coroutines.flow.onCompletion] to handle that.
+   *
+   *     If you have a general cleanup operation that is fast and thread-safe then you could use
+   *     [Job.invokeOnCompletion][kotlinx.coroutines.Job.invokeOnCompletion].
+   *
    * **Note Carefully**: Neither [workflowScope] nor any of these transformed/computed dependencies
    * should be stored by this Workflow instance. This could be re-created, or re-used unexpectedly
    * and should not have its own state. Instead, the transformed/computed dependencies must be

workflow-testing/src/main/java/com/squareup/workflow1/testing/RenderTesterWorkers.kt

@@ -27,7 +27,6 @@ import kotlin.reflect.typeOf
  * @param description Optional string that will be used to describe this expectation in error
  * messages.
  */
-@OptIn(ExperimentalStdlibApi::class)
 public inline fun <PropsT, StateT, OutputT, RenderingT>
   RenderTester<PropsT, StateT, OutputT, RenderingT>.expectWorkerOutputting(
     outputType: KType,
@@ -60,7 +59,6 @@ public inline fun <PropsT, StateT, OutputT, RenderingT>
  * @param description Optional string that will be used to describe this expectation in error
  * messages.
  */
-@OptIn(ExperimentalStdlibApi::class)
 public inline fun <
   PropsT,
   StateT,
@@ -149,7 +147,6 @@ public inline fun <
  * @param description Optional string that will be used to describe this expectation in error
  * messages.
  */
-@OptIn(ExperimentalStdlibApi::class)
 public fun <PropsT, StateT, OutputT, RenderingT>
   RenderTester<PropsT, StateT, OutputT, RenderingT>.expectWorker(
     workerType: KType,

workflow-testing/src/test/java/com/squareup/workflow1/testing/RealRenderTesterTest.kt

@@ -729,9 +729,6 @@ internal class RealRenderTesterTest {
     val stringWorker: Worker<String> = emptyFlow<String>().asWorker()
 
     val workflow = Workflow.stateless<Unit, Nothing, Unit> {
-      // Suppress usage as we are testing a comparisons of unique workers
-      // even though they have the same key.
-      @Suppress("DEPRECATION")
       runningWorker(lifecycleWorker)
       runningWorker(stringWorker) { noAction() }
     }
@@ -745,7 +742,6 @@ internal class RealRenderTesterTest {
 
   // Suppress runningWorker in this test as we are testing the
   // uniqueness of workers using similar objects as keys
-  @Suppress("DEPRECATION")
   @Test
   fun `runningWorker distinguishes between specific Nothing workers`() {
     val workerA = object : LifecycleWorker() {}