diff --git a/compose/api/compose.klib.api b/compose/api/compose.klib.api index d46c351..31f7f8e 100644 --- a/compose/api/compose.klib.api +++ b/compose/api/compose.klib.api @@ -193,8 +193,6 @@ final class app.tinyui.node/PatchProblem { // app.tinyui.node/PatchProblem|null[ final class app.tinyui.node/UINode { // app.tinyui.node/UINode|null[0] final val children // app.tinyui.node/UINode.children|{}children[0] final fun (): androidx.compose.runtime.snapshots/SnapshotStateList // app.tinyui.node/UINode.children.|(){}[0] - final val commands // app.tinyui.node/UINode.commands|{}commands[0] - final fun (): androidx.compose.runtime.snapshots/SnapshotStateList // app.tinyui.node/UINode.commands.|(){}[0] final val events // app.tinyui.node/UINode.events|{}events[0] final fun (): androidx.compose.runtime.snapshots/SnapshotStateMap // app.tinyui.node/UINode.events.|(){}[0] final val id // app.tinyui.node/UINode.id|{}id[0] diff --git a/compose/src/commonMain/kotlin/app/tinyui/node/NodeTree.kt b/compose/src/commonMain/kotlin/app/tinyui/node/NodeTree.kt index 637ca78..4f72ca7 100644 --- a/compose/src/commonMain/kotlin/app/tinyui/node/NodeTree.kt +++ b/compose/src/commonMain/kotlin/app/tinyui/node/NodeTree.kt @@ -28,13 +28,17 @@ class PatchProblem(val op: String, val reason: String) { class NodeTree(private val registry: ComponentRegistry, private val report: (PatchProblem) -> Unit) { val root = UINode(ROOT_ID, "root") private val nodes = HashMap().apply { put(ROOT_ID, root) } - private val born = ArrayList() - val size: Int get() = nodes.size fun node(id: Int): UINode? = nodes[id] - /** Applies one flush atomically: other threads see the tree before or after, never in between. */ + /** + * Applies one flush atomically: other threads see the tree before or after, never in between. Nothing leaves the + * snapshot while it is open: removed nodes' queues are cancelled, commands sent and problems reported only once the + * flush is applied, in that order. So a component never gets a command ahead of the props it came with, a flush that + * fails to apply leaves no trace beyond its report, and a sink hears of a flush once it is complete: if it writes to + * the tree from [report], that is a flush of its own, after this one. + */ fun apply(json: String) { val ops = try { Json.parseToJsonElement(json).jsonArray @@ -42,101 +46,113 @@ class NodeTree(private val registry: ComponentRegistry, private val report: (Pat report(PatchProblem(json.take(200), "message is not a JSON array (${e.message})")) return } - Snapshot.withMutableSnapshot { - for (op in ops) applyOp(op) - for (node in born) { - node.created = true - registry.schema(node.type)?.props?.forEach { (key, spec) -> - if (spec.required && node.props[key] == null) report(PatchProblem("[\"c\",${node.id},\"${node.type}\"]", "required prop $key was not set in the creating flush")) + val flush = Flush() + try { + Snapshot.withMutableSnapshot { + for (op in ops) applyOp(op, flush) + for (node in flush.born) { + node.created = true + registry.schema(node.type)?.props?.forEach { (key, spec) -> + if (spec.required && node.props[key] == null) flush.problem("[\"c\",${node.id},\"${node.type}\"]", "required prop $key was not set in the creating flush") + } } } - born.clear() + } catch (t: Throwable) { + // what went wrong is still worth hearing; nothing else of this flush happens + for (p in flush.problems) report(p) + throw t } + // cancelled first: a command for a node removed in the same flush is dropped, not delivered + for (node in flush.removed) node.commands.cancel() + for ((node, command) in flush.sent) node.commands.trySend(command) + for (p in flush.problems) report(p) } + /** Drops every node like `r` would: their queues close, so nothing waits on a node that is gone. */ fun clear() { + for (node in nodes.values) if (node !== root) node.commands.cancel() root.children.clear() nodes.clear() nodes[ROOT_ID] = root } - private fun applyOp(op: JsonElement) { - val fields = op as? JsonArray ?: return report(PatchProblem(op.toString(), "op is not an array")) - val kind = fields.getOrNull(0)?.string() ?: return report(PatchProblem(op.toString(), "op without kind")) + private fun applyOp(op: JsonElement, flush: Flush) { + val fields = op as? JsonArray ?: return flush.problem(op.toString(), "op is not an array") + val kind = fields.getOrNull(0)?.string() ?: return flush.problem(op.toString(), "op without kind") when (kind) { - "c" -> create(fields, op) - "p" -> setProp(fields, op) - "i" -> insert(fields, op) - "m" -> move(fields, op) - "r" -> remove(fields, op) - "x" -> command(fields, op) - else -> report(PatchProblem(op.toString(), "unknown op")) + "c" -> create(fields, op, flush) + "p" -> setProp(fields, op, flush) + "i" -> insert(fields, op, flush) + "m" -> move(fields, op, flush) + "r" -> remove(fields, op, flush) + "x" -> command(fields, op, flush) + else -> flush.problem(op.toString(), "unknown op") } } - private fun create(f: JsonArray, op: JsonElement) { - val id = f.int(1) ?: return report(PatchProblem(op.toString(), "c without id")) - val type = f.string(2) ?: return report(PatchProblem(op.toString(), "c without type")) - if (nodes.containsKey(id)) return report(PatchProblem(op.toString(), "id already exists")) + private fun create(f: JsonArray, op: JsonElement, flush: Flush) { + val id = f.int(1) ?: return flush.problem(op.toString(), "c without id") + val type = f.string(2) ?: return flush.problem(op.toString(), "c without type") + if (nodes.containsKey(id)) return flush.problem(op.toString(), "id already exists") // the requested type stays on the node: Render falls back to Placeholder, which shows it in debug - if (registry.schema(type) == null) report(PatchProblem(op.toString(), "unknown component type, rendering Placeholder")) - nodes[id] = UINode(id, type).also { born += it } + if (registry.schema(type) == null) flush.problem(op.toString(), "unknown component type, rendering Placeholder") + nodes[id] = UINode(id, type).also { flush.born += it } } - private fun setProp(f: JsonArray, op: JsonElement) { - val node = child(f, op, at = 1) ?: return - val key = f.string(2) ?: return report(PatchProblem(op.toString(), "p without key")) + private fun setProp(f: JsonArray, op: JsonElement, flush: Flush) { + val node = child(f, op, flush, at = 1) ?: return + val key = f.string(2) ?: return flush.problem(op.toString(), "p without key") val value = f.getOrNull(3) ?: JsonNull val schema = registry.schema(node.type) ?: return if (key.length > 2 && key.startsWith("on") && key[2].isUpperCase()) { - if (key !in schema.events) return report(PatchProblem(op.toString(), "event not in schema of ${node.type}")) - val flag = (value as? JsonPrimitive)?.booleanOrNull ?: return report(PatchProblem(op.toString(), "event flag must be a boolean")) + if (key !in schema.events) return flush.problem(op.toString(), "event not in schema of ${node.type}") + val flag = (value as? JsonPrimitive)?.booleanOrNull ?: return flush.problem(op.toString(), "event flag must be a boolean") if (flag) node.events[key] = true else node.events.remove(key) return } - val spec = schema.prop(key) ?: return report(PatchProblem(op.toString(), "prop not in schema of ${node.type}")) - if (spec.initial && node.created) return report(PatchProblem(op.toString(), "$key is an initial prop of ${node.type}: writes after creation are ignored")) + val spec = schema.prop(key) ?: return flush.problem(op.toString(), "prop not in schema of ${node.type}") + if (spec.initial && node.created) return flush.problem(op.toString(), "$key is an initial prop of ${node.type}: writes after creation are ignored") if (value is JsonNull) { - if (spec.required && spec.default == null) return report(PatchProblem(op.toString(), "$key is required on ${node.type}; null is not allowed")) + if (spec.required && spec.default == null) return flush.problem(op.toString(), "$key is required on ${node.type}; null is not allowed") node.props[key] = spec.default return } - val primitive = value as? JsonPrimitive ?: return report(PatchProblem(op.toString(), "prop value must be a scalar")) - val converted = spec.convert(primitive) ?: return report(PatchProblem(op.toString(), "cannot convert ${primitive} to ${spec.kind} for ${node.type}.$key")) + val primitive = value as? JsonPrimitive ?: return flush.problem(op.toString(), "prop value must be a scalar") + val converted = spec.convert(primitive) ?: return flush.problem(op.toString(), "cannot convert ${primitive} to ${spec.kind} for ${node.type}.$key") node.props[key] = converted } - private fun insert(f: JsonArray, op: JsonElement) { - val parent = f.int(1)?.let { nodes[it] } ?: return report(PatchProblem(op.toString(), "i: unknown parent")) - val node = child(f, op) ?: return - if (node.parent != null) return report(PatchProblem(op.toString(), "i: node already has a parent")) - if (node === parent || isAncestor(node, of = parent)) return report(PatchProblem(op.toString(), "i: would create a cycle")) - val index = clamp(f.int(3), parent.children.size, op) ?: return + private fun insert(f: JsonArray, op: JsonElement, flush: Flush) { + val parent = f.int(1)?.let { nodes[it] } ?: return flush.problem(op.toString(), "i: unknown parent") + val node = child(f, op, flush) ?: return + if (node.parent != null) return flush.problem(op.toString(), "i: node already has a parent") + if (node === parent || isAncestor(node, of = parent)) return flush.problem(op.toString(), "i: would create a cycle") + val index = clamp(f.int(3), parent.children.size, op, flush) ?: return node.parent = parent parent.children.add(index, node) } - private fun move(f: JsonArray, op: JsonElement) { - val parent = f.int(1)?.let { nodes[it] } ?: return report(PatchProblem(op.toString(), "m: unknown parent")) - val node = child(f, op) ?: return - if (node.parent !== parent) return report(PatchProblem(op.toString(), "m: node is not a child of parent")) - val index = clamp(f.int(3), parent.children.size - 1, op) ?: return + private fun move(f: JsonArray, op: JsonElement, flush: Flush) { + val parent = f.int(1)?.let { nodes[it] } ?: return flush.problem(op.toString(), "m: unknown parent") + val node = child(f, op, flush) ?: return + if (node.parent !== parent) return flush.problem(op.toString(), "m: node is not a child of parent") + val index = clamp(f.int(3), parent.children.size - 1, op, flush) ?: return parent.children.remove(node) parent.children.add(index, node) } - private fun remove(f: JsonArray, op: JsonElement) { - val node = child(f, op, at = 1) ?: return + private fun remove(f: JsonArray, op: JsonElement, flush: Flush) { + val node = child(f, op, flush, at = 1) ?: return node.parent?.children?.remove(node) node.parent = null - forget(node) + forget(node, flush) } /** Ops that act on a node as a child never target the root container. */ - private fun child(f: JsonArray, op: JsonElement, at: Int = 2): UINode? { - val id = f.int(at) ?: run { report(PatchProblem(op.toString(), "op without node id")); return null } - val node = nodes[id] ?: run { report(PatchProblem(op.toString(), "unknown node $id")); return null } - if (node.id == ROOT_ID) { report(PatchProblem(op.toString(), "the root container is not a child")); return null } + private fun child(f: JsonArray, op: JsonElement, flush: Flush, at: Int = 2): UINode? { + val id = f.int(at) ?: run { flush.problem(op.toString(), "op without node id"); return null } + val node = nodes[id] ?: run { flush.problem(op.toString(), "unknown node $id"); return null } + if (node.id == ROOT_ID) { flush.problem(op.toString(), "the root container is not a child"); return null } return node } @@ -146,30 +162,32 @@ class NodeTree(private val registry: ComponentRegistry, private val report: (Pat return false } - private fun command(f: JsonArray, op: JsonElement) { - val node = child(f, op, at = 1) ?: return - val name = f.string(2) ?: return report(PatchProblem(op.toString(), "x without name")) + private fun command(f: JsonArray, op: JsonElement, flush: Flush) { + val node = child(f, op, flush, at = 1) ?: return + val name = f.string(2) ?: return flush.problem(op.toString(), "x without name") val schema = registry.schema(node.type) ?: return - val fields = schema.commands[name] ?: return report(PatchProblem(op.toString(), "command not in schema of ${node.type}")) + val fields = schema.commands[name] ?: return flush.problem(op.toString(), "command not in schema of ${node.type}") val given = f.getOrNull(3) as? JsonObject ?: JsonObject(emptyMap()) val args = HashMap() for ((field, spec) in fields) { - val v = given[field] as? JsonPrimitive ?: return report(PatchProblem(op.toString(), "command $name needs $field")) - if (!spec.accepts(v)) return report(PatchProblem(op.toString(), "command $name: $field must be $spec")) + val v = given[field] as? JsonPrimitive ?: return flush.problem(op.toString(), "command $name needs $field") + if (!spec.accepts(v)) return flush.problem(op.toString(), "command $name: $field must be $spec") args[field] = v.scalar() } - node.commands.add(Command(name, args)) + flush.sent += node to Command(name, args) } - private fun forget(node: UINode) { + private fun forget(node: UINode, flush: Flush) { nodes.remove(node.id) - for (child in node.children) forget(child) + // deleted for good (ids are never reused): its queue is cancelled once the flush is applied + flush.removed += node + for (child in node.children) forget(child, flush) } - private fun clamp(index: Int?, size: Int, op: JsonElement): Int? { - if (index == null) { report(PatchProblem(op.toString(), "missing index")); return null } + private fun clamp(index: Int?, size: Int, op: JsonElement, flush: Flush): Int? { + if (index == null) { flush.problem(op.toString(), "missing index"); return null } if (index < 0 || index > size) { - report(PatchProblem(op.toString(), "index $index out of [0, $size], clamped")) + flush.problem(op.toString(), "index $index out of [0, $size], clamped") return index.coerceIn(0, size) } return index @@ -185,3 +203,15 @@ class NodeTree(private val registry: ComponentRegistry, private val report: (Pat else -> doubleOrNull } } + +/** What one flush does outside the snapshot, once it is applied; the ops only run with one in hand. */ +private class Flush { + val problems = ArrayList() + val born = ArrayList() + val sent = ArrayList>() + val removed = ArrayList() + + fun problem(op: String, reason: String) { + problems += PatchProblem(op, reason) + } +} diff --git a/compose/src/commonMain/kotlin/app/tinyui/node/UINode.kt b/compose/src/commonMain/kotlin/app/tinyui/node/UINode.kt index 7026a1c..c33dbf0 100644 --- a/compose/src/commonMain/kotlin/app/tinyui/node/UINode.kt +++ b/compose/src/commonMain/kotlin/app/tinyui/node/UINode.kt @@ -4,6 +4,7 @@ import androidx.compose.runtime.mutableStateListOf import androidx.compose.runtime.mutableStateMapOf import androidx.compose.runtime.snapshots.SnapshotStateList import androidx.compose.runtime.snapshots.SnapshotStateMap +import kotlinx.coroutines.channels.Channel /** One node of the page tree; the recomposition unit (docs/adr-003-kotlin-node-tree-and-registry.md). */ class UINode internal constructor(val id: Int, val type: String) { @@ -12,8 +13,12 @@ class UINode internal constructor(val id: Int, val type: String) { /** Events the JS side registered a handler for. */ val events: SnapshotStateMap = mutableStateMapOf() val children: SnapshotStateList = mutableStateListOf() - /** One-shot commands waiting for the component to consume them (`x` op). */ - val commands: SnapshotStateList = mutableStateListOf() + /** + * One-shot commands for the component (`x` op), oldest first, each sent once its flush is applied. A queue, not + * snapshot state (docs/adr-004 ยง3.2): the engine thread sends while the main thread receives, and a state list + * written from both conflicts when either snapshot applies. + */ + internal val commands = Channel(Channel.UNLIMITED) internal var parent: UINode? = null /** Set once the creating flush is applied: `initial` props stop accepting writes. */ internal var created = false diff --git a/compose/src/commonMain/kotlin/app/tinyui/schema/NodeScope.kt b/compose/src/commonMain/kotlin/app/tinyui/schema/NodeScope.kt index 91bb85d..6ac5b6a 100644 --- a/compose/src/commonMain/kotlin/app/tinyui/schema/NodeScope.kt +++ b/compose/src/commonMain/kotlin/app/tinyui/schema/NodeScope.kt @@ -5,7 +5,6 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.compositionLocalOf import androidx.compose.runtime.getValue import androidx.compose.runtime.rememberUpdatedState -import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color import app.tinyui.node.Command @@ -51,11 +50,7 @@ fun NodeScope.Commands(handler: suspend (Command) -> Unit) { // the effect outlives recompositions: without this it would keep the first handler and the props it closed over val current by rememberUpdatedState(handler) LaunchedEffect(node) { - snapshotFlow { node.commands.toList() }.collect { pending -> - if (pending.isEmpty()) return@collect - node.commands.removeAll(pending) - for (c in pending) current(c) - } + for (c in node.commands) current(c) } } diff --git a/compose/src/commonTest/kotlin/app/tinyui/node/NodeTreeTest.kt b/compose/src/commonTest/kotlin/app/tinyui/node/NodeTreeTest.kt index 070afe2..134856e 100644 --- a/compose/src/commonTest/kotlin/app/tinyui/node/NodeTreeTest.kt +++ b/compose/src/commonTest/kotlin/app/tinyui/node/NodeTreeTest.kt @@ -1,11 +1,15 @@ package app.tinyui.node +import androidx.compose.runtime.snapshots.Snapshot import androidx.compose.ui.graphics.Color import app.tinyui.schema.ColorValue import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import app.tinyui.components.registerBuiltins import app.tinyui.schema.ComponentRegistry +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNull @@ -82,11 +86,100 @@ class NodeTreeTest { assertEquals(1, problems.size); assertTrue("initial prop" in problems.single().reason) problems.clear() tree.apply("""[["x",1,"setText",{"text":"z"}],["x",1,"setText",{}],["x",1,"setText",{"text":3}],["x",1,"focus",{}]]""") - assertEquals(listOf("setText", "focus"), tree.node(1)!!.commands.map { it.name }) - assertEquals("z", tree.node(1)!!.commands[0].args["text"]) + val sent = received(tree.node(1)!!) + assertEquals(listOf("setText", "focus"), sent.map { it.name }) + assertEquals("z", sent[0].args["text"]) assertEquals(2, problems.size) } + @Test + fun receivingCommandsNeverConflictsWithAFlushBeingApplied() { + tree.apply("""[["c",1,"TextField"],["i",0,1,0],["x",1,"setText",{"text":"a"}]]""") + val node = tree.node(1)!! + // the main thread takes a command inside its own snapshot while the engine thread applies the next flush: + // with commands as snapshot state both wrote the same list, and the later apply failed with a conflict + val main = Snapshot.takeMutableSnapshot() + val first = main.enter { node.commands.tryReceive().getOrNull() } + tree.apply("""[["x",1,"focus",{}]]""") + main.apply().check() + main.dispose() + assertEquals("setText", first?.name) + assertEquals(listOf("focus"), received(node).map { it.name }) + } + + @Test + fun aCommandArrivesOnlyAfterItsFlushIsApplied() = runTest { + tree.apply("""[["c",1,"TextField"],["i",0,1,0]]""") + val node = tree.node(1)!! + val events = ArrayList() + // runs on the spot, inside the send: if the command went out before the commit, it would come first + val receiver = launch(UnconfinedTestDispatcher(testScheduler)) { node.commands.receive(); events += "received" } + val applied = Snapshot.registerApplyObserver { changed, _ -> if (node.props in changed) events += "applied" } + tree.apply("""[["x",1,"focus",{}],["p",1,"placeholder","hint"]]""") + applied.dispose() + receiver.cancel() + assertEquals(listOf("applied", "received"), events) + } + + @Test + fun problemsAreReportedOnceTheFlushIsApplied() { + val events = ArrayList() + lateinit var reporting: NodeTree + // what the sink sees of the tree at the moment it hears about a problem + reporting = NodeTree(ComponentRegistry().registerBuiltins()) { events += "reported:${reporting.node(1)!!.props["placeholder"]}" } + reporting.apply("""[["c",1,"TextField"],["i",0,1,0]]""") + val node = reporting.node(1)!! + val applied = Snapshot.registerApplyObserver { changed, _ -> if (node.props in changed) events += "applied" } + // the bad op comes first in the flush, the sink hears of it after the commit, with the whole flush in place + reporting.apply("""[["x",1,"nope",{}],["p",1,"placeholder","hint"]]""") + applied.dispose() + assertEquals(listOf("applied", "reported:hint"), events) + } + + @Test + fun aSinkWritingToTheTreeFromReportGetsAFlushOfItsOwn() { + var reentered = false + lateinit var reentrant: NodeTree + reentrant = NodeTree(ComponentRegistry().registerBuiltins()) { + problems += it + // reported for the missing required prop; the sink answers by adding a node + if (!reentered) { reentered = true; reentrant.apply("""[["c",2,"Text"],["p",2,"text","x"],["i",0,2,1]]""") } + } + reentrant.apply("""[["c",1,"Text"],["i",0,1,0]]""") + assertTrue(reentered) + assertEquals(listOf(1, 2), ids(reentrant.root)) + assertEquals("x", reentrant.node(2)!!.props["text"]) + assertEquals("required prop text was not set in the creating flush", problems.single().reason) + } + + @Test + fun aSinkHearsOfAFlushOnlyOnceItsCommandsAreSent() { + var reentered = false + lateinit var reentrant: NodeTree + reentrant = NodeTree(ComponentRegistry().registerBuiltins()) { + if (!reentered) { reentered = true; reentrant.apply("""[["x",1,"setText",{"text":"inner"}]]""") } + } + reentrant.apply("""[["c",1,"TextField"],["i",0,1,0]]""") + // the bad op reports after the flush is complete: the sink's own command queues up behind this flush's + reentrant.apply("""[["x",1,"focus",{}],["x",1,"nope",{}]]""") + assertTrue(reentered) + assertEquals(listOf("focus", "setText"), received(reentrant.node(1)!!).map { it.name }) + } + + @Test + fun aRemovedNodeGetsNoMoreCommands() { + tree.apply("""[["c",1,"TextField"],["i",0,1,0],["c",2,"TextField"],["i",0,2,1]]""") + val first = tree.node(1)!! + val second = tree.node(2)!! + // one still waiting from an earlier flush, one sent in the flush that removes the node + tree.apply("""[["x",1,"focus",{}]]""") + tree.apply("""[["x",1,"setText",{"text":"a"}],["r",1],["x",2,"focus",{}]]""") + assertTrue(first.commands.isClosedForReceive) + assertEquals(listOf("focus"), received(second).map { it.name }) + } + + private fun received(node: UINode): List = generateSequence { node.commands.tryReceive().getOrNull() }.toList() + @Test fun requiredPropsAreCheckedWhenTheCreatingFlushEnds() { tree.apply("""[["c",1,"Text"],["i",0,1,0]]""")