http_compat re-examined (#1291)

rename http_compat to http_ext
Remove remaining convenience functions (apart from Response::map)
Replace Request::mock() with fake_builder()

NEXT_CHANGELOG.md

@@ -26,6 +26,13 @@ By [@USERNAME](https://github.com/USERNAME) in https://github.com/apollographql/
 # [0.9.6] (unreleased) - 2022-mm-dd
 ## ❗ BREAKING ❗
 
+### Rename http_compat to http_ext ([PR #1291](https://github.com/apollographql/router/pull/1291)
+
+The module provides extensions to the `http` crate which are specific to the way we use that crate in the router. This change also cleans up the provided extensions and fixes a few potential sources of error (by removing them)
+such as the Request::mock() fn.
+
+By [@garypen](https://github.com/garypen) in https://github.com/apollographql/router/pull/1257
+
 ### Rework the entire public API structure ([PR #1216](https://github.com/apollographql/router/pull/1216),  [PR #1242](https://github.com/apollographql/router/pull/1242),  [PR #1267](https://github.com/apollographql/router/pull/1267),  [PR #1277](https://github.com/apollographql/router/pull/1277))
 
 * Many items have been removed from the public API and made private.
@@ -179,12 +186,12 @@ use apollo_router::services::RouterService;
 use apollo_router::services::SubgraphRequest;
 use apollo_router::services::SubgraphResponse;
 use apollo_router::services::SubgraphService;
-use apollo_router::services::http_compat::FakeNewRequestBuilder;
-use apollo_router::services::http_compat::IntoHeaderName;
-use apollo_router::services::http_compat::IntoHeaderValue;
-use apollo_router::services::http_compat::NewRequestBuilder;
-use apollo_router::services::http_compat::Request;
-use apollo_router::services::http_compat::Response;
+use apollo_router::services::http_ext::FakeNewRequestBuilder;
+use apollo_router::services::http_ext::IntoHeaderName;
+use apollo_router::services::http_ext::IntoHeaderValue;
+use apollo_router::services::http_ext::NewRequestBuilder;
+use apollo_router::services::http_ext::Request;
+use apollo_router::services::http_ext::Response;
 use apollo_router::subscriber::RouterSubscriber;
 use apollo_router::subscriber::is_global_subscriber_set;
 use apollo_router::subscriber::replace_layer;

apollo-router/src/axum_http_server_factory.rs

@@ -1,6 +1,6 @@
 //! Axum http server factory. Axum provides routing capability on top of Hyper HTTP.
 use crate::configuration::{Configuration, ListenAddr};
-use crate::http_compat;
+use crate::http_ext;
 use crate::http_server_factory::{HttpServerFactory, HttpServerHandle, Listener, NetworkStream};
 use crate::layers::DEFAULT_BUFFER_SIZE;
 use crate::plugin::Handler;
@@ -56,11 +56,11 @@ impl AxumHttpServerFactory {
 
 type BufferedService = Buffer<
     BoxService<
-        http_compat::Request<crate::Request>,
-        http_compat::Response<BoxStream<'static, ResponseBody>>,
+        http_ext::Request<crate::Request>,
+        http_ext::Response<BoxStream<'static, ResponseBody>>,
         BoxError,
     >,
-    http_compat::Request<crate::Request>,
+    http_ext::Request<crate::Request>,
 >;
 
 impl HttpServerFactory for AxumHttpServerFactory {
@@ -75,15 +75,15 @@ impl HttpServerFactory for AxumHttpServerFactory {
     ) -> Self::Future
     where
         RS: Service<
-                http_compat::Request<crate::Request>,
-                Response = http_compat::Response<BoxStream<'static, ResponseBody>>,
+                http_ext::Request<crate::Request>,
+                Response = http_ext::Response<BoxStream<'static, ResponseBody>>,
                 Error = BoxError,
             > + Send
             + Sync
             + Clone
             + 'static,
 
-        <RS as Service<http_compat::Request<crate::Request>>>::Future: std::marker::Send,
+        <RS as Service<http_ext::Request<crate::Request>>>::Future: std::marker::Send,
     {
         let boxed_service = Buffer::new(service.boxed(), DEFAULT_BUFFER_SIZE);
         Box::pin(async move {
@@ -392,7 +392,7 @@ async fn custom_plugin_handler(
         .map_err(|err| err.to_string())?;
     head.uri = Uri::from_str(&format!("http://{}{}", host, head.uri))
         .expect("if the authority is some then the URL is valid; qed");
-    let req = http_compat::Request::from_parts(head, body);
+    let req = Request::from_parts(head, body).into();
     let res = handler.oneshot(req).await.map_err(|err| err.to_string())?;
 
     let is_json = matches!(
@@ -489,10 +489,7 @@ async fn run_graphql_request(
         Ok(mut service) => {
             let (head, body) = http_request.into_parts();
 
-            match service
-                .call(http_compat::Request::from_parts(head, body))
-                .await
-            {
+            match service.call(Request::from_parts(head, body).into()).await {
                 Err(e) => {
                     tracing::error!("router service call failed: {}", e);
                     (
@@ -502,7 +499,7 @@ async fn run_graphql_request(
                         .into_response()
                 }
                 Ok(response) => {
-                    let (parts, mut stream) = response.into_parts();
+                    let (parts, mut stream) = http::Response::from(response).into_parts();
                     match stream.next().await {
                         None => {
                             tracing::error!("router service is not available to process request",);
@@ -514,8 +511,10 @@ async fn run_graphql_request(
                         }
                         Some(response) => {
                             tracing::trace_span!("serialize_response").in_scope(|| {
-                                http_compat::Response::from_parts(parts, response).into_response()
-                                //response.into_response()
+                                http_ext::Response::from(http::Response::from_parts(
+                                    parts, response,
+                                ))
+                                .into_response()
                             })
                         }
                     }
@@ -660,7 +659,7 @@ impl<B> MakeSpan<B> for PropagatingMakeSpan {
 mod tests {
     use super::*;
     use crate::configuration::Cors;
-    use crate::http_compat::Request;
+    use crate::http_ext::Request;
     use async_compression::tokio::write::GzipEncoder;
     use http::header::{self, ACCEPT_ENCODING, CONTENT_TYPE};
     use mockall::mock;
@@ -719,7 +718,7 @@ mod tests {
     mock! {
         #[derive(Debug)]
         RouterService {
-            fn service_call(&mut self, req: Request<crate::Request>) -> Result<http_compat::Response<BoxStream<'static, ResponseBody>>, BoxError>;
+            fn service_call(&mut self, req: Request<crate::Request>) -> Result<http_ext::Response<BoxStream<'static, ResponseBody>>, BoxError>;
         }
     }
 
@@ -891,7 +890,7 @@ mod tests {
             .times(2)
             .returning(move |_req| {
                 let example_response = example_response.clone();
-                Ok(http_compat::Response::from_response_to_stream(
+                Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
                         .body(ResponseBody::GraphQL(example_response))
@@ -982,7 +981,7 @@ mod tests {
             })
             .returning(move |_req| {
                 let example_response = example_response.clone();
-                Ok(http_compat::Response::from_response_to_stream(
+                Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
                         .body(ResponseBody::GraphQL(example_response))
@@ -1044,7 +1043,7 @@ mod tests {
             .times(2)
             .returning(move |_| {
                 let example_response = example_response.clone();
-                Ok(http_compat::Response::from_response_to_stream(
+                Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
                         .body(ResponseBody::GraphQL(example_response))
@@ -1146,7 +1145,7 @@ mod tests {
             .times(2)
             .returning(move |_| {
                 let example_response = example_response.clone();
-                Ok(http_compat::Response::from_response_to_stream(
+                Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
                         .body(ResponseBody::GraphQL(example_response))
@@ -1215,7 +1214,7 @@ mod tests {
             .times(2)
             .returning(move |_| {
                 let example_response = example_response.clone();
-                Ok(http_compat::Response::from_response_to_stream(
+                Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
                         .body(ResponseBody::GraphQL(example_response))
@@ -1284,7 +1283,7 @@ mod tests {
             .times(4)
             .returning(move |_| {
                 let example_response = example_response.clone();
-                Ok(http_compat::Response::from_response_to_stream(
+                Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
                         .body(ResponseBody::GraphQL(example_response))
@@ -1376,7 +1375,7 @@ mod tests {
             })
             .returning(move |_| {
                 let example_response = example_response.clone();
-                Ok(http_compat::Response::from_response_to_stream(
+                Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
                         .body(ResponseBody::GraphQL(example_response))
@@ -1436,7 +1435,7 @@ mod tests {
             })
             .returning(move |_| {
                 let example_response = example_response.clone();
-                Ok(http_compat::Response::from_response_to_stream(
+                Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
                         .body(ResponseBody::GraphQL(example_response))
@@ -1475,7 +1474,7 @@ mod tests {
                     reason: "Mock error".to_string(),
                 }
                 .to_response();
-                Ok(http_compat::Response::from_response_to_stream(
+                Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
                         .body(ResponseBody::GraphQL(example_response))
@@ -1570,7 +1569,7 @@ mod tests {
             .returning(move |_| {
                 let example_response = example_response.clone();
 
-                Ok(http_compat::Response::from_response_to_stream(
+                Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
                         .body(ResponseBody::GraphQL(example_response))
@@ -1759,8 +1758,8 @@ Content-Type: application/json\r
     async fn it_answers_to_custom_endpoint() -> Result<(), ApolloRouterError> {
         let expectations = MockRouterService::new();
         let plugin_handler = Handler::new(
-            service_fn(|req: http_compat::Request<Bytes>| async move {
-                Ok::<_, BoxError>(http_compat::Response {
+            service_fn(|req: http_ext::Request<Bytes>| async move {
+                Ok::<_, BoxError>(http_ext::Response {
                     inner: http::Response::builder()
                         .status(StatusCode::OK)
                         .body(ResponseBody::Text(format!(
@@ -1844,7 +1843,7 @@ Content-Type: application/json\r
             .expect_service_call()
             .times(2)
             .returning(move |req| {
-                Ok(http_compat::Response::from_response_to_stream(
+                Ok(http_ext::Response::from_response_to_stream(
                     http::Response::builder()
                         .status(200)
                         .body(ResponseBody::Text(format!(

apollo-router/src/context.rs

@@ -13,7 +13,7 @@ use tower::BoxError;
 /// Holds [`Context`] entries.
 pub(crate) type Entries = Arc<DashMap<String, Value>>;
 
-/// Context for a [`crate::http_compat::Request`]
+/// Context for a [`crate::http_ext::Request`]
 ///
 /// Context makes use of [`DashMap`] under the hood which tries to handle concurrency
 /// by allowing concurrency across threads without requiring locking. This is great

apollo-router/src/http_server_factory.rs

@@ -1,6 +1,6 @@
 use super::router::ApolloRouterError;
 use crate::configuration::{Configuration, ListenAddr};
-use crate::http_compat::{Request, Response};
+use crate::http_ext::{Request, Response};
 use crate::plugin::Handler;
 use crate::ResponseBody;
 use derivative::Derivative;

apollo-router/src/lib.rs

@@ -59,7 +59,7 @@ pub use executable::{main, Executable};
 pub use request::Request;
 pub use response::Response;
 pub use router::{ApolloRouter, ConfigurationKind, SchemaKind, ShutdownKind};
-pub use services::http_compat;
+pub use services::http_ext;
 pub use spec::Schema;
 
 // TODO: clean these up and import from relevant modules instead

apollo-router/src/plugin/mod.rs

@@ -19,7 +19,7 @@ pub mod test;
 
 use crate::layers::ServiceBuilderExt;
 use crate::{
-    http_compat, ExecutionRequest, ExecutionResponse, QueryPlannerRequest, QueryPlannerResponse,
+    http_ext, ExecutionRequest, ExecutionResponse, QueryPlannerRequest, QueryPlannerResponse,
     Response, ResponseBody, RouterRequest, RouterResponse, SubgraphRequest, SubgraphResponse,
 };
 use ::serde::{de::DeserializeOwned, Deserialize};
@@ -318,48 +318,40 @@ macro_rules! register_plugin {
 #[derive(Clone)]
 pub struct Handler {
     service: Buffer<
-        BoxService<http_compat::Request<Bytes>, http_compat::Response<ResponseBody>, BoxError>,
-        http_compat::Request<Bytes>,
+        BoxService<http_ext::Request<Bytes>, http_ext::Response<ResponseBody>, BoxError>,
+        http_ext::Request<Bytes>,
     >,
 }
 
 impl Handler {
     pub fn new(
-        service: BoxService<
-            http_compat::Request<Bytes>,
-            http_compat::Response<ResponseBody>,
-            BoxError,
-        >,
+        service: BoxService<http_ext::Request<Bytes>, http_ext::Response<ResponseBody>, BoxError>,
     ) -> Self {
         Self {
             service: ServiceBuilder::new().buffered().service(service),
         }
     }
 }
 
-impl Service<http_compat::Request<Bytes>> for Handler {
-    type Response = http_compat::Response<ResponseBody>;
+impl Service<http_ext::Request<Bytes>> for Handler {
+    type Response = http_ext::Response<ResponseBody>;
     type Error = BoxError;
     type Future = ResponseFuture<BoxFuture<'static, Result<Self::Response, Self::Error>>>;
 
     fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
         self.service.poll_ready(cx)
     }
 
-    fn call(&mut self, req: http_compat::Request<Bytes>) -> Self::Future {
+    fn call(&mut self, req: http_ext::Request<Bytes>) -> Self::Future {
         self.service.call(req)
     }
 }
 
-impl From<BoxService<http_compat::Request<Bytes>, http_compat::Response<ResponseBody>, BoxError>>
+impl From<BoxService<http_ext::Request<Bytes>, http_ext::Response<ResponseBody>, BoxError>>
     for Handler
 {
     fn from(
-        original: BoxService<
-            http_compat::Request<Bytes>,
-            http_compat::Response<ResponseBody>,
-            BoxError,
-        >,
+        original: BoxService<http_ext::Request<Bytes>, http_ext::Response<ResponseBody>, BoxError>,
     ) -> Self {
         Self::new(original)
     }

apollo-router/src/plugin/test/mock/subgraph.rs

@@ -50,7 +50,7 @@ impl Service<SubgraphRequest> for MockSubgraph {
                 .expect("Response is serializable; qed");
 
             // Create a compatible Response
-            let compat_response = crate::http_compat::Response {
+            let compat_response = crate::http_ext::Response {
                 inner: http_response,
             };
 

apollo-router/src/plugins/forbid_mutations.rs

@@ -62,7 +62,7 @@ impl Plugin for ForbidMutations {
 #[cfg(test)]
 mod forbid_http_get_mutations_tests {
     use super::*;
-    use crate::http_compat::Request;
+    use crate::http_ext::Request;
     use crate::plugin::test::MockExecutionService;
     use crate::query_planner::fetch::OperationKind;
     use crate::query_planner::{PlanNode, QueryPlan};

apollo-router/src/plugins/headers.rs

@@ -233,7 +233,7 @@ where
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::http_compat;
+    use crate::http_ext;
     use crate::plugin::test::MockSubgraphService;
     use crate::plugins::headers::{Config, HeadersLayer};
     use crate::query_planner::fetch::OperationKind;
@@ -516,7 +516,7 @@ mod test {
     fn example_request() -> SubgraphRequest {
         SubgraphRequest {
             originating_request: Arc::new(
-                http_compat::Request::fake_builder()
+                http_ext::Request::fake_builder()
                     .header("da", "vda")
                     .header("db", "vdb")
                     .header("db", "vdb")
@@ -527,7 +527,7 @@ mod test {
                     .build()
                     .expect("expecting valid request"),
             ),
-            subgraph_request: http_compat::Request::fake_builder()
+            subgraph_request: http_ext::Request::fake_builder()
                 .header("aa", "vaa")
                 .header("ab", "vab")
                 .header("ac", "vac")