Repository: awslabs/aws-lambda-rust-runtime (language: rust)

Issue:
LambdaRequest Accessibility
Are there any disadvantages to making `lambda_http::LambdaRequest` public?  

For testing http handlers, I would like to deserialize json to a `http::Request`

I am essentially after performing something akin to https://github.com/awslabs/aws-lambda-rust-runtime/blob/master/lambda-http/src/request.rs#L361.


Discussion / hints:
@dbettin Not really? We merged https://github.com/awslabs/aws-lambda-rust-runtime/pull/85 a few days ago, but didn't actually release it on crates.io—you should be able to take a git dependency on it and rely on it on test code.
@dbettin thanks for reaching out. 

There are good reasons for not making private interfaces public. It makes it possible to change without breaking application code. Depending on what your specific usecase is, there may be alternative ways of enabling what your trying to accomplish and still keep implementation details private.

Can you walk me through your use case in more depth?

As mentioned above, it's currently possible ( on master ) to construct full http requests using http crate builder methods and lambda http request ext methods.



@softprops thanks for the response.

I am quite aware of the general principle of information hiding.  I am more interested in why this specific struct is not public.

Our use case: we are calling our lambdas locally when developing and would like to pass in a file with json as the event.  This works well with non-http lambdas but it is not trivial if we would like to take advantage of the lambda http crate.  

A public LambdaRequest struct makes this a breeze.

Thanks!


I'm thinking an interface that would just look like a func exposed that took in some bytes and returned the request, similar to serde's `from_xxx` functions would satisfy the need without leaking internal impl details.

Would a `lambda_http::request::from_str(loaded_from_disk)` fn work for you?
The reason why the struct is not public is because its an implementation detail. It's only intended as an intermediary to get to the http interface. It's not meant to be a public interface.
@softprops That would work great!

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ab1ca26a..8f914433 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
 # next (unreleased)
 
-- **New**: `lambda_http::RequestExt` crate now exposes mock helper methods under `cfg(test)` builds to facilitate straightforward unit testability of handlers
+- **New**: The `lambda_http` crate now exposes mock helper methods for `RequestExt` under `cfg(test)` builds to facilitate straight forward unit testability of handlers.
+- **New**: The `lambda_http` crate now exposes two new functions for deserializing requests from text and raw IO: `lambda_http::request::{from_str,from_reader}`.
 
 # 0.2.0
 
diff --git a/lambda-http/src/request.rs b/lambda-http/src/request.rs
index a1c9fd27..ab2ddfef 100644
--- a/lambda-http/src/request.rs
+++ b/lambda-http/src/request.rs
@@ -1,10 +1,8 @@
-//! ALB andAPI Gateway request types.
+//! ALB and API Gateway request types.
 //!
 //! Typically these are exposed via the `request_context`
 //! request extension method provided by [lambda_http::RequestExt](../trait.RequestExt.html)
 //!
