Bring response in line with http-types#562
Conversation
Fishrock123
left a comment
Fishrock123
left a comment
There was a problem hiding this comment.
Overall looks good - guess I didn't have in my brain what all had happened in http_types last week
| /// | ||
| /// The content type is set to `text/plain; charset=utf-8`. | ||
| #[must_use] | ||
| pub fn body_string(mut self, string: String) -> Self { |
There was a problem hiding this comment.
Not so sure about removing these - are we depending on always mime sniffing now? There's definitely some overhead if that's the case.
Otherwise the docs should probably be clear that the user needs to set it themselves.
There was a problem hiding this comment.
Not so sure about removing these - are we depending on always mime sniffing now? There's definitely some overhead if that's the case.
These methods have been moved to rely on Body instead and go through the set_body method instead. Mime sniffing is only applied for Body::from_file -- we set default mime types when creating a body from string, from json, or from bytes -- but in the most cases people will need to set it themselves. The set_body method covers most of the ground the previous methods used to:
// old API
res.body(io::Cursor::new(b"hello world".to_owned()));
res.body_string("hello world".to_string());
// new API
res.set_body(b"hello world");
res.set_body("hello world");The only set of APIs that become a bit more awkward by this are the Serde-related APIs. But at least #523 is showing us improvements for some applications, and with a impl From<Body> for Response bound we could move this even further ahead (in a future patch):
// old API
res.body_json(Cat { name: "chashu" });
// new API
res.set_body(Body::from_json(Cat { name: "chashu" });
// http-rs/tide#523 (return type from function)
Ok(json!({ "name": "chashu" }));
// future option #1
res.set_body(json!({ "name": "chashu" });
// future option #2 (return type from function)
Ok(Body::from_json(Cat { name: "chashu" }));Either way, I hope this shows what the intended workflow around Response bodies is with this patch, and shows some options we could explore in the future to improve this further.
Co-authored-by: Jeremiah Senkpiel <fishrock123@rocketmail.com>
yoshuawuyts
force-pushed
the
normalize-response
branch
from
d15faf2 to
5073c51
Compare
2 participants
This is a follow-up to #537. Brings Response in line with Request. Thanks!
Changes
Response::redirectin favor oftide::RedirectReponse::set_cookietoResponse::insert_cookie.Responsemethods no longer returnSelf.Response::newnow acceptsu16as well asStatusCodeas arguments.Response::header_mutResponse::set_headertoResponse::insert_header.Response::{body_string, body_form, body_json, body}in favor ofResponse::set_body.Response::swap_body.Reponse::set_exttoResponse::insert_ext.