Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a Kotlin DSL for the functional Web API [SPR-15065] #19631

Closed
spring-issuemaster opened this issue Dec 28, 2016 · 14 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Dec 28, 2016

Sébastien Deleuze opened SPR-15065 and commented

It would be nice to find a way to avoid specifying explicitly HandlerFunction lambda type for Kotlin user since they won't be able to fix this for Kotlin 1.1 (see this explanation on KT-14923).

Possible solutions are :

  • Provide Kotlin specific API for the router
  • Provide Kotlin extensions for RouterFunction and RouterFunctions
  • Modify Java API signature to avoid this issue (unlikely but this is a possible solution)

Issue Links:

  • #19521 Improve nested routes API
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 8, 2017

Yevhenii Melnyk commented

According to this stackoverflow post it is impossible now to create static extension methods. You can only create extensions to compation objects that already exist and java classes such as RouterFunctions does not have any.

Have you considered creating routing dsl using Kotlin?
Something like the following:

"/user" {
    get { findAll() }
    post { create() }
    "/{login}"{
        handleGet { findOne(it) }
        post { createOne() }
    }
    "/info/{login}"{
        handleGet { req ->
            ServerResponse.ok().body(fromPublisher(Mono.just("Login: ${req.pathVariable("login")}")))
        }
        post { createOne() }
    }
}

I managed to create a simple example that can be found here. We can create router functions by passing HandlerFunction or creating lambda.
I think it is also possible to pass method references.
I took inspiration for this tree-like dsl in akka-http framework:
http://doc.akka.io/docs/akka/2.4.7/scala/http/routing-dsl/overview.html
http://doc.akka.io/docs/akka/2.4.9/scala/http/routing-dsl/routes.html

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 9, 2017

Sébastien Deleuze commented

Yevhenii Melnyk Thanks for this proposal, indeed providing such Kotlin DSL for routing could make sense as part of our Kotlin support. I will have a deeper look and give you my feedback.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 10, 2017

Sébastien Deleuze commented

Yevhenii Melnyk I have integrated an updated version of your router DSL proposal in Mixit sample application for now, in order to allow us to iterate before integrating this in Spring Framework master. See this commit for more details.

I think I like the principle of such DSL, that seems more idiomatic Kotlin code, avoid the generic type casting issue we had previously, and is more readable. The changes I made are:

  • I am using GET instead of get is order to be consistent with the Java API (we did that choice for various reasons, one is to make it more clear it is about GET HTTP method and not related to some kind of getter)
  • I introduced a fun route(predicate: RequestPredicate, f: () -> HandlerFunction<ServerResponse>) function to be able to add easily other RequestPredicate since composability of our routing API is a key feature
  • I also added support for resources

So the API look like:

"/" {
    "**" {                  resources(ClassPathResource("static/")) }
    "custom" {              route(contentType(APPLICATION_JSON)) { foo()} }
    "user" {
        "/" {                GET { findAllView() } }
        "/{login}" {         GET { findViewById() } }
    }
    "api/user" {
        "/" {
                             POST { create() }
                             GET  { findAll() }
        }
        "/{login}" {         GET { findOne() } }
    }
}

But I would like also to see with you if an API that would rely on RequestPredicate more than on String is possible since this is a key building block in our routing API and would allow more flexible and powerful routing logic. Something like:

path("/") {
    path("**") { resources(ClassPathResource("static/")) }
    GET("custom").and(contentType(APPLICATION_JSON)) { foo() } 
    path("user") {
        GET("/") { findAllView() }
        GET("/{login}") { findViewById() }
    }
    path("api/user") {
        POST("/") { create() }
        GET("/") { findAll() }
        GET("/{login}") { findOne() }
    }
}

I have not tried yet to see if this possible, but I think (or at least hope) it is. Any thoughts about that before I go further?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 11, 2017

Yevhenii Melnyk commented

Sébastien, the idea of creating custom Path class based on String was to provide hierarchical structure for building urls. I was not able to achieve this using RequestPredicate. Maybe, there is a way, but I haven't found it.

There are some features that I consider to be valuable for the DSL:

  • Method reference for both Kotlin and Java methods. See usage in web-function-sample.
    It seems to that I managed to implement this approach. The sample can be found here. You can pass handlers as function reference, HandlerFunction or inline handlers.
