From f3601a08972f0d58779930a4e116b36364debb4a Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 12:07:25 +0200 Subject: [PATCH 01/28] =?UTF-8?q?Remove=20no-op=20use=20of=20make=5Fservic?= =?UTF-8?q?e=20on=20axum=E2=80=99s=20Router?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `Router` type already implements `Service` and `Clone`. --- apollo-router/src/axum_http_server_factory.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/apollo-router/src/axum_http_server_factory.rs b/apollo-router/src/axum_http_server_factory.rs index 14faf10e20..0cab3e5d43 100644 --- a/apollo-router/src/axum_http_server_factory.rs +++ b/apollo-router/src/axum_http_server_factory.rs @@ -44,7 +44,6 @@ use tokio::net::UnixListener; use tokio::sync::Notify; use tower::util::BoxService; use tower::BoxError; -use tower::MakeService; use tower::ServiceExt; use tower_http::compression::CompressionLayer; use tower_http::trace::MakeSpan; @@ -188,8 +187,6 @@ impl HttpServerFactory for AxumHttpServerFactory { ); } - let svc = router.into_make_service(); - // if we received a TCP listener, reuse it, otherwise create a new one #[cfg_attr(not(unix), allow(unused_mut))] let mut listener = if let Some(listener) = listener { @@ -233,7 +230,7 @@ impl HttpServerFactory for AxumHttpServerFactory { break; } res = listener.accept() => { - let mut svc = svc.clone(); + let app = router.clone(); let connection_shutdown = connection_shutdown.clone(); match res { @@ -246,8 +243,6 @@ impl HttpServerFactory for AxumHttpServerFactory { tokio::task::spawn(async move{ match res { NetworkStream::Tcp(stream) => { - // TODO: unwrap? - let app = svc.make_service(&stream).await.unwrap(); stream .set_nodelay(true) .expect( @@ -275,8 +270,6 @@ impl HttpServerFactory for AxumHttpServerFactory { } #[cfg(unix)] NetworkStream::Unix(stream) => { - // TODO: unwrap? - let app = svc.make_service(&stream).await.unwrap(); let connection = Http::new() .http1_keep_alive(true) .serve_connection(stream, app); From a66a665335a7a2605b15d465e5bb6af735076af5 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 13:37:33 +0200 Subject: [PATCH 02/28] =?UTF-8?q?Make=20constructors=20private=20when=20th?= =?UTF-8?q?ere=E2=80=99s=20a=20public=20builder?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This uses https://github.com/BrynCooke/buildstructor/issues/70 --- NEXT_CHANGELOG.md | 19 ++++++ apollo-router/src/executable.rs | 4 +- apollo-router/src/graphql.rs | 4 +- apollo-router/src/plugin/test/mod.rs | 4 +- apollo-router/src/request.rs | 8 +-- apollo-router/src/response.rs | 4 +- apollo-router/src/router.rs | 4 +- apollo-router/src/services/http_ext.rs | 8 +-- apollo-router/src/services/mod.rs | 68 ++++++++++---------- apollo-router/src/services/router_service.rs | 4 +- 10 files changed, 73 insertions(+), 54 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 9a377f16dc..9e5686fb4b 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -82,6 +82,25 @@ This rewrites the mocked services API to remove the `build()` method, and make t using an `expect_clone` call with mockall. By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/1440 + +### Removed constructors when there is a public builder ([PR #FIXME]) + +Many types in the Router API can be constructed with the builder pattern. +We use the [`buildstructor`](https://crates.io/crates/buildstructor) crate +to auto-generate builder boilerplate based on the parameters of a constructor. +These constructors have been made private so that users must go through the builder instead, +which will allow us to add parameters in the future without a breaking API change. +If you were using one of these constructors, the migration generally looks like this: + +```diff +-apollo_router::graphql::Error::new(m, vec![l], Some(p), Default::default()) ++apollo_router::graphql::Error::build() ++ .message(m) ++ .location(l) ++ .path(p) ++ .build() +``` + ## πŸš€ Features ### Expose query plan in extensions for GraphQL response (experimental) ([PR #1470](https://github.com/apollographql/router/pull/1470)) diff --git a/apollo-router/src/executable.rs b/apollo-router/src/executable.rs index 587b184b6d..b4605e8063 100644 --- a/apollo-router/src/executable.rs +++ b/apollo-router/src/executable.rs @@ -174,8 +174,8 @@ impl Executable { /// ``` /// Note that if you do not specify a runtime you must be in the context of an existing tokio runtime. /// - #[builder(entry = "builder", exit = "start")] - pub async fn start( + #[builder(entry = "builder", exit = "start", visibility = "pub")] + async fn start( router_builder_fn: Option ApolloRouter>, ) -> Result<()> { let opt = Opt::parse(); diff --git a/apollo-router/src/graphql.rs b/apollo-router/src/graphql.rs index 01d538ea8b..eb30ecd906 100644 --- a/apollo-router/src/graphql.rs +++ b/apollo-router/src/graphql.rs @@ -33,8 +33,8 @@ pub struct Error { #[buildstructor::buildstructor] impl Error { - #[builder] - pub fn new( + #[builder(visibility = "pub")] + fn new( message: String, locations: Vec, path: Option, diff --git a/apollo-router/src/plugin/test/mod.rs b/apollo-router/src/plugin/test/mod.rs index 511aeb5369..6108ace62b 100644 --- a/apollo-router/src/plugin/test/mod.rs +++ b/apollo-router/src/plugin/test/mod.rs @@ -89,8 +89,8 @@ impl PluginTestHarness { /// /// returns: Result> /// - #[builder] - pub async fn new( + #[builder(visibility = "pub")] + async fn new( plugin: P, schema: IntoSchema, mock_router_service: Option, diff --git a/apollo-router/src/request.rs b/apollo-router/src/request.rs index 3cf3f84e0a..df1adc0b21 100644 --- a/apollo-router/src/request.rs +++ b/apollo-router/src/request.rs @@ -45,8 +45,8 @@ where #[buildstructor::buildstructor] impl Request { - #[builder] - pub fn new( + #[builder(visibility = "pub")] + fn new( query: Option, operation_name: Option, variables: Option, @@ -60,8 +60,8 @@ impl Request { } } - #[builder] - pub fn fake_new( + #[builder(visibility = "pub")] + fn fake_new( query: Option, operation_name: Option, variables: Option, diff --git a/apollo-router/src/response.rs b/apollo-router/src/response.rs index 8aa22737e9..47ebd46f41 100644 --- a/apollo-router/src/response.rs +++ b/apollo-router/src/response.rs @@ -45,8 +45,8 @@ pub struct Response { #[buildstructor::buildstructor] impl Response { /// Constructor - #[builder] - pub fn new( + #[builder(visibility = "pub")] + fn new( label: Option, data: Option, path: Option, diff --git a/apollo-router/src/router.rs b/apollo-router/src/router.rs index 7808e4929f..e5ba27637c 100644 --- a/apollo-router/src/router.rs +++ b/apollo-router/src/router.rs @@ -372,8 +372,8 @@ pub struct ApolloRouter { #[buildstructor::buildstructor] impl ApolloRouter { /// Build a new Apollo router. - #[builder] - pub fn new( + #[builder(visibility = "pub")] + fn new( configuration: ConfigurationKind, schema: SchemaKind, shutdown: Option, diff --git a/apollo-router/src/services/http_ext.rs b/apollo-router/src/services/http_ext.rs index c9717736f5..afe4a6b626 100644 --- a/apollo-router/src/services/http_ext.rs +++ b/apollo-router/src/services/http_ext.rs @@ -89,8 +89,8 @@ impl Request { /// This is the constructor (or builder) to use when constructing a real Request. /// /// Required parameters are required in non-testing code to create a Request. - #[builder] - pub fn new( + #[builder(visibility = "pub")] + fn new( headers: MultiMap, uri: http::Uri, method: http::Method, @@ -116,8 +116,8 @@ impl Request { /// difficult to construct and not required for the purposes of the test. /// /// In addition, fake requests are expected to be valid, and will panic if given invalid values. - #[builder] - pub fn fake_new( + #[builder(visibility = "pub")] + fn fake_new( headers: MultiMap, uri: Option, method: Option, diff --git a/apollo-router/src/services/mod.rs b/apollo-router/src/services/mod.rs index 9a9a07723a..661b06f055 100644 --- a/apollo-router/src/services/mod.rs +++ b/apollo-router/src/services/mod.rs @@ -68,8 +68,8 @@ impl RouterRequest { /// /// Required parameters are required in non-testing code to create a RouterRequest. #[allow(clippy::too_many_arguments)] - #[builder] - pub fn new( + #[builder(visibility = "pub")] + fn new( query: Option, operation_name: Option, variables: HashMap, @@ -116,8 +116,8 @@ impl RouterRequest { /// difficult to construct and not required for the purposes of the test. /// /// In addition, fake requests are expected to be valid, and will panic if given invalid values. - #[builder] - pub fn fake_new( + #[builder(visibility = "pub")] + fn fake_new( query: Option, operation_name: Option, variables: HashMap, @@ -153,8 +153,8 @@ impl RouterResponse { /// /// Required parameters are required in non-testing code to create a RouterResponse.. #[allow(clippy::too_many_arguments)] - #[builder] - pub fn new( + #[builder(visibility = "pub")] + fn new( data: Option, path: Option, errors: Vec, @@ -208,8 +208,8 @@ impl RouterResponse { /// /// In addition, fake responses are expected to be valid, and will panic if given invalid values. #[allow(clippy::too_many_arguments)] - #[builder] - pub fn fake_new( + #[builder(visibility = "pub")] + fn fake_new( data: Option, path: Option, errors: Vec, @@ -232,8 +232,8 @@ impl RouterResponse { /// This is the constructor (or builder) to use when constructing a RouterResponse that represents a global error. /// It has no path and no response data. /// This is useful for things such as authentication errors. - #[builder] - pub fn error_new( + #[builder(visibility = "pub")] + fn error_new( errors: Vec, status_code: Option, headers: MultiMap, @@ -298,8 +298,8 @@ impl QueryPlannerRequest { /// This is the constructor (or builder) to use when constructing a real QueryPlannerRequest. /// /// Required parameters are required in non-testing code to create a QueryPlannerRequest. - #[builder] - pub fn new( + #[builder(visibility = "pub")] + pub(crate) fn new( query: String, operation_name: Option, context: Context, @@ -347,8 +347,8 @@ impl QueryPlannerResponse { /// It has no path and no response data. /// This is useful for things such as authentication errors. #[allow(unused_variables)] - #[builder] - pub fn error_new( + #[builder(visibility = "pub")] + fn error_new( errors: Vec, status_code: Option, headers: MultiMap, @@ -388,8 +388,8 @@ impl SubgraphRequest { /// This is the constructor (or builder) to use when constructing a real SubgraphRequest. /// /// Required parameters are required in non-testing code to create a SubgraphRequest. - #[builder] - pub fn new( + #[builder(visibility = "pub")] + fn new( originating_request: Arc>, subgraph_request: http_ext::Request, operation_kind: OperationKind, @@ -408,8 +408,8 @@ impl SubgraphRequest { /// This does not enforce the provision of the data that is required for a fully functional /// SubgraphRequest. It's usually enough for testing, when a fully consructed SubgraphRequest is /// difficult to construct and not required for the pusposes of the test. - #[builder] - pub fn fake_new( + #[builder(visibility = "pub")] + fn fake_new( originating_request: Option>>, subgraph_request: Option>, operation_kind: Option, @@ -466,8 +466,8 @@ impl SubgraphResponse { /// /// The parameters are not optional, because in a live situation all of these properties must be /// set and be correct to create a SubgraphResponse. - #[builder] - pub fn new( + #[builder(visibility = "pub")] + fn new( label: Option, data: Option, path: Option, @@ -507,8 +507,8 @@ impl SubgraphResponse { /// This does not enforce the provision of the data that is required for a fully functional /// SubgraphResponse. It's usually enough for testing, when a fully consructed SubgraphResponse is /// difficult to construct and not required for the pusposes of the test. - #[builder] - pub fn fake_new( + #[builder(visibility = "pub")] + fn fake_new( label: Option, data: Option, path: Option, @@ -531,8 +531,8 @@ impl SubgraphResponse { /// This is the constructor (or builder) to use when constructing a SubgraphResponse that represents a global error. /// It has no path and no response data. /// This is useful for things such as authentication errors. - #[builder] - pub fn error_new( + #[builder(visibility = "pub")] + fn error_new( errors: Vec, status_code: Option, context: Context, @@ -566,8 +566,8 @@ impl ExecutionRequest { /// /// The parameters are not optional, because in a live situation all of these properties must be /// set and be correct to create a ExecutionRequest. - #[builder] - pub fn new( + #[builder(visibility = "pub")] + fn new( originating_request: http_ext::Request, query_plan: Arc, context: Context, @@ -584,8 +584,8 @@ impl ExecutionRequest { /// This does not enforce the provision of the data that is required for a fully functional /// ExecutionRequest. It's usually enough for testing, when a fully consructed ExecutionRequest is /// difficult to construct and not required for the pusposes of the test. - #[builder] - pub fn fake_new( + #[builder(visibility = "pub")] + fn fake_new( originating_request: Option>, query_plan: Option, context: Option, @@ -620,8 +620,8 @@ impl ExecutionResponse { /// /// The parameters are not optional, because in a live situation all of these properties must be /// set and be correct to create a RouterRequest. - #[builder] - pub fn new( + #[builder(visibility = "pub")] + fn new( label: Option, data: Option, path: Option, @@ -661,8 +661,8 @@ impl ExecutionResponse { /// This does not enforce the provision of the data that is required for a fully functional /// ExecutionResponse. It's usually enough for testing, when a fully consructed /// ExecutionResponse is difficult to construct and not required for the pusposes of the test. - #[builder] - pub fn fake_new( + #[builder(visibility = "pub")] + fn fake_new( label: Option, data: Option, path: Option, @@ -686,8 +686,8 @@ impl ExecutionResponse { /// It has no path and no response data. /// This is useful for things such as authentication errors. #[allow(unused_variables)] - #[builder] - pub fn error_new( + #[builder(visibility = "pub")] + fn error_new( errors: Vec, status_code: Option, headers: MultiMap, diff --git a/apollo-router/src/services/router_service.rs b/apollo-router/src/services/router_service.rs index 9d3286a01b..22242cda30 100644 --- a/apollo-router/src/services/router_service.rs +++ b/apollo-router/src/services/router_service.rs @@ -64,8 +64,8 @@ pub struct RouterService { #[buildstructor::buildstructor] impl RouterService { - #[builder] - pub fn new( + #[builder(visibility = "pub")] + fn new( query_planner_service: QueryPlannerService, execution_service_factory: ExecutionFactory, schema: Arc, From 4ee35e78f6240e6f5a61ce76e02e0c64e7d0358c Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 14:20:10 +0200 Subject: [PATCH 03/28] Rename enums *Kind -> *Source --- NEXT_CHANGELOG.md | 10 ++++ apollo-router/src/executable.rs | 22 +++---- apollo-router/src/router.rs | 102 ++++++++++++++++---------------- 3 files changed, 73 insertions(+), 61 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 9e5686fb4b..a5203023a9 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -83,6 +83,16 @@ using an `expect_clone` call with mockall. By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/1440 +### Some items were renamed ([PR #FIXME]) + +* `SchemaKind` β†’ `SchemaSource` +* `SchemaKind::String(String)` β†’ `SchemaSource::Static { schema_sdl: String }` +* `ConfigurationKind` β†’ `ConfigurationSource` +* `ConfigurationKind::Instance` β†’ `ConfigurationSource::Static` +* `ShutdownKind` β†’ `ShutdownSource` + +By [@SimonSapin](https://github.com/SimonSapin) + ### Removed constructors when there is a public builder ([PR #FIXME]) Many types in the Router API can be constructed with the builder pattern. diff --git a/apollo-router/src/executable.rs b/apollo-router/src/executable.rs index b4605e8063..dee5c1fbb8 100644 --- a/apollo-router/src/executable.rs +++ b/apollo-router/src/executable.rs @@ -26,9 +26,9 @@ use crate::configuration::generate_config_schema; use crate::configuration::Configuration; use crate::configuration::ConfigurationError; use crate::router::ApolloRouter; -use crate::router::ConfigurationKind; -use crate::router::SchemaKind; -use crate::router::ShutdownKind; +use crate::router::ConfigurationSource; +use crate::router::SchemaSource; +use crate::router::ShutdownSource; pub(crate) static GLOBAL_ENV_FILTER: OnceCell = OnceCell::new(); @@ -159,7 +159,7 @@ impl Executable { /// You may optionally supply a `router_builder_fn` to override building of the router. /// /// ```no_run - /// use apollo_router::{ApolloRouter, Executable, ShutdownKind}; + /// use apollo_router::{ApolloRouter, Executable, ShutdownSource}; /// # use anyhow::Result; /// # #[tokio::main] /// # async fn main()->Result<()> { @@ -167,7 +167,7 @@ impl Executable { /// .router_builder_fn(|configuration, schema| ApolloRouter::builder() /// .configuration(configuration) /// .schema(schema) - /// .shutdown(ShutdownKind::CtrlC) + /// .shutdown(ShutdownSource::CtrlC) /// .build()) /// .start().await /// # } @@ -176,7 +176,7 @@ impl Executable { /// #[builder(entry = "builder", exit = "start", visibility = "pub")] async fn start( - router_builder_fn: Option ApolloRouter>, + router_builder_fn: Option ApolloRouter>, ) -> Result<()> { let opt = Opt::parse(); @@ -216,7 +216,7 @@ impl Executable { } async fn inner_start( - router_builder_fn: Option ApolloRouter>, + router_builder_fn: Option ApolloRouter>, opt: Opt, dispatcher: Dispatch, ) -> Result<()> { @@ -232,7 +232,7 @@ impl Executable { path.to_path_buf() }; - ConfigurationKind::File { + ConfigurationSource::File { path, watch: opt.hot_reload, delay: None, @@ -251,7 +251,7 @@ impl Executable { } else { supergraph_path }; - SchemaKind::File { + SchemaSource::File { path: supergraph_path, watch: opt.hot_reload, delay: None, @@ -277,7 +277,7 @@ impl Executable { error: err.to_string(), })?; - SchemaKind::Registry { + SchemaSource::Registry { apollo_key, apollo_graph_ref, urls: uplink_endpoints, @@ -323,7 +323,7 @@ impl Executable { ApolloRouter::builder() .configuration(configuration) .schema(schema) - .shutdown(ShutdownKind::CtrlC) + .shutdown(ShutdownSource::CtrlC) .build() })(configuration, schema); if let Err(err) = router.serve().await { diff --git a/apollo-router/src/router.rs b/apollo-router/src/router.rs index e5ba27637c..bd2f3fded6 100644 --- a/apollo-router/src/router.rs +++ b/apollo-router/src/router.rs @@ -81,10 +81,10 @@ pub enum ApolloRouterError { /// The user supplied schema. Either a static string or a stream for hot reloading. #[derive(From, Display, Derivative)] #[derivative(Debug)] -pub enum SchemaKind { +pub enum SchemaSource { /// A static schema. #[display(fmt = "String")] - String(String), + Static { schema_sdl: String }, /// A stream of schema. #[display(fmt = "Stream")] @@ -120,19 +120,23 @@ pub enum SchemaKind { }, } -impl From<&'_ str> for SchemaKind { +impl From<&'_ str> for SchemaSource { fn from(s: &'_ str) -> Self { - Self::String(s.to_owned()) + Self::Static { + schema_sdl: s.to_owned(), + } } } -impl SchemaKind { +impl SchemaSource { /// Convert this schema into a stream regardless of if is static or not. Allows for unified handling later. fn into_stream(self) -> impl Stream { match self { - SchemaKind::String(schema) => stream::once(future::ready(UpdateSchema(schema))).boxed(), - SchemaKind::Stream(stream) => stream.map(UpdateSchema).boxed(), - SchemaKind::File { path, watch, delay } => { + SchemaSource::Static { schema_sdl: schema } => { + stream::once(future::ready(UpdateSchema(schema))).boxed() + } + SchemaSource::Stream(stream) => stream.map(UpdateSchema).boxed(), + SchemaSource::File { path, watch, delay } => { // Sanity check, does the schema file exists, if it doesn't then bail. if !path.exists() { tracing::error!( @@ -162,7 +166,7 @@ impl SchemaKind { } } } - SchemaKind::Registry { + SchemaSource::Registry { apollo_key, apollo_graph_ref, urls, @@ -193,11 +197,11 @@ type ConfigurationStream = Pin + Send>>; /// The user supplied config. Either a static instance or a stream for hot reloading. #[derive(From, Display, Derivative)] #[derivative(Debug)] -pub enum ConfigurationKind { +pub enum ConfigurationSource { /// A static configuration. #[display(fmt = "Instance")] #[from(types(Configuration))] - Instance(Box), + Static(Box), /// A configuration stream where the server will react to new configuration. If possible /// the configuration will be applied without restarting the internal http server. @@ -218,17 +222,17 @@ pub enum ConfigurationKind { }, } -impl ConfigurationKind { +impl ConfigurationSource { /// Convert this config into a stream regardless of if is static or not. Allows for unified handling later. fn into_stream(self) -> impl Stream { match self { - ConfigurationKind::Instance(instance) => { + ConfigurationSource::Static(instance) => { stream::iter(vec![UpdateConfiguration(instance)]).boxed() } - ConfigurationKind::Stream(stream) => { + ConfigurationSource::Stream(stream) => { stream.map(|x| UpdateConfiguration(Box::new(x))).boxed() } - ConfigurationKind::File { path, watch, delay } => { + ConfigurationSource::File { path, watch, delay } => { // Sanity check, does the config file exists, if it doesn't then bail. if !path.exists() { tracing::error!( @@ -237,18 +241,20 @@ impl ConfigurationKind { ); stream::empty().boxed() } else { - match ConfigurationKind::read_config(&path) { + match ConfigurationSource::read_config(&path) { Ok(configuration) => { if watch { crate::files::watch(path.to_owned(), delay) .filter_map(move |_| { - future::ready(match ConfigurationKind::read_config(&path) { - Ok(config) => Some(config), - Err(err) => { - tracing::error!("{}", err); - None - } - }) + future::ready( + match ConfigurationSource::read_config(&path) { + Ok(config) => Some(config), + Err(err) => { + tracing::error!("{}", err); + None + } + }, + ) }) .map(|x| UpdateConfiguration(Box::new(x))) .boxed() @@ -281,10 +287,10 @@ impl ConfigurationKind { type ShutdownFuture = Pin + Send>>; -/// The user supplied shutdown hook. +/// Specifies when the Router’s HTTP server should gracefully shutdown #[derive(Display, Derivative)] #[derivative(Debug)] -pub enum ShutdownKind { +pub enum ShutdownSource { /// No graceful shutdown #[display(fmt = "None")] None, @@ -298,13 +304,13 @@ pub enum ShutdownKind { CtrlC, } -impl ShutdownKind { +impl ShutdownSource { /// Convert this shutdown hook into a future. Allows for unified handling later. fn into_stream(self) -> impl Stream { match self { - ShutdownKind::None => stream::pending::().boxed(), - ShutdownKind::Custom(future) => future.map(|_| Shutdown).into_stream().boxed(), - ShutdownKind::CtrlC => async { + ShutdownSource::None => stream::pending::().boxed(), + ShutdownSource::Custom(future) => future.map(|_| Shutdown).into_stream().boxed(), + ShutdownSource::CtrlC => async { tokio::signal::ctrl_c() .await .expect("Failed to install CTRL+C signal handler"); @@ -323,7 +329,6 @@ impl ShutdownKind { /// ``` /// use apollo_router::ApolloRouter; /// use apollo_router::Configuration; -/// use apollo_router::ShutdownKind; /// /// async { /// let configuration = serde_yaml::from_str::("Config").unwrap(); @@ -331,7 +336,6 @@ impl ShutdownKind { /// let server = ApolloRouter::builder() /// .configuration(configuration) /// .schema(schema) -/// .shutdown(ShutdownKind::CtrlC) /// .build(); /// server.serve().await; /// }; @@ -341,7 +345,6 @@ impl ShutdownKind { /// ``` /// use apollo_router::ApolloRouter; /// use apollo_router::Configuration; -/// use apollo_router::ShutdownKind; /// /// async { /// let configuration = serde_yaml::from_str::("Config").unwrap(); @@ -349,7 +352,6 @@ impl ShutdownKind { /// let server = ApolloRouter::builder() /// .configuration(configuration) /// .schema(schema) -/// .shutdown(ShutdownKind::CtrlC) /// .build(); /// let handle = server.serve(); /// drop(handle); @@ -358,13 +360,13 @@ impl ShutdownKind { /// pub struct ApolloRouter { /// The Configuration that the server will use. This can be static or a stream for hot reloading. - pub(crate) configuration: ConfigurationKind, + pub(crate) configuration: ConfigurationSource, /// The Schema that the server will use. This can be static or a stream for hot reloading. - pub(crate) schema: SchemaKind, + pub(crate) schema: SchemaSource, /// A future that when resolved will shut down the server. - pub(crate) shutdown: ShutdownKind, + pub(crate) shutdown: ShutdownSource, pub(crate) router_factory: YamlRouterServiceFactory, } @@ -374,14 +376,14 @@ impl ApolloRouter { /// Build a new Apollo router. #[builder(visibility = "pub")] fn new( - configuration: ConfigurationKind, - schema: SchemaKind, - shutdown: Option, + configuration: ConfigurationSource, + schema: SchemaSource, + shutdown: Option, ) -> ApolloRouter { ApolloRouter { configuration, schema, - shutdown: shutdown.unwrap_or(ShutdownKind::CtrlC), + shutdown: shutdown.unwrap_or(ShutdownSource::CtrlC), router_factory: YamlRouterServiceFactory::default(), } } @@ -487,9 +489,9 @@ impl ApolloRouter { /// This merges all contributing streams and sets up shutdown handling. /// When a shutdown message is received no more events are emitted. fn generate_event_stream( - shutdown: ShutdownKind, - configuration: ConfigurationKind, - schema: SchemaKind, + shutdown: ShutdownSource, + configuration: ConfigurationSource, + schema: SchemaSource, shutdown_receiver: oneshot::Receiver<()>, ) -> impl Stream { // Chain is required so that the final shutdown message is sent. @@ -570,7 +572,7 @@ mod tests { let (path, mut file) = create_temp_file(); let contents = include_str!("testdata/supergraph_config.yaml"); write_and_flush(&mut file, contents).await; - let mut stream = ConfigurationKind::File { + let mut stream = ConfigurationSource::File { path, watch: true, delay: Some(Duration::from_millis(10)), @@ -600,7 +602,7 @@ mod tests { async fn config_by_file_invalid() { let (path, mut file) = create_temp_file(); write_and_flush(&mut file, "Garbage").await; - let mut stream = ConfigurationKind::File { + let mut stream = ConfigurationSource::File { path, watch: true, delay: None, @@ -613,7 +615,7 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn config_by_file_missing() { - let mut stream = ConfigurationKind::File { + let mut stream = ConfigurationSource::File { path: temp_dir().join("does_not_exit"), watch: true, delay: None, @@ -630,7 +632,7 @@ mod tests { let contents = include_str!("testdata/supergraph_config.yaml"); write_and_flush(&mut file, contents).await; - let mut stream = ConfigurationKind::File { + let mut stream = ConfigurationSource::File { path, watch: false, delay: None, @@ -648,7 +650,7 @@ mod tests { let (path, mut file) = create_temp_file(); let schema = include_str!("testdata/supergraph.graphql"); write_and_flush(&mut file, schema).await; - let mut stream = SchemaKind::File { + let mut stream = SchemaSource::File { path, watch: true, delay: Some(Duration::from_millis(10)), @@ -666,7 +668,7 @@ mod tests { #[test(tokio::test)] async fn schema_by_file_missing() { - let mut stream = SchemaKind::File { + let mut stream = SchemaSource::File { path: temp_dir().join("does_not_exit"), watch: true, delay: None, @@ -683,7 +685,7 @@ mod tests { let schema = include_str!("testdata/supergraph.graphql"); write_and_flush(&mut file, schema).await; - let mut stream = SchemaKind::File { + let mut stream = SchemaSource::File { path, watch: false, delay: None, From 194ec7c8ea754e438792cdb569e9a62b8a0b3d19 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 14:58:19 +0200 Subject: [PATCH 04/28] Rename ApolloRouter to RouterHttpServer --- NEXT_CHANGELOG.md | 1 + apollo-router/src/executable.rs | 12 ++++++------ apollo-router/src/router.rs | 20 ++++++++++---------- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index a5203023a9..9b0b00eacb 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -90,6 +90,7 @@ By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/p * `ConfigurationKind` β†’ `ConfigurationSource` * `ConfigurationKind::Instance` β†’ `ConfigurationSource::Static` * `ShutdownKind` β†’ `ShutdownSource` +* `ApolloRouter` β†’ `RouterHttpServer` By [@SimonSapin](https://github.com/SimonSapin) diff --git a/apollo-router/src/executable.rs b/apollo-router/src/executable.rs index dee5c1fbb8..c16ccc4c63 100644 --- a/apollo-router/src/executable.rs +++ b/apollo-router/src/executable.rs @@ -25,8 +25,8 @@ use url::Url; use crate::configuration::generate_config_schema; use crate::configuration::Configuration; use crate::configuration::ConfigurationError; -use crate::router::ApolloRouter; use crate::router::ConfigurationSource; +use crate::router::RouterHttpServer; use crate::router::SchemaSource; use crate::router::ShutdownSource; @@ -159,12 +159,12 @@ impl Executable { /// You may optionally supply a `router_builder_fn` to override building of the router. /// /// ```no_run - /// use apollo_router::{ApolloRouter, Executable, ShutdownSource}; + /// use apollo_router::{RouterHttpServer, Executable, ShutdownSource}; /// # use anyhow::Result; /// # #[tokio::main] /// # async fn main()->Result<()> { /// Executable::builder() - /// .router_builder_fn(|configuration, schema| ApolloRouter::builder() + /// .router_builder_fn(|configuration, schema| RouterHttpServer::builder() /// .configuration(configuration) /// .schema(schema) /// .shutdown(ShutdownSource::CtrlC) @@ -176,7 +176,7 @@ impl Executable { /// #[builder(entry = "builder", exit = "start", visibility = "pub")] async fn start( - router_builder_fn: Option ApolloRouter>, + router_builder_fn: Option RouterHttpServer>, ) -> Result<()> { let opt = Opt::parse(); @@ -216,7 +216,7 @@ impl Executable { } async fn inner_start( - router_builder_fn: Option ApolloRouter>, + router_builder_fn: Option RouterHttpServer>, opt: Opt, dispatcher: Dispatch, ) -> Result<()> { @@ -320,7 +320,7 @@ impl Executable { }; let router = router_builder_fn.unwrap_or(|configuration, schema| { - ApolloRouter::builder() + RouterHttpServer::builder() .configuration(configuration) .schema(schema) .shutdown(ShutdownSource::CtrlC) diff --git a/apollo-router/src/router.rs b/apollo-router/src/router.rs index bd2f3fded6..122d09ad2b 100644 --- a/apollo-router/src/router.rs +++ b/apollo-router/src/router.rs @@ -327,13 +327,13 @@ impl ShutdownSource { /// # Examples /// /// ``` -/// use apollo_router::ApolloRouter; +/// use apollo_router::RouterHttpServer; /// use apollo_router::Configuration; /// /// async { /// let configuration = serde_yaml::from_str::("Config").unwrap(); /// let schema = "schema"; -/// let server = ApolloRouter::builder() +/// let server = RouterHttpServer::builder() /// .configuration(configuration) /// .schema(schema) /// .build(); @@ -343,13 +343,13 @@ impl ShutdownSource { /// /// Shutdown via handle. /// ``` -/// use apollo_router::ApolloRouter; +/// use apollo_router::RouterHttpServer; /// use apollo_router::Configuration; /// /// async { /// let configuration = serde_yaml::from_str::("Config").unwrap(); /// let schema = "schema"; -/// let server = ApolloRouter::builder() +/// let mut server = RouterHttpServer::builder() /// .configuration(configuration) /// .schema(schema) /// .build(); @@ -358,7 +358,7 @@ impl ShutdownSource { /// }; /// ``` /// -pub struct ApolloRouter { +pub struct RouterHttpServer { /// The Configuration that the server will use. This can be static or a stream for hot reloading. pub(crate) configuration: ConfigurationSource, @@ -372,15 +372,15 @@ pub struct ApolloRouter { } #[buildstructor::buildstructor] -impl ApolloRouter { +impl RouterHttpServer { /// Build a new Apollo router. #[builder(visibility = "pub")] fn new( configuration: ConfigurationSource, schema: SchemaSource, shutdown: Option, - ) -> ApolloRouter { - ApolloRouter { + ) -> RouterHttpServer { + RouterHttpServer { configuration, schema, shutdown: shutdown.unwrap_or(ShutdownSource::CtrlC), @@ -444,7 +444,7 @@ impl Future for RouterHandle { } } -impl ApolloRouter { +impl RouterHttpServer { /// Start the federated server on a separate thread. /// /// Dropping the server handle will shutdown the server. @@ -526,7 +526,7 @@ mod tests { serde_yaml::from_str::(include_str!("testdata/supergraph_config.yaml")) .unwrap(); let schema = include_str!("testdata/supergraph.graphql"); - ApolloRouter::builder() + RouterHttpServer::builder() .configuration(configuration) .schema(schema) .build() From 53dbad614712019aacb02381e0b395b098c999e9 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 15:34:35 +0200 Subject: [PATCH 05/28] Merge RouterHandle into RouterHttpServer --- NEXT_CHANGELOG.md | 19 ++ apollo-router/src/configuration/mod.rs | 4 +- ...nfiguration__tests__schema_generation.snap | 2 +- apollo-router/src/executable.rs | 6 +- apollo-router/src/router.rs | 227 +++++++++--------- 5 files changed, 142 insertions(+), 116 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 9b0b00eacb..cda7b90d5e 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -94,6 +94,25 @@ By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/p By [@SimonSapin](https://github.com/SimonSapin) +### Router startup API changes ([PR #FIXME]) + +The `RouterHttpServer::serve` method and its return type `RouterHandle` were removed, +their functionality merged into `RouterHttpServer` (formerly `ApolloRouter`). +The builder for `RouterHttpServer` now ends with a `start` method instead of `build`. +This method immediatly starts the server in a new Tokio task. + +```diff + RouterHttpServer::builder() + .configuration(configuration) + .schema(schema) +- .build() +- .serve() ++ .start() + .await +``` + +By [@SimonSapin](https://github.com/SimonSapin) + ### Removed constructors when there is a public builder ([PR #FIXME]) Many types in the Router API can be constructed with the builder pattern. diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index 0006b8a848..2e6481a137 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -74,7 +74,9 @@ pub enum ConfigurationError { } /// The configuration for the router. -/// Currently maintains a mapping of subgraphs. +/// +/// Can be created through `serde::Deserialize` from various formats, +/// or inline in Rust code with `serde_json::json!` and `serde_json::from_value`. #[derive(Clone, Derivative, Deserialize, Serialize, JsonSchema, Default)] #[derivative(Debug)] pub struct Configuration { diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap index 03c3e6300f..ac22afd039 100644 --- a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap @@ -6,7 +6,7 @@ expression: "&schema" { "$schema": "https://json-schema.org/draft/2019-09/schema", "title": "Configuration", - "description": "The configuration for the router. Currently maintains a mapping of subgraphs.", + "description": "The configuration for the router.\n\nCan be created through `serde::Deserialize` from various formats, or inline in Rust code with `serde_json::json!` and `serde_json::from_value`.", "type": "object", "properties": { "csrf": { diff --git a/apollo-router/src/executable.rs b/apollo-router/src/executable.rs index c16ccc4c63..b2c4cfca18 100644 --- a/apollo-router/src/executable.rs +++ b/apollo-router/src/executable.rs @@ -168,7 +168,7 @@ impl Executable { /// .configuration(configuration) /// .schema(schema) /// .shutdown(ShutdownSource::CtrlC) - /// .build()) + /// .start()) /// .start().await /// # } /// ``` @@ -324,9 +324,9 @@ impl Executable { .configuration(configuration) .schema(schema) .shutdown(ShutdownSource::CtrlC) - .build() + .start() })(configuration, schema); - if let Err(err) = router.serve().await { + if let Err(err) = router.await { tracing::error!("{}", err); return Err(err.into()); } diff --git a/apollo-router/src/router.rs b/apollo-router/src/router.rs index 122d09ad2b..a190f000ab 100644 --- a/apollo-router/src/router.rs +++ b/apollo-router/src/router.rs @@ -199,7 +199,10 @@ type ConfigurationStream = Pin + Send>>; #[derivative(Debug)] pub enum ConfigurationSource { /// A static configuration. - #[display(fmt = "Instance")] + /// + /// Can be created through `serde::Deserialize` from various formats, + /// or inline in Rust code with `serde_json::json!` and `serde_json::from_value`. + #[display(fmt = "Static")] #[from(types(Configuration))] Static(Box), @@ -322,7 +325,7 @@ impl ShutdownSource { } } -/// Federated server takes requests and federates a response based on calls to subgraphs. +/// The entry point for running the Router’s HTTP server. /// /// # Examples /// @@ -333,11 +336,11 @@ impl ShutdownSource { /// async { /// let configuration = serde_yaml::from_str::("Config").unwrap(); /// let schema = "schema"; -/// let server = RouterHttpServer::builder() +/// RouterHttpServer::builder() /// .configuration(configuration) /// .schema(schema) -/// .build(); -/// server.serve().await; +/// .start() +/// .await; /// }; /// ``` /// @@ -352,41 +355,104 @@ impl ShutdownSource { /// let mut server = RouterHttpServer::builder() /// .configuration(configuration) /// .schema(schema) -/// .build(); -/// let handle = server.serve(); -/// drop(handle); +/// .start(); +/// drop(server); /// }; /// ``` /// pub struct RouterHttpServer { - /// The Configuration that the server will use. This can be static or a stream for hot reloading. - pub(crate) configuration: ConfigurationSource, - - /// The Schema that the server will use. This can be static or a stream for hot reloading. - pub(crate) schema: SchemaSource, - - /// A future that when resolved will shut down the server. - pub(crate) shutdown: ShutdownSource, - - pub(crate) router_factory: YamlRouterServiceFactory, + result: Pin> + Send>>, + listen_address: Arc>>, + shutdown_sender: Option>, } #[buildstructor::buildstructor] impl RouterHttpServer { - /// Build a new Apollo router. - #[builder(visibility = "pub")] - fn new( - configuration: ConfigurationSource, + /// Returns a builder to start an HTTP server in a separate Tokio task. + /// + /// Builder methods: + /// + /// * `.schema(impl Into<`[`SchemaSource`]`>)` + /// Required. + /// Specifies where to find the supergraph schema definition. + /// Some sources support hot-reloading. + /// + /// * `.configuration(impl Into<`[`ConfigurationSource`]`>)` + /// Optional. + /// Specifies where to find the router configuration. + /// If not provided, the default configuration as with an empty YAML file. + /// + /// * `.shutdown(impl Into<`[`ShutdownSource`]`>)` + /// Optional. + /// Specifies when the server should gracefully shut down. + /// If not provided, the default is [`ShutdownSource::CtrlC`]. + /// + /// * `.start()` + /// Finishes the builder, + /// starts an HTTP server in a separate Tokio task, + /// and returns a `RouterHttpServer` handle. + /// + /// The server handle can be used in multiple ways. + /// As a [`Future`], it resolves to `Result<(), `[`ApolloRouterError`]`>` + /// either when the server has finished gracefully shutting down + /// or when it encounters a fatal error that prevents it from starting. + /// + /// If the handle is dropped before being awaited as a future, + /// a graceful shutdown is triggered. + /// However there is no way to wait until it finishes + /// since the server is running in a separate task. + #[builder(visibility = "pub", entry = "builder", exit = "start")] + fn start( schema: SchemaSource, + configuration: Option, shutdown: Option, ) -> RouterHttpServer { - RouterHttpServer { - configuration, + let (shutdown_sender, shutdown_receiver) = oneshot::channel::<()>(); + let event_stream = generate_event_stream( + shutdown.unwrap_or(ShutdownSource::CtrlC), + configuration.unwrap_or_else(|| ConfigurationSource::Static(Default::default())), schema, - shutdown: shutdown.unwrap_or(ShutdownSource::CtrlC), - router_factory: YamlRouterServiceFactory::default(), + shutdown_receiver, + ); + let server_factory = AxumHttpServerFactory::new(); + let router_factory = YamlRouterServiceFactory::default(); + let state_machine = StateMachine::new(server_factory, router_factory); + let listen_address = state_machine.listen_address.clone(); + let result = spawn( + async move { state_machine.process_events(event_stream).await } + .with_current_subscriber(), + ) + .map(|r| match r { + Ok(Ok(ok)) => Ok(ok), + Ok(Err(err)) => Err(err), + Err(err) => { + tracing::error!("{}", err); + Err(ApolloRouterError::StartupError) + } + }) + .with_current_subscriber() + .boxed(); + + RouterHttpServer { + result, + shutdown_sender: Some(shutdown_sender), + listen_address, } } + + /// Returns the listen address when the router is ready to receive requests. + /// + /// This can be useful when the `server.listen` configuration specifies TCP port 0, + /// which instructs the operating system to pick an available port number. + /// + /// Note: if configuration is dynamic, the listen address can change over time. + pub async fn listen_address(&self) -> Result { + self.listen_address + .read() + .await + .clone() + .ok_or(ApolloRouterError::StartupError) + } } /// Messages that are broadcast across the app. @@ -408,25 +474,7 @@ pub(crate) enum Event { Shutdown, } -/// A handle that allows the client to await for various server events. -pub struct RouterHandle { - result: Pin> + Send>>, - listen_address: Arc>>, - shutdown_sender: Option>, -} - -impl RouterHandle { - /// Returns the listen address when the router is ready to receive requests. - pub async fn listen_address(&self) -> Result { - self.listen_address - .read() - .await - .clone() - .ok_or(ApolloRouterError::StartupError) - } -} - -impl Drop for RouterHandle { +impl Drop for RouterHttpServer { fn drop(&mut self) { let _ = self .shutdown_sender @@ -436,7 +484,7 @@ impl Drop for RouterHandle { } } -impl Future for RouterHandle { +impl Future for RouterHttpServer { type Output = Result<(), ApolloRouterError>; fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { @@ -444,68 +492,26 @@ impl Future for RouterHandle { } } -impl RouterHttpServer { - /// Start the federated server on a separate thread. - /// - /// Dropping the server handle will shutdown the server. - /// - /// returns: RouterHandle - /// - pub fn serve(self) -> RouterHandle { - let server_factory = AxumHttpServerFactory::new(); - let (shutdown_sender, shutdown_receiver) = oneshot::channel::<()>(); - let event_stream = Self::generate_event_stream( - self.shutdown, - self.configuration, - self.schema, - shutdown_receiver, - ); - - let state_machine = StateMachine::new(server_factory, self.router_factory); - let listen_address = state_machine.listen_address.clone(); - let result = spawn( - async move { state_machine.process_events(event_stream).await } - .with_current_subscriber(), - ) - .map(|r| match r { - Ok(Ok(ok)) => Ok(ok), - Ok(Err(err)) => Err(err), - Err(err) => { - tracing::error!("{}", err); - Err(ApolloRouterError::StartupError) - } - }) - .with_current_subscriber() - .boxed(); - - RouterHandle { - result, - shutdown_sender: Some(shutdown_sender), - listen_address, - } - } - - /// Create the unified event stream. - /// This merges all contributing streams and sets up shutdown handling. - /// When a shutdown message is received no more events are emitted. - fn generate_event_stream( - shutdown: ShutdownSource, - configuration: ConfigurationSource, - schema: SchemaSource, - shutdown_receiver: oneshot::Receiver<()>, - ) -> impl Stream { - // Chain is required so that the final shutdown message is sent. - let messages = stream::select_all(vec![ - shutdown.into_stream().boxed(), - configuration.into_stream().boxed(), - schema.into_stream().boxed(), - shutdown_receiver.into_stream().map(|_| Shutdown).boxed(), - ]) - .take_while(|msg| future::ready(!matches!(msg, Shutdown))) - .chain(stream::iter(vec![Shutdown])) - .boxed(); - messages - } +/// Create the unified event stream. +/// This merges all contributing streams and sets up shutdown handling. +/// When a shutdown message is received no more events are emitted. +fn generate_event_stream( + shutdown: ShutdownSource, + configuration: ConfigurationSource, + schema: SchemaSource, + shutdown_receiver: oneshot::Receiver<()>, +) -> impl Stream { + // Chain is required so that the final shutdown message is sent. + let messages = stream::select_all(vec![ + shutdown.into_stream().boxed(), + configuration.into_stream().boxed(), + schema.into_stream().boxed(), + shutdown_receiver.into_stream().map(|_| Shutdown).boxed(), + ]) + .take_while(|msg| future::ready(!matches!(msg, Shutdown))) + .chain(stream::iter(vec![Shutdown])) + .boxed(); + messages } #[cfg(test)] @@ -521,7 +527,7 @@ mod tests { use crate::graphql; use crate::graphql::Request; - fn init_with_server() -> RouterHandle { + fn init_with_server() -> RouterHttpServer { let configuration = serde_yaml::from_str::(include_str!("testdata/supergraph_config.yaml")) .unwrap(); @@ -529,8 +535,7 @@ mod tests { RouterHttpServer::builder() .configuration(configuration) .schema(schema) - .build() - .serve() + .start() } #[tokio::test(flavor = "multi_thread")] From 4f10bc01a93ce4653d1bc51a53d4f175f0ed8075 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 17:17:23 +0200 Subject: [PATCH 06/28] Replace router_builder_fn() with shutdown() --- NEXT_CHANGELOG.md | 27 ++++++++++++++++++++++ apollo-router/src/executable.rs | 41 +++++++++++++++------------------ 2 files changed, 45 insertions(+), 23 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index cda7b90d5e..dee4f080de 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -113,6 +113,33 @@ This method immediatly starts the server in a new Tokio task. By [@SimonSapin](https://github.com/SimonSapin) +### `router_builder_fn` replaced by `shutdown` in the `Executable` builder ([PR #FIXME]) + +The builder for `apollo_router::Executable` had a `router_builder_fn` method +allowing to specify how a `RouterHttpServer` (previously `ApolloRouter`) was to be created +with a provided configuration and schema. +The only possible variation there was specifying when the server should shut down +with a `ShutdownSource` parameter, +so `router_builder_fn` was replaced with a new `shutdown` method that takes that. + +```diff + use apollo_router::Executable; +-use apollo_router::RouterHttpServer; + use apollo_router::ShutdownSource; + + Executable::builder() +- .router_builder_fn(|configuration, schema| RouterHttpServer::builder() +- .configuration(configuration) +- .schema(schema) +- .shutdown(ShutdownSource::None) +- .start()) ++ .shutdown(ShutdownSource::None) + .start() + .await +``` + +By [@SimonSapin](https://github.com/SimonSapin) + ### Removed constructors when there is a public builder ([PR #FIXME]) Many types in the Router API can be constructed with the builder pattern. diff --git a/apollo-router/src/executable.rs b/apollo-router/src/executable.rs index b2c4cfca18..e784b2e8ba 100644 --- a/apollo-router/src/executable.rs +++ b/apollo-router/src/executable.rs @@ -155,29 +155,26 @@ pub struct Executable {} #[buildstructor::buildstructor] impl Executable { - /// Build an executable that will parse commandline options and set up logging. - /// You may optionally supply a `router_builder_fn` to override building of the router. + /// Build an executable that will parse commandline options and set up logging, + /// then start an HTTP server. + /// + /// You may optionally specify when the server should gracefully shut down. + /// The default is on CTRL+C on the terminal (or a `SIGINT` signal). /// /// ```no_run - /// use apollo_router::{RouterHttpServer, Executable, ShutdownSource}; - /// # use anyhow::Result; + /// use apollo_router::{Executable, ShutdownSource}; /// # #[tokio::main] - /// # async fn main()->Result<()> { + /// # async fn main() -> anyhow::Result<()> { /// Executable::builder() - /// .router_builder_fn(|configuration, schema| RouterHttpServer::builder() - /// .configuration(configuration) - /// .schema(schema) - /// .shutdown(ShutdownSource::CtrlC) - /// .start()) - /// .start().await + /// .shutdown(ShutdownSource::None) + /// .start() + /// .await /// # } /// ``` /// Note that if you do not specify a runtime you must be in the context of an existing tokio runtime. /// #[builder(entry = "builder", exit = "start", visibility = "pub")] - async fn start( - router_builder_fn: Option RouterHttpServer>, - ) -> Result<()> { + async fn start(shutdown: Option) -> Result<()> { let opt = Opt::parse(); if opt.version { @@ -210,13 +207,13 @@ impl Executable { // The dispatcher we created is passed explicitely here to make sure we display the logs // in the initialization pahse and in the state machine code, before a global subscriber // is set using the configuration file - Self::inner_start(router_builder_fn, opt, dispatcher.clone()) + Self::inner_start(shutdown, opt, dispatcher.clone()) .with_subscriber(dispatcher) .await } async fn inner_start( - router_builder_fn: Option RouterHttpServer>, + shutdown: Option, opt: Opt, dispatcher: Dispatch, ) -> Result<()> { @@ -319,13 +316,11 @@ impl Executable { } }; - let router = router_builder_fn.unwrap_or(|configuration, schema| { - RouterHttpServer::builder() - .configuration(configuration) - .schema(schema) - .shutdown(ShutdownSource::CtrlC) - .start() - })(configuration, schema); + let router = RouterHttpServer::builder() + .configuration(configuration) + .schema(schema) + .shutdown(shutdown.unwrap_or(ShutdownSource::CtrlC)) + .start(); if let Err(err) = router.await { tracing::error!("{}", err); return Err(err.into()); From 772da4fc6f0b12c0eb612e417a7adc42976d935a Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 17:34:35 +0200 Subject: [PATCH 07/28] Add RouterHttpServer::shutdown --- NEXT_CHANGELOG.md | 19 +++++++++++++++++++ apollo-router/src/router.rs | 27 +++++++++++++++++---------- 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index dee4f080de..782a89f6e7 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -189,6 +189,25 @@ traffic_shaping: By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/1347 +### Explicit `shutdown` for `RouterHttpServer` handle ([PR #FIXME]) + +If you explicitly create a `RouterHttpServer` handle, +dropping it while the server is running instructs the server shut down gracefuly. +However with the handle dropped, there is no way to wait for shutdown to end +or check that it went without error. +Instead, the new `shutdown` async method can be called explicitly +to obtain a `Result`: + +```diff + use RouterHttpServer; + let server = RouterHttpServer::builder().schema("schema").start(); + // … +-drop(server); ++server.shutdown().await.unwrap(); +``` + +By [@SimonSapin](https://github.com/SimonSapin) + ## πŸ› Fixes ## πŸ›  Maintenance diff --git a/apollo-router/src/router.rs b/apollo-router/src/router.rs index a190f000ab..afd1dd74f3 100644 --- a/apollo-router/src/router.rs +++ b/apollo-router/src/router.rs @@ -356,7 +356,8 @@ impl ShutdownSource { /// .configuration(configuration) /// .schema(schema) /// .start(); -/// drop(server); +/// // … +/// server.shutdown().await /// }; /// ``` /// @@ -399,8 +400,8 @@ impl RouterHttpServer { /// /// If the handle is dropped before being awaited as a future, /// a graceful shutdown is triggered. - /// However there is no way to wait until it finishes - /// since the server is running in a separate task. + /// In order to wait until shutdown finishes, + /// use the [`shutdown`][Self::shutdown] method instead. #[builder(visibility = "pub", entry = "builder", exit = "start")] fn start( schema: SchemaSource, @@ -453,6 +454,14 @@ impl RouterHttpServer { .clone() .ok_or(ApolloRouterError::StartupError) } + + /// Trigger and wait for graceful shutdown + pub async fn shutdown(&mut self) -> Result<(), ApolloRouterError> { + if let Some(sender) = self.shutdown_sender.take() { + let _ = sender.send(()); + } + (&mut self.result).await + } } /// Messages that are broadcast across the app. @@ -476,11 +485,9 @@ pub(crate) enum Event { impl Drop for RouterHttpServer { fn drop(&mut self) { - let _ = self - .shutdown_sender - .take() - .expect("shutdown sender must be present") - .send(()); + if let Some(sender) = self.shutdown_sender.take() { + let _ = sender.send(()); + } } } @@ -540,13 +547,13 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn basic_request() { - let router_handle = init_with_server(); + let mut router_handle = init_with_server(); let listen_address = router_handle .listen_address() .await .expect("router failed to start"); assert_federated_response(&listen_address, r#"{ topProducts { name } }"#).await; - drop(router_handle); + router_handle.shutdown().await.unwrap(); } async fn assert_federated_response(listen_addr: &ListenAddr, request: &str) { From fb445b1eee4343fe3564100184141ae312ed8923 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 19:46:31 +0200 Subject: [PATCH 08/28] Revert glob imports at the crate root --- apollo-router/src/error.rs | 1 + apollo-router/src/lib.rs | 11 ++++++++--- apollo-router/src/plugins/traffic_shaping/mod.rs | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/apollo-router/src/error.rs b/apollo-router/src/error.rs index 52bc63bd08..62607887e1 100644 --- a/apollo-router/src/error.rs +++ b/apollo-router/src/error.rs @@ -23,6 +23,7 @@ pub(crate) use crate::graphql::Error; use crate::graphql::Response; use crate::json_ext::Path; use crate::json_ext::Value; +pub use crate::router::ApolloRouterError; pub use crate::spec::SpecError; /// Error types for execution. diff --git a/apollo-router/src/lib.rs b/apollo-router/src/lib.rs index e619f077dc..b6484ce79b 100644 --- a/apollo-router/src/lib.rs +++ b/apollo-router/src/lib.rs @@ -70,10 +70,15 @@ pub mod services; mod spec; mod state_machine; -pub use configuration::*; +pub use configuration::Configuration; +pub use configuration::ListenAddr; pub use context::Context; -pub use executable::*; -pub use router::*; +pub use executable::main; +pub use executable::Executable; +pub use router::ConfigurationSource; +pub use router::RouterHttpServer; +pub use router::SchemaSource; +pub use router::ShutdownSource; #[doc(hidden)] pub use router_factory::__create_test_service_factory_from_yaml; pub use services::http_ext; diff --git a/apollo-router/src/plugins/traffic_shaping/mod.rs b/apollo-router/src/plugins/traffic_shaping/mod.rs index 4130061035..8dfe5708ac 100644 --- a/apollo-router/src/plugins/traffic_shaping/mod.rs +++ b/apollo-router/src/plugins/traffic_shaping/mod.rs @@ -32,6 +32,7 @@ use self::rate::RateLimitLayer; pub(crate) use self::rate::RateLimited; pub(crate) use self::timeout::Elapsed; use self::timeout::TimeoutLayer; +use crate::error::ConfigurationError; use crate::layers::ServiceBuilderExt; use crate::plugin::Plugin; use crate::plugin::PluginInit; @@ -40,7 +41,6 @@ use crate::register_plugin; use crate::services::subgraph_service::Compression; use crate::services::RouterRequest; use crate::services::RouterResponse; -use crate::ConfigurationError; use crate::QueryPlannerRequest; use crate::QueryPlannerResponse; use crate::SubgraphRequest; From 6e26603d3b101fe2e1c18656427097ac313b9315 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 19:52:49 +0200 Subject: [PATCH 09/28] Remove deprecated type aliases --- NEXT_CHANGELOG.md | 21 +++++++++++++++++++++ apollo-router/src/lib.rs | 15 ++++----------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 782a89f6e7..5bc2bea742 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -158,6 +158,27 @@ If you were using one of these constructors, the migration generally looks like + .build() ``` +### Removed deprecated type aliases ([PR #FIXME]) + +A few versions ago, some types were moved from the crate root to a new `graphql` module. +To help the transition, type aliases were left at the old location with a deprecation warning. +These aliases are now removed, remaining imports must be changed to the new location: + +```diff +-use apollo_router::Error; +-use apollo_router::Request; +-use apollo_router::Response; ++use apollo_router::graphql::Error; ++use apollo_router::graphql::Request; ++use apollo_router::graphql::Response; +``` + +Alternatively, import the module with `use apollo_router::graphql` +then use qualified paths such as `graphql::Request`. +This can help disambiguate when multiple types share a name. + +By [@SimonSapin](https://github.com/SimonSapin) + ## πŸš€ Features ### Expose query plan in extensions for GraphQL response (experimental) ([PR #1470](https://github.com/apollographql/router/pull/1470)) diff --git a/apollo-router/src/lib.rs b/apollo-router/src/lib.rs index b6484ce79b..bc01675aa0 100644 --- a/apollo-router/src/lib.rs +++ b/apollo-router/src/lib.rs @@ -84,17 +84,6 @@ pub use router_factory::__create_test_service_factory_from_yaml; pub use services::http_ext; pub use spec::Schema; -#[deprecated(note = "use apollo_router::graphql::Request instead")] -pub type Request = graphql::Request; -#[deprecated(note = "use apollo_router::graphql::Response instead")] -pub type Response = graphql::Response; -#[deprecated(note = "use apollo_router::graphql::Error instead")] -pub type Error = graphql::Error; - -// TODO: clean these up and import from relevant modules instead -pub(crate) use services::*; -pub(crate) use spec::*; - /// Reexports for macros #[doc(hidden)] pub mod _private { @@ -102,3 +91,7 @@ pub mod _private { pub use serde_json; pub use startup; } + +// TODO: clean these up and import from relevant modules instead +pub(crate) use services::*; +pub(crate) use spec::*; From 64e54bbe7456de335900ca2389990b087629789e Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 21:58:36 +0200 Subject: [PATCH 10/28] RouterRequest::fake_builder defaults to Content-Type: application/json --- NEXT_CHANGELOG.md | 17 ++++++++++ apollo-router/src/plugins/csrf.rs | 33 +++++++----------- apollo-router/src/services/http_ext.rs | 46 ++++++++++++++++++++++++-- apollo-router/src/services/mod.rs | 8 ++++- 4 files changed, 80 insertions(+), 24 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 5bc2bea742..bc2a55743e 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -179,6 +179,23 @@ This can help disambiguate when multiple types share a name. By [@SimonSapin](https://github.com/SimonSapin) +### `RouterRequest::fake_builder` defaults to `Content-Type: application/json` ([PR #FIXME]) + +`apollo_router::services::RouterRequest` has a builder for creating a β€œfake” request during tests. +When no `Content-Type` header is specified, this builder will now default to `application/json`. +This will help tests where a request goes through mandatory plugins including CSRF protection. +which makes the request be accepted by CSRF protection. + +If a test requires a request specifically *without* a `Content-Type` header, +this default can be removed from a `RouterRequest` after building it: + +```rust +let mut router_request = RouterRequesT::fake_builder().build(); +router_request.originating_request.headers_mut().remove("content-type"); +``` + +By [@SimonSapin](https://github.com/SimonSapin) + ## πŸš€ Features ### Expose query plan in extensions for GraphQL response (experimental) ([PR #1470](https://github.com/apollographql/router/pull/1470)) diff --git a/apollo-router/src/plugins/csrf.rs b/apollo-router/src/plugins/csrf.rs index a8a80746cd..4d90de9c12 100644 --- a/apollo-router/src/plugins/csrf.rs +++ b/apollo-router/src/plugins/csrf.rs @@ -240,21 +240,13 @@ mod csrf_tests { async fn it_lets_preflighted_request_pass_through() { let config = CSRFConfig::default(); let with_preflight_content_type = RouterRequest::fake_builder() - .headers( - [("content-type".into(), "application/json".into())] - .into_iter() - .collect(), - ) + .header("content-type", "application/json") .build() .unwrap(); assert_accepted(config.clone(), with_preflight_content_type).await; let with_preflight_header = RouterRequest::fake_builder() - .headers( - [("apollo-require-preflight".into(), "this-is-a-test".into())] - .into_iter() - .collect(), - ) + .header("apollo-require-preflight", "this-is-a-test") .build() .unwrap(); assert_accepted(config, with_preflight_header).await; @@ -263,7 +255,13 @@ mod csrf_tests { #[tokio::test] async fn it_rejects_non_preflighted_headers_request() { let config = CSRFConfig::default(); - let non_preflighted_request = RouterRequest::fake_builder().build().unwrap(); + let mut non_preflighted_request = RouterRequest::fake_builder().build().unwrap(); + // fake_builder defaults to `Content-Type: application/json`, + // specifically to avoid the case we’re testing here. + non_preflighted_request + .originating_request + .headers_mut() + .remove("content-type"); assert_rejected(config, non_preflighted_request).await } @@ -271,21 +269,13 @@ mod csrf_tests { async fn it_rejects_non_preflighted_content_type_request() { let config = CSRFConfig::default(); let non_preflighted_request = RouterRequest::fake_builder() - .headers( - [("content-type".into(), "text/plain".into())] - .into_iter() - .collect(), - ) + .header("content-type", "text/plain") .build() .unwrap(); assert_rejected(config.clone(), non_preflighted_request).await; let non_preflighted_request = RouterRequest::fake_builder() - .headers( - [("content-type".into(), "text/plain; charset=utf8".into())] - .into_iter() - .collect(), - ) + .header("content-type", "text/plain; charset=utf8") .build() .unwrap(); assert_rejected(config, non_preflighted_request).await; @@ -322,6 +312,7 @@ mod csrf_tests { .await .unwrap(); + assert_eq!(res.errors, []); assert_eq!(res.data.unwrap(), json!({ "test": 1234_u32 })); } diff --git a/apollo-router/src/services/http_ext.rs b/apollo-router/src/services/http_ext.rs index afe4a6b626..5589699fa0 100644 --- a/apollo-router/src/services/http_ext.rs +++ b/apollo-router/src/services/http_ext.rs @@ -23,7 +23,7 @@ use crate::graphql; /// Temporary holder of header name while for use while building requests and responses. Required /// because header name creation is fallible. -#[derive(Eq, Hash, PartialEq)] +#[derive(Eq)] pub enum IntoHeaderName { String(String), HeaderName(HeaderName), @@ -31,12 +31,54 @@ pub enum IntoHeaderName { /// Temporary holder of header value while for use while building requests and responses. Required /// because header value creation is fallible. -#[derive(Eq, Hash, PartialEq)] +#[derive(Eq)] pub enum IntoHeaderValue { String(String), HeaderValue(HeaderValue), } +impl PartialEq for IntoHeaderName { + fn eq(&self, other: &Self) -> bool { + self.as_bytes() == other.as_bytes() + } +} + +impl PartialEq for IntoHeaderValue { + fn eq(&self, other: &Self) -> bool { + self.as_bytes() == other.as_bytes() + } +} + +impl Hash for IntoHeaderName { + fn hash(&self, state: &mut H) { + self.as_bytes().hash(state); + } +} + +impl Hash for IntoHeaderValue { + fn hash(&self, state: &mut H) { + self.as_bytes().hash(state); + } +} + +impl IntoHeaderName { + fn as_bytes(&self) -> &[u8] { + match self { + IntoHeaderName::String(s) => s.as_bytes(), + IntoHeaderName::HeaderName(h) => h.as_str().as_bytes(), + } + } +} + +impl IntoHeaderValue { + fn as_bytes(&self) -> &[u8] { + match self { + IntoHeaderValue::String(s) => s.as_bytes(), + IntoHeaderValue::HeaderValue(v) => v.as_bytes(), + } + } +} + impl From for IntoHeaderName where T: std::fmt::Display, diff --git a/apollo-router/src/services/mod.rs b/apollo-router/src/services/mod.rs index 661b06f055..1149ac29df 100644 --- a/apollo-router/src/services/mod.rs +++ b/apollo-router/src/services/mod.rs @@ -123,8 +123,14 @@ impl RouterRequest { variables: HashMap, extensions: HashMap, context: Option, - headers: MultiMap, + mut headers: MultiMap, ) -> Result { + // Avoid testing requests getting blocked by the CSRF-prevention plugin + headers + .entry(IntoHeaderName::HeaderName(http::header::CONTENT_TYPE)) + .or_insert(IntoHeaderValue::HeaderValue(HeaderValue::from_static( + "application/json", + ))); RouterRequest::new( query, operation_name, From 472e315d4c7531131cac95d048fcf77d20996d5c Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 21:21:27 +0200 Subject: [PATCH 11/28] Add apollo_router::TestHarness --- NEXT_CHANGELOG.md | 10 ++ apollo-router/src/lib.rs | 2 + apollo-router/src/plugin/test/mock/mod.rs | 2 +- apollo-router/src/plugin/test/mod.rs | 1 + apollo-router/src/router.rs | 11 +- apollo-router/src/router_factory.rs | 11 +- apollo-router/src/services/router_service.rs | 2 +- apollo-router/src/state_machine.rs | 13 +- apollo-router/src/test_harness.rs | 177 +++++++++++++++++++ 9 files changed, 217 insertions(+), 12 deletions(-) create mode 100644 apollo-router/src/test_harness.rs diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index bc2a55743e..a84eb6b9ae 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -246,6 +246,16 @@ to obtain a `Result`: By [@SimonSapin](https://github.com/SimonSapin) +### Added `apollo_router::TestHarness` ([PR #FIXME]) + +This is a builder for the part of an Apollo Router that handles GraphQL requests, +as a `tower::Service`. +This allows tests, benchmarks, etc +to manipulate request and response objects in memory without going over the network. +See the API documentation for an example. (It can be built with `cargo doc --open`.) + +By [@SimonSapin](https://github.com/SimonSapin) + ## πŸ› Fixes ## πŸ›  Maintenance diff --git a/apollo-router/src/lib.rs b/apollo-router/src/lib.rs index bc01675aa0..799ce86951 100644 --- a/apollo-router/src/lib.rs +++ b/apollo-router/src/lib.rs @@ -69,6 +69,7 @@ mod router_factory; pub mod services; mod spec; mod state_machine; +mod test_harness; pub use configuration::Configuration; pub use configuration::ListenAddr; @@ -83,6 +84,7 @@ pub use router::ShutdownSource; pub use router_factory::__create_test_service_factory_from_yaml; pub use services::http_ext; pub use spec::Schema; +pub use test_harness::TestHarness; /// Reexports for macros #[doc(hidden)] diff --git a/apollo-router/src/plugin/test/mock/mod.rs b/apollo-router/src/plugin/test/mock/mod.rs index 2737e4ae0d..963d0df1a7 100644 --- a/apollo-router/src/plugin/test/mock/mod.rs +++ b/apollo-router/src/plugin/test/mock/mod.rs @@ -1,2 +1,2 @@ -pub(super) mod canned; +pub(crate) mod canned; pub(super) mod subgraph; diff --git a/apollo-router/src/plugin/test/mod.rs b/apollo-router/src/plugin/test/mod.rs index 6108ace62b..2f1332543b 100644 --- a/apollo-router/src/plugin/test/mod.rs +++ b/apollo-router/src/plugin/test/mod.rs @@ -19,6 +19,7 @@ use tower::Service; use tower::ServiceBuilder; use tower::ServiceExt; +pub(crate) use self::mock::canned; use super::DynPlugin; use crate::cache::DeduplicatingCache; use crate::introspection::Introspection; diff --git a/apollo-router/src/router.rs b/apollo-router/src/router.rs index afd1dd74f3..a5793eb276 100644 --- a/apollo-router/src/router.rs +++ b/apollo-router/src/router.rs @@ -17,6 +17,7 @@ use futures::FutureExt; use thiserror::Error; use tokio::sync::RwLock; use tokio::task::spawn; +use tower::BoxError; use tracing::subscriber::SetGlobalDefaultError; use tracing_futures::WithSubscriber; use url::Url; @@ -63,7 +64,7 @@ pub enum ApolloRouterError { ReadSchemaError(crate::error::SchemaError), /// could not create the HTTP pipeline: {0} - ServiceCreationError(tower::BoxError), + ServiceCreationError(BoxError), /// could not create the HTTP server: {0} ServerCreationError(std::io::Error), @@ -225,6 +226,12 @@ pub enum ConfigurationSource { }, } +impl Default for ConfigurationSource { + fn default() -> Self { + ConfigurationSource::Static(Default::default()) + } +} + impl ConfigurationSource { /// Convert this config into a stream regardless of if is static or not. Allows for unified handling later. fn into_stream(self) -> impl Stream { @@ -411,7 +418,7 @@ impl RouterHttpServer { let (shutdown_sender, shutdown_receiver) = oneshot::channel::<()>(); let event_stream = generate_event_stream( shutdown.unwrap_or(ShutdownSource::CtrlC), - configuration.unwrap_or_else(|| ConfigurationSource::Static(Default::default())), + configuration.unwrap_or_default(), schema, shutdown_receiver, ); diff --git a/apollo-router/src/router_factory.rs b/apollo-router/src/router_factory.rs index 92066e2e01..09c66be77f 100644 --- a/apollo-router/src/router_factory.rs +++ b/apollo-router/src/router_factory.rs @@ -52,6 +52,7 @@ pub(crate) trait RouterServiceConfigurator: Send + Sync + 'static { configuration: Arc, schema: Arc, previous_router: Option<&'a Self::RouterServiceFactory>, + extra_plugins: Option)>>, ) -> Result; } @@ -68,9 +69,13 @@ impl RouterServiceConfigurator for YamlRouterServiceFactory { configuration: Arc, schema: Arc, _previous_router: Option<&'a Self::RouterServiceFactory>, + extra_plugins: Option)>>, ) -> Result { // Process the plugins. - let plugins = create_plugins(&configuration, &schema).await?; + let mut plugins = create_plugins(&configuration, &schema).await?; + if let Some(extra) = extra_plugins { + plugins.extend(extra); + } let mut builder = PluggableRouterServiceBuilder::new(schema.clone()); builder = builder.with_configuration(configuration); @@ -108,7 +113,7 @@ pub async fn __create_test_service_factory_from_yaml(schema: &str, configuration let schema: Schema = Schema::parse(schema, &Default::default()).unwrap(); let service = YamlRouterServiceFactory::default() - .create(Arc::new(config), Arc::new(schema), None) + .create(Arc::new(config), Arc::new(schema), None, None) .await; assert_eq!( service.map(|_| ()).unwrap_err().to_string().as_str(), @@ -395,7 +400,7 @@ mod test { let schema = Schema::parse(schema, &config).unwrap(); let service = YamlRouterServiceFactory::default() - .create(Arc::new(config), Arc::new(schema), None) + .create(Arc::new(config), Arc::new(schema), None, None) .await; service.map(|_| ()) } diff --git a/apollo-router/src/services/router_service.rs b/apollo-router/src/services/router_service.rs index 22242cda30..cceabb3851 100644 --- a/apollo-router/src/services/router_service.rs +++ b/apollo-router/src/services/router_service.rs @@ -424,7 +424,7 @@ impl RouterServiceFactory for RouterCreator { } impl RouterCreator { - fn make( + pub(crate) fn make( &self, ) -> impl Service< RouterRequest, diff --git a/apollo-router/src/state_machine.rs b/apollo-router/src/state_machine.rs index e209ac07b5..2263284f85 100644 --- a/apollo-router/src/state_machine.rs +++ b/apollo-router/src/state_machine.rs @@ -300,7 +300,7 @@ where let router_factory = self .router_configurator - .create(configuration.clone(), schema.clone(), None) + .create(configuration.clone(), schema.clone(), None, None) .await .map_err(|err| { tracing::error!("cannot create the router: {}", err); @@ -355,6 +355,7 @@ where new_configuration.clone(), new_schema.clone(), Some(&router_service), + None, ) .await { @@ -434,6 +435,7 @@ mod tests { use crate::http_ext::Request; use crate::http_ext::Response; use crate::http_server_factory::Listener; + use crate::plugin::DynPlugin; use crate::plugin::Handler; use crate::router_factory::RouterServiceConfigurator; use crate::router_factory::RouterServiceFactory; @@ -574,7 +576,7 @@ mod tests { router_factory .expect_create() .times(1) - .returning(|_, _, _| Err(BoxError::from("Error"))); + .returning(|_, _, _, _| Err(BoxError::from("Error"))); let (server_factory, shutdown_receivers) = create_mock_server_factory(0); @@ -601,7 +603,7 @@ mod tests { .expect_create() .times(1) .in_sequence(&mut seq) - .returning(|_, _, _| { + .returning(|_, _, _, _| { let mut router = MockMyRouterFactory::new(); router.expect_clone().return_once(MockMyRouterFactory::new); router.expect_custom_endpoints().returning(HashMap::new); @@ -611,7 +613,7 @@ mod tests { .expect_create() .times(1) .in_sequence(&mut seq) - .returning(|_, _, _| Err(BoxError::from("error"))); + .returning(|_, _, _, _| Err(BoxError::from("error"))); let (server_factory, shutdown_receivers) = create_mock_server_factory(1); @@ -645,6 +647,7 @@ mod tests { configuration: Arc, schema: Arc, previous_router: Option<&'a MockMyRouterFactory>, + extra_plugins: Option)>>, ) -> Result; } } @@ -780,7 +783,7 @@ mod tests { router_factory .expect_create() .times(expect_times_called) - .returning(move |_, _, _| { + .returning(move |_, _, _, _| { let mut router = MockMyRouterFactory::new(); router.expect_clone().return_once(MockMyRouterFactory::new); router.expect_custom_endpoints().returning(HashMap::new); diff --git a/apollo-router/src/test_harness.rs b/apollo-router/src/test_harness.rs new file mode 100644 index 0000000000..284103bf71 --- /dev/null +++ b/apollo-router/src/test_harness.rs @@ -0,0 +1,177 @@ +use std::sync::Arc; + +use tower::util::BoxCloneService; +use tower::util::BoxService; +use tower::BoxError; +use tower::ServiceExt; + +use crate::configuration::Configuration; +use crate::plugin::test::canned; +use crate::plugin::DynPlugin; +use crate::plugin::Plugin; +use crate::plugin::PluginInit; +use crate::router_factory::RouterServiceConfigurator; +use crate::router_factory::YamlRouterServiceFactory; +use crate::services::RouterRequest; +use crate::services::RouterResponse; +use crate::services::SubgraphRequest; +use crate::services::SubgraphResponse; +use crate::Schema; + +/// Builder for the part of an Apollo Router that handles GraphQL requests, as a [`tower::Service`]. +/// +/// This allows tests, benchmarks, etc +/// to manipulate request and response objects in memory +/// without going over the network on the supergraph side. +/// +/// On the subgraph side, this test harness never makes network requests to subgraphs +/// unless [`with_subgraph_network_requests`][Self::with_subgraph_network_requests] is called. +/// +/// Compared to running a full [`RouterHttpServer`][crate::RouterHttpServer], +/// this test harness is lacking: +/// +/// * Custom endpoints from plugins +/// * The health check endpoint +/// * CORS (FIXME: should this include CORS?) +/// * HTTP compression +/// +/// Example making a single request: +/// +/// ``` +/// use apollo_router::services::RouterRequest; +/// use apollo_router::TestHarness; +/// use tower::util::ServiceExt; +/// +/// # #[tokio::main] async fn main() -> Result<(), tower::BoxError> { +/// let config = serde_json::json!({"server": {"introspection": false}}); +/// let request = RouterRequest::fake_builder() +/// // Request building here +/// .build() +/// .unwrap(); +/// let response = TestHarness::builder() +/// .configuration(serde_json::from_value(config)?) +/// .build() +/// .await? +/// .oneshot(request) +/// .await? +/// .next_response() +/// .await +/// .unwrap(); +/// # Ok(()) } +/// ``` +pub struct TestHarness<'a> { + schema: Option<&'a str>, + configuration: Option>, + extra_plugins: Vec<(String, Box)>, + subgraph_network_requests: bool, +} + +// Not using buildstructor because `extra_plugin` has non-trivial signature and behavior +impl<'a> TestHarness<'a> { + pub fn builder() -> Self { + Self { + schema: None, + configuration: None, + extra_plugins: Vec::new(), + subgraph_network_requests: false, + } + } + + /// Specifies the (static) supergraph schema definition. + /// + /// Panics if called more than once. + /// + /// If this isn’t called, a default β€œcanned” schema is used. + /// It can be found in the Router repository at `examples/graphql/local.graphql`. + /// In that case, subgraph responses are overridden with some β€œcanned” data. + pub fn schema(mut self, schema: &'a str) -> Self { + assert!(self.schema.is_none(), "schema was specified twice"); + self.schema = Some(schema); + self + } + + /// Specifies the (static) router configuration. + pub fn configuration(mut self, configuration: Arc) -> Self { + assert!( + self.configuration.is_none(), + "configuration was specified twice" + ); + self.configuration = Some(configuration); + self + } + + /// Adds an extra, already instanciated plugin. + /// + /// May be called multiple times. + /// These extra plugins are added after plugins specified in configuration. + pub fn extra_plugin(mut self, plugin: impl Plugin) -> Self { + fn type_name_of(_: &T) -> &'static str { + std::any::type_name::() + } + let name = format!( + "extra_plugins.{}.{}", + self.extra_plugins.len(), + type_name_of(&plugin) + ); + self.extra_plugins.push((name, Box::new(plugin))); + self + } + + /// Enables this test harness to make network requests to subgraphs. + /// + /// If this is not called, all subgraph requests get an empty response by default + /// (unless [`schema`][Self::schema] is also not called). + /// This behavior can be changed by implementing [`Plugin::subgraph_service`] + /// on a plugin given to [`extra_plugin`][Self::extra_plugin]. + pub fn with_subgraph_network_requests(mut self) -> Self { + self.subgraph_network_requests = true; + self + } + + /// Builds the GraphQL service + pub async fn build( + self, + ) -> Result, BoxError> { + let builder = if self.schema.is_none() { + self.extra_plugin(CannedSubgraphResponses) + } else { + self + }; + let config = builder.configuration.unwrap_or_default(); + let canned_schema = include_str!("../../examples/graphql/local.graphql"); + let schema = builder.schema.unwrap_or(canned_schema); + let schema = Arc::new(Schema::parse(schema, &config)?); + let router_creator = YamlRouterServiceFactory + .create(config, schema, None, Some(builder.extra_plugins)) + .await?; + Ok(tower::service_fn(move |request| { + let service = router_creator.make(); + async move { service.oneshot(request).await } + }) + .boxed_clone()) + } +} + +struct CannedSubgraphResponses; + +#[async_trait::async_trait] +impl Plugin for CannedSubgraphResponses { + type Config = (); + + async fn new(_: PluginInit) -> Result { + Ok(Self) + } + + fn subgraph_service( + &self, + subgraph_name: &str, + default: BoxService, + ) -> BoxService { + match subgraph_name { + "products" => canned::products_subgraph().boxed(), + "accounts" => canned::accounts_subgraph().boxed(), + "reviews" => canned::reviews_subgraph().boxed(), + _ => default, + } + } +} From 8b81aa59699e787f2954f0b6aeb5393a336170e3 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 22:45:31 +0200 Subject: [PATCH 12/28] Port rhai_tests to the new TestHarness --- apollo-router/tests/rhai_tests.rs | 68 ++++++++++--------------------- 1 file changed, 21 insertions(+), 47 deletions(-) diff --git a/apollo-router/tests/rhai_tests.rs b/apollo-router/tests/rhai_tests.rs index 4e2c65ca9f..59ad9fd57c 100644 --- a/apollo-router/tests/rhai_tests.rs +++ b/apollo-router/tests/rhai_tests.rs @@ -1,14 +1,5 @@ -use std::str::FromStr; -use std::sync::Arc; - -use apollo_router::graphql::Request; -use apollo_router::http_ext; -use apollo_router::plugin::plugins; -use apollo_router::plugin::DynPlugin; -use apollo_router::services::PluggableRouterServiceBuilder; -use apollo_router::services::SubgraphService; -use apollo_router::Schema; -use serde_json::Value; +use apollo_router::services::RouterRequest; +use apollo_router::TestHarness; use tower::ServiceExt; // This test will fail if run with the "multi_thread" flavor. @@ -22,47 +13,30 @@ async fn all_rhai_callbacks_are_invoked() { let _guard = tracing::dispatcher::set_default(&subscriber); - let dyn_plugin: Box = plugins() - .get("apollo.rhai") - .expect("Plugin not found") - .create_instance( - &Value::from_str(r#"{"scripts":"tests/fixtures", "main": "test_callbacks.rhai"}"#) - .unwrap(), - Default::default(), - ) + let config = serde_json::json!({ + "rhai": { + "scripts": "tests/fixtures", + "main": "test_callbacks.rhai", + } + }); + let router = TestHarness::builder() + .configuration(serde_json::from_value(config).unwrap()) + .schema(include_str!("./fixtures/supergraph.graphql")) + .build() .await .unwrap(); - - let schema = include_str!("./fixtures/supergraph.graphql"); - let schema = Arc::new(Schema::parse(schema, &Default::default()).unwrap()); - - let mut builder = PluggableRouterServiceBuilder::new(schema.clone()) - .with_dyn_plugin("apollo.rhai".to_string(), dyn_plugin); - - let subgraphs = schema.subgraphs(); - for (name, _url) in subgraphs { - let service = SubgraphService::new(name.to_owned()); - builder = builder.with_subgraph_service(name, service); - } - let router = builder.build().await.unwrap().test_service(); - - let request = http_ext::Request::fake_builder() - .body( - Request::builder() - .query(r#"{ topProducts { name } }"#.to_string()) - .build(), - ) + let request = RouterRequest::fake_builder() + .query("{ topProducts { name } }") .build() .unwrap(); - - let _ = router - .oneshot(request.into()) + let _response = router + .oneshot(request) .await .unwrap() .next_response() .await .unwrap(); - + dbg!(_response); for expected_log in [ "router_service setup", "from_router_request", @@ -76,9 +50,9 @@ async fn all_rhai_callbacks_are_invoked() { "subgraph_service setup", "from_subgraph_request", ] { - assert!(tracing_test::internal::logs_with_scope_contain( - "apollo_router", - expected_log - )); + assert!( + tracing_test::internal::logs_with_scope_contain("apollo_router", expected_log), + "log not found: {expected_log}" + ); } } From 5c65ca8d11d927a48011077df1bdc883ae264239 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 23:03:31 +0200 Subject: [PATCH 13/28] Port integration_tests to the new TestHarness --- apollo-router/src/plugins/telemetry/mod.rs | 16 +++- apollo-router/tests/integration_tests.rs | 98 +++++++++++----------- 2 files changed, 61 insertions(+), 53 deletions(-) diff --git a/apollo-router/src/plugins/telemetry/mod.rs b/apollo-router/src/plugins/telemetry/mod.rs index 99e705cc63..a13cdec17f 100644 --- a/apollo-router/src/plugins/telemetry/mod.rs +++ b/apollo-router/src/plugins/telemetry/mod.rs @@ -161,7 +161,7 @@ impl Plugin for Telemetry { type Config = config::Conf; async fn new(init: PluginInit) -> Result { - Self::new_with_subscriber::(init, None).await + Self::new_common::(init.config, None).await } fn router_service( @@ -533,14 +533,24 @@ impl Plugin for Telemetry { impl Telemetry { /// This method can be used instead of `Plugin::new` to override the subscriber pub async fn new_with_subscriber( - init: PluginInit<::Config>, + config: serde_json::Value, + subscriber: S, + ) -> Result + where + S: Subscriber + Send + Sync + for<'span> LookupSpan<'span>, + { + Self::new_common(serde_json::from_value(config)?, Some(subscriber)).await + } + + /// This method can be used instead of `Plugin::new` to override the subscriber + pub async fn new_common( + mut config: ::Config, subscriber: Option, ) -> Result where S: Subscriber + Send + Sync + for<'span> LookupSpan<'span>, { // Apollo config is special because we enable tracing if some env variables are present. - let mut config = init.config; let apollo = config .apollo .as_mut() diff --git a/apollo-router/tests/integration_tests.rs b/apollo-router/tests/integration_tests.rs index 383beb8ea6..515affc4fb 100644 --- a/apollo-router/tests/integration_tests.rs +++ b/apollo-router/tests/integration_tests.rs @@ -14,25 +14,19 @@ use apollo_router::json_ext::Object; use apollo_router::json_ext::ValueExt; use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; -use apollo_router::plugins::csrf; -use apollo_router::plugins::telemetry::apollo; -use apollo_router::plugins::telemetry::config::Tracing; use apollo_router::plugins::telemetry::Telemetry; -use apollo_router::plugins::telemetry::{self}; -use apollo_router::services::PluggableRouterServiceBuilder; use apollo_router::services::RouterRequest; use apollo_router::services::RouterResponse; use apollo_router::services::SubgraphRequest; -use apollo_router::services::SubgraphService; -use apollo_router::Configuration; +use apollo_router::services::SubgraphResponse; use apollo_router::Context; -use apollo_router::Schema; use http::Method; use maplit::hashmap; use serde_json::to_string_pretty; use serde_json_bytes::json; use test_span::prelude::*; use tower::util::BoxCloneService; +use tower::util::BoxService; use tower::BoxError; use tower::ServiceExt; use tracing_subscriber::prelude::__tracing_subscriber_SubscriberExt; @@ -433,7 +427,7 @@ async fn mutation_should_work_over_post() { #[tokio::test(flavor = "multi_thread")] async fn automated_persisted_queries() { - let (router, registry) = setup_router_and_registry(Default::default()).await; + let (router, registry) = setup_router_and_registry(serde_json::json!({})).await; let mut extensions: Object = Default::default(); extensions.insert("code", "PERSISTED_QUERY_NOT_FOUND".into()); @@ -594,10 +588,9 @@ async fn missing_variables() { #[tokio::test(flavor = "multi_thread")] async fn query_just_under_recursion_limit() { - let config = json!({ + let config = serde_json::json!({ "server": {"experimental_parser_recursion_limit": 12_usize} }); - let config = serde_json_bytes::from_value(config).unwrap(); let request = Request::builder() .query(r#"{ me { reviews { author { reviews { author { name } } } } } }"#) .build(); @@ -621,10 +614,9 @@ async fn query_just_under_recursion_limit() { #[tokio::test(flavor = "multi_thread")] async fn query_just_at_recursion_limit() { - let config = json!({ + let config = serde_json::json!({ "server": {"experimental_parser_recursion_limit": 11_usize} }); - let config = serde_json_bytes::from_value(config).unwrap(); let request = Request::builder() .query(r#"{ me { reviews { author { reviews { author { name } } } } } }"#) .build(); @@ -673,63 +665,45 @@ async fn query_node( async fn query_rust( request: RouterRequest, ) -> (apollo_router::graphql::Response, CountingServiceRegistry) { - query_rust_with_config(request, Default::default()).await + query_rust_with_config(request, serde_json::json!({})).await } async fn query_rust_with_config( request: RouterRequest, - config: Arc, + config: serde_json::Value, ) -> (apollo_router::graphql::Response, CountingServiceRegistry) { let (router, counting_registry) = setup_router_and_registry(config).await; (query_with_router(router, request).await, counting_registry) } async fn setup_router_and_registry( - config: Arc, + config: serde_json::Value, ) -> ( BoxCloneService, CountingServiceRegistry, ) { - let schema = include_str!("fixtures/supergraph.graphql"); - let schema = Arc::new(Schema::parse(schema, &config).unwrap()); + let config = serde_json::from_value(config).unwrap(); let counting_registry = CountingServiceRegistry::new(); - let subgraphs = schema.subgraphs(); - let mut builder = PluggableRouterServiceBuilder::new(schema.clone()).with_configuration(config); - let telemetry_plugin = Telemetry::new_with_subscriber( - PluginInit::new( - telemetry::config::Conf { - metrics: Option::default(), - tracing: Some(Tracing::default()), - apollo: Some(apollo::Config::default()), - }, - Default::default(), - ), - Some(tracing_subscriber::registry().with(test_span::Layer {})), + let telemetry = Telemetry::new_with_subscriber( + serde_json::json!({ + "tracing": {}, + "apollo": { + "schema_id": "" + } + }), + tracing_subscriber::registry().with(test_span::Layer {}), ) .await .unwrap(); - let csrf_plugin = csrf::Csrf::new(PluginInit::new(Default::default(), Default::default())) + let router = apollo_router::TestHarness::builder() + .with_subgraph_network_requests() + .configuration(config) + .schema(include_str!("fixtures/supergraph.graphql")) + .extra_plugin(counting_registry.clone()) + .extra_plugin(telemetry) + .build() .await .unwrap(); - builder = builder - .with_dyn_plugin("apollo.telemetry".to_string(), Box::new(telemetry_plugin)) - .with_dyn_plugin("apollo.csrf".to_string(), Box::new(csrf_plugin)); - for (name, _url) in subgraphs { - let cloned_counter = counting_registry.clone(); - let cloned_name = name.clone(); - - let service = - SubgraphService::new(name.to_owned()).map_request(move |request: SubgraphRequest| { - let cloned_counter = cloned_counter.clone(); - cloned_counter.increment(cloned_name.as_str()); - - request - }); - builder = builder.with_subgraph_service(name, service); - } - - let router = builder.build().await.unwrap().test_service(); - (router, counting_registry) } @@ -774,3 +748,27 @@ impl CountingServiceRegistry { self.counts.lock().unwrap().clone() } } + +#[async_trait::async_trait] +impl Plugin for CountingServiceRegistry { + type Config = (); + + async fn new(_: PluginInit) -> Result { + unreachable!() + } + + fn subgraph_service( + &self, + subgraph_name: &str, + service: BoxService, + ) -> BoxService { + let name = subgraph_name.to_owned(); + let counters = self.clone(); + service + .map_request(move |request| { + counters.increment(&name); + request + }) + .boxed() + } +} From 537979ce366a6b9c4a647276e06abb71436a46f0 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 23:31:12 +0200 Subject: [PATCH 14/28] Port apollo-router-benchmarks to the new TestHarness --- .../benches/basic_composition.rs | 2 +- apollo-router-benchmarks/src/lib.rs | 2 +- apollo-router-benchmarks/src/shared.rs | 51 ++++++++++++++----- 3 files changed, 40 insertions(+), 15 deletions(-) diff --git a/apollo-router-benchmarks/benches/basic_composition.rs b/apollo-router-benchmarks/benches/basic_composition.rs index d6ac6f4086..ea21f45a11 100644 --- a/apollo-router-benchmarks/benches/basic_composition.rs +++ b/apollo-router-benchmarks/benches/basic_composition.rs @@ -12,7 +12,7 @@ fn from_elem(c: &mut Criterion) { let router = runtime.block_on(builder.build()).unwrap(); b.to_async(runtime) - .iter(|| basic_composition_benchmark(router.test_service())); + .iter(|| basic_composition_benchmark(router.clone())); }); } diff --git a/apollo-router-benchmarks/src/lib.rs b/apollo-router-benchmarks/src/lib.rs index 13fa867128..b36a324134 100644 --- a/apollo-router-benchmarks/src/lib.rs +++ b/apollo-router-benchmarks/src/lib.rs @@ -8,6 +8,6 @@ pub mod tests { let builder = setup(); let router = runtime.block_on(builder.build()).unwrap(); - runtime.block_on(async move { basic_composition_benchmark(router.test_service()).await }); + runtime.block_on(async move { basic_composition_benchmark(router).await }); } } diff --git a/apollo-router-benchmarks/src/shared.rs b/apollo-router-benchmarks/src/shared.rs index eed37547a8..e58cd3afd4 100644 --- a/apollo-router-benchmarks/src/shared.rs +++ b/apollo-router-benchmarks/src/shared.rs @@ -1,15 +1,20 @@ // this file is shared between the tests and benchmarks, using // include!() instead of as a pub module, so it is only compiled // in dev mode +use apollo_router::plugin::Plugin; +use apollo_router::plugin::PluginInit; use apollo_router::plugin::test::MockSubgraph; -use apollo_router::services::{ RouterRequest, RouterResponse}; -use apollo_router::services::PluggableRouterServiceBuilder; -use apollo_router::Schema; +use apollo_router::services::RouterRequest; +use apollo_router::services::RouterResponse; +use apollo_router::services::SubgraphRequest; +use apollo_router::services::SubgraphResponse; +use apollo_router::TestHarness; 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 std::collections::HashMap; +use tower::util::{BoxCloneService, BoxService}; +use tower::{BoxError, Service, ServiceExt}; static EXPECTED_RESPONSE: Lazy = 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() @@ -39,7 +44,7 @@ pub async fn basic_composition_benchmark( assert_eq!(response, *EXPECTED_RESPONSE); } -pub fn setup() -> PluggableRouterServiceBuilder { +pub fn setup() -> TestHarness<'static> { let account_mocks = vec![ ( json!{{ @@ -217,13 +222,33 @@ pub fn setup() -> PluggableRouterServiceBuilder { ].into_iter().map(|(query, response)| (serde_json::from_value(query).unwrap(), serde_json::from_value(response).unwrap())).collect(); let product_service = MockSubgraph::new(product_mocks); + let mut mocks = HashMap::new(); + mocks.insert("accounts", account_service); + mocks.insert("reviews", review_service); + mocks.insert("products", product_service); + let schema = include_str!("../benches/fixtures/supergraph.graphql"); - let schema = Arc::new(Schema::parse(schema, &Default::default()).unwrap()); + TestHarness::builder().schema(schema).extra_plugin(MockedSubgraphs(mocks)) +} - let builder = PluggableRouterServiceBuilder::new(schema); +struct MockedSubgraphs(HashMap<&'static str, MockSubgraph>); - builder - .with_subgraph_service("accounts", account_service) - .with_subgraph_service("reviews", review_service) - .with_subgraph_service("products", product_service) -} +#[async_trait::async_trait] +impl Plugin for MockedSubgraphs { + type Config = (); + + async fn new(_: PluginInit) -> Result { + unreachable!() + } + + fn subgraph_service( + &self, + subgraph_name: &str, + default: BoxService, + ) -> BoxService { + self.0 + .get(subgraph_name) + .map(|service| service.clone().boxed()) + .unwrap_or(default) + } +} \ No newline at end of file From 7dae42c275778e6af292e1e71038046b915c46ed Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 23:33:40 +0200 Subject: [PATCH 15/28] Port examples/embedded to the new TestHarness --- examples/embedded/src/main.rs | 36 ++++++++++------------------------- 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/examples/embedded/src/main.rs b/examples/embedded/src/main.rs index f97aed7b0d..210408ccdd 100644 --- a/examples/embedded/src/main.rs +++ b/examples/embedded/src/main.rs @@ -1,29 +1,15 @@ -use std::sync::Arc; - -use anyhow::anyhow; -use anyhow::Result; -use apollo_router::services::PluggableRouterServiceBuilder; use apollo_router::services::RouterRequest; -use apollo_router::services::SubgraphService; +use apollo_router::TestHarness; use tower::ServiceExt; #[tokio::main] -async fn main() -> Result<()> { - // get the supergraph from ../../examples/graphql/supergraph.graphql - let schema = include_str!("../../graphql/supergraph.graphql"); - let schema = Arc::new(apollo_router::Schema::parse(schema, &Default::default())?); - - // PluggableRouterServiceBuilder creates a GraphQL pipeline to process queries against a supergraph Schema - // The whole pipeline is set up... - let mut router_builder = PluggableRouterServiceBuilder::new(schema); - - // ... except the SubgraphServices, so we'll let it know Requests against the `accounts` service - // can be performed with an http client against the `https://accounts.demo.starstuff.dev` url - let subgraph_service = SubgraphService::new("accounts".to_string()); - router_builder = router_builder.with_subgraph_service("accounts", subgraph_service); - - // We can now build our service stack... - let router_service = router_builder.build().await?; +async fn main() -> Result<(), tower::BoxError> { + // TestHarness creates a GraphQL pipeline to process queries against a supergraph Schema + let router = TestHarness::builder() + .schema(include_str!("../../graphql/supergraph.graphql")) + .with_subgraph_network_requests() + .build() + .await?; // ...then create a GraphQL request... let request = RouterRequest::fake_builder() @@ -32,11 +18,9 @@ async fn main() -> Result<()> { .expect("expecting valid request"); // ... and run it against the router service! - let res = router_service - .test_service() + let res = router .oneshot(request) - .await - .map_err(|e| anyhow!("router_service call failed: {}", e))? + .await? .next_response() .await .unwrap(); From f4b54a9f0b0cb1cf4334dda31a39f09dbb850a29 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Mon, 8 Aug 2022 23:52:38 +0200 Subject: [PATCH 16/28] Make some things private --- NEXT_CHANGELOG.md | 31 +++++++++++ apollo-router/src/error.rs | 20 ++----- apollo-router/src/json_ext.rs | 6 +- apollo-router/src/layers/cache.rs | 1 - apollo-router/src/lib.rs | 6 +- apollo-router/src/plugin/mod.rs | 1 + apollo-router/src/plugin/test/mod.rs | 1 + apollo-router/src/plugin/test/service.rs | 1 - apollo-router/src/query_planner/mod.rs | 2 +- .../src/services/execution_service.rs | 13 +---- apollo-router/src/services/mod.rs | 6 +- apollo-router/src/services/router_service.rs | 33 ++++------- .../src/services/subgraph_service.rs | 6 +- apollo-router/src/spec/mod.rs | 2 +- apollo-router/tests/infrastructure_tests.rs | 14 +++-- apollo-router/tests/integration_tests.rs | 55 ++++++++++++++++++- examples/cookies-to-headers/src/main.rs | 2 +- 17 files changed, 127 insertions(+), 73 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index a84eb6b9ae..da0f341536 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -85,6 +85,8 @@ By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/p ### Some items were renamed ([PR #FIXME]) +At the crate root: + * `SchemaKind` β†’ `SchemaSource` * `SchemaKind::String(String)` β†’ `SchemaSource::Static { schema_sdl: String }` * `ConfigurationKind` β†’ `ConfigurationSource` @@ -94,6 +96,35 @@ By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/p By [@SimonSapin](https://github.com/SimonSapin) +### Some items were removed from the public API ([PR #FIXME]) + +If you used some of them and don’t find a replacement, +please [file an issue](https://github.com/apollographql/router/issues/) +with details about the use case. + +``` +apollo_router::errors::CacheResolverError +apollo_router::errors::JsonExtError +apollo_router::errors::PlanError +apollo_router::errors::PlannerError +apollo_router::errors::PlannerErrors +apollo_router::errors::QueryPlannerError +apollo_router::errors::ServiceBuildError +apollo_router::json_ext +apollo_router::mock_service! +apollo_router::query_planner::QueryPlan::execute +apollo_router::services::ExecutionService +apollo_router::services::MakeSubgraphService +apollo_router::services::PluggableRouterServiceBuilder +apollo_router::services::RouterService +apollo_router::services::RouterCreator +apollo_router::services::SubgraphService +apollo_router::services::SubgraphServiceFactory +apollo_router::Schema +``` + +By [@SimonSapin](https://github.com/SimonSapin) + ### Router startup API changes ([PR #FIXME]) The `RouterHttpServer::serve` method and its return type `RouterHandle` were removed, diff --git a/apollo-router/src/error.rs b/apollo-router/src/error.rs index 62607887e1..5f35f9c397 100644 --- a/apollo-router/src/error.rs +++ b/apollo-router/src/error.rs @@ -7,9 +7,8 @@ use miette::NamedSource; use miette::Report; use miette::SourceSpan; use router_bridge::introspect::IntrospectionError; -pub use router_bridge::planner::PlanError; use router_bridge::planner::PlanErrors; -pub use router_bridge::planner::PlannerError; +use router_bridge::planner::PlannerError; use router_bridge::planner::UsageReporting; use serde::Deserialize; use serde::Serialize; @@ -157,7 +156,7 @@ impl From for FetchError { /// Error types for CacheResolver #[derive(Error, Debug, Display, Clone)] -pub enum CacheResolverError { +pub(crate) enum CacheResolverError { /// value retrieval failed: {0} RetrievalError(Arc), } @@ -168,25 +167,16 @@ impl From for CacheResolverError { } } -/// An error while processing JSON data. -#[derive(Debug, Error, Display)] -pub enum JsonExtError { - /// Could not find path in JSON. - PathNotFound, - /// Attempt to flatten on non-array node. - InvalidFlatten, -} - /// Error types for service building. #[derive(Error, Debug, Display, Clone)] -pub enum ServiceBuildError { +pub(crate) enum ServiceBuildError { /// couldn't build Router Service: {0} QueryPlannerError(QueryPlannerError), } /// Error types for QueryPlanner #[derive(Error, Debug, Display, Clone)] -pub enum QueryPlannerError { +pub(crate) enum QueryPlannerError { /// couldn't instantiate query planner; invalid schema: {0} SchemaValidationErrors(PlannerErrors), @@ -217,7 +207,7 @@ pub enum QueryPlannerError { #[derive(Clone, Debug, Error)] /// Container for planner setup errors -pub struct PlannerErrors(Arc>); +pub(crate) struct PlannerErrors(Arc>); impl std::fmt::Display for PlannerErrors { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { diff --git a/apollo-router/src/json_ext.rs b/apollo-router/src/json_ext.rs index 860c177765..4f7047e625 100644 --- a/apollo-router/src/json_ext.rs +++ b/apollo-router/src/json_ext.rs @@ -7,12 +7,12 @@ use serde::Serialize; use serde_json_bytes::ByteString; use serde_json_bytes::Entry; use serde_json_bytes::Map; -pub use serde_json_bytes::Value; +pub(crate) use serde_json_bytes::Value; use crate::error::FetchError; /// A JSON object. -pub type Object = Map; +pub(crate) type Object = Map; macro_rules! extract_key_value_from_object { ($object:expr, $key:literal, $pattern:pat => $var:ident) => {{ @@ -41,7 +41,7 @@ macro_rules! ensure_object { #[doc(hidden)] /// Extension trait for [`serde_json::Value`]. -pub trait ValueExt { +pub(crate) trait ValueExt { /// Deep merge the JSON objects, array and override the values in `&mut self` if they already /// exists. #[track_caller] diff --git a/apollo-router/src/layers/cache.rs b/apollo-router/src/layers/cache.rs index 0071a1af7c..a82bcd74c1 100644 --- a/apollo-router/src/layers/cache.rs +++ b/apollo-router/src/layers/cache.rs @@ -182,7 +182,6 @@ mod test { use tower::ServiceExt; use super::*; - use crate::mock_service; #[derive(Default, Clone)] struct Slow; diff --git a/apollo-router/src/lib.rs b/apollo-router/src/lib.rs index 799ce86951..28f6328eba 100644 --- a/apollo-router/src/lib.rs +++ b/apollo-router/src/lib.rs @@ -46,7 +46,9 @@ macro_rules! failfast_error { } #[macro_use] -pub mod json_ext; +mod json_ext; +#[macro_use] +pub mod plugin; mod axum_http_server_factory; mod cache; @@ -59,7 +61,6 @@ pub mod graphql; mod http_server_factory; mod introspection; pub mod layers; -pub mod plugin; pub mod plugins; pub mod query_planner; mod request; @@ -83,7 +84,6 @@ pub use router::ShutdownSource; #[doc(hidden)] pub use router_factory::__create_test_service_factory_from_yaml; pub use services::http_ext; -pub use spec::Schema; pub use test_harness::TestHarness; /// Reexports for macros diff --git a/apollo-router/src/plugin/mod.rs b/apollo-router/src/plugin/mod.rs index cd5bb0b87c..c52d19ded6 100644 --- a/apollo-router/src/plugin/mod.rs +++ b/apollo-router/src/plugin/mod.rs @@ -15,6 +15,7 @@ //! mechanism for interacting with the request and response. pub mod serde; +#[macro_use] pub mod test; use std::collections::HashMap; diff --git a/apollo-router/src/plugin/test/mod.rs b/apollo-router/src/plugin/test/mod.rs index 2f1332543b..fba7d8ffc3 100644 --- a/apollo-router/src/plugin/test/mod.rs +++ b/apollo-router/src/plugin/test/mod.rs @@ -1,6 +1,7 @@ //! Utilities which make it easy to test with [`crate::plugin`]. mod mock; +#[macro_use] mod service; use std::collections::HashMap; diff --git a/apollo-router/src/plugin/test/service.rs b/apollo-router/src/plugin/test/service.rs index ac840e2819..3b7d1ecb8b 100644 --- a/apollo-router/src/plugin/test/service.rs +++ b/apollo-router/src/plugin/test/service.rs @@ -9,7 +9,6 @@ use crate::SubgraphRequest; use crate::SubgraphResponse; /// Build a mock service handler for the router pipeline. -#[macro_export] macro_rules! mock_service { ($name:ident, $request_type:ty, $response_type:ty) => { paste::item! { diff --git a/apollo-router/src/query_planner/mod.rs b/apollo-router/src/query_planner/mod.rs index 0e4f4b0a0e..ad56e0b50a 100644 --- a/apollo-router/src/query_planner/mod.rs +++ b/apollo-router/src/query_planner/mod.rs @@ -258,7 +258,7 @@ impl PlanNode { impl QueryPlan { /// Execute the plan and return a [`Response`]. - pub async fn execute<'a, SF>( + pub(crate) async fn execute<'a, SF>( &self, context: &'a Context, service_factory: &'a Arc, diff --git a/apollo-router/src/services/execution_service.rs b/apollo-router/src/services/execution_service.rs index 0ee52c115e..2ccef14fc9 100644 --- a/apollo-router/src/services/execution_service.rs +++ b/apollo-router/src/services/execution_service.rs @@ -26,22 +26,11 @@ use crate::Schema; /// [`Service`] for query execution. #[derive(Clone)] -pub struct ExecutionService { +pub(crate) struct ExecutionService { pub(crate) schema: Arc, pub(crate) subgraph_creator: Arc, } -//#[buildstructor::buildstructor] -impl ExecutionService { - //#[builder] - pub fn new(schema: Arc, subgraph_creator: Arc) -> Self { - Self { - schema, - subgraph_creator, - } - } -} - impl Service for ExecutionService where SF: SubgraphServiceFactory, diff --git a/apollo-router/src/services/mod.rs b/apollo-router/src/services/mod.rs index 1149ac29df..f255f8a759 100644 --- a/apollo-router/src/services/mod.rs +++ b/apollo-router/src/services/mod.rs @@ -19,9 +19,9 @@ use serde_json_bytes::ByteString; use static_assertions::assert_impl_all; use tower::BoxError; -pub use self::execution_service::*; -pub use self::router_service::*; -pub use self::subgraph_service::*; +pub(crate) use self::execution_service::*; +pub(crate) use self::router_service::*; +pub(crate) use self::subgraph_service::*; use crate::error::Error; use crate::graphql::Request; use crate::graphql::Response; diff --git a/apollo-router/src/services/router_service.rs b/apollo-router/src/services/router_service.rs index cceabb3851..a4fe94e72e 100644 --- a/apollo-router/src/services/router_service.rs +++ b/apollo-router/src/services/router_service.rs @@ -35,7 +35,6 @@ use crate::http_ext::Request; use crate::introspection::Introspection; use crate::layers::DEFAULT_BUFFER_SIZE; use crate::plugin::DynPlugin; -use crate::plugin::Plugin; use crate::query_planner::BridgeQueryPlanner; use crate::query_planner::CachingQueryPlanner; use crate::router_factory::RouterServiceFactory; @@ -55,7 +54,7 @@ pub(crate) type Plugins = IndexMap>; /// Containing [`Service`] in the request lifecyle. #[derive(Clone)] -pub struct RouterService { +pub(crate) struct RouterService { query_planner_service: QueryPlannerService, execution_service_factory: ExecutionFactory, ready_query_planner_service: Option, @@ -64,8 +63,8 @@ pub struct RouterService { #[buildstructor::buildstructor] impl RouterService { - #[builder(visibility = "pub")] - fn new( + #[builder] + pub(crate) fn new( query_planner_service: QueryPlannerService, execution_service_factory: ExecutionFactory, schema: Arc, @@ -240,7 +239,7 @@ where /// collection of plugins, collection of subgraph services are assembled to generate a /// [`tower::util::BoxCloneService`] capable of processing a router request /// through the entire stack to return a response. -pub struct PluggableRouterServiceBuilder { +pub(crate) struct PluggableRouterServiceBuilder { schema: Arc, plugins: Plugins, subgraph_services: Vec<(String, Arc)>, @@ -248,7 +247,7 @@ pub struct PluggableRouterServiceBuilder { } impl PluggableRouterServiceBuilder { - pub fn new(schema: Arc) -> Self { + pub(crate) fn new(schema: Arc) -> Self { Self { schema, plugins: Default::default(), @@ -257,16 +256,7 @@ impl PluggableRouterServiceBuilder { } } - pub fn with_plugin( - mut self, - plugin_name: String, - plugin: E, - ) -> PluggableRouterServiceBuilder { - self.plugins.insert(plugin_name, Box::new(plugin)); - self - } - - pub fn with_dyn_plugin( + pub(crate) fn with_dyn_plugin( mut self, plugin_name: String, plugin: Box, @@ -275,7 +265,7 @@ impl PluggableRouterServiceBuilder { self } - pub fn with_subgraph_service( + pub(crate) fn with_subgraph_service( mut self, name: &str, service_maker: S, @@ -288,7 +278,7 @@ impl PluggableRouterServiceBuilder { self } - pub fn with_configuration( + pub(crate) fn with_configuration( mut self, configuration: Arc, ) -> PluggableRouterServiceBuilder { @@ -300,7 +290,7 @@ impl PluggableRouterServiceBuilder { &mut self.plugins } - pub async fn build(mut self) -> Result { + pub(crate) async fn build(mut self) -> Result { // Note: The plugins are always applied in reverse, so that the // fold is applied in the correct sequence. We could reverse // the list of plugins, but we want them back in the original @@ -371,7 +361,7 @@ impl PluggableRouterServiceBuilder { /// A collection of services and data which may be used to create a "router". #[derive(Clone)] -pub struct RouterCreator { +pub(crate) struct RouterCreator { query_planner_service: CachingQueryPlanner< Buffer< BoxService, @@ -454,7 +444,8 @@ impl RouterCreator { } /// Create a test service. - pub fn test_service( + #[cfg(test)] + pub(crate) fn test_service( &self, ) -> tower::util::BoxCloneService { Buffer::new(self.make(), 512).boxed_clone() diff --git a/apollo-router/src/services/subgraph_service.rs b/apollo-router/src/services/subgraph_service.rs index 91b8dc9feb..a40d380504 100644 --- a/apollo-router/src/services/subgraph_service.rs +++ b/apollo-router/src/services/subgraph_service.rs @@ -62,13 +62,13 @@ impl Display for Compression { /// Client for interacting with subgraphs. #[derive(Clone)] -pub struct SubgraphService { +pub(crate) struct SubgraphService { client: Decompression>>, service: Arc, } impl SubgraphService { - pub fn new(service: impl Into) -> Self { + pub(crate) fn new(service: impl Into) -> Self { let connector = hyper_rustls::HttpsConnectorBuilder::new() .with_native_roots() .https_or_http() @@ -266,7 +266,7 @@ pub(crate) async fn compress(body: String, headers: &HeaderMap) -> Result; + macro_rules! assert_federated_response { ($query:expr, $service_requests:expr $(,)?) => { let request = Request::builder() @@ -772,3 +775,49 @@ impl Plugin for CountingServiceRegistry { .boxed() } } + +trait ValueExt { + fn eq_and_ordered(&self, other: &Self) -> bool; +} + +impl ValueExt for Value { + fn eq_and_ordered(&self, other: &Self) -> bool { + match (self, other) { + (Value::Object(a), Value::Object(b)) => { + let mut it_a = a.iter(); + let mut it_b = b.iter(); + + loop { + match (it_a.next(), it_b.next()) { + (Some(_), None) | (None, Some(_)) => break false, + (None, None) => break true, + (Some((field_a, value_a)), Some((field_b, value_b))) + if field_a == field_b && ValueExt::eq_and_ordered(value_a, value_b) => + { + continue + } + (Some(_), Some(_)) => break false, + } + } + } + (Value::Array(a), Value::Array(b)) => { + let mut it_a = a.iter(); + let mut it_b = b.iter(); + + loop { + match (it_a.next(), it_b.next()) { + (Some(_), None) | (None, Some(_)) => break false, + (None, None) => break true, + (Some(value_a), Some(value_b)) + if ValueExt::eq_and_ordered(value_a, value_b) => + { + continue + } + (Some(_), Some(_)) => break false, + } + } + } + (a, b) => a == b, + } + } +} diff --git a/examples/cookies-to-headers/src/main.rs b/examples/cookies-to-headers/src/main.rs index 16609d7287..55250d9212 100644 --- a/examples/cookies-to-headers/src/main.rs +++ b/examples/cookies-to-headers/src/main.rs @@ -32,12 +32,12 @@ fn main() -> Result<()> { #[cfg(test)] mod tests { - use apollo_router::http_ext; use apollo_router::plugin::test; use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::plugins::rhai::Conf; use apollo_router::plugins::rhai::Rhai; + use apollo_router::services::http_ext; use apollo_router::services::SubgraphRequest; use apollo_router::services::SubgraphResponse; use http::header::HeaderName; From 889326c90d1afd7c3d63ae3a7457c35d6dba77bf Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 9 Aug 2022 09:11:32 +0200 Subject: [PATCH 17/28] Add apollo_router::stages, replacing apollo_router::services in the public API --- NEXT_CHANGELOG.md | 71 ++++++++++-- apollo-router-benchmarks/src/shared.rs | 16 ++- .../plugin/src/plugins/{{snake_name}}.rs | 37 +++---- apollo-router/src/layers/mod.rs | 102 +++++++++--------- apollo-router/src/lib.rs | 18 ++-- apollo-router/src/plugin/mod.rs | 68 ++++-------- apollo-router/src/plugin/test/mod.rs | 3 +- apollo-router/src/plugins/csrf.rs | 7 +- apollo-router/src/plugins/forbid_mutations.rs | 7 +- apollo-router/src/plugins/headers.rs | 9 +- .../src/plugins/include_subgraph_errors.rs | 9 +- apollo-router/src/plugins/override_url.rs | 9 +- apollo-router/src/plugins/rhai.rs | 58 +++------- apollo-router/src/plugins/telemetry/mod.rs | 27 ++--- .../src/plugins/traffic_shaping/mod.rs | 23 ++-- apollo-router/src/query_planner/mod.rs | 2 +- .../src/services/execution_service.rs | 6 +- apollo-router/src/services/mod.rs | 2 +- apollo-router/src/services/router_service.rs | 9 +- apollo-router/src/stages.rs | 43 ++++++++ apollo-router/src/test_harness.rs | 12 +-- apollo-router/tests/integration_tests.rs | 33 +++--- apollo-router/tests/rhai_tests.rs | 4 +- docs/source/customizations/native.mdx | 22 ++-- examples/add-timestamp-header/src/main.rs | 9 +- examples/async-auth/README.md | 4 +- .../src/allow_client_id_from_file.rs | 30 +++--- examples/context/src/context_data.rs | 24 ++--- examples/cookies-to-headers/src/main.rs | 11 +- examples/embedded/src/main.rs | 4 +- .../forbid-anonymous-operations/README.md | 4 +- .../src/forbid_anonymous_operations.rs | 26 ++--- examples/hello-world/src/hello_world.rs | 33 ++---- examples/jwt-auth/README.md | 4 +- examples/jwt-auth/src/jwt.rs | 38 +++---- examples/op-name-to-header/src/main.rs | 9 +- .../rhai-data-response-mutate/src/main.rs | 4 +- .../rhai-error-response-mutate/src/main.rs | 4 +- examples/rhai-logging/src/main.rs | 9 +- .../rhai-subgraph-request-log/src/main.rs | 4 +- examples/rhai-surrogate-cache-key/src/main.rs | 4 +- .../src/propagate_status_code.rs | 44 +++----- examples/supergraph_sdl/src/supergraph_sdl.rs | 11 +- 43 files changed, 404 insertions(+), 469 deletions(-) create mode 100644 apollo-router/src/stages.rs diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index da0f341536..e49db68785 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -83,7 +83,7 @@ using an `expect_clone` call with mockall. By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/1440 -### Some items were renamed ([PR #FIXME]) +### Some items were renamed or moved ([PR #FIXME]) At the crate root: @@ -94,6 +94,67 @@ At the crate root: * `ShutdownKind` β†’ `ShutdownSource` * `ApolloRouter` β†’ `RouterHttpServer` +A new `apollo_router::stages` module replaces `apollo_router::services` in the public API, +reexporting its items and adding `BoxService` and `BoxCloneService` type aliases. +In pseudo-syntax: + +```rust +mod router { + use apollo_router::services::RouterRequest as Request; + use apollo_router::services::RouterResponse as Response; + type BoxService = tower::util::BoxService; + type BoxCloneService = tower::util::BoxCloneService; +} + +mod query_planner { + use apollo_router::services::QueryPlannerRequest as Request; + use apollo_router::services::QueryPlannerResponse as Response; + type BoxService = tower::util::BoxService; + type BoxCloneService = tower::util::BoxCloneService; + + // Reachable from Request or Response: + use apollo_router::query_planner::QueryPlan; + use apollo_router::query_planner::QueryPlanOptions; + use apollo_router::services::QueryPlannerContent; + use apollo_router::spec::Query; +} + +mod execution { + use apollo_router::services::ExecutionRequest as Request; + use apollo_router::services::ExecutionResponse as Response; + type BoxService = tower::util::BoxService; + type BoxCloneService = tower::util::BoxCloneService; +} + +mod subgraph { + use super::*; + use apollo_router::services::SubgraphRequest as Request; + use apollo_router::services::SubgraphResponse as Response; + type BoxService = tower::util::BoxService; + type BoxCloneService = tower::util::BoxCloneService; + + // Reachable from Request or Response: + use apollo_router::query_planner::OperationKind; +} +``` + +Migration example: + +```diff +-use tower::util::BoxService; +-use tower::BoxError; +-use apollo_router::services::{RouterRequest, RouterResponse}; ++use apollo_router::stages::router; + +-async fn example(service: BoxService) -> RouterResponse { ++async fn example(service: router::BoxService) -> router::Response { +- let request = RouterRequest::builder()/*…*/.build(); ++ let request = router::Request::builder()/*…*/.build(); + service.oneshot(request).await + } +``` + + By [@SimonSapin](https://github.com/SimonSapin) ### Some items were removed from the public API ([PR #FIXME]) @@ -113,13 +174,7 @@ apollo_router::errors::ServiceBuildError apollo_router::json_ext apollo_router::mock_service! apollo_router::query_planner::QueryPlan::execute -apollo_router::services::ExecutionService -apollo_router::services::MakeSubgraphService -apollo_router::services::PluggableRouterServiceBuilder -apollo_router::services::RouterService -apollo_router::services::RouterCreator -apollo_router::services::SubgraphService -apollo_router::services::SubgraphServiceFactory +apollo_router::services apollo_router::Schema ``` diff --git a/apollo-router-benchmarks/src/shared.rs b/apollo-router-benchmarks/src/shared.rs index e58cd3afd4..17bb1d9247 100644 --- a/apollo-router-benchmarks/src/shared.rs +++ b/apollo-router-benchmarks/src/shared.rs @@ -4,16 +4,14 @@ use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::plugin::test::MockSubgraph; -use apollo_router::services::RouterRequest; -use apollo_router::services::RouterResponse; -use apollo_router::services::SubgraphRequest; -use apollo_router::services::SubgraphResponse; +use apollo_router::stages::router; +use apollo_router::stages::subgraph; use apollo_router::TestHarness; use apollo_router::graphql::Response; use once_cell::sync::Lazy; use serde_json::json; use std::collections::HashMap; -use tower::util::{BoxCloneService, BoxService}; + use tower::{BoxError, Service, ServiceExt}; static EXPECTED_RESPONSE: Lazy = Lazy::new(|| { @@ -23,9 +21,9 @@ static EXPECTED_RESPONSE: Lazy = Lazy::new(|| { 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, + mut router_service: router::BoxCloneService, ) { - let request = RouterRequest::fake_builder() + let request = router::Request::fake_builder() .query(QUERY.to_string()) .variable("first", 2usize) .build().expect("expecting valid request"); @@ -244,8 +242,8 @@ impl Plugin for MockedSubgraphs { fn subgraph_service( &self, subgraph_name: &str, - default: BoxService, - ) -> BoxService { + default: subgraph::BoxService, + ) -> subgraph::BoxService { self.0 .get(subgraph_name) .map(|service| service.clone().boxed()) diff --git a/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs b/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs index acb349c98e..f3fbfe49a1 100644 --- a/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs +++ b/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs @@ -1,28 +1,25 @@ use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::register_plugin; +use apollo_router::stages::router; {{#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}; +use apollo_router::stages::execution; +use apollo_router::stages::query_planner; +use apollo_router::stages::subgraph; {{/if}} {{#if type_auth}} -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::{RouterRequest, RouterResponse}; use apollo_router::layers::ServiceBuilderExt; use tower::ServiceExt; use tower::ServiceBuilder; {{/if}} use schemars::JsonSchema; use serde::Deserialize; -use tower::util::BoxService; use tower::BoxError; #[derive(Debug)] @@ -52,8 +49,8 @@ impl Plugin for {{pascal_name}} { // Delete this function if you are not customizing it. fn router_service( &self, - service: BoxService, - ) -> BoxService { + service: router::BoxService, + ) -> router::BoxService { // Always use service builder to compose your plugins. // It provides off the shelf building blocks for your plugin. // @@ -68,16 +65,16 @@ impl Plugin for {{pascal_name}} { // Delete this function if you are not customizing it. fn query_planning_service( &self, - service: BoxService, - ) -> BoxService { + service: query_planner::BoxService, + ) -> query_planner::BoxService { service } // Delete this function if you are not customizing it. fn execution_service( &self, - service: BoxService, - ) -> BoxService { + service: execution::BoxService, + ) -> execution::BoxService { service } @@ -85,8 +82,8 @@ impl Plugin for {{pascal_name}} { fn subgraph_service( &self, _name: &str, - service: BoxService, - ) -> BoxService { + service: subgraph::BoxService, + ) -> subgraph::BoxService { service } } @@ -104,11 +101,11 @@ impl Plugin for {{pascal_name}} { fn router_service( &self, - service: BoxService, - ) -> BoxService { + service: router::BoxService, + ) -> router::BoxService { ServiceBuilder::new() - .checkpoint_async(|request : RouterRequest| async { + .checkpoint_async(|request : router::Request| async { // Do some async call here to auth, and decide if to continue or not. Ok(ControlFlow::Continue(request)) }) @@ -131,8 +128,8 @@ impl Plugin for {{pascal_name}} { fn router_service( &self, - service: BoxService, - ) -> BoxService { + service: router::BoxService, + ) -> router::BoxService { ServiceBuilder::new() .instrument(|_request| { diff --git a/apollo-router/src/layers/mod.rs b/apollo-router/src/layers/mod.rs index 18a6f46614..ec3912c4f2 100644 --- a/apollo-router/src/layers/mod.rs +++ b/apollo-router/src/layers/mod.rs @@ -54,9 +54,9 @@ pub trait ServiceBuilderExt: Sized { /// # use tower_service::Service; /// # use tracing::info_span; /// # use apollo_router::graphql::Response; - /// # use apollo_router::services::{RouterRequest, RouterResponse}; + /// # use apollo_router::stages::router; /// # use apollo_router::layers::ServiceBuilderExt; - /// # fn test + Send>(service: S) { + /// # fn test(service: router::BoxService) { /// //TODO This doc has highlighted a couple of issues that need to be resolved /// //let _ = ServiceBuilder::new() /// // .cache(Cache::builder().time_to_live(Duration::from_secs(1)).max_capacity(100).build(), @@ -101,22 +101,21 @@ pub trait ServiceBuilderExt: Sized { /// # use tower::ServiceBuilder; /// # use tower_service::Service; /// # use tracing::info_span; - /// # use apollo_router::services::{RouterRequest, RouterResponse}; + /// # use apollo_router::stages::router; /// # use apollo_router::layers::ServiceBuilderExt; - /// # fn test>> + 'static + Send>(service: S) where >::Future: Send, >::Error: Send + Sync + std::error::Error, >::Response: Send { + /// # fn test(service: router::BoxService) { /// let _ = ServiceBuilder::new() - /// .checkpoint(|req:RouterRequest|{ - /// if req.originating_request.method() == Method::GET { - /// Ok(ControlFlow::Break(RouterResponse::builder() - /// .data("Only get requests allowed") - /// .context(req.context).build()) - /// ) - /// } - /// else { - /// Ok(ControlFlow::Continue(req)) - /// } - /// }) - /// .service(service); + /// .checkpoint(|req: router::Request|{ + /// if req.originating_request.method() == Method::GET { + /// Ok(ControlFlow::Break(router::Response::builder() + /// .data("Only get requests allowed") + /// .context(req.context) + /// .build()?)) + /// } else { + /// Ok(ControlFlow::Continue(req)) + /// } + /// }) + /// .service(service); /// # } /// ``` fn checkpoint( @@ -162,23 +161,25 @@ pub trait ServiceBuilderExt: Sized { /// # use tower::ServiceBuilder; /// # use tower_service::Service; /// # use tracing::info_span; - /// # use apollo_router::services::{RouterRequest, RouterResponse}; + /// # use apollo_router::stages::router; /// # use apollo_router::layers::ServiceBuilderExt; - /// # fn test>> + 'static + Send>(service: S) where >::Future: Send, >::Error: Send + Sync + std::error::Error, >::Response: Send { + /// # fn test(service: router::BoxService) { /// let _ = ServiceBuilder::new() - /// .checkpoint_async(|req:RouterRequest| async { - /// if req.originating_request.method() == Method::GET { - /// Ok(ControlFlow::Break(RouterResponse::builder() - /// .data("Only get requests allowed") - /// .context(req.context).build()) - /// ) - /// } - /// else { - /// Ok(ControlFlow::Continue(req)) - /// } - /// }.boxed()) - /// .buffered() - /// .service(service); + /// .checkpoint_async(|req: router::Request| + /// async { + /// if req.originating_request.method() == Method::GET { + /// Ok(ControlFlow::Break(router::Response::builder() + /// .data("Only get requests allowed") + /// .context(req.context) + /// .build()?)) + /// } else { + /// Ok(ControlFlow::Continue(req)) + /// } + /// } + /// .boxed() + /// ) + /// .buffered() + /// .service(service); /// # } /// ``` fn checkpoint_async( @@ -205,9 +206,9 @@ pub trait ServiceBuilderExt: Sized { /// # use tower::ServiceBuilder; /// # use tower_service::Service; /// # use tracing::info_span; - /// # use apollo_router::services::RouterRequest; + /// # use apollo_router::stages::router; /// # use apollo_router::layers::ServiceBuilderExt; - /// # fn test + 'static + Send>(service: S) where >::Future: Send, >::Error: Send + Sync + std::error::Error, >::Response: Send { + /// # fn test(service: router::BoxService) { /// let _ = ServiceBuilder::new() /// .buffered() /// .service(service); @@ -234,9 +235,9 @@ pub trait ServiceBuilderExt: Sized { /// # use tower::ServiceBuilder; /// # use tower_service::Service; /// # use tracing::info_span; - /// # use apollo_router::services::RouterRequest; + /// # use apollo_router::stages::router; /// # use apollo_router::layers::ServiceBuilderExt; - /// # fn test + Send>(service: S) { + /// # fn test(service: router::BoxService) { /// let instrumented = ServiceBuilder::new() /// .instrument(|_request| info_span!("query_planning")) /// .service(service); @@ -271,15 +272,15 @@ pub trait ServiceBuilderExt: Sized { /// # use tower_service::Service; /// # use tracing::info_span; /// # use apollo_router::Context; - /// # use apollo_router::services::{RouterRequest, RouterResponse}; + /// # use apollo_router::stages::router; /// # use apollo_router::layers::ServiceBuilderExt; - /// # fn test> + 'static + Send>(service: S) where >::Future: Send, >::Error: Send + Sync + std::error::Error, >::Response: Send { - /// let _ : BoxService = ServiceBuilder::new() - /// .map_future_with_context(|req: &RouterRequest| req.context.clone(), |ctx : Context, fut| async { - /// fut.await - /// }) - /// .service(service) - /// .boxed(); + /// # fn test(service: router::BoxService) { + /// let _ : router::BoxService = ServiceBuilder::new() + /// .map_future_with_context( + /// |req: &router::Request| req.context.clone(), + /// |ctx : Context, fut| async { fut.await }) + /// .service(service) + /// .boxed(); /// # } /// ``` fn map_future_with_context( @@ -333,15 +334,16 @@ pub trait ServiceExt: Service { /// # use tower_service::Service; /// # use tracing::info_span; /// # use apollo_router::Context; - /// # use apollo_router::services::{RouterRequest, RouterResponse}; + /// # use apollo_router::stages::router; /// # use apollo_router::layers::ServiceBuilderExt; /// # use apollo_router::layers::ServiceExt as ApolloServiceExt; - /// # fn test> + 'static + Send>(service: S) where >::Future: Send, >::Error: Send + Sync + std::error::Error, >::Response: Send { - /// let _ : BoxService = service - /// .map_future_with_context(|req: &RouterRequest| req.context.clone(), |ctx : Context, fut| async { - /// fut.await - /// }) - /// .boxed(); + /// # fn test(service: router::BoxService) { + /// let _ : router::BoxService = service + /// .map_future_with_context( + /// |req: &router::Request| req.context.clone(), + /// |ctx : Context, fut| async { fut.await } + /// ) + /// .boxed(); /// # } /// ``` fn map_future_with_context( diff --git a/apollo-router/src/lib.rs b/apollo-router/src/lib.rs index 28f6328eba..5a6136c82c 100644 --- a/apollo-router/src/lib.rs +++ b/apollo-router/src/lib.rs @@ -3,19 +3,20 @@ //! Most of these modules are of varying interest to different audiences. //! //! If your interests are confined to developing plugins, then the following modules -//! are likely to be of most interest to you: [`self`] [`error`] [`graphql`] [`layers`] [`plugin`] [`services`] +//! are likely to be of most interest to you: //! -//! self - this module (apollo_router) contains high level building blocks for a federated GraphQL router +//! * [`self`] - this module (apollo_router) contains high level building blocks for a federated GraphQL router //! -//! error - the various errors that the router is expected to handle +//! * [`error`] - the various errors that the router is expected to handle //! -//! graphql - graphql specific functionality for requests, responses, errors +//! * [`graphql`] - graphql specific functionality for requests, responses, errors //! -//! layers - examples of tower layers used to implement plugins +//! * [`layers`] - examples of tower layers used to implement plugins //! -//! plugin - various APIs for implementing a plugin +//! * [`plugin`] - various APIs for implementing a plugin //! -//! services - definition of the various services a plugin may process +//! * [`stages`] - the various stages of handling a GraphQL requests, +//! and APIs for plugins to intercept them //! //! Ultimately, you might want to be interested in all aspects of the implementation, in which case //! you'll want to become familiar with all of the code. @@ -67,8 +68,9 @@ mod request; mod response; mod router; mod router_factory; -pub mod services; +mod services; mod spec; +pub mod stages; mod state_machine; mod test_harness; diff --git a/apollo-router/src/plugin/mod.rs b/apollo-router/src/plugin/mod.rs index c52d19ded6..f206a69ec3 100644 --- a/apollo-router/src/plugin/mod.rs +++ b/apollo-router/src/plugin/mod.rs @@ -41,14 +41,10 @@ use tower::ServiceBuilder; use crate::http_ext; use crate::layers::ServiceBuilderExt; -use crate::ExecutionRequest; -use crate::ExecutionResponse; -use crate::QueryPlannerRequest; -use crate::QueryPlannerResponse; -use crate::RouterRequest; -use crate::RouterResponse; -use crate::SubgraphRequest; -use crate::SubgraphResponse; +use crate::stages::execution; +use crate::stages::query_planner; +use crate::stages::router; +use crate::stages::subgraph; type InstanceFactory = fn(&serde_json::Value, Arc) -> BoxFuture, BoxError>>; @@ -165,10 +161,7 @@ pub trait Plugin: Send + Sync + 'static + Sized { /// This service runs at the very beginning and very end of the request lifecycle. /// Define router_service if your customization needs to interact at the earliest or latest point possible. /// For example, this is a good opportunity to perform JWT verification before allowing a request to proceed further. - fn router_service( - &self, - service: BoxService, - ) -> BoxService { + fn router_service(&self, service: router::BoxService) -> router::BoxService { service } @@ -180,17 +173,14 @@ pub trait Plugin: Send + Sync + 'static + Sized { /// must be performed on the query, they should be done in router service plugins. fn query_planning_service( &self, - service: BoxService, - ) -> BoxService { + service: query_planner::BoxService, + ) -> query_planner::BoxService { service } /// This service handles initiating the execution of a query plan after it's been generated. /// Define `execution_service` if your customization includes logic to govern execution (for example, if you want to block a particular query based on a policy decision). - fn execution_service( - &self, - service: BoxService, - ) -> BoxService { + fn execution_service(&self, service: execution::BoxService) -> execution::BoxService { service } @@ -200,8 +190,8 @@ pub trait Plugin: Send + Sync + 'static + Sized { fn subgraph_service( &self, _subgraph_name: &str, - service: BoxService, - ) -> BoxService { + service: subgraph::BoxService, + ) -> subgraph::BoxService { service } @@ -236,10 +226,7 @@ pub trait DynPlugin: Send + Sync + 'static { /// It's the entrypoint of every requests and also the last hook before sending the response. /// Define router_service if your customization needs to interact at the earliest or latest point possible. /// For example, this is a good opportunity to perform JWT verification before allowing a request to proceed further. - fn router_service( - &self, - service: BoxService, - ) -> BoxService; + fn router_service(&self, service: router::BoxService) -> router::BoxService; /// This service handles generating the query plan for each incoming request. /// Define `query_planning_service` if your customization needs to interact with query planning functionality (for example, to log query plan details). @@ -249,15 +236,12 @@ pub trait DynPlugin: Send + Sync + 'static { /// must be performed on the query, they should be done in router service plugins. fn query_planning_service( &self, - service: BoxService, - ) -> BoxService; + service: query_planner::BoxService, + ) -> query_planner::BoxService; /// This service handles initiating the execution of a query plan after it's been generated. /// Define `execution_service` if your customization includes logic to govern execution (for example, if you want to block a particular query based on a policy decision). - fn execution_service( - &self, - service: BoxService, - ) -> BoxService; + fn execution_service(&self, service: execution::BoxService) -> execution::BoxService; /// This service handles communication between the Apollo Router and your subgraphs. /// Define `subgraph_service` to configure this communication (for example, to dynamically add headers to pass to a subgraph). @@ -265,8 +249,8 @@ pub trait DynPlugin: Send + Sync + 'static { fn subgraph_service( &self, _subgraph_name: &str, - service: BoxService, - ) -> BoxService; + service: subgraph::BoxService, + ) -> subgraph::BoxService; /// The `custom_endpoint` method lets you declare a new endpoint exposed for your plugin. /// For now it's only accessible for official `apollo.` plugins and for `experimental.`. This endpoint will be accessible via `/plugins/group.plugin_name` @@ -287,32 +271,22 @@ where self.activate() } - fn router_service( - &self, - service: BoxService, - ) -> BoxService { + fn router_service(&self, service: router::BoxService) -> router::BoxService { self.router_service(service) } fn query_planning_service( &self, - service: BoxService, - ) -> BoxService { + service: query_planner::BoxService, + ) -> query_planner::BoxService { self.query_planning_service(service) } - fn execution_service( - &self, - service: BoxService, - ) -> BoxService { + fn execution_service(&self, service: execution::BoxService) -> execution::BoxService { self.execution_service(service) } - fn subgraph_service( - &self, - name: &str, - service: BoxService, - ) -> BoxService { + fn subgraph_service(&self, name: &str, service: subgraph::BoxService) -> subgraph::BoxService { self.subgraph_service(name, service) } diff --git a/apollo-router/src/plugin/test/mod.rs b/apollo-router/src/plugin/test/mod.rs index fba7d8ffc3..ba5cbfe98d 100644 --- a/apollo-router/src/plugin/test/mod.rs +++ b/apollo-router/src/plugin/test/mod.rs @@ -37,12 +37,13 @@ use crate::services::Plugins; use crate::services::RouterRequest; use crate::services::RouterResponse; use crate::services::SubgraphRequest; +use crate::stages::router; use crate::Configuration; use crate::RouterService; use crate::Schema; pub struct PluginTestHarness { - router_service: BoxService, + router_service: router::BoxService, } pub enum IntoSchema { String(String), diff --git a/apollo-router/src/plugins/csrf.rs b/apollo-router/src/plugins/csrf.rs index 4d90de9c12..e05b19d820 100644 --- a/apollo-router/src/plugins/csrf.rs +++ b/apollo-router/src/plugins/csrf.rs @@ -6,7 +6,6 @@ use http::HeaderMap; use http::StatusCode; use schemars::JsonSchema; use serde::Deserialize; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -15,6 +14,7 @@ use crate::layers::ServiceBuilderExt; use crate::plugin::Plugin; use crate::plugin::PluginInit; use crate::register_plugin; +use crate::stages::router; use crate::RouterRequest; use crate::RouterResponse; @@ -101,10 +101,7 @@ impl Plugin for Csrf { }) } - fn router_service( - &self, - service: BoxService, - ) -> BoxService { + fn router_service(&self, service: router::BoxService) -> router::BoxService { if !self.config.unsafe_disabled { let required_headers = self.config.required_headers.clone(); ServiceBuilder::new() diff --git a/apollo-router/src/plugins/forbid_mutations.rs b/apollo-router/src/plugins/forbid_mutations.rs index b335cf997d..43bfd68a3f 100644 --- a/apollo-router/src/plugins/forbid_mutations.rs +++ b/apollo-router/src/plugins/forbid_mutations.rs @@ -1,7 +1,6 @@ use std::ops::ControlFlow; use http::StatusCode; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -12,6 +11,7 @@ use crate::layers::ServiceBuilderExt; use crate::plugin::Plugin; use crate::plugin::PluginInit; use crate::register_plugin; +use crate::stages::execution; use crate::ExecutionRequest; use crate::ExecutionResponse; @@ -30,10 +30,7 @@ impl Plugin for ForbidMutations { }) } - fn execution_service( - &self, - service: BoxService, - ) -> BoxService { + fn execution_service(&self, service: execution::BoxService) -> execution::BoxService { if self.forbid { ServiceBuilder::new() .checkpoint(|req: ExecutionRequest| { diff --git a/apollo-router/src/plugins/headers.rs b/apollo-router/src/plugins/headers.rs index cf77b72cbb..651233a709 100644 --- a/apollo-router/src/plugins/headers.rs +++ b/apollo-router/src/plugins/headers.rs @@ -18,7 +18,6 @@ use lazy_static::lazy_static; use regex::Regex; use schemars::JsonSchema; use serde::Deserialize; -use tower::util::BoxService; use tower::BoxError; use tower::Layer; use tower::ServiceBuilder; @@ -33,8 +32,8 @@ use crate::plugin::serde::deserialize_regex; use crate::plugin::Plugin; use crate::plugin::PluginInit; use crate::register_plugin; +use crate::stages::subgraph; use crate::SubgraphRequest; -use crate::SubgraphResponse; register_plugin!("apollo", "headers", Headers); @@ -121,11 +120,7 @@ impl Plugin for Headers { config: init.config, }) } - fn subgraph_service( - &self, - name: &str, - service: BoxService, - ) -> BoxService { + fn subgraph_service(&self, name: &str, service: subgraph::BoxService) -> subgraph::BoxService { let mut operations = self.config.all.clone(); if let Some(subgraph_operations) = self.config.subgraphs.get(name) { operations.append(&mut subgraph_operations.clone()) diff --git a/apollo-router/src/plugins/include_subgraph_errors.rs b/apollo-router/src/plugins/include_subgraph_errors.rs index 1282bfff26..7e24b9ddbc 100644 --- a/apollo-router/src/plugins/include_subgraph_errors.rs +++ b/apollo-router/src/plugins/include_subgraph_errors.rs @@ -3,7 +3,6 @@ use std::collections::HashMap; use once_cell::sync::Lazy; use schemars::JsonSchema; use serde::Deserialize; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceExt; @@ -11,7 +10,7 @@ use crate::error::Error as SubgraphError; use crate::plugin::Plugin; use crate::plugin::PluginInit; use crate::register_plugin; -use crate::SubgraphRequest; +use crate::stages::subgraph; use crate::SubgraphResponse; #[allow(clippy::field_reassign_with_default)] @@ -52,11 +51,7 @@ impl Plugin for IncludeSubgraphErrors { }) } - fn subgraph_service( - &self, - name: &str, - service: BoxService, - ) -> BoxService { + fn subgraph_service(&self, name: &str, service: subgraph::BoxService) -> subgraph::BoxService { // Search for subgraph in our configured subgraph map. // If we can't find it, use the "all" value if !*self.config.subgraphs.get(name).unwrap_or(&self.config.all) { diff --git a/apollo-router/src/plugins/override_url.rs b/apollo-router/src/plugins/override_url.rs index 0147a10f25..2ed5724904 100644 --- a/apollo-router/src/plugins/override_url.rs +++ b/apollo-router/src/plugins/override_url.rs @@ -4,15 +4,14 @@ use std::collections::HashMap; use std::str::FromStr; use http::Uri; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceExt; use crate::plugin::Plugin; use crate::plugin::PluginInit; use crate::register_plugin; +use crate::stages::subgraph; use crate::SubgraphRequest; -use crate::SubgraphResponse; #[derive(Debug, Clone)] struct OverrideSubgraphUrl { @@ -36,8 +35,8 @@ impl Plugin for OverrideSubgraphUrl { fn subgraph_service( &self, subgraph_name: &str, - service: BoxService, - ) -> BoxService { + service: subgraph::BoxService, + ) -> subgraph::BoxService { let new_url = self.urls.get(subgraph_name).cloned(); service .map_request(move |mut req: SubgraphRequest| { @@ -63,11 +62,11 @@ mod tests { use tower::Service; use tower::ServiceExt; - use super::*; use crate::plugin::test::MockSubgraphService; use crate::plugin::DynPlugin; use crate::Context; use crate::SubgraphRequest; + use crate::SubgraphResponse; #[tokio::test] async fn plugin_registered() { diff --git a/apollo-router/src/plugins/rhai.rs b/apollo-router/src/plugins/rhai.rs index fc34b79610..a6fe745831 100644 --- a/apollo-router/src/plugins/rhai.rs +++ b/apollo-router/src/plugins/rhai.rs @@ -51,12 +51,8 @@ use crate::register_plugin; use crate::Context; use crate::ExecutionRequest; use crate::ExecutionResponse; -use crate::QueryPlannerRequest; -use crate::QueryPlannerResponse; use crate::RouterRequest; use crate::RouterResponse; -use crate::SubgraphRequest; -use crate::SubgraphResponse; trait OptionDance { fn with_mut(&self, f: impl FnOnce(&mut T) -> R) -> R; @@ -92,35 +88,21 @@ impl OptionDance for SharedMut { } mod router { - use super::*; - - pub(crate) type Request = RouterRequest; - pub(crate) type Response = RhaiRouterResponse; - pub(crate) type Service = BoxService; + pub(crate) use crate::stages::router::*; + pub(crate) type Response = super::RhaiRouterResponse; } mod query_planner { - use super::*; - - pub(crate) type Request = QueryPlannerRequest; - pub(crate) type Response = QueryPlannerResponse; - pub(crate) type Service = BoxService; + pub(crate) use crate::stages::query_planner::*; } mod execution { - use super::*; - - pub(crate) type Request = ExecutionRequest; - pub(crate) type Response = RhaiExecutionResponse; - pub(crate) type Service = BoxService; + pub(crate) use crate::stages::execution::*; + pub(crate) type Response = super::RhaiExecutionResponse; } mod subgraph { - use super::*; - - pub(crate) type Request = SubgraphRequest; - pub(crate) type Response = SubgraphResponse; - pub(crate) type Service = BoxService; + pub(crate) use crate::stages::subgraph::*; } #[export_module] @@ -345,10 +327,7 @@ impl Plugin for Rhai { Ok(Self { ast, engine }) } - fn router_service( - &self, - service: BoxService, - ) -> BoxService { + fn router_service(&self, service: router::BoxService) -> router::BoxService { const FUNCTION_NAME_SERVICE: &str = "router_service"; if !self.ast_has_function(FUNCTION_NAME_SERVICE) { return service; @@ -367,8 +346,8 @@ impl Plugin for Rhai { fn query_planning_service( &self, - service: BoxService, - ) -> BoxService { + service: query_planner::BoxService, + ) -> query_planner::BoxService { const FUNCTION_NAME_SERVICE: &str = "query_planner_service"; if !self.ast_has_function(FUNCTION_NAME_SERVICE) { return service; @@ -385,10 +364,7 @@ impl Plugin for Rhai { shared_service.take_unwrap() } - fn execution_service( - &self, - service: BoxService, - ) -> BoxService { + fn execution_service(&self, service: execution::BoxService) -> execution::BoxService { const FUNCTION_NAME_SERVICE: &str = "execution_service"; if !self.ast_has_function(FUNCTION_NAME_SERVICE) { return service; @@ -405,11 +381,7 @@ impl Plugin for Rhai { shared_service.take_unwrap() } - fn subgraph_service( - &self, - name: &str, - service: BoxService, - ) -> BoxService { + fn subgraph_service(&self, name: &str, service: subgraph::BoxService) -> subgraph::BoxService { const FUNCTION_NAME_SERVICE: &str = "subgraph_service"; if !self.ast_has_function(FUNCTION_NAME_SERVICE) { return service; @@ -429,10 +401,10 @@ impl Plugin for Rhai { #[derive(Clone, Debug)] pub(crate) enum ServiceStep { - Router(SharedMut), - QueryPlanner(SharedMut), - Execution(SharedMut), - Subgraph(SharedMut), + Router(SharedMut), + QueryPlanner(SharedMut), + Execution(SharedMut), + Subgraph(SharedMut), } // Actually use the checkpoint function so that we can shortcut requests which fail diff --git a/apollo-router/src/plugins/telemetry/mod.rs b/apollo-router/src/plugins/telemetry/mod.rs index a13cdec17f..1c668f131f 100644 --- a/apollo-router/src/plugins/telemetry/mod.rs +++ b/apollo-router/src/plugins/telemetry/mod.rs @@ -35,7 +35,6 @@ use opentelemetry::KeyValue; use router_bridge::planner::UsageReporting; use tower::service_fn; use tower::steer::Steer; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -68,11 +67,13 @@ use crate::plugins::telemetry::metrics::MetricsExporterHandle; use crate::plugins::telemetry::tracing::TracingConfigurator; use crate::query_planner::USAGE_REPORTING; use crate::register_plugin; +use crate::stages::execution; +use crate::stages::query_planner; +use crate::stages::router; +use crate::stages::subgraph; use crate::Context; use crate::ExecutionRequest; -use crate::ExecutionResponse; use crate::QueryPlannerRequest; -use crate::QueryPlannerResponse; use crate::RouterRequest; use crate::RouterResponse; use crate::SubgraphRequest; @@ -164,10 +165,7 @@ impl Plugin for Telemetry { Self::new_common::(init.config, None).await } - fn router_service( - &self, - service: BoxService, - ) -> BoxService { + fn router_service(&self, service: router::BoxService) -> router::BoxService { let metrics_sender = self.apollo_metrics_sender.clone(); let metrics = BasicMetrics::new(&self.meter_provider); let config = Arc::new(self.config.clone()); @@ -261,8 +259,8 @@ impl Plugin for Telemetry { fn query_planning_service( &self, - service: BoxService, - ) -> BoxService { + service: query_planner::BoxService, + ) -> query_planner::BoxService { ServiceBuilder::new() .instrument(move |req: &QueryPlannerRequest| { let query = req.query.clone(); @@ -278,10 +276,7 @@ impl Plugin for Telemetry { .boxed() } - fn execution_service( - &self, - service: BoxService, - ) -> BoxService { + fn execution_service(&self, service: execution::BoxService) -> execution::BoxService { ServiceBuilder::new() .instrument(move |req: &ExecutionRequest| { let query = req @@ -306,11 +301,7 @@ impl Plugin for Telemetry { .boxed() } - fn subgraph_service( - &self, - name: &str, - service: BoxService, - ) -> BoxService { + fn subgraph_service(&self, name: &str, service: subgraph::BoxService) -> subgraph::BoxService { let metrics = BasicMetrics::new(&self.meter_provider); let subgraph_attribute = KeyValue::new("subgraph", name.to_string()); let name = name.to_owned(); diff --git a/apollo-router/src/plugins/traffic_shaping/mod.rs b/apollo-router/src/plugins/traffic_shaping/mod.rs index 8dfe5708ac..93f0a32c6b 100644 --- a/apollo-router/src/plugins/traffic_shaping/mod.rs +++ b/apollo-router/src/plugins/traffic_shaping/mod.rs @@ -23,7 +23,6 @@ use http::header::CONTENT_ENCODING; use http::HeaderValue; use schemars::JsonSchema; use serde::Deserialize; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -39,12 +38,11 @@ use crate::plugin::PluginInit; use crate::plugins::traffic_shaping::deduplication::QueryDeduplicationLayer; use crate::register_plugin; use crate::services::subgraph_service::Compression; -use crate::services::RouterRequest; -use crate::services::RouterResponse; +use crate::stages::query_planner; +use crate::stages::router; +use crate::stages::subgraph; use crate::QueryPlannerRequest; -use crate::QueryPlannerResponse; use crate::SubgraphRequest; -use crate::SubgraphResponse; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(30); trait Merge { @@ -175,10 +173,7 @@ impl Plugin for TrafficShaping { }) } - fn router_service( - &self, - service: BoxService, - ) -> BoxService { + fn router_service(&self, service: router::BoxService) -> router::BoxService { ServiceBuilder::new() .layer(TimeoutLayer::new( self.config @@ -192,11 +187,7 @@ impl Plugin for TrafficShaping { .boxed() } - fn subgraph_service( - &self, - name: &str, - service: BoxService, - ) -> BoxService { + fn subgraph_service(&self, name: &str, service: subgraph::BoxService) -> subgraph::BoxService { // Either we have the subgraph config and we merge it with the all config, or we just have the all config or we have nothing. let all_config = self.config.all.as_ref(); let subgraph_config = self.config.subgraphs.get(name); @@ -244,8 +235,8 @@ impl Plugin for TrafficShaping { fn query_planning_service( &self, - service: BoxService, - ) -> BoxService { + service: query_planner::BoxService, + ) -> query_planner::BoxService { if matches!(self.config.variables_deduplication, Some(true)) { service .map_request(|mut req: QueryPlannerRequest| { diff --git a/apollo-router/src/query_planner/mod.rs b/apollo-router/src/query_planner/mod.rs index ad56e0b50a..0bb9196ecb 100644 --- a/apollo-router/src/query_planner/mod.rs +++ b/apollo-router/src/query_planner/mod.rs @@ -5,7 +5,6 @@ use std::sync::Arc; pub(crate) use bridge_query_planner::*; pub(crate) use caching_query_planner::*; -pub use fetch::OperationKind; use futures::future::join_all; use futures::prelude::*; use opentelemetry::trace::SpanKind; @@ -16,6 +15,7 @@ use tokio::sync::broadcast::Sender; use tokio_stream::wrappers::BroadcastStream; use tracing::Instrument; +pub use self::fetch::OperationKind; use crate::error::Error; use crate::graphql::Request; use crate::graphql::Response; diff --git a/apollo-router/src/services/execution_service.rs b/apollo-router/src/services/execution_service.rs index 2ccef14fc9..7a1aab03eb 100644 --- a/apollo-router/src/services/execution_service.rs +++ b/apollo-router/src/services/execution_service.rs @@ -8,7 +8,6 @@ use futures::future::BoxFuture; use futures::stream::once; use futures::stream::BoxStream; use futures::StreamExt; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -20,6 +19,7 @@ use super::new_service::NewService; use super::subgraph_service::SubgraphServiceFactory; use super::Plugins; use crate::graphql::Response; +use crate::stages::execution; use crate::ExecutionRequest; use crate::ExecutionResponse; use crate::Schema; @@ -105,7 +105,7 @@ impl NewService for ExecutionCreator where SF: SubgraphServiceFactory, { - type Service = BoxService; + type Service = execution::BoxService; fn new_service(&self) -> Self::Service { ServiceBuilder::new() @@ -125,7 +125,7 @@ where } impl ExecutionServiceFactory for ExecutionCreator { - type ExecutionService = BoxService; + type ExecutionService = execution::BoxService; type Future = < as NewService>::Service as Service< ExecutionRequest, >>::Future; diff --git a/apollo-router/src/services/mod.rs b/apollo-router/src/services/mod.rs index f255f8a759..fb50504b8d 100644 --- a/apollo-router/src/services/mod.rs +++ b/apollo-router/src/services/mod.rs @@ -31,7 +31,7 @@ use crate::json_ext::Value; use crate::query_planner::fetch::OperationKind; use crate::query_planner::QueryPlan; use crate::query_planner::QueryPlanOptions; -pub use crate::spec::Query; +use crate::spec::Query; use crate::Context; mod execution_service; diff --git a/apollo-router/src/services/router_service.rs b/apollo-router/src/services/router_service.rs index a4fe94e72e..423c0aa520 100644 --- a/apollo-router/src/services/router_service.rs +++ b/apollo-router/src/services/router_service.rs @@ -40,6 +40,7 @@ use crate::query_planner::CachingQueryPlanner; use crate::router_factory::RouterServiceFactory; use crate::services::layers::apq::APQLayer; use crate::services::layers::ensure_query_presence::EnsureQueryPresence; +use crate::stages::query_planner; use crate::Configuration; use crate::ExecutionRequest; use crate::ExecutionResponse; @@ -362,12 +363,8 @@ impl PluggableRouterServiceBuilder { /// A collection of services and data which may be used to create a "router". #[derive(Clone)] pub(crate) struct RouterCreator { - query_planner_service: CachingQueryPlanner< - Buffer< - BoxService, - QueryPlannerRequest, - >, - >, + query_planner_service: + CachingQueryPlanner>, subgraph_creator: Arc, schema: Arc, plugins: Arc, diff --git a/apollo-router/src/stages.rs b/apollo-router/src/stages.rs new file mode 100644 index 0000000000..a8a755cd1e --- /dev/null +++ b/apollo-router/src/stages.rs @@ -0,0 +1,43 @@ +//! The four stages of handling a GraphQL request + +use tower::BoxError; + +pub mod router { + use super::*; + pub use crate::services::RouterRequest as Request; + pub use crate::services::RouterResponse as Response; + pub type BoxService = tower::util::BoxService; + pub type BoxCloneService = tower::util::BoxCloneService; +} + +pub mod query_planner { + use super::*; + pub use crate::services::QueryPlannerRequest as Request; + pub use crate::services::QueryPlannerResponse as Response; + pub type BoxService = tower::util::BoxService; + pub type BoxCloneService = tower::util::BoxCloneService; + + // Reachable from Request or Response: + pub use crate::query_planner::QueryPlan; + pub use crate::services::QueryPlannerContent; + pub use crate::spec::Query; +} + +pub mod execution { + use super::*; + pub use crate::services::ExecutionRequest as Request; + pub use crate::services::ExecutionResponse as Response; + pub type BoxService = tower::util::BoxService; + pub type BoxCloneService = tower::util::BoxCloneService; +} + +pub mod subgraph { + use super::*; + pub use crate::services::SubgraphRequest as Request; + pub use crate::services::SubgraphResponse as Response; + pub type BoxService = tower::util::BoxService; + pub type BoxCloneService = tower::util::BoxCloneService; + + // Reachable from Request or Response: + pub use crate::query_planner::OperationKind; +} diff --git a/apollo-router/src/test_harness.rs b/apollo-router/src/test_harness.rs index 284103bf71..5a18c5d807 100644 --- a/apollo-router/src/test_harness.rs +++ b/apollo-router/src/test_harness.rs @@ -1,7 +1,6 @@ use std::sync::Arc; use tower::util::BoxCloneService; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceExt; @@ -14,8 +13,7 @@ use crate::router_factory::RouterServiceConfigurator; use crate::router_factory::YamlRouterServiceFactory; use crate::services::RouterRequest; use crate::services::RouterResponse; -use crate::services::SubgraphRequest; -use crate::services::SubgraphResponse; +use crate::stages::subgraph; use crate::Schema; /// Builder for the part of an Apollo Router that handles GraphQL requests, as a [`tower::Service`]. @@ -38,13 +36,13 @@ use crate::Schema; /// Example making a single request: /// /// ``` -/// use apollo_router::services::RouterRequest; +/// use apollo_router::stages::router; /// use apollo_router::TestHarness; /// use tower::util::ServiceExt; /// /// # #[tokio::main] async fn main() -> Result<(), tower::BoxError> { /// let config = serde_json::json!({"server": {"introspection": false}}); -/// let request = RouterRequest::fake_builder() +/// let request = router::Request::fake_builder() /// // Request building here /// .build() /// .unwrap(); @@ -165,8 +163,8 @@ impl Plugin for CannedSubgraphResponses { fn subgraph_service( &self, subgraph_name: &str, - default: BoxService, - ) -> BoxService { + default: subgraph::BoxService, + ) -> subgraph::BoxService { match subgraph_name { "products" => canned::products_subgraph().boxed(), "accounts" => canned::accounts_subgraph().boxed(), diff --git a/apollo-router/tests/integration_tests.rs b/apollo-router/tests/integration_tests.rs index 3831b0866e..25fae2c71b 100644 --- a/apollo-router/tests/integration_tests.rs +++ b/apollo-router/tests/integration_tests.rs @@ -9,14 +9,12 @@ use std::sync::Mutex; use apollo_router::graphql; use apollo_router::graphql::Request; +use apollo_router::http_ext; use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::plugins::telemetry::Telemetry; -use apollo_router::services::http_ext; -use apollo_router::services::RouterRequest; -use apollo_router::services::RouterResponse; -use apollo_router::services::SubgraphRequest; -use apollo_router::services::SubgraphResponse; +use apollo_router::stages::router; +use apollo_router::stages::subgraph; use apollo_router::Context; use http::Method; use maplit::hashmap; @@ -26,8 +24,6 @@ use serde_json_bytes::ByteString; use serde_json_bytes::Map; use serde_json_bytes::Value; use test_span::prelude::*; -use tower::util::BoxCloneService; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceExt; use tracing_subscriber::prelude::__tracing_subscriber_SubscriberExt; @@ -271,7 +267,7 @@ async fn queries_should_work_with_compression() { .build() .expect("expecting valid request"); - let request = RouterRequest { + let request = router::Request { originating_request: http_request, context: Context::new(), }; @@ -305,7 +301,7 @@ async fn queries_should_work_over_post() { .build() .expect("expecting valid request"); - let request = RouterRequest { + let request = router::Request { originating_request: http_request, context: Context::new(), }; @@ -417,7 +413,7 @@ async fn mutation_should_work_over_post() { .build() .expect("expecting valid request"); - let request = RouterRequest { + let request = router::Request { originating_request: http_request, context: Context::new(), }; @@ -666,13 +662,13 @@ async fn query_node( } async fn query_rust( - request: RouterRequest, + request: router::Request, ) -> (apollo_router::graphql::Response, CountingServiceRegistry) { query_rust_with_config(request, serde_json::json!({})).await } async fn query_rust_with_config( - request: RouterRequest, + request: router::Request, config: serde_json::Value, ) -> (apollo_router::graphql::Response, CountingServiceRegistry) { let (router, counting_registry) = setup_router_and_registry(config).await; @@ -681,10 +677,7 @@ async fn query_rust_with_config( async fn setup_router_and_registry( config: serde_json::Value, -) -> ( - BoxCloneService, - CountingServiceRegistry, -) { +) -> (router::BoxCloneService, CountingServiceRegistry) { let config = serde_json::from_value(config).unwrap(); let counting_registry = CountingServiceRegistry::new(); let telemetry = Telemetry::new_with_subscriber( @@ -711,8 +704,8 @@ async fn setup_router_and_registry( } async fn query_with_router( - router: BoxCloneService, - request: RouterRequest, + router: router::BoxCloneService, + request: router::Request, ) -> graphql::Response { router .oneshot(request) @@ -763,8 +756,8 @@ impl Plugin for CountingServiceRegistry { fn subgraph_service( &self, subgraph_name: &str, - service: BoxService, - ) -> BoxService { + service: subgraph::BoxService, + ) -> subgraph::BoxService { let name = subgraph_name.to_owned(); let counters = self.clone(); service diff --git a/apollo-router/tests/rhai_tests.rs b/apollo-router/tests/rhai_tests.rs index 59ad9fd57c..5b7943ff5a 100644 --- a/apollo-router/tests/rhai_tests.rs +++ b/apollo-router/tests/rhai_tests.rs @@ -1,4 +1,4 @@ -use apollo_router::services::RouterRequest; +use apollo_router::stages::router; use apollo_router::TestHarness; use tower::ServiceExt; @@ -25,7 +25,7 @@ async fn all_rhai_callbacks_are_invoked() { .build() .await .unwrap(); - let request = RouterRequest::fake_builder() + let request = router::Request::fake_builder() .query("{ topProducts { name } }") .build() .unwrap(); diff --git a/docs/source/customizations/native.mdx b/docs/source/customizations/native.mdx index fb45343dae..06ed845a7b 100644 --- a/docs/source/customizations/native.mdx +++ b/docs/source/customizations/native.mdx @@ -76,6 +76,8 @@ The trait also provides a default implementations for each hook, which returns t ```rust title="hello_world.rs" // This is a bare-bones plugin that you can duplicate when creating your own. use apollo_router::plugin::PluginInit; +use apollo_router::plugin::Plugin; +use apollo_router::stages::*; #[async_trait::async_trait] impl Plugin for HelloWorld { @@ -95,22 +97,22 @@ impl Plugin for HelloWorld { // implementation returns its associated service with no changes. fn router_service( &mut self, - service: BoxService, - ) -> BoxService { + service: router::BoxService, + ) -> router::BoxService { service } fn query_planning_service( &mut self, - service: BoxService, - ) -> BoxService { + service: query_planner::BoxService, + ) -> query_planner::BoxService { service } fn execution_service( &mut self, - service: BoxService, - ) -> BoxService { + service: execution::BoxService, + ) -> execution::BoxService { service } @@ -120,8 +122,8 @@ impl Plugin for HelloWorld { fn subgraph_service( &mut self, name: &str, - service: BoxService, - ) -> BoxService { + service: subgraph::BoxService, + ) -> subgraph::BoxService { service } } @@ -140,8 +142,8 @@ use apollo_router::ServiceBuilderExt as ApolloServiceBuilderExt; fn router_service( &mut self, - service: BoxService, -) -> BoxService { + service: router::BoxService, +) -> router::BoxService { // Always use service builder to compose your plugins. // It provides off-the-shelf building blocks for your plugin. ServiceBuilder::new() diff --git a/examples/add-timestamp-header/src/main.rs b/examples/add-timestamp-header/src/main.rs index a48b84af18..27747ae015 100644 --- a/examples/add-timestamp-header/src/main.rs +++ b/examples/add-timestamp-header/src/main.rs @@ -17,8 +17,7 @@ mod tests { use apollo_router::plugin::PluginInit; use apollo_router::plugins::rhai::Conf; use apollo_router::plugins::rhai::Rhai; - use apollo_router::services::RouterRequest; - use apollo_router::services::RouterResponse; + use apollo_router::stages::router; use http::StatusCode; use tower::util::ServiceExt; @@ -34,9 +33,9 @@ mod tests { mock_service .expect_call() .once() - .returning(move |req: RouterRequest| { + .returning(move |req: router::Request| { // Preserve our context from request to response - Ok(RouterResponse::fake_builder() + Ok(router::Response::fake_builder() .context(req.context) .data(expected_mock_response_data) .build() @@ -57,7 +56,7 @@ mod tests { let service_stack = rhai.router_service(mock_service.boxed()); // Let's create a request with our operation name - let request_with_appropriate_name = RouterRequest::fake_builder() + let request_with_appropriate_name = router::Request::fake_builder() .operation_name("me".to_string()) .build() .unwrap(); diff --git a/examples/async-auth/README.md b/examples/async-auth/README.md index cf008c7dee..dc00e01b42 100644 --- a/examples/async-auth/README.md +++ b/examples/async-auth/README.md @@ -16,8 +16,8 @@ authentication server. ```rust fn router_service( &mut self, - service: BoxService, - ) -> BoxService { + service: router::BoxService, + ) -> router::BoxService { ServiceBuilder::new() .checkpoint_async(...) // Authentication happens here .buffer(20_000) // Required, see note below diff --git a/examples/async-auth/src/allow_client_id_from_file.rs b/examples/async-auth/src/allow_client_id_from_file.rs index c2c4d24c45..755365f7bc 100644 --- a/examples/async-auth/src/allow_client_id_from_file.rs +++ b/examples/async-auth/src/allow_client_id_from_file.rs @@ -6,13 +6,11 @@ use apollo_router::layers::ServiceBuilderExt; use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::register_plugin; -use apollo_router::services::RouterRequest; -use apollo_router::services::RouterResponse; +use apollo_router::stages::router; use http::StatusCode; use schemars::JsonSchema; use serde::Deserialize; use serde_json_bytes::Value; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -48,10 +46,7 @@ impl Plugin for AllowClientIdFromFile { // While this is not the most performant and efficient usecase, // We could easily change the place where the file list is stored, // switching the async file read with an async http request - fn router_service( - &self, - service: BoxService, - ) -> BoxService { + fn router_service(&self, service: router::BoxService) -> router::BoxService { let header_key = self.header.clone(); // async_checkpoint is an async function. // this means it will run whenever the service `await`s it @@ -63,14 +58,14 @@ impl Plugin for AllowClientIdFromFile { // see https://rust-lang.github.io/async-book/03_async_await/01_chapter.html#async-lifetimes for more information let allowed_ids_path = self.allowed_ids_path.clone(); - let handler = move |req: RouterRequest| { + let handler = move |req: router::Request| { // If we set a res, then we are going to break execution // If not, we are continuing let mut res = None; if !req.originating_request.headers().contains_key(&header_key) { // Prepare an HTTP 401 response with a GraphQL error message res = Some( - RouterResponse::error_builder() + router::Response::error_builder() .error(graphql::Error { message: format!("Missing '{header_key}' header"), ..Default::default() @@ -103,7 +98,7 @@ impl Plugin for AllowClientIdFromFile { if !allowed_clients.contains(&client_id.to_string()) { // Prepare an HTTP 403 response with a GraphQL error message res = Some( - RouterResponse::builder() + router::Response::builder() .data(Value::default()) .error(graphql::Error { message: "client-id is not allowed".to_string(), @@ -119,7 +114,7 @@ impl Plugin for AllowClientIdFromFile { Err(_not_a_string_error) => { // Prepare an HTTP 400 response with a GraphQL error message res = Some( - RouterResponse::error_builder() + router::Response::error_builder() .error(graphql::Error { message: format!("'{header_key}' value is not a string"), ..Default::default() @@ -180,8 +175,7 @@ mod tests { use apollo_router::plugin::test; use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; - use apollo_router::services::RouterRequest; - use apollo_router::services::RouterResponse; + use apollo_router::stages::router; use http::StatusCode; use serde_json::json; use tower::ServiceExt; @@ -228,7 +222,7 @@ mod tests { .router_service(mock_service.boxed()); // Let's create a request without a client id... - let request_without_client_id = RouterRequest::fake_builder() + let request_without_client_id = router::Request::fake_builder() .build() .expect("expecting valid request"); @@ -272,7 +266,7 @@ mod tests { .router_service(mock_service.boxed()); // Let's create a request with a not allowed client id... - let request_with_unauthorized_client_id = RouterRequest::fake_builder() + let request_with_unauthorized_client_id = router::Request::fake_builder() .header("x-client-id", "invalid_client_id") .build() .expect("expecting valid request"); @@ -309,7 +303,7 @@ mod tests { mock_service .expect_call() .times(1) - .returning(move |req: RouterRequest| { + .returning(move |req: router::Request| { assert_eq!( valid_client_id, // we're ok with unwrap's here because we're running a test @@ -322,7 +316,7 @@ mod tests { .unwrap() ); // let's return the expected data - Ok(RouterResponse::fake_builder() + Ok(router::Response::fake_builder() .data(expected_mock_response_data) .build() .unwrap()) @@ -342,7 +336,7 @@ mod tests { .router_service(mock_service.boxed()); // Let's create a request with an valid client id... - let request_with_valid_client_id = RouterRequest::fake_builder() + let request_with_valid_client_id = router::Request::fake_builder() .header("x-client-id", valid_client_id) .build() .expect("expecting valid request"); diff --git a/examples/context/src/context_data.rs b/examples/context/src/context_data.rs index be681c1304..471c2c826d 100644 --- a/examples/context/src/context_data.rs +++ b/examples/context/src/context_data.rs @@ -1,12 +1,9 @@ use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::register_plugin; -use apollo_router::services::RouterRequest; -use apollo_router::services::RouterResponse; -use apollo_router::services::SubgraphRequest; -use apollo_router::services::SubgraphResponse; +use apollo_router::stages::router; +use apollo_router::stages::subgraph; use http::StatusCode; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -44,15 +41,12 @@ impl Plugin for ContextData { Ok(Self::default()) } - fn router_service( - &self, - service: BoxService, - ) -> BoxService { + fn router_service(&self, service: router::BoxService) -> router::BoxService { // `ServiceBuilder` provides us with `map_request` and `map_response` methods. // // These allow basic interception and transformation of request and response messages. ServiceBuilder::new() - .map_request(|req: RouterRequest| { + .map_request(|req: router::Request| { // Populate a value in context for use later. // Context values must be serializable to serde_json::Value. if let Err(e) = req.context.insert("incoming_data", "world!".to_string()) { @@ -74,13 +68,9 @@ impl Plugin for ContextData { .boxed() } - fn subgraph_service( - &self, - _name: &str, - service: BoxService, - ) -> BoxService { + fn subgraph_service(&self, _name: &str, service: subgraph::BoxService) -> subgraph::BoxService { ServiceBuilder::new() - .map_request(|req: SubgraphRequest| { + .map_request(|req: subgraph::Request| { // Pick up a value from the context that was populated earlier. if let Ok(Some(data)) = req.context.get::<_, String>("incoming_data") { tracing::info!("hello {}", data); // Hello world! @@ -88,7 +78,7 @@ impl Plugin for ContextData { req }) .service(service) - .map_response(|mut resp: SubgraphResponse| { + .map_response(|mut resp: subgraph::Response| { // A single context is created for the entire request. // We use upsert because there may be multiple downstream subgraph requests. // Upserts are guaranteed to be applied serially. diff --git a/examples/cookies-to-headers/src/main.rs b/examples/cookies-to-headers/src/main.rs index 55250d9212..097c6a0b4c 100644 --- a/examples/cookies-to-headers/src/main.rs +++ b/examples/cookies-to-headers/src/main.rs @@ -32,14 +32,13 @@ fn main() -> Result<()> { #[cfg(test)] mod tests { + use apollo_router::http_ext; use apollo_router::plugin::test; use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::plugins::rhai::Conf; use apollo_router::plugins::rhai::Rhai; - use apollo_router::services::http_ext; - use apollo_router::services::SubgraphRequest; - use apollo_router::services::SubgraphResponse; + use apollo_router::stages::subgraph; use http::header::HeaderName; use http::HeaderValue; use http::StatusCode; @@ -57,7 +56,7 @@ mod tests { mock_service .expect_call() .once() - .returning(move |req: SubgraphRequest| { + .returning(move |req: subgraph::Request| { // Let's make sure our request contains our new headers assert_eq!( req.subgraph_request @@ -73,7 +72,7 @@ mod tests { .expect("tasty_cookie is present"), "strawberry" ); - Ok(SubgraphResponse::fake_builder() + Ok(subgraph::Response::fake_builder() .data(expected_mock_response_data) .build()) }); @@ -115,7 +114,7 @@ mod tests { } // Let's create a request with our cookies - let request_with_appropriate_cookies = SubgraphRequest::fake_builder() + let request_with_appropriate_cookies = subgraph::Request::fake_builder() .originating_request(std::sync::Arc::new(originating_request)) .subgraph_request(sub_request) .build(); diff --git a/examples/embedded/src/main.rs b/examples/embedded/src/main.rs index 210408ccdd..b97185c6ff 100644 --- a/examples/embedded/src/main.rs +++ b/examples/embedded/src/main.rs @@ -1,4 +1,4 @@ -use apollo_router::services::RouterRequest; +use apollo_router::stages::router; use apollo_router::TestHarness; use tower::ServiceExt; @@ -12,7 +12,7 @@ async fn main() -> Result<(), tower::BoxError> { .await?; // ...then create a GraphQL request... - let request = RouterRequest::fake_builder() + let request = router::Request::fake_builder() .query(r#"query Query { me { name } }"#) .build() .expect("expecting valid request"); diff --git a/examples/forbid-anonymous-operations/README.md b/examples/forbid-anonymous-operations/README.md index 2b38f3ef15..babfc1406e 100644 --- a/examples/forbid-anonymous-operations/README.md +++ b/examples/forbid-anonymous-operations/README.md @@ -14,8 +14,8 @@ cargo run -- -s ../graphql/supergraph.graphql -c ./router.yaml ```rust fn router_service( &mut self, - service: BoxService, - ) -> BoxService { + service: router::BoxService, + ) -> router::BoxService { ServiceBuilder::new() .checkpoint(...) // Validation happens here .service(service) diff --git a/examples/forbid-anonymous-operations/src/forbid_anonymous_operations.rs b/examples/forbid-anonymous-operations/src/forbid_anonymous_operations.rs index a3d190fff9..8e99631f8f 100644 --- a/examples/forbid-anonymous-operations/src/forbid_anonymous_operations.rs +++ b/examples/forbid-anonymous-operations/src/forbid_anonymous_operations.rs @@ -5,10 +5,8 @@ use apollo_router::layers::ServiceBuilderExt; use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::register_plugin; -use apollo_router::services::RouterRequest; -use apollo_router::services::RouterResponse; +use apollo_router::stages::router; use http::StatusCode; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -33,16 +31,13 @@ impl Plugin for ForbidAnonymousOperations { // Forbidding anonymous operations can happen at the very beginning of our GraphQL request lifecycle. // We will thus put the logic it in the `router_service` section of our plugin. - fn router_service( - &self, - service: BoxService, - ) -> BoxService { + fn router_service(&self, service: router::BoxService) -> router::BoxService { // `ServiceBuilder` provides us with a `checkpoint` method. // // This method allows us to return ControlFlow::Continue(request) if we want to let the request through, // or ControlFlow::Return(response) with a crafted response if we don't want the request to go through. ServiceBuilder::new() - .checkpoint(|req: RouterRequest| { + .checkpoint(|req: router::Request| { // The http_request is stored in a `RouterRequest` context. // Its `body()` is an `apollo_router::Request`, that contains: // - Zero or one query @@ -59,7 +54,7 @@ impl Plugin for ForbidAnonymousOperations { tracing::error!("Operation is not allowed!"); // Prepare an HTTP 400 response with a GraphQL error message - let res = RouterResponse::error_builder() + let res = router::Response::error_builder() .error(graphql::Error { message: "Anonymous operations are not allowed".to_string(), ..Default::default() @@ -99,8 +94,7 @@ mod tests { use apollo_router::graphql; use apollo_router::plugin::test; use apollo_router::plugin::Plugin; - use apollo_router::services::RouterRequest; - use apollo_router::services::RouterResponse; + use apollo_router::stages::router; use http::StatusCode; use serde_json::Value; use tower::ServiceExt; @@ -134,7 +128,7 @@ mod tests { ForbidAnonymousOperations::default().router_service(mock_service.boxed()); // Let's create a request without an operation name... - let request_without_any_operation_name = RouterRequest::fake_builder() + let request_without_any_operation_name = router::Request::fake_builder() .build() .expect("expecting valid request"); @@ -169,7 +163,7 @@ mod tests { ForbidAnonymousOperations::default().router_service(mock_service.boxed()); // Let's create a request with an empty operation name... - let request_with_empty_operation_name = RouterRequest::fake_builder() + let request_with_empty_operation_name = router::Request::fake_builder() .operation_name("".to_string()) .build() .expect("expecting valid request"); @@ -206,7 +200,7 @@ mod tests { mock_service .expect_call() .times(1) - .returning(move |req: RouterRequest| { + .returning(move |req: router::Request| { assert_eq!( operation_name, // we're ok with unwrap's here because we're running a test @@ -218,7 +212,7 @@ mod tests { .unwrap() ); // let's return the expected data - Ok(RouterResponse::fake_builder() + Ok(router::Response::fake_builder() .data(expected_mock_response_data) .build() .unwrap()) @@ -229,7 +223,7 @@ mod tests { ForbidAnonymousOperations::default().router_service(mock_service.boxed()); // Let's create a request with an valid operation name... - let request_with_operation_name = RouterRequest::fake_builder() + let request_with_operation_name = router::Request::fake_builder() .operation_name(operation_name) .build() .expect("expecting valid request"); diff --git a/examples/hello-world/src/hello_world.rs b/examples/hello-world/src/hello_world.rs index 0b71f1f919..eb71458f0c 100644 --- a/examples/hello-world/src/hello_world.rs +++ b/examples/hello-world/src/hello_world.rs @@ -1,17 +1,12 @@ use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::register_plugin; -use apollo_router::services::ExecutionRequest; -use apollo_router::services::ExecutionResponse; -use apollo_router::services::QueryPlannerRequest; -use apollo_router::services::QueryPlannerResponse; -use apollo_router::services::RouterRequest; -use apollo_router::services::RouterResponse; -use apollo_router::services::SubgraphRequest; -use apollo_router::services::SubgraphResponse; +use apollo_router::stages::execution; +use apollo_router::stages::query_planner; +use apollo_router::stages::router; +use apollo_router::stages::subgraph; use schemars::JsonSchema; use serde::Deserialize; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -39,10 +34,7 @@ impl Plugin for HelloWorld { }) } - fn router_service( - &self, - service: BoxService, - ) -> BoxService { + fn router_service(&self, service: router::BoxService) -> router::BoxService { // Say hello when our service is added to the router_service // stage of the router plugin pipeline. #[cfg(test)] @@ -63,28 +55,21 @@ impl Plugin for HelloWorld { fn query_planning_service( &self, - service: BoxService, - ) -> BoxService { + service: query_planner::BoxService, + ) -> query_planner::BoxService { // This is the default implementation and does not modify the default service. // The trait also has this implementation, and we just provide it here for illustration. service } - fn execution_service( - &self, - service: BoxService, - ) -> BoxService { + fn execution_service(&self, service: execution::BoxService) -> execution::BoxService { //This is the default implementation and does not modify the default service. // The trait also has this implementation, and we just provide it here for illustration. service } // Called for each subgraph - fn subgraph_service( - &self, - _name: &str, - service: BoxService, - ) -> BoxService { + fn subgraph_service(&self, _name: &str, service: subgraph::BoxService) -> subgraph::BoxService { // Always use service builder to compose your plugins. // It provides off the shelf building blocks for your plugin. ServiceBuilder::new() diff --git a/examples/jwt-auth/README.md b/examples/jwt-auth/README.md index 34e9c80bf1..76e8a9c06b 100644 --- a/examples/jwt-auth/README.md +++ b/examples/jwt-auth/README.md @@ -17,8 +17,8 @@ cargo run -- -s ../graphql/supergraph.graphql -c ./router.yaml ```rust fn router_service( &mut self, - service: BoxService, - ) -> BoxService { + service: router::BoxService, + ) -> router::BoxService { ServiceBuilder::new() .checkpoint(...) // Validation happens here .service(service) diff --git a/examples/jwt-auth/src/jwt.rs b/examples/jwt-auth/src/jwt.rs index 998e553fb7..4b6c555c00 100644 --- a/examples/jwt-auth/src/jwt.rs +++ b/examples/jwt-auth/src/jwt.rs @@ -68,8 +68,7 @@ use apollo_router::layers::ServiceBuilderExt; use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::register_plugin; -use apollo_router::services::RouterRequest; -use apollo_router::services::RouterResponse; +use apollo_router::stages::router; use apollo_router::Context; use http::header::AUTHORIZATION; use http::StatusCode; @@ -79,7 +78,6 @@ use schemars::JsonSchema; use serde::de; use serde::Deserialize; use strum_macros::EnumString; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -211,10 +209,7 @@ impl Plugin for JwtAuth { }) } - fn router_service( - &self, - service: BoxService, - ) -> BoxService { + fn router_service(&self, service: router::BoxService) -> router::BoxService { // We are going to use the `jwt-simple` crate for our JWT verification. // The crate provides straightforward support for the popular JWT algorithms. @@ -232,15 +227,15 @@ impl Plugin for JwtAuth { let max_token_life = self.max_token_life; ServiceBuilder::new() - .checkpoint(move |req: RouterRequest| { + .checkpoint(move |req: router::Request| { // We are going to do a lot of similar checking so let's define a local function // to help reduce repetition fn failure_message( context: Context, msg: String, status: StatusCode, - ) -> Result, BoxError> { - let res = RouterResponse::error_builder() + ) -> Result, BoxError> { + let res = router::Response::error_builder() .errors(vec![graphql::Error { message: msg, ..Default::default() @@ -251,7 +246,7 @@ impl Plugin for JwtAuth { Ok(ControlFlow::Break(res)) } - // The http_request is stored in a `RouterRequest` context. + // The http_request is stored in a `Router::Request` context. // We are going to check the headers for the presence of the header we're looking for // We are implementing: https://www.rfc-editor.org/rfc/rfc6750 // so check for our AUTHORIZATION header. @@ -393,8 +388,7 @@ mod tests { use apollo_router::graphql; use apollo_router::plugin::test; use apollo_router::plugin::Plugin; - use apollo_router::services::RouterRequest; - use apollo_router::services::RouterResponse; + use apollo_router::stages::router; use super::*; @@ -424,7 +418,7 @@ mod tests { let service_stack = JwtAuth::default().router_service(mock_service.boxed()); // Let's create a request without an authorization header - let request_without_any_authorization_header = RouterRequest::fake_builder() + let request_without_any_authorization_header = router::Request::fake_builder() .build() .expect("expecting valid request"); @@ -458,7 +452,7 @@ mod tests { let service_stack = JwtAuth::default().router_service(mock_service.boxed()); // Let's create a request with a badly formatted authorization header - let request_with_no_bearer_in_auth = RouterRequest::fake_builder() + let request_with_no_bearer_in_auth = router::Request::fake_builder() .header("authorization", "should start with Bearer") .build() .expect("expecting valid request"); @@ -493,7 +487,7 @@ mod tests { let service_stack = JwtAuth::default().router_service(mock_service.boxed()); // Let's create a request with a badly formatted authorization header - let request_with_too_many_spaces_in_auth = RouterRequest::fake_builder() + let request_with_too_many_spaces_in_auth = router::Request::fake_builder() .header("authorization", "Bearer ") .build() .expect("expecting valid request"); @@ -529,7 +523,7 @@ mod tests { // Let's create a request with a properly formatted authorization header // Note: (The token isn't valid, but the format is...) - let request_with_appropriate_auth = RouterRequest::fake_builder() + let request_with_appropriate_auth = router::Request::fake_builder() .header("authorization", "Bearer atoken") .build() .expect("expecting valid request"); @@ -567,7 +561,7 @@ mod tests { mock_service .expect_call() .once() - .returning(move |req: RouterRequest| { + .returning(move |req: router::Request| { // Let's make sure our request contains (some of) our JWTClaims let claims: JWTClaims = req .context @@ -578,7 +572,7 @@ mod tests { assert_eq!(claims.subject, Some("subject".to_string())); assert_eq!(claims.jwt_id, Some("jwt_id".to_string())); assert_eq!(claims.nonce, Some("nonce".to_string())); - Ok(RouterResponse::fake_builder() + Ok(router::Response::fake_builder() .data(expected_mock_response_data) .build() .expect("expecting valid request")) @@ -612,7 +606,7 @@ mod tests { let token = verifier.authenticate(claims).unwrap(); // Let's create a request with a properly formatted authorization header - let request_with_appropriate_auth = RouterRequest::fake_builder() + let request_with_appropriate_auth = router::Request::fake_builder() .header("authorization", &format!("Bearer {token}")) .build() .expect("expecting valid request"); @@ -662,7 +656,7 @@ mod tests { let token = verifier.authenticate(claims).unwrap(); // Let's create a request with a properly formatted authorization header - let request_with_appropriate_auth = RouterRequest::fake_builder() + let request_with_appropriate_auth = router::Request::fake_builder() .header("authorization", format!("Bearer {token}")) .build() .expect("expecting valid request"); @@ -717,7 +711,7 @@ mod tests { let token = verifier.authenticate(claims).unwrap(); // Let's create a request with a properly formatted authorization header - let request_with_appropriate_auth = RouterRequest::fake_builder() + let request_with_appropriate_auth = router::Request::fake_builder() .header("authorization", format!("Bearer {token}")) .build() .expect("expecting valid request"); diff --git a/examples/op-name-to-header/src/main.rs b/examples/op-name-to-header/src/main.rs index 3634d27ca9..c2543f417e 100644 --- a/examples/op-name-to-header/src/main.rs +++ b/examples/op-name-to-header/src/main.rs @@ -17,8 +17,7 @@ mod tests { use apollo_router::plugin::PluginInit; use apollo_router::plugins::rhai::Conf; use apollo_router::plugins::rhai::Rhai; - use apollo_router::services::RouterRequest; - use apollo_router::services::RouterResponse; + use apollo_router::stages::router; use http::StatusCode; use tower::util::ServiceExt; @@ -34,7 +33,7 @@ mod tests { mock_service .expect_call() .once() - .returning(move |req: RouterRequest| { + .returning(move |req: router::Request| { // Let's make sure our request contains our new header assert_eq!( req.originating_request @@ -43,7 +42,7 @@ mod tests { .expect("X-operation-name is present"), "me" ); - Ok(RouterResponse::fake_builder() + Ok(router::Response::fake_builder() .data(expected_mock_response_data) .build() .unwrap()) @@ -64,7 +63,7 @@ mod tests { let service_stack = rhai.router_service(mock_service.boxed()); // Let's create a request with our operation name - let request_with_appropriate_name = RouterRequest::fake_builder() + let request_with_appropriate_name = router::Request::fake_builder() .operation_name("me".to_string()) .build() .unwrap(); diff --git a/examples/rhai-data-response-mutate/src/main.rs b/examples/rhai-data-response-mutate/src/main.rs index affafbf1af..4a3c0a2b88 100644 --- a/examples/rhai-data-response-mutate/src/main.rs +++ b/examples/rhai-data-response-mutate/src/main.rs @@ -18,7 +18,7 @@ mod tests { use apollo_router::plugin::PluginInit; use apollo_router::plugins::rhai::Conf; use apollo_router::plugins::rhai::Rhai; - use apollo_router::services::RouterRequest; + use apollo_router::stages::router; use apollo_router::Context; use http::StatusCode; @@ -53,7 +53,7 @@ mod tests { let context: Option = None; let mut service_response = test_harness .call( - RouterRequest::fake_builder() + router::Request::fake_builder() .header("name_header", "test_client") .header("version_header", "1.0-test") .query(query) diff --git a/examples/rhai-error-response-mutate/src/main.rs b/examples/rhai-error-response-mutate/src/main.rs index 1ca244d041..19c81b76d1 100644 --- a/examples/rhai-error-response-mutate/src/main.rs +++ b/examples/rhai-error-response-mutate/src/main.rs @@ -18,7 +18,7 @@ mod tests { use apollo_router::plugin::PluginInit; use apollo_router::plugins::rhai::Conf; use apollo_router::plugins::rhai::Rhai; - use apollo_router::services::RouterRequest; + use apollo_router::stages::router; use apollo_router::Context; use http::StatusCode; @@ -53,7 +53,7 @@ mod tests { let context: Option = None; let mut service_response = test_harness .call( - RouterRequest::fake_builder() + router::Request::fake_builder() .header("name_header", "test_client") .header("version_header", "1.0-test") .query(query) diff --git a/examples/rhai-logging/src/main.rs b/examples/rhai-logging/src/main.rs index 6f9c0c4a38..048723e02f 100644 --- a/examples/rhai-logging/src/main.rs +++ b/examples/rhai-logging/src/main.rs @@ -18,8 +18,7 @@ mod tests { use apollo_router::plugin::PluginInit; use apollo_router::plugins::rhai::Conf; use apollo_router::plugins::rhai::Rhai; - use apollo_router::services::RouterRequest; - use apollo_router::services::RouterResponse; + use apollo_router::stages::router; use http::StatusCode; use tower::util::ServiceExt; @@ -35,8 +34,8 @@ mod tests { mock_service .expect_call() .once() - .returning(move |_req: RouterRequest| { - Ok(RouterResponse::fake_builder() + .returning(move |_req: router::Request| { + Ok(router::Response::fake_builder() .data(expected_mock_response_data) .build() .unwrap()) @@ -56,7 +55,7 @@ mod tests { let service_stack = rhai.router_service(mock_service.boxed()); // Let's create a request with our operation name - let request_with_appropriate_name = RouterRequest::fake_builder() + let request_with_appropriate_name = router::Request::fake_builder() .operation_name("me".to_string()) .build() .unwrap(); diff --git a/examples/rhai-subgraph-request-log/src/main.rs b/examples/rhai-subgraph-request-log/src/main.rs index 6c28ad1060..9a1adeb5d7 100644 --- a/examples/rhai-subgraph-request-log/src/main.rs +++ b/examples/rhai-subgraph-request-log/src/main.rs @@ -18,7 +18,7 @@ mod tests { use apollo_router::plugin::PluginInit; use apollo_router::plugins::rhai::Conf; use apollo_router::plugins::rhai::Rhai; - use apollo_router::services::RouterRequest; + use apollo_router::stages::router; use apollo_router::Context; use http::StatusCode; @@ -53,7 +53,7 @@ mod tests { let context: Option = None; let mut service_response = test_harness .call( - RouterRequest::fake_builder() + router::Request::fake_builder() .header("name_header", "test_client") .header("version_header", "1.0-test") .query(query) diff --git a/examples/rhai-surrogate-cache-key/src/main.rs b/examples/rhai-surrogate-cache-key/src/main.rs index da4fffe229..131cf8a634 100644 --- a/examples/rhai-surrogate-cache-key/src/main.rs +++ b/examples/rhai-surrogate-cache-key/src/main.rs @@ -18,7 +18,7 @@ mod tests { use apollo_router::plugin::PluginInit; use apollo_router::plugins::rhai::Conf; use apollo_router::plugins::rhai::Rhai; - use apollo_router::services::RouterRequest; + use apollo_router::stages::router; use apollo_router::Context; use http::StatusCode; @@ -53,7 +53,7 @@ mod tests { let context: Option = None; let mut service_response = test_harness .call( - RouterRequest::fake_builder() + router::Request::fake_builder() .header("name_header", "test_client") .header("version_header", "1.0-test") .query(query) diff --git a/examples/status-code-propagation/src/propagate_status_code.rs b/examples/status-code-propagation/src/propagate_status_code.rs index c164c2ef7c..de812d54c8 100644 --- a/examples/status-code-propagation/src/propagate_status_code.rs +++ b/examples/status-code-propagation/src/propagate_status_code.rs @@ -1,15 +1,12 @@ use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::register_plugin; -use apollo_router::services::RouterRequest; -use apollo_router::services::RouterResponse; -use apollo_router::services::SubgraphRequest; -use apollo_router::services::SubgraphResponse; +use apollo_router::stages::router; +use apollo_router::stages::subgraph; use http::StatusCode; use schemars::JsonSchema; use serde::Deserialize; use serde::Serialize; -use tower::util::BoxService; use tower::BoxError; use tower::ServiceExt; @@ -36,11 +33,7 @@ impl Plugin for PropagateStatusCode { }) } - fn subgraph_service( - &self, - _name: &str, - service: BoxService, - ) -> BoxService { + fn subgraph_service(&self, _name: &str, service: subgraph::BoxService) -> subgraph::BoxService { let all_status_codes = self.status_codes.clone(); service .map_response(move |res| { @@ -68,10 +61,7 @@ impl Plugin for PropagateStatusCode { } // At this point, all subgraph_services will have pushed their status codes if they match the `watch list`. - fn router_service( - &self, - service: BoxService, - ) -> BoxService { + fn router_service(&self, service: router::BoxService) -> router::BoxService { service .map_response(move |mut res| { if let Some(code) = res @@ -101,10 +91,8 @@ mod tests { use apollo_router::plugin::test; use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; - use apollo_router::services::RouterRequest; - use apollo_router::services::RouterResponse; - use apollo_router::services::SubgraphRequest; - use apollo_router::services::SubgraphResponse; + use apollo_router::stages::router; + use apollo_router::stages::subgraph; use http::StatusCode; use serde_json::json; use tower::ServiceExt; @@ -141,7 +129,7 @@ mod tests { // Return StatusCode::FORBIDDEN, which shall be added to our status_codes mock_service.expect_call().times(1).returning(move |_| { - Ok(SubgraphResponse::fake_builder() + Ok(subgraph::Response::fake_builder() .status_code(StatusCode::FORBIDDEN) .build()) }); @@ -158,7 +146,7 @@ mod tests { .expect("couldn't create plugin") .subgraph_service("accounts", mock_service.boxed()); - let subgraph_request = SubgraphRequest::fake_builder().build(); + let subgraph_request = subgraph::Request::fake_builder().build(); let service_response = service_stack.oneshot(subgraph_request).await.unwrap(); @@ -178,7 +166,7 @@ mod tests { // Return StatusCode::OK, which shall NOT be added to our status_codes mock_service.expect_call().times(1).returning(move |_| { - Ok(SubgraphResponse::fake_builder() + Ok(subgraph::Response::fake_builder() .status_code(StatusCode::OK) .build()) }); @@ -195,7 +183,7 @@ mod tests { .expect("couldn't create plugin") .subgraph_service("accounts", mock_service.boxed()); - let subgraph_request = SubgraphRequest::fake_builder().build(); + let subgraph_request = subgraph::Request::fake_builder().build(); let service_response = service_stack.oneshot(subgraph_request).await.unwrap(); @@ -218,14 +206,14 @@ mod tests { mock_service .expect_call() .times(1) - .returning(move |router_request: RouterRequest| { + .returning(move |router_request: router::Request| { let context = router_request.context; // Insert several status codes which shall override the router response status context .insert(&"status_code".to_string(), json!(500u16)) .expect("couldn't insert status_code"); - Ok(RouterResponse::fake_builder() + Ok(router::Response::fake_builder() .context(context) .build() .unwrap()) @@ -243,7 +231,7 @@ mod tests { .expect("couldn't create plugin") .router_service(mock_service.boxed()); - let router_request = RouterRequest::fake_builder() + let router_request = router::Request::fake_builder() .build() .expect("expecting valid request"); @@ -264,10 +252,10 @@ mod tests { mock_service .expect_call() .times(1) - .returning(move |router_request: RouterRequest| { + .returning(move |router_request: router::Request| { let context = router_request.context; // Don't insert any StatusCode - Ok(RouterResponse::fake_builder() + Ok(router::Response::fake_builder() .context(context) .build() .unwrap()) @@ -285,7 +273,7 @@ mod tests { .expect("couldn't create plugin") .router_service(mock_service.boxed()); - let router_request = RouterRequest::fake_builder() + let router_request = router::Request::fake_builder() .build() .expect("expecting valid request"); diff --git a/examples/supergraph_sdl/src/supergraph_sdl.rs b/examples/supergraph_sdl/src/supergraph_sdl.rs index c2cec433cf..978c04890c 100644 --- a/examples/supergraph_sdl/src/supergraph_sdl.rs +++ b/examples/supergraph_sdl/src/supergraph_sdl.rs @@ -4,9 +4,7 @@ use apollo_compiler::ApolloCompiler; use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::register_plugin; -use apollo_router::services::RouterRequest; -use apollo_router::services::RouterResponse; -use tower::util::BoxService; +use apollo_router::stages::router; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -29,17 +27,14 @@ impl Plugin for SupergraphSDL { }) } - fn router_service( - &self, - service: BoxService, - ) -> BoxService { + fn router_service(&self, service: router::BoxService) -> router::BoxService { // Clone our supergraph_sdl for use in map_request let supergraph_sdl = self.supergraph_sdl.clone(); // `ServiceBuilder` provides us with `map_request` and `map_response` methods. // // These allow basic interception and transformation of request and response messages. ServiceBuilder::new() - .map_request(move |req: RouterRequest| { + .map_request(move |req: router::Request| { // If we have a query if let Some(query) = &req.originating_request.body().query { // Compile our supergraph_sdl and query From e5590027506337381887dadef9baadd063e05830 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 9 Aug 2022 10:44:15 +0200 Subject: [PATCH 18/28] Make more things private --- NEXT_CHANGELOG.md | 4 ++ apollo-router/src/plugin/mod.rs | 40 +++++++++---------- apollo-router/src/plugin/test/mod.rs | 2 +- .../src/allow_client_id_from_file.rs | 19 +++++---- .../src/forbid_anonymous_operations.rs | 14 ++++--- examples/hello-world/src/hello_world.rs | 14 +++++-- examples/jwt-auth/src/jwt.rs | 15 +++++-- .../src/propagate_status_code.rs | 17 ++++---- 8 files changed, 77 insertions(+), 48 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index e49db68785..a1dea7a815 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -173,6 +173,10 @@ apollo_router::errors::QueryPlannerError apollo_router::errors::ServiceBuildError apollo_router::json_ext apollo_router::mock_service! +apollo_router::plugin::plugins +apollo_router::plugin::PluginFactory +apollo_router::plugin::DynPlugin +apollo_router::plugin::test::MockSubgraphFactory apollo_router::query_planner::QueryPlan::execute apollo_router::services apollo_router::Schema diff --git a/apollo-router/src/plugin/mod.rs b/apollo-router/src/plugin/mod.rs index f206a69ec3..5d124082fa 100644 --- a/apollo-router/src/plugin/mod.rs +++ b/apollo-router/src/plugin/mod.rs @@ -89,20 +89,20 @@ where /// Factories for plugin schema and configuration. #[derive(Clone)] -pub struct PluginFactory { +pub(crate) struct PluginFactory { instance_factory: InstanceFactory, schema_factory: SchemaFactory, } impl PluginFactory { - pub fn new(instance_factory: InstanceFactory, schema_factory: SchemaFactory) -> Self { + pub(crate) fn new(instance_factory: InstanceFactory, schema_factory: SchemaFactory) -> Self { Self { instance_factory, schema_factory, } } - pub async fn create_instance( + pub(crate) async fn create_instance( &self, configuration: &serde_json::Value, supergraph_sdl: Arc, @@ -118,7 +118,7 @@ impl PluginFactory { (self.instance_factory)(configuration, Default::default()).await } - pub fn create_schema(&self, gen: &mut SchemaGenerator) -> schemars::schema::Schema { + pub(crate) fn create_schema(&self, gen: &mut SchemaGenerator) -> schemars::schema::Schema { (self.schema_factory)(gen) } } @@ -129,7 +129,17 @@ static PLUGIN_REGISTRY: Lazy>> = Lazy::new( }); /// Register a plugin factory. -pub fn register_plugin(name: String, plugin_factory: PluginFactory) { +pub fn register_plugin(name: String) { + let plugin_factory = PluginFactory::new( + |configuration, schema| { + Box::pin(async move { + let init = PluginInit::try_new(configuration.clone(), schema)?; + let plugin = P::new(init).await?; + Ok(Box::new(plugin) as Box) + }) + }, + |gen| gen.subschema_for::<

