improve NaiveIntrospection behavior and execute introspection when needed

CHANGELOG.md

@@ -36,6 +36,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ## ❗ BREAKING ❗
 ## 🚀 Features
 
+- **Add better support of introspection queries** ([PR #802](https://github.com/apollographql/router/pull/802))
+
+  Before this feature the Router didn't execute all the introspection queries, only a small  of the most used ones was executed. Now it detects if it's an introspection query, try to fetch it from cache, if it's not in the cache we execute it and put the response in the cache.
+
 - **Add an option to disable the landing page** ([PR #801](https://github.com/apollographql/router/pull/801))
 
   By default the router will display a landing page, which could be useful in development. If this is not

apollo-router-core/src/naive_introspection.rs → apollo-router-core/src/introspection.rs

@@ -1,8 +1,9 @@
 use crate::prelude::graphql::*;
 use include_dir::include_dir;
 use once_cell::sync::Lazy;
-use router_bridge::introspect;
+use router_bridge::introspect::{self, IntrospectionError};
 use std::collections::HashMap;
+use tokio::sync::RwLock;
 
 /// KNOWN_INTROSPECTION_QUERIES we will serve through NaiveIntrospection.
 ///
@@ -24,15 +25,17 @@ static KNOWN_INTROSPECTION_QUERIES: Lazy<Vec<String>> = Lazy::new(|| {
 });
 
 /// A cache containing our well known introspection queries.
-#[derive(Debug, Clone)]
-pub struct NaiveIntrospection {
-    cache: HashMap<String, Response>,
+#[derive(Debug)]
+pub struct Introspection {
+    cache: RwLock<HashMap<String, Response>>,
 }
 
-impl NaiveIntrospection {
+impl Introspection {
     #[cfg(test)]
     pub fn from_cache(cache: HashMap<String, Response>) -> Self {
-        Self { cache }
+        Self {
+            cache: RwLock::new(cache),
+        }
     }
 
     /// Create a `NaiveIntrospection` from a `Schema`.
@@ -86,21 +89,64 @@ impl NaiveIntrospection {
         })
         .unwrap_or_default();
 
-        Self { cache }
+        Self {
+            cache: RwLock::new(cache),
+        }
     }
 
     /// Get a cached response for a query.
-    pub fn get(&self, query: &str) -> Option<Response> {
-        self.cache.get(query).map(std::clone::Clone::clone)
+    pub async fn get(&self, query: &str) -> Option<Response> {
+        self.cache
+            .read()
+            .await
+            .get(query)
+            .map(std::clone::Clone::clone)
+    }
+
+    /// Execute an introspection and cache the response.
+    pub async fn execute(
+        &self,
+        schema_sdl: &str,
+        query: &str,
+    ) -> Result<Response, IntrospectionError> {
+        // Do the introspection query and cache it
+        let mut response = introspect::batch_introspect(schema_sdl, vec![query.to_owned()])
+            .map_err(|err| IntrospectionError {
+                message: format!("Deno runtime error: {:?}", err).into(),
+            })??;
+        let introspection_result = response
+            .pop()
+            .ok_or_else(|| IntrospectionError {
+                message: String::from("cannot find the introspection response").into(),
+            })?
+            .into_result()
+            .map_err(|err| IntrospectionError {
+                message: format!(
+                    "introspection error : {}",
+                    err.into_iter()
+                        .map(|err| err.to_string())
+                        .collect::<Vec<String>>()
+                        .join(", "),
+                )
+                .into(),
+            })?;
+        let response = Response::builder().data(introspection_result).build();
+
+        self.cache
+            .write()
+            .await
+            .insert(query.to_string(), response.clone());
+
+        Ok(response)
     }
 }
 
 #[cfg(test)]
 mod naive_introspection_tests {
     use super::*;
 
-    #[test]
-    fn test_plan() {
+    #[tokio::test]
+    async fn test_plan() {
         let query_to_test = "this is a test query";
         let expected_data = Response::builder()
             .data(serde_json::Value::Number(42.into()))
@@ -110,11 +156,11 @@ mod naive_introspection_tests {
             .iter()
             .cloned()
             .collect();
-        let naive_introspection = NaiveIntrospection::from_cache(cache);
+        let naive_introspection = Introspection::from_cache(cache);
 
         assert_eq!(
             expected_data,
-            naive_introspection.get(query_to_test).unwrap()
+            naive_introspection.get(query_to_test).await.unwrap()
         );
     }
 

apollo-router-core/src/lib.rs

@@ -25,9 +25,9 @@ macro_rules! failfast_error {
 mod cache;
 mod context;
 mod error;
+mod introspection;
 mod json_ext;
 mod layers;
-mod naive_introspection;
 pub mod plugin;
 pub mod plugin_utils;
 pub mod plugins;
@@ -43,9 +43,9 @@ mod traits;
 pub use cache::*;
 pub use context::*;
 pub use error::*;
+pub use introspection::*;
 pub use json_ext::*;
 pub use layers::*;
-pub use naive_introspection::*;
 pub use plugin::*;
 pub use plugins::*;
 pub use query_cache::*;

apollo-router-core/src/services/router_service.rs

@@ -4,7 +4,7 @@ use crate::forbid_http_get_mutations::ForbidHttpGetMutationsLayer;
 use crate::services::execution_service::ExecutionService;
 use crate::{
     plugin_utils, BridgeQueryPlanner, CachingQueryPlanner, DynPlugin, ExecutionRequest,
-    ExecutionResponse, NaiveIntrospection, Plugin, QueryCache, QueryPlannerRequest,
+    ExecutionResponse, Introspection, Plugin, QueryCache, QueryPlannerRequest,
     QueryPlannerResponse, ResponseBody, RouterRequest, RouterResponse, Schema, ServiceBuildError,
     ServiceBuilderExt, SubgraphRequest, SubgraphResponse, DEFAULT_BUFFER_SIZE,
 };
@@ -28,7 +28,7 @@ pub struct RouterService<QueryPlannerService, ExecutionService> {
     ready_query_execution_service: Option<ExecutionService>,
     schema: Arc<Schema>,
     query_cache: Arc<QueryCache>,
-    naive_introspection: Option<NaiveIntrospection>,
+    introspection: Option<Arc<Introspection>>,
 }
 
 impl<QueryPlannerService, ExecutionService> Service<RouterRequest>
@@ -74,29 +74,31 @@ where
         // Consume our cloned services and allow ownership to be transferred to the async block.
         let mut planning = self.ready_query_planner_service.take().unwrap();
         let mut execution = self.ready_query_execution_service.take().unwrap();
+        let naive_introspection = self.introspection.clone();
 
         let schema = self.schema.clone();
         let query_cache = self.query_cache.clone();
 
-        if let Some(naive_introspection) = self.naive_introspection.as_ref() {
-            if let Some(response) = naive_introspection.get(
-                req.context
-                    .request
-                    .body()
-                    .query
-                    .as_ref()
-                    .expect("apollo.ensure-query-is-present has checked this already; qed"),
-            ) {
-                return Box::pin(async move {
-                    Ok(RouterResponse {
+        let context_cloned = req.context.clone();
+        let fut = async move {
+            // Check if we already have the query in the known introspection queries
+            if let Some(naive_introspection) = naive_introspection.as_ref() {
+                if let Some(response) =
+                    naive_introspection
+                        .get(
+                            req.context.request.body().query.as_ref().expect(
+                                "apollo.ensure-query-is-present has checked this already; qed",
+                            ),
+                        )
+                        .await
+                {
+                    return Ok(RouterResponse {
                         response: http::Response::new(ResponseBody::GraphQL(response)).into(),
                         context: req.context.into(),
-                    })
-                });
+                    });
+                }
             }
-        }
-        let context_cloned = req.context.clone();
-        let fut = async move {
+
             let context = req.context;
             let body = context.request.body();
             let variables = body.variables.clone();
@@ -109,6 +111,24 @@ where
                 )
                 .await;
 
+            // Check if it's an introspection query
+            if let Some(current_query) = query.as_ref().filter(|q| q.contains_introspection()) {
+                if let Some(naive_introspection) = naive_introspection.as_ref() {
+                    match naive_introspection
+                        .execute(schema.as_str(), current_query.as_str())
+                        .await
+                    {
+                        Ok(resp) => {
+                            return Ok(RouterResponse {
+                                response: http::Response::new(ResponseBody::GraphQL(resp)).into(),
+                                context: context.into(),
+                            });
+                        }
+                        Err(err) => return Err(BoxError::from(err)),
+                    }
+                }
+            }
+
             if let Some(err) = query
                 .as_ref()
                 .and_then(|q| q.validate_variables(body, &schema).err())
@@ -170,7 +190,7 @@ pub struct PluggableRouterServiceBuilder {
         String,
         BoxService<SubgraphRequest, SubgraphResponse, BoxError>,
     )>,
-    naive_introspection: bool,
+    introspection: bool,
 }
 
 impl PluggableRouterServiceBuilder {
@@ -179,7 +199,7 @@ impl PluggableRouterServiceBuilder {
             schema,
             plugins: Default::default(),
             subgraph_services: Default::default(),
-            naive_introspection: false,
+            introspection: false,
         }
     }
 
@@ -227,7 +247,7 @@ impl PluggableRouterServiceBuilder {
     }
 
     pub fn with_naive_introspection(mut self) -> PluggableRouterServiceBuilder {
-        self.naive_introspection = true;
+        self.introspection = true;
         self
     }
 
@@ -307,16 +327,16 @@ impl PluggableRouterServiceBuilder {
             .unwrap_or(100);
         let query_cache = Arc::new(QueryCache::new(query_cache_limit, self.schema.clone()));
 
-        let naive_introspection = if self.naive_introspection {
-            // NaiveIntrospection instantiation can potentially block for some time
+        let introspection = if self.introspection {
+            // Introspection instantiation can potentially block for some time
             // We don't need to use the api schema here because on the deno side we always convert to API schema
 
             let schema = self.schema.clone();
-            Some(
-                tokio::task::spawn_blocking(move || NaiveIntrospection::from_schema(&schema))
+            Some(Arc::new(
+                tokio::task::spawn_blocking(move || Introspection::from_schema(&schema))
                     .await
-                    .expect("NaiveIntrospection instantiation panicked"),
-            )
+                    .expect("Introspection instantiation panicked"),
+            ))
         } else {
             None
         };
@@ -353,7 +373,7 @@ impl PluggableRouterServiceBuilder {
                             .query_execution_service(execution_service)
                             .schema(self.schema)
                             .query_cache(query_cache)
-                            .naive_introspection(naive_introspection)
+                            .introspection(introspection)
                             .build()
                             .boxed(),
                         |acc, (_, e)| e.router_service(acc),

apollo-router-core/src/spec/field_type.rs

@@ -7,6 +7,8 @@ pub(crate) struct InvalidValue;
 // Primitives are taken from scalars: https://spec.graphql.org/draft/#sec-Scalars
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub(crate) enum FieldType {
+    /// Only used for introspection queries when types are prefixed by __
+    Introspection(String),
     Named(String),
     List(Box<FieldType>),
     NonNull(Box<FieldType>),
@@ -94,7 +96,7 @@ impl FieldType {
     /// Example if we get the field `list: [User!]!`, it will return "User"
     pub fn inner_type_name(&self) -> Option<&str> {
         match self {
-            FieldType::Named(name) => Some(name.as_str()),
+            FieldType::Named(name) | FieldType::Introspection(name) => Some(name.as_str()),
             FieldType::List(inner) | FieldType::NonNull(inner) => inner.inner_type_name(),
             FieldType::String
             | FieldType::Int
@@ -106,7 +108,10 @@ impl FieldType {
 
     pub fn is_builtin_scalar(&self) -> bool {
         match self {
-            FieldType::Named(_) | FieldType::List(_) | FieldType::NonNull(_) => false,
+            FieldType::Named(_)
+            | FieldType::Introspection(_)
+            | FieldType::List(_)
+            | FieldType::NonNull(_) => false,
             FieldType::String
             | FieldType::Int
             | FieldType::Float