Merge pull request #1232 from square/sedwards/action-naming

steve-the-edwards · web-flow · commit e370cbb32a15 · 2024-11-14T16:52:45.000-05:00
Add public `debuggingName` to `WorkflowAction`, use it in default `toString()`
diff --git a/workflow-core/api/workflow-core.api b/workflow-core/api/workflow-core.api
@@ -330,6 +330,8 @@ public abstract class com/squareup/workflow1/WorkflowAction {
 	public static final field Companion Lcom/squareup/workflow1/WorkflowAction$Companion;
 	public fun <init> ()V
 	public abstract fun apply (Lcom/squareup/workflow1/WorkflowAction$Updater;)V
+	public fun getDebuggingName ()Ljava/lang/String;
+	public fun toString ()Ljava/lang/String;
 }
 
 public final class com/squareup/workflow1/WorkflowAction$Companion {
diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/Sink.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/Sink.kt
@@ -4,7 +4,6 @@
 package com.squareup.workflow1
 
 import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.collect
 import kotlinx.coroutines.suspendCancellableCoroutine
 import kotlin.coroutines.resume
 import kotlin.jvm.JvmMultifileClass
@@ -87,6 +86,7 @@ internal suspend fun <
   suspendCancellableCoroutine<Unit> { continuation ->
     val resumingAction = object : WorkflowAction<PropsT, StateT, OutputT>() {
       override fun toString(): String = "sendAndAwaitApplication($action)"
+
       override fun Updater.apply() {
         // Don't execute anything if the caller was cancelled while we were in the queue.
         if (!continuation.isActive) return
diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatefulWorkflow.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatefulWorkflow.kt
@@ -998,15 +998,18 @@ public fun <PropsT, StateT, OutputT, RenderingT>
  * of the receiving [StatefulWorkflow]. The action will invoke the given [lambda][update]
  * when it is [applied][WorkflowAction.apply].
  *
- * @param name Function that returns a string describing the update for debugging, included
- * in [toString].
+ * @param name Function that returns a string describing the update for debugging, this will
+ *  be returned by [WorkflowAction.debuggingName], which is in turn included in the default
+ *  [WorkflowAction.toString].
  * @param update Function that defines the workflow update.
  */
 public fun <PropsT, StateT, OutputT, RenderingT>
   StatefulWorkflow<PropsT, StateT, OutputT, RenderingT>.action(
     name: () -> String,
     update: WorkflowAction<PropsT, StateT, OutputT>.Updater.() -> Unit
   ): WorkflowAction<PropsT, StateT, OutputT> = object : WorkflowAction<PropsT, StateT, OutputT>() {
+  override val debuggingName: String
+    get() = name()
+
   override fun Updater.apply() = update.invoke(this)
-  override fun toString(): String = "action(${name()})-${this@action}"
 }
diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatelessWorkflow.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/StatelessWorkflow.kt
@@ -141,15 +141,18 @@ public fun <PropsT, OutputT, RenderingT>
  * of the receiving [StatefulWorkflow]. The action will invoke the given [lambda][update]
  * when it is [applied][WorkflowAction.apply].
  *
- * @param name Function that returns a string describing the update for debugging, included in
- * [toString].
+ * @param name Function that returns a string describing the update for debugging, this will
+ *  be returned by [WorkflowAction.debuggingName], which is in turn included in the default
+ *  [WorkflowAction.toString].
  * @param update Function that defines the workflow update.
  */
 public fun <PropsT, OutputT, RenderingT>
   StatelessWorkflow<PropsT, OutputT, RenderingT>.action(
     name: () -> String,
     update: WorkflowAction<PropsT, *, OutputT>.Updater.() -> Unit
   ): WorkflowAction<PropsT, Nothing, OutputT> = object : WorkflowAction<PropsT, Nothing, OutputT>() {
+  override val debuggingName: String
+    get() = name()
+
   override fun Updater.apply() = update.invoke(this)
-  override fun toString(): String = "action(${name()})-${this@action}"
 }
diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkerWorkflow.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkerWorkflow.kt
@@ -88,8 +88,8 @@ private class EmitWorkerOutputAction<P, S, O>(
   private val renderKey: String,
   private val output: O,
 ) : WorkflowAction<P, S, O>() {
-  override fun toString(): String =
-    WorkflowIdentifierTypeNamer.uniqueName(EmitWorkerOutputAction::class) +
+  override val debuggingName: String
+    get() = CommonKClassTypeNamer.uniqueName(EmitWorkerOutputAction::class) +
       "(worker=$worker, key=$renderKey)"
 
   override fun Updater.apply() {
diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkflowAction.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkflowAction.kt
@@ -31,6 +31,12 @@ import kotlin.jvm.JvmOverloads
  */
 public abstract class WorkflowAction<in PropsT, StateT, out OutputT> {
 
+  /**
+   * The name to use for debugging. This is handy for logging and is used by the default
+   * [toString] implementation provided here.
+   */
+  public open val debuggingName: String = CommonKClassTypeNamer.uniqueName(this::class)
+
   /**
    * The context for calls to [WorkflowAction.apply]. Allows the action to read and change the
    * [state], and to emit an [output][setOutput] value.
@@ -60,6 +66,8 @@ public abstract class WorkflowAction<in PropsT, StateT, out OutputT> {
    */
   public abstract fun Updater.apply()
 
+  public override fun toString(): String = "action($debuggingName)-@${hashCode()}"
+
   public companion object {
     /**
      * Returns a [WorkflowAction] that does nothing: no output will be emitted, and
@@ -72,7 +80,7 @@ public abstract class WorkflowAction<in PropsT, StateT, out OutputT> {
       NO_ACTION as WorkflowAction<Any?, StateT, OutputT>
 
     private val NO_ACTION = object : WorkflowAction<Any?, Any?, Any?>() {
-      override fun toString(): String = "WorkflowAction.noAction()"
+      override val debuggingName: String = "noAction()"
 
       override fun Updater.apply() {
         // Noop
@@ -124,9 +132,10 @@ public fun <PropsT, StateT, OutputT> action(
   name: () -> String,
   apply: WorkflowAction<PropsT, StateT, OutputT>.Updater.() -> Unit
 ): WorkflowAction<PropsT, StateT, OutputT> = object : WorkflowAction<PropsT, StateT, OutputT>() {
-  override fun Updater.apply() = apply.invoke(this)
+  override val debuggingName: String
+    get() = name()
 
-  override fun toString(): String = "WorkflowAction(${name()})@${hashCode()}"
+  override fun Updater.apply() = apply.invoke(this)
 }
 
 /** Applies this [WorkflowAction] to [state]. */
diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkflowIdentifierType.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkflowIdentifierType.kt
@@ -24,7 +24,7 @@ public sealed class WorkflowIdentifierType {
     val kClass: KClass<*>? = null,
   ) : WorkflowIdentifierType() {
     public constructor(kClass: KClass<*>) : this(
-      WorkflowIdentifierTypeNamer.uniqueName(kClass),
+      CommonKClassTypeNamer.uniqueName(kClass),
       kClass
     )
   }
@@ -46,6 +46,6 @@ public sealed class WorkflowIdentifierType {
   }
 }
 
-internal expect object WorkflowIdentifierTypeNamer {
+internal expect object CommonKClassTypeNamer {
   public fun uniqueName(kClass: KClass<*>): String
 }
diff --git a/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/WorkflowIdentifierTest.kt b/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/WorkflowIdentifierTest.kt
@@ -186,7 +186,7 @@ internal class WorkflowIdentifierTest {
   ) : Workflow<Nothing, Nothing, Nothing>, ImpostorWorkflow {
     override val realIdentifier: WorkflowIdentifier = proxied.identifier
     override fun describeRealIdentifier(): String =
-      "TestImpostor1(${WorkflowIdentifierTypeNamer.uniqueName(proxied::class)})"
+      "TestImpostor1(${CommonKClassTypeNamer.uniqueName(proxied::class)})"
     override fun asStatefulWorkflow(): StatefulWorkflow<Nothing, *, Nothing, Nothing> =
       throw NotImplementedError()
   }
diff --git a/workflow-core/src/iosMain/kotlin/com.squareup.workflow1/CommonKClassTypeNamer.kt b/workflow-core/src/iosMain/kotlin/com.squareup.workflow1/CommonKClassTypeNamer.kt
@@ -2,7 +2,7 @@ package com.squareup.workflow1
 
 import kotlin.reflect.KClass
 
-internal actual object WorkflowIdentifierTypeNamer {
+internal actual object CommonKClassTypeNamer {
   public actual fun uniqueName(kClass: KClass<*>): String {
     return kClass.qualifiedName ?: kClass.toString()
   }
diff --git a/workflow-core/src/jsMain/kotlin/com.squareup.workflow1/CommonKClassTypeNamer.kt b/workflow-core/src/jsMain/kotlin/com.squareup.workflow1/CommonKClassTypeNamer.kt
@@ -2,7 +2,7 @@ package com.squareup.workflow1
 
 import kotlin.reflect.KClass
 
-internal actual object WorkflowIdentifierTypeNamer {
+internal actual object CommonKClassTypeNamer {
   // Stores mappings between KClass instances and their assigned names.
   val mappings = mutableMapOf<KClass<*>, String>()
 
diff --git a/workflow-core/src/jvmMain/kotlin/com/squareup/workflow1/CommonKClassTypeNamer.kt b/workflow-core/src/jvmMain/kotlin/com/squareup/workflow1/CommonKClassTypeNamer.kt
@@ -2,7 +2,7 @@ package com.squareup.workflow1
 
 import kotlin.reflect.KClass
 
-internal actual object WorkflowIdentifierTypeNamer {
+internal actual object CommonKClassTypeNamer {
   public actual fun uniqueName(kClass: KClass<*>): String {
     return kClass.qualifiedName ?: kClass.toString()
   }
diff --git a/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/RealRenderContext.kt b/workflow-runtime/src/commonMain/kotlin/com/squareup/workflow1/internal/RealRenderContext.kt
@@ -43,7 +43,8 @@ internal class RealRenderContext<out PropsT, StateT, OutputT>(
   override fun send(value: WorkflowAction<PropsT, StateT, OutputT>) {
     if (!frozen) {
       throw UnsupportedOperationException(
-        "Expected sink to not be sent to until after the render pass. Received action: $value"
+        "Expected sink to not be sent to until after the render pass. " +
+          "Received action: ${value.debuggingName}"
       )
     }
     eventActionsChannel.trySend(value)
diff --git a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/internal/RealRenderContextTest.kt b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/internal/RealRenderContextTest.kt
@@ -124,12 +124,12 @@ internal class RealRenderContextTest {
   @Test fun send_allows_multiple_sends() {
     val context = createdPoisonedContext()
     val firstAction = object : WorkflowAction<String, String, String>() {
+      override val debuggingName: String = "firstAction"
       override fun Updater.apply() = Unit
-      override fun toString(): String = "firstAction"
     }
     val secondAction = object : WorkflowAction<String, String, String>() {
+      override val debuggingName: String = "secondAction"
       override fun Updater.apply() = Unit
-      override fun toString(): String = "secondAction"
     }
     // Enable sink sends.
     context.freeze()
@@ -143,8 +143,8 @@ internal class RealRenderContextTest {
   @Test fun send_throws_before_render_returns() {
     val context = createdPoisonedContext()
     val action = object : WorkflowAction<String, String, String>() {
+      override val debuggingName: String = "action"
       override fun Updater.apply() = Unit
-      override fun toString(): String = "action"
     }
 
     val error = assertFailsWith<UnsupportedOperationException> {
diff --git a/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/internal/WorkflowNodeTest.kt b/workflow-runtime/src/commonTest/kotlin/com/squareup/workflow1/internal/WorkflowNodeTest.kt
@@ -1143,15 +1143,15 @@ internal class WorkflowNodeTest {
     assertTrue(
       error.message!!.startsWith(
         "Expected sink to not be sent to until after the render pass. " +
-          "Received action: WorkflowAction(eventHandler)@"
+          "Received action: eventHandler"
       )
     )
   }
 
   @Test fun send_fails_before_render_pass_completed() {
     class TestAction : WorkflowAction<Unit, Nothing, Nothing>() {
       override fun Updater.apply() = fail("Expected sink send to fail.")
-      override fun toString(): String = "TestAction()"
+      override val debuggingName: String = "TestAction()"
     }
 
     val workflow = Workflow.stateless {
diff --git a/workflow-testing/src/main/java/com/squareup/workflow1/testing/RealRenderTester.kt b/workflow-testing/src/main/java/com/squareup/workflow1/testing/RealRenderTester.kt
@@ -216,7 +216,7 @@ internal class RealRenderTester<PropsT, StateT, OutputT, RenderingT>(
     if (match.output != null) {
       check(processedAction == null) {
         "Expected only one output to be expected: $description expected to emit " +
-          "${match.output.value} but $processedAction was already processed."
+          "${match.output.value} but ${processedAction?.debuggingName} was already processed."
       }
       @Suppress("UNCHECKED_CAST")
       processedAction = handler(match.output.value as ChildOutputT)
@@ -273,8 +273,8 @@ internal class RealRenderTester<PropsT, StateT, OutputT, RenderingT>(
     checkNoOutputs()
     check(processedAction == null) {
       "Tried to send action to sink after another action was already processed:\n" +
-        "  processed action=$processedAction\n" +
-        "  attempted action=$value"
+        "  processed action=${processedAction?.debuggingName}\n" +
+        "  attempted action=${value.debuggingName}"
     }
     processedAction = value
   }
diff --git a/workflow-testing/src/test/java/com/squareup/workflow1/testing/RealRenderTesterTest.kt b/workflow-testing/src/test/java/com/squareup/workflow1/testing/RealRenderTesterTest.kt
@@ -64,7 +64,7 @@ internal class RealRenderTesterTest {
     }
     assertEquals(
       "Expected only one output to be expected: child ${child2.identifier} " +
-        "expected to emit kotlin.Unit but WorkflowAction.noAction() was already processed.",
+        "expected to emit kotlin.Unit but noAction() was already processed.",
       failure.message
     )
   }
@@ -87,7 +87,7 @@ internal class RealRenderTesterTest {
     assertEquals(
       "Expected only one output to be expected: " +
         "child ${typeOf<Worker<Unit>>()} expected to emit " +
-        "kotlin.Unit but WorkflowAction.noAction() was already processed.",
+        "kotlin.Unit but noAction() was already processed.",
       failure.message
     )
   }
@@ -154,9 +154,9 @@ internal class RealRenderTesterTest {
   }
 
   @Test fun `sending to sink throws when called multiple times`() {
-    class TestAction(private val name: String) : WorkflowAction<Unit, Unit, Nothing>() {
+    class TestAction(name: String) : WorkflowAction<Unit, Unit, Nothing>() {
       override fun Updater.apply() {}
-      override fun toString(): String = "TestAction($name)"
+      override val debuggingName: String = "TestAction($name)"
     }
 
     val workflow = Workflow.stateful<Unit, Nothing, Sink<TestAction>>(
@@ -175,8 +175,8 @@ internal class RealRenderTesterTest {
         }
         assertEquals(
           "Tried to send action to sink after another action was already processed:\n" +
-            "  processed action=$action1\n" +
-            "  attempted action=$action2",
+            "  processed action=${action1.debuggingName}\n" +
+            "  attempted action=${action2.debuggingName}",
           error.message
         )
       }
@@ -185,7 +185,7 @@ internal class RealRenderTesterTest {
   @Test fun `sending to sink throws when child output expected`() {
     class TestAction : WorkflowAction<Unit, Unit, Nothing>() {
       override fun Updater.apply() {}
-      override fun toString(): String = "TestAction"
+      override val debuggingName: String = "TestAction"
     }
 
     val workflow = Workflow.stateful<Unit, Nothing, Sink<TestAction>>(
@@ -205,7 +205,7 @@ internal class RealRenderTesterTest {
         }
         assertEquals(
           "Tried to send action to sink after another action was already processed:\n" +
-            "  processed action=WorkflowAction.noAction()\n" +
+            "  processed action=noAction()\n" +
             "  attempted action=TestAction",
           error.message
         )
@@ -964,9 +964,9 @@ internal class RealRenderTesterTest {
     assertEquals("bad props: wrong props", error.message)
   }
 
-  private class TestAction(val name: String) : WorkflowAction<Unit, Nothing, Nothing>() {
+  private class TestAction(name: String) : WorkflowAction<Unit, Nothing, Nothing>() {
     override fun Updater.apply() {}
-    override fun toString(): String = "TestAction($name)"
+    override val debuggingName: String = name
   }
 
   @Test fun `verifyAction failure fails test`() {
@@ -997,7 +997,7 @@ internal class RealRenderTesterTest {
 
     testResult.verifyAction {
       assertTrue(it is TestAction)
-      assertEquals("output", it.name)
+      assertEquals("output", it.debuggingName)
     }
   }
 
@@ -1012,7 +1012,7 @@ internal class RealRenderTesterTest {
 
     testResult.verifyAction {
       assertTrue(it is TestAction)
-      assertEquals("output", it.name)
+      assertEquals("output", it.debuggingName)
     }
   }
 
@@ -1027,7 +1027,7 @@ internal class RealRenderTesterTest {
 
     testResult.verifyAction {
       assertTrue(it is TestAction)
-      assertEquals("event", it.name)
+      assertEquals("event", it.debuggingName)
     }
   }
 
@@ -1087,6 +1087,7 @@ internal class RealRenderTesterTest {
 
   @Test fun `testNextRender could daisy-chain consecutive renderings with verifyAction`() {
     data class TestAction(val add: Int) : WorkflowAction<Unit, Int, Int>() {
+      override val debuggingName: String get() = "add:$add"
       override fun Updater.apply() {
         setOutput(state)
         state += add
@@ -1123,6 +1124,8 @@ internal class RealRenderTesterTest {
 
   @Test fun `testNextRender could daisy-chain consecutive renderings with verifyActionResult`() {
     data class TestAction(val add: Int) : WorkflowAction<Unit, Int, Int>() {
+      override val debuggingName: String get() = "add:$add"
+
       override fun Updater.apply() {
         setOutput(state)
         state += add
@@ -1162,6 +1165,8 @@ internal class RealRenderTesterTest {
 
   @Test fun `testNextRenderWithProps respects new props`() {
     data class TestAction(val add: Int) : WorkflowAction<Int, Int, Int>() {
+      override val debuggingName: String get() = "add:$add"
+
       override fun Updater.apply() {
         setOutput(state)
         state += props * add
@@ -1201,6 +1206,8 @@ internal class RealRenderTesterTest {
 
   @Test fun `testNextRenderWithProps uses onPropsChanged`() {
     data class TestAction(val add: Int) : WorkflowAction<Int, Int, Int>() {
+      override val debuggingName: String get() = "add:$add"
+
       override fun Updater.apply() {
         setOutput(state)
         state += props * add
diff --git a/workflow-tracing/src/test/java/com/squareup/workflow1/diagnostic/tracing/TracingWorkflowInterceptorTest.kt b/workflow-tracing/src/test/java/com/squareup/workflow1/diagnostic/tracing/TracingWorkflowInterceptorTest.kt
diff --git a/workflow-tracing/src/test/resources/com/squareup/workflow1/diagnostic/tracing/expected_trace_file.txt b/workflow-tracing/src/test/resources/com/squareup/workflow1/diagnostic/tracing/expected_trace_file.txt

Original file line number	Diff line number	Diff line change
`@@ -330,6 +330,8 @@ public abstract class com/squareup/workflow1/WorkflowAction {`
`330`	`330`	`public static final field Companion Lcom/squareup/workflow1/WorkflowAction$Companion;`
`331`	`331`	`public fun <init> ()V`
`332`	`332`	`public abstract fun apply (Lcom/squareup/workflow1/WorkflowAction$Updater;)V`
	`333`	`+ public fun getDebuggingName ()Ljava/lang/String;`
	`334`	`+ public fun toString ()Ljava/lang/String;`
`333`	`335`	`}`
`334`	`336`
`335`	`337`	`public final class com/squareup/workflow1/WorkflowAction$Companion {`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ public sealed class WorkflowIdentifierType {`
`24`	`24`	`val kClass: KClass<*>? = null,`
`25`	`25`	`) : WorkflowIdentifierType() {`
`26`	`26`	`public constructor(kClass: KClass<*>) : this(`
`27`		`- WorkflowIdentifierTypeNamer.uniqueName(kClass),`
	`27`	`+ CommonKClassTypeNamer.uniqueName(kClass),`
`28`	`28`	`kClass`
`29`	`29`	`)`
`30`	`30`	`}`
`@@ -46,6 +46,6 @@ public sealed class WorkflowIdentifierType {`
`46`	`46`	`}`
`47`	`47`	`}`
`48`	`48`
`49`		`-internal expect object WorkflowIdentifierTypeNamer {`
	`49`	`+internal expect object CommonKClassTypeNamer {`
`50`	`50`	`public fun uniqueName(kClass: KClass<*>): String`
`51`	`51`	`}`
Original file line number	Diff line number	Diff line change
`@@ -186,7 +186,7 @@ internal class WorkflowIdentifierTest {`
`186`	`186`	`) : Workflow<Nothing, Nothing, Nothing>, ImpostorWorkflow {`
`187`	`187`	`override val realIdentifier: WorkflowIdentifier = proxied.identifier`
`188`	`188`	`override fun describeRealIdentifier(): String =`
`189`		`- "TestImpostor1(${WorkflowIdentifierTypeNamer.uniqueName(proxied::class)})"`
	`189`	`+ "TestImpostor1(${CommonKClassTypeNamer.uniqueName(proxied::class)})"`
`190`	`190`	`override fun asStatefulWorkflow(): StatefulWorkflow<Nothing, *, Nothing, Nothing> =`
`191`	`191`	`throw NotImplementedError()`
`192`	`192`	`}`
Original file line number	Diff line number	Diff line change
`@@ -2,7 +2,7 @@ package com.squareup.workflow1`
`2`	`2`
`3`	`3`	`import kotlin.reflect.KClass`
`4`	`4`
`5`		`-internal actual object WorkflowIdentifierTypeNamer {`
	`5`	`+internal actual object CommonKClassTypeNamer {`
`6`	`6`	`public actual fun uniqueName(kClass: KClass<*>): String {`
`7`	`7`	`return kClass.qualifiedName ?: kClass.toString()`
`8`	`8`	`}`