Make juniper_hyper generic over hyper::body::Body (#1263, #1102)

elcharitas · LegNeato · tyranron · web-flow · commit 257bc69dded0 · 2024-09-26T17:30:37.000+02:00
Co-authored-by: Christian Legnitto &lt;LegNeato@users.noreply.github.com&gt;
Co-authored-by: Kai Ren &lt;tyranron@gmail.com&gt;
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -153,17 +153,17 @@ jobs:
           - juniper_graphql_ws
           - juniper_actix
           - juniper_axum
-          - juniper_hyper
+          #- juniper_hyper
           - juniper_rocket
           - juniper_warp
         os:
           - ubuntu
           - macOS
           - windows
-        #include:
-        #  - { msrv: "1.75.0", crate: "juniper_actix", os: "ubuntu" }
-        #  - { msrv: "1.75.0", crate: "juniper_actix", os: "macOS" }
-        #  - { msrv: "1.75.0", crate: "juniper_actix", os: "windows" }
+        include:
+          - { msrv: "1.79.0", crate: "juniper_hyper", os: "ubuntu" }
+          - { msrv: "1.79.0", crate: "juniper_hyper", os: "macOS" }
+          - { msrv: "1.79.0", crate: "juniper_hyper", os: "windows" }
     runs-on: ${{ matrix.os }}-latest
     steps:
       - uses: actions/checkout@v4
diff --git a/juniper_hyper/CHANGELOG.md b/juniper_hyper/CHANGELOG.md
@@ -10,9 +10,11 @@ All user visible changes to `juniper_hyper` crate will be documented in this fil
 
 ### BC Breaks
 
-- Bumped up [MSRV] to 1.75. ([#1272])
+- Bumped up [MSRV] to 1.79. ([#1263])
+- Made `hyper::Request` in `graphql()` and `graphql_sync()` functions generic over `B: hyper::body::Body`. ([#1263], [#1102])
 
-[#1272]: /../../pull/1272
+[#1102]: /../../issues/1102
+[#1263]: /../../pull/1263
 
 
 
diff --git a/juniper_hyper/Cargo.toml b/juniper_hyper/Cargo.toml
@@ -2,7 +2,7 @@
 name = "juniper_hyper"
 version = "0.9.0"
 edition = "2021"
-rust-version = "1.75"
+rust-version = "1.79"
 description = "`juniper` GraphQL integration with `hyper`."
 license = "BSD-2-Clause"
 authors = [
diff --git a/juniper_hyper/README.md b/juniper_hyper/README.md
@@ -4,7 +4,7 @@
 [![Crates.io](https://img.shields.io/crates/v/juniper_hyper.svg?maxAge=2592000)](https://crates.io/crates/juniper_hyper)
 [![Documentation](https://docs.rs/juniper_hyper/badge.svg)](https://docs.rs/juniper_hyper)
 [![CI](https://github.com/graphql-rust/juniper/workflows/CI/badge.svg?branch=master "CI")](https://github.com/graphql-rust/juniper/actions?query=workflow%3ACI+branch%3Amaster)
-[![Rust 1.75+](https://img.shields.io/badge/rustc-1.75+-lightgray.svg "Rust 1.75+")](https://blog.rust-lang.org/2023/12/28/Rust-1.75.0.html)
+[![Rust 1.79+](https://img.shields.io/badge/rustc-1.79+-lightgray.svg "Rust 1.79+")](https://blog.rust-lang.org/2024/06/13/Rust-1.79.0.html)
 
 - [Changelog](https://github.com/graphql-rust/juniper/blob/juniper_hyper-v0.9.0/juniper_hyper/CHANGELOG.md)
 
diff --git a/juniper_hyper/src/lib.rs b/juniper_hyper/src/lib.rs
@@ -4,7 +4,7 @@ use std::{error::Error, fmt, string::FromUtf8Error, sync::Arc};
 
 use http_body_util::BodyExt as _;
 use hyper::{
-    body,
+    body::Body,
     header::{self, HeaderValue},
     Method, Request, Response, StatusCode,
 };
@@ -15,10 +15,10 @@ use juniper::{
 use serde_json::error::Error as SerdeError;
 use url::form_urlencoded;
 
-pub async fn graphql_sync<CtxT, QueryT, MutationT, SubscriptionT, S>(
+pub async fn graphql_sync<CtxT, QueryT, MutationT, SubscriptionT, S, B>(
     root_node: Arc<RootNode<'static, QueryT, MutationT, SubscriptionT, S>>,
     context: Arc<CtxT>,
-    req: Request<body::Incoming>,
+    req: Request<B>,
 ) -> Response<String>
 where
     QueryT: GraphQLType<S, Context = CtxT>,
@@ -29,17 +29,18 @@ where
     SubscriptionT::TypeInfo: Sync,
     CtxT: Sync,
     S: ScalarValue + Send + Sync,
+    B: Body<Error: fmt::Display>,
 {
     match parse_req(req).await {
         Ok(req) => execute_request_sync(root_node, context, req).await,
         Err(resp) => resp,
     }
 }
 
-pub async fn graphql<CtxT, QueryT, MutationT, SubscriptionT, S>(
+pub async fn graphql<CtxT, QueryT, MutationT, SubscriptionT, S, B>(
     root_node: Arc<RootNode<'static, QueryT, MutationT, SubscriptionT, S>>,
     context: Arc<CtxT>,
-    req: Request<body::Incoming>,
+    req: Request<B>,
 ) -> Response<String>
 where
     QueryT: GraphQLTypeAsync<S, Context = CtxT>,
@@ -50,16 +51,19 @@ where
     SubscriptionT::TypeInfo: Sync,
     CtxT: Sync,
     S: ScalarValue + Send + Sync,
+    B: Body<Error: fmt::Display>,
 {
     match parse_req(req).await {
         Ok(req) => execute_request(root_node, context, req).await,
         Err(resp) => resp,
     }
 }
 
-async fn parse_req<S: ScalarValue>(
-    req: Request<body::Incoming>,
-) -> Result<GraphQLBatchRequest<S>, Response<String>> {
+async fn parse_req<S, B>(req: Request<B>) -> Result<GraphQLBatchRequest<S>, Response<String>>
+where
+    S: ScalarValue,
+    B: Body<Error: fmt::Display>,
+{
     match *req.method() {
         Method::GET => parse_get_req(req),
         Method::POST => {
@@ -78,9 +82,11 @@ async fn parse_req<S: ScalarValue>(
     .map_err(render_error)
 }
 
-fn parse_get_req<S: ScalarValue>(
-    req: Request<body::Incoming>,
-) -> Result<GraphQLBatchRequest<S>, GraphQLRequestError> {
+fn parse_get_req<S, B>(req: Request<B>) -> Result<GraphQLBatchRequest<S>, GraphQLRequestError<B>>
+where
+    S: ScalarValue,
+    B: Body,
+{
     req.uri()
         .query()
         .map(|q| gql_request_from_get(q).map(GraphQLBatchRequest::Single))
@@ -91,9 +97,13 @@ fn parse_get_req<S: ScalarValue>(
         })
 }
 
-async fn parse_post_json_req<S: ScalarValue>(
-    body: body::Incoming,
-) -> Result<GraphQLBatchRequest<S>, GraphQLRequestError> {
+async fn parse_post_json_req<S, B>(
+    body: B,
+) -> Result<GraphQLBatchRequest<S>, GraphQLRequestError<B>>
+where
+    S: ScalarValue,
+    B: Body,
+{
     let chunk = body
         .collect()
         .await
@@ -106,9 +116,13 @@ async fn parse_post_json_req<S: ScalarValue>(
         .map_err(GraphQLRequestError::BodyJSONError)
 }
 
-async fn parse_post_graphql_req<S: ScalarValue>(
-    body: body::Incoming,
-) -> Result<GraphQLBatchRequest<S>, GraphQLRequestError> {
+async fn parse_post_graphql_req<S, B>(
+    body: B,
+) -> Result<GraphQLBatchRequest<S>, GraphQLRequestError<B>>
+where
+    S: ScalarValue,
+    B: Body,
+{
     let chunk = body
         .collect()
         .await
@@ -143,7 +157,10 @@ pub async fn playground(
     resp
 }
 
-fn render_error(err: GraphQLRequestError) -> Response<String> {
+fn render_error<B>(err: GraphQLRequestError<B>) -> Response<String>
+where
+    B: Body<Error: fmt::Display>,
+{
     let mut resp = new_response(StatusCode::BAD_REQUEST);
     *resp.body_mut() = err.to_string();
     resp
@@ -211,9 +228,12 @@ where
     resp
 }
 
-fn gql_request_from_get<S>(input: &str) -> Result<JuniperGraphQLRequest<S>, GraphQLRequestError>
+fn gql_request_from_get<S, B>(
+    input: &str,
+) -> Result<JuniperGraphQLRequest<S>, GraphQLRequestError<B>>
 where
     S: ScalarValue,
+    B: Body,
 {
     let mut query = None;
     let mut operation_name = None;
@@ -254,7 +274,7 @@ where
     }
 }
 
-fn invalid_err(parameter_name: &str) -> GraphQLRequestError {
+fn invalid_err<B: Body>(parameter_name: &str) -> GraphQLRequestError<B> {
     GraphQLRequestError::Invalid(format!(
         "`{parameter_name}` parameter is specified multiple times",
     ))
@@ -275,35 +295,57 @@ fn new_html_response(code: StatusCode) -> Response<String> {
     resp
 }
 
-#[derive(Debug)]
-enum GraphQLRequestError {
-    BodyHyper(hyper::Error),
+enum GraphQLRequestError<B: Body> {
+    BodyHyper(B::Error),
     BodyUtf8(FromUtf8Error),
     BodyJSONError(SerdeError),
     Variables(SerdeError),
     Invalid(String),
 }
 
-impl fmt::Display for GraphQLRequestError {
+// NOTE: Manual implementation instead of `#[derive(Debug)]` is used to omit imposing unnecessary
+//       `B: Debug` bound on the implementation.
+impl<B> fmt::Debug for GraphQLRequestError<B>
+where
+    B: Body<Error: fmt::Debug>,
+{
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            GraphQLRequestError::BodyHyper(err) => fmt::Display::fmt(err, f),
-            GraphQLRequestError::BodyUtf8(err) => fmt::Display::fmt(err, f),
-            GraphQLRequestError::BodyJSONError(err) => fmt::Display::fmt(err, f),
-            GraphQLRequestError::Variables(err) => fmt::Display::fmt(err, f),
-            GraphQLRequestError::Invalid(err) => fmt::Display::fmt(err, f),
+            Self::BodyHyper(e) => fmt::Debug::fmt(e, f),
+            Self::BodyUtf8(e) => fmt::Debug::fmt(e, f),
+            Self::BodyJSONError(e) => fmt::Debug::fmt(e, f),
+            Self::Variables(e) => fmt::Debug::fmt(e, f),
+            Self::Invalid(e) => fmt::Debug::fmt(e, f),
         }
     }
 }
 
-impl Error for GraphQLRequestError {
+impl<B> fmt::Display for GraphQLRequestError<B>
+where
+    B: Body<Error: fmt::Display>,
+{
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Self::BodyHyper(e) => fmt::Display::fmt(e, f),
+            Self::BodyUtf8(e) => fmt::Display::fmt(e, f),
+            Self::BodyJSONError(e) => fmt::Display::fmt(e, f),
+            Self::Variables(e) => fmt::Display::fmt(e, f),
+            Self::Invalid(e) => fmt::Display::fmt(e, f),
+        }
+    }
+}
+
+impl<B> Error for GraphQLRequestError<B>
+where
+    B: Body<Error: Error + 'static>,
+{
     fn source(&self) -> Option<&(dyn Error + 'static)> {
         match self {
-            GraphQLRequestError::BodyHyper(err) => Some(err),
-            GraphQLRequestError::BodyUtf8(err) => Some(err),
-            GraphQLRequestError::BodyJSONError(err) => Some(err),
-            GraphQLRequestError::Variables(err) => Some(err),
-            GraphQLRequestError::Invalid(_) => None,
+            Self::BodyHyper(e) => Some(e),
+            Self::BodyUtf8(e) => Some(e),
+            Self::BodyJSONError(e) => Some(e),
+            Self::Variables(e) => Some(e),
+            Self::Invalid(_) => None,
         }
     }
 }
@@ -314,7 +356,11 @@ mod tests {
         convert::Infallible, error::Error, net::SocketAddr, panic, sync::Arc, time::Duration,
     };
 
-    use hyper::{server::conn::http1, service::service_fn, Method, Response, StatusCode};
+    use http_body_util::BodyExt as _;
+    use hyper::{
+        body::Incoming, server::conn::http1, service::service_fn, Method, Request, Response,
+        StatusCode,
+    };
     use hyper_util::rt::TokioIo;
     use juniper::{
         http::tests as http_tests,
@@ -376,8 +422,7 @@ mod tests {
         }
     }
 
-    async fn run_hyper_integration(is_sync: bool) {
-        let port = if is_sync { 3002 } else { 3001 };
+    async fn run_hyper_integration(port: u16, is_sync: bool, is_custom_type: bool) {
         let addr = SocketAddr::from(([127, 0, 0, 1], port));
 
         let db = Arc::new(Database::new());
@@ -405,7 +450,7 @@ mod tests {
                         if let Err(e) = http1::Builder::new()
                             .serve_connection(
                                 io,
-                                service_fn(move |req| {
+                                service_fn(move |req: Request<Incoming>| {
                                     let root_node = root_node.clone();
                                     let db = db.clone();
                                     let matches = {
@@ -419,10 +464,30 @@ mod tests {
                                     };
                                     async move {
                                         Ok::<_, Infallible>(if matches {
-                                            if is_sync {
-                                                super::graphql_sync(root_node, db, req).await
+                                            if is_custom_type {
+                                                let (parts, mut body) = req.into_parts();
+                                                let body = {
+                                                    let mut buf = String::new();
+                                                    if let Some(Ok(frame)) = body.frame().await {
+                                                        if let Ok(bytes) = frame.into_data() {
+                                                            buf = String::from_utf8_lossy(&bytes)
+                                                                .to_string();
+                                                        }
+                                                    }
+                                                    buf
+                                                };
+                                                let req = Request::from_parts(parts, body);
+                                                if is_sync {
+                                                    super::graphql_sync(root_node, db, req).await
+                                                } else {
+                                                    super::graphql(root_node, db, req).await
+                                                }
                                             } else {
-                                                super::graphql(root_node, db, req).await
+                                                if is_sync {
+                                                    super::graphql_sync(root_node, db, req).await
+                                                } else {
+                                                    super::graphql(root_node, db, req).await
+                                                }
                                             }
                                         } else {
                                             let mut resp = Response::new(String::new());
@@ -460,11 +525,21 @@ mod tests {
 
     #[tokio::test]
     async fn test_hyper_integration() {
-        run_hyper_integration(false).await
+        run_hyper_integration(3000, false, false).await
     }
 
     #[tokio::test]
     async fn test_sync_hyper_integration() {
-        run_hyper_integration(true).await
+        run_hyper_integration(3001, true, false).await
+    }
+
+    #[tokio::test]
+    async fn test_custom_request_hyper_integration() {
+        run_hyper_integration(3002, false, false).await
+    }
+
+    #[tokio::test]
+    async fn test_custom_request_sync_hyper_integration() {
+        run_hyper_integration(3003, true, true).await
     }
 }
