Skip to content

Commit

Permalink
Track the index of a ProtocolNode within its parent container (#1645)
Browse files Browse the repository at this point in the history
This uses a new type, ProtocolChildren, to maintain a list of ProtocolNodes corresponding to a Widget.Children grouping. This class is mostly a copy of ChildrenNode from redwood-compose. In the future hopefully we can unify their logic.
  • Loading branch information
JakeWharton authored Oct 27, 2023
1 parent 1473ab0 commit 7ac2071
Show file tree
Hide file tree
Showing 6 changed files with 248 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ public class ProtocolBridge<W : Any>(
when (change) {
is Add -> {
val child = node(change.childId)
children.insert(change.index, child.widget)
child.attachTo(children)
children.insert(change.index, child)
}
is Move -> {
children.move(change.fromIndex, change.toIndex, change.count)
Expand Down Expand Up @@ -131,8 +130,10 @@ public class ProtocolBridge<W : Any>(

@OptIn(RedwoodCodegenApi::class)
private class RootProtocolNode<W : Any>(
private val children: Widget.Children<W>,
children: Widget.Children<W>,
) : ProtocolNode<W>(), Widget<W> {
private val children = ProtocolChildren(children)

override fun apply(change: PropertyChange, eventSink: EventSink) {
throw AssertionError("unexpected: $change")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import app.cash.redwood.protocol.ChildrenTag
import app.cash.redwood.protocol.EventSink
import app.cash.redwood.protocol.PropertyChange
import app.cash.redwood.widget.Widget
import kotlin.math.max
import kotlin.math.min

/**
* A node which consumes protocol changes and applies them to a platform-specific representation.
Expand All @@ -31,17 +33,10 @@ import app.cash.redwood.widget.Widget
public abstract class ProtocolNode<W : Any> {
public abstract val widget: Widget<W>

private var container: Widget.Children<W>? = null
/** The index of [widget] within its parent [container]. */
internal var index: Int = -1

/**
* Record that this node's [widget] has been inserted into [container].
* Updates to this node's layout modifier will notify [container].
* This function may only be invoked once on each instance.
*/
public fun attachTo(container: Widget.Children<W>) {
check(this.container == null)
this.container = container
}
internal var container: Widget.Children<W>? = null

public abstract fun apply(change: PropertyChange, eventSink: EventSink)

Expand All @@ -57,5 +52,61 @@ public abstract class ProtocolNode<W : Any> {
* If `null` is returned, the caller should make every effort to ignore these children and
* continue executing.
*/
public abstract fun children(tag: ChildrenTag): Widget.Children<W>?
public abstract fun children(tag: ChildrenTag): ProtocolChildren<W>?
}

/**
* @suppress
*/
@RedwoodCodegenApi
public class ProtocolChildren<W : Any>(
public val children: Widget.Children<W>,
) {
private val nodes = mutableListOf<ProtocolNode<W>>()

internal fun insert(index: Int, node: ProtocolNode<W>) {
nodes.let { nodes ->
// Bump the index of any nodes which will be shifted.
for (i in index until nodes.size) {
nodes[i].index++
}

node.index = index
nodes.add(index, node)
}

children.let { children ->
node.container = children
children.insert(index, node.widget)
}
}

internal fun remove(index: Int, count: Int) {
nodes.let { nodes ->
nodes.remove(index, count)

// Drop the index of any nodes shifted after the removal.
for (i in index until nodes.size) {
nodes[i].index -= count
}
}

children.remove(index, count)
}

internal fun move(from: Int, to: Int, count: Int) {
nodes.let { nodes ->
nodes.move(from, to, count)

// If moving up, lower bound is from. If moving down, lower bound is to.
val lowerBound = min(from, to)
// If moving up, upper bound is to, If moving down, upper bound is from + count.
val upperBound = max(to, from + count)
for (i in lowerBound until upperBound) {
nodes[i].index = i
}
}

children.move(from, to, count)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
/*
* Copyright (C) 2023 Square, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package app.cash.redwood.protocol.widget

import app.cash.redwood.Modifier
import app.cash.redwood.RedwoodCodegenApi
import app.cash.redwood.protocol.ChildrenTag
import app.cash.redwood.protocol.EventSink
import app.cash.redwood.protocol.PropertyChange
import app.cash.redwood.widget.MutableListChildren
import app.cash.redwood.widget.Widget
import assertk.assertThat
import assertk.assertions.isEqualTo
import kotlin.test.Test

/**
* This class tests an implementation detail of [ProtocolChildren] which maintains the correct
* index value on its child [ProtocolNode]s. While it could conceivably be tested through public
* API it's far easier to validate the behavior this way.
*/
@OptIn(RedwoodCodegenApi::class)
class ChildrenNodeIndexTest {
private val root = ProtocolChildren(MutableListChildren<String>())

@Test fun insert() {
val a = WidgetNode(StringWidget("a"))
assertThat(a.index).isEqualTo(-1)
root.insert(0, a)
assertThat(a.index).isEqualTo(0)

val b = WidgetNode(StringWidget("a"))
root.insert(1, b)
assertThat(a.index).isEqualTo(0)
assertThat(b.index).isEqualTo(1)

val c = WidgetNode(StringWidget("a"))
root.insert(0, c)
assertThat(c.index).isEqualTo(0)
assertThat(a.index).isEqualTo(1)
assertThat(b.index).isEqualTo(2)
}

@Test fun remove() {
val a = WidgetNode(StringWidget("a"))
val b = WidgetNode(StringWidget("b"))
val c = WidgetNode(StringWidget("c"))
val d = WidgetNode(StringWidget("d"))
val e = WidgetNode(StringWidget("e"))
root.insert(0, a)
root.insert(1, b)
root.insert(2, c)
root.insert(3, d)
root.insert(4, e)
assertThat(a.index).isEqualTo(0)
assertThat(b.index).isEqualTo(1)
assertThat(c.index).isEqualTo(2)
assertThat(d.index).isEqualTo(3)
assertThat(e.index).isEqualTo(4)

root.remove(2, 1) // c
assertThat(a.index).isEqualTo(0)
assertThat(b.index).isEqualTo(1)
assertThat(d.index).isEqualTo(2)
assertThat(e.index).isEqualTo(3)

root.remove(1, 2) // b, d
assertThat(a.index).isEqualTo(0)
assertThat(e.index).isEqualTo(1)

root.remove(1, 1) // e
assertThat(a.index).isEqualTo(0)
}

@Test fun move() {
val a = WidgetNode(StringWidget("a"))
val b = WidgetNode(StringWidget("b"))
val c = WidgetNode(StringWidget("c"))
val d = WidgetNode(StringWidget("d"))
val e = WidgetNode(StringWidget("e"))
root.insert(0, a)
root.insert(1, b)
root.insert(2, c)
root.insert(3, d)
root.insert(4, e)
assertThat(a.index).isEqualTo(0)
assertThat(b.index).isEqualTo(1)
assertThat(c.index).isEqualTo(2)
assertThat(d.index).isEqualTo(3)
assertThat(e.index).isEqualTo(4)

root.move(0, 5, 1) // a 0 --> 4
assertThat(b.index).isEqualTo(0)
assertThat(c.index).isEqualTo(1)
assertThat(d.index).isEqualTo(2)
assertThat(e.index).isEqualTo(3)
assertThat(a.index).isEqualTo(4)

root.move(1, 4, 2) // c,d 1 --> 2
assertThat(b.index).isEqualTo(0)
assertThat(e.index).isEqualTo(1)
assertThat(c.index).isEqualTo(2)
assertThat(d.index).isEqualTo(3)
assertThat(a.index).isEqualTo(4)

root.move(2, 1, 3) // c,d,a 2 --> 1
assertThat(b.index).isEqualTo(0)
assertThat(c.index).isEqualTo(1)
assertThat(d.index).isEqualTo(2)
assertThat(a.index).isEqualTo(3)
assertThat(e.index).isEqualTo(4)
}
}

@OptIn(RedwoodCodegenApi::class)
private class WidgetNode(override val widget: StringWidget) : ProtocolNode<String>() {
override fun apply(change: PropertyChange, eventSink: EventSink) {
throw UnsupportedOperationException()
}

override fun children(tag: ChildrenTag): ProtocolChildren<String>? {
throw UnsupportedOperationException()
}
}

private class StringWidget(override val value: String) : Widget<String> {
override var modifier: Modifier = Modifier
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ internal object WidgetProtocol {
val ProtocolMismatchHandler =
ClassName("app.cash.redwood.protocol.widget", "ProtocolMismatchHandler")
val ProtocolNode = ClassName("app.cash.redwood.protocol.widget", "ProtocolNode")
val ProtocolChildren = ClassName("app.cash.redwood.protocol.widget", "ProtocolChildren")
val GeneratedProtocolFactory = ClassName("app.cash.redwood.protocol.widget", "GeneratedProtocolFactory")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ internal fun generateProtocolNode(
val type = schema.protocolNodeType(widget, host)
val widgetType = schema.widgetType(widget).parameterizedBy(typeVariableW)
val protocolType = WidgetProtocol.ProtocolNode.parameterizedBy(typeVariableW)
val (childrens, properties) = widget.traits.partition { it is ProtocolChildren }
return FileSpec.builder(type)
.addType(
TypeSpec.classBuilder(type)
Expand Down Expand Up @@ -262,7 +263,6 @@ internal fun generateProtocolNode(
.build(),
)
.apply {
val (childrens, properties) = widget.traits.partition { it is ProtocolChildren }
var nextSerializerId = 0
val serializerIds = mutableMapOf<TypeName, Int>()

Expand Down Expand Up @@ -360,38 +360,46 @@ internal fun generateProtocolNode(
.build(),
)

addFunction(
FunSpec.builder("children")
.addModifiers(OVERRIDE)
.addParameter("tag", Protocol.ChildrenTag)
.returns(RedwoodWidget.WidgetChildrenOfW.copy(nullable = true))
.apply {
if (childrens.isNotEmpty()) {
beginControlFlow("return when (tag.value)")
for (children in childrens) {
addStatement("%L -> _widget.%N", children.tag, children.name)
}
beginControlFlow("else ->")
addStatement(
"mismatchHandler.onUnknownChildren(%T(%L), tag)",
Protocol.WidgetTag,
widget.tag,
)
addStatement("null")
endControlFlow()
endControlFlow()
} else {
addStatement(
"mismatchHandler.onUnknownChildren(%T(%L), tag)",
Protocol.WidgetTag,
widget.tag,
)
addStatement("return null")
for (children in childrens) {
addProperty(
PropertySpec.builder(children.name, WidgetProtocol.ProtocolChildren.parameterizedBy(typeVariableW))
.addModifiers(PRIVATE)
.initializer("%T(widget.%N)", WidgetProtocol.ProtocolChildren, children.name)
.build(),
)
}
}
.addFunction(
FunSpec.builder("children")
.addModifiers(OVERRIDE)
.addParameter("tag", Protocol.ChildrenTag)
.returns(WidgetProtocol.ProtocolChildren.parameterizedBy(typeVariableW).copy(nullable = true))
.apply {
if (childrens.isNotEmpty()) {
beginControlFlow("return when (tag.value)")
for (children in childrens) {
addStatement("%L -> %N", children.tag, children.name)
}
beginControlFlow("else ->")
addStatement(
"mismatchHandler.onUnknownChildren(%T(%L), tag)",
Protocol.WidgetTag,
widget.tag,
)
addStatement("null")
endControlFlow()
endControlFlow()
} else {
addStatement(
"mismatchHandler.onUnknownChildren(%T(%L), tag)",
Protocol.WidgetTag,
widget.tag,
)
addStatement("return null")
}
.build(),
)
}
}
.build(),
)
.build(),
)
.build()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import app.cash.redwood.RedwoodCodegenApi
import app.cash.redwood.protocol.ChildrenTag
import app.cash.redwood.protocol.EventSink
import app.cash.redwood.protocol.PropertyChange
import app.cash.redwood.protocol.widget.ProtocolChildren
import app.cash.redwood.protocol.widget.ProtocolNode
import app.cash.redwood.widget.Widget
import kotlinx.serialization.json.JsonPrimitive

/**
Expand All @@ -34,7 +34,7 @@ internal class FakeProtocolNode : ProtocolNode<FakeWidget>() {
widget.label = (change.value as JsonPrimitive).content
}

override fun children(tag: ChildrenTag): Widget.Children<FakeWidget>? {
override fun children(tag: ChildrenTag): ProtocolChildren<FakeWidget>? {
error("unexpected call")
}
}

0 comments on commit 7ac2071

Please sign in to comment.