::Config>(), + ); PLUGIN_REGISTRY .lock() .expect("Lock poisoned") @@ -137,7 +147,7 @@ pub fn register_plugin(name: String, plugin_factory: PluginFactory) { } /// Get a copy of the registered plugin factories. -pub fn plugins() -> HashMap { +pub(crate) fn plugins() -> HashMap { PLUGIN_REGISTRY.lock().expect("Lock poisoned").clone() } @@ -148,7 +158,7 @@ pub fn plugins() -> HashMap { /// For more information about the plugin lifecycle please check this documentation #[async_trait] pub trait Plugin: Send + Sync + 'static + Sized { - type Config: JsonSchema + DeserializeOwned; + type Config: JsonSchema + DeserializeOwned + Send; /// This is invoked once after the router starts and compiled-in /// plugins are registered. @@ -217,7 +227,7 @@ fn get_type_of(_: &T) -> &'static str { /// The trait also provides a default implementations for each hook, which returns the associated service unmodified. /// For more information about the plugin lifecycle please check this documentation #[async_trait] -pub trait DynPlugin: Send + Sync + 'static { +pub(crate) trait DynPlugin: Send + Sync + 'static { /// This is invoked after all plugins have been created and we're ready to go live. /// This method MUST not panic. fn activate(&mut self); @@ -304,7 +314,7 @@ where /// Plugins will appear in the configuration as a layer property called: {group}.{name} #[macro_export] macro_rules! register_plugin { - ($group: literal, $name: literal, $value: ident) => { + ($group: literal, $name: literal, $plugin_type: ident) => { $crate::_private::startup::on_startup! { let qualified_name = if $group == "" { $name.to_string() @@ -313,17 +323,7 @@ macro_rules! register_plugin { format!("{}.{}", $group, $name) }; - $crate::plugin::register_plugin( - qualified_name, - $crate::plugin::PluginFactory::new( - |configuration, schema| Box::pin(async move { - let init = $crate::plugin::PluginInit::try_new(configuration.clone(), schema)?; - let plugin = $value::new(init).await?; - Ok(Box::new(plugin) as Box) - }), - |gen| gen.subschema_for::<<$value as $crate::plugin::Plugin>::Config>() - ) - ); + $crate::plugin::register_plugin::<$plugin_type>(qualified_name); } }; } diff --git a/apollo-router/src/plugin/test/mod.rs b/apollo-router/src/plugin/test/mod.rs index ba5cbfe98d..488d4d01fe 100644 --- a/apollo-router/src/plugin/test/mod.rs +++ b/apollo-router/src/plugin/test/mod.rs @@ -190,7 +190,7 @@ impl PluginTestHarness { } #[derive(Clone)] -pub struct MockSubgraphFactory { +pub(crate) struct MockSubgraphFactory { pub(crate) subgraphs: HashMap>, pub(crate) plugins: Arc, } diff --git a/examples/async-auth/src/allow_client_id_from_file.rs b/examples/async-auth/src/allow_client_id_from_file.rs index 755365f7bc..392ca5c36c 100644 --- a/examples/async-auth/src/allow_client_id_from_file.rs +++ b/examples/async-auth/src/allow_client_id_from_file.rs @@ -176,6 +176,7 @@ mod tests { use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; use apollo_router::stages::router; + use apollo_router::TestHarness; use http::StatusCode; use serde_json::json; use tower::ServiceExt; @@ -189,13 +190,17 @@ mod tests { // see router.yaml for more information #[tokio::test] async fn plugin_registered() { - apollo_router::plugin::plugins() - .get("example.allow_client_id_from_file") - .expect("Plugin not found") - .create_instance( - &json!({"header": "x-client-id","path": "allowedClientIds.json"}), - Default::default(), - ) + let config = json!({ + "plugins": { + "example.allow_client_id_from_file": { + "header": "x-client-id", + "path": "allowedClientIds.json", + } + } + }); + TestHarness::builder() + .configuration(serde_json::from_value(config).unwrap()) + .build() .await .unwrap(); } diff --git a/examples/forbid-anonymous-operations/src/forbid_anonymous_operations.rs b/examples/forbid-anonymous-operations/src/forbid_anonymous_operations.rs index 8e99631f8f..dde549edca 100644 --- a/examples/forbid-anonymous-operations/src/forbid_anonymous_operations.rs +++ b/examples/forbid-anonymous-operations/src/forbid_anonymous_operations.rs @@ -96,7 +96,7 @@ mod tests { use apollo_router::plugin::Plugin; use apollo_router::stages::router; use http::StatusCode; - use serde_json::Value; + use serde_json::json; use tower::ServiceExt; use super::ForbidAnonymousOperations; @@ -107,10 +107,14 @@ mod tests { // see router.yml for more information #[tokio::test] async fn plugin_registered() { - apollo_router::plugin::plugins() - .get("example.forbid_anonymous_operations") - .expect("Plugin not found") - .create_instance(&Value::Null, Default::default()) + let config = json!({ + "plugins": { + "example.forbid_anonymous_operations": null + } + }); + apollo_router::TestHarness::builder() + .configuration(serde_json::from_value(config).unwrap()) + .build() .await .unwrap(); } diff --git a/examples/hello-world/src/hello_world.rs b/examples/hello-world/src/hello_world.rs index eb71458f0c..54ee455146 100644 --- a/examples/hello-world/src/hello_world.rs +++ b/examples/hello-world/src/hello_world.rs @@ -102,10 +102,16 @@ mod tests { #[tokio::test] async fn plugin_registered() { - apollo_router::plugin::plugins() - .get("example.hello_world") - .expect("Plugin not found") - .create_instance(&serde_json::json!({"name" : "Bob"}), Default::default()) + let config = serde_json::json!({ + "plugins": { + "example.hello_world": { + "name": "Bob" + } + } + }); + apollo_router::TestHarness::builder() + .configuration(serde_json::from_value(config).unwrap()) + .build() .await .unwrap(); } diff --git a/examples/jwt-auth/src/jwt.rs b/examples/jwt-auth/src/jwt.rs index 4b6c555c00..8968234524 100644 --- a/examples/jwt-auth/src/jwt.rs +++ b/examples/jwt-auth/src/jwt.rs @@ -398,10 +398,17 @@ mod tests { // see `router.yaml` for more information #[tokio::test] async fn plugin_registered() { - apollo_router::plugin::plugins() - .get("example.jwt") - .expect("Plugin not found") - .create_instance(&serde_json::json!({ "algorithm": "HS256" , "key": "629709bdc3bd794312ccc3a1c47beb03ac7310bc02d32d4587e59b5ad81c99ba"}), Default::default()) + let config = serde_json::json!({ + "plugins": { + "example.jwt": { + "algorithm": "HS256" , + "key": "629709bdc3bd794312ccc3a1c47beb03ac7310bc02d32d4587e59b5ad81c99ba" + } + } + }); + apollo_router::TestHarness::builder() + .configuration(serde_json::from_value(config).unwrap()) + .build() .await .unwrap(); } diff --git a/examples/status-code-propagation/src/propagate_status_code.rs b/examples/status-code-propagation/src/propagate_status_code.rs index de812d54c8..b491f47b15 100644 --- a/examples/status-code-propagation/src/propagate_status_code.rs +++ b/examples/status-code-propagation/src/propagate_status_code.rs @@ -106,13 +106,16 @@ mod tests { // see `router.yaml` for more information #[tokio::test] async fn plugin_registered() { - apollo_router::plugin::plugins() - .get("example.propagate_status_code") - .expect("Plugin not found") - .create_instance( - &json!({ "status_codes" : [500, 403, 401] }), - Default::default(), - ) + let config = serde_json::json!({ + "plugins": { + "example.propagate_status_code": { + "status_codes" : [500, 403, 401] + } + } + }); + apollo_router::TestHarness::builder() + .configuration(serde_json::from_value(config).unwrap()) + .build() .await .unwrap(); } From 9d65cf01ffe87738e73e09fe5fa3cfc599ac30e1 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 9 Aug 2022 15:51:57 +0200 Subject: [PATCH 19/28] Add ad-hoc plugins with callbacks in TestHarness --- apollo-router/src/test_harness.rs | 124 ++++++++++++++++++++++++++---- 1 file changed, 107 insertions(+), 17 deletions(-) diff --git a/apollo-router/src/test_harness.rs b/apollo-router/src/test_harness.rs index 5a18c5d807..11251a7dd6 100644 --- a/apollo-router/src/test_harness.rs +++ b/apollo-router/src/test_harness.rs @@ -1,6 +1,5 @@ use std::sync::Arc; -use tower::util::BoxCloneService; use tower::BoxError; use tower::ServiceExt; @@ -11,8 +10,9 @@ use crate::plugin::Plugin; use crate::plugin::PluginInit; use crate::router_factory::RouterServiceConfigurator; use crate::router_factory::YamlRouterServiceFactory; -use crate::services::RouterRequest; -use crate::services::RouterResponse; +use crate::stages::execution; +use crate::stages::query_planner; +use crate::stages::router; use crate::stages::subgraph; use crate::Schema; @@ -115,6 +115,41 @@ impl<'a> TestHarness<'a> { self } + /// Adds an ad-hoc plugin that has [`Plugin::router_service`] implemented with `callback`. + pub fn extra_router_plugin( + self, + callback: impl Fn(router::BoxService) -> router::BoxService + Send + Sync + 'static, + ) -> Self { + self.extra_plugin(RouterServicePlugin(callback)) + } + + /// Adds an ad-hoc plugin that has [`Plugin::query_planning_service`] implemented with `callback`. + pub fn extra_query_planner_plugin( + self, + callback: impl Fn(query_planner::BoxService) -> query_planner::BoxService + + Send + + Sync + + 'static, + ) -> Self { + self.extra_plugin(QueryPlannerServicePlugin(callback)) + } + + /// Adds an ad-hoc plugin that has [`Plugin::execution_service`] implemented with `callback`. + pub fn extra_execution_plugin( + self, + callback: impl Fn(execution::BoxService) -> execution::BoxService + Send + Sync + 'static, + ) -> Self { + self.extra_plugin(ExecutionServicePlugin(callback)) + } + + /// Adds an ad-hoc plugin that has [`Plugin::subgraph_service`] implemented with `callback`. + pub fn extra_subgraph_plugin( + self, + callback: impl Fn(&str, subgraph::BoxService) -> subgraph::BoxService + Send + Sync + 'static, + ) -> Self { + self.extra_plugin(SubgraphServicePlugin(callback)) + } + /// Enables this test harness to make network requests to subgraphs. /// /// If this is not called, all subgraph requests get an empty response by default @@ -127,11 +162,14 @@ impl<'a> TestHarness<'a> { } /// Builds the GraphQL service - pub async fn build( - self, - ) -> Result, BoxError> { + pub async fn build(self) -> Result { let builder = if self.schema.is_none() { - self.extra_plugin(CannedSubgraphResponses) + self.extra_subgraph_plugin(|subgraph_name, default| match subgraph_name { + "products" => canned::products_subgraph().boxed(), + "accounts" => canned::accounts_subgraph().boxed(), + "reviews" => canned::reviews_subgraph().boxed(), + _ => default, + }) } else { self }; @@ -150,26 +188,78 @@ impl<'a> TestHarness<'a> { } } -struct CannedSubgraphResponses; +struct RouterServicePlugin(F); +struct QueryPlannerServicePlugin(F); +struct ExecutionServicePlugin(F); +struct SubgraphServicePlugin(F); + +#[async_trait::async_trait] +impl Plugin for RouterServicePlugin +where + F: 'static + Send + Sync + Fn(router::BoxService) -> router::BoxService, +{ + type Config = (); + + async fn new(_: PluginInit) -> Result { + unreachable!() + } + + fn router_service(&self, service: router::BoxService) -> router::BoxService { + (self.0)(service) + } +} + +#[async_trait::async_trait] +impl Plugin for QueryPlannerServicePlugin +where + F: 'static + Send + Sync + Fn(query_planner::BoxService) -> query_planner::BoxService, +{ + type Config = (); + + async fn new(_: PluginInit) -> Result { + unreachable!() + } + + fn query_planning_service( + &self, + service: query_planner::BoxService, + ) -> query_planner::BoxService { + (self.0)(service) + } +} #[async_trait::async_trait] -impl Plugin for CannedSubgraphResponses { +impl Plugin for ExecutionServicePlugin +where + F: 'static + Send + Sync + Fn(execution::BoxService) -> execution::BoxService, +{ type Config = (); async fn new(_: PluginInit) -> Result { - Ok(Self) + unreachable!() + } + + fn execution_service(&self, service: execution::BoxService) -> execution::BoxService { + (self.0)(service) + } +} + +#[async_trait::async_trait] +impl Plugin for SubgraphServicePlugin +where + F: 'static + Send + Sync + Fn(&str, subgraph::BoxService) -> subgraph::BoxService, +{ + type Config = (); + + async fn new(_: PluginInit) -> Result { + unreachable!() } fn subgraph_service( &self, subgraph_name: &str, - default: subgraph::BoxService, + service: subgraph::BoxService, ) -> subgraph::BoxService { - match subgraph_name { - "products" => canned::products_subgraph().boxed(), - "accounts" => canned::accounts_subgraph().boxed(), - "reviews" => canned::reviews_subgraph().boxed(), - _ => default, - } + (self.0)(subgraph_name, service) } } From f60586c17e41f45214cd7683c43c7088f185b819 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 9 Aug 2022 15:58:53 +0200 Subject: [PATCH 20/28] Actually implement TestHarness::with_subgraph_network_requests Oops --- apollo-router/src/test_harness.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/apollo-router/src/test_harness.rs b/apollo-router/src/test_harness.rs index 11251a7dd6..58bca0d718 100644 --- a/apollo-router/src/test_harness.rs +++ b/apollo-router/src/test_harness.rs @@ -173,6 +173,20 @@ impl<'a> TestHarness<'a> { } else { self }; + let builder = if builder.subgraph_network_requests { + builder + } else { + builder.extra_subgraph_plugin(|_name, _default| { + tower::service_fn(|request: subgraph::Request| { + let empty_response = subgraph::Response::builder() + .extensions(crate::json_ext::Object::new()) + .context(request.context) + .build(); + std::future::ready(Ok(empty_response)) + }) + .boxed() + }) + }; let config = builder.configuration.unwrap_or_default(); let canned_schema = include_str!("../../examples/graphql/local.graphql"); let schema = builder.schema.unwrap_or(canned_schema); From 909c56f35fd61d002d815c06d5b87ae40af666c0 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 9 Aug 2022 16:12:19 +0200 Subject: [PATCH 21/28] Add TestHarness::configuration_json --- apollo-router/src/test_harness.rs | 11 ++++++++++- apollo-router/tests/integration_tests.rs | 3 ++- apollo-router/tests/rhai_tests.rs | 3 ++- examples/async-auth/src/allow_client_id_from_file.rs | 3 ++- .../src/forbid_anonymous_operations.rs | 3 ++- examples/hello-world/src/hello_world.rs | 3 ++- examples/jwt-auth/src/jwt.rs | 3 ++- .../src/propagate_status_code.rs | 3 ++- 8 files changed, 24 insertions(+), 8 deletions(-) diff --git a/apollo-router/src/test_harness.rs b/apollo-router/src/test_harness.rs index 58bca0d718..a6caa1c2bb 100644 --- a/apollo-router/src/test_harness.rs +++ b/apollo-router/src/test_harness.rs @@ -47,7 +47,7 @@ use crate::Schema; /// .build() /// .unwrap(); /// let response = TestHarness::builder() -/// .configuration(serde_json::from_value(config)?) +/// .configuration_json(config)? /// .build() /// .await? /// .oneshot(request) @@ -98,6 +98,15 @@ impl<'a> TestHarness<'a> { self } + /// Specifies the (static) router configuration as a JSON value, + /// such as from the `serde_json::json!` macro. + pub fn configuration_json( + self, + configuration: serde_json::Value, + ) -> Result { + Ok(self.configuration(serde_json::from_value(configuration)?)) + } + /// Adds an extra, already instanciated plugin. /// /// May be called multiple times. diff --git a/apollo-router/tests/integration_tests.rs b/apollo-router/tests/integration_tests.rs index 25fae2c71b..7dfcc135b0 100644 --- a/apollo-router/tests/integration_tests.rs +++ b/apollo-router/tests/integration_tests.rs @@ -693,7 +693,8 @@ async fn setup_router_and_registry( .unwrap(); let router = apollo_router::TestHarness::builder() .with_subgraph_network_requests() - .configuration(config) + .configuration_json(config) + .unwrap() .schema(include_str!("fixtures/supergraph.graphql")) .extra_plugin(counting_registry.clone()) .extra_plugin(telemetry) diff --git a/apollo-router/tests/rhai_tests.rs b/apollo-router/tests/rhai_tests.rs index 5b7943ff5a..b67e042f57 100644 --- a/apollo-router/tests/rhai_tests.rs +++ b/apollo-router/tests/rhai_tests.rs @@ -20,7 +20,8 @@ async fn all_rhai_callbacks_are_invoked() { } }); let router = TestHarness::builder() - .configuration(serde_json::from_value(config).unwrap()) + .configuration_json(config) + .unwrap() .schema(include_str!("./fixtures/supergraph.graphql")) .build() .await diff --git a/examples/async-auth/src/allow_client_id_from_file.rs b/examples/async-auth/src/allow_client_id_from_file.rs index 392ca5c36c..5233ffebb0 100644 --- a/examples/async-auth/src/allow_client_id_from_file.rs +++ b/examples/async-auth/src/allow_client_id_from_file.rs @@ -199,7 +199,8 @@ mod tests { } }); TestHarness::builder() - .configuration(serde_json::from_value(config).unwrap()) + .configuration_json(config) + .unwrap() .build() .await .unwrap(); diff --git a/examples/forbid-anonymous-operations/src/forbid_anonymous_operations.rs b/examples/forbid-anonymous-operations/src/forbid_anonymous_operations.rs index dde549edca..a9788cddbb 100644 --- a/examples/forbid-anonymous-operations/src/forbid_anonymous_operations.rs +++ b/examples/forbid-anonymous-operations/src/forbid_anonymous_operations.rs @@ -113,7 +113,8 @@ mod tests { } }); apollo_router::TestHarness::builder() - .configuration(serde_json::from_value(config).unwrap()) + .configuration_json(config) + .unwrap() .build() .await .unwrap(); diff --git a/examples/hello-world/src/hello_world.rs b/examples/hello-world/src/hello_world.rs index 54ee455146..dda5d20e70 100644 --- a/examples/hello-world/src/hello_world.rs +++ b/examples/hello-world/src/hello_world.rs @@ -110,7 +110,8 @@ mod tests { } }); apollo_router::TestHarness::builder() - .configuration(serde_json::from_value(config).unwrap()) + .configuration_json(config) + .unwrap() .build() .await .unwrap(); diff --git a/examples/jwt-auth/src/jwt.rs b/examples/jwt-auth/src/jwt.rs index 8968234524..4c473e5454 100644 --- a/examples/jwt-auth/src/jwt.rs +++ b/examples/jwt-auth/src/jwt.rs @@ -407,7 +407,8 @@ mod tests { } }); apollo_router::TestHarness::builder() - .configuration(serde_json::from_value(config).unwrap()) + .configuration_json(config) + .unwrap() .build() .await .unwrap(); diff --git a/examples/status-code-propagation/src/propagate_status_code.rs b/examples/status-code-propagation/src/propagate_status_code.rs index b491f47b15..2a8c86fdf3 100644 --- a/examples/status-code-propagation/src/propagate_status_code.rs +++ b/examples/status-code-propagation/src/propagate_status_code.rs @@ -114,7 +114,8 @@ mod tests { } }); apollo_router::TestHarness::builder() - .configuration(serde_json::from_value(config).unwrap()) + .configuration_json(config) + .unwrap() .build() .await .unwrap(); From 2c6b5d9b9bf5b62175a82dbfdb556479e693553a Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 9 Aug 2022 16:41:11 +0200 Subject: [PATCH 22/28] Port apollo-router-scaffold to the new TestHarness --- .../plugin/src/plugins/{{snake_name}}.rs | 54 +++++++------------ apollo-router/src/plugin/test/mod.rs | 7 +-- apollo-router/src/services/mod.rs | 22 ++++++++ 3 files changed, 42 insertions(+), 41 deletions(-) diff --git a/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs b/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs index f3fbfe49a1..8d70aea41e 100644 --- a/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs +++ b/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs @@ -154,53 +154,37 @@ register_plugin!("{{project_name}}", "{{snake_name}}", {{pascal_name}}); #[cfg(test)] mod tests { - use super::{Conf, {{pascal_name}}}; - - use apollo_router::plugin::test::IntoSchema::Canned; - use apollo_router::plugin::test::PluginTestHarness; - use apollo_router::plugin::Plugin; - use apollo_router::plugin::PluginInit; + use apollo_router::TestHarness; + use apollo_router::stages::router; use tower::BoxError; - - #[tokio::test] - async fn plugin_registered() { - apollo_router::plugin::plugins() - .get("{{project_name}}.{{snake_name}}") - .expect("Plugin not found") - .create_instance(&serde_json::json!({"message" : "Starting my plugin"}), Default::default()) - .await - .unwrap(); - } + use tower::ServiceExt; #[tokio::test] async fn basic_test() -> Result<(), BoxError> { - // Define a configuration to use with our plugin - let conf = Conf { - message: "Starting my plugin".to_string(), - }; - - // Build an instance of our plugin to use in the test harness - let plugin = {{pascal_name}}::new(PluginInit::new(conf, Default::default())).await.expect("created plugin"); - - // Create the test harness. You can add mocks for individual services, or use prebuilt canned services. - let mut test_harness = PluginTestHarness::builder() - .plugin(plugin) - .schema(Canned) + let test_harness = TestHarness::builder() + .configuration_json(serde_json::json!({ + "plugins": { + "{{project_name}}.{{snake_name}}": { + "message" : "Starting my plugin" + } + } + })) + .unwrap() .build() - .await?; - - // Send a request - let mut result = test_harness.call_canned().await?; + .await + .unwrap(); + let request = router::Request::canned(); + let mut streamed_response = test_harness.oneshot(request).await?; - let first_response = result + let first_response = streamed_response .next_response() .await .expect("couldn't get primary 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()); + // You could keep calling .next_response() until it yields None if you're expexting more parts. + assert!(streamed_response.next_response().await.is_none()); Ok(()) } } diff --git a/apollo-router/src/plugin/test/mod.rs b/apollo-router/src/plugin/test/mod.rs index 488d4d01fe..a7632881a4 100644 --- a/apollo-router/src/plugin/test/mod.rs +++ b/apollo-router/src/plugin/test/mod.rs @@ -179,12 +179,7 @@ impl PluginTestHarness { self.router_service .ready() .await? - .call( - RouterRequest::fake_builder() - .query("query TopProducts($first: Int) { topProducts(first: $first) { upc name reviews { id product { name } author { id name } } } }") - .variable("first", 2usize) - .build()?, - ) + .call(RouterRequest::canned()) .await } } diff --git a/apollo-router/src/services/mod.rs b/apollo-router/src/services/mod.rs index fb50504b8d..50cf51f41e 100644 --- a/apollo-router/src/services/mod.rs +++ b/apollo-router/src/services/mod.rs @@ -142,6 +142,28 @@ impl RouterRequest { Method::GET, ) } + + /// Create an example request for tests + pub fn canned() -> Self { + let query = " + query TopProducts($first: Int) { + topProducts(first: $first) { + upc + name + reviews { + id + product { name } + author { id name } + } + } + } + "; + Self::fake_builder() + .query(query) + .variable("first", 2usize) + .build() + .unwrap() + } } assert_impl_all!(RouterResponse: Send); From 33326e79247e25a63f78364d080ce2beb96a59d0 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 9 Aug 2022 17:04:02 +0200 Subject: [PATCH 23/28] Port remaining examples/* to the new TestHarness --- Cargo.lock | 1 + .../plugin/src/plugins/{{snake_name}}.rs | 2 +- apollo-router/src/plugin/test/mod.rs | 2 +- apollo-router/src/services/mod.rs | 25 ++- examples/add-timestamp-header/src/main.rs | 64 ++++---- examples/cookies-to-headers/Cargo.toml | 1 + examples/cookies-to-headers/src/main.rs | 142 +++++++++--------- examples/hello-world/src/hello_world.rs | 42 +----- examples/op-name-to-header/src/main.rs | 74 ++++----- .../rhai-data-response-mutate/src/main.rs | 36 ++--- .../rhai-error-response-mutate/src/main.rs | 36 ++--- examples/rhai-logging/src/main.rs | 57 +++---- .../rhai-subgraph-request-log/src/main.rs | 36 ++--- examples/rhai-surrogate-cache-key/src/main.rs | 36 ++--- 14 files changed, 243 insertions(+), 311 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 486eadfaa8..5fb3461d14 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1118,6 +1118,7 @@ version = "0.1.0" dependencies = [ "anyhow", "apollo-router", + "futures", "http", "serde_json", "tokio", diff --git a/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs b/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs index 8d70aea41e..e10d13879a 100644 --- a/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs +++ b/apollo-router-scaffold/templates/plugin/src/plugins/{{snake_name}}.rs @@ -173,7 +173,7 @@ mod tests { .build() .await .unwrap(); - let request = router::Request::canned(); + let request = router::Request::canned_builder().build().unwrap(); let mut streamed_response = test_harness.oneshot(request).await?; let first_response = streamed_response diff --git a/apollo-router/src/plugin/test/mod.rs b/apollo-router/src/plugin/test/mod.rs index a7632881a4..5671f8a6ab 100644 --- a/apollo-router/src/plugin/test/mod.rs +++ b/apollo-router/src/plugin/test/mod.rs @@ -179,7 +179,7 @@ impl PluginTestHarness { self.router_service .ready() .await? - .call(RouterRequest::canned()) + .call(RouterRequest::canned_builder().build().unwrap()) .await } } diff --git a/apollo-router/src/services/mod.rs b/apollo-router/src/services/mod.rs index 50cf51f41e..967f7abddb 100644 --- a/apollo-router/src/services/mod.rs +++ b/apollo-router/src/services/mod.rs @@ -143,8 +143,14 @@ impl RouterRequest { ) } - /// Create an example request for tests - pub fn canned() -> Self { + /// Create a request with an example query, for tests + #[builder(visibility = "pub")] + fn canned_new( + operation_name: Option, + extensions: HashMap, + context: Option, + headers: MultiMap, + ) -> Result { let query = " query TopProducts($first: Int) { topProducts(first: $first) { @@ -158,11 +164,16 @@ impl RouterRequest { } } "; - Self::fake_builder() - .query(query) - .variable("first", 2usize) - .build() - .unwrap() + let mut variables = HashMap::new(); + variables.insert("first".to_owned(), 2_usize.into()); + Self::fake_new( + Some(query.to_owned()), + operation_name, + variables, + extensions, + context, + headers, + ) } } diff --git a/examples/add-timestamp-header/src/main.rs b/examples/add-timestamp-header/src/main.rs index 27747ae015..c4464f238f 100644 --- a/examples/add-timestamp-header/src/main.rs +++ b/examples/add-timestamp-header/src/main.rs @@ -13,60 +13,64 @@ fn main() -> Result<()> { #[cfg(test)] mod tests { use apollo_router::plugin::test; - use apollo_router::plugin::Plugin; - use apollo_router::plugin::PluginInit; - use apollo_router::plugins::rhai::Conf; - use apollo_router::plugins::rhai::Rhai; use apollo_router::stages::router; use http::StatusCode; use tower::util::ServiceExt; #[tokio::test] async fn test_router_service_adds_timestamp_header() { - // create a mock service we will use to test our plugin let mut mock_service = test::MockRouterService::new(); + // create a mock service we will use to test our plugin // The expected reply is going to be JSON returned in the RouterResponse { data } section. let expected_mock_response_data = "response created within the mock"; // Let's set up our mock to make sure it will be called once - mock_service - .expect_call() - .once() - .returning(move |req: router::Request| { - // Preserve our context from request to response - Ok(router::Response::fake_builder() - .context(req.context) - .data(expected_mock_response_data) - .build() - .unwrap()) - }); - - let conf: Conf = serde_json::from_value(serde_json::json!({ - "scripts": "src", - "main": "add_timestamp_header.rhai", - })) - .expect("json must be valid"); + mock_service.expect_clone().return_once(move || { + let mut mock_service = test::MockRouterService::new(); + mock_service + .expect_call() + .once() + .returning(move |req: router::Request| { + // Preserve our context from request to response + Ok(router::Response::fake_builder() + .context(req.context) + .data(expected_mock_response_data) + .build() + .unwrap()) + }); + mock_service + }); - // Build an instance of our plugin to use in the test harness - let rhai = Rhai::new(PluginInit::new(conf, Default::default())) + let config = serde_json::json!({ + "rhai": { + "scripts": "src", + "main": "add_timestamp_header.rhai", + } + }); + let test_harness = apollo_router::TestHarness::builder() + .configuration_json(config) + .unwrap() + .extra_router_plugin(move |_| mock_service.clone().boxed()) + .build() .await - .expect("created plugin"); - - let service_stack = rhai.router_service(mock_service.boxed()); + .unwrap(); // Let's create a request with our operation name - let request_with_appropriate_name = router::Request::fake_builder() + let request_with_appropriate_name = router::Request::canned_builder() .operation_name("me".to_string()) .build() .unwrap(); // ...And call our service stack with it - let mut service_response = service_stack + let mut service_response = test_harness .oneshot(request_with_appropriate_name) .await .unwrap(); + let response = service_response.next_response().await.unwrap(); + assert_eq!(response.errors, []); + // Rhai should return a 200... assert_eq!(StatusCode::OK, service_response.response.status()); @@ -78,8 +82,6 @@ mod tests { .expect("x-elapsed-time is present"); // with the expected message - let response = service_response.next_response().await.unwrap(); - assert!(response.errors.is_empty()); assert_eq!(expected_mock_response_data, response.data.as_ref().unwrap()); } } diff --git a/examples/cookies-to-headers/Cargo.toml b/examples/cookies-to-headers/Cargo.toml index 982b7137fc..268d96b3b6 100644 --- a/examples/cookies-to-headers/Cargo.toml +++ b/examples/cookies-to-headers/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" [dependencies] anyhow = "1" apollo-router = { path = "../../apollo-router" } +futures = "0.3.21" http = "0.2" serde_json = "1" tokio = { version = "1", features = ["full"] } diff --git a/examples/cookies-to-headers/src/main.rs b/examples/cookies-to-headers/src/main.rs index 097c6a0b4c..1d6c7fcc43 100644 --- a/examples/cookies-to-headers/src/main.rs +++ b/examples/cookies-to-headers/src/main.rs @@ -32,15 +32,10 @@ fn main() -> Result<()> { #[cfg(test)] mod tests { - use apollo_router::http_ext; use apollo_router::plugin::test; - use apollo_router::plugin::Plugin; - use apollo_router::plugin::PluginInit; - use apollo_router::plugins::rhai::Conf; - use apollo_router::plugins::rhai::Rhai; + use apollo_router::stages::router; use apollo_router::stages::subgraph; - use http::header::HeaderName; - use http::HeaderValue; + use futures::stream::StreamExt; use http::StatusCode; use tower::util::ServiceExt; @@ -53,86 +48,83 @@ mod tests { let expected_mock_response_data = "response created within the mock"; // Let's set up our mock to make sure it will be called once - mock_service - .expect_call() - .once() - .returning(move |req: subgraph::Request| { - // Let's make sure our request contains our new headers - assert_eq!( - req.subgraph_request - .headers() - .get("yummy_cookie") - .expect("yummy_cookie is present"), - "choco" - ); - assert_eq!( - req.subgraph_request - .headers() - .get("tasty_cookie") - .expect("tasty_cookie is present"), - "strawberry" - ); - Ok(subgraph::Response::fake_builder() - .data(expected_mock_response_data) - .build()) - }); - - let conf: Conf = serde_json::from_value(serde_json::json!({ - "scripts": "src", - "main": "cookies_to_headers.rhai", - })) - .expect("json must be valid"); - - // Build an instance of our plugin to use in the test harness - let rhai = Rhai::new(PluginInit::new(conf, Default::default())) + mock_service.expect_clone().return_once(move || { + let mut mock_service = test::MockSubgraphService::new(); + mock_service + .expect_call() + .once() + .returning(move |req: subgraph::Request| { + // Let's make sure our request contains our new headers + assert_eq!( + req.subgraph_request + .headers() + .get("yummy_cookie") + .expect("yummy_cookie is present"), + "choco" + ); + assert_eq!( + req.subgraph_request + .headers() + .get("tasty_cookie") + .expect("tasty_cookie is present"), + "strawberry" + ); + req.context + .insert("mock_data", expected_mock_response_data.to_owned()) + .unwrap(); + Ok(subgraph::Response::fake_builder().build()) + }); + mock_service + }); + + let config = serde_json::json!({ + "rhai": { + "scripts": "src", + "main": "cookies_to_headers.rhai", + } + }); + let test_harness = apollo_router::TestHarness::builder() + .configuration_json(config) + .unwrap() + .extra_subgraph_plugin(move |_, _| mock_service.clone().boxed()) + .extra_router_plugin(|service| { + service + .map_response(|mut response| { + let mock_data = response.context.get("mock_data").unwrap(); + let body = response.response.body_mut(); + let dummy = futures::stream::empty().boxed(); + let stream = std::mem::replace(body, dummy); + *body = stream + .map(move |mut resp| { + resp.data = mock_data.clone(); + resp + }) + .boxed(); + response + }) + .boxed() + }) + .build() .await - .expect("created plugin"); - - let service_stack = rhai.subgraph_service("mock", mock_service.boxed()); + .unwrap(); - let mut sub_request = http_ext::Request::fake_builder() - .headers(Default::default()) - .body(Default::default()) - .build() - .expect("fake builds should always work; qed"); - let mut originating_request = http_ext::Request::fake_builder() - .headers(Default::default()) - .body(Default::default()) + let request_with_appropriate_cookies = router::Request::canned_builder() + .header("cookie", "yummy_cookie=choco;tasty_cookie=strawberry") .build() - .expect("fake builds should always work; qed"); - - let headers = vec![( - HeaderName::from_static("cookie"), - HeaderValue::from_static("yummy_cookie=choco;tasty_cookie=strawberry"), - )]; - - for (name, value) in headers { - sub_request - .headers_mut() - .insert(name.clone(), value.clone()); - originating_request.headers_mut().insert(name, value); - } - - // Let's create a request with our cookies - let request_with_appropriate_cookies = subgraph::Request::fake_builder() - .originating_request(std::sync::Arc::new(originating_request)) - .subgraph_request(sub_request) - .build(); + .unwrap(); // ...And call our service stack with it - let service_response = service_stack + let mut service_response = test_harness .oneshot(request_with_appropriate_cookies) .await .unwrap(); + let response = service_response.next_response().await.unwrap(); + assert_eq!(response.errors, []); // Rhai should return a 200... assert_eq!(StatusCode::OK, service_response.response.status()); // with the expected message - let graphql_response: apollo_router::graphql::Response = - http::Response::from(service_response.response).into_body(); - - assert!(graphql_response.errors.is_empty()); - assert_eq!(expected_mock_response_data, graphql_response.data.unwrap()) + assert_eq!(expected_mock_response_data, response.data.unwrap()) } } diff --git a/examples/hello-world/src/hello_world.rs b/examples/hello-world/src/hello_world.rs index dda5d20e70..e3c0762b66 100644 --- a/examples/hello-world/src/hello_world.rs +++ b/examples/hello-world/src/hello_world.rs @@ -92,16 +92,10 @@ register_plugin!("example", "hello_world", HelloWorld); #[cfg(test)] mod tests { - use apollo_router::plugin::test::IntoSchema::Canned; - use apollo_router::plugin::test::PluginTestHarness; - use apollo_router::plugin::Plugin; - use apollo_router::plugin::PluginInit; - - use super::Conf; - use super::HelloWorld; - + // If we run this test as follows: cargo test -- --nocapture + // we will see the message "Hello Bob" printed to standard out #[tokio::test] - async fn plugin_registered() { + async fn display_message() { let config = serde_json::json!({ "plugins": { "example.hello_world": { @@ -109,36 +103,14 @@ mod tests { } } }); - apollo_router::TestHarness::builder() - .configuration_json(config) - .unwrap() - .build() - .await - .unwrap(); - } - - // If we run this test as follows: cargo test -- --nocapture - // we will see the message "Hello Bob" printed to standard out - #[tokio::test] - async fn display_message() { - // Define a configuration to use with our plugin - let conf = Conf { - name: "Bob".to_string(), - }; - - // Build an instance of our plugin to use in the test harness - let plugin = HelloWorld::new(PluginInit::new(conf, Default::default())) - .await - .expect("created plugin"); - // Build a test harness. Usually we'd use this and send requests to // it, but in this case it's enough to build the harness to see our // output when our service registers. - let _test_harness = PluginTestHarness::builder() - .plugin(plugin) - .schema(Canned) + let _test_harness = apollo_router::TestHarness::builder() + .configuration_json(config) + .unwrap() .build() .await - .expect("building harness"); + .unwrap(); } } diff --git a/examples/op-name-to-header/src/main.rs b/examples/op-name-to-header/src/main.rs index c2543f417e..5f0e3f33ef 100644 --- a/examples/op-name-to-header/src/main.rs +++ b/examples/op-name-to-header/src/main.rs @@ -13,10 +13,6 @@ fn main() -> Result<()> { #[cfg(test)] mod tests { use apollo_router::plugin::test; - use apollo_router::plugin::Plugin; - use apollo_router::plugin::PluginInit; - use apollo_router::plugins::rhai::Conf; - use apollo_router::plugins::rhai::Rhai; use apollo_router::stages::router; use http::StatusCode; use tower::util::ServiceExt; @@ -30,56 +26,60 @@ mod tests { let expected_mock_response_data = "response created within the mock"; // Let's set up our mock to make sure it will be called once - mock_service - .expect_call() - .once() - .returning(move |req: router::Request| { - // Let's make sure our request contains our new header - assert_eq!( - req.originating_request - .headers() - .get("X-operation-name") - .expect("X-operation-name is present"), - "me" - ); - Ok(router::Response::fake_builder() - .data(expected_mock_response_data) - .build() - .unwrap()) - }); + mock_service.expect_clone().return_once(move || { + let mut mock_service = test::MockRouterService::new(); + mock_service + .expect_call() + .once() + .returning(move |req: router::Request| { + // Let's make sure our request contains our new header + assert_eq!( + req.originating_request + .headers() + .get("X-operation-name") + .expect("X-operation-name is present"), + "me" + ); + Ok(router::Response::fake_builder() + .data(expected_mock_response_data) + .build() + .unwrap()) + }); + mock_service + }); - let conf: Conf = serde_json::from_value(serde_json::json!({ - "scripts": "src", - "main": "op_name_to_header.rhai", - })) - .expect("json must be valid"); - - // Build a rhai plugin instance from our conf - // Build an instance of our plugin to use in the test harness - let rhai = Rhai::new(PluginInit::new(conf, Default::default())) + let config = serde_json::json!({ + "rhai": { + "scripts": "src", + "main": "op_name_to_header.rhai", + } + }); + let test_harness = apollo_router::TestHarness::builder() + .configuration_json(config) + .unwrap() + .extra_router_plugin(move |_| mock_service.clone().boxed()) + .build() .await - .expect("created plugin"); - - let service_stack = rhai.router_service(mock_service.boxed()); + .unwrap(); // Let's create a request with our operation name - let request_with_appropriate_name = router::Request::fake_builder() + let request_with_appropriate_name = router::Request::canned_builder() .operation_name("me".to_string()) .build() .unwrap(); // ...And call our service stack with it - let mut service_response = service_stack + let mut service_response = test_harness .oneshot(request_with_appropriate_name) .await .unwrap(); + let response = service_response.next_response().await.unwrap(); + assert_eq!(response.errors, []); // Rhai should return a 200... assert_eq!(StatusCode::OK, service_response.response.status()); // with the expected message - let response = service_response.next_response().await.unwrap(); - assert!(response.errors.is_empty()); assert_eq!(expected_mock_response_data, response.data.as_ref().unwrap()); } } diff --git a/examples/rhai-data-response-mutate/src/main.rs b/examples/rhai-data-response-mutate/src/main.rs index 4a3c0a2b88..631bdba9e0 100644 --- a/examples/rhai-data-response-mutate/src/main.rs +++ b/examples/rhai-data-response-mutate/src/main.rs @@ -12,37 +12,25 @@ fn main() -> Result<()> { #[cfg(test)] mod tests { - use apollo_router::plugin::test::IntoSchema::Canned; - use apollo_router::plugin::test::PluginTestHarness; - use apollo_router::plugin::Plugin; - use apollo_router::plugin::PluginInit; - use apollo_router::plugins::rhai::Conf; - use apollo_router::plugins::rhai::Rhai; use apollo_router::stages::router; use apollo_router::Context; use http::StatusCode; + use tower::util::ServiceExt; #[tokio::test] async fn test_subgraph_mutates_data() { - // Define a configuration to use with our plugin - let conf: Conf = serde_json::from_value(serde_json::json!({ - "scripts": "src", - "main": "rhai_data_response_mutate.rhai", - })) - .expect("valid conf supplied"); - - // Build an instance of our plugin to use in the test harness - let plugin = Rhai::new(PluginInit::new(conf, Default::default())) - .await - .expect("created plugin"); - - // Build a test harness. - let mut test_harness = PluginTestHarness::builder() - .plugin(plugin) - .schema(Canned) + let config = serde_json::json!({ + "rhai": { + "scripts": "src", + "main": "rhai_data_response_mutate.rhai", + } + }); + let test_harness = apollo_router::TestHarness::builder() + .configuration_json(config) + .unwrap() .build() .await - .expect("building harness"); + .unwrap(); // The expected reply is going to be JSON returned in the RouterResponse { data } section. let _expected_mock_response_data = "response created within the mock"; @@ -52,7 +40,7 @@ mod tests { let operation_name: Option<&str> = None; let context: Option = None; let mut service_response = test_harness - .call( + .oneshot( router::Request::fake_builder() .header("name_header", "test_client") .header("version_header", "1.0-test") diff --git a/examples/rhai-error-response-mutate/src/main.rs b/examples/rhai-error-response-mutate/src/main.rs index 19c81b76d1..cfe1fcebfe 100644 --- a/examples/rhai-error-response-mutate/src/main.rs +++ b/examples/rhai-error-response-mutate/src/main.rs @@ -12,37 +12,25 @@ fn main() -> Result<()> { #[cfg(test)] mod tests { - use apollo_router::plugin::test::IntoSchema::Canned; - use apollo_router::plugin::test::PluginTestHarness; - use apollo_router::plugin::Plugin; - use apollo_router::plugin::PluginInit; - use apollo_router::plugins::rhai::Conf; - use apollo_router::plugins::rhai::Rhai; use apollo_router::stages::router; use apollo_router::Context; use http::StatusCode; + use tower::util::ServiceExt; #[tokio::test] async fn test_subgraph_mutates_error() { - // Define a configuration to use with our plugin - let conf: Conf = serde_json::from_value(serde_json::json!({ - "scripts": "src", - "main": "rhai_error_response_mutate.rhai", - })) - .expect("valid conf supplied"); - - // Build an instance of our plugin to use in the test harness - let plugin = Rhai::new(PluginInit::new(conf, Default::default())) - .await - .expect("created plugin"); - - // Build a test harness. - let mut test_harness = PluginTestHarness::builder() - .plugin(plugin) - .schema(Canned) + let config = serde_json::json!({ + "rhai": { + "scripts": "src", + "main": "rhai_error_response_mutate.rhai", + } + }); + let test_harness = apollo_router::TestHarness::builder() + .configuration_json(config) + .unwrap() .build() .await - .expect("building harness"); + .unwrap(); // The expected reply is going to be JSON returned in the RouterResponse { error } section. let _expected_mock_response_error = "error created within the mock"; @@ -52,7 +40,7 @@ mod tests { let operation_name: Option<&str> = None; let context: Option = None; let mut service_response = test_harness - .call( + .oneshot( router::Request::fake_builder() .header("name_header", "test_client") .header("version_header", "1.0-test") diff --git a/examples/rhai-logging/src/main.rs b/examples/rhai-logging/src/main.rs index 048723e02f..885a6989c8 100644 --- a/examples/rhai-logging/src/main.rs +++ b/examples/rhai-logging/src/main.rs @@ -14,10 +14,6 @@ fn main() -> Result<()> { #[cfg(test)] mod tests { use apollo_router::plugin::test; - use apollo_router::plugin::Plugin; - use apollo_router::plugin::PluginInit; - use apollo_router::plugins::rhai::Conf; - use apollo_router::plugins::rhai::Rhai; use apollo_router::stages::router; use http::StatusCode; use tower::util::ServiceExt; @@ -31,47 +27,52 @@ mod tests { let expected_mock_response_data = "response created within the mock"; // Let's set up our mock to make sure it will be called once - mock_service - .expect_call() - .once() - .returning(move |_req: router::Request| { - Ok(router::Response::fake_builder() - .data(expected_mock_response_data) - .build() - .unwrap()) - }); + mock_service.expect_clone().return_once(move || { + let mut mock_service = test::MockRouterService::new(); + mock_service + .expect_call() + .once() + .returning(move |_req: router::Request| { + Ok(router::Response::fake_builder() + .data(expected_mock_response_data) + .build() + .unwrap()) + }); + mock_service + }); - let conf: Conf = serde_json::from_value(serde_json::json!({ - "scripts": "src", - "main": "rhai_logging.rhai", - })) - .expect("valid conf supplied"); - - // Build an instance of our plugin to use in the test harness - let rhai = Rhai::new(PluginInit::new(conf, Default::default())) + let config = serde_json::json!({ + "rhai": { + "scripts": "src", + "main": "rhai_logging.rhai", + } + }); + let test_harness = apollo_router::TestHarness::builder() + .configuration_json(config) + .unwrap() + .extra_router_plugin(move |_| mock_service.clone().boxed()) + .build() .await - .expect("created plugin"); - - let service_stack = rhai.router_service(mock_service.boxed()); + .unwrap(); // Let's create a request with our operation name - let request_with_appropriate_name = router::Request::fake_builder() + let request_with_appropriate_name = router::Request::canned_builder() .operation_name("me".to_string()) .build() .unwrap(); // ...And call our service stack with it - let mut service_response = service_stack + let mut service_response = test_harness .oneshot(request_with_appropriate_name) .await .unwrap(); + let response = service_response.next_response().await.unwrap(); + assert_eq!(response.errors, []); // Rhai should return a 200... assert_eq!(StatusCode::OK, service_response.response.status()); // with the expected message - let response = service_response.next_response().await.unwrap(); - assert!(response.errors.is_empty()); assert_eq!(expected_mock_response_data, response.data.as_ref().unwrap()); } } diff --git a/examples/rhai-subgraph-request-log/src/main.rs b/examples/rhai-subgraph-request-log/src/main.rs index 9a1adeb5d7..8db62705ad 100644 --- a/examples/rhai-subgraph-request-log/src/main.rs +++ b/examples/rhai-subgraph-request-log/src/main.rs @@ -12,37 +12,25 @@ fn main() -> Result<()> { #[cfg(test)] mod tests { - use apollo_router::plugin::test::IntoSchema::Canned; - use apollo_router::plugin::test::PluginTestHarness; - use apollo_router::plugin::Plugin; - use apollo_router::plugin::PluginInit; - use apollo_router::plugins::rhai::Conf; - use apollo_router::plugins::rhai::Rhai; use apollo_router::stages::router; use apollo_router::Context; use http::StatusCode; + use tower::util::ServiceExt; #[tokio::test] async fn test_subgraph_logs_data() { - // Define a configuration to use with our plugin - let conf: Conf = serde_json::from_value(serde_json::json!({ - "scripts": "src", - "main": "rhai_subgraph_request_log.rhai", - })) - .expect("valid conf supplied"); - - // Build an instance of our plugin to use in the test harness - let plugin = Rhai::new(PluginInit::new(conf, Default::default())) - .await - .expect("created plugin"); - - // Build a test harness. - let mut test_harness = PluginTestHarness::builder() - .plugin(plugin) - .schema(Canned) + let config = serde_json::json!({ + "rhai": { + "scripts": "src", + "main": "rhai_subgraph_request_log.rhai", + } + }); + let test_harness = apollo_router::TestHarness::builder() + .configuration_json(config) + .unwrap() .build() .await - .expect("building harness"); + .unwrap(); // The expected reply is going to be JSON returned in the RouterResponse { data } section. let _expected_mock_response_data = "response created within the mock"; @@ -52,7 +40,7 @@ mod tests { let operation_name: Option<&str> = None; let context: Option = None; let mut service_response = test_harness - .call( + .oneshot( router::Request::fake_builder() .header("name_header", "test_client") .header("version_header", "1.0-test") diff --git a/examples/rhai-surrogate-cache-key/src/main.rs b/examples/rhai-surrogate-cache-key/src/main.rs index 131cf8a634..f9e0939940 100644 --- a/examples/rhai-surrogate-cache-key/src/main.rs +++ b/examples/rhai-surrogate-cache-key/src/main.rs @@ -12,37 +12,25 @@ fn main() -> Result<()> { #[cfg(test)] mod tests { - use apollo_router::plugin::test::IntoSchema::Canned; - use apollo_router::plugin::test::PluginTestHarness; - use apollo_router::plugin::Plugin; - use apollo_router::plugin::PluginInit; - use apollo_router::plugins::rhai::Conf; - use apollo_router::plugins::rhai::Rhai; use apollo_router::stages::router; use apollo_router::Context; use http::StatusCode; + use tower::util::ServiceExt; #[tokio::test] async fn test_surrogate_cache_key_created() { - // Define a configuration to use with our plugin - let conf: Conf = serde_json::from_value(serde_json::json!({ - "scripts": "src", - "main": "rhai_surrogate_cache_key.rhai", - })) - .expect("valid conf supplied"); - - // Build an instance of our plugin to use in the test harness - let plugin = Rhai::new(PluginInit::new(conf, Default::default())) - .await - .expect("created plugin"); - - // Build a test harness. - let mut test_harness = PluginTestHarness::builder() - .plugin(plugin) - .schema(Canned) + let config = serde_json::json!({ + "rhai": { + "scripts": "src", + "main": "rhai_surrogate_cache_key.rhai", + } + }); + let test_harness = apollo_router::TestHarness::builder() + .configuration_json(config) + .unwrap() .build() .await - .expect("building harness"); + .unwrap(); // The expected reply is going to be JSON returned in the RouterResponse { data } section. let _expected_mock_response_data = "response created within the mock"; @@ -52,7 +40,7 @@ mod tests { let operation_name: Option<&str> = None; let context: Option = None; let mut service_response = test_harness - .call( + .oneshot( router::Request::fake_builder() .header("name_header", "test_client") .header("version_header", "1.0-test") From 0a79e205b3f7904f581642c2515e4ead7c63bcb0 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 9 Aug 2022 18:38:03 +0200 Subject: [PATCH 24/28] Port telemetry::metrics::apollo to the new test harness --- apollo-router/src/plugin/mod.rs | 18 ++++++--------- .../src/plugins/telemetry/metrics/apollo.rs | 15 +++++------- apollo-router/src/router_factory.rs | 13 +++++++---- apollo-router/src/test_harness.rs | 23 +++++++++++-------- 4 files changed, 35 insertions(+), 34 deletions(-) diff --git a/apollo-router/src/plugin/mod.rs b/apollo-router/src/plugin/mod.rs index 5d124082fa..3e23be9b6b 100644 --- a/apollo-router/src/plugin/mod.rs +++ b/apollo-router/src/plugin/mod.rs @@ -18,6 +18,7 @@ pub mod serde; #[macro_use] pub mod test; +use std::any::TypeId; use std::collections::HashMap; use std::sync::Arc; use std::sync::Mutex; @@ -92,16 +93,10 @@ where pub(crate) struct PluginFactory { instance_factory: InstanceFactory, schema_factory: SchemaFactory, + pub(crate) type_id: TypeId, } impl PluginFactory { - pub(crate) fn new(instance_factory: InstanceFactory, schema_factory: SchemaFactory) -> Self { - Self { - instance_factory, - schema_factory, - } - } - pub(crate) async fn create_instance( &self, configuration: &serde_json::Value, @@ -130,16 +125,17 @@ static PLUGIN_REGISTRY: Lazy>> = Lazy::new( /// Register a plugin factory. pub fn register_plugin(name: String) { - let plugin_factory = PluginFactory::new( - |configuration, schema| { + let plugin_factory = PluginFactory { + instance_factory: |configuration, schema| { Box::pin(async move { let init = PluginInit::try_new(configuration.clone(), schema)?; let plugin = P::new(init).await?; Ok(Box::new(plugin) as Box) }) }, - |gen| gen.subschema_for::<

