-
Notifications
You must be signed in to change notification settings - Fork 4
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't it collect as tuple? This does Vec
allocation in every loop inside map
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How many cookies are there to iterate? I thought there should be only one cookie?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for this, it can also be Cow
or AsRef<str>
to make use of both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this take &[(header::HeaderName, &str)]
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh
@@ -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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cookie = "0.13.3" | |
// testing | |
cookie = "0.13.3" |
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.
e456c1d
to
edf1fce
Compare
Cookies
Session Structure
Session middleware
resolve #42