Change the behaviour of .query: if the caller has not specified que…

…ry parameters, try to deserialise in any case from an empty string. This allows successful deserialisation of structs which do not have mandatory fields (e.g. all fields are Option<X>).

Return detailed error message when deserialisation fails, to help the caller debug what was wrong with their request.

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);
+}