::Config>(), - ); + schema_factory: |gen| gen.subschema_for::<

::Config>(), + type_id: TypeId::of::

(), + }; PLUGIN_REGISTRY .lock() .expect("Lock poisoned") diff --git a/apollo-router/src/plugins/telemetry/metrics/apollo.rs b/apollo-router/src/plugins/telemetry/metrics/apollo.rs index 2c07a5f5f6..f9e6a0d0f2 100644 --- a/apollo-router/src/plugins/telemetry/metrics/apollo.rs +++ b/apollo-router/src/plugins/telemetry/metrics/apollo.rs @@ -247,11 +247,10 @@ mod test { use std::future::Future; use http::header::HeaderName; + use tower::ServiceExt; use super::super::super::config; use super::*; - use crate::plugin::test::IntoSchema::Canned; - use crate::plugin::test::PluginTestHarness; use crate::plugin::Plugin; use crate::plugin::PluginInit; use crate::plugins::telemetry::apollo; @@ -259,6 +258,7 @@ mod test { use crate::plugins::telemetry::STUDIO_EXCLUDE; use crate::Context; use crate::RouterRequest; + use crate::TestHarness; #[tokio::test] async fn apollo_metrics_disabled() -> Result<(), BoxError> { @@ -356,13 +356,11 @@ mod test { // Replace the apollo metrics sender so we can test metrics collection. let (tx, rx) = futures::channel::mpsc::channel(100); plugin.apollo_metrics_sender = Sender::Spaceport(tx); - let mut test_harness = PluginTestHarness::builder() - .plugin(plugin) - .schema(Canned) + TestHarness::builder() + .extra_plugin(plugin) .build() - .await?; - let _ = test_harness - .call( + .await? + .oneshot( RouterRequest::fake_builder() .header("name_header", "test_client") .header("version_header", "1.0-test") @@ -377,7 +375,6 @@ mod test { .await .unwrap(); - drop(test_harness); let results = rx .collect::>() .await diff --git a/apollo-router/src/router_factory.rs b/apollo-router/src/router_factory.rs index 09c66be77f..db803cd4ab 100644 --- a/apollo-router/src/router_factory.rs +++ b/apollo-router/src/router_factory.rs @@ -72,10 +72,7 @@ impl RouterServiceConfigurator for YamlRouterServiceFactory { extra_plugins: Option)>>, ) -> Result { // Process the plugins. - let mut plugins = create_plugins(&configuration, &schema).await?; - if let Some(extra) = extra_plugins { - plugins.extend(extra); - } + let plugins = create_plugins(&configuration, &schema, extra_plugins).await?; let mut builder = PluggableRouterServiceBuilder::new(schema.clone()); builder = builder.with_configuration(configuration); @@ -130,6 +127,7 @@ caused by async fn create_plugins( configuration: &Configuration, schema: &Schema, + extra_plugins: Option)>>, ) -> Result)>, BoxError> { // List of mandatory plugins. Ordering is important!! let mandatory_plugins = vec![ @@ -141,9 +139,13 @@ async fn create_plugins( let mut errors = Vec::new(); let plugin_registry = crate::plugin::plugins(); let mut plugin_instances = Vec::new(); + let extra = extra_plugins.unwrap_or_default(); for (name, mut configuration) in configuration.plugins().into_iter() { - let name = name.clone(); + if extra.iter().any(|(n, _)| *n == name) { + // An instance of this plugin was already added through TestHarness::extra_plugin + continue; + } match plugin_registry.get(name.as_str()) { Some(factory) => { @@ -172,6 +174,7 @@ async fn create_plugins( None => errors.push(ConfigurationError::PluginUnknown(name)), } } + plugin_instances.extend(extra); // At this point we've processed all of the plugins that were provided in configuration. // We now need to do process our list of mandatory plugins: diff --git a/apollo-router/src/test_harness.rs b/apollo-router/src/test_harness.rs index a6caa1c2bb..7f9c6bded1 100644 --- a/apollo-router/src/test_harness.rs +++ b/apollo-router/src/test_harness.rs @@ -111,15 +111,20 @@ impl<'a> TestHarness<'a> { /// /// May be called multiple times. /// These extra plugins are added after plugins specified in configuration. - pub fn extra_plugin(mut self, plugin: impl Plugin) -> Self { - fn type_name_of(_: &T) -> &'static str { - std::any::type_name::() - } - let name = format!( - "extra_plugins.{}.{}", - self.extra_plugins.len(), - type_name_of(&plugin) - ); + pub fn extra_plugin(mut self, plugin: P) -> Self { + let type_id = std::any::TypeId::of::

(); + let name = match crate::plugin::plugins() + .iter() + .find(|(_name, factory)| factory.type_id == type_id) + { + Some((name, _factory)) => name.clone(), + None => format!( + "extra_plugins.{}.{}", + self.extra_plugins.len(), + std::any::type_name::

(), + ), + }; + self.extra_plugins.push((name, Box::new(plugin))); self } From 3c1787823f7514d51644795e9b86e5d6185348c5 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 9 Aug 2022 18:43:53 +0200 Subject: [PATCH 25/28] Remove PluginTestHarness, as it was replaced by TestHarness --- NEXT_CHANGELOG.md | 5 +- apollo-router/src/plugin/test/mod.rs | 196 +----------------- .../src/services/subgraph_service.rs | 2 +- 3 files changed, 6 insertions(+), 197 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index a1dea7a815..04ce88efcb 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -154,7 +154,6 @@ Migration example: } ``` - By [@SimonSapin](https://github.com/SimonSapin) ### Some items were removed from the public API ([PR #FIXME]) @@ -176,7 +175,9 @@ apollo_router::mock_service! apollo_router::plugin::plugins apollo_router::plugin::PluginFactory apollo_router::plugin::DynPlugin +apollo_router::plugin::test::IntoSchema apollo_router::plugin::test::MockSubgraphFactory +apollo_router::plugin::test::PluginTestHarness apollo_router::query_planner::QueryPlan::execute apollo_router::services apollo_router::Schema @@ -248,6 +249,8 @@ If you were using one of these constructors, the migration generally looks like + .build() ``` +By [@SimonSapin](https://github.com/SimonSapin) + ### Removed deprecated type aliases ([PR #FIXME]) A few versions ago, some types were moved from the crate root to a new `graphql` module. diff --git a/apollo-router/src/plugin/test/mod.rs b/apollo-router/src/plugin/test/mod.rs index 5671f8a6ab..db84770057 100644 --- a/apollo-router/src/plugin/test/mod.rs +++ b/apollo-router/src/plugin/test/mod.rs @@ -7,182 +7,20 @@ mod service; use std::collections::HashMap; use std::sync::Arc; -use indexmap::IndexMap; pub use mock::subgraph::MockSubgraph; pub use service::MockExecutionService; pub use service::MockQueryPlanningService; pub use service::MockRouterService; pub use service::MockSubgraphService; -use tower::buffer::Buffer; use tower::util::BoxService; use tower::BoxError; use tower::Service; -use tower::ServiceBuilder; -use tower::ServiceExt; pub(crate) use self::mock::canned; -use super::DynPlugin; -use crate::cache::DeduplicatingCache; -use crate::introspection::Introspection; -use crate::layers::DEFAULT_BUFFER_SIZE; -use crate::plugin::Plugin; -use crate::query_planner::BridgeQueryPlanner; -use crate::query_planner::CachingQueryPlanner; -use crate::services::layers::apq::APQLayer; -use crate::services::layers::ensure_query_presence::EnsureQueryPresence; -use crate::services::subgraph_service::MakeSubgraphService; use crate::services::subgraph_service::SubgraphServiceFactory; -use crate::services::ExecutionCreator; +use crate::services::MakeSubgraphService; use crate::services::Plugins; -use crate::services::RouterRequest; -use crate::services::RouterResponse; use crate::services::SubgraphRequest; -use crate::stages::router; -use crate::Configuration; -use crate::RouterService; -use crate::Schema; - -pub struct PluginTestHarness { - router_service: router::BoxService, -} -pub enum IntoSchema { - String(String), - Schema(Box), - Canned, -} - -impl From for IntoSchema { - fn from(schema: Schema) -> Self { - IntoSchema::Schema(Box::new(schema)) - } -} -impl From for IntoSchema { - fn from(schema: String) -> Self { - IntoSchema::String(schema) - } -} - -impl IntoSchema { - fn into_schema(self, config: &Configuration) -> Schema { - match self { - IntoSchema::String(s) => Schema::parse(&s, config).expect("test schema must be valid"), - IntoSchema::Schema(s) => *s, - IntoSchema::Canned => Schema::parse( - include_str!("../../../../examples/graphql/local.graphql"), - config, - ) - .expect("test schema must be valid"), - } - } -} - -#[buildstructor::buildstructor] -impl PluginTestHarness { - /// Plugin test harness gives you an easy way to test your plugins against a mock subgraph. - /// Currently mocking is basic, and only a request for topProducts is supported - /// - /// # Arguments - /// - /// * `plugin`: The plugin to test - /// * `schema`: The schema, either Canned, or a custom schema. - /// * `mock_router_service`: (Optional) router service. If none is supplied it will be defaulted. - /// * `mock_query_planner_service`: (Optional) query planner service. If none is supplied it will be defaulted. - /// * `mock_execution_service`: (Optional) execution service. If none is supplied it will be defaulted. - /// * `mock_subgraph_services`: (Optional) subgraph service. If none is supplied it will be defaulted. - /// - /// returns: Result> - /// - #[builder(visibility = "pub")] - async fn new( - plugin: P, - schema: IntoSchema, - mock_router_service: Option, - mock_query_planner_service: Option, - mut subgraph_services: HashMap>, - ) -> Result { - // If we're using the canned schema then add some canned results - if let IntoSchema::Canned = schema { - subgraph_services - .entry("products".to_string()) - .or_insert_with(|| Arc::new(mock::canned::products_subgraph())); - subgraph_services - .entry("accounts".to_string()) - .or_insert_with(|| Arc::new(mock::canned::accounts_subgraph())); - subgraph_services - .entry("reviews".to_string()) - .or_insert_with(|| Arc::new(mock::canned::reviews_subgraph())); - } - - let schema = Arc::new(schema.into_schema(&Default::default())); - - let query_planner = CachingQueryPlanner::new( - BridgeQueryPlanner::new( - schema.clone(), - Some(Arc::new(Introspection::from_schema(&schema))), - Default::default(), - ) - .await?, - DEFAULT_BUFFER_SIZE, - ) - .await - .boxed(); - let query_planner_service = plugin.query_planning_service( - mock_query_planner_service - .map(|s| s.boxed()) - .unwrap_or(query_planner), - ); - - let mut plugins = IndexMap::new(); - plugins.insert( - "tested_plugin".to_string(), - Box::new(plugin) as Box, - ); - let plugins = Arc::new(plugins); - - let apq = APQLayer::with_cache(DeduplicatingCache::new().await); - let router_service = mock_router_service.map(|s| s.boxed()).unwrap_or_else(|| { - BoxService::new( - RouterService::builder() - .query_planner_service(Buffer::new(query_planner_service, DEFAULT_BUFFER_SIZE)) - .execution_service_factory(ExecutionCreator { - schema: schema.clone(), - plugins: plugins.clone(), - subgraph_creator: Arc::new(MockSubgraphFactory { - plugins: plugins.clone(), - subgraphs: subgraph_services, - }), - }) - .schema(schema.clone()) - .build(), - ) - }); - let router_service = ServiceBuilder::new() - .layer(apq) - .layer(EnsureQueryPresence::default()) - .service( - plugins - .get("tested_plugin") - .unwrap() - .router_service(router_service), - ) - .boxed(); - Ok(Self { router_service }) - } - - /// Call the test harness with a request. Not that you will need to have set up appropriate responses via mocks. - pub async fn call(&mut self, request: RouterRequest) -> Result { - self.router_service.ready().await?.call(request).await - } - - /// If using the canned schema this canned request will give a response. - pub async fn call_canned(&mut self) -> Result { - self.router_service - .ready() - .await? - .call(RouterRequest::canned_builder().build().unwrap()) - .await - } -} #[derive(Clone)] pub(crate) struct MockSubgraphFactory { @@ -207,35 +45,3 @@ impl SubgraphServiceFactory for MockSubgraphFactory { }) } } - -#[cfg(test)] -mod testing { - use insta::assert_json_snapshot; - - use super::*; - use crate::plugin::PluginInit; - - struct EmptyPlugin {} - #[async_trait::async_trait] - impl Plugin for EmptyPlugin { - type Config = (); - - async fn new(_init: PluginInit) -> Result { - Ok(Self {}) - } - } - - #[tokio::test] - async fn test_test_harness() -> Result<(), BoxError> { - let mut harness = PluginTestHarness::builder() - .plugin(EmptyPlugin {}) - .schema(IntoSchema::Canned) - .build() - .await?; - let graphql = harness.call_canned().await?.next_response().await.unwrap(); - insta::with_settings!({sort_maps => true}, { - assert_json_snapshot!(graphql.data); - }); - Ok(()) - } -} diff --git a/apollo-router/src/services/subgraph_service.rs b/apollo-router/src/services/subgraph_service.rs index a40d380504..21de461eac 100644 --- a/apollo-router/src/services/subgraph_service.rs +++ b/apollo-router/src/services/subgraph_service.rs @@ -301,7 +301,7 @@ impl SubgraphCreator { /// make new instances of the subgraph service /// /// there can be multiple instances of that service executing at any given time -pub trait MakeSubgraphService: Send + Sync + 'static { +pub(crate) trait MakeSubgraphService: Send + Sync + 'static { fn make(&self) -> BoxService; } From d43081353d5b12a9ee64f301d87d812ebec3dd5b Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 9 Aug 2022 18:56:12 +0200 Subject: [PATCH 26/28] Make Schema actually private --- apollo-router/src/spec/schema.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/apollo-router/src/spec/schema.rs b/apollo-router/src/spec/schema.rs index 8e8ea8a026..6b92bec12a 100644 --- a/apollo-router/src/spec/schema.rs +++ b/apollo-router/src/spec/schema.rs @@ -21,7 +21,7 @@ use crate::*; /// A GraphQL schema. #[derive(Debug, Default, Clone)] -pub struct Schema { +pub(crate) struct Schema { string: Arc, subtype_map: HashMap>, subgraphs: HashMap, @@ -31,12 +31,12 @@ pub struct Schema { pub(crate) custom_scalars: HashSet, pub(crate) enums: HashMap>, api_schema: Option>, - pub schema_id: Option, + pub(crate) schema_id: Option, root_operations: HashMap, } impl Schema { - pub fn parse(s: &str, configuration: &Configuration) -> Result { + pub(crate) fn parse(s: &str, configuration: &Configuration) -> Result { let mut schema = parse(s, configuration)?; schema.api_schema = Some(Box::new(api_schema(s, configuration)?)); return Ok(schema); @@ -432,7 +432,7 @@ impl Schema { impl Schema { /// Extracts a string containing the entire [`Schema`]. - pub fn as_string(&self) -> &Arc { + pub(crate) fn as_string(&self) -> &Arc { &self.string } @@ -444,21 +444,17 @@ impl Schema { } /// Return an iterator over subgraphs that yields the subgraph name and its URL. - pub fn subgraphs(&self) -> impl Iterator { + pub(crate) fn subgraphs(&self) -> impl Iterator { self.subgraphs.iter() } - pub fn api_schema(&self) -> &Schema { + pub(crate) fn api_schema(&self) -> &Schema { match &self.api_schema { Some(schema) => schema, None => self, } } - pub fn boxed(self) -> Box { - Box::new(self) - } - fn with_introspection(schema: &str) -> String { format!( "{}\n{}", From 3499374ec67a95e4d9796061839f222ab185cfd8 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 9 Aug 2022 18:56:21 +0200 Subject: [PATCH 27/28] Make plugins private --- NEXT_CHANGELOG.md | 1 + apollo-router/src/lib.rs | 33 +++++++------ apollo-router/src/plugins/csrf.rs | 4 +- apollo-router/src/plugins/mod.rs | 8 ++-- apollo-router/src/plugins/rhai.rs | 4 +- apollo-router/src/plugins/telemetry/apollo.rs | 12 ++--- apollo-router/src/plugins/telemetry/config.rs | 48 +++++++++---------- .../src/plugins/telemetry/metrics/mod.rs | 2 +- .../plugins/telemetry/metrics/prometheus.rs | 2 +- apollo-router/src/plugins/telemetry/mod.rs | 6 +-- apollo-router/src/plugins/telemetry/otlp.rs | 30 ++++++------ .../src/plugins/telemetry/tracing/datadog.rs | 4 +- .../src/plugins/telemetry/tracing/jaeger.rs | 8 ++-- .../src/plugins/telemetry/tracing/mod.rs | 4 +- .../src/plugins/telemetry/tracing/zipkin.rs | 4 +- apollo-router/src/router_factory.rs | 2 +- apollo-router/tests/integration_tests.rs | 4 +- apollo-router/tests/telemetry_test.rs | 4 +- 18 files changed, 92 insertions(+), 88 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 04ce88efcb..30b9eef3f8 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -172,6 +172,7 @@ apollo_router::errors::QueryPlannerError apollo_router::errors::ServiceBuildError apollo_router::json_ext apollo_router::mock_service! +apollo_router::plugins apollo_router::plugin::plugins apollo_router::plugin::PluginFactory apollo_router::plugin::DynPlugin diff --git a/apollo-router/src/lib.rs b/apollo-router/src/lib.rs index 5a6136c82c..059e4a8437 100644 --- a/apollo-router/src/lib.rs +++ b/apollo-router/src/lib.rs @@ -62,7 +62,7 @@ pub mod graphql; mod http_server_factory; mod introspection; pub mod layers; -pub mod plugins; +mod plugins; pub mod query_planner; mod request; mod response; @@ -74,26 +74,29 @@ pub mod stages; mod state_machine; mod test_harness; -pub use configuration::Configuration; -pub use configuration::ListenAddr; -pub use context::Context; -pub use executable::main; -pub use executable::Executable; -pub use router::ConfigurationSource; -pub use router::RouterHttpServer; -pub use router::SchemaSource; -pub use router::ShutdownSource; -#[doc(hidden)] -pub use router_factory::__create_test_service_factory_from_yaml; -pub use services::http_ext; -pub use test_harness::TestHarness; +pub use crate::configuration::Configuration; +pub use crate::configuration::ListenAddr; +pub use crate::context::Context; +pub use crate::executable::main; +pub use crate::executable::Executable; +pub use crate::router::ConfigurationSource; +pub use crate::router::RouterHttpServer; +pub use crate::router::SchemaSource; +pub use crate::router::ShutdownSource; +pub use crate::services::http_ext; +pub use crate::test_harness::TestHarness; -/// Reexports for macros +/// Not part of the public API #[doc(hidden)] pub mod _private { + // Reexports for macros pub use router_bridge; pub use serde_json; pub use startup; + + // For tests + pub use crate::plugins::telemetry::Telemetry as TelemetryPlugin; + pub use crate::router_factory::create_test_service_factory_from_yaml; } // TODO: clean these up and import from relevant modules instead diff --git a/apollo-router/src/plugins/csrf.rs b/apollo-router/src/plugins/csrf.rs index e05b19d820..6fc59866da 100644 --- a/apollo-router/src/plugins/csrf.rs +++ b/apollo-router/src/plugins/csrf.rs @@ -21,7 +21,7 @@ use crate::RouterResponse; /// CSRF Configuration. #[derive(Deserialize, Debug, Clone, JsonSchema)] #[serde(deny_unknown_fields)] -pub struct CSRFConfig { +pub(crate) struct CSRFConfig { /// The CSRF plugin is enabled by default; /// set unsafe_disabled = true to disable the plugin behavior /// Note that setting this to true is deemed unsafe. @@ -87,7 +87,7 @@ static NON_PREFLIGHTED_CONTENT_TYPES: &[&str] = &[ /// won't execute operations at the request of origins who our CORS policy will /// block. #[derive(Debug, Clone)] -pub struct Csrf { +pub(crate) struct Csrf { config: CSRFConfig, } diff --git a/apollo-router/src/plugins/mod.rs b/apollo-router/src/plugins/mod.rs index aee7a9c5fb..7c4401d275 100644 --- a/apollo-router/src/plugins/mod.rs +++ b/apollo-router/src/plugins/mod.rs @@ -2,12 +2,12 @@ //! //! These plugins are compiled into the router and configured via YAML configuration. -pub mod csrf; +pub(crate) mod csrf; mod expose_query_plan; mod forbid_mutations; mod headers; mod include_subgraph_errors; -pub mod override_url; -pub mod rhai; -pub mod telemetry; +pub(crate) mod override_url; +pub(crate) mod rhai; +pub(crate) mod telemetry; pub(crate) mod traffic_shaping; diff --git a/apollo-router/src/plugins/rhai.rs b/apollo-router/src/plugins/rhai.rs index a6fe745831..669a6b6d20 100644 --- a/apollo-router/src/plugins/rhai.rs +++ b/apollo-router/src/plugins/rhai.rs @@ -293,7 +293,7 @@ mod router_plugin_mod { /// Plugin which implements Rhai functionality #[derive(Default, Clone)] -pub struct Rhai { +pub(crate) struct Rhai { ast: AST, engine: Arc, } @@ -301,7 +301,7 @@ pub struct Rhai { /// Configuration for the Rhai Plugin #[derive(Deserialize, JsonSchema)] #[serde(deny_unknown_fields)] -pub struct Conf { +pub(crate) struct Conf { scripts: Option, main: Option, } diff --git a/apollo-router/src/plugins/telemetry/apollo.rs b/apollo-router/src/plugins/telemetry/apollo.rs index fbacd15368..c4bb01f98a 100644 --- a/apollo-router/src/plugins/telemetry/apollo.rs +++ b/apollo-router/src/plugins/telemetry/apollo.rs @@ -9,31 +9,31 @@ use crate::plugin::serde::deserialize_header_name; #[derive(Debug, Clone, Deserialize, JsonSchema)] #[serde(deny_unknown_fields)] -pub struct Config { +pub(crate) struct Config { #[schemars(with = "Option")] - pub endpoint: Option, + pub(crate) endpoint: Option, #[schemars(skip)] #[serde(skip, default = "apollo_key")] - pub apollo_key: Option, + pub(crate) apollo_key: Option, #[schemars(skip)] #[serde(skip, default = "apollo_graph_reference")] - pub apollo_graph_ref: Option, + pub(crate) apollo_graph_ref: Option, #[schemars(with = "Option", default = "client_name_header_default_str")] #[serde( deserialize_with = "deserialize_header_name", default = "client_name_header_default" )] - pub client_name_header: HeaderName, + pub(crate) client_name_header: HeaderName, #[schemars(with = "Option", default = "client_version_header_default_str")] #[serde( deserialize_with = "deserialize_header_name", default = "client_version_header_default" )] - pub client_version_header: HeaderName, + pub(crate) client_version_header: HeaderName, // This'll get overridden if a user tries to set it. // The purpose is to allow is to pass this in to the plugin. diff --git a/apollo-router/src/plugins/telemetry/config.rs b/apollo-router/src/plugins/telemetry/config.rs index a32e5a54dc..e8663961c8 100644 --- a/apollo-router/src/plugins/telemetry/config.rs +++ b/apollo-router/src/plugins/telemetry/config.rs @@ -13,7 +13,7 @@ use super::metrics::MetricsAttributesConf; use super::*; use crate::plugins::telemetry::metrics; -pub trait GenericWith +pub(crate) trait GenericWith where Self: Sized, { @@ -41,49 +41,49 @@ impl GenericWith for T where Self: Sized {} #[serde(deny_unknown_fields, rename_all = "snake_case")] pub struct Conf { #[allow(dead_code)] - pub metrics: Option, - pub tracing: Option, - pub apollo: Option, + pub(crate) metrics: Option, + pub(crate) tracing: Option, + pub(crate) apollo: Option, } #[derive(Clone, Default, Debug, Deserialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case")] #[allow(dead_code)] -pub struct Metrics { - pub common: Option, - pub otlp: Option, - pub prometheus: Option, +pub(crate) struct Metrics { + pub(crate) common: Option, + pub(crate) otlp: Option, + pub(crate) prometheus: Option, } #[derive(Clone, Default, Debug, Deserialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case")] -pub struct MetricsCommon { +pub(crate) struct MetricsCommon { /// Configuration to add custom labels/attributes to metrics - pub attributes: Option, + pub(crate) attributes: Option, #[serde(default)] /// Resources - pub resources: HashMap, + pub(crate) resources: HashMap, } #[derive(Clone, Default, Debug, Deserialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case")] -pub struct Tracing { - pub propagation: Option, - pub trace_config: Option, - pub otlp: Option, - pub jaeger: Option, - pub zipkin: Option, - pub datadog: Option, +pub(crate) struct Tracing { + pub(crate) propagation: Option, + pub(crate) trace_config: Option, + pub(crate) otlp: Option, + pub(crate) jaeger: Option, + pub(crate) zipkin: Option, + pub(crate) datadog: Option, } #[derive(Clone, Default, Debug, Deserialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case")] -pub struct Propagation { - pub baggage: Option, - pub trace_context: Option, - pub jaeger: Option, - pub datadog: Option, - pub zipkin: Option, +pub(crate) struct Propagation { + pub(crate) baggage: Option, + pub(crate) trace_context: Option, + pub(crate) jaeger: Option, + pub(crate) datadog: Option, + pub(crate) zipkin: Option, } #[derive(Default, Debug, Clone, Deserialize, JsonSchema)] diff --git a/apollo-router/src/plugins/telemetry/metrics/mod.rs b/apollo-router/src/plugins/telemetry/metrics/mod.rs index fe7eb56e9b..e135d28b2a 100644 --- a/apollo-router/src/plugins/telemetry/metrics/mod.rs +++ b/apollo-router/src/plugins/telemetry/metrics/mod.rs @@ -46,7 +46,7 @@ pub(crate) type CustomEndpoint = #[derive(Debug, Clone, Deserialize, JsonSchema)] #[serde(deny_unknown_fields)] /// Configuration to add custom attributes/labels on metrics -pub struct MetricsAttributesConf { +pub(crate) struct MetricsAttributesConf { /// Configuration to forward header values or body values from router request/response in metric attributes/labels pub(crate) router: Option, /// Configuration to forward header values or body values from subgraph request/response in metric attributes/labels diff --git a/apollo-router/src/plugins/telemetry/metrics/prometheus.rs b/apollo-router/src/plugins/telemetry/metrics/prometheus.rs index ecad837f97..343f4b9a67 100644 --- a/apollo-router/src/plugins/telemetry/metrics/prometheus.rs +++ b/apollo-router/src/plugins/telemetry/metrics/prometheus.rs @@ -22,7 +22,7 @@ use crate::plugins::telemetry::metrics::MetricsConfigurator; #[derive(Debug, Clone, Deserialize, JsonSchema)] #[serde(deny_unknown_fields)] -pub struct Config { +pub(crate) struct Config { enabled: bool, } diff --git a/apollo-router/src/plugins/telemetry/mod.rs b/apollo-router/src/plugins/telemetry/mod.rs index 1c668f131f..f898ae0e92 100644 --- a/apollo-router/src/plugins/telemetry/mod.rs +++ b/apollo-router/src/plugins/telemetry/mod.rs @@ -79,8 +79,8 @@ use crate::RouterResponse; use crate::SubgraphRequest; use crate::SubgraphResponse; -pub mod apollo; -pub mod config; +pub(crate) mod apollo; +pub(crate) mod config; mod metrics; mod otlp; mod tracing; @@ -534,7 +534,7 @@ impl Telemetry { } /// This method can be used instead of `Plugin::new` to override the subscriber - pub async fn new_common( + async fn new_common( mut config: ::Config, subscriber: Option, ) -> Result diff --git a/apollo-router/src/plugins/telemetry/otlp.rs b/apollo-router/src/plugins/telemetry/otlp.rs index 5bfc4a05e6..da60fccbc8 100644 --- a/apollo-router/src/plugins/telemetry/otlp.rs +++ b/apollo-router/src/plugins/telemetry/otlp.rs @@ -24,21 +24,21 @@ use crate::plugins::telemetry::tracing::parse_url_for_endpoint; #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields)] -pub struct Config { +pub(crate) struct Config { #[serde(deserialize_with = "deser_endpoint")] #[schemars(with = "String")] - pub endpoint: Endpoint, - pub protocol: Option, + pub(crate) endpoint: Endpoint, + pub(crate) protocol: Option, #[serde(deserialize_with = "humantime_serde::deserialize", default)] #[schemars(with = "String", default)] - pub timeout: Option, - pub grpc: Option, - pub http: Option, + pub(crate) timeout: Option, + pub(crate) grpc: Option, + pub(crate) http: Option, } impl Config { - pub fn exporter + From>( + pub(crate) fn exporter + From>( &self, ) -> Result { let endpoint = match &self.endpoint { @@ -79,7 +79,7 @@ impl Config { #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case", untagged)] -pub enum Endpoint { +pub(crate) enum Endpoint { Default(EndpointDefault), Url(Url), } @@ -100,28 +100,28 @@ where #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case")] -pub enum EndpointDefault { +pub(crate) enum EndpointDefault { Default, } #[derive(Debug, Clone, Deserialize, Serialize, Default, JsonSchema)] #[serde(deny_unknown_fields)] -pub struct HttpExporter { - pub headers: Option>, +pub(crate) struct HttpExporter { + pub(crate) headers: Option>, } #[derive(Debug, Clone, Deserialize, Serialize, Default, JsonSchema)] #[serde(deny_unknown_fields)] -pub struct GrpcExporter { +pub(crate) struct GrpcExporter { #[serde(flatten)] - pub tls_config: Option, + pub(crate) tls_config: Option, #[serde( deserialize_with = "metadata_map_serde::deserialize", serialize_with = "metadata_map_serde::serialize", default )] #[schemars(schema_with = "option_metadata_map", default)] - pub metadata: Option, + pub(crate) metadata: Option, } fn option_metadata_map(gen: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema { @@ -178,7 +178,7 @@ impl Secret { #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case")] -pub enum Protocol { +pub(crate) enum Protocol { Grpc, Http, } diff --git a/apollo-router/src/plugins/telemetry/tracing/datadog.rs b/apollo-router/src/plugins/telemetry/tracing/datadog.rs index f19b366eb5..1dacfe529a 100644 --- a/apollo-router/src/plugins/telemetry/tracing/datadog.rs +++ b/apollo-router/src/plugins/telemetry/tracing/datadog.rs @@ -13,10 +13,10 @@ use crate::plugins::telemetry::tracing::TracingConfigurator; #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields)] -pub struct Config { +pub(crate) struct Config { #[serde(deserialize_with = "deser_endpoint")] #[schemars(with = "String", default = "default_agent_endpoint")] - pub endpoint: AgentEndpoint, + pub(crate) endpoint: AgentEndpoint, } const fn default_agent_endpoint() -> &'static str { "default" diff --git a/apollo-router/src/plugins/telemetry/tracing/jaeger.rs b/apollo-router/src/plugins/telemetry/tracing/jaeger.rs index 4192881596..3e9411c92f 100644 --- a/apollo-router/src/plugins/telemetry/tracing/jaeger.rs +++ b/apollo-router/src/plugins/telemetry/tracing/jaeger.rs @@ -20,14 +20,14 @@ use crate::plugins::telemetry::tracing::TracingConfigurator; #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] // Can't use #[serde(deny_unknown_fields)] because we're using flatten for endpoint -pub struct Config { +pub(crate) struct Config { #[serde(flatten)] #[schemars(schema_with = "endpoint_schema")] - pub endpoint: Endpoint, + pub(crate) endpoint: Endpoint, #[serde(deserialize_with = "humantime_serde::deserialize", default)] #[schemars(with = "String", default)] - pub scheduled_delay: Option, + pub(crate) scheduled_delay: Option, } // This is needed because of the use of flatten. @@ -57,7 +57,7 @@ fn endpoint_schema(gen: &mut SchemaGenerator) -> Schema { #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case")] -pub enum Endpoint { +pub(crate) enum Endpoint { Agent { #[schemars(with = "String", default = "default_agent_endpoint")] #[serde(deserialize_with = "deser_endpoint")] diff --git a/apollo-router/src/plugins/telemetry/tracing/mod.rs b/apollo-router/src/plugins/telemetry/tracing/mod.rs index b8a3d1acf9..8fd5a1b300 100644 --- a/apollo-router/src/plugins/telemetry/tracing/mod.rs +++ b/apollo-router/src/plugins/telemetry/tracing/mod.rs @@ -22,14 +22,14 @@ pub(crate) trait TracingConfigurator { #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case", untagged)] -pub enum AgentEndpoint { +pub(crate) enum AgentEndpoint { Default(AgentDefault), Url(Url), } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case")] -pub enum AgentDefault { +pub(crate) enum AgentDefault { Default, } diff --git a/apollo-router/src/plugins/telemetry/tracing/zipkin.rs b/apollo-router/src/plugins/telemetry/tracing/zipkin.rs index b86e2c5901..880580943d 100644 --- a/apollo-router/src/plugins/telemetry/tracing/zipkin.rs +++ b/apollo-router/src/plugins/telemetry/tracing/zipkin.rs @@ -15,10 +15,10 @@ use crate::plugins::telemetry::tracing::TracingConfigurator; #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] #[serde(deny_unknown_fields)] -pub struct Config { +pub(crate) struct Config { #[schemars(with = "String", default = "default_agent_endpoint")] #[serde(deserialize_with = "deser_endpoint")] - pub endpoint: AgentEndpoint, + pub(crate) endpoint: AgentEndpoint, } fn default_agent_endpoint() -> &'static str { diff --git a/apollo-router/src/router_factory.rs b/apollo-router/src/router_factory.rs index db803cd4ab..95206b4655 100644 --- a/apollo-router/src/router_factory.rs +++ b/apollo-router/src/router_factory.rs @@ -104,7 +104,7 @@ impl RouterServiceConfigurator for YamlRouterServiceFactory { /// test only helper method to create a router factory in integration tests /// /// not meant to be used directly -pub async fn __create_test_service_factory_from_yaml(schema: &str, configuration: &str) { +pub async fn create_test_service_factory_from_yaml(schema: &str, configuration: &str) { let config: Configuration = serde_yaml::from_str(configuration).unwrap(); let schema: Schema = Schema::parse(schema, &Default::default()).unwrap(); diff --git a/apollo-router/tests/integration_tests.rs b/apollo-router/tests/integration_tests.rs index 7dfcc135b0..7b38b3d6a3 100644 --- a/apollo-router/tests/integration_tests.rs +++ b/apollo-router/tests/integration_tests.rs @@ -12,10 +12,10 @@ use apollo_router::graphql::Request; use apollo_router::http_ext; use apollo_router::plugin::Plugin; use apollo_router::plugin::PluginInit; -use apollo_router::plugins::telemetry::Telemetry; use apollo_router::stages::router; use apollo_router::stages::subgraph; use apollo_router::Context; +use apollo_router::_private::TelemetryPlugin; use http::Method; use maplit::hashmap; use serde_json::to_string_pretty; @@ -680,7 +680,7 @@ async fn setup_router_and_registry( ) -> (router::BoxCloneService, CountingServiceRegistry) { let config = serde_json::from_value(config).unwrap(); let counting_registry = CountingServiceRegistry::new(); - let telemetry = Telemetry::new_with_subscriber( + let telemetry = TelemetryPlugin::new_with_subscriber( serde_json::json!({ "tracing": {}, "apollo": { diff --git a/apollo-router/tests/telemetry_test.rs b/apollo-router/tests/telemetry_test.rs index ba67d06b6b..80ac4a6ba8 100644 --- a/apollo-router/tests/telemetry_test.rs +++ b/apollo-router/tests/telemetry_test.rs @@ -1,10 +1,10 @@ -use apollo_router::__create_test_service_factory_from_yaml; +use apollo_router::_private::create_test_service_factory_from_yaml; // This test must use the multi_thread tokio executor or the opentelemetry hang bug will // be encountered. (See https://github.com/open-telemetry/opentelemetry-rust/issues/536) #[tokio::test(flavor = "multi_thread")] async fn test_telemetry_doesnt_hang_with_invalid_schema() { - __create_test_service_factory_from_yaml( + create_test_service_factory_from_yaml( include_str!("../src/testdata/invalid_supergraph.graphql"), r#" telemetry: From 56690dbddfb4f52100e6c11caaf7f5cc41b3503f Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 12 Aug 2022 16:39:33 +0200 Subject: [PATCH 28/28] Add apollo_router::stages::*::Result aliases They alias Result --- apollo-router/src/stages.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apollo-router/src/stages.rs b/apollo-router/src/stages.rs index a8a755cd1e..ce942919af 100644 --- a/apollo-router/src/stages.rs +++ b/apollo-router/src/stages.rs @@ -8,6 +8,7 @@ pub mod router { pub use crate::services::RouterResponse as Response; pub type BoxService = tower::util::BoxService; pub type BoxCloneService = tower::util::BoxCloneService; + pub type Result = std::result::Result; } pub mod query_planner { @@ -16,6 +17,7 @@ pub mod query_planner { pub use crate::services::QueryPlannerResponse as Response; pub type BoxService = tower::util::BoxService; pub type BoxCloneService = tower::util::BoxCloneService; + pub type Result = std::result::Result; // Reachable from Request or Response: pub use crate::query_planner::QueryPlan; @@ -29,6 +31,7 @@ pub mod execution { pub use crate::services::ExecutionResponse as Response; pub type BoxService = tower::util::BoxService; pub type BoxCloneService = tower::util::BoxCloneService; + pub type Result = std::result::Result; } pub mod subgraph { @@ -37,6 +40,7 @@ pub mod subgraph { pub use crate::services::SubgraphResponse as Response; pub type BoxService = tower::util::BoxService; pub type BoxCloneService = tower::util::BoxCloneService; + pub type Result = std::result::Result; // Reachable from Request or Response: pub use crate::query_planner::OperationKind;