Remove non-GraphQL body variants from RouterResponse

NEXT_CHANGELOG.md

@@ -233,6 +233,36 @@ Migration tips:
 
 By [@bryncooke](https://github.com/bryncooke) in https://github.com/apollographql/router/pull/1227 https://github.com/apollographql/router/pull/1234 https://github.com/apollographql/router/pull/1239 https://github.com/apollographql/router/pull/1263
 
+### Non-GraphQL response body variants removed from `RouterResponse` ([PR #1307](https://github.com/apollographql/router/pull/1307))
+
+Previously the `ResponseBody` enum was used in `RouterResponse`.
+One of this enum’s variants contains a `apollo_router::graphql::Response`,
+which is now used directly instead.
+
+Various type signatures will need changes such as:
+
+```diff
+- RouterResponse<BoxStream<'static, ResponseBody>>
++ RouterResponse<BoxStream<'static, graphql::Response>>
+```
+
+Necessary code changes might look like:
+
+```diff
+- return ResponseBody::GraphQL(response);
++ return response;
+```
+```diff
+- if let ResponseBody::GraphQL(graphql_response) = res {
+-     assert_eq!(&graphql_response.errors[0], expected_error);
+- } else {
+-     panic!("expected a graphql response");
+- }
++ assert_eq!(&res.errors[0], expected_error);
+```
+
+By [@SimonSapin](https://github.com/SimonSapin)
+
 ### Fixed control flow in helm chart for volume mounts & environment variables ([PR #1283](https://github.com/apollographql/router/issues/1283))
 
 You will now be able to actually use the helm chart without being on a managed graph. 

apollo-router-benchmarks/src/shared.rs

@@ -2,23 +2,24 @@
 // include!() instead of as a pub module, so it is only compiled
 // in dev mode
 use apollo_router::plugin::test::MockSubgraph;
-use apollo_router::services::{ResponseBody, RouterRequest, RouterResponse};
+use apollo_router::services::{ RouterRequest, RouterResponse};
 use apollo_router::services::PluggableRouterServiceBuilder;
 use apollo_router::Schema;
+use apollo_router::graphql::Response;
 use once_cell::sync::Lazy;
 use serde_json::json;
 use std::sync::Arc;
 use tower::{util::BoxCloneService, BoxError, Service, ServiceExt};
 use futures::stream::{BoxStream};
 
-static EXPECTED_RESPONSE: Lazy<ResponseBody> = Lazy::new(|| {
-    ResponseBody::GraphQL(serde_json::from_str(r#"{"data":{"topProducts":[{"upc":"1","name":"Table","reviews":[{"id":"1","product":{"name":"Table"},"author":{"id":"1","name":"Ada Lovelace"}},{"id":"4","product":{"name":"Table"},"author":{"id":"2","name":"Alan Turing"}}]},{"upc":"2","name":"Couch","reviews":[{"id":"2","product":{"name":"Couch"},"author":{"id":"1","name":"Ada Lovelace"}}]}]}}"#).unwrap())
+static EXPECTED_RESPONSE: Lazy<Response> = Lazy::new(|| {
+    serde_json::from_str(r#"{"data":{"topProducts":[{"upc":"1","name":"Table","reviews":[{"id":"1","product":{"name":"Table"},"author":{"id":"1","name":"Ada Lovelace"}},{"id":"4","product":{"name":"Table"},"author":{"id":"2","name":"Alan Turing"}}]},{"upc":"2","name":"Couch","reviews":[{"id":"2","product":{"name":"Couch"},"author":{"id":"1","name":"Ada Lovelace"}}]}]}}"#).unwrap()
 });
 
 static QUERY: &str = r#"query TopProducts($first: Int) { topProducts(first: $first) { upc name reviews { id product { name } author { id name } } } }"#;
 
 pub async fn basic_composition_benchmark(
-    mut router_service: BoxCloneService<RouterRequest, RouterResponse<BoxStream<'static,ResponseBody>>, BoxError>,
+    mut router_service: BoxCloneService<RouterRequest, RouterResponse<BoxStream<'static,Response>>, BoxError>,
 ) {
     let request = RouterRequest::fake_builder()
         .query(QUERY.to_string())

apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs

@@ -1,23 +1,20 @@
 use apollo_router::plugin::Plugin;
 use apollo_router::register_plugin;
-{{#if type_basic}}
 use apollo_router::graphql::Response;
-use apollo_router::services::ResponseBody;
+{{#if type_basic}}
 use apollo_router::services::{ExecutionRequest, ExecutionResponse};
 use apollo_router::services::{QueryPlannerRequest, QueryPlannerResponse};
 use apollo_router::services::{RouterRequest, RouterResponse};
 use apollo_router::services::{SubgraphRequest, SubgraphResponse};
 {{/if}}
 {{#if type_auth}}
-use apollo_router::services::ResponseBody;
 use apollo_router::services::{RouterRequest, RouterResponse};
 use apollo_router::layers::ServiceBuilderExt;
 use std::ops::ControlFlow;
 use tower::ServiceExt;
 use tower::ServiceBuilder;
 {{/if}}
 {{#if type_tracing}}
-use apollo_router::services::ResponseBody;
 use apollo_router::services::{RouterRequest, RouterResponse};
 use apollo_router::layers::ServiceBuilderExt;
 use tower::ServiceExt;
@@ -56,8 +53,8 @@ impl Plugin for {{pascal_name}} {
     // Delete this function if you are not customizing it.
     fn router_service(
         &mut self,
-        service: BoxService<RouterRequest, RouterResponse<BoxStream<'static, ResponseBody>>, BoxError>,
-    ) -> BoxService<RouterRequest, RouterResponse<BoxStream<'static, ResponseBody>>, BoxError> {
+        service: BoxService<RouterRequest, RouterResponse<BoxStream<'static, Response>>, BoxError>,
+    ) -> BoxService<RouterRequest, RouterResponse<BoxStream<'static, Response>>, BoxError> {
         // Always use service builder to compose your plugins.
         // It provides off the shelf building blocks for your plugin.
         //
@@ -108,8 +105,8 @@ impl Plugin for {{pascal_name}} {
 
     fn router_service(
         &mut self,
-        service: BoxService<RouterRequest, RouterResponse<BoxStream<'static, ResponseBody>>, BoxError>,
-    ) -> BoxService<RouterRequest, RouterResponse<BoxStream<'static, ResponseBody>>, BoxError> {
+        service: BoxService<RouterRequest, RouterResponse<BoxStream<'static, Response>>, BoxError>,
+    ) -> BoxService<RouterRequest, RouterResponse<BoxStream<'static, Response>>, BoxError> {
 
         ServiceBuilder::new()
                     .checkpoint_async(|request : RouterRequest| async {
@@ -135,8 +132,8 @@ impl Plugin for {{pascal_name}} {
 
     fn router_service(
         &mut self,
-        service: BoxService<RouterRequest, RouterResponse<BoxStream<'static, ResponseBody>>, BoxError>,
-    ) -> BoxService<RouterRequest, RouterResponse<BoxStream<'static, ResponseBody>>, BoxError> {
+        service: BoxService<RouterRequest, RouterResponse<BoxStream<'static, Response>>, BoxError>,
+    ) -> BoxService<RouterRequest, RouterResponse<BoxStream<'static, Response>>, BoxError> {
 
         ServiceBuilder::new()
                     .instrument(|_request| {
@@ -166,7 +163,6 @@ mod tests {
     use apollo_router::plugin::test::IntoSchema::Canned;
     use apollo_router::plugin::test::PluginTestHarness;
     use apollo_router::plugin::Plugin;
-    use apollo_router::services::ResponseBody;
     use tower::BoxError;
 
     #[tokio::test]
@@ -204,11 +200,7 @@ mod tests {
             .await
             .expect("couldn't get primary response");
 
-        if let ResponseBody::GraphQL(graphql) = first_response {
-            assert!(graphql.data.is_some());
-        } else {
-            panic!("expected graphql response")
-        }
+        assert!(first_response.data.is_some());
 
         // You could keep calling result.next_response() until it yields None if you're expexting more parts.
         assert!(result.next_response().await.is_none());

apollo-router/src/axum_http_server_factory.rs

@@ -78,7 +78,7 @@ impl AxumHttpServerFactory {
 type BufferedService = Buffer<
     BoxService<
         http_ext::Request<graphql::Request>,
-        http_ext::Response<BoxStream<'static, ResponseBody>>,
+        http_ext::Response<BoxStream<'static, graphql::Response>>,
         BoxError,
     >,
     http_ext::Request<graphql::Request>,
@@ -97,7 +97,7 @@ impl HttpServerFactory for AxumHttpServerFactory {
     where
         RS: Service<
                 http_ext::Request<graphql::Request>,
-                Response = http_ext::Response<BoxStream<'static, ResponseBody>>,
+                Response = http_ext::Response<BoxStream<'static, graphql::Response>>,
                 Error = BoxError,
             > + Send
             + Sync
@@ -747,7 +747,7 @@ mod tests {
     mock! {
         #[derive(Debug)]
         RouterService {
-            fn service_call(&mut self, req: Request<graphql::Request>) -> Result<http_ext::Response<BoxStream<'static, ResponseBody>>, BoxError>;
+            fn service_call(&mut self, req: Request<graphql::Request>) -> Result<http_ext::Response<BoxStream<'static, graphql::Response>>, BoxError>;
         }
     }
 
@@ -922,7 +922,7 @@ mod tests {
                 Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
-                        .body(ResponseBody::GraphQL(example_response))
+                        .body(example_response)
                         .unwrap(),
                 ))
             });
@@ -1013,7 +1013,7 @@ mod tests {
                 Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
-                        .body(ResponseBody::GraphQL(example_response))
+                        .body(example_response)
                         .unwrap(),
                 ))
             });
@@ -1075,7 +1075,7 @@ mod tests {
                 Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
-                        .body(ResponseBody::GraphQL(example_response))
+                        .body(example_response)
                         .unwrap(),
                 ))
             });
@@ -1177,7 +1177,7 @@ mod tests {
                 Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
-                        .body(ResponseBody::GraphQL(example_response))
+                        .body(example_response)
                         .unwrap(),
                 ))
             });
@@ -1246,7 +1246,7 @@ mod tests {
                 Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
-                        .body(ResponseBody::GraphQL(example_response))
+                        .body(example_response)
                         .unwrap(),
                 ))
             });
@@ -1315,7 +1315,7 @@ mod tests {
                 Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
-                        .body(ResponseBody::GraphQL(example_response))
+                        .body(example_response)
                         .unwrap(),
                 ))
             });
@@ -1407,7 +1407,7 @@ mod tests {
                 Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
-                        .body(ResponseBody::GraphQL(example_response))
+                        .body(example_response)
                         .unwrap(),
                 ))
             });
@@ -1467,7 +1467,7 @@ mod tests {
                 Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
-                        .body(ResponseBody::GraphQL(example_response))
+                        .body(example_response)
                         .unwrap(),
                 ))
             });
@@ -1506,7 +1506,7 @@ mod tests {
                 Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
-                        .body(ResponseBody::GraphQL(example_response))
+                        .body(example_response)
                         .unwrap(),
                 ))
             });
