Skip to content

Commit

Permalink
Reapply "Defer serializing event args to JSON (#2218)" (#2239) (#2262)
Browse files Browse the repository at this point in the history
* Reapply "Defer serializing event args to JSON (#2218)" (#2239)

This reverts commit 5f8ca01.

* Track API change in test
  • Loading branch information
squarejesse authored Aug 26, 2024
1 parent 56d3e81 commit ebf4516
Show file tree
Hide file tree
Showing 16 changed files with 187 additions and 74 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ New:
- Introduce a `LoadingStrategy` interface to manage `LazyList` preloading.

Changed:
- In Treehouse, events from the UI are now serialized on a background thread. This means that there is both a delay and a thread change between when a UI binding sends an event and when that object is converted to JSON. All arguments to events must not be mutable and support property reads on any thread. Best practice is for all event arguments to be completely immutable.
- `ProtocolFactory` interface is now sealed as arbitrary subtypes were never supported. Only schema-generated subtypes should be used.
- `UIViewLazyList` doesn't crash with a `NullPointerException` if cells are added, removed, and re-added without being reused.
- Change `UiConfiguration.viewportSize` to be nullable. A null `viewportSize` indicates the viewport's size has not been resolved yet.
Expand Down
21 changes: 19 additions & 2 deletions redwood-protocol-host/api/redwood-protocol-host.api
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ public abstract interface class app/cash/redwood/protocol/host/GeneratedProtocol
}

public final class app/cash/redwood/protocol/host/HostProtocolAdapter : app/cash/redwood/protocol/ChangesSink {
public synthetic fun <init> (Ljava/lang/String;Lapp/cash/redwood/widget/Widget$Children;Lapp/cash/redwood/protocol/host/ProtocolFactory;Lapp/cash/redwood/protocol/EventSink;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (Ljava/lang/String;Lapp/cash/redwood/widget/Widget$Children;Lapp/cash/redwood/protocol/host/ProtocolFactory;Lapp/cash/redwood/protocol/host/UiEventSink;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public final fun close ()V
public fun sendChanges (Ljava/util/List;)V
}
Expand Down Expand Up @@ -35,7 +35,7 @@ public final class app/cash/redwood/protocol/host/ProtocolMismatchHandler$Compan

public abstract class app/cash/redwood/protocol/host/ProtocolNode {
public synthetic fun <init> (IILkotlin/jvm/internal/DefaultConstructorMarker;)V
public abstract fun apply (Lapp/cash/redwood/protocol/PropertyChange;Lapp/cash/redwood/protocol/EventSink;)V
public abstract fun apply (Lapp/cash/redwood/protocol/PropertyChange;Lapp/cash/redwood/protocol/host/UiEventSink;)V
public abstract fun children-dBpC-2Y (I)Lapp/cash/redwood/protocol/host/ProtocolChildren;
public abstract fun detach ()V
public final fun getId-0HhLjSo ()I
Expand All @@ -46,6 +46,23 @@ public abstract class app/cash/redwood/protocol/host/ProtocolNode {
public abstract fun visitIds (Lkotlin/jvm/functions/Function1;)V
}

public final class app/cash/redwood/protocol/host/UiEvent {
public synthetic fun <init> (IILjava/util/List;Ljava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (IILjava/util/List;Ljava/util/List;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun equals (Ljava/lang/Object;)Z
public final fun getArgs ()Ljava/util/List;
public final fun getId-0HhLjSo ()I
public final fun getSerializationStrategies ()Ljava/util/List;
public final fun getTag-RNF89mI ()I
public fun hashCode ()I
public final fun toProtocol (Lkotlinx/serialization/json/Json;)Lapp/cash/redwood/protocol/Event;
public fun toString ()Ljava/lang/String;
}

public abstract interface class app/cash/redwood/protocol/host/UiEventSink {
public abstract fun sendEvent (Lapp/cash/redwood/protocol/host/UiEvent;)V
}

public final class app/cash/redwood/protocol/host/VersionKt {
public static final fun getHostRedwoodVersion ()Ljava/lang/String;
}
Expand Down
26 changes: 24 additions & 2 deletions redwood-protocol-host/api/redwood-protocol-host.klib.api
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
// - Show declarations: true

// Library unique name: <app.cash.redwood:redwood-protocol-host>
abstract fun interface app.cash.redwood.protocol.host/UiEventSink { // app.cash.redwood.protocol.host/UiEventSink|null[0]
abstract fun sendEvent(app.cash.redwood.protocol.host/UiEvent) // app.cash.redwood.protocol.host/UiEventSink.sendEvent|sendEvent(app.cash.redwood.protocol.host.UiEvent){}[0]
}

abstract interface <#A: kotlin/Any> app.cash.redwood.protocol.host/GeneratedProtocolFactory : app.cash.redwood.protocol.host/ProtocolFactory<#A> { // app.cash.redwood.protocol.host/GeneratedProtocolFactory|null[0]
abstract fun createModifier(app.cash.redwood.protocol/ModifierElement): app.cash.redwood/Modifier // app.cash.redwood.protocol.host/GeneratedProtocolFactory.createModifier|createModifier(app.cash.redwood.protocol.ModifierElement){}[0]
abstract fun createNode(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/WidgetTag): app.cash.redwood.protocol.host/ProtocolNode<#A>? // app.cash.redwood.protocol.host/GeneratedProtocolFactory.createNode|createNode(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.WidgetTag){}[0]
Expand Down Expand Up @@ -41,15 +45,15 @@ abstract class <#A: kotlin/Any> app.cash.redwood.protocol.host/ProtocolNode { //
final fun <get-id>(): app.cash.redwood.protocol/Id // app.cash.redwood.protocol.host/ProtocolNode.id.<get-id>|<get-id>(){}[0]
final fun <set-id>(app.cash.redwood.protocol/Id) // app.cash.redwood.protocol.host/ProtocolNode.id.<set-id>|<set-id>(app.cash.redwood.protocol.Id){}[0]

abstract fun apply(app.cash.redwood.protocol/PropertyChange, app.cash.redwood.protocol/EventSink) // app.cash.redwood.protocol.host/ProtocolNode.apply|apply(app.cash.redwood.protocol.PropertyChange;app.cash.redwood.protocol.EventSink){}[0]
abstract fun apply(app.cash.redwood.protocol/PropertyChange, app.cash.redwood.protocol.host/UiEventSink) // app.cash.redwood.protocol.host/ProtocolNode.apply|apply(app.cash.redwood.protocol.PropertyChange;app.cash.redwood.protocol.host.UiEventSink){}[0]
abstract fun children(app.cash.redwood.protocol/ChildrenTag): app.cash.redwood.protocol.host/ProtocolChildren<#A>? // app.cash.redwood.protocol.host/ProtocolNode.children|children(app.cash.redwood.protocol.ChildrenTag){}[0]
abstract fun detach() // app.cash.redwood.protocol.host/ProtocolNode.detach|detach(){}[0]
abstract fun visitIds(kotlin/Function1<app.cash.redwood.protocol/Id, kotlin/Unit>) // app.cash.redwood.protocol.host/ProtocolNode.visitIds|visitIds(kotlin.Function1<app.cash.redwood.protocol.Id,kotlin.Unit>){}[0]
final fun updateModifier(app.cash.redwood/Modifier) // app.cash.redwood.protocol.host/ProtocolNode.updateModifier|updateModifier(app.cash.redwood.Modifier){}[0]
}

final class <#A: kotlin/Any> app.cash.redwood.protocol.host/HostProtocolAdapter : app.cash.redwood.protocol/ChangesSink { // app.cash.redwood.protocol.host/HostProtocolAdapter|null[0]
constructor <init>(app.cash.redwood.protocol/RedwoodVersion, app.cash.redwood.widget/Widget.Children<#A>, app.cash.redwood.protocol.host/ProtocolFactory<#A>, app.cash.redwood.protocol/EventSink) // app.cash.redwood.protocol.host/HostProtocolAdapter.<init>|<init>(app.cash.redwood.protocol.RedwoodVersion;app.cash.redwood.widget.Widget.Children<1:0>;app.cash.redwood.protocol.host.ProtocolFactory<1:0>;app.cash.redwood.protocol.EventSink){}[0]
constructor <init>(app.cash.redwood.protocol/RedwoodVersion, app.cash.redwood.widget/Widget.Children<#A>, app.cash.redwood.protocol.host/ProtocolFactory<#A>, app.cash.redwood.protocol.host/UiEventSink) // app.cash.redwood.protocol.host/HostProtocolAdapter.<init>|<init>(app.cash.redwood.protocol.RedwoodVersion;app.cash.redwood.widget.Widget.Children<1:0>;app.cash.redwood.protocol.host.ProtocolFactory<1:0>;app.cash.redwood.protocol.host.UiEventSink){}[0]

final fun close() // app.cash.redwood.protocol.host/HostProtocolAdapter.close|close(){}[0]
final fun sendChanges(kotlin.collections/List<app.cash.redwood.protocol/Change>) // app.cash.redwood.protocol.host/HostProtocolAdapter.sendChanges|sendChanges(kotlin.collections.List<app.cash.redwood.protocol.Change>){}[0]
Expand All @@ -65,5 +69,23 @@ final class <#A: kotlin/Any> app.cash.redwood.protocol.host/ProtocolChildren { /
final fun visitIds(kotlin/Function1<app.cash.redwood.protocol/Id, kotlin/Unit>) // app.cash.redwood.protocol.host/ProtocolChildren.visitIds|visitIds(kotlin.Function1<app.cash.redwood.protocol.Id,kotlin.Unit>){}[0]
}

final class app.cash.redwood.protocol.host/UiEvent { // app.cash.redwood.protocol.host/UiEvent|null[0]
constructor <init>(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/EventTag, kotlin.collections/List<kotlin/Any?> = ..., kotlin.collections/List<kotlinx.serialization/SerializationStrategy<kotlin/Any?>> = ...) // app.cash.redwood.protocol.host/UiEvent.<init>|<init>(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.EventTag;kotlin.collections.List<kotlin.Any?>;kotlin.collections.List<kotlinx.serialization.SerializationStrategy<kotlin.Any?>>){}[0]

final val args // app.cash.redwood.protocol.host/UiEvent.args|{}args[0]
final fun <get-args>(): kotlin.collections/List<kotlin/Any?> // app.cash.redwood.protocol.host/UiEvent.args.<get-args>|<get-args>(){}[0]
final val id // app.cash.redwood.protocol.host/UiEvent.id|{}id[0]
final fun <get-id>(): app.cash.redwood.protocol/Id // app.cash.redwood.protocol.host/UiEvent.id.<get-id>|<get-id>(){}[0]
final val serializationStrategies // app.cash.redwood.protocol.host/UiEvent.serializationStrategies|{}serializationStrategies[0]
final fun <get-serializationStrategies>(): kotlin.collections/List<kotlinx.serialization/SerializationStrategy<kotlin/Any?>> // app.cash.redwood.protocol.host/UiEvent.serializationStrategies.<get-serializationStrategies>|<get-serializationStrategies>(){}[0]
final val tag // app.cash.redwood.protocol.host/UiEvent.tag|{}tag[0]
final fun <get-tag>(): app.cash.redwood.protocol/EventTag // app.cash.redwood.protocol.host/UiEvent.tag.<get-tag>|<get-tag>(){}[0]

final fun equals(kotlin/Any?): kotlin/Boolean // app.cash.redwood.protocol.host/UiEvent.equals|equals(kotlin.Any?){}[0]
final fun hashCode(): kotlin/Int // app.cash.redwood.protocol.host/UiEvent.hashCode|hashCode(){}[0]
final fun toProtocol(kotlinx.serialization.json/Json): app.cash.redwood.protocol/Event // app.cash.redwood.protocol.host/UiEvent.toProtocol|toProtocol(kotlinx.serialization.json.Json){}[0]
final fun toString(): kotlin/String // app.cash.redwood.protocol.host/UiEvent.toString|toString(){}[0]
}

final val app.cash.redwood.protocol.host/hostRedwoodVersion // app.cash.redwood.protocol.host/hostRedwoodVersion|{}hostRedwoodVersion[0]
final fun <get-hostRedwoodVersion>(): app.cash.redwood.protocol/RedwoodVersion // app.cash.redwood.protocol.host/hostRedwoodVersion.<get-hostRedwoodVersion>|<get-hostRedwoodVersion>(){}[0]
1 change: 1 addition & 0 deletions redwood-protocol-host/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ redwoodBuild {
}

apply plugin: 'com.github.gmazzo.buildconfig'
apply plugin: 'dev.drewhamilton.poko'

kotlin {
sourceSets {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import app.cash.redwood.protocol.ChildrenChange.Move
import app.cash.redwood.protocol.ChildrenChange.Remove
import app.cash.redwood.protocol.ChildrenTag
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.PropertyChange
Expand All @@ -50,7 +49,7 @@ public class HostProtocolAdapter<W : Any>(
guestVersion: RedwoodVersion,
container: Widget.Children<W>,
factory: ProtocolFactory<W>,
private val eventSink: EventSink,
private val eventSink: UiEventSink,
) : ChangesSink {
private val factory = when (factory) {
is GeneratedProtocolFactory -> factory
Expand Down Expand Up @@ -381,7 +380,7 @@ private class RootProtocolNode<W : Any>(
Widget<W> {
private val children = ProtocolChildren(children)

override fun apply(change: PropertyChange, eventSink: EventSink) {
override fun apply(change: PropertyChange, eventSink: UiEventSink) {
throw AssertionError("unexpected: $change")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package app.cash.redwood.protocol.host
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.Id
import app.cash.redwood.protocol.PropertyChange
import app.cash.redwood.protocol.WidgetTag
Expand Down Expand Up @@ -49,7 +48,7 @@ public abstract class ProtocolNode<W : Any>(
/** Assigned when the node is added to the pool. */
internal var shapeHash = 0L

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

public fun updateModifier(modifier: Modifier) {
widget.modifier = modifier
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/*
* Copyright (C) 2024 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.host

import app.cash.redwood.protocol.Event
import app.cash.redwood.protocol.EventSink
import app.cash.redwood.protocol.EventTag
import app.cash.redwood.protocol.Id
import dev.drewhamilton.poko.Poko
import kotlinx.serialization.SerializationStrategy
import kotlinx.serialization.json.Json

/**
* A version of [Event] whose arguments have not yet been serialized to JSON and is thus
* cheap to create on the UI thread.
*/
@Poko
public class UiEvent(
public val id: Id,
public val tag: EventTag,
public val args: List<Any?> = emptyList(),
public val serializationStrategies: List<SerializationStrategy<Any?>> = emptyList(),
) {
init {
check(args.size == serializationStrategies.size) {
"Properties 'args' and 'serializationStrategies' must have the same size. " +
"Found ${args.size} and ${serializationStrategies.size}"
}
}

/** Serialize [args] into a JSON model using [serializationStrategies] into an [Event]. */
public fun toProtocol(json: Json): Event {
return Event(
id = id,
tag = tag,
args = List(args.size) {
json.encodeToJsonElement(serializationStrategies[it], args[it])
},
)
}
}

/** A version of [EventSink] which consumes [UiEvent]s. */
public fun interface UiEventSink {
public fun sendEvent(uiEvent: UiEvent)
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package app.cash.redwood.protocol.host
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.Id
import app.cash.redwood.protocol.PropertyChange
import app.cash.redwood.protocol.WidgetTag
Expand Down Expand Up @@ -128,7 +127,7 @@ class ChildrenNodeIndexTest {

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import app.cash.redwood.layout.testing.RedwoodLayoutTestingWidgetFactory
import app.cash.redwood.lazylayout.testing.RedwoodLazyLayoutTestingWidgetFactory
import app.cash.redwood.protocol.ChildrenTag
import app.cash.redwood.protocol.Event
import app.cash.redwood.protocol.EventSink
import app.cash.redwood.protocol.EventTag
import app.cash.redwood.protocol.Id
import app.cash.redwood.protocol.ModifierElement
Expand Down Expand Up @@ -258,7 +257,7 @@ class ProtocolFactoryTest {
)
val textInput = factory.createNode(Id(1), WidgetTag(5))!!

val throwingEventSink = EventSink { error(it) }
val throwingEventSink = UiEventSink { error(it) }
textInput.apply(PropertyChange(Id(1), PropertyTag(2), JsonPrimitive("PT10S")), throwingEventSink)

assertThat((textInput.widget.value as TextInputValue).customType).isEqualTo(10.seconds)
Expand All @@ -275,7 +274,7 @@ class ProtocolFactoryTest {
val button = factory.createNode(Id(1), WidgetTag(4))!!

val change = PropertyChange(Id(1), PropertyTag(345432))
val eventSink = EventSink { throw UnsupportedOperationException() }
val eventSink = UiEventSink { throw UnsupportedOperationException() }
val t = assertFailsWith<IllegalArgumentException> {
button.apply(change, eventSink)
}
Expand Down Expand Up @@ -315,12 +314,12 @@ class ProtocolFactoryTest {
)
val textInput = factory.createNode(Id(1), WidgetTag(5))!!

val eventSink = RecordingEventSink()
val eventSink = RecordingUiEventSink()
textInput.apply(PropertyChange(Id(1), PropertyTag(4), JsonPrimitive(true)), eventSink)

(textInput.widget.value as TextInputValue).onChangeCustomType!!.invoke(10.seconds)

assertThat(eventSink.events.single())
assertThat(eventSink.events.single().toProtocol(json))
.isEqualTo(Event(Id(1), EventTag(4), listOf(JsonPrimitive("PT10S"))))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,10 @@
*/
package app.cash.redwood.protocol.host

import app.cash.redwood.protocol.Event
import app.cash.redwood.protocol.EventSink
class RecordingUiEventSink : UiEventSink {
val events = mutableListOf<UiEvent>()

class RecordingEventSink : EventSink {
val events = mutableListOf<Event>()

override fun sendEvent(event: Event) {
events += event
override fun sendEvent(uiEvent: UiEvent) {
events += uiEvent
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import com.example.redwood.testapp.testing.TestSchemaTestingWidgetFactory
import com.example.redwood.testapp.widget.TestSchemaWidgetSystem
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.coroutineScope
import kotlinx.serialization.json.Json

/**
* Like [TestSchemaTester], but this also hooks up Redwood's protocol mechanism. That's necessary
Expand All @@ -64,7 +65,7 @@ class ViewRecyclingTester(
container = widgetContainer,
factory = widgetProtocolFactory,
eventSink = { event ->
guestAdapter.sendEvent(event)
guestAdapter.sendEvent(event.toProtocol(Json.Default))
},
)

Expand Down
Loading

0 comments on commit ebf4516

Please sign in to comment.