Skip to content

Commit 4605b4a

Browse files
committed
Fix ownership on 'NextCallback'
One big drawback of not passing the 'NextCallback' ownership forward is that it cannot be called after the callee has returned (e.g. in a thread or asynchronously). Make it owned across the whole 'perform_routing' recursion.
1 parent e549e1b commit 4605b4a

File tree

6 files changed

+44
-13
lines changed

6 files changed

+44
-13
lines changed

examples/modular-app/admin-router.vala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public class AdminRouter : Router {
2828
/**
2929
* Verify the user credentials and perform an authentication.
3030
*/
31-
public bool authenticate (Request req, Response res, NextCallback next) throws Error {
31+
public bool authenticate (Request req, Response res, owned NextCallback next) throws Error {
3232
string @value;
3333
req.lookup_signed_cookie ("session", ChecksumType.SHA256, "impossible to break".data, out @value);
3434
if (@value == "admin") {
@@ -65,7 +65,7 @@ public class AdminRouter : Router {
6565
/**
6666
* Restricted content.
6767
*/
68-
public bool view (Request req, Response res, NextCallback next, Context ctx) throws Error {
68+
public bool view (Request req, Response res, owned NextCallback next, Context ctx) throws Error {
6969
res.headers.set_content_type ("text/plain", null);
7070
return res.expand_utf8 ("Hello admin!");
7171
}

examples/modular-app/user-router.vala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ public class UserRouter : Router {
2424
get ("/user/<int:id>", view);
2525
}
2626

27-
public bool view (Request req, Response res, NextCallback next, Context ctx) throws Error {
27+
public bool view (Request req, Response res, owned NextCallback next, Context ctx) throws Error {
2828
res.headers.set_content_type ("text/plain", null);
2929
return res.expand_utf8 ("Hello, user %s!".printf (ctx["id"].get_string ()));
3030
}

src/valum/valum-forward.vala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ namespace Valum {
2525
* continuation.
2626
*/
2727
[Version (since = "0.3")]
28-
public bool forward (Request req, Response res, NextCallback next) throws Error {
28+
public bool forward (Request req, Response res, owned NextCallback next) throws Error {
2929
return next ();
3030
}
3131
}

src/valum/valum-router.vala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ namespace Valum {
346346
private bool perform_routing (SequenceIter<Route> routes,
347347
Request req,
348348
Response res,
349-
NextCallback next,
349+
owned NextCallback next,
350350
Context context) throws Informational,
351351
Success,
352352
Redirection,
@@ -362,7 +362,7 @@ namespace Valum {
362362
if (node.next ().is_end ()) {
363363
return next ();
364364
} else {
365-
return perform_routing (node.next (), req, res, next, local_context);
365+
return perform_routing (node.next (), req, res, (owned) next, local_context);
366366
}
367367
}, local_context);
368368
}

src/valum/valum.vala

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,15 @@ namespace Valum {
6464
* otherwise 'false'
6565
*/
6666
[Version (since = "0.1")]
67-
public delegate bool HandlerCallback (Request req,
68-
Response res,
69-
NextCallback next,
70-
Context context) throws Error;
67+
public delegate bool HandlerCallback (Request req,
68+
Response res,
69+
owned NextCallback next,
70+
Context context) throws Informational,
71+
Success,
72+
Redirection,
73+
ClientError,
74+
ServerError,
75+
Error;
7176

7277
/**
7378
* Define a type of {@link Valum.HandlerCallback} that forward a generic
@@ -76,7 +81,7 @@ namespace Valum {
7681
[Version (since = "0.3")]
7782
public delegate bool ForwardCallback<T> (Request req,
7883
Response res,
79-
NextCallback next,
84+
owned NextCallback next,
8085
Context context,
8186
T @value) throws Error;
8287

tests/router-test.vala

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,13 +1015,39 @@ public int main (string[] args) {
10151015
var request = new Request (null, "GET", new Soup.URI ("http://localhost/"));
10161016
var response = new Response (request, Soup.Status.NOT_FOUND);
10171017

1018+
assert (router.handle (request, response));
1019+
});
1020+
1021+
Test.add_func ("/router/next_in_thread", () => {
1022+
var router = new Router ();
1023+
Thread<bool>? thread = null;
1024+
1025+
router.get ("/", (req, res, next) => {
1026+
thread = new Thread<bool> (null, () => {
1027+
try {
1028+
return next ();
1029+
} catch (Error err) {
1030+
assert_not_reached ();
1031+
}
1032+
});
1033+
return true;
1034+
});
1035+
1036+
router.get ("/", (req, res) => {
1037+
return true;
1038+
});
1039+
1040+
var request = new Request (null, "GET", new Soup.URI ("http://localhost/"));
1041+
var response = new Response (request, Soup.Status.NOT_FOUND);
1042+
10181043
try {
1019-
router.handle (request, response);
1044+
assert (router.handle (request, response));
10201045
} catch (Error err) {
10211046
assert_not_reached ();
10221047
}
10231048

1024-
assert (418 == response.status);
1049+
assert (thread != null);
1050+
assert (thread.join ());
10251051
});
10261052

10271053
Test.add_func ("/router/next/not_found", () => {

0 commit comments

Comments
 (0)