Move protocol modifier deserialization into factory (#1640)

This is in preparation of adding index tracking to the protocol nodes to match support in the native widget nodes. ProtocolNode became an abstract class because of this, and common functionality to all subtypes moved into the class. Additionally, create a split between the protocol factory which users need to reference and the generated protocol factory APIs that are unstable and used internally. This simplifies how these types and API signatures which use them look to users.
cashapp · Oct 27, 2023 · 85cc945 · 85cc945
1 parent 020ee47
commit 85cc945
Show file tree

Hide file tree

Showing 27 changed files with 224 additions and 299 deletions.
diff --git a/redwood-compose/src/commonTest/kotlin/app/cash/redwood/compose/ChangeListenerTest.kt b/redwood-compose/src/commonTest/kotlin/app/cash/redwood/compose/ChangeListenerTest.kt
@@ -35,7 +35,7 @@ import com.example.redwood.testing.compose.TestRow
 import com.example.redwood.testing.compose.TestSchemaProtocolBridge
 import com.example.redwood.testing.compose.TestScope
 import com.example.redwood.testing.compose.Text
-import com.example.redwood.testing.widget.TestSchemaProtocolNodeFactory
+import com.example.redwood.testing.widget.TestSchemaProtocolFactory
 import com.example.redwood.testing.widget.TestSchemaTestingWidgetFactory
 import com.example.redwood.testing.widget.TestSchemaWidgetFactories
 import com.example.redwood.testing.widget.TestSchemaWidgetFactory
@@ -56,7 +56,7 @@ class ProtocolChangeListenerTest : AbstractChangeListenerTest() {
     snapshot: () -> T,
   ): TestRedwoodComposition<T> {
     val composeBridge = TestSchemaProtocolBridge.create()
-    val widgetBridge = ProtocolBridge(MutableListChildren(), TestSchemaProtocolNodeFactory(factories)) {
+    val widgetBridge = ProtocolBridge(MutableListChildren(), TestSchemaProtocolFactory(factories)) {
       throw AssertionError()
     }
     return TestRedwoodComposition(this, composeBridge.provider, composeBridge.root) {

diff --git a/...od-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/widget/ProtocolBridge.kt b/...od-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/widget/ProtocolBridge.kt
@@ -28,7 +28,6 @@ import app.cash.redwood.protocol.Create
 import app.cash.redwood.protocol.EventSink
 import app.cash.redwood.protocol.Id
 import app.cash.redwood.protocol.ModifierChange
-import app.cash.redwood.protocol.ModifierElement
 import app.cash.redwood.protocol.PropertyChange
 import app.cash.redwood.widget.ChangeListener
 import app.cash.redwood.widget.Widget
@@ -45,9 +44,13 @@ import kotlin.native.ObjCName
 @ObjCName("ProtocolBridge", exact = true)
 public class ProtocolBridge<W : Any>(
   container: Widget.Children<W>,
-  private val factory: ProtocolNodeFactory<W>,
+  factory: ProtocolFactory<W>,
   private val eventSink: EventSink,
 ) : ChangesSink {
+  private val factory = requireNotNull(factory as? GeneratedProtocolFactory<W>) {
+    "Factory ${factory::class} was not generated by Redwood or is out of date"
+  }
+
   private val nodes = mutableMapOf<Id, ProtocolNode<W>>(
     Id.Root to RootProtocolNode(container),
   )
@@ -59,7 +62,7 @@ public class ProtocolBridge<W : Any>(
       val id = change.id
       when (change) {
         is Create -> {
-          val node = factory.create(change.tag) ?: continue
+          val node = factory.createNode(change.tag) ?: continue
           val old = nodes.put(change.id, node)
           require(old == null) {
             "Insert attempted to replace existing widget with ID ${change.id.value}"
@@ -90,8 +93,11 @@ public class ProtocolBridge<W : Any>(
           }
         }
         is ModifierChange -> {
+          val modifier = change.elements.fold<_, Modifier>(Modifier) { modifier, element ->
+            modifier.then(factory.createModifier(element))
+          }
           val node = node(id)
-          node.updateModifier(change.elements)
+          node.updateModifier(modifier)
 
           val widget = node.widget
           if (widget is ChangeListener) {
@@ -126,11 +132,7 @@ public class ProtocolBridge<W : Any>(
 @OptIn(RedwoodCodegenApi::class)
 private class RootProtocolNode<W : Any>(
   private val children: Widget.Children<W>,
-) : ProtocolNode<W>, Widget<W> {
-  override fun updateModifier(elements: List<ModifierElement>) {
-    throw AssertionError("unexpected: $elements")
-  }
-
+) : ProtocolNode<W>(), Widget<W> {
   override fun apply(change: PropertyChange, eventSink: EventSink) {
     throw AssertionError("unexpected: $change")
   }
@@ -142,10 +144,6 @@ private class RootProtocolNode<W : Any>(
 
   override val widget: Widget<W> get() = this
 
-  override fun attachTo(container: Widget.Children<W>) {
-    throw AssertionError()
-  }
-
   override val value: W get() = throw AssertionError()
 
   override var modifier: Modifier

diff --git a/...od/protocol/widget/ProtocolNodeFactory.kt → ...edwood/protocol/widget/ProtocolFactory.kt b/...od/protocol/widget/ProtocolNodeFactory.kt → ...edwood/protocol/widget/ProtocolFactory.kt
@@ -15,21 +15,41 @@
  */
 package app.cash.redwood.protocol.widget
 
+import app.cash.redwood.Modifier
 import app.cash.redwood.RedwoodCodegenApi
+import app.cash.redwood.protocol.ModifierElement
 import app.cash.redwood.protocol.WidgetTag
 import kotlin.native.ObjCName
 
-@ObjCName("ProtocolNodeFactory", exact = true)
-public interface ProtocolNodeFactory<W : Any> {
+/**
+ * A marker interface for the widget-side factory of the protocol.
+ *
+ * @see ProtocolBridge
+ */
+@ObjCName("ProtocolFactory", exact = true)
+public interface ProtocolFactory<W : Any>
+
+/**
+ * [ProtocolFactory] but containing codegen APIs.
+ *
+ * @suppress
+ */
+@RedwoodCodegenApi
+public interface GeneratedProtocolFactory<W : Any> : ProtocolFactory<W> {
   /**
    * Create a new protocol node of the specified [tag].
    *
    * Invalid [tag] values can either produce an exception or result in `null` being returned.
    * If `null` is returned, the caller should make every effort to ignore this node and
    * continue executing.
+   */
+  public fun createNode(tag: WidgetTag): ProtocolNode<W>?
+
+  /**
+   * Create a new modifier from the specified [element].
    *
-   * @suppress
+   * Invalid [`element.tag`][ModifierElement.tag] values can either produce an exception
+   * or result in the unit [`Modifier`][Modifier.Companion] being returned.
    */
-  @RedwoodCodegenApi
-  public fun create(tag: WidgetTag): ProtocolNode<W>?
+  public fun createModifier(element: ModifierElement): Modifier
 }
diff --git a/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/widget/ProtocolNode.kt b/redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/widget/ProtocolNode.kt
@@ -15,10 +15,10 @@
  */
 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.ModifierElement
 import app.cash.redwood.protocol.PropertyChange
 import app.cash.redwood.widget.Widget
 
@@ -28,19 +28,27 @@ import app.cash.redwood.widget.Widget
  * @suppress
  */
 @RedwoodCodegenApi
-public interface ProtocolNode<W : Any> {
-  public val widget: Widget<W>
+public abstract class ProtocolNode<W : Any> {
+  public abstract val widget: Widget<W>
+
+  private var container: Widget.Children<W>? = null
 
   /**
    * 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>)
+  public fun attachTo(container: Widget.Children<W>) {
+    check(this.container == null)
+    this.container = container
+  }
 
-  public fun apply(change: PropertyChange, eventSink: EventSink)
+  public abstract fun apply(change: PropertyChange, eventSink: EventSink)
 
-  public fun updateModifier(elements: List<ModifierElement>)
+  public fun updateModifier(modifier: Modifier) {
+    widget.modifier = modifier
+    container?.onModifierUpdated()
+  }
 
   /**
    * Return one of this node's children groups by its [tag].
@@ -49,5 +57,5 @@ public interface ProtocolNode<W : Any> {
    * If `null` is returned, the caller should make every effort to ignore these children and
    * continue executing.
    */
-  public fun children(tag: ChildrenTag): Widget.Children<W>?
+  public abstract fun children(tag: ChildrenTag): Widget.Children<W>?
 }
diff --git a/...rotocol-host/src/commonTest/kotlin/app/cash/redwood/protocol/widget/ProtocolBridgeTest.kt b/...rotocol-host/src/commonTest/kotlin/app/cash/redwood/protocol/widget/ProtocolBridgeTest.kt
@@ -28,7 +28,7 @@ import app.cash.redwood.widget.MutableListChildren
 import assertk.assertThat
 import assertk.assertions.hasMessage
 import assertk.assertions.isEqualTo
-import com.example.redwood.testing.widget.TestSchemaProtocolNodeFactory
+import com.example.redwood.testing.widget.TestSchemaProtocolFactory
 import com.example.redwood.testing.widget.TestSchemaWidgetFactories
 import kotlin.test.Test
 import kotlin.test.assertFailsWith
@@ -38,7 +38,7 @@ class ProtocolBridgeTest {
   @Test fun createRootIdThrows() {
     val bridge = ProtocolBridge(
       container = MutableListChildren(),
-      factory = TestSchemaProtocolNodeFactory(
+      factory = TestSchemaProtocolFactory(
         provider = TestSchemaWidgetFactories(
           TestSchema = EmptyTestSchemaWidgetFactory(),
           RedwoodLayout = EmptyRedwoodLayoutWidgetFactory(),
@@ -62,7 +62,7 @@ class ProtocolBridgeTest {
   @Test fun duplicateIdThrows() {
     val bridge = ProtocolBridge(
       container = MutableListChildren(),
-      factory = TestSchemaProtocolNodeFactory(
+      factory = TestSchemaProtocolFactory(
         provider = TestSchemaWidgetFactories(
           TestSchema = EmptyTestSchemaWidgetFactory(),
           RedwoodLayout = EmptyRedwoodLayoutWidgetFactory(),
@@ -87,7 +87,7 @@ class ProtocolBridgeTest {
   @Test fun removeRemoves() {
     val bridge = ProtocolBridge(
       container = MutableListChildren(),
-      factory = TestSchemaProtocolNodeFactory(
+      factory = TestSchemaProtocolFactory(
         provider = TestSchemaWidgetFactories(
           TestSchema = EmptyTestSchemaWidgetFactory(),
           RedwoodLayout = EmptyRedwoodLayoutWidgetFactory(),
@@ -144,7 +144,7 @@ class ProtocolBridgeTest {
     var modifierUpdateCount = 0
     val bridge = ProtocolBridge(
       container = MutableListChildren(modifierUpdated = { modifierUpdateCount++ }),
-      factory = TestSchemaProtocolNodeFactory(
+      factory = TestSchemaProtocolFactory(
         provider = TestSchemaWidgetFactories(
           TestSchema = EmptyTestSchemaWidgetFactory(),
           RedwoodLayout = EmptyRedwoodLayoutWidgetFactory(),