From 138c87c92e45a84c419c6dab98f33799c4e329c8 Mon Sep 17 00:00:00 2001 From: Victor Ng Date: Fri, 8 Jul 2022 19:32:54 -0700 Subject: [PATCH] Adds WorkflowIdentifierTypeNamer to differentiate identifier logic based on platform --- .../com/squareup/workflow1/WorkerWorkflow.kt | 5 +- .../workflow1/WorkflowIdentifierType.kt | 6 +- .../workflow1/WorkflowIdentifierTest.kt | 51 +------------- .../WorkflowIdentifierTypeNamer.kt | 9 +++ .../workflow1/NativeWorkflowIdentifierTest.kt | 36 ++++++++++ .../WorkflowIdentifierTypeNamer.kt | 31 +++++++++ .../JsWorkflowIdentifierTest.kt | 69 +++++++++++++++++++ .../mocks.workflows1/JsMockWorkflows.kt | 14 ++++ .../mocks.workflows2/JsMockWorkflow1.kt | 11 +++ .../workflow1/WorkflowIdentifierTypeNamer.kt | 9 +++ .../workflow1/JvmWorkflowIdentifierTest.kt | 34 +++++++++ 11 files changed, 223 insertions(+), 52 deletions(-) create mode 100644 workflow-core/src/iosMain/kotlin/com.squareup.workflow1/WorkflowIdentifierTypeNamer.kt create mode 100644 workflow-core/src/jsMain/kotlin/com.squareup.workflow1/WorkflowIdentifierTypeNamer.kt create mode 100644 workflow-core/src/jsTest/kotlin/com.squareup.workflow1/JsWorkflowIdentifierTest.kt create mode 100644 workflow-core/src/jsTest/kotlin/com.squareup.workflow1/mocks.workflows1/JsMockWorkflows.kt create mode 100644 workflow-core/src/jsTest/kotlin/com.squareup.workflow1/mocks.workflows2/JsMockWorkflow1.kt create mode 100644 workflow-core/src/jvmMain/kotlin/com/squareup/workflow1/WorkflowIdentifierTypeNamer.kt diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkerWorkflow.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkerWorkflow.kt index c5ec0bba9..080bb5eaa 100644 --- a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkerWorkflow.kt +++ b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkerWorkflow.kt @@ -86,10 +86,11 @@ internal suspend fun runWorker( private class EmitWorkerOutputAction( private val worker: Worker<*>, private val renderKey: String, - private val output: O + private val output: O, ) : WorkflowAction() { 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) diff --git a/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkflowIdentifierType.kt b/workflow-core/src/commonMain/kotlin/com/squareup/workflow1/WorkflowIdentifierType.kt index 49c3b7415..6adafb727 100644 --- 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( - 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 +} diff --git a/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/WorkflowIdentifierTest.kt b/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/WorkflowIdentifierTest.kt index 14278a8f6..b3bb5b7fd 100644 --- a/workflow-core/src/commonTest/kotlin/com/squareup/workflow1/WorkflowIdentifierTest.kt +++ b/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, ImpostorWorkflow { - override val realIdentifier: WorkflowIdentifier = TestWorkflow1.identifier - override fun describeRealIdentifier(): String = - "TestImpostor(${TestWorkflow1::class.simpleName})" - - override fun asStatefulWorkflow(): StatefulWorkflow = - 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, ImpostorWorkflow { - override val realIdentifier: WorkflowIdentifier = TestWorkflow1.identifier - override fun describeRealIdentifier(): String? = null - - override fun asStatefulWorkflow(): StatefulWorkflow = - 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, 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 = throw NotImplementedError() } diff --git a/workflow-core/src/iosMain/kotlin/com.squareup.workflow1/WorkflowIdentifierTypeNamer.kt b/workflow-core/src/iosMain/kotlin/com.squareup.workflow1/WorkflowIdentifierTypeNamer.kt new file mode 100644 index 000000000..6f7b00f55 --- /dev/null +++ b/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() + } +} diff --git a/workflow-core/src/iosTest/kotlin/com/squareup/workflow1/NativeWorkflowIdentifierTest.kt b/workflow-core/src/iosTest/kotlin/com/squareup/workflow1/NativeWorkflowIdentifierTest.kt index e2a28e123..fdc5f2c88 100644 --- a/workflow-core/src/iosTest/kotlin/com/squareup/workflow1/NativeWorkflowIdentifierTest.kt +++ b/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, ImpostorWorkflow { + override val realIdentifier: WorkflowIdentifier = TestWorkflow1.identifier + override fun describeRealIdentifier(): String? = null + + override fun asStatefulWorkflow(): StatefulWorkflow = + 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()) assertEquals( diff --git a/workflow-core/src/jsMain/kotlin/com.squareup.workflow1/WorkflowIdentifierTypeNamer.kt b/workflow-core/src/jsMain/kotlin/com.squareup.workflow1/WorkflowIdentifierTypeNamer.kt new file mode 100644 index 000000000..a74052e8b --- /dev/null +++ b/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, String>() + + // Note: This implementation does not differentiate between generic workflows. + // (ie. SomeGenericWorkflow and SomeGenericWorkflow 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 + } +} diff --git a/workflow-core/src/jsTest/kotlin/com.squareup.workflow1/JsWorkflowIdentifierTest.kt b/workflow-core/src/jsTest/kotlin/com.squareup.workflow1/JsWorkflowIdentifierTest.kt new file mode 100644 index 000000000..106ca7905 --- /dev/null +++ b/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, ImpostorWorkflow { + override val realIdentifier: WorkflowIdentifier = TestWorkflow1.identifier + override fun describeRealIdentifier(): String? = null + + override fun asStatefulWorkflow(): StatefulWorkflow = + 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()) + } +} diff --git a/workflow-core/src/jsTest/kotlin/com.squareup.workflow1/mocks.workflows1/JsMockWorkflows.kt b/workflow-core/src/jsTest/kotlin/com.squareup.workflow1/mocks.workflows1/JsMockWorkflows.kt new file mode 100644 index 000000000..ecf938806 --- /dev/null +++ b/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 { + override fun asStatefulWorkflow(): StatefulWorkflow = + throw NotImplementedError() +} + +public class JsMockWorkflow2 : Workflow { + override fun asStatefulWorkflow(): StatefulWorkflow = + throw NotImplementedError() +} diff --git a/workflow-core/src/jsTest/kotlin/com.squareup.workflow1/mocks.workflows2/JsMockWorkflow1.kt b/workflow-core/src/jsTest/kotlin/com.squareup.workflow1/mocks.workflows2/JsMockWorkflow1.kt new file mode 100644 index 000000000..841db956e --- /dev/null +++ b/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 { + override fun asStatefulWorkflow(): StatefulWorkflow = + throw NotImplementedError() +} diff --git a/workflow-core/src/jvmMain/kotlin/com/squareup/workflow1/WorkflowIdentifierTypeNamer.kt b/workflow-core/src/jvmMain/kotlin/com/squareup/workflow1/WorkflowIdentifierTypeNamer.kt new file mode 100644 index 000000000..6f7b00f55 --- /dev/null +++ b/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() + } +} diff --git a/workflow-core/src/jvmTest/kotlin/com/squareup/workflow1/JvmWorkflowIdentifierTest.kt b/workflow-core/src/jvmTest/kotlin/com/squareup/workflow1/JvmWorkflowIdentifierTest.kt index 372f23a66..272d3eb70 100644 --- a/workflow-core/src/jvmTest/kotlin/com/squareup/workflow1/JvmWorkflowIdentifierTest.kt +++ b/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, ImpostorWorkflow { + override val realIdentifier: WorkflowIdentifier = TestWorkflow1.identifier + override fun describeRealIdentifier(): String? = null + + override fun asStatefulWorkflow(): StatefulWorkflow = + 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