refactor(query_planner): put queryplan options in private and wrap QueryPlanContent in an opaque type

apollo-router/src/query_planner/bridge_query_planner.rs

@@ -84,11 +84,12 @@ impl BridgeQueryPlanner {
                     .await
                     .map_err(QueryPlannerError::Introspection)?;
 
-                Ok(QueryPlannerContent::Introspection {
+                Ok(QueryPlannerContentInner::Introspection {
                     response: Box::new(response),
-                })
+                }
+                .into())
             }
-            None => Ok(QueryPlannerContent::IntrospectionDisabled),
+            None => Ok(QueryPlannerContentInner::IntrospectionDisabled.into()),
         }
     }
 
@@ -114,14 +115,15 @@ impl BridgeQueryPlanner {
             } => {
                 let subselections = node.parse_subselections(&*self.schema);
                 selections.subselections = subselections;
-                Ok(QueryPlannerContent::Plan {
+                Ok(QueryPlannerContentInner::Plan {
                     plan: Arc::new(query_planner::QueryPlan {
                         usage_reporting,
                         root: node,
                         options,
                     }),
                     query: Arc::new(selections),
-                })
+                }
+                .into())
             }
             PlanSuccess {
                 data: QueryPlan { node: None },
@@ -217,7 +219,7 @@ mod tests {
             ))
             .await
             .unwrap();
