From 6755a69f5583ec34ad5323a16e51aba9faecba0f Mon Sep 17 00:00:00 2001
From: Jeb Bearer <jeb@espressosys.com>
Date: Mon, 22 Aug 2022 11:13:51 -0700
Subject: [PATCH] Support optional parameters.

* Removes a panic in request parsing when a formal parameter from
  another route pattern is not specified
* Fixes some redundancy in route spec parsing
* Removes `RequestParam::required`, as it is not used and not really
  meaningful (a parameter is required relative to a particular route
  pattern, but a route can have many patterns, and Tide already
  ensures that all the parameters for a given pattern are present
  before dispatching to that pattern)
* Adds a new test

Closes #37
---
 src/app.rs     | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++
 src/request.rs |  4 +---
 src/route.rs   | 37 ++++++-------------------------
 3 files changed, 68 insertions(+), 33 deletions(-)

diff --git a/src/app.rs b/src/app.rs
index a32cae50..ce84cf80 100644
--- a/src/app.rs
+++ b/src/app.rs
@@ -681,4 +681,64 @@ mod test {
         };
         assert_eq!(body, "SOCKET");
     }
+
+    /// Test route dispatching for routes with patterns containing different parmaeters
+    #[async_std::test]
+    async fn test_param_dispatch() {
+        let mut app = App::<_, ServerError>::with_state(RwLock::new(()));
+        let api_toml = toml! {
+            [meta]
+            FORMAT_VERSION = "0.1.0"
+
+            [route.test]
+            PATH = ["/test/a/:a", "/test/b/:b"]
+            ":a" = "Integer"
+            ":b" = "Boolean"
+        };
+        {
+            let mut api = app.module::<ServerError>("mod", api_toml).unwrap();
+            api.get("test", |req, _state| {
+                async move {
+                    if let Some(a) = req.opt_integer_param::<_, i32>("a")? {
+                        Ok(("a", a.to_string()))
+                    } else {
+                        Ok(("b", req.boolean_param("b")?.to_string()))
+                    }
+                }
+                .boxed()
+            })
+            .unwrap();
+        }
+        let port = pick_unused_port().unwrap();
+        let url: Url = format!("http://localhost:{}", port).parse().unwrap();
+        spawn(app.serve(format!("0.0.0.0:{}", port)));
+        wait_for_server(&url, SERVER_STARTUP_RETRIES, SERVER_STARTUP_SLEEP_MS).await;
+
+        let client: surf::Client = surf::Config::new()
+            .set_base_url(url.clone())
+            .try_into()
+            .unwrap();
+
+        let mut res = client
+            .get(url.join("mod/test/a/42").unwrap())
+            .send()
+            .await
+            .unwrap();
+        assert_eq!(res.status(), StatusCode::Ok);
+        assert_eq!(
+            res.body_json::<(String, String)>().await.unwrap(),
+            ("a".to_string(), "42".to_string())
+        );
+
+        let mut res = client
+            .get(url.join("mod/test/b/true").unwrap())
+            .send()
+            .await
+            .unwrap();
+        assert_eq!(res.status(), StatusCode::Ok);
+        assert_eq!(
+            res.body_json::<(String, String)>().await.unwrap(),
+            ("b".to_string(), "true".to_string())
+        );
+    }
 }
diff --git a/src/request.rs b/src/request.rs
index 5a549c3b..c316e7d1 100644
--- a/src/request.rs
+++ b/src/request.rs
@@ -430,7 +430,7 @@ impl RequestParamValue {
         if let Ok(param) = req.param(&formal.name) {
             Self::parse(param, formal).map(Some)
         } else {
-            unimplemented!("check for the parameter in the request body")
+            Ok(None)
         }
     }
 
@@ -553,7 +553,6 @@ pub enum RequestParamType {
 pub struct RequestParam {
     pub name: String,
     pub param_type: RequestParamType,
-    pub required: bool,
 }
 
 pub(crate) fn best_response_type(
@@ -613,7 +612,6 @@ mod test {
             &RequestParam {
                 name: name.to_string(),
                 param_type: ty,
-                required: true,
             },
         )
         .unwrap()
diff --git a/src/route.rs b/src/route.rs
index 541d9aab..50475d0f 100644
--- a/src/route.rs
+++ b/src/route.rs
@@ -253,23 +253,21 @@ impl<State, Error> Route<State, Error> {
                     .to_string()
             })
             .collect();
-        let mut pmap = HashMap::<String, RequestParam>::new();
+        let mut params = HashMap::<String, RequestParam>::new();
         for path in paths.iter() {
             for seg in path.split('/') {
-                if seg.starts_with(':') {
-                    // TODO https://github.com/EspressoSystems/tide-disco/issues/56
-                    let ptype = RequestParamType::from_str(
+                if let Some(name) = seg.strip_prefix(':') {
+                    let param_type = RequestParamType::from_str(
                         spec[seg]
                             .as_str()
                             .ok_or(RouteParseError::InvalidTypeExpression)?,
                     )
                     .map_err(|_| RouteParseError::UnrecognizedType)?;
-                    pmap.insert(
+                    params.insert(
                         seg.to_string(),
                         RequestParam {
-                            name: seg.to_string(),
-                            param_type: ptype,
-                            required: true,
+                            name: name.to_string(),
+                            param_type,
                         },
                     );
                 }
@@ -300,28 +298,7 @@ impl<State, Error> Route<State, Error> {
                     .collect::<Result<_, _>>()?,
                 _ => return Err(RouteParseError::IncorrectPathType),
             },
-            params: spec
-                .as_table()
-                .context(RouteMustBeTableSnafu)?
-                .iter()
-                .filter_map(|(key, val)| {
-                    if !key.starts_with(':') {
-                        return None;
-                    }
-                    let ty = match val.as_str() {
-                        Some(ty) => match ty.parse() {
-                            Ok(ty) => ty,
-                            Err(_) => return Some(Err(RouteParseError::IncorrectParamType)),
-                        },
-                        None => return Some(Err(RouteParseError::IncorrectParamType)),
-                    };
-                    Some(Ok(RequestParam {
-                        name: key[1..].to_string(),
-                        param_type: ty,
-                        required: true,
-                    }))
-                })
-                .collect::<Result<_, _>>()?,
+            params: params.into_values().collect(),
             handler,
             doc: match spec.get("DOC") {
                 Some(doc) => markdown::to_html(doc.as_str().context(IncorrectDocTypeSnafu)?),