vert-x3 / vertx-lang-kotlin

Vert.x for Kotlin
Apache License 2.0
296 stars 68 forks source link

Add Coroutine Handlers to accept a suspend handlers #194

Closed hantsy closed 1 year ago

hantsy commented 3 years ago

Discuss in the group: https://groups.google.com/g/vertx/c/DsSPkvtLxos/m/YN_wXbrUAwAJ

I have created a simple vertex project to experience its Kotlin Coroutine feature, https://github.com/hantsy/vertx-sandbox/tree/master/kotlin-co

The Route handler method does not accept suspend handler.

I have to convert it myself.

router.get("/posts/:id")
    .produces("application/json")
    .handler {
        launch(it.vertx().dispatcher()) {
            handlers.getById(it)
        }
    }

Or use a private extension(it requires a CoroutineScopt to run) to make it work.

   private fun Route.coroutineHandler(fn: suspend (RoutingContext) -> Unit) {
        handler { ctx ->
            launch(ctx.vertx().dispatcher()) {
                try {
                    fn(ctx)
                } catch (e: Exception) {
                    ctx.fail(e)
                }
            }
        }
    }

// and use it like this.
  router.get("/posts/:id")
      .produces("application/json")
      .coroutineHandler {
          handlers.getById(it)
      }

Maybe adding a CoroutineRouter is better.(In Spring, for coroutine router definition DSL, Spring added a new coRouter {})

tsegismont commented 3 years ago

@vietj this looks like a good addition to me, wdyt?

@pmlopes on the user group you commented that perhaps this should be considered together with reworking the blockingHandler method from Vert.x Web. Can elaborate here? Thanks

pmlopes commented 3 years ago

I was just thinking, at a high level, lets consider that we introduce a factory in the router itself like:

// Router.java
<T> coFactory(Function<T, Handler<RoutingContext>> factory);

As the router creates all routes the factory reference is passed to the route, so we can have a method:

// Route.java
<T> coHandler(T coHandler);

This means that for example, we could use the same functionality to solve several use cases:

1.

// blocking
router.coFactory((Handler<RoutingContext> handler) -> ctx -> vertx.executeBlocking(handler));
// and then
router.route("/blocking/handler").coHandler(ctx -> {
  Thread.sleep(5);
  ctx.next()
});

2.