"/user" {
    GET(findAll())
"/empty" {
    GET { req ->
        ServerResponse.ok().body(fromPublisher(Flux.just(User("empty"))))
    }
}
"/login/{login}"{
    GET(::findOther)
}
"/controller"{
    GET(this@UserController::findOne)
}
"/comp"{
    GET(Companion::findOtherOne)
}
"/java"{
    GET(personHandler::getPerson)
}
"/javaStatic"{
    GET(PersonHandler::getOtherPerson)
}
  • Usage of RequestPredicate. For now I managed to use them as the leaves of route tree like this
(GET("/include") and contentType(APPLICATION_JSON)) {
    include()
}

There is also a point to be considered in case of using tree-like routing: how to handle exclusions . To my mind the path exclusion should be done according to the base url. So for the following example all urls except "/example/exclude" should handle request.

For example:

"/example"{
    "/**"{
        (!GET("/exclude")) {
            findAll()
        }
    }
}
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 11, 2017

Sébastien Deleuze commented

Yevhenii Melnyk Interesting, I am going to have a look thanks. For nested route support, my plan is to wait #19521 to be complete and then leverage the new capabilities from the Java API to add such support in the Kotlin one.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 11, 2017

Sébastien Deleuze commented

Yevhenii Melnyk I think we are approaching from something worth to merge. I have created a RouterFunctionDsl that can be used as following:

override fun route(request: ServerRequest) = RouterFunctionDsl {
    accept(TEXT_HTML).apply {
        (GET("/user/") or GET("/users/")) { findAllView() }
        GET("/user/{login}") { findViewById() }
    }
    accept(APPLICATION_JSON).apply {
        (GET("/api/user/") or GET("/api/users/")) { findAll() }
        POST("/api/user/") { create() }
        POST("/api/user/{login}") { findOne() }
    }
} (request)

Pretty nice, isn't it?

The missing feature is providing a way to use method references, I tried to introduce it but I then get a conflict between the 2 versions of the method. I guess we can find a way to fix that. Feel free to fork https://github.com/mix-it/mixit/ and send a PR if you have an idea.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 11, 2017

Sébastien Deleuze commented

I have merged this first version in order to make it possible to have feedbacks from early adopters using snapshots, I keep this issue open in order to polish and improve it based on feebacks. Nested route support will be added later as part of #19521.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 13, 2017

Sébastien Deleuze commented

Function references should be possible with current implementation when KT-15667 will be fixed so I plan no specific workaround for now on Spring side.

Ilya Ryzhenkov from Kotlin team made a few proposal to polish the DSL:

  • Add varargs for multiple path matching: I think I prefer only using or at least for the initial version
  • Remove the need for apply on RequestPredicate like accept() or GET() by adding dedicated extension: we need apply to differentiate from handler invocation
  • Replace RouterFunctionDsl &#123;...&#125; (request) by route(request) &#123;...&#125; this one is also probably a good idea, the current construct seems a bit weird.
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 13, 2017

Sébastien Deleuze commented

With this additional commit I think this issue can now be marked as resolved.

The final DSL is:

fun route(request: ServerRequest) = route(request) {
    accept(TEXT_HTML).apply {
            (GET("/user/") or GET("/users/")) { findAllView() }
            GET("/user/{login}") { findViewById() }
    }
    accept(APPLICATION_JSON).apply {
            (GET("/api/user/") or GET("/api/users/")) { findAll() }
            POST("/api/user/") { create() }
            POST("/api/user/{login}") { findOne() }
    }
 }
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 12, 2017

Yevhenii Melnyk commented

Sébastien Deleuze, there are some deficiencies in docs for this feature:

  • Nested routes in accept method or chaining two GET's together won't work without explicit import.
    import org.springframework.web.reactive.function.server.RequestPredicates.*. IDE can hardly find solution so from my point of view this should be mentioned somehow.
  • The method reference noted in javadoc shows only reference to the method outside any class. Most likely the method is located inside class to use injected properties so the reference will look like this this@SampleController::someMethod
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 13, 2017

Sébastien Deleuze commented

Yevhenii Melnyk Fixed via this commit + I have integrated the new json(), html() and xml() shortcuts.

Do you see a way to be able to write:

json {
    (GET("/api/user/") or GET("/api/users/")) { findAll() }
    POST("/api/user/", this@FooController::create)
}

Instead of what we currently support with this new commit:

json().apply {
    (GET("/api/user/") or GET("/api/users/")) { findAll() }
    POST("/api/user/", this@FooController::create)
}
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 13, 2017

Sébastien Deleuze commented

Also keep in mind that the nested routing support we are about to start working on with Arjen Poutsma may help to support that more easily, so that's maybe better to wait #19521 to be fixed before digging into that.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 14, 2017

Yevhenii Melnyk commented

I've investigated the issue with json().apply and it seems the only way to do this curretly is like this:

fun json(f: RequestPredicate.() -> Unit) {
	RequestPredicates.json().apply(f)
}

The most right way to build dsl seems to use invoke on RouterFunction but it seems I've stuck with KT-14923.
The following code doesn't work(ambiguous call) but it would be great if it works some day:

class RouterDsl {
...
val html: RequestPredicate
	get() = RequestPredicates.html()

operator fun RequestPredicate.invoke(f: RequestPredicate.() -> Unit): RequestPredicate = this.apply(f)
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 22, 2017

Sébastien Deleuze commented

Nested routing now works including with pathPrefix() predicate and with any level of nested routes, see this example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.