expose the custom endpoints from RouterServiceFactory

NEXT_CHANGELOG.md

@@ -53,6 +53,13 @@ cargo features. We now use reqwest for that.
 
 By [@geal](https://github.com/geal) in https://github.com/apollographql/router/pull/1392
 
+### Expose the custom endpoints from RouterServiceFactory [PR #1402](https://github.com/apollographql/router/pull/1402)
+
+Plugin HTTP endpoints registration was broken during the Tower refactoring. We now make sure that the list
+of endpoints is generated from the `RouterServiceFactory` instance.
+
+By [@geal](https://github.com/geal) in https://github.com/apollographql/router/pull/1402
+
 ## 🛠 Maintenance
 
 ### Dependency updates [PR #1389](https://github.com/apollographql/router/issues/1389) [PR #1395](https://github.com/apollographql/router/issues/1395)

apollo-router/src/axum_http_server_factory.rs

@@ -765,6 +765,10 @@ mod tests {
         type Future = <<TestRouterServiceFactory as NewService<
             http_ext::Request<graphql::Request>,
         >>::Service as Service<http_ext::Request<graphql::Request>>>::Future;
+
+        fn custom_endpoints(&self) -> HashMap<String, Handler> {
+            HashMap::new()
+        }
     }
 
     async fn init(mut mock: MockRouterService) -> (HttpServerHandle, Client) {

apollo-router/src/router_factory.rs

@@ -1,8 +1,8 @@
+use std::collections::HashMap;
 // This entire file is license key functionality
 use std::sync::Arc;
 
 use futures::stream::BoxStream;
-use indexmap::IndexMap;
 use serde_json::Map;
 use serde_json::Value;
 use tower::BoxError;
@@ -14,8 +14,8 @@ use crate::graphql;
 use crate::http_ext::Request;
 use crate::http_ext::Response;
 use crate::plugin::DynPlugin;
+use crate::plugin::Handler;
 use crate::services::new_service::NewService;
-use crate::services::Plugins;
 use crate::services::RouterCreator;
 use crate::services::SubgraphService;
 use crate::PluggableRouterServiceBuilder;
@@ -35,6 +35,8 @@ pub(crate) trait RouterServiceFactory:
             Future = Self::Future,
         > + Send;
     type Future: Send;
+
+    fn custom_endpoints(&self) -> HashMap<String, Handler>;
 }
 
 /// Factory for creating a RouterServiceFactory
@@ -50,7 +52,7 @@ pub(crate) trait RouterServiceConfigurator: Send + Sync + 'static {
         configuration: Arc<Configuration>,
         schema: Arc<crate::Schema>,
         previous_router: Option<&'a Self::RouterServiceFactory>,
-    ) -> Result<(Self::RouterServiceFactory, Plugins), BoxError>;
+    ) -> Result<Self::RouterServiceFactory, BoxError>;
 }
 
 /// Main implementation of the RouterService factory, supporting the extensions system
@@ -66,7 +68,7 @@ impl RouterServiceConfigurator for YamlRouterServiceFactory {
         configuration: Arc<Configuration>,
         schema: Arc<Schema>,
         _previous_router: Option<&'a Self::RouterServiceFactory>,
-    ) -> Result<(Self::RouterServiceFactory, Plugins), BoxError> {
+    ) -> Result<Self::RouterServiceFactory, BoxError> {
         let mut builder = PluggableRouterServiceBuilder::new(schema.clone());
         if configuration.server.introspection {
             builder = builder.with_naive_introspection();
@@ -95,7 +97,7 @@ impl RouterServiceConfigurator for YamlRouterServiceFactory {
 
         let pluggable_router_service = builder.build().await?;
 
-        Ok((pluggable_router_service, IndexMap::new()))
+        Ok(pluggable_router_service)
     }
 }
 

apollo-router/src/services/router_service.rs

@@ -375,6 +375,18 @@ impl RouterServiceFactory for RouterCreator {
     type Future = <<RouterCreator as NewService<Request<graphql::Request>>>::Service as Service<
         Request<graphql::Request>,
     >>::Future;
+
+    fn custom_endpoints(&self) -> std::collections::HashMap<String, crate::plugin::Handler> {
+        self.plugins
+            .iter()
+            .filter_map(|(plugin_name, plugin)| {
+                (plugin_name.starts_with("apollo.") || plugin_name.starts_with("experimental."))
+                    .then(|| plugin.custom_endpoint())
+                    .flatten()
+                    .map(|h| (plugin_name.clone(), h))
+            })
+            .collect()
+    }
 }
 
 impl RouterCreator {

apollo-router/src/state_machine.rs

@@ -26,7 +26,7 @@ use crate::configuration::Configuration;
 use crate::configuration::ListenAddr;
 use crate::plugin::Handler;
 use crate::router_factory::RouterServiceConfigurator;
-use crate::services::Plugins;
+use crate::router_factory::RouterServiceFactory;
 use crate::Schema;
 
 /// This state maintains private information that is not exposed to the user via state listener.
@@ -44,8 +44,6 @@ enum State<RS> {
         #[derivative(Debug = "ignore")]
         router_service_factory: RS,
         server_handle: HttpServerHandle,
-        #[derivative(Debug = "ignore")]
-        plugins: Plugins,
     },
     Stopped,
     Errored(ApolloRouterError),
@@ -85,6 +83,7 @@ impl<S, FA> StateMachine<S, FA>
 where
     S: HttpServerFactory,
     FA: RouterServiceConfigurator + Send,
+    FA::RouterServiceFactory: RouterServiceFactory,
 {
     pub(crate) fn new(http_server_factory: S, router_factory: FA) -> Self {
         let ready = Arc::new(RwLock::new(None));
@@ -157,7 +156,6 @@ where
                         schema,
                         router_service_factory: router_service,
                         server_handle,
-                        plugins,
                     },
                     UpdateSchema(new_schema),
                 ) => {
@@ -167,7 +165,6 @@ where
                         schema,
                         router_service,
                         server_handle,
-                        plugins,
                         None,
                         Some(Arc::new(*new_schema)),
                     )
@@ -182,7 +179,6 @@ where
                         schema,
                         router_service_factory: router_service,
                         server_handle,
-                        plugins,
                     },
                     UpdateConfiguration(new_configuration),
                 ) => {
@@ -192,7 +188,6 @@ where
                         schema,
                         router_service,
                         server_handle,
-                        plugins,
                         Some(Arc::new(*new_configuration)),
                         None,
                     )
