reorder query planner execution

NEXT_CHANGELOG.md

@@ -62,7 +62,18 @@ It is now possible to create a subscriber and pass it explicitely to the telemet
 when creating it. It will then be modified to integrate the telemetry plugin's layer.
 
 By [@geal](https://github.com/geal) in https://github.com/apollographql/router/pull/1463
->>>>>>> be6590826afb60bf3c683315c68f9ed47594a2a3
+
+
+### Reorder query planner execution ([PR #1484](https://github.com/apollographql/router/pull/1484))
+
+Query planning is deterministic, it only depends on the query, operation name and query planning
+options. As such, we can cache the result of the entire process.
+
+This changes the pipeline to apply query planner plugins between the cache and the bridge planner,
+so those plugins will only be called once on the same query. If changes must be done per query,
+they should happen in a supergraph service.
+
+By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/1464
 
 ## 🚀 Features
 

apollo-router/src/error.rs

@@ -15,6 +15,7 @@ use serde::Deserialize;
 use serde::Serialize;
 use thiserror::Error;
 use tokio::task::JoinError;
+use tower::BoxError;
 use tracing::level_filters::LevelFilter;
 
 pub use crate::configuration::ConfigurationError;
@@ -157,11 +158,11 @@ impl From<QueryPlannerError> for FetchError {
 #[derive(Error, Debug, Display, Clone)]
 pub enum CacheResolverError {
     /// value retrieval failed: {0}
-    RetrievalError(Arc<QueryPlannerError>),
+    RetrievalError(Arc<BoxError>),
 }
 
-impl From<QueryPlannerError> for CacheResolverError {
-    fn from(err: QueryPlannerError) -> Self {
+impl From<BoxError> for CacheResolverError {
+    fn from(err: BoxError) -> Self {
         CacheResolverError::RetrievalError(Arc::new(err))
     }
 }

apollo-router/src/lib.rs

@@ -69,7 +69,6 @@ mod router_factory;
 pub mod services;
 mod spec;
 mod state_machine;
-mod traits;
 
 pub use configuration::*;
 pub use context::Context;

apollo-router/src/plugin/mod.rs

@@ -173,6 +173,10 @@ pub trait Plugin: Send + Sync + 'static + Sized {
 
     /// 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).
+    ///
+    /// Query planning uses a cache that will store the result of the query planner and query planning plugins execution, so if the same query is
+    /// performed twice, the query planner plugins will onyl see it once. The caching key contains the query and operation name. If modifications
+    /// must be performed on the query, they should be done in router service plugins.
     fn query_planning_service(
         &self,
         service: BoxService<QueryPlannerRequest, QueryPlannerResponse, BoxError>,
@@ -238,6 +242,10 @@ pub trait DynPlugin: Send + Sync + 'static {
 
     /// 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).
+    ///
+    /// Query planning uses a cache that will store the result of the query planner and query planning plugins execution, so if the same query is
+    /// performed twice, the query planner plugins will onyl see it once. The caching key contains the query and operation name. If modifications
+    /// must be performed on the query, they should be done in router service plugins.
     fn query_planning_service(
         &self,
         service: BoxService<QueryPlannerRequest, QueryPlannerResponse, BoxError>,

apollo-router/src/query_planner/bridge_query_planner.rs

@@ -3,7 +3,6 @@
 use std::fmt::Debug;
 use std::sync::Arc;
 
-use async_trait::async_trait;
 use futures::future::BoxFuture;
 use opentelemetry::trace::SpanKind;
 use router_bridge::planner::DeferStreamSupport;
@@ -16,12 +15,11 @@ use tower::Service;
 use tracing::Instrument;
 
 use super::PlanNode;
+use super::QueryKey;
 use super::QueryPlanOptions;
 use crate::error::QueryPlannerError;
 use crate::introspection::Introspection;
 use crate::services::QueryPlannerContent;
-use crate::traits::QueryKey;
-use crate::traits::QueryPlanner;
 use crate::*;
 
 pub(crate) static USAGE_REPORTING: &str = "apollo_telemetry::usage_reporting";
@@ -172,8 +170,7 @@ impl Service<QueryPlannerRequest> for BridgeQueryPlanner {
     }
 }
 
-#[async_trait]
-impl QueryPlanner for BridgeQueryPlanner {
+impl BridgeQueryPlanner {
     async fn get(&self, key: QueryKey) -> Result<QueryPlannerContent, QueryPlannerError> {
         let selections = self.parse_selections(key.0.clone()).await?;