Merge pull request #384 from LukeMathWalker/query-parsing

Change the behaviour of `.query`: optional params and explicit error in the response body

src/request.rs

@@ -267,12 +267,16 @@ impl<State> Request<State> {
 
     /// Get the URL querystring.
     pub fn query<'de, T: Deserialize<'de>>(&'de self) -> Result<T, crate::Error> {
-        let query = self.uri().query();
-        if query.is_none() {
-            return Err(crate::Error::from(http::StatusCode::BAD_REQUEST));
-        }
-        Ok(serde_qs::from_str(query.unwrap())
-            .map_err(|_| crate::Error::from(http::StatusCode::BAD_REQUEST))?)
+        // Default to an empty query string if no query parameter has been specified.
+        // This allows successful deserialisation of structs where all fields are optional
+        // when none of those fields has actually been passed by the caller.
+        let query = self.uri().query().unwrap_or("");
+        serde_qs::from_str(query).map_err(|e| {
+            // Return the displayable version of the deserialisation error to the caller
+            // for easier debugging.
+            let response = crate::Response::new(400).body_string(format!("{}", e));
+            crate::Error::from(response)
+        })
     }
 
     /// Parse the request body as a form.

tests/querystring.rs

@@ -1,53 +1,87 @@
-// use futures::executor::block_on;
-// use http_service::Body;
-// use http_service_mock::make_server;
-// use tide::*;
-
-// #[derive(Deserialize)]
-// struct Params {
-//     msg: String,
-// }
-
-// async fn handler(cx: crate::Request<()>) -> Result<String, Error> {
-//     let p = cx.url_query::<Params>()?;
-//     Ok(p.msg)
-// }
-
-// fn app() -> crate::Server<()> {
-//     let mut app = crate::Server::new();
-//     app.at("/").get(handler);
-//     app
-// }
-
-// #[test]
-// fn successfully_deserialize_query() {
-//     let app = app();
-//     let mut server = make_server(app.into_http_service()).unwrap();
-//     let req = http::Request::get("/?msg=Hello")
-//         .body(Body::empty())
-//         .unwrap();
-//     let res = server.simulate(req).unwrap();
-//     assert_eq!(res.status(), 200);
-//     let body = block_on(res.into_body().into_vec()).unwrap();
-//     assert_eq!(&*body, &*b"Hello");
-// }
-
-// #[test]
-// fn unsuccessfully_deserialize_query() {
-//     let app = app();
-//     let mut server = make_server(app.into_http_service()).unwrap();
-//     let req = http::Request::get("/").body(Body::empty()).unwrap();
-//     let res = server.simulate(req).unwrap();
-//     assert_eq!(res.status(), 400);
-// }
-
-// #[test]
-// fn malformatted_query() {
-//     let app = app();
-//     let mut server = make_server(app.into_http_service()).unwrap();
-//     let req = http::Request::get("/?error=should_fail")
-//         .body(Body::empty())
-//         .unwrap();
-//     let res = server.simulate(req).unwrap();
-//     assert_eq!(res.status(), 400);
-// }
+use async_std::io::prelude::ReadExt;
+use futures::executor::block_on;
+use http_service::Body;
+use http_service_mock::{make_server, TestBackend};
+use serde::Deserialize;
+use tide::{server::Service, IntoResponse, Request, Response, Server};
+
+#[derive(Deserialize)]
+struct Params {
+    msg: String,
+}
+
+#[derive(Deserialize)]
+struct OptionalParams {
+    _msg: Option<String>,
+    _time: Option<u64>,
+}
+
+async fn handler(cx: Request<()>) -> Response {
+    let p = cx.query::<Params>();
+    match p {
+        Ok(params) => params.msg.into_response(),
+        Err(error) => error.into_response(),
+    }
+}
+
+async fn optional_handler(cx: Request<()>) -> Response {
+    let p = cx.query::<OptionalParams>();
+    match p {
+        Ok(_) => Response::new(200),
+        Err(error) => error.into_response(),
+    }
+}
+
+fn get_server() -> TestBackend<Service<()>> {
+    let mut app = Server::new();
+    app.at("/").get(handler);
+    app.at("/optional").get(optional_handler);
+    make_server(app.into_http_service()).unwrap()
+}
+
+#[test]
+fn successfully_deserialize_query() {
+    let mut server = get_server();
+    let req = http::Request::get("/?msg=Hello")
+        .body(Body::empty())
+        .unwrap();
+    let res = server.simulate(req).unwrap();
+    assert_eq!(res.status(), 200);
+    let mut body = String::new();
+    block_on(res.into_body().read_to_string(&mut body)).unwrap();
+    assert_eq!(body, "Hello");
+}
+
+#[test]
+fn unsuccessfully_deserialize_query() {
+    let mut server = get_server();
+    let req = http::Request::get("/").body(Body::empty()).unwrap();
+    let res = server.simulate(req).unwrap();
+    assert_eq!(res.status(), 400);
+
+    let mut body = String::new();
+    block_on(res.into_body().read_to_string(&mut body)).unwrap();
+    assert_eq!(body, "failed with reason: missing field `msg`");
+}
+
+#[test]
+fn malformatted_query() {
+    let mut server = get_server();
+    let req = http::Request::get("/?error=should_fail")
+        .body(Body::empty())
+        .unwrap();
+    let res = server.simulate(req).unwrap();
+    assert_eq!(res.status(), 400);
+
+    let mut body = String::new();
+    block_on(res.into_body().read_to_string(&mut body)).unwrap();
+    assert_eq!(body, "failed with reason: missing field `msg`");
+}
+
+#[test]
+fn empty_query_string_for_struct_with_no_required_fields() {
+    let mut server = get_server();
+    let req = http::Request::get("/optional").body(Body::empty()).unwrap();
+    let res = server.simulate(req).unwrap();
+    assert_eq!(res.status(), 200);
+}