@@ -1601,7 +1601,7 @@ mod tests {
                 Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
-                        .body(ResponseBody::GraphQL(example_response))
+                        .body(example_response)
                         .unwrap(),
                 ))
             });
@@ -1877,12 +1877,16 @@ Content-Type: application/json\r
                 Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
-                        .body(ResponseBody::Text(format!(
-                            "{} + {} + {:?}",
-                            req.method(),
-                            req.uri(),
-                            serde_json::to_string(req.body()).unwrap()
-                        )))
+                        .body(
+                            graphql::Response::builder()
+                                .data(json!(format!(
+                                    "{} + {} + {:?}",
+                                    req.method(),
+                                    req.uri(),
+                                    serde_json::to_string(req.body()).unwrap()
+                                )))
+                                .build(),
+                        )
                         .unwrap(),
                 ))
             });
@@ -1897,11 +1901,14 @@ Content-Type: application/json\r
         assert_eq!(response.status(), StatusCode::OK);
         assert_eq!(
             response.text().await.unwrap(),
-            serde_json::to_string(&format!(
-                "GET + {}?query=query + {:?}",
-                url,
-                serde_json::to_string(&query).unwrap()
-            ))
+            serde_json::to_string(&json!({
+                "data":
+                    format!(
+                        "GET + {}?query=query + {:?}",
+                        url,
+                        serde_json::to_string(&query).unwrap()
+                    )
+            }))
             .unwrap()
         );
         let response = client
