Merge pull request #212 from Derek52/Move-outboundMessageCatcher-to-W…

…ebBrowser

Move outbound message catcher to web browser

src/main/kotlin/kweb/Element.kt

@@ -90,7 +90,7 @@ open class Element(
     fun setAttribute(name: String, value: JsonPrimitive): Element {
         val htmlDoc = browser.htmlDocument.get()
         when {
-            browser.kweb.isCatchingOutbound() != null -> {
+            browser.isCatchingOutbound() != null -> {
                 callJsFunction("document.getElementById({}).setAttribute({}, {})",
                         JsonPrimitive(id), JsonPrimitive(name), value)
             }
@@ -146,7 +146,7 @@ open class Element(
 
     fun removeAttribute(name: String): Element {
         when {
-            browser.kweb.isCatchingOutbound() != null -> {
+            browser.isCatchingOutbound() != null -> {
                 callJsFunction("document.getElementById({}).removeAttribute", JsonPrimitive(id), JsonPrimitive(name))
             }
             else -> {
@@ -309,7 +309,7 @@ open class Element(
         val jsoupDoc = browser.htmlDocument.get()
         val setTextJS = """document.getElementById({}).textContent = {};""".trimIndent()
         when {
-            browser.kweb.isCatchingOutbound() != null -> {
+            browser.isCatchingOutbound() != null -> {
                 callJsFunction(setTextJS, JsonPrimitive(id), JsonPrimitive(value))
             }
             jsoupDoc != null -> {
@@ -355,7 +355,7 @@ open class Element(
             document.getElementById({}).appendChild(ntn);
         """.trimIndent()
         when {
-            browser.kweb.isCatchingOutbound() != null -> {
+            browser.isCatchingOutbound() != null -> {
                 callJsFunction(createTextNodeJs, JsonPrimitive(value), JsonPrimitive(id))
             }
             jsoupDoc != null -> {

src/main/kotlin/kweb/ElementCreator.kt

@@ -58,7 +58,7 @@ open class ElementCreator<out PARENT_TYPE : Element>(
         val id: String = mutAttributes.computeIfAbsent("id") { JsonPrimitive("K" + browser.generateId()) }.content
         val htmlDoc = browser.htmlDocument.get()
         when {
-            parent.browser.kweb.isCatchingOutbound() != null -> {
+            parent.browser.isCatchingOutbound() != null -> {
                 val createElementJs = """
                     let tag = {};
                     let attributes = {};

src/main/kotlin/kweb/Kweb.kt

@@ -142,76 +142,39 @@ class Kweb private constructor(
 
     private var server: JettyApplicationEngine? = null
 
-    /**
-     * Are outbound messages being cached for this thread (for example, because we're inside an immediateEvent callback block)?
-     */
-    fun isCatchingOutbound() = outboundMessageCatcher.get()?.catcherType
-
-    /**
-     * Execute a block of code in which any JavaScript sent to the browser during the execution of the block will be stored
-     * and returned by this function.
-     *
-     * The main use-case is recording changes made to the DOM within an onImmediate event callback so that these can be
-     * replayed in the browser when an event is triggered without a server round-trip.
-     */
-    fun catchOutbound(catchingType: CatcherType, f: () -> Unit): List<FunctionCall> {
-        require(outboundMessageCatcher.get() == null) { "Can't nest withThreadLocalOutboundMessageCatcher()" }
-
-        val jsList = ArrayList<FunctionCall>()
-        outboundMessageCatcher.set(OutboundMessageCatcher(catchingType, jsList))
-        f()
-        outboundMessageCatcher.set(null)
-        return jsList
-    }
-
-    fun batch(sessionId: String, catchingType: CatcherType, f: () -> Unit) {
-        val caughtMessages = catchOutbound(catchingType, f)
+    fun sendMessage(sessionId: String, server2ClientMessage: Server2ClientMessage) {
         val wsClientData = clientState.getIfPresent(sessionId) ?: error("Client id $sessionId not found")
         //TODO, do we need to change lastModified here? callJs will set it when the functionCall is originally created.
         wsClientData.lastModified = Instant.now()
-        val server2ClientMessage = Server2ClientMessage(sessionId, caughtMessages)
         wsClientData.send(server2ClientMessage)
     }
 
     /*
     FunctionCalls that point to functions already in the cache will not contain the jsCode needed to create the debugToken
     So we have to pass it separately here.
      */
-    fun callJs(sessionId: String, funcCall: FunctionCall, javascript: String) {
+    fun callJs(sessionId: String, funcCall: FunctionCall, debugInfo: DebugInfo? = null) {
         val wsClientData = clientState.getIfPresent(sessionId)
                 ?: error("Client id $sessionId not found")
         wsClientData.lastModified = Instant.now()
-        val debugToken: String? = if(!debug) null else {
+        if(debug) {
             val dt = abs(random.nextLong()).toString(16)
-            wsClientData.debugTokens[dt] = DebugInfo(javascript, "executing", Throwable())
-            dt
-        }
-        val outboundMessageCatcher = outboundMessageCatcher.get()
-        //TODO to make debugToken and shouldExecute in Server2ClientMessage vals instead of vars I decided to
-        //make new functionCall objects. I don't know if changing these vals to vars is worth the overhead of creating
-        //a new object though. So we may want to revert this change.
-        if (outboundMessageCatcher == null) {
-            val jsFuncCall = FunctionCall(debugToken, funcCall)
-            wsClientData.send(Server2ClientMessage(sessionId, jsFuncCall))
-        } else {
-            logger.debug("Temporarily storing message for $sessionId in threadlocal outboundMessageCatcher")
-            outboundMessageCatcher.functionList.add(funcCall)
-            //Setting `shouldExecute` to false tells the server not to add this jsFunction to the client's cache,
-            //but to not actually run the code. This is used to pre-cache functions on initial page render.
-            val jsFuncCall = FunctionCall(debugToken, shouldExecute = false, funcCall)
-            wsClientData.send(Server2ClientMessage(sessionId, jsFuncCall))
+            debugInfo?.let {
+                wsClientData.debugTokens[dt] = it
+            }
         }
+        wsClientData.send(Server2ClientMessage(sessionId, funcCall))
     }
 
     fun callJsWithCallback(sessionId: String, funcCall: FunctionCall,
-                           javascript: String, callback: (JsonElement) -> Unit) {
+                           debugInfo: DebugInfo? = null, callback: (JsonElement) -> Unit) {
         val wsClientData = clientState.getIfPresent(sessionId)
                 ?: error("Client id $sessionId not found")
         wsClientData.lastModified = Instant.now()
         funcCall.callbackId?.let {
             wsClientData.handlers[it] = callback
         } ?: error("Javascript function callback wasn't given a callbackId")
-        callJs(sessionId, funcCall, javascript)
+        callJs(sessionId, funcCall, debugInfo)
     }
 
     fun removeCallback(clientId: String, callbackId: Int) {
@@ -391,7 +354,7 @@ class Kweb private constructor(
                 //A plugin with an empty js string was breaking functionality.
                 if (js != "") {
                     val pluginFunction = FunctionCall(js = js)
-                    callJs(kwebSessionId, pluginFunction, js)
+                    callJs(kwebSessionId, pluginFunction)
                 }
             }
 
@@ -480,17 +443,7 @@ class Kweb private constructor(
         }
     }
 
-    //TODO I think some of these things could be renamed for clarity. I think it is understandable as is, but there is room for improvement
-    enum class CatcherType {
-        EVENT, IMMEDIATE_EVENT, RENDER
-    }
-    data class OutboundMessageCatcher(var catcherType: CatcherType, val functionList: MutableList<FunctionCall>)
 
-    /**
-     * Allow us to catch outbound messages temporarily and only for this thread.  This is used for immediate
-     * execution of event handlers, see `Element.onImmediate`
-     */
-    val outboundMessageCatcher: ThreadLocal<OutboundMessageCatcher?> = ThreadLocal.withInitial { null }
 
 }
 

src/main/kotlin/kweb/WebBrowser.kt

@@ -6,6 +6,7 @@ import kotlinx.serialization.json.JsonElement
 import kotlinx.serialization.json.JsonPrimitive
 import kweb.client.FunctionCall
 import kweb.client.HttpRequestInfo
+import kweb.client.Server2ClientMessage
 import kweb.html.Document
 import kweb.html.HtmlDocumentSupplier
 import kweb.plugins.KwebPlugin
@@ -29,7 +30,7 @@ import kotlin.reflect.jvm.jvmName
 
 private val logger = KotlinLogging.logger {}
 
-class WebBrowser(val sessionId: String, val httpRequestInfo: HttpRequestInfo, val kweb: Kweb) {
+class WebBrowser(private val sessionId: String, val httpRequestInfo: HttpRequestInfo, val kweb: Kweb) {
 
     private val idCounter = AtomicInteger(0)
 
@@ -52,6 +53,46 @@ class WebBrowser(val sessionId: String, val httpRequestInfo: HttpRequestInfo, va
         HtmlDocumentSupplier.appliedPlugins.map { it::class to it }.toMap()
     }
 
+    //TODO I think some of these things could be renamed for clarity. I think it is understandable as is, but there is room for improvement
+    enum class CatcherType {
+        EVENT, IMMEDIATE_EVENT, RENDER
+    }
+    data class OutboundMessageCatcher(var catcherType: CatcherType, val functionList: MutableList<FunctionCall>)
+
+    /**
+     * Allow us to catch outbound messages temporarily and only for this thread.  This is used for immediate
+     * execution of event handlers, see `Element.onImmediate`
+     */
+    val outboundMessageCatcher: ThreadLocal<OutboundMessageCatcher?> = ThreadLocal.withInitial { null }
+
+    /**
+     * Are outbound messages being cached for this thread (for example, because we're inside an immediateEvent callback block)?
+     */
+    fun isCatchingOutbound() = outboundMessageCatcher.get()?.catcherType
+
+    /**
+     * Execute a block of code in which any JavaScript sent to the browser during the execution of the block will be stored
+     * and returned by this function.
+     *
+     * The main use-case is recording changes made to the DOM within an onImmediate event callback so that these can be
+     * replayed in the browser when an event is triggered without a server round-trip.
+     */
+    fun catchOutbound(catchingType: CatcherType, f: () -> Unit): List<FunctionCall> {
+        require(outboundMessageCatcher.get() == null) { "Can't nest withThreadLocalOutboundMessageCatcher()" }
+
+        val jsList = ArrayList<FunctionCall>()
+        outboundMessageCatcher.set(OutboundMessageCatcher(catchingType, jsList))
+        f()
+        outboundMessageCatcher.set(null)
+        return jsList
+    }
+
+    fun batch(catchingType: CatcherType, f: () -> Unit) {
+        val caughtMessages = catchOutbound(catchingType, f)
+        val server2ClientMessage = Server2ClientMessage(sessionId, caughtMessages)
+        kweb.sendMessage(sessionId, server2ClientMessage)
+    }
+
     @Suppress("UNCHECKED_CAST")
     internal fun <P : KwebPlugin> plugin(plugin: KClass<out P>): P {
         return (plugins[plugin] ?: error("Plugin $plugin is missing")) as P
@@ -97,36 +138,62 @@ class WebBrowser(val sessionId: String, val httpRequestInfo: HttpRequestInfo, va
     }
 
     fun callJsFunction(jsBody: String, vararg args: JsonElement) {
-        cachedFunctions[jsBody]?.let {
-            val cachedFunctionCall = FunctionCall(jsId = it, arguments = listOf(*args))
-            kweb.callJs(sessionId, cachedFunctionCall, jsBody)
-        } ?: run {
+        val functionCall  = if (cachedFunctions[jsBody] != null) {
+            FunctionCall(jsId = cachedFunctions[jsBody], arguments = listOf(*args))
+        } else {
             val cacheId = generateCacheId()
             val func = makeJsFunction(jsBody)
             //we add the user's unmodified js as a key and the cacheId as it's value in the hashmap
             cachedFunctions[jsBody] = cacheId
             //we send the modified js to the client to be cached there.
             //we don't cache the modified js on the server, because then we'd have to modify JS on the server, everytime we want to check the server's cache
-            val cacheAndCallFunction = FunctionCall(jsId = cacheId, js = func.js, parameters = func.params,
+            FunctionCall(jsId = cacheId, js = func.js, parameters = func.params,
                     arguments = listOf(*args))
-            kweb.callJs(sessionId, cacheAndCallFunction, jsBody)
+        }
+        val debugInfo: DebugInfo? = if(!kweb.debug) null else {
+            DebugInfo(jsBody, "executing", Throwable())
+        }
+        val outboundMessageCatcher = outboundMessageCatcher.get()
+        if (outboundMessageCatcher == null) {
+            kweb.callJs(sessionId, functionCall, debugInfo)
+        } else {
+            logger.debug("Temporarily storing message for $sessionId in threadlocal outboundMessageCatcher")
+            outboundMessageCatcher.functionList.add(functionCall)
+            //funcToCache is a functionCall object with the shouldExecute parameter set to false
+            //This tells the client to cache the function call, but to not run the code.
+            //I believe this is used to cache js code that is called in events.
+            val funcToCache = FunctionCall(debugToken = functionCall.debugToken, shouldExecute = false, functionCall)
+            kweb.callJs(sessionId, funcToCache, debugInfo)
         }
     }
 
     fun callJsFunctionWithCallback(jsBody: String, callbackId: Int, callback: (JsonElement) -> Unit, vararg args: JsonElement) {
-        cachedFunctions[jsBody]?.let {
-            val cachedFunctionCall = FunctionCall(jsId = it, arguments = listOf(*args), callbackId = callbackId)
-            kweb.callJsWithCallback(sessionId, cachedFunctionCall, jsBody, callback)
-        } ?: run {
+        val functionCall = if (cachedFunctions[jsBody] != null) {
+            FunctionCall(jsId = cachedFunctions[jsBody], arguments = listOf(*args), callbackId = callbackId)
+        } else {
             val cacheId = generateCacheId()
             val func = makeJsFunction(jsBody)
             //we add the user's unmodified js as a key and the cacheId as it's value in the hashmap
             cachedFunctions[jsBody] = cacheId
             //we send the modified js to the client to be cached there.
             //we don't cache the modified js on the server, because then we'd have to modify JS on the server, everytime we want to check the server's cache
-            val cacheAndCallFunction = FunctionCall(jsId = cacheId, js = func.js, parameters = func.params,
-            arguments = listOf(*args), callbackId = callbackId)
-            kweb.callJsWithCallback(sessionId, cacheAndCallFunction, jsBody, callback)
+            FunctionCall(jsId = cacheId, js = func.js, parameters = func.params,
+                    arguments = listOf(*args), callbackId = callbackId)
+        }
+        val debugInfo: DebugInfo? = if(!kweb.debug) null else {
+            DebugInfo(jsBody, "executing", Throwable())
+        }
+        val outboundMessageCatcher = outboundMessageCatcher.get()
+        if (outboundMessageCatcher == null) {
+            kweb.callJsWithCallback(sessionId, functionCall, debugInfo, callback)
+        } else {
+            logger.debug("Temporarily storing message for $sessionId in threadlocal outboundMessageCatcher")
+            outboundMessageCatcher.functionList.add(functionCall)
+            //funcToCache is a functionCall object with the shouldExecute parameter set to false
+            //This tells the client to cache the function call, but to not run the code.
+            //I believe this is used to cache js code that is called in events.
+            val funcToCache = FunctionCall(debugToken = functionCall.debugToken, shouldExecute = false, functionCall)
+            kweb.callJsWithCallback(sessionId, funcToCache, debugInfo, callback)
         }
     }
 
@@ -135,7 +202,7 @@ class WebBrowser(val sessionId: String, val httpRequestInfo: HttpRequestInfo, va
     }
 
     suspend fun callJsFunctionWithResult(jsBody: String, vararg args: JsonElement): JsonElement {
-        require(kweb.isCatchingOutbound() == null) {
+        require(isCatchingOutbound() == null) {
             "You can not read the DOM inside a batched code block"
         }
         val callbackId = abs(random.nextInt())
@@ -190,6 +257,5 @@ class WebBrowser(val sessionId: String, val httpRequestInfo: HttpRequestInfo, va
             return change.pathQueryFragment
         }
     } )
-
 }
 

src/main/kotlin/kweb/html/ElementReader.kt

@@ -15,7 +15,7 @@ open class ElementReader(protected val receiver: WebBrowser, internal val elemen
     init {
         //TODO I'm not sure if we want to allow reading the DOM during a render or non immediate event
         //require(receiver.kweb.isCatchingOutbound() != Kweb.CatcherType.IMMEDIATE_EVENT)
-        require(receiver.kweb.isCatchingOutbound() == null) {
+        require(receiver.isCatchingOutbound() == null) {
             """
             Reading the DOM when an outboundMessageCatcher is set is likely to have unintended consequences.
             Most likely you are trying to read the DOM within an `onImmediate {...}` block.

src/main/kotlin/kweb/html/events/OnImmediateReceiver.kt

@@ -12,7 +12,7 @@ import kweb.util.KWebDSL
 @KWebDSL
 class OnImmediateReceiver<T: EventGenerator<T>>(internal val source: T) {
     fun event(eventName: String, callback: () -> Unit): T {
-        val caughtJsFunctions = source.browser.kweb.catchOutbound(Kweb.CatcherType.IMMEDIATE_EVENT) {
+        val caughtJsFunctions = source.browser.catchOutbound(WebBrowser.CatcherType.IMMEDIATE_EVENT) {
             callback()
         }
         val immediateJs = mutableListOf<String>()

src/main/kotlin/kweb/html/events/OnReceiver.kt

@@ -4,6 +4,7 @@ import kotlinx.serialization.json.Json
 import kotlinx.serialization.json.JsonElement
 import kotlinx.serialization.serializer
 import kweb.Kweb
+import kweb.WebBrowser
 import kweb.util.KWebDSL
 import mu.KotlinLogging
 import java.util.concurrent.ConcurrentHashMap
@@ -29,8 +30,8 @@ class OnReceiver<T : EventGenerator<T>>(val source: T, private val retrieveJs: S
         return event(eventName, eventPropertyNames) { propertiesAsElement ->
             val props = Json.decodeFromJsonElement(serializer, propertiesAsElement)
             try {
-                if (source.browser.kweb.isCatchingOutbound() == null) {
-                    source.browser.kweb.batch(source.browser.sessionId, Kweb.CatcherType.EVENT) {
+                if (source.browser.isCatchingOutbound() == null) {
+                    source.browser.batch(WebBrowser.CatcherType.EVENT) {
                         callback(props)
                     }
                 }

src/main/kotlin/kweb/state/render.kt

@@ -35,8 +35,8 @@ fun <T : Any?> ElementCreator<*>.render(
 
     fun eraseAndRender() {
         do {
-            if (parent.browser.kweb.isCatchingOutbound() == null) {
-                parent.browser.kweb.batch(parent.browser.sessionId, Kweb.CatcherType.RENDER) {
+            if (parent.browser.isCatchingOutbound() == null) {
+                parent.browser.batch(WebBrowser.CatcherType.RENDER) {
                     containerElement.removeChildren() // REMOVE ALL ELEMENTS BETWEEN startSpan and endSpan
                     containerElement.new {
                         previousElementCreator.getAndSet(this)?.cleanup()
@@ -107,7 +107,7 @@ fun <T : Any> ElementCreator<*>.toVar(shoebox: Shoebox<T>, key: String): KVar<T>
     val value = shoebox[key] ?: throw NoSuchElementException("Key $key not found")
     val w = KVar(value)
     w.addListener { _, n ->
-        require(this.browser.kweb.isCatchingOutbound() != Kweb.CatcherType.IMMEDIATE_EVENT) {
+        require(this.browser.isCatchingOutbound() != WebBrowser.CatcherType.IMMEDIATE_EVENT) {
             """You appear to be modifying Shoebox state from within an onImmediate callback, which
                 |should only make simple modifications to the DOM.""".trimMargin()
         }