Remove the ResponseBody enum

NEXT_CHANGELOG.md

@@ -262,11 +262,15 @@ 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))
+### Non-GraphQL response body variants removed from `RouterResponse` ([PR #1307](https://github.com/apollographql/router/pull/1307), [PR #1328](https://github.com/apollographql/router/pull/1328))
 
-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.
+The `ResponseBody` enum has been removed.
+It had variants for GraphQL and non-GraphQL responses.
+
+It was used:
+
+* In `RouterResponse` which now uses `apollo_router::graphql::Response` instead
+* In `Handler` for plugin custom endpoints which now uses `bytes::Bytes` instead
 
 Various type signatures will need changes such as:
 

apollo-router/src/axum_http_server_factory.rs

@@ -61,7 +61,6 @@ use crate::http_server_factory::NetworkStream;
 use crate::layers::DEFAULT_BUFFER_SIZE;
 use crate::plugin::Handler;
 use crate::router::ApolloRouterError;
-use crate::ResponseBody;
 
 /// A basic http server using Axum.
 /// Uses streaming as primary method of response.
@@ -414,31 +413,7 @@ async fn custom_plugin_handler(
     head.uri = Uri::from_str(&format!("http://{}{}", host, head.uri))
         .expect("if the authority is some then the URL is valid; qed");
     let req = Request::from_parts(head, body).into();
-    let res = handler.oneshot(req).await.map_err(|err| err.to_string())?;
-
-    let is_json = matches!(
-        res.body(),
-        ResponseBody::GraphQL(_) | ResponseBody::RawJSON(_)
-    );
-
-    let mut res = res.map(|body| match body {
-        ResponseBody::GraphQL(res) => {
-            Bytes::from(serde_json::to_vec(&res).expect("responsebody is serializable; qed"))
-        }
-        ResponseBody::RawJSON(res) => {
-            Bytes::from(serde_json::to_vec(&res).expect("responsebody is serializable; qed"))
-        }
-        ResponseBody::Text(res) => Bytes::from(res),
-    });
-
-    if is_json {
-        res.headers_mut().insert(
-            http::header::CONTENT_TYPE,
-            HeaderValue::from_static("application/json"),
-        );
-    }
-
-    Ok::<_, String>(res)
+    handler.oneshot(req).await.map_err(|err| err.to_string())
 }
 
 async fn handle_get(
@@ -1793,11 +1768,7 @@ Content-Type: application/json\r
                 Ok::<_, BoxError>(http_ext::Response {
                     inner: http::Response::builder()
                         .status(StatusCode::OK)
-                        .body(ResponseBody::Text(format!(
-                            "{} + {}",
-                            req.method(),
-                            req.uri().path()
-                        )))
+                        .body(format!("{} + {}", req.method(), req.uri().path()).into())
                         .unwrap(),
                 })
             })

apollo-router/src/plugin/mod.rs

@@ -45,7 +45,6 @@ use crate::ExecutionRequest;
 use crate::ExecutionResponse;
 use crate::QueryPlannerRequest;
 use crate::QueryPlannerResponse;
-use crate::ResponseBody;
 use crate::RouterRequest;
 use crate::RouterResponse;
 use crate::SubgraphRequest;