@@ -272,23 +267,15 @@ where
             let configuration = Arc::new(configuration);
             let schema = Arc::new(schema);
 
-            let (router_factory, plugins) = self
+            let router_factory = self
                 .router_configurator
                 .create(configuration.clone(), schema.clone(), None)
                 .await
                 .map_err(|err| {
                     tracing::error!("cannot create the router: {}", err);
                     Errored(ApolloRouterError::ServiceCreationError(err))
                 })?;
-            let plugin_handlers: HashMap<String, Handler> = plugins
-                .iter()
-                .filter_map(|(plugin_name, plugin)| {
-                    (plugin_name.starts_with("apollo.") || plugin_name.starts_with("experimental."))
-                        .then(|| plugin.custom_endpoint())
-                        .flatten()
-                        .map(|h| (plugin_name.clone(), h))
-                })
-                .collect();
+            let plugin_handlers = router_factory.custom_endpoints();
 
             let server_handle = self
                 .http_server_factory
@@ -309,7 +296,6 @@ where
                 schema,
                 router_service_factory: router_factory,
                 server_handle,
-                plugins,
             })
         } else {
             Ok(state)
@@ -323,7 +309,6 @@ where
         schema: Arc<Schema>,
         router_service: <FA as RouterServiceConfigurator>::RouterServiceFactory,
         server_handle: HttpServerHandle,
-        plugins: Plugins,
         new_configuration: Option<Arc<Configuration>>,
         new_schema: Option<Arc<Schema>>,
     ) -> Result<
@@ -342,17 +327,9 @@ where
             )
             .await
         {
-            Ok((new_router_service, plugins)) => {
-                let plugin_handlers: HashMap<String, Handler> = plugins
-                    .iter()
-                    .filter_map(|(plugin_name, plugin)| {
-                        (plugin_name.starts_with("apollo.")
-                            || plugin_name.starts_with("experimental."))
-                        .then(|| plugin.custom_endpoint())
-                        .flatten()
-                        .map(|handler| (plugin_name.clone(), handler))
-                    })
-                    .collect();
+            Ok(new_router_service) => {
+                let plugin_handlers =
+                    new_router_service.custom_endpoints();
 
                 let server_handle = server_handle
                     .restart(
@@ -371,7 +348,6 @@ where
                     schema: new_schema,
                     router_service_factory: new_router_service,
                     server_handle,
-                    plugins,
                 })
             }
             Err(err) => {
@@ -384,7 +360,6 @@ where
                     schema,
                     router_service_factory: router_service,
                     server_handle,
-                    plugins,
                 })
             }
         }
@@ -597,7 +572,8 @@ mod tests {
             .returning(|_, _, _| {
                 let mut router = MockMyRouterFactory::new();
                 router.expect_clone().return_once(MockMyRouterFactory::new);
-                Ok((router, Default::default()))
+                router.expect_custom_endpoints().returning(HashMap::new);
+                Ok(router)
             });
         router_factory
             .expect_create()
@@ -637,7 +613,7 @@ mod tests {
                 configuration: Arc<Configuration>,
                 schema: Arc<crate::Schema>,
                 previous_router: Option<&'a MockMyRouterFactory>,
-            ) -> Result<(MockMyRouterFactory, Plugins), BoxError>;
+            ) -> Result<MockMyRouterFactory, BoxError>;
         }
     }
 
@@ -648,6 +624,7 @@ mod tests {
         impl RouterServiceFactory for MyRouterFactory {
             type RouterService = MockMyRouter;
             type Future = <Self::RouterService as Service<Request<graphql::Request>>>::Future;
+            fn custom_endpoints(&self) -> std::collections::HashMap<String, crate::plugin::Handler>;
         }
         impl  NewService<Request<graphql::Request>> for MyRouterFactory {
             type Service = MockMyRouter;
@@ -774,7 +751,8 @@ mod tests {
             .returning(move |_, _, _| {
                 let mut router = MockMyRouterFactory::new();
                 router.expect_clone().return_once(MockMyRouterFactory::new);
-                Ok((router, Default::default()))
+                router.expect_custom_endpoints().returning(HashMap::new);
+                Ok(router)
             });
         router_factory
     }