On Fri, 26 Jul 2013 11:19:39 GMT, Manu Evans wrote:
So, here are some first impressions/issues that came up immediately.
Perhaps I'm doing it wrong, and if so, I'd appreciate guidance.1) req.form, req.query, etc, maps throw if you try to access an element that isn't there. This leads to endless spam like this:
if("xys" in req.form) { string xyz = req.form["xyz"]; ... }or
string xyz = "xyz" in req.form ? req.form["xyz"] : null;That's ugly and annoying. Is that really the accepted pattern?
Surely:
string xyz = req.form["xyz"];...is better, and xyz should just be assigned null if not present?
You also have req.form.get("xyz", null) which does exactly that (see http://dlang.org/hash-map.html). Since this is standard D AA behavior, I didn't want to deviate from that potentially creating confusion.
2) HTTPServerRequest has no way to carry user data. URL router is a linear process, and functions like 'check login' that need to look up a user profile should be able to carry that profile across into the following handler. Otherwise the actual handler just has to look up the same user again.
Ie, there is no way to carry state through the URL route. I have a few situations of this sort where it would be simpler and more efficient to carry some state.
The HTTPServerRequest.params field is meant for such cases where data needs to be passed down the request handler chain. Unfortunately for historic reasons it is a string[string] map, although it should rather be Variant[string]. The problem here is just finding a transition that doesn't break all existing code (adding an additional field would be the easy option, but is also kind of ugly unless it actually has a better name...).
3) Fiber Local Storage. Since TLS isn't reliable in the vibe.d environment, an explicit solution should be offered, otherwise users are likely to try using TLS and run into unexpected problems...
There is vibe.core.core.setTaskLocal and getTaskLocal for task local storage. They use a Variant[string] map internally, which is not ideal regarding performance (incl. memory allocations). But a really good solution would probably require some form of compiler support.... well thinking about it, maybe not. It should be possible to create a TaskLocal!T template which registers a slot in a linear per-task memory area.
Task local storage is of course the obvious alternative for 2) until the params field can carry arbitrary types.
BTW 2) and 3) have come up now a number of times, so I'll see how their visibility can be improved in the documentation.
On Fri, 26 Jul 2013 11:26:27 GMT, Manu Evans wrote:
1.2) It's pretty lame that all the examples on the website do stuff like:
res.redirect("/users/"~req.post["user"]);With no comment to note that the application will CRASH if post["user"] is not present. My presumption was that it would just be handled as a null string.
It should actually be caught by the HTTP server and result in a 500 error. However, on Windows and especially in the Visual Studio debugger, any kind of exception can sometimes be unrecoverable due to some issues with the injected exception handler or other issues that I didn't really track down. Maybe that is what you've seen? Anyway, proper input verification would surely be in order for the examples.