From 804ca648788266b7b4eecd35b19118b673b11b90 Mon Sep 17 00:00:00 2001 From: Ian Clarke Date: Mon, 26 Dec 2022 13:34:44 -0600 Subject: [PATCH] Make value synchronization more robust and fix bug (#422) --- api/kweb-core.api | 20 +++ build.gradle.kts | 15 ++ src/main/kotlin/kweb/ValueElement.kt | 169 ++++++++++++++++++++++ src/main/kotlin/kweb/prelude.kt | 131 +---------------- src/main/resources/kweb/kweb_bootstrap.js | 2 +- src/test/kotlin/kweb/SelectValueTest.kt | 3 - src/test/kotlin/kweb/StringDiffTest.kt | 4 +- 7 files changed, 212 insertions(+), 132 deletions(-) create mode 100644 src/main/kotlin/kweb/ValueElement.kt diff --git a/api/kweb-core.api b/api/kweb-core.api index 4fcb88a42e..4cf720b27a 100644 --- a/api/kweb-core.api +++ b/api/kweb-core.api @@ -596,6 +596,26 @@ public final class kweb/ValueElement$DiffData$Companion { public final fun serializer ()Lkotlinx/serialization/KSerializer; } +public final class kweb/ValueElement$LastModificationSource : java/lang/Enum { + public static final field Browser Lkweb/ValueElement$LastModificationSource; + public static final field Server Lkweb/ValueElement$LastModificationSource; + public static fun valueOf (Ljava/lang/String;)Lkweb/ValueElement$LastModificationSource; + public static fun values ()[Lkweb/ValueElement$LastModificationSource; +} + +public final class kweb/ValueElement$Value { + public fun (Ljava/lang/String;Lkweb/ValueElement$LastModificationSource;)V + public final fun component1 ()Ljava/lang/String; + public final fun component2 ()Lkweb/ValueElement$LastModificationSource; + public final fun copy (Ljava/lang/String;Lkweb/ValueElement$LastModificationSource;)Lkweb/ValueElement$Value; + public static synthetic fun copy$default (Lkweb/ValueElement$Value;Ljava/lang/String;Lkweb/ValueElement$LastModificationSource;ILjava/lang/Object;)Lkweb/ValueElement$Value; + public fun equals (Ljava/lang/Object;)Z + public final fun getLastModificationSource ()Lkweb/ValueElement$LastModificationSource; + public final fun getValue ()Ljava/lang/String; + public fun hashCode ()I + public fun toString ()Ljava/lang/String; +} + public abstract class kweb/ViewportHeight { public abstract fun getValue ()Ljava/lang/String; } diff --git a/build.gradle.kts b/build.gradle.kts index 0e8c0004c4..c8772b66ff 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -1,3 +1,4 @@ +import org.gradle.api.tasks.testing.logging.TestExceptionFormat.FULL import java.net.URL plugins { @@ -74,6 +75,20 @@ dependencies { testImplementation("org.awaitility:awaitility:4.2.0") } +tasks.test { + testLogging { + events("failed") + + showExceptions = true + exceptionFormat = FULL + showCauses = true + showStackTraces = true + + showStandardStreams = false + } +} + + tasks.dokkaHtml { dokkaSourceSets { configureEach { diff --git a/src/main/kotlin/kweb/ValueElement.kt b/src/main/kotlin/kweb/ValueElement.kt new file mode 100644 index 0000000000..9d05c6d75e --- /dev/null +++ b/src/main/kotlin/kweb/ValueElement.kt @@ -0,0 +1,169 @@ +package kweb + +import kotlinx.serialization.Serializable +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonNull +import kotlinx.serialization.json.JsonPrimitive +import kweb.ValueElement.LastModificationSource.Browser +import kweb.ValueElement.LastModificationSource.Server +import kweb.html.events.Event +import kweb.state.CloseReason +import kweb.state.KVal +import kweb.state.KVar +import kweb.state.ReversibleFunction +import kweb.util.json + +/** + * Abstract class for the various elements that have a `value` attribute and which support `change` and `input` events. + * + * @param kvarUpdateEvent The [value] of this element will update on this event, defaults to [input](https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/input_event) + */ +abstract class ValueElement( + open val element: Element, val kvarUpdateEvent: String = "input", + val initialValue: String? = null +) : Element(element) { + val valueJsExpression: String by lazy { "document.getElementById(\"$id\").value" } + + suspend fun getValue(): String { + return when (val result = + element.browser.callJsFunctionWithResult("return document.getElementById({}).value;", id.json)) { + is JsonPrimitive -> result.content + else -> error("Needs to be JsonPrimitive") + } + } + + //language=JavaScript + fun setValue(newValue: String) { + element.browser.callJsFunction( + """ + const element = document.getElementById({}); + element.value = {}; + delete element.dataset.previousInput; + """.trimIndent(), + element.id.json, newValue.json + ) + } + + fun setValue(newValue: KVal) { + val initialValue = newValue.value + setValue(initialValue) + val listenerHandle = newValue.addListener { _, new -> + setValue(new) + } + element.creator?.onCleanup(true) { + newValue.removeListener(listenerHandle) + } + } + + data class Value(val value: String, val lastModificationSource: LastModificationSource) + enum class LastModificationSource { + Server, Browser + } + + private var _valueKvar: KVar? = null + + private lateinit var _stringValueKvar: KVar + + + /** + * A KVar bidirectionally synchronized with the [value of a select element](https://www.w3schools.com/jsref/prop_select_value.asp). + * This [KVar] will update if the select element is changed (depending on [kvarUpdateEvent]), and will modify + * the element value if the KVar is changed. + * + * [value] can be set to a `KVar` to synchronize with an existing KVar, or it will create a new `KVar("")` + * if not set. + */ + var value: KVar + get() { + if (_valueKvar == null) { + synchronized(this) { + _valueKvar = KVar(Value(initialValue ?: "", Server)) + _stringValueKvar = + _valueKvar!!.map(object : ReversibleFunction("ValueElement.value") { + override fun invoke(from: Value): String = from.value + + override fun reverse(original: Value, change: String): Value = + Value(change, Server) + + }) + this.creator?.onCleanup(true) { + value.close(CloseReason("Parent element closed")) + } + attachListeners(_valueKvar!!) + updateKVar(_valueKvar!!, updateOn = kvarUpdateEvent) + } + } + return _stringValueKvar + } + set(v) { + if (_valueKvar != null) error("`value` may only be set once, and cannot be set after it has been retrieved") + synchronized(this) { + setValue(v.value) + _stringValueKvar = v + _valueKvar = _stringValueKvar.map(object : ReversibleFunction("ValueElement.value") { + override fun invoke(from: String): Value = Value(from, Server) + + override fun reverse(original: String, change: Value): String = change.value + + }) + attachListeners(_valueKvar!!) + updateKVar(_valueKvar!!, updateOn = kvarUpdateEvent) + } + } + + private fun attachListeners(kv: KVar) { + val handle = kv.addListener { _, value -> + // Only update the DOM element if the source of the change was the server + if (value.lastModificationSource == Server) { + setValue(value.value) + } + } + element.creator?.onCleanup(true) { + kv.removeListener(handle) + } + } + + /** + * Automatically update `toBind` with the value of this INPUT element when `updateOn` event occurs. + */ + + @Serializable + data class DiffData(val prefixEndIndex: Int, val postfixOffset: Int, val diffString: String) + + private fun applyDiff(oldString: String, diffData: DiffData): String { + + val newString = when { + diffData.postfixOffset == -1 -> {//these 2 edge cases prevent the prefix or the postfix from being + // repeated when you append text to the beginning of the text or the end of the text + oldString.substring(0, diffData.prefixEndIndex) + diffData.diffString + } + + diffData.prefixEndIndex == 0 -> { + diffData.diffString + oldString.substring(oldString.length - diffData.postfixOffset) + } + + else -> { + oldString.substring(0, diffData.prefixEndIndex) + diffData.diffString + + oldString.substring(oldString.length - diffData.postfixOffset) + } + } + return newString + } + + private fun updateKVar(toBind: KVar, updateOn: String = "input") { + on( + //language=JavaScript + retrieveJs = "get_diff_changes(document.getElementById(\"${element.id}\"))" + ) + .event(updateOn) { + //TODO, this check shouldn't be necessary. It should be impossible for get_diff_changes() to return a null, + //but we had a null check previously, so I went ahead and added it. + if (it.retrieved != JsonNull) { + val diffDataJson = it.retrieved + val diffData = Json.decodeFromJsonElement(DiffData.serializer(), diffDataJson) + toBind.value = Value(applyDiff(toBind.value.value, diffData), Browser) + } + } + } + +} \ No newline at end of file diff --git a/src/main/kotlin/kweb/prelude.kt b/src/main/kotlin/kweb/prelude.kt index 4e1a04ed28..80c0854f0c 100644 --- a/src/main/kotlin/kweb/prelude.kt +++ b/src/main/kotlin/kweb/prelude.kt @@ -2,11 +2,12 @@ package kweb import io.ktor.server.routing.* import io.mola.galimatias.URL -import kotlinx.serialization.Serializable -import kotlinx.serialization.json.* +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.boolean +import kotlinx.serialization.json.jsonPrimitive import kweb.html.HeadElement import kweb.html.TitleElement -import kweb.html.events.Event import kweb.html.fileUpload.FileFormInput import kweb.routing.PathTemplate import kweb.routing.RouteReceiver @@ -552,130 +553,6 @@ fun ElementCreator.label( } } -/** - * Abstract class for the various elements that have a `value` attribute and which support `change` and `input` events. - * - * @param kvarUpdateEvent The [value] of this element will update on this event, defaults to [input](https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/input_event) - */ -abstract class ValueElement( - open val element: Element, val kvarUpdateEvent: String = "input", - val initialValue: String? = null -) : Element(element) { - val valueJsExpression: String by lazy { "document.getElementById(\"$id\").value" } - - suspend fun getValue(): String { - return when (val result = - element.browser.callJsFunctionWithResult("return document.getElementById({}).value;", id.json)) { - is JsonPrimitive -> result.content - else -> error("Needs to be JsonPrimitive") - } - } - - //language=JavaScript - fun setValue(newValue: String) { - element.browser.callJsFunction( - "document.getElementById({}).value = {};", - element.id.json, newValue.json - ) - } - - fun setValue(newValue: KVal) { - val initialValue = newValue.value - setValue(initialValue) - val listenerHandle = newValue.addListener { _, new -> - setValue(new) - } - element.creator?.onCleanup(true) { - newValue.removeListener(listenerHandle) - } - } - - @Volatile - private var _valueKvar: KVar? = null - - /** - * A KVar bidirectionally synchronized with the [value of a select element](https://www.w3schools.com/jsref/prop_select_value.asp). - * This [KVar] will update if the select element is changed (depending on [kvarUpdateEvent]), and will modify - * the element value if the KVar is changed. - * - * [value] can be set to a `KVar` to synchronize with an existing KVar, or it will create a new `KVar("")` - * if not set. - */ - var value: KVar - get() { - synchronized(this) { - if (_valueKvar == null) { - value = KVar(initialValue ?: "") - this.creator?.onCleanup(true) { - value.close(CloseReason("Parent element closed")) - } - attachListeners(value) - } - } - return _valueKvar!! - } - set(v) { - if (_valueKvar != null) error("`value` may only be set once, and cannot be set after it has been retrieved") - updateKVar(v, updateOn = kvarUpdateEvent) - attachListeners(v) - setValue(v.value) - _valueKvar = v - } - - private fun attachListeners(kv : KVar) { - val handle = kv.addListener { _, newValue -> - setValue(newValue) - } - element.creator?.onCleanup(true) { - kv.removeListener(handle) - } - } - - /** - * Automatically update `toBind` with the value of this INPUT element when `updateOn` event occurs. - */ - - @Serializable - data class DiffData(val prefixEndIndex: Int, val postfixOffset: Int, val diffString: String) - - private fun applyDiff(oldString: String, diffData: DiffData): String { - - val newString = when { - diffData.postfixOffset == -1 -> {//these 2 edge cases prevent the prefix or the postfix from being - // repeated when you append text to the beginning of the text or the end of the text - oldString.substring(0, diffData.prefixEndIndex) + diffData.diffString - } - - diffData.prefixEndIndex == 0 -> { - diffData.diffString + oldString.substring(oldString.length - diffData.postfixOffset) - } - - else -> { - oldString.substring(0, diffData.prefixEndIndex) + diffData.diffString + - oldString.substring(oldString.length - diffData.postfixOffset) - } - } - return newString - } - - private fun updateKVar(toBind: KVar, updateOn: String = "input") { - on( - //language=JavaScript - retrieveJs = "get_diff_changes(document.getElementById(\"${element.id}\"))" - ) - .event(updateOn) { - //TODO, this check shouldn't be necessary. It should be impossible for get_diff_changes() to return a null, - //but we had a null check previously, so I went ahead and added it. - if (it.retrieved != JsonNull) { - val diffDataJson = it.retrieved - val diffData = Json.decodeFromJsonElement(DiffData.serializer(), diffDataJson) - toBind.value = applyDiff(toBind.value, diffData) - } - } - } - -} - /****************************** * Route extension ******************************/ diff --git a/src/main/resources/kweb/kweb_bootstrap.js b/src/main/resources/kweb/kweb_bootstrap.js index 5814ecb13f..549e5d361e 100644 --- a/src/main/resources/kweb/kweb_bootstrap.js +++ b/src/main/resources/kweb/kweb_bootstrap.js @@ -228,7 +228,7 @@ function get_diff_changes(htmlInputElement) { savePreviousInput(newString, htmlInputElement)//put the newString into the data attribute so it can be used as the oldString the next time this method is run - if (oldString == undefined) {//the first time this is run previous-input should be undefined so we just return the new string + if (oldString === undefined) {//the first time this is run previous-input should be undefined so we just return the new string return new DiffPatchData(0, 0, newString); } let commonPrefixEnd = 0; diff --git a/src/test/kotlin/kweb/SelectValueTest.kt b/src/test/kotlin/kweb/SelectValueTest.kt index 322ae29e2a..1dc890f58a 100644 --- a/src/test/kotlin/kweb/SelectValueTest.kt +++ b/src/test/kotlin/kweb/SelectValueTest.kt @@ -63,9 +63,6 @@ class SelectValueTestApp { option().set("value", "cat").text("Cat") } selectValue = select.value - selectValue.addListener { old, new -> - println("$old -> $new") - } } } } diff --git a/src/test/kotlin/kweb/StringDiffTest.kt b/src/test/kotlin/kweb/StringDiffTest.kt index 20df761d29..42ae84f09b 100644 --- a/src/test/kotlin/kweb/StringDiffTest.kt +++ b/src/test/kotlin/kweb/StringDiffTest.kt @@ -59,7 +59,9 @@ class StringDiffTest(@Arguments("--headless") unprotectedDriver: ChromeDriver) { driver.get("http://localhost:7660/") val inputField = driver.findElement(By.tagName("input")) inputField.sendKeys(" Jumped") - inputField.getAttribute("value") shouldBe "Lazy Brown Fox Jumped" + await().pollInSameThread().untilAsserted { + inputField.getAttribute("value") shouldBe "Lazy Brown Fox Jumped" + } } @Test