Skip to content

Commit

Permalink
refactor(*): Request is now accessible through a Response instance
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@Ryman
Ryman committed Aug 1, 2015
1 parent 23bb400 commit ebc6aa9
Show file tree
Hide file tree
Showing 21 changed files with 294 additions and 294 deletions.
17 changes: 9 additions & 8 deletions examples/cookies_example.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,23 @@ fn main() {
let mut server = Nickel::with_data(data);

// Try curl -b MyCookie=bar localhost:6767
server.get("/", middleware! { |req|
let cookie = req.cookies().find("MyCookie");
server.get("/", middleware! { |mut res|
let cookie = res.cookies().find("MyCookie");
format!("MyCookie={:?}", cookie.map(|c| c.value))
});

// Note: Don't use get for login in real applications ;)
// Try http://localhost:6767/login?name=foo
server.get("/login", middleware! { |req, mut res|
server.get("/login", middleware! { |mut res|
let cookie = {
let name = res.request.query().get("name")
.unwrap_or("default_name");
Cookie::new("MyCookie".to_owned(), name.to_owned())
};

let jar = res.cookies_mut()
// long life cookies!
.permanent();

let name = req.query().get("name")
.unwrap_or("default_name");
let cookie = Cookie::new("MyCookie".to_owned(),
name.to_owned());
jar.add(cookie);

"Cookie set!"
Expand Down
40 changes: 21 additions & 19 deletions examples/example.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use std::collections::BTreeMap;
use std::io::Write;
use nickel::status::StatusCode::{self, NotFound, BadRequest};
use nickel::{
Nickel, NickelError, Continue, Halt, Request,
QueryString, JsonBody, StaticFilesHandler, HttpRouter, Action, MediaType
Nickel, NickelError, Continue, Halt, QueryString, JsonBody,
StaticFilesHandler, HttpRouter, Action, MediaType
};
use regex::Regex;
use rustc_serialize::json::{Json, ToJson};
Expand Down Expand Up @@ -35,31 +35,31 @@ fn main() {

//this is an example middleware function that just logs each request
// middleware is optional and can be registered with `utilize`
server.utilize(middleware! { |request|
println!("logging request: {:?}", request.origin.uri);
server.utilize(middleware! { |response|
println!("logging request: {:?}", response.request.origin.uri);
});

let mut router = Nickel::router();

// go to http://localhost:6767/user/4711 to see this route in action
router.get("/user/:userid", middleware! { |request|
format!("This is user: {}", request.param("userid").unwrap())
router.get("/user/:userid", middleware! { |response|
format!("This is user: {}", response.param("userid").unwrap())
});

// go to http://localhost:6767/bar to see this route in action
router.get("/bar", middleware!("This is the /bar handler"));

// go to http://localhost:6767/content-type to see this route in action
router.get("/content-type", middleware! { |_, mut response|
router.get("/content-type", middleware! { |mut response|
response.set(MediaType::Json);
"{'foo':'bar'}"
});

let hello_regex = Regex::new("/hello/(?P<name>[a-zA-Z]+)").unwrap();

// go to http://localhost:6767/hello/moomah to see this route in action
router.get(hello_regex, middleware! { |request|
format!("Hello {}", request.param("name").unwrap())
router.get(hello_regex, middleware! { |response|
format!("Hello {}", response.param("name").unwrap())
});

// go to http://localhost:6767/some/crazy/route to see this route in action
Expand All @@ -74,8 +74,8 @@ fn main() {

// try it with curl
// curl 'http://localhost:6767/a/post/request' -H 'Content-Type: application/json;charset=UTF-8' --data-binary $'{ "firstname": "John","lastname": "Connor" }'
router.post("/a/post/request", middleware! { |request, response|
let person = request.json_as::<Person>().unwrap();
router.post("/a/post/request", middleware! { |mut response|
let person = response.request.json_as::<Person>().unwrap();
format!("Hello {} {}", person.firstname, person.lastname)
});

Expand All @@ -89,8 +89,8 @@ fn main() {
});

// try calling http://localhost:6767/query?foo=bar
router.get("/query", middleware! { |request|
if let Some(vals) = request.query().all("foo") {
router.get("/query", middleware! { |mut response|
if let Some(vals) = response.request.query().all("foo") {
format!("Your foo values in the query string are: {:?}", vals)
} else {
format!("You didn't provide any foo values!")
Expand All @@ -99,12 +99,14 @@ fn main() {

// try calling http://localhost:6767/strict?state=valid
// then try calling http://localhost:6767/strict?state=invalid
router.get("/strict", middleware! { |request|
if request.query().get("state") != Some("valid") {
router.get("/strict", middleware! { |mut response|
let reply = if response.request.query().get("state") != Some("valid") {
(BadRequest, "Error Parsing JSON")
} else {
(StatusCode::Ok, "Congratulations on conforming!")
}
};

reply
});

server.utilize(router);
Expand All @@ -113,8 +115,8 @@ fn main() {
server.utilize(StaticFilesHandler::new("examples/assets/"));

//this is how to overwrite the default error handler to handle 404 cases with a custom view
fn custom_404<'a, D>(err: &mut NickelError<D>, _req: &mut Request<D>) -> Action {
if let Some(ref mut res) = err.stream {
fn custom_404<'a, D>(err: &mut NickelError<D>) -> Action {
if let Some(ref mut res) = err.response_mut() {
if res.status() == NotFound {
let _ = res.write_all(b"<h1>Call the police!</h1>");
return Halt(())
Expand All @@ -126,7 +128,7 @@ fn main() {


// issue #20178
let custom_handler: fn(&mut NickelError<()>, &mut Request<()>) -> Action = custom_404;
let custom_handler: fn(&mut NickelError<()>) -> Action = custom_404;

server.handle_error(custom_handler);

Expand Down
2 changes: 1 addition & 1 deletion examples/example_template.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::collections::HashMap;
fn main() {
let mut server = Nickel::new();

server.get("/", middleware! { |_, res|
server.get("/", middleware! { |res|
let mut data = HashMap::<&str, &str>::new();
data.insert("name", "user");
return res.render("examples/assets/template.tpl", &data)
Expand Down
55 changes: 29 additions & 26 deletions examples/macro_example.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ extern crate hyper;
use std::io::Write;
use nickel::status::StatusCode::{self, NotFound};
use nickel::{
Nickel, NickelError, Continue, Halt, Request, Response, MediaType,
QueryString, JsonBody, StaticFilesHandler, MiddlewareResult, HttpRouter, Action
Nickel, NickelError, Continue, Halt, Response, MediaType, QueryString,
JsonBody, StaticFilesHandler, MiddlewareResult, HttpRouter, Action
};
use regex::Regex;
use hyper::header::Location;
Expand All @@ -19,14 +19,14 @@ struct Person {
}

//this is an example middleware function that just logs each request
fn logger<'a, D>(request: &mut Request<D>, response: Response<'a, D>) -> MiddlewareResult<'a, D> {
println!("logging request: {:?}", request.origin.uri);
fn logger<'a, 'k, D>(response: Response<'a, 'k, D>) -> MiddlewareResult<'a, 'k, D> {
println!("logging request: {:?}", response.request.origin.uri);
Ok(Continue(response))
}

//this is how to overwrite the default error handler to handle 404 cases with a custom view
fn custom_404<'a, D>(err: &mut NickelError<D>, _req: &mut Request<D>) -> Action {
if let Some(ref mut res) = err.stream {
fn custom_404<'a, D>(err: &mut NickelError<D>) -> Action {
if let Some(ref mut res) = err.response_mut() {
if res.status() == NotFound {
let _ = res.write_all(b"<h1>Call the police!</h1>");
return Halt(())
Expand All @@ -50,16 +50,16 @@ fn main() {
// The return type for a route can be anything that implements `Responder`
server.utilize(router!(
// go to http://localhost:6767/user/4711 to see this route in action
get "/user/:userid" => |request| {
get "/user/:userid" => |response| {
// returning a String
format!("This is user: {}", request.param("userid").unwrap())
format!("This is user: {}", response.param("userid").unwrap())
}

// go to http://localhost:6767/no_alloc/4711 to see this route in action
get "/no_alloc/:userid" => |request, response| {
// returning a slice of T where T: Display
&["This is user: ", request.param("userid").unwrap()][..]
}
// get "/no_alloc/:userid" => |response| {
// // returning a slice of T where T: Display
// &["This is user: ", response.param("userid").unwrap()][..]
// }

// go to http://localhost:6767/bar to see this route in action
get "/bar" => {
Expand All @@ -68,18 +68,18 @@ fn main() {
}

// go to http://localhost:6767/content-type to see this route in action
get "/content-type" => |_, mut response| {
get "/content-type" => |mut response| {
response.set(MediaType::Json);
"{'foo':'bar'}"
}

// go to http://localhost:6767/hello/moomah to see this route in action
get hello_regex => |request| {
format!("Hello {}", request.param("name").unwrap())
get hello_regex => |response| {
format!("Hello {}", response.param("name").unwrap())
}

// go to http://localhost:6767/redirect to see this route in action
get "/redirect" => |_, mut response| {
get "/redirect" => |mut response| {
response.set(Location("http://nickel.rs".into()));

StatusCode::PermanentRedirect
Expand All @@ -104,25 +104,28 @@ fn main() {

// try it with curl
// curl 'http://localhost:6767/a/post/request' -H 'Content-Type: application/json;charset=UTF-8' --data-binary $'{ "firstname": "John","lastname": "Connor" }'
post "/a/post/request" => |request| {
let person = request.json_as::<Person>().unwrap();
post "/a/post/request" => |mut response| {
let person = response.request.json_as::<Person>().unwrap();
format!("Hello {} {}", person.firstname, person.lastname)
}

// try calling http://localhost:6767/query?foo=bar
get "/query" => |request| {
let query = request.query();
let foo = query.get("foo").unwrap_or("This is only a default value");
let bar = query.get("bar").unwrap_or("This is only a default value");
let text = format!("<p>Your foo values in the query string are: {:?}\
<p>Your bar values are: {:?}",
foo, bar);
get "/query" => |mut response| {
let text = {
let query = response.request.query();
let foo = query.get("foo").unwrap_or("This is only a default value");
let bar = query.get("bar").unwrap_or("This is only a default value");
format!("<p>Your foo values in the query string are: {:?}\
<p>Your bar values are: {:?}",
foo, bar);
};

text
}
));

// issue #20178
let custom_handler: fn(&mut NickelError<()>, &mut Request<()>) -> Action = custom_404;
let custom_handler: fn(&mut NickelError<()>) -> Action = custom_404;

server.handle_error(custom_handler);

Expand Down
49 changes: 13 additions & 36 deletions src/cookies.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use {Request, Response};
use Response;
use plugin::{Plugin, Pluggable};
use typemap::Key;
use hyper::header;
Expand All @@ -11,26 +11,11 @@ pub struct SecretKey(pub [u8; 32]);
pub struct CookiePlugin;
impl Key for CookiePlugin { type Value = CookieJar<'static>; }

impl<'a, 'b, 'k, D> Plugin<Request<'a, 'b, 'k, D>> for CookiePlugin
impl<'a, 'k, D> Plugin<Response<'a, 'k, D>> for CookiePlugin
where D: AsRef<SecretKey> {
type Error = ();

fn eval(req: &mut Request<D>) -> Result<CookieJar<'static>, ()> {
let key = req.data().as_ref();
let jar = match req.origin.headers.get::<header::Cookie>() {
Some(c) => c.to_cookie_jar(&key.0),
None => CookieJar::new(&key.0)
};

Ok(jar)
}
}

impl<'a, 'b, 'k, D> Plugin<Response<'a, D>> for CookiePlugin
where D: AsRef<SecretKey> {
type Error = ();

fn eval(res: &mut Response<'a, D>) -> Result<CookieJar<'static>, ()> {
fn eval(res: &mut Response<'a, 'k, D>) -> Result<CookieJar<'static>, ()> {
// Schedule the cookie to be written when headers are being sent
res.on_send(|res| {
let header = {
Expand All @@ -40,15 +25,17 @@ impl<'a, 'b, 'k, D> Plugin<Response<'a, D>> for CookiePlugin
res.set(header);
});

// Initialise the CookieJar from our Request
let key = res.data().as_ref();
Ok(CookieJar::new(&key.0))
let jar = match res.request.origin.headers.get::<header::Cookie>() {
Some(c) => c.to_cookie_jar(&key.0),
None => CookieJar::new(&key.0)
};

Ok(jar)
}
}

/// Trait to whitelist access to `&'mut CookieJar` via the `Cookies` trait.
pub trait AllowMutCookies {}
impl<'a, D> AllowMutCookies for Response<'a, D> {}

/// Provides access to a `CookieJar`.
///
/// Access to cookies for a `Request` is read-only and represents the cookies
Expand All @@ -67,25 +54,15 @@ pub trait Cookies {
fn cookies(&mut self) -> &CookieJar<'static>;

/// Provides access to a mutable CookieJar.
fn cookies_mut(&mut self) -> &mut CookieJar<'static> where Self: AllowMutCookies;
}

impl<'a, 'b, 'k, D: AsRef<SecretKey>> Cookies for Request<'a, 'b, 'k, D> {
fn cookies(&mut self) -> &<CookiePlugin as Key>::Value {
self.get_ref::<CookiePlugin>().unwrap()
}

fn cookies_mut(&mut self) -> &mut <CookiePlugin as Key>::Value where Self: AllowMutCookies {
unreachable!()
}
fn cookies_mut(&mut self) -> &mut CookieJar<'static>;
}

impl<'a, D: AsRef<SecretKey>> Cookies for Response<'a, D> {
impl<'a, 'k, D: AsRef<SecretKey>> Cookies for Response<'a, 'k, D> {
fn cookies(&mut self) -> &<CookiePlugin as Key>::Value {
self.get_ref::<CookiePlugin>().unwrap()
}

fn cookies_mut(&mut self) -> &mut <CookiePlugin as Key>::Value where Self: AllowMutCookies {
fn cookies_mut(&mut self) -> &mut <CookiePlugin as Key>::Value {
self.get_mut::<CookiePlugin>().unwrap()
}
}
10 changes: 5 additions & 5 deletions src/default_error_handler.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use hyper::status::StatusCode::{NotFound, BadRequest};
use request::Request;
use middleware::{ErrorHandler, Action, Halt};
use nickel_error::NickelError;
use std::io::Write;
Expand All @@ -8,17 +7,18 @@ use std::io::Write;
pub struct DefaultErrorHandler;

impl<D> ErrorHandler<D> for DefaultErrorHandler {
fn handle_error(&self, err: &mut NickelError<D>, _req: &mut Request<D>) -> Action {
if let Some(ref mut res) = err.stream {
fn handle_error(&self, err: &mut NickelError<D>) -> Action {
// Not sure why the borrow isn't being released for the None branch
if err.response().is_none() { println!("Error: {}", err.message) }

if let Some(ref mut res) = err.response_mut() {
let msg : &[u8] = match res.status() {
NotFound => b"Not Found",
BadRequest => b"Bad Request",
_ => b"Internal Server Error"
};

let _ = res.write_all(msg);
} else {
println!("Error: {}", err.message);
}

Halt(())
Expand Down

3 comments on commit ebc6aa9

@cburgdorf
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate on that? Would the odd syntax ((foo, bar).baz()) only apply to Plugins? Because I think the average user writes far less plugins compared to route handler which seem too look a little odd after this change.

@Ryman
Copy link
Owner Author

@Ryman Ryman commented on ebc6aa9 Aug 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wouldn't be for plugin authors, but for usage. For example, the Session plugin requires access to both Request and Response (one for incoming data, one to set outgoing), then the user would need to call something awkward like (req, res).session().use_the_session().

I am concerned about this being unintuitive, can you pin-point which part you find weird in the diff? My guess is response.request? Do you think a renaming would add intuition? (response -> environment / better word)

@cburgdorf
Copy link

@cburgdorf cburgdorf commented on ebc6aa9 Aug 10, 2015 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.