Skip to content

Commit 92ee4e7

Browse files
author
Edgar Espina
committed
kotlin: mount of coroutine routes doesn't work
- fix #3228
1 parent 285fa53 commit 92ee4e7

2 files changed

Lines changed: 104 additions & 24 deletions

File tree

jooby/src/main/java/io/jooby/internal/RouterImpl.java

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.Set;
2929
import java.util.concurrent.ConcurrentHashMap;
3030
import java.util.concurrent.Executor;
31+
import java.util.function.BiConsumer;
3132
import java.util.function.Consumer;
3233
import java.util.function.Function;
3334
import java.util.function.Predicate;
@@ -308,8 +309,7 @@ public RouteSet mount(@NonNull Predicate<Context> predicate, @NonNull Runnable b
308309
@NonNull @Override
309310
public Router mount(@NonNull Predicate<Context> predicate, @NonNull Router subrouter) {
310311
/** Override services: */
311-
overrideServices(subrouter);
312-
312+
overrideAll(this, subrouter);
313313
/** Routes: */
314314
mount(
315315
predicate,
@@ -325,7 +325,7 @@ public Router mount(@NonNull Predicate<Context> predicate, @NonNull Router subro
325325
@NonNull @Override
326326
public Router mount(@NonNull String path, @NonNull Router router) {
327327
/** Override services: */
328-
overrideServices(router);
328+
overrideAll(this, router);
329329
/** Merge error handler: */
330330
mergeErrorHandler(router);
331331
/** Routes: */
@@ -857,7 +857,7 @@ public String toString() {
857857
buff.append(String.format("\n %-" + size + "s", r.getMethod()))
858858
.append(r.getPattern()));
859859
}
860-
return buff.length() > 0 ? buff.substring(1) : "";
860+
return !buff.isEmpty() ? buff.substring(1) : "";
861861
}
862862

863863
private Router newStack(RouteTree tree, String pattern, Runnable action, Route.Filter... filter) {
@@ -870,7 +870,7 @@ private Stack push(RouteTree tree) {
870870

871871
private Stack push(RouteTree tree, String pattern) {
872872
Stack stack = new Stack(tree, Router.leadingSlash(pattern));
873-
if (this.stack.size() > 0) {
873+
if (!this.stack.isEmpty()) {
874874
Stack parent = this.stack.getLast();
875875
stack.executor = parent.executor;
876876
}
@@ -880,34 +880,38 @@ private Stack push(RouteTree tree, String pattern) {
880880
private Router newStack(@NonNull Stack stack, @NonNull Runnable action, Route.Filter... filter) {
881881
Stream.of(filter).forEach(stack::then);
882882
this.stack.addLast(stack);
883-
if (action != null) {
884-
action.run();
885-
}
883+
action.run();
886884
this.stack.removeLast().clear();
887885
return this;
888886
}
889887

890888
private void copy(Route src, Route it) {
891-
Route.Filter decorator =
889+
var filter =
892890
Optional.ofNullable(it.getFilter())
893-
.map(filter -> Optional.ofNullable(src.getFilter()).map(filter::then).orElse(filter))
891+
.map(e -> Optional.ofNullable(src.getFilter()).map(e::then).orElse(e))
894892
.orElseGet(src::getFilter);
895893

896-
Route.After after =
894+
var after =
897895
Optional.ofNullable(it.getAfter())
898-
.map(filter -> Optional.ofNullable(src.getAfter()).map(filter::then).orElse(filter))
896+
.map(e -> Optional.ofNullable(src.getAfter()).map(e::then).orElse(e))
899897
.orElseGet(src::getAfter);
900898

901-
it.setFilter(decorator);
899+
it.setPathKeys(src.getPathKeys());
900+
it.setFilter(filter);
902901
it.setAfter(after);
903-
904-
it.setConsumes(src.getConsumes());
905-
it.setProduces(src.getProduces());
906-
it.setHandle(src.getHandle());
902+
it.setEncoder(src.getEncoder());
907903
it.setReturnType(src.getReturnType());
904+
it.setHandle(src.getHandle());
905+
it.setProduces(src.getProduces());
906+
it.setConsumes(src.getConsumes());
908907
it.setAttributes(src.getAttributes());
909908
it.setExecutorKey(src.getExecutorKey());
910-
it.setHandle(src.getHandle());
909+
it.setTags(src.getTags());
910+
it.setDescription(src.getDescription());
911+
it.setDecoders(src.getDecoders());
912+
it.setMvcMethod(it.getMvcMethod());
913+
it.setNonBlocking(src.isNonBlocking());
914+
it.setSummary(src.getSummary());
911915
}
912916

913917
private void putPredicate(@NonNull Predicate<Context> predicate, Chi tree) {
@@ -968,17 +972,22 @@ private void copyRoutes(@NonNull String path, @NonNull Router router) {
968972
}
969973
}
970974

971-
private void overrideServices(Router router) {
975+
private static void override(
976+
RouterImpl src, Router router, BiConsumer<RouterImpl, RouterImpl> consumer) {
972977
if (router instanceof Jooby) {
973978
Jooby app = (Jooby) router;
974-
overrideServices(app.getRouter());
975-
} else if (router instanceof RouterImpl) {
976-
RouterImpl that = (RouterImpl) router;
977-
// Inherited the services from router owner
978-
that.services = this.services;
979+
override(src, app.getRouter(), consumer);
980+
} else if (router instanceof RouterImpl that) {
981+
consumer.accept((RouterImpl) src, that);
979982
}
980983
}
981984

985+
private void overrideAll(RouterImpl src, Router router) {
986+
override(src, router, (self, that) -> that.services = self.services);
987+
override(src, router, (self, that) -> that.worker = self.worker);
988+
override(src, router, (self, that) -> that.attributes.putAll(self.attributes));
989+
}
990+
982991
private void mergeErrorHandler(Router router) {
983992
if (router instanceof Jooby) {
984993
Jooby app = (Jooby) router;
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* Jooby https://jooby.io
3+
* Apache License Version 2.0 https://jooby.io/LICENSE.txt
4+
* Copyright 2014 Edgar Espina
5+
*/
6+
package i3228
7+
8+
import io.jooby.junit.ServerTest
9+
import io.jooby.junit.ServerTestRunner
10+
import io.jooby.kt.Kooby
11+
import java.util.concurrent.Executor
12+
import org.junit.jupiter.api.Assertions
13+
14+
class RouterWithoutWorker :
15+
Kooby({
16+
coroutine {
17+
get("/i3228/without-worker") {
18+
"nonBlocking: " +
19+
ctx.route.isNonBlocking +
20+
", coroutine: " +
21+
ctx.route.attributes["coroutine"]
22+
}
23+
}
24+
})
25+
26+
class RouterWithoutWorkerNoCoroutine :
27+
Kooby({
28+
get("/i3228/without-worker-no-coroutine") {
29+
"nonBlocking: " +
30+
ctx.route.isNonBlocking +
31+
", coroutine: " +
32+
ctx.route.attributes["coroutine"]
33+
}
34+
})
35+
36+
class RouterWithWorker(worker: Executor) :
37+
Kooby({
38+
this.worker = worker
39+
coroutine {
40+
get("/i3228/with-worker") {
41+
"nonBlocking: " +
42+
ctx.route.isNonBlocking +
43+
", coroutine: " +
44+
ctx.route.attributes["coroutine"]
45+
}
46+
}
47+
})
48+
49+
class Issue3228 {
50+
@ServerTest
51+
fun shouldCopyCoroutineState(runner: ServerTestRunner) =
52+
runner
53+
.use {
54+
Kooby { ->
55+
mount(RouterWithWorker(this.worker))
56+
mount(RouterWithoutWorker())
57+
coroutine { mount(RouterWithoutWorkerNoCoroutine()) }
58+
}
59+
}
60+
.ready { client ->
61+
client.get("/i3228/without-worker") { rsp ->
62+
Assertions.assertEquals("nonBlocking: true, coroutine: true", rsp.body!!.string())
63+
}
64+
client.get("/i3228/without-worker-no-coroutine") { rsp ->
65+
Assertions.assertEquals("nonBlocking: true, coroutine: true", rsp.body!!.string())
66+
}
67+
client.get("/i3228/with-worker") { rsp ->
68+
Assertions.assertEquals("nonBlocking: true, coroutine: true", rsp.body!!.string())
69+
}
70+
}
71+
}

0 commit comments

Comments
 (0)