kwebio / kweb-core

A Kotlin web framework
https://docs.kweb.io/book
GNU Lesser General Public License v3.0
969 stars 57 forks source link

url(simpleUrlParser) is broken #122

Closed alkoclick closed 4 years ago

alkoclick commented 4 years ago

Again, I may be misdirected here somehow, but seeing the new WebBrowser.url I wanted to try it out in the following way (which used to work)

url(simpleUrlParser)

The way I understand this, this should also allow me to access the the extensions such as query, path, etc. However, this throws immediately as the scheme is missing

Adding the above line to the FeatureApp, produces the following:

io.mola.galimatias.GalimatiasParseException: Missing scheme
    at io.mola.galimatias.GalimatiasParseException$Builder.build(GalimatiasParseException.java:103)
    at io.mola.galimatias.URLParser.handleFatalMissingSchemeError(URLParser.java:173)
    at io.mola.galimatias.URLParser.parse(URLParser.java:387)
    at io.mola.galimatias.URL.parse(URL.java:307)
    at kweb.PreludeKt$simpleUrlParser$1.invoke(prelude.kt:420)
    at kweb.PreludeKt$simpleUrlParser$1.invoke(prelude.kt:419)
    at kweb.state.KVar.map(KVar.kt:30)
    at kweb.WebBrowser.url(WebBrowser.kt:123)
    at kweb.demos.feature.FeatureAppKt$kwebFeature$2$1$1.invoke(FeatureApp.kt:31)
    at kweb.demos.feature.FeatureAppKt$kwebFeature$2$1$1.invoke(FeatureApp.kt)
    at kweb.ElementKt.new(Element.kt:386)
    at kweb.ElementKt.new$default(Element.kt:383)
    at kweb.demos.feature.FeatureAppKt$kwebFeature$2$1.invoke(FeatureApp.kt:30)
    at kweb.demos.feature.FeatureAppKt$kwebFeature$2$1.invoke(FeatureApp.kt)
    at kweb.Kweb$listenForHTTPConnection$1$2.invoke(Kweb.kt:387)
    at kweb.Kweb$listenForHTTPConnection$1$2.invoke(Kweb.kt:54)

In WebBrowser.url we are in fact dropping the scheme:

    val url: KVar<String>
            by lazy {
                val originRelativeURL = URL.parse(httpRequestInfo.requestedUrl).pathQueryFragment
                val url = KVar(originRelativeURL)
                ...
                url
            }

The options I see are:

sanity commented 4 years ago

Hmm, definitely looks like a problem - could you clarify how you're trying to use it? Is it possible to use val url directly without using the Galimatias parser? If I recall correctly, your issue before was that it was stripping the query and fragment from the path, but you can now do something like:

url.value = "/path?query#fragment"
alkoclick commented 4 years ago
fun WebBrowser.getQueryParams(): Parameters =
    parseQueryString(
        URL.parse(httpRequestInfo.requestedUrl).query()?.replace("+", " ") 
        ?: ""
     )

(parseQueryString is from ktor)

Instead of that, I would love to instead do:

fun WebBrowser.getQueryParams(): Parameters =
    parseQueryString(
        url(simpleUrlParser).query.value 
        ?: ""
     )

Generally there is a bunch of extension methods such as val KVar<URL>.query that I would like to use, but they are currently unusable :/

sanity commented 4 years ago

This should be an easy fix, just trying to think through the ergonomics. How about something like this:

class WebBrowser {
   ...
    val gurl : KVar<URL> = url.map(object : ReversibleFunction<String, URL>(label = "gurl") {
        override fun invoke(from: String): URL {
            return URL.parse(this@WebBrowser.httpRequestInfo.requestedUrl).resolve(from)
        }

        override fun reverse(original: String, change: URL): String {
            return change.pathQueryFragment
        }
    } )
   ...
}

This would allow you to use the KVar<URL> extension methods on gurl:

gurl.query.value = "query"
alkoclick commented 4 years ago

Yeah, but I was trying to have the functionality of WebBrowser.url as well. This is currently pushState and I dunno if you're doing anything else with it? So this is about operating on WebBrowser.url using the methods created for Kvar (query, path, fragment, etc)

sanity commented 4 years ago

I'm afraid I'm not sure what you mean, could you give an example of how it should work?

alkoclick commented 4 years ago

No, sorry, my bad. I had missed that in your example, gurl is mapped on url (url.map). That is an easy fix indeed and has the functionality I expected, though I would also add some short documentation about this (previously some examples did url(simpleUrlParser) so maybe others like me are using this, and it's currently broken OOTB)

sanity commented 4 years ago

Gotcha, I'll search the documentation for references to simpleUrlParser and update them.