@@ -319,14 +318,14 @@ macro_rules! register_plugin {
 #[derive(Clone)]
 pub struct Handler {
     service: Buffer<
-        BoxService<http_ext::Request<Bytes>, http_ext::Response<ResponseBody>, BoxError>,
+        BoxService<http_ext::Request<Bytes>, http_ext::Response<Bytes>, BoxError>,
         http_ext::Request<Bytes>,
     >,
 }
 
 impl Handler {
     pub fn new(
-        service: BoxService<http_ext::Request<Bytes>, http_ext::Response<ResponseBody>, BoxError>,
+        service: BoxService<http_ext::Request<Bytes>, http_ext::Response<Bytes>, BoxError>,
     ) -> Self {
         Self {
             service: ServiceBuilder::new().buffered().service(service),
@@ -335,7 +334,7 @@ impl Handler {
 }
 
 impl Service<http_ext::Request<Bytes>> for Handler {
-    type Response = http_ext::Response<ResponseBody>;
+    type Response = http_ext::Response<Bytes>;
     type Error = BoxError;
     type Future = ResponseFuture<BoxFuture<'static, Result<Self::Response, Self::Error>>>;
 
@@ -348,11 +347,9 @@ impl Service<http_ext::Request<Bytes>> for Handler {
     }
 }
 
-impl From<BoxService<http_ext::Request<Bytes>, http_ext::Response<ResponseBody>, BoxError>>
-    for Handler
-{
+impl From<BoxService<http_ext::Request<Bytes>, http_ext::Response<Bytes>, BoxError>> for Handler {
     fn from(
-        original: BoxService<http_ext::Request<Bytes>, http_ext::Response<ResponseBody>, BoxError>,
+        original: BoxService<http_ext::Request<Bytes>, http_ext::Response<Bytes>, BoxError>,
     ) -> Self {
         Self::new(original)
     }

apollo-router/src/plugins/rhai.rs

@@ -52,7 +52,6 @@ use crate::ExecutionRequest;
 use crate::ExecutionResponse;
 use crate::QueryPlannerRequest;
 use crate::QueryPlannerResponse;
-use crate::ResponseBody;
 use crate::RouterRequest;
 use crate::RouterResponse;
 use crate::SubgraphRequest;
@@ -1354,7 +1353,6 @@ impl Rhai {
             .register_type::<(Option<HeaderName>, HeaderValue)>()
             .register_type::<Request>()
             .register_type::<Object>()
-            .register_type::<ResponseBody>()
             .register_type::<Response>()
             .register_type::<Value>()
             .register_type::<Error>()
@@ -1502,107 +1500,6 @@ impl Rhai {
                 *x = Uri::from_parts(parts).map_err(|e| e.to_string())?;
                 Ok(())
             })
-            // ResponseBody "short-cuts" to bypass the enum
-            // ResponseBody.label
-            .register_get_result("label", |x: &mut ResponseBody| {
-                match x {
-                    ResponseBody::GraphQL(resp) => {
-                        // Because we are short-cutting the response here
-                        // we need to select the label from our resp
-                        to_dynamic(resp.label.clone())
-                    }
-                    _ => Err("wrong type of response".into()),
-                }
-            })
-            .register_set_result("label", |x: &mut ResponseBody, value: Dynamic| {
-                match x {
-                    ResponseBody::GraphQL(resp) => {
-                        // Because we are short-cutting the response here
-                        // we need to update the label on our resp
-                        resp.label = from_dynamic(&value)?;
-                        Ok(())
-                    }
-                    _ => Err("wrong type of response".into()),
-                }
-            })
-            // ResponseBody.data
-            .register_get_result("data", |x: &mut ResponseBody| match x {
-                ResponseBody::GraphQL(resp) => {
-                    // Because we are short-cutting the response here
-                    // we need to select the data from our resp
-                    to_dynamic(resp.data.clone())
-                }
-                _ => Err("wrong type of response".into()),
-            })
-            .register_set_result("data", |x: &mut ResponseBody, om: Map| match x {
-                ResponseBody::GraphQL(resp) => {
-                    // Because we are short-cutting the response here
-                    // we need to update the data on our resp
-                    resp.data = from_dynamic(&om.into())?;
-                    Ok(())
-                }
-                _ => Err("wrong type of response".into()),
-            })
-            // ResponseBody.path (Not Implemented)
-            // ResponseBody.errors
-            .register_get_result("errors", |x: &mut ResponseBody| {
-                match x {
-                    ResponseBody::GraphQL(resp) => {
-                        // Because we are short-cutting the response here
-                        // we need to select the errors from our resp
-                        to_dynamic(resp.errors.clone())
-                    }
-                    _ => Err("wrong type of response".into()),
-                }
-            })
-            .register_set_result("errors", |x: &mut ResponseBody, value: Dynamic| match x {
-                ResponseBody::GraphQL(resp) => {
-                    resp.errors = from_dynamic(&value)?;
-                    Ok(())
-                }
-                _ => Err("wrong type of response".into()),
-            })
-            // ResponseBody.extensions
-            .register_get_result("extensions", |x: &mut ResponseBody| {
-                match x {
-                    ResponseBody::GraphQL(resp) => {
-                        // Because we are short-cutting the response here
-                        // we need to select the extensions from our resp
-                        to_dynamic(resp.extensions.clone())
-                    }
-                    _ => Err("wrong type of response".into()),
-                }
-            })
-            .register_set_result("extensions", |x: &mut ResponseBody, om: Map| {
-                match x {
-                    ResponseBody::GraphQL(resp) => {
-                        // Because we are short-cutting the response here
-                        // we need to update the extensions on our resp
-                        resp.extensions = from_dynamic(&om.into())?;
-                        Ok(())
-                    }
-                    _ => Err("wrong type of response".into()),
-                }
-            })
-            // ResponseBody.response
-            /* We short-cut the treatment of ResponseBody processing to directly
-             * manipulate "data" rather than moving the enum as we probably should.
-             * We do this to "harmonise" the interactions with response data for Rhai
-             * scripts and (effectively) hide the ResponseBody enum from sight.
-             * At some point: ResponseBody should probably be taken out of the
-             * codebase, so this is probably a good decision.
-            .register_get_result("response", |x: &mut ResponseBody| match x {
-                ResponseBody::GraphQL(resp) => Ok(resp.clone()),
-                _ => Err("wrong type of response".into()),
-            })
-            .register_set_result("response", |x: &mut ResponseBody, value: Response| {
-                match x {
-                    ResponseBody::GraphQL(resp) => std::mem::replace(resp, value),
-                    _ => return Err("wrong type of response".into()),
-                };
-                Ok(())
-            })
-            */
             // Response.label
             .register_get("label", |x: &mut Response| {
                 x.label.clone().map_or(Dynamic::UNIT, Dynamic::from)
@@ -1698,9 +1595,6 @@ impl Rhai {
             .register_fn("to_string", |x: &mut Request| -> String {
                 format!("{:?}", x)
             })
-            .register_fn("to_string", |x: &mut ResponseBody| -> String {
-                format!("{:?}", x)
-            })
             .register_fn("to_string", |x: &mut Response| -> String {
                 format!("{:?}", x)
             })

apollo-router/src/plugins/telemetry/metrics/mod.rs

@@ -35,15 +35,14 @@ use crate::plugins::telemetry::config::MetricsCommon;
 use crate::plugins::telemetry::metrics::apollo::Sender;
 use crate::services::RouterResponse;
 use crate::Context;
-use crate::ResponseBody;
 
 pub(crate) mod apollo;
 pub(crate) mod otlp;
 pub(crate) mod prometheus;
 
 pub(crate) type MetricsExporterHandle = Box<dyn Any + Send + Sync + 'static>;
 pub(crate) type CustomEndpoint =
-    BoxService<http_ext::Request<Bytes>, http_ext::Response<ResponseBody>, BoxError>;
+    BoxService<http_ext::Request<Bytes>, http_ext::Response<Bytes>, BoxError>;
 
 #[derive(Debug, Clone, Deserialize, JsonSchema)]
 #[serde(deny_unknown_fields)]

apollo-router/src/plugins/telemetry/metrics/prometheus.rs

@@ -17,7 +17,6 @@ use crate::http_ext;
 use crate::plugins::telemetry::config::MetricsCommon;
 use crate::plugins::telemetry::metrics::MetricsBuilder;
 use crate::plugins::telemetry::metrics::MetricsConfigurator;
-use crate::ResponseBody;
 
 #[derive(Debug, Clone, Deserialize, JsonSchema)]
 #[serde(deny_unknown_fields)]
@@ -57,7 +56,7 @@ pub(crate) struct PrometheusService {
 }
 
 impl Service<http_ext::Request<Bytes>> for PrometheusService {
-    type Response = http_ext::Response<ResponseBody>;
+    type Response = http_ext::Response<Bytes>;
     type Error = BoxError;
     type Future = BoxFuture<'static, Result<Self::Response, Self::Error>>;
 
@@ -74,9 +73,7 @@ impl Service<http_ext::Request<Bytes>> for PrometheusService {
             Ok(http_ext::Response {
                 inner: http::Response::builder()
                     .status(StatusCode::OK)
-                    .body(ResponseBody::Text(
-                        String::from_utf8_lossy(&result).into_owned(),
-                    ))
+                    .body(result.into())
                     .map_err(|err| BoxError::from(err.to_string()))?,
             })
         })