-        if let QueryPlannerContent::Plan { plan, .. } = result {
+        if let QueryPlannerContentInner::Plan { plan, .. } = result.0 {
             insta::with_settings!({sort_maps => true}, {
                 insta::assert_json_snapshot!("plan_usage_reporting", plan.usage_reporting);
             });

apollo-router/src/query_planner/caching_query_planner.rs

@@ -101,7 +101,7 @@ where
             qp.get(key)
                 .await
                 .map(|query_planner_content| {
-                    if let QueryPlannerContent::Plan { plan, .. } = &query_planner_content {
+                    if let QueryPlannerContentInner::Plan { plan, .. } = &query_planner_content.0 {
                         match (&plan.usage_reporting).serialize(Serializer) {
                             Ok(v) => {
                                 request.context.insert_json_value(USAGE_REPORTING, v);

apollo-router/src/query_planner/mod.rs

@@ -30,9 +30,9 @@ mod selection;
 
 /// Query planning options.
 #[derive(Clone, Eq, Hash, PartialEq, Debug, Default)]
-pub struct QueryPlanOptions {
+pub(crate) struct QueryPlanOptions {
     /// Enable the variable deduplication optimization on the QueryPlan
-    pub enable_variable_deduplication: bool,
+    pub(crate) enable_variable_deduplication: bool,
 }
 
 /// A plan for a given GraphQL query
@@ -250,12 +250,6 @@ impl PlanNode {
 }
 
 impl QueryPlan {
-    /// Pass some options to the QueryPlan
-    pub fn with_options(mut self, options: QueryPlanOptions) -> Self {
-        self.options = options;
-        self
-    }
-
     /// Execute the plan and return a [`Response`].
     pub async fn execute<'a, SF>(
         &self,

apollo-router/src/services/mod.rs

@@ -282,13 +282,13 @@ impl RouterResponse {
 }
 
 assert_impl_all!(QueryPlannerRequest: Send);
-/// [`Context`] and [`QueryPlanOptions`] for the request.
+/// [`Context`] for the request.
 #[derive(Clone, Debug)]
 pub struct QueryPlannerRequest {
     pub query: String,
     pub operation_name: Option<String>,
     /// Query plan options
-    pub query_plan_options: QueryPlanOptions,
+    pub(crate) query_plan_options: QueryPlanOptions,
 
     pub context: Context,
 }
@@ -302,28 +302,45 @@ impl QueryPlannerRequest {
     pub fn new(
         query: String,
         operation_name: Option<String>,
-        query_plan_options: QueryPlanOptions,
         context: Context,
     ) -> QueryPlannerRequest {
         Self {
             query,
             operation_name,
-            query_plan_options,
+            query_plan_options: QueryPlanOptions::default(),
             context,
         }
     }
 }
 
 assert_impl_all!(QueryPlannerResponse: Send);
-/// [`Context`] and [`QueryPlan`] for the response..
+/// [`Context`] and [`QueryPlan`] for the response.
 pub struct QueryPlannerResponse {
     pub content: QueryPlannerContent,
     pub context: Context,
 }
 
+/// Query, QueryPlan and Introspection data in an opaque type.
+#[derive(Debug, Clone)]
+pub struct QueryPlannerContent(pub(crate) QueryPlannerContentInner);
+
+impl std::ops::Deref for QueryPlannerContent {
+    type Target = QueryPlannerContentInner;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl From<QueryPlannerContentInner> for QueryPlannerContent {
+    fn from(inner: QueryPlannerContentInner) -> Self {
+        Self(inner)
+    }
+}
+
 /// Query, QueryPlan and Introspection data.
 #[derive(Debug, Clone)]
-pub enum QueryPlannerContent {
+pub enum QueryPlannerContentInner {
     Plan {
         query: Arc<Query>,
         plan: Arc<QueryPlan>,
@@ -333,6 +350,11 @@ pub enum QueryPlannerContent {
     },
     IntrospectionDisabled,
 }
+impl From<QueryPlannerContent> for QueryPlannerContentInner {
+    fn from(global: QueryPlannerContent) -> Self {
+        global.0
+    }
+}
 
 #[buildstructor::buildstructor]
 impl QueryPlannerResponse {
@@ -357,10 +379,11 @@ impl QueryPlannerResponse {
     ) -> Result<QueryPlannerResponse, BoxError> {
         tracing::warn!("no way to propagate error response from QueryPlanner");
         Ok(QueryPlannerResponse::new(
-            QueryPlannerContent::Plan {
+            QueryPlannerContentInner::Plan {
                 plan: Arc::new(QueryPlan::fake_builder().build()),
                 query: Arc::new(Query::default()),
-            },
+            }
+            .into(),
             context,
         ))
     }

apollo-router/src/services/router_service.rs

@@ -25,7 +25,7 @@ use super::subgraph_service::MakeSubgraphService;
 use super::subgraph_service::SubgraphCreator;
 use super::ExecutionCreator;
 use super::ExecutionServiceFactory;
-use super::QueryPlannerContent;
+use super::QueryPlannerContentInner;
 use crate::cache::DeduplicatingCache;
 use crate::error::QueryPlannerError;
 use crate::error::ServiceBuildError;
@@ -38,7 +38,6 @@ use crate::plugin::DynPlugin;
 use crate::plugin::Plugin;
 use crate::query_planner::BridgeQueryPlanner;
 use crate::query_planner::CachingQueryPlanner;
-use crate::query_planner::QueryPlanOptions;
 use crate::router_factory::RouterServiceFactory;
 use crate::services::layers::apq::APQLayer;
 use crate::services::layers::ensure_query_presence::EnsureQueryPresence;
@@ -125,17 +124,16 @@ where
                                 .expect("the query presence was already checked by a plugin"),
                         )
                         .and_operation_name(body.operation_name.clone())
-                        .query_plan_options(QueryPlanOptions::default())
                         .context(context)
                         .build(),
                 )
                 .await?;
 
-            match content {
-                QueryPlannerContent::Introspection { response } => Ok(
+            match content.0 {
+                QueryPlannerContentInner::Introspection { response } => Ok(
                     RouterResponse::new_from_graphql_response(*response, context),
                 ),
-                QueryPlannerContent::IntrospectionDisabled => {
+                QueryPlannerContentInner::IntrospectionDisabled => {
                     let mut resp = http::Response::new(
                         once(ready(
                             graphql::Response::builder()
@@ -153,7 +151,7 @@ where
                         context,
                     })
                 }
-                QueryPlannerContent::Plan { query, plan } => {
+                QueryPlannerContentInner::Plan { query, plan } => {
                     let is_deferred = plan.root.contains_defer();
 
                     if let Some(err) = query.validate_variables(body, &schema).err() {