impl http multi value query str parameters (#59)

softprops · davidbarsky · commit 9966ee3b639a · 2018-12-26T08:51:25.000-05:00
* impl multi_value_headers

* remove redundant closure

* impl multi_value_headers in response

* fmt

* unprefix

* missing import in test

* rustfmt

* impl multi_value_query_string_parameters

* improve test assertion msg

* rustfmt
diff --git a/lambda-http/src/ext.rs b/lambda-http/src/ext.rs
@@ -78,7 +78,13 @@ pub enum PayloadError {
 pub trait RequestExt {
     /// Return pre-parsed http query string parameters, parameters
     /// provided after the `?` portion of a url,
-    /// associated with the API gateway request. No query parameters
+    /// associated with the API gateway request.
+    ///
+    /// The yielded value represents both single and multi-valued
+    /// parameters alike. When multiple query string parameters with the same
+    /// name are expected, `query_string_parameters().get_all("many")` to retrieve them all.
+    ///
+    /// No query parameters
     /// will yield an empty `StrMap`.
     fn query_string_parameters(&self) -> StrMap;
     /// Return pre-extracted path parameters, parameter provided in url placeholders
@@ -163,7 +169,7 @@ mod tests {
         let mut headers = HeaderMap::new();
         headers.insert("Host", "www.rust-lang.org".parse().unwrap());
         let mut query = HashMap::new();
-        query.insert("foo".to_owned(), "bar".to_owned());
+        query.insert("foo".to_owned(), vec!["bar".to_owned()]);
         let gwr: GatewayRequest<'_> = GatewayRequest {
             path: "/foo".into(),
             headers,
diff --git a/lambda-http/src/request.rs b/lambda-http/src/request.rs
@@ -35,6 +35,8 @@ pub(crate) struct GatewayRequest<'a> {
     pub(crate) multi_value_headers: HeaderMap<HeaderValue>,
     #[serde(deserialize_with = "nullable_default")]
     pub(crate) query_string_parameters: StrMap,
+    #[serde(default, deserialize_with = "nullable_default")]
+    pub(crate) multi_value_query_string_parameters: StrMap,
     #[serde(deserialize_with = "nullable_default")]
     pub(crate) path_parameters: StrMap,
     #[serde(deserialize_with = "nullable_default")]
@@ -200,6 +202,7 @@ impl<'a> From<GatewayRequest<'a>> for HttpRequest<Body> {
             headers,
             mut multi_value_headers,
             query_string_parameters,
+            multi_value_query_string_parameters,
             path_parameters,
             stage_variables,
             body,
@@ -220,8 +223,16 @@ impl<'a> From<GatewayRequest<'a>> for HttpRequest<Body> {
                 path
             )
         });
-
-        builder.extension(QueryStringParameters(query_string_parameters));
+        // multi valued query string parameters are always a super
+        // set of singly valued query string parameters,
+        // when present, multi-valued query string parameters are preferred
+        builder.extension(QueryStringParameters(
+            if multi_value_query_string_parameters.is_empty() {
+                query_string_parameters
+            } else {
+                multi_value_query_string_parameters
+            },
+        ));
         builder.extension(PathParameters(path_parameters));
         builder.extension(StageVariables(stage_variables));
         builder.extension(request_context);
@@ -262,6 +273,7 @@ impl<'a> From<GatewayRequest<'a>> for HttpRequest<Body> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::RequestExt;
     use serde_json;
     use std::collections::HashMap;
 
@@ -296,7 +308,20 @@ mod tests {
         // https://docs.aws.amazon.com/apigateway/latest/developerguide/set-up-lambda-proxy-integrations.html#api-gateway-simple-proxy-for-lambda-input-format
         let input = include_str!("../tests/data/apigw_multi_value_proxy_request.json");
         let result = serde_json::from_str::<GatewayRequest<'_>>(&input);
-        assert!(result.is_ok(), format!("event was not parsed as expected {:?}", result));
+        assert!(
+            result.is_ok(),
+            format!("event is was not parsed as expected {:?}", result)
+        );
+        let apigw = result.unwrap();
+        assert!(!apigw.query_string_parameters.is_empty());
+        assert!(!apigw.multi_value_query_string_parameters.is_empty());
+        let actual = HttpRequest::from(apigw);
+
+        // test RequestExt#query_string_parameters does the right thing
+        assert_eq!(
+            actual.query_string_parameters().get_all("multivalueName"),
+            Some(vec!["you", "me"])
+        );
     }
 
     #[test]
diff --git a/lambda-http/src/strmap.rs b/lambda-http/src/strmap.rs
@@ -9,14 +9,27 @@ use serde::{
     Deserialize, Deserializer,
 };
 
-/// A read-only view into a map of string data
+/// A read-only view into a map of string data which may contain multiple values
+///
+/// Internally data is always represented as many valued
 #[derive(Default, Debug, PartialEq)]
-pub struct StrMap(pub(crate) Arc<HashMap<String, String>>);
+pub struct StrMap(pub(crate) Arc<HashMap<String, Vec<String>>>);
 
 impl StrMap {
-    /// Return a named value where available
+    /// Return a named value where available.
+    /// If there is more than one value associated with this name,
+    /// the first one will be returned
     pub fn get(&self, key: &str) -> Option<&str> {
-        self.0.get(key).map(|value| value.as_ref())
+        self.0
+            .get(key)
+            .and_then(|values| values.first().map(|owned| owned.as_str()))
+    }
+
+    /// Return all values associated with name where available
+    pub fn get_all(&self, key: &str) -> Option<Vec<&str>> {
+        self.0
+            .get(key)
+            .map(|values| values.iter().map(|owned| owned.as_str()).collect::<Vec<_>>())
     }
 
     /// Return true if the underlying map is empty
@@ -39,16 +52,17 @@ impl Clone for StrMap {
         StrMap(self.0.clone())
     }
 }
-impl From<HashMap<String, String>> for StrMap {
-    fn from(inner: HashMap<String, String>) -> Self {
+
+impl From<HashMap<String, Vec<String>>> for StrMap {
+    fn from(inner: HashMap<String, Vec<String>>) -> Self {
         StrMap(Arc::new(inner))
     }
 }
 
 /// A read only reference to `StrMap` key and value slice pairings
 pub struct StrMapIter<'a> {
     data: &'a StrMap,
-    keys: Keys<'a, String, String>,
+    keys: Keys<'a, String, Vec<String>>,
 }
 
 impl<'a> Iterator for StrMapIter<'a> {
@@ -60,6 +74,15 @@ impl<'a> Iterator for StrMapIter<'a> {
     }
 }
 
+/// internal type used when deserializing StrMaps from
+/// potentially one or many valued maps
+#[derive(serde_derive::Deserialize)]
+#[serde(untagged)]
+enum OneOrMany {
+    One(String),
+    Many(Vec<String>),
+}
+
 impl<'de> Deserialize<'de> for StrMap {
     fn deserialize<D>(deserializer: D) -> Result<StrMap, D::Error>
     where
@@ -78,9 +101,17 @@ impl<'de> Deserialize<'de> for StrMap {
             where
                 A: MapAccess<'de>,
             {
-                let mut inner = HashMap::new();
-                while let Some((key, value)) = map.next_entry()? {
-                    inner.insert(key, value);
+                let mut inner = map.size_hint().map(HashMap::with_capacity).unwrap_or_else(HashMap::new);
+                // values may either be String or Vec<String>
+                // to handle both single and multi value data
+                while let Some((key, value)) = map.next_entry::<_, OneOrMany>()? {
+                    inner.insert(
+                        key,
+                        match value {
+                            OneOrMany::One(one) => vec![one],
+                            OneOrMany::Many(many) => many,
+                        },
+                    );
                 }
                 Ok(StrMap(Arc::new(inner)))
             }
@@ -103,17 +134,26 @@ mod tests {
     #[test]
     fn str_map_get() {
         let mut data = HashMap::new();
-        data.insert("foo".into(), "bar".into());
+        data.insert("foo".into(), vec!["bar".into()]);
         let strmap = StrMap(data.into());
         assert_eq!(strmap.get("foo"), Some("bar"));
         assert_eq!(strmap.get("bar"), None);
     }
 
+    #[test]
+    fn str_map_get_all() {
+        let mut data = HashMap::new();
+        data.insert("foo".into(), vec!["bar".into(), "baz".into()]);
+        let strmap = StrMap(data.into());
+        assert_eq!(strmap.get_all("foo"), Some(vec!["bar", "baz"]));
+        assert_eq!(strmap.get_all("bar"), None);
+    }
+
     #[test]
     fn str_map_iter() {
         let mut data = HashMap::new();
-        data.insert("foo".into(), "bar".into());
-        data.insert("baz".into(), "boom".into());
+        data.insert("foo".into(), vec!["bar".into()]);
+        data.insert("baz".into(), vec!["boom".into()]);
         let strmap = StrMap(data.into());
         let mut values = strmap.iter().map(|(_, v)| v).collect::<Vec<_>>();
         values.sort();