// Loom
router.coFactory((Handler<RoutingContext> handler) -> ctx -> Thread.startVirtualThread(() -> handler.handle(ctx));
// and then
router.route("/loom/handler").coHandler(ctx -> {
  Thread.sleep(5);
  ctx.next()
});

3.

Can we do the kotlin types to fit this knowing that vertx-web does not depend on kotlin?

tsegismont commented 3 years ago

@pmlopes I think we could make it work with a coroutine handler. But does this mean every handler created by this router instance would then becoming, for example in case 2, blocking? In this case, how do you mix different type of handlers on the same router?

pmlopes commented 3 years ago

In this example, the co routine handle is dependent on the router instance, you could mix by having several routers and use subRouter() to merge them.

Alternatively... a more verbose alternative but allowing all the mix and match:

router.route("/xyz").coHandler(KotlinFactory, { ctx ->
  ctx.next()
})

And we would need a @VertxGen factory interface that languages would implement. This approach would then allow on the same router to mix, kotlin coroutines, loom, rx, blocking, etc...

pmlopes commented 3 years ago

Or split into a chain:

router
  .route("/xyz")
  .withCoroutineFactory(Kotlin)
  .handler({ ctx ->
    ctx.next()
  })

In this case we can have again the handler like in the general API.

tsegismont commented 3 years ago

The chain idea sounds better to me: it's aligned with the Vert.x Web programming style.

I guess the withCoroutineFactory name is just for example's sake, right? In practice it would be something generic like handlerFactory ?

pmlopes commented 3 years ago

Yes, that's just me writting down in english what I expect the code will be doing :) handlerFactory is good and closer to our APIs.

This method would then wrap the current Route object and delegate to the source for all methods, except the handler, which would first pass through the factory and then delegate the result back to the source.

All the fluent methods would return the wrap instead of the original, this to allow multiple calls to handler, failureHandler and maybe respond without requiring to call the factory every single time.

vincentfree commented 3 years ago

Or you could introduce the factory in the initialization of the Router. At the moment we use Router.router(vertx) in the most default way to set it up. Her you can add a feature like Router.router(vertx, RouterConcurrencyHelper.CoroutineFactory()) and have the same for something like loom: Router.router(vertx, RouterConcurrencyHelper.LoomFactory()). Names are examples of course.

This would set up the router to use a different model as a default. The plain Router.router(vertx) can then still work as it is now with an option to change the default model. You might also want the withCoroutineFactory or withConcurrencyFactory or something like that to be able to change the model on the fly.

In my code with Kotlin I see that I mix between coroutines and the default Vertx promise/future setup so a withCoroutineFactory method might help when switching paradigm while configuring the router.

pmlopes commented 3 years ago

@vincentfree that was more or less the original concept I wrote, that the coroutine factory would be fixed per router. The different here is that you're saying that this factory would always be present, for example:

Router router = Router.router(vertx, Kotlin.factory())

// in this case the handler is already expected to be a `suspend fun (rc: RoutingContext) -> Unit`
router.handler({ ctx -> ... })

And in the current state

Router router = Router.router(vertx)

// This means the factory is in fact the Identity Function
router.handler({ ctx -> ... })

Is that it?

The question to check here is how can we make use of generics to ensure that handler() is type safe.

vincentfree commented 3 years ago

Your right @pmlopes, to me it can be useful to eliminate some boilerplate by passing the Identity Function in the Router directly. I don't know if this would make it fully typesafe, you'll store a function in the implementation of the router so that would be the default function for the handler. This would make it a bit hard to also do withCoroutineFactory although that might be on the Route it self right?

pmlopes commented 3 years ago

I'm looking at the Router and Route interfaces in order to have this at described using the Router.create(vertx, factory) it means that we will need to introduce generics on pretty much all methods, which can be a huge breaking change.

If we do it at the Route level with the handlerFactory then it seems that adding the method:

// Route.java
<T> CoRoute<T> handlerFactory(Function<T, Handler<RoutingContext>> factory);

And create the CoRoute as:

public interface CoRoute<T> extends Route {
...
  CoRoute<T> handler(T requestHandler);
...
}

Seems to still work without requiring any code change by the end user once the feature is complete. But I'd call @jponge @vietj @tsegismont for their knowledge on do we consider adding generics a breaking change or not, as most of the time, a recompile would work as an upgrade route.

vietj commented 3 years ago

I created this for a talk a time ago : https://github.com/vietj/kotlin-conf-inter-reactive/blob/master/src/main/kotlin/com/julienviet/utils/WebUtils.kt

and it looked simpler without the need to add specific type

pmlopes commented 3 years ago

@vietj the idea I was describing was to attempt to have a common way we could use not only with kotlin but also blocking, loom, executors, etc...

Yet I was trying to preserve the API as type safe as possible.

hantsy commented 3 years ago

@pmlopes Why not consider a new CoRouter, and make all methods to accept coroutine variants, like the Spring coRouter{} DSL desgin.

pmlopes commented 3 years ago

@hantsy that is totally possible, but it requires a bit more maintenance. Such router would need to be created on this project https://github.com/vert-x3/vertx-lang-kotlin but it isn't generated like the rest of the API from the source upstream modules (in this case vertx-web). This means that all changes performed upstream in terms of API changes need to be backported here manually.

My goal with the extra method handlerFactory() and CoRoute was to be able to auto generate this and keep it always up to date on each release. On the other hand it also addresses the case where users may want to use 2 different kinds of handlers.

The case for 2 different kinds is important, because it allows users to rely on the platform provided handlers, StaticHandler, OAuth2Handler, SessionHandler, etc... and also use the desired co routine style.

On top of that I was just trying to find a good abstraction so we could handle not just Kotlin but others like Loom which will probably became more popular in the near future.

gregopet commented 3 years ago

In case of a CoRouter, would it be possible to use it with Vertx libs that create their own routers? vertx-web-api-contract for example?

jponge commented 3 years ago

Perhaps the new router respond method could be used here?

ScottPierce commented 3 years ago

This is sorely needed. Currently using coroutines for requests is painful.

vietj commented 3 years ago

it would be good to make progress here :-)