-use std::{borrow::Cow, collections::HashMap, fmt, mem};
-
 use http::{
     self,
     header::{HeaderName, HeaderValue, HOST},
@@ -12,7 +10,8 @@ use http::{
 };
 use serde::de::{Deserialize, Deserializer, Error as DeError, MapAccess, Visitor};
 use serde_derive::Deserialize;
-use serde_json::Value;
+use serde_json::{error::Error as JsonError, Value};
+use std::{borrow::Cow, collections::HashMap, fmt, io::Read, mem};
 
 use crate::{
     body::Body,
@@ -322,12 +321,55 @@ impl<'a> From<LambdaRequest<'a>> for HttpRequest<Body> {
     }
 }
 
+/// Deserializes a Request from an IO stream of JSON.
+///
+/// # Example
+///
+/// ```rust,no_run
+/// use lambda_http::request::from_reader;
+/// use std::fs::File;
+/// use std::error::Error;
+///
+/// fn main() -> Result<(), Box<dyn Error>> {
+///     let request = from_reader(
+///         File::open("path/to/request.json")?
+///     )?;
+///     Ok(println!("{:#?}", request))
+/// }
+/// ```
+pub fn from_reader<R>(rdr: R) -> Result<crate::Request, JsonError>
+where
+    R: Read,
+{
+    serde_json::from_reader(rdr).map(LambdaRequest::into)
+}
+
+/// Deserializes a Request from a string of JSON text.
+///
+/// # Example
+///
+/// ```rust,no_run
+/// use lambda_http::request::from_str;
+/// use std::fs::File;
+/// use std::error::Error;
+///
+/// fn main() -> Result<(), Box<dyn Error>> {
+///     let request = from_str(
+///         r#"{ ...raw json here... }"#
+///     )?;
+///     Ok(println!("{:#?}", request))
+/// }
+/// ```
+pub fn from_str(s: &str) -> Result<crate::Request, JsonError> {
+    serde_json::from_str(s).map(LambdaRequest::into)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
     use crate::RequestExt;
     use serde_json;
-    use std::collections::HashMap;
+    use std::{collections::HashMap, fs::File};
 
     #[test]
     fn requests_convert() {
@@ -345,12 +387,21 @@ mod tests {
         assert_eq!(expected.method(), actual.method());
     }
 
+    #[test]
+    fn deserializes_apigw_request_events_from_readables() {
+        // from the docs
+        // https://docs.aws.amazon.com/lambda/latest/dg/eventsources.html#eventsources-api-gateway-request
+        // note: file paths are relative to the directory of the crate at runtime
+        let result = from_reader(File::open("tests/data/apigw_proxy_request.json").expect("expected file"));
+        assert!(result.is_ok(), format!("event was not parsed as expected {:?}", result));
+    }
+
     #[test]
     fn deserializes_apigw_request_events() {
         // from the docs
         // https://docs.aws.amazon.com/lambda/latest/dg/eventsources.html#eventsources-api-gateway-request
         let input = include_str!("../tests/data/apigw_proxy_request.json");
-        let result = serde_json::from_str::<LambdaRequest<'_>>(&input);
+        let result = from_str(input);
         assert!(result.is_ok(), format!("event was not parsed as expected {:?}", result));
     }
 
@@ -359,7 +410,7 @@ mod tests {
         // from the docs
         // https://docs.aws.amazon.com/elasticloadbalancing/latest/application/lambda-functions.html#multi-value-headers
         let input = include_str!("../tests/data/alb_request.json");
-        let result = serde_json::from_str::<LambdaRequest<'_>>(&input);
+        let result = from_str(input);
         assert!(result.is_ok(), format!("event was not parsed as expected {:?}", result));
     }
 
@@ -368,19 +419,18 @@ mod tests {
         // from docs
         // 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::<LambdaRequest<'_>>(&input);
+        let result = from_str(input);
         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);
+        let unwrapped = result.unwrap();
+
+        assert!(!unwrapped.query_string_parameters().is_empty());
 
         // test RequestExt#query_string_parameters does the right thing
         assert_eq!(
-            actual.query_string_parameters().get_all("multivalueName"),
+            unwrapped.query_string_parameters().get_all("multivalueName"),
             Some(vec!["you", "me"])
         );
     }
@@ -390,19 +440,17 @@ mod tests {
         // from docs
         // 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/alb_multi_value_request.json");
-        let result = serde_json::from_str::<LambdaRequest<'_>>(&input);
+        let result = from_str(input);
         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);
+        let unwrapped = result.unwrap();
+        assert!(!unwrapped.query_string_parameters().is_empty());
 
         // test RequestExt#query_string_parameters does the right thing
         assert_eq!(
-            actual.query_string_parameters().get_all("myKey"),
+            unwrapped.query_string_parameters().get_all("myKey"),
             Some(vec!["val1", "val2"])
         );
     }


Test patch: