Handling Sessions with an 'Environment' #246
Conversation
This can be used to add some compile time dependencies for Middleware and Plugins. To fix code broken by this, you will need to introduce a type parameter for Request, Response, MiddlewareResult and NickelError. ``` // e.g. This fn foo<'a>(&mut Request, Response<'a>) -> MiddlewareResult<'a> // Should become: fn foo<'a, D>(&mut Request<D>, Response<'a, D>) -> MiddlewareResult<'a, D> ``` You can add bounds to `D` in the above to place compile-time restrictions on the server data (can be used for configuration). BREAKING CHANGE
Also adds `on_send` which can be used to execute code when the Response headers are being sent.
The 1.2 beta has a regression for this kind of code, so being explicit should allow us to compile on all targets.
|
Exciting! Should we perhaps document the type hinting for the middleware macro somewhere? I think that one example that uses it will be very easy to miss if one runs into this problem. Other than that, I have no hesitation. Good job! 👍 |
This is motivated to allow writing Plugins which are able to access both the current Response and Request without having to rely on odd syntax such as `(foo, bar).baz()`. BREAKING CHANGE: This breaks a *lot* of nickel usage, checking the diff from the examples should help to migrate code relying on the old structure.
This can sometimes be required when using some middleware which are
predicated on the datatype of the Server. An alternative would be to
litter the handler with extra type annotations (which is awkward without
type ascription), or to use explicit type-annotated functions as middleware.
Example usage:
```
middleware! { |res| <ServerData>
// res is of type Response<ServerData>
}
```
|
Rebased on #247 and updated Ryman@edea3ab with a note about how to use the middleware! macro with a type hint. I'm thinking we should add an explicit trait requirement for I'm sure a lot of this needs a bit more annotation, so let me know where else to add! |
|
Nice. Yes, perhaps an explicit trait could be a good idea. We should probably await comments from someone else. You and I have been working on this for some time, so things that appear to be clear to us might be confusing to others. |
|
I'm currently building a real world project with nickel that needs Session handling. I figured that this branch may be the most up to date and wanted to give it a try. I'm running into |
|
It was added in cookie 0.1.21 by the looks of it, have you tried |
|
Ooops, bloody noob mistake. Never mind ;) |
| format!("You are logged in as: {:?}\n", res.session()) | ||
| }); | ||
|
|
||
| server.post("/login", middleware!{|mut res| |
There was a problem hiding this comment.
I may not see the forest for the tree here. How do I fully test this example? Of course I can send the right json to the running server using something like curl or postman and I get the "successfully logged in" text. But I'd like to test out the entire example (session cookie). So I thought, ok, let's just render a small <form> for the / route that can be used for the login. But that's not gonna work as non ajax submitted forms aren't send as JSON so I would have to change other parts of the example, too.
So I wonder, did you actually test the entire example to see if the session cookie is created by the browser after successful login? If so, how did you test that out?
There was a problem hiding this comment.
See nickel-org/nickel-auth#1, it has a test shell script. (but yes we should have a test written in rust!)
|
I played with this a bit more and noticed that in the How would I go if I would like to create the session just after a specific route was called? |
|
@cburgdorf The session is quite eager, we could hold off on creating it until there is data written to it (as opposed to only read). In the example, when hitting '/' or '/secret', it would create the session as it gets read, fails to find an active session, and so creates a default one ( And yeah, you can't use session at all without implementing the Can you shed light on why you want the Session cookie to only exist after a specific route is called? |
|
I don't really need it that way but it was unexpected at least. Also some
|
|
It's a fair point, so perhaps we should only create the session on writes, we'll also need to consider what options are available to tinker with things, e.g. to set the applicable domain for the cookie. |
👍
Yep, that's a good point. Actually I will need to be able to set the domain for the use case that I currently have. |
|
Hey @Ryman, I have the server that I'm writing as far that it basically does what I want it to do but with a |
|
Closing in favor of #272 |
This builds upon the ideas from #235 to add an 'environment' to each request. See nickel-org/nickel-auth#1 for a related discussion which lead to this proposed change.
Sessionwould be really awkward to use correctly without something along the lines of this.cc @simonpersson