rgmz commented 3 years ago

In this example, the co routine handle is dependent on the router instance, you could mix by having several routers and use subRouter() to merge them.

Alternatively... a more verbose alternative but allowing all the mix and match:

router.route("/xyz").coHandler(KotlinFactory, { ctx ->
  ctx.next()
})

And we would need a @VertxGen factory interface that languages would implement. This approach would then allow on the same router to mix, kotlin coroutines, loom, rx, blocking, etc...

Is there a compelling use-case for mixing handler types in a single router instance? I'd expect projects to just use one.


@pmlopes if I understand you correctly, the decision is between:

I personally prefer the latter method; defining the factory per-route seems precarious, unless something like below possible (although it may be functionally equivalent to the per-router implementation).

val router = Router.router(vertx)

// Define it once, at the top-level.
router.handlerFactory(Kotlin.factory())

// subsequent handlers are all `suspend fun (rc: RoutingContext) -> Unit`
router.route("/abc").handler { ctx -> ... }
router.route("/xyz").handler { ctx -> ... }
pmlopes commented 3 years ago

Lets focus on this for the next iteration now that we also have loom close

chengenzhao commented 2 years ago

We have similar problem here is it possible to allow other handler owner to have the same mechanism not just router?

e.g.

websocket.handlerFactory(Kotlin.factory())
socket.handlerFactory(Kotlin.factory())
RecordPaser.handlerFactory(Kotlin.factory)
//etc.

or

websocket.withCoroutineFactory(Kotlin).handler{...}
socket.withCoroutineFactory(Kotlin).handler{...}
RecordPaser.withCoroutineFactory(Kotlin).handler{...}

either way is OK for us

dodalovic commented 2 years ago

I created this for a talk a time ago : https://github.com/vietj/kotlin-conf-inter-reactive/blob/master/src/main/kotlin/com/julienviet/utils/WebUtils.kt

and it looked simpler without the need to add a specific type

@vietj This example is using GlobalScope to achieve its goal. Is this really the solution to the problem at hand?

hantsy commented 2 years ago

@rgmz The handler should accept a general handler and a suspend block at the same time. So I prefer to use a different named handler for it, such as coHandler method for the continuation handling in Kotlin coroutines.

sishbi commented 2 years ago

We created an extension function for Route to allow the coroutine context to be managed for the route. Now I would like to upgrade to Vertx 4.3.1 but I cannot do that as our custom handler methods break, they are marked as 'User' handlers even though some of them are 'Platform' or 'Security' handlers. The reason why they are marked as User handlers is that the generic Kotlin extension method does not implement one of the marker interfaces.

Feavy commented 1 year ago

It would be good to save the coroutineScope in context so it can be used in the handler.

fun Route.coHandler(fn: suspend (RoutingContext) -> Unit) {
    handler { ctx ->
        val context = Vertx.currentContext()
        launch(context.dispatcher()) {
            context.put("coroutineScope", this) // <--
            try {
                fn(ctx)
            } catch (e: Exception) {
                ctx.fail(e)
            }
        }
    }
}
tsegismont commented 1 year ago

I've created #253 for this and feedback is welcome

tsegismont commented 1 year ago

Closed by 7fb969c