Feature/cookies session #46
Conversation
|
Blocked until #47 completed. |
src/context.rs
Outdated
| if let Some(ref cookie) = cookie_data.cookie_jar().get(name) { | ||
| return Some(*cookie); | ||
| } |
There was a problem hiding this comment.
It it possible to just use:
return cookie_data.cookie_jar().get(name)
src/middleware/cookie_parser.rs
Outdated
|
|
||
| cookie_str | ||
| .split("; ") | ||
| .map(|x| x.trim().splitn(2, '=').collect::<Vec<&str>>()) |
There was a problem hiding this comment.
Shouldn't it collect as tuple? This does Vec allocation in every loop inside map.
There was a problem hiding this comment.
I was thinking on if there is extra '=' being sent in the http header. Anyway, the whole cookies session will be revamped as the core flow is being changed.
src/router/handler.rs
Outdated
|
|
||
| if let Some(cookies) = response.cookies() { | ||
| if let Some(response_headers) = res.headers_mut() { | ||
| cookies.iter().for_each(move |cookie| { |
There was a problem hiding this comment.
How many cookies are there to iterate? I thought there should be only one cookie?
There was a problem hiding this comment.
set-cookie header can be multiple.
| use http::StatusCode; | ||
| use hyper::{header, Body}; | ||
| use serde::ser::Serialize; | ||
|
|
||
| pub struct Response { | ||
| body: Body, | ||
| status: StatusCode, | ||
| headers: Option<Vec<(header::HeaderName, &'static str)>>, | ||
| headers: Option<Vec<(header::HeaderName, String)>>, |
There was a problem hiding this comment.
Why not use Cow<&'static str> here to allow both &str and String with just little computation cost? If we want it to be smaller, we can use https://crates.io/crates/beef.
| @@ -32,11 +35,15 @@ impl Response { | |||
| self.body | |||
| } | |||
|
|
|||
| pub fn headers(&self) -> &Option<Vec<(header::HeaderName, &'static str)>> { | |||
| pub fn headers(&self) -> &Option<Vec<(header::HeaderName, String)>> { | |||
There was a problem hiding this comment.
Same for this, it can also be Cow or AsRef<str> to make use of both.
There was a problem hiding this comment.
Failed to do this for multiple headers as Cow does not implement Copy. Conversion from Into<Cow<'static, str>> is not possible.
src/router/response.rs
Outdated
| self | ||
| } | ||
|
|
||
| pub fn set_cookies(mut self, mut cookies: Vec<Cookie<'static>>) -> Self { |
There was a problem hiding this comment.
Shouldn't this take &[Cookie<'static>]?
src/router/response.rs
Outdated
| self | ||
| } | ||
|
|
||
| pub fn set_headers(mut self, headers: Vec<(header::HeaderName, &str)>) -> Self { |
There was a problem hiding this comment.
Shouldn't this take &[(header::HeaderName, &str)]?
| @@ -33,6 +33,7 @@ serde_json = '1.0.44' | |||
| url = '2.1.0' | |||
| async-std = '1.4.0' | |||
| async-trait = '0.1.22' | |||
| cookie = "0.13.3" | |||
There was a problem hiding this comment.
| cookie = "0.13.3" | |
| // testing | |
| cookie = "0.13.3" |
plwai
force-pushed
the
feature/cookies_session
branch
from
4548009
to
e456c1d
Compare
Implement set cookies using cookies trait as there are a lot of options within cookie properties.
Implement cookie parser middleware. Enable context cookie getter. Fix context dynamic data get and get_mut return wrong type issue.
Revamp the flow to fit current routing handler. Cookie flow and convertion are improved in this commit too.
plwai
force-pushed
the
feature/cookies_session
branch
from
e456c1d
to
edf1fce
Compare
Cookies
Session Structure
Session middleware
resolve #42