Adds WorkflowIdentifierTypeNamer to differentiate identifier logic ba…

…sed on platform

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

@@ -86,10 +86,11 @@ internal suspend fun <OutputT> runWorker(
 private class EmitWorkerOutputAction<P, S, O>(
   private val worker: Worker<*>,
   private val renderKey: String,
-  private val output: O
+  private val output: O,
 ) : WorkflowAction<P, S, O>() {
   override fun toString(): String =
-    "${EmitWorkerOutputAction::class.qualifiedName}(worker=$worker, key=\"$renderKey\")"
+    WorkflowIdentifierTypeNamer.uniqueName(EmitWorkerOutputAction::class) +
+      "(worker=$worker, key=\"$renderKey\")"
 
   override fun Updater.apply() {
     setOutput(output)

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(
-      kClass.qualifiedName ?: kClass.toString(),
+      WorkflowIdentifierTypeNamer.uniqueName(kClass),
       kClass
     )
   }
@@ -45,3 +45,7 @@ public sealed class WorkflowIdentifierType {
     override val typeName: String = kType.toString()
   }
 }
+
+internal expect object WorkflowIdentifierTypeNamer {
+  public fun uniqueName(kClass: KClass<*>): String
+}

workflow-core/src/commonTest/kotlin/com/squareup/workflow1/WorkflowIdentifierTest.kt

@@ -15,54 +15,6 @@ import kotlin.test.assertNull
 @OptIn(ExperimentalStdlibApi::class)
 internal class WorkflowIdentifierTest {
 
-  @Test fun flat_identifier_toString() {
-    val id = TestWorkflow1.identifier
-    assertEquals(
-      "WorkflowIdentifier(com.squareup.workflow1.WorkflowIdentifierTest.TestWorkflow1)",
-      id.toString()
-    )
-  }
-
-  @Test fun impostor_identifier_toString_uses_describeRealIdentifier_when_non_null() {
-    class TestImpostor : Workflow<Nothing, Nothing, Nothing>, ImpostorWorkflow {
-      override val realIdentifier: WorkflowIdentifier = TestWorkflow1.identifier
-      override fun describeRealIdentifier(): String =
-        "TestImpostor(${TestWorkflow1::class.simpleName})"
-
-      override fun asStatefulWorkflow(): StatefulWorkflow<Nothing, *, Nothing, Nothing> =
-        throw NotImplementedError()
-    }
-
-    val id = TestImpostor().identifier
-    assertEquals("TestImpostor(TestWorkflow1)", id.toString())
-  }
-
-  @Test
-  fun impostor_identifier_toString_uses_full_chain_when_describeRealIdentifier_returns_null() {
-    class TestImpostor : Workflow<Nothing, Nothing, Nothing>, ImpostorWorkflow {
-      override val realIdentifier: WorkflowIdentifier = TestWorkflow1.identifier
-      override fun describeRealIdentifier(): String? = null
-
-      override fun asStatefulWorkflow(): StatefulWorkflow<Nothing, *, Nothing, Nothing> =
-        throw NotImplementedError()
-    }
-
-    val id = TestImpostor().identifier
-    assertEquals(
-      "WorkflowIdentifier(${TestImpostor::class}, " +
-        "com.squareup.workflow1.WorkflowIdentifierTest.TestWorkflow1)",
-      id.toString()
-    )
-  }
-
-  @Test fun impostor_identifier_description() {
-    val id = TestImpostor1(TestWorkflow1).identifier
-    assertEquals(
-      "TestImpostor1(com.squareup.workflow1.WorkflowIdentifierTest.TestWorkflow1)",
-      id.toString()
-    )
-  }
-
   @Test fun restored_identifier_toString() {
     val id = TestWorkflow1.identifier
     val serializedId = id.toByteStringOrNull()!!
@@ -231,7 +183,8 @@ internal class WorkflowIdentifierTest {
     private val proxied: Workflow<*, *, *>
   ) : Workflow<Nothing, Nothing, Nothing>, ImpostorWorkflow {
     override val realIdentifier: WorkflowIdentifier = proxied.identifier
-    override fun describeRealIdentifier(): String = "TestImpostor1(${proxied::class.qualifiedName})"
+    override fun describeRealIdentifier(): String =
+      "TestImpostor1(${WorkflowIdentifierTypeNamer.uniqueName(proxied::class)})"
     override fun asStatefulWorkflow(): StatefulWorkflow<Nothing, *, Nothing, Nothing> =
       throw NotImplementedError()
   }

workflow-core/src/iosMain/kotlin/com.squareup.workflow1/WorkflowIdentifierTypeNamer.kt

@@ -0,0 +1,9 @@
+package com.squareup.workflow1
+
+import kotlin.reflect.KClass
+
+internal actual object WorkflowIdentifierTypeNamer {
+  public actual fun uniqueName(kClass: KClass<*>): String {
+    return kClass.qualifiedName ?: kClass.toString()
+  }
+}

workflow-core/src/iosTest/kotlin/com/squareup/workflow1/NativeWorkflowIdentifierTest.kt

@@ -1,12 +1,48 @@
 package com.squareup.workflow1
 
+import com.squareup.workflow1.WorkflowIdentifierTest.TestImpostor1
 import com.squareup.workflow1.WorkflowIdentifierTest.TestUnsnapshottableImpostor
+import com.squareup.workflow1.WorkflowIdentifierTest.TestWorkflow1
 import kotlin.reflect.typeOf
 import kotlin.test.Test
 import kotlin.test.assertEquals
 
 class NativeWorkflowIdentifierTest {
 
+  @Test fun `flat identifier toString`() {
+    val id = TestWorkflow1.identifier
+    assertEquals(
+      "WorkflowIdentifier(com.squareup.workflow1.WorkflowIdentifierTest.TestWorkflow1)",
+      id.toString()
+    )
+  }
+
+  @Test
+  fun `impostor identifier toString uses full chain when describeRealIdentifier returns null`() {
+    class TestImpostor : Workflow<Nothing, Nothing, Nothing>, ImpostorWorkflow {
+      override val realIdentifier: WorkflowIdentifier = TestWorkflow1.identifier
+      override fun describeRealIdentifier(): String? = null
+
+      override fun asStatefulWorkflow(): StatefulWorkflow<Nothing, *, Nothing, Nothing> =
+        throw NotImplementedError()
+    }
+
+    val id = TestImpostor().identifier
+    assertEquals(
+      "WorkflowIdentifier(${TestImpostor::class}, " +
+        "com.squareup.workflow1.WorkflowIdentifierTest.TestWorkflow1)",
+      id.toString()
+    )
+  }
+
+  @Test fun `impostor identifier description`() {
+    val id = TestImpostor1(TestWorkflow1).identifier
+    assertEquals(
+      "TestImpostor1(com.squareup.workflow1.WorkflowIdentifierTest.TestWorkflow1)",
+      id.toString()
+    )
+  }
+
   @Test fun `unsnapshottable identifier toString`() {
     val id = unsnapshottableIdentifier(typeOf<String>())
     assertEquals(

workflow-core/src/jsMain/kotlin/com.squareup.workflow1/WorkflowIdentifierTypeNamer.kt

@@ -0,0 +1,31 @@
+package com.squareup.workflow1
+
+import kotlin.reflect.KClass
+
+internal actual object WorkflowIdentifierTypeNamer {
+  // Stores mappings between KClass instances and their assigned names.
+  val mappings = mutableMapOf<KClass<*>, String>()
+
+  // Note: This implementation does not differentiate between generic workflows.
+  // (ie. SomeGenericWorkflow<String> and SomeGenericWorkflow<Int> would both return back the same
+  // value.)
+  //
+  // Recommended workarounds:
+  // - Always provide a key for generic workflows
+  // - Create non-generic subclasses of generic workflows
+  public actual fun uniqueName(kClass: KClass<*>): String {
+    // Note: `kClass.qualifiedName` cannot be used here like other platforms as it's not supported
+    // for JS. Therefore, we construct a unique name of each static KClass based on its simple name
+    // and an index of when it was encountered.
+
+    val mapping = mappings[kClass]
+    if (mapping != null) {
+      return mapping
+    }
+
+    // `simpleName` does not differentiate between generic workflows.
+    val identifier = "${kClass.simpleName ?: kClass.hashCode()}(${mappings.size})"
+    mappings[kClass] = identifier
+    return identifier
+  }
+}

workflow-core/src/jsTest/kotlin/com.squareup.workflow1/JsWorkflowIdentifierTest.kt

@@ -0,0 +1,69 @@
+package com.squareup.workflow1
+
+import com.squareup.workflow1.WorkflowIdentifierTest.TestImpostor1
+import com.squareup.workflow1.WorkflowIdentifierTest.TestWorkflow1
+import com.squareup.workflow1.mocks.workflows1.JsMockWorkflow1
+import com.squareup.workflow1.mocks.workflows1.JsMockWorkflow2
+import kotlin.js.RegExp
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertNotEquals
+import kotlin.test.assertTrue
+
+@OptIn(ExperimentalStdlibApi::class)
+internal class JsWorkflowIdentifierTest {
+  @Test fun flat_identifier_toString() {
+    val id = JsMockWorkflow1().identifier
+
+    // Due to the dynamic naming of workflow identifiers (based on other identifiers that have been
+    // created so far), we can only verify the composition of the name.
+    // Expected value should be something like this: "WorkflowIdentifier(JsMockWorkflow1(7))"
+    val idStructure = RegExp("WorkflowIdentifier\\(JsMockWorkflow1\\((\\d)+\\)\\)")
+    assertTrue(idStructure.test(id.toString()))
+  }
+
+  @Test
+  fun impostor_identifier_toString_uses_full_chain_when_describeRealIdentifier_returns_null() {
+    class TestImpostor : Workflow<Nothing, Nothing, Nothing>, ImpostorWorkflow {
+      override val realIdentifier: WorkflowIdentifier = TestWorkflow1.identifier
+      override fun describeRealIdentifier(): String? = null
+
+      override fun asStatefulWorkflow(): StatefulWorkflow<Nothing, *, Nothing, Nothing> =
+        throw NotImplementedError()
+    }
+
+    val id = TestImpostor().identifier
+
+    // Expected value should be something like this:
+    // "WorkflowIdentifier(TestImpostor(3), TestWorkflow1(1))"
+    val idStructure = RegExp(
+      "WorkflowIdentifier\\(TestImpostor\\((\\d)+\\), TestWorkflow1\\((\\d)+\\)\\)"
+    )
+    assertTrue(idStructure.test(id.toString()))
+  }
+
+  @Test fun impostor_identifier_description() {
+    // Expected value should be something like this: "WorkflowIdentifier(TestWorkflow1(1))"
+    val id = TestImpostor1(TestWorkflow1).identifier
+    val idStructure = RegExp("TestImpostor1\\(TestWorkflow1\\((\\d)+\\)\\)")
+    assertTrue(idStructure.test(id.toString()))
+  }
+
+  @Test fun same_workflow_returns_same_identifier() {
+    val id1 = JsMockWorkflow1().identifier
+    val id2 = JsMockWorkflow1().identifier
+    assertEquals(id1.toString(), id2.toString())
+  }
+
+  @Test fun different_workflows_same_namespace() {
+    val id1 = JsMockWorkflow1().identifier
+    val id2 = JsMockWorkflow2().identifier
+    assertNotEquals(id1.toString(), id2.toString())
+  }
+
+  @Test fun same_workflow_name_different_namespace() {
+    val id1 = JsMockWorkflow1().identifier
+    val id2 = com.squareup.workflow1.mocks.workflows2.JsMockWorkflow1().identifier
+    assertNotEquals(id1.toString(), id2.toString())
+  }
+}

workflow-core/src/jsTest/kotlin/com.squareup.workflow1/mocks.workflows1/JsMockWorkflows.kt

@@ -0,0 +1,14 @@
+package com.squareup.workflow1.mocks.workflows1
+
+import com.squareup.workflow1.StatefulWorkflow
+import com.squareup.workflow1.Workflow
+
+public class JsMockWorkflow1 : Workflow<Nothing, Nothing, Nothing> {
+  override fun asStatefulWorkflow(): StatefulWorkflow<Nothing, *, Nothing, Nothing> =
+    throw NotImplementedError()
+}
+
+public class JsMockWorkflow2 : Workflow<Nothing, Nothing, Nothing> {
+  override fun asStatefulWorkflow(): StatefulWorkflow<Nothing, *, Nothing, Nothing> =
+    throw NotImplementedError()
+}

workflow-core/src/jsTest/kotlin/com.squareup.workflow1/mocks.workflows2/JsMockWorkflow1.kt

@@ -0,0 +1,11 @@
+package com.squareup.workflow1.mocks.workflows2
+
+import com.squareup.workflow1.StatefulWorkflow
+import com.squareup.workflow1.Workflow
+
+// This is purposely duplicated to be like com.squareup.workflow1.mocks.workflows1.JsMockWorkflow1,
+// but with a slightly different Output.
+public class JsMockWorkflow1 : Workflow<Nothing, String, Nothing> {
+  override fun asStatefulWorkflow(): StatefulWorkflow<Nothing, *, String, Nothing> =
+    throw NotImplementedError()
+}

workflow-core/src/jvmMain/kotlin/com/squareup/workflow1/WorkflowIdentifierTypeNamer.kt

@@ -0,0 +1,9 @@
+package com.squareup.workflow1
+
+import kotlin.reflect.KClass
+
+internal actual object WorkflowIdentifierTypeNamer {
+  public actual fun uniqueName(kClass: KClass<*>): String {
+    return kClass.qualifiedName ?: kClass.toString()
+  }
+}

workflow-core/src/jvmTest/kotlin/com/squareup/workflow1/JvmWorkflowIdentifierTest.kt

@@ -12,6 +12,40 @@ import kotlin.test.assertNotEquals
 
 class JvmWorkflowIdentifierTest {
 
+  @Test fun `flat identifier toString`() {
+    val id = TestWorkflow1.identifier
+    assertEquals(
+      "WorkflowIdentifier(com.squareup.workflow1.WorkflowIdentifierTest.TestWorkflow1)",
+      id.toString()
+    )
+  }
+
+  @Test
+  fun `impostor identifier toString uses full chain when describeRealIdentifier returns null`() {
+    class TestImpostor : Workflow<Nothing, Nothing, Nothing>, ImpostorWorkflow {
+      override val realIdentifier: WorkflowIdentifier = TestWorkflow1.identifier
+      override fun describeRealIdentifier(): String? = null
+
+      override fun asStatefulWorkflow(): StatefulWorkflow<Nothing, *, Nothing, Nothing> =
+        throw NotImplementedError()
+    }
+
+    val id = TestImpostor().identifier
+    assertEquals(
+      "WorkflowIdentifier(${TestImpostor::class}, " +
+        "com.squareup.workflow1.WorkflowIdentifierTest.TestWorkflow1)",
+      id.toString()
+    )
+  }
+
+  @Test fun `impostor identifier description`() {
+    val id = TestImpostor1(TestWorkflow1).identifier
+    assertEquals(
+      "TestImpostor1(com.squareup.workflow1.WorkflowIdentifierTest.TestWorkflow1)",
+      id.toString()
+    )
+  }
+
   @Test fun `workflowIdentifier from Workflow class is equal to identifier from workflow`() {
     val instanceId = TestWorkflow1.identifier
     val classId = TestWorkflow1::class.workflowIdentifier