@@ -1914,11 +1921,14 @@ Content-Type: application/json\r
         assert_eq!(response.status(), StatusCode::OK);
         assert_eq!(
             response.text().await.unwrap(),
-            serde_json::to_string(&format!(
-                "POST + {} + {:?}",
-                url,
-                serde_json::to_string(&query).unwrap()
-            ))
+            serde_json::to_string(&json!({
+                "data":
+                    format!(
+                        "POST + {} + {:?}",
+                        url,
+                        serde_json::to_string(&query).unwrap()
+                    )
+            }))
             .unwrap()
         );
         server.shutdown().await

apollo-router/src/http_server_factory.rs

@@ -16,7 +16,6 @@ use crate::graphql;
 use crate::http_ext::Request;
 use crate::http_ext::Response;
 use crate::plugin::Handler;
-use crate::ResponseBody;
 
 /// Factory for creating the http server component.
 ///
@@ -35,7 +34,7 @@ pub(crate) trait HttpServerFactory {
     where
         RS: Service<
                 Request<graphql::Request>,
-                Response = Response<BoxStream<'static, ResponseBody>>,
+                Response = Response<BoxStream<'static, graphql::Response>>,
                 Error = BoxError,
             > + Send
             + Sync
@@ -101,7 +100,7 @@ impl HttpServerHandle {
         SF: HttpServerFactory,
         RS: Service<
                 Request<graphql::Request>,
-                Response = Response<BoxStream<'static, ResponseBody>>,
+                Response = Response<BoxStream<'static, graphql::Response>>,
                 Error = BoxError,
             > + Send
             + Sync

apollo-router/src/layers/map_future_with_context.rs

@@ -85,9 +85,9 @@ mod test {
     use tower::ServiceBuilder;
     use tower::ServiceExt;
 
+    use crate::graphql::Response;
     use crate::layers::ServiceBuilderExt;
     use crate::plugin::test::MockRouterService;
-    use crate::ResponseBody;
     use crate::RouterRequest;
     use crate::RouterResponse;
 
@@ -109,7 +109,7 @@ mod test {
                         .unwrap()
                 },
                 |ctx: HeaderValue, resp| async move {
-                    let resp: Result<RouterResponse<BoxStream<'static, ResponseBody>>, BoxError> =
+                    let resp: Result<RouterResponse<BoxStream<'static, Response>>, BoxError> =
                         resp.await;
                     resp.map(|mut response| {
                         response.response.headers_mut().insert("hello", ctx.clone());