Merge branch 'dev' into geal/introspection-dedup-fix

.changesets/feat_lrlna_enable_generate_query_fragments_by_default.md

@@ -0,0 +1,26 @@
+### Compress subgraph operations by generating fragments
+
+The router now compresses operations sent to subgraphs by default by generating fragment
+definitions and using them in the operation.
+
+Initially, the router is using a very simple transformation that is implemented in both
+the JavaScript and Native query planners. We will improve the algorithm after the JavaScript
+planner is no longer supported.
+
+This replaces a previous experimental algorithm that was enabled by default.
+`experimental_reuse_query_fragments` attempted to intelligently reuse the fragment definitions
+from the original operation. Fragment generation is much faster, and in most cases produces
+better outputs too.
+
+If you are relying on the shape of fragments in your subgraph operations or tests, you can opt
+out of the new algorithm with the configuration below. Note we strongly recommend against
+relying on the shape of planned operations as new router features and optimizations may affect
+it, and we intend to remove `experimental_reuse_query_fragments` in a future release.
+
+```yaml
+supergraph:
+  generate_query_fragments: false
+  experimental_reuse_query_fragments: true
+```
+
+By [@lrlna](https://github.com/lrlna) in https://github.com/apollographql/router/pull/6013

apollo-federation/src/query_plan/query_planner.rs

@@ -63,7 +63,7 @@ pub struct QueryPlannerConfig {
     /// queries with many fragments. This option may be removed in the future in favour of
     /// [`generate_query_fragments`][QueryPlannerConfig::generate_query_fragments].
     ///
-    /// Defaults to true.
+    /// Defaults to false.
     pub reuse_query_fragments: bool,
 
     /// If enabled, the query planner will extract inline fragments into fragment
@@ -103,15 +103,16 @@ pub struct QueryPlannerConfig {
     pub type_conditioned_fetching: bool,
 }
 
+#[allow(clippy::derivable_impls)] // it's derivable right now, but we might change the defaults
 impl Default for QueryPlannerConfig {
     fn default() -> Self {
         Self {
-            reuse_query_fragments: true,
+            reuse_query_fragments: false,
             generate_query_fragments: false,
             subgraph_graphql_validation: false,
             incremental_delivery: Default::default(),
             debug: Default::default(),
-            type_conditioned_fetching: Default::default(),
+            type_conditioned_fetching: false,
         }
     }
 }
@@ -1381,7 +1382,11 @@ type User
         )
         .unwrap();
 
-        let planner = QueryPlanner::new(&supergraph, Default::default()).unwrap();
+        let config = QueryPlannerConfig {
+            reuse_query_fragments: true,
+            ..Default::default()
+        };
+        let planner = QueryPlanner::new(&supergraph, config).unwrap();
         let plan = planner
             .build_query_plan(&document, None, Default::default())
             .unwrap();
@@ -1442,7 +1447,11 @@ type User
         )
         .unwrap();
 
-        let planner = QueryPlanner::new(&supergraph, Default::default()).unwrap();
+        let config = QueryPlannerConfig {
+            reuse_query_fragments: true,
+            ..Default::default()
+        };
+        let planner = QueryPlanner::new(&supergraph, config).unwrap();
         let plan = planner
             .build_query_plan(&document, None, Default::default())
             .unwrap();
@@ -1504,7 +1513,11 @@ type User
         )
         .unwrap();
 
-        let planner = QueryPlanner::new(&supergraph, Default::default()).unwrap();
+        let config = QueryPlannerConfig {
+            reuse_query_fragments: true,
+            ..Default::default()
+        };
+        let planner = QueryPlanner::new(&supergraph, config).unwrap();
         let plan = planner
             .build_query_plan(&document, None, Default::default())
             .unwrap();

apollo-federation/tests/query_plan/build_query_plan_tests/named_fragments.rs

@@ -1,6 +1,16 @@
+use apollo_federation::query_plan::query_planner::QueryPlannerConfig;
+
+fn reuse_fragments_config() -> QueryPlannerConfig {
+    QueryPlannerConfig {
+        reuse_query_fragments: true,
+        ..Default::default()
+    }
+}
+
 #[test]
 fn handles_mix_of_fragments_indirection_and_unions() {
     let planner = planner!(
+        config = reuse_fragments_config(),
         Subgraph1: r#"
           type Query {
             parent: Parent
@@ -74,6 +84,7 @@ fn another_mix_of_fragments_indirection_and_unions() {
     // This tests that the issue reported on https://github.com/apollographql/router/issues/3172 is resolved.
 
     let planner = planner!(
+        config = reuse_fragments_config(),
         Subgraph1: r#"
           type Query {
             owner: Owner!
@@ -252,6 +263,7 @@ fn another_mix_of_fragments_indirection_and_unions() {
 #[test]
 fn handles_fragments_with_interface_field_subtyping() {
     let planner = planner!(
+        config = reuse_fragments_config(),
         Subgraph1: r#"
           type Query {
             t1: T1!
@@ -313,6 +325,7 @@ fn handles_fragments_with_interface_field_subtyping() {
 #[test]
 fn can_reuse_fragments_in_subgraph_where_they_only_partially_apply_in_root_fetch() {
     let planner = planner!(
+        config = reuse_fragments_config(),
         Subgraph1: r#"
           type Query {
             t1: T
@@ -416,6 +429,7 @@ fn can_reuse_fragments_in_subgraph_where_they_only_partially_apply_in_root_fetch
 #[test]
 fn can_reuse_fragments_in_subgraph_where_they_only_partially_apply_in_entity_fetch() {
     let planner = planner!(
+        config = reuse_fragments_config(),
         Subgraph1: r#"
           type Query {
             t: T

apollo-federation/tests/query_plan/build_query_plan_tests/named_fragments_preservation.rs

@@ -1,8 +1,16 @@
 use apollo_federation::query_plan::query_planner::QueryPlannerConfig;
 
+fn reuse_fragments_config() -> QueryPlannerConfig {
+    QueryPlannerConfig {
+        reuse_query_fragments: true,
+        ..Default::default()
+    }
+}
+
 #[test]
 fn it_works_with_nested_fragments_1() {
     let planner = planner!(
+        config = reuse_fragments_config(),
         Subgraph1: r#"
           type Query {
             a: Anything
@@ -127,6 +135,7 @@ fn it_works_with_nested_fragments_1() {
 #[test]
 fn it_avoid_fragments_usable_only_once() {
     let planner = planner!(
+        config = reuse_fragments_config(),
             Subgraph1: r#"
           type Query {
             t: T
@@ -324,10 +333,9 @@ mod respects_query_planner_option_reuse_query_fragments {
 
     #[test]
     fn respects_query_planner_option_reuse_query_fragments_true() {
-        let reuse_query_fragments = true;
         let planner = planner!(
-          config = QueryPlannerConfig {reuse_query_fragments, ..Default::default()},
-          Subgraph1: SUBGRAPH1,
+            config = reuse_fragments_config(),
+            Subgraph1: SUBGRAPH1,
         );
         let query = QUERY;
 
@@ -395,6 +403,7 @@ mod respects_query_planner_option_reuse_query_fragments {
 #[test]
 fn it_works_with_nested_fragments_when_only_the_nested_fragment_gets_preserved() {
     let planner = planner!(
+        config = reuse_fragments_config(),
         Subgraph1: r#"
           type Query {
             t: T
@@ -463,6 +472,7 @@ fn it_works_with_nested_fragments_when_only_the_nested_fragment_gets_preserved()
 fn it_preserves_directives_when_fragment_not_used() {
     // (because used only once)
     let planner = planner!(
+        config = reuse_fragments_config(),
         Subgraph1: r#"
           type Query {
             t: T
@@ -511,6 +521,7 @@ fn it_preserves_directives_when_fragment_not_used() {
 #[test]
 fn it_preserves_directives_when_fragment_is_reused() {
     let planner = planner!(
+        config = reuse_fragments_config(),
         Subgraph1: r#"
           type Query {
             t: T
@@ -572,6 +583,7 @@ fn it_does_not_try_to_apply_fragments_that_are_not_valid_for_the_subgraph() {
     // server would reject it when validating the query, and we must make sure it
     // is not reused.
     let planner = planner!(
+        config = reuse_fragments_config(),
         Subgraph1: r#"
           type Query {
             i1: I
@@ -652,6 +664,7 @@ fn it_handles_fragment_rebasing_in_a_subgraph_where_some_subtyping_relation_diff
     // Previous versions of the code were not handling this case and were error out by
     // creating the invalid selection (#2721), and this test ensures this is fixed.
     let planner = planner!(
+        config = reuse_fragments_config(),
         Subgraph1: r#"
           type V @shareable {
             x: Int
@@ -988,6 +1001,7 @@ fn it_handles_fragment_rebasing_in_a_subgraph_where_some_union_membership_relati
     // This test is similar to the subtyping case (it tests the same problems), but test the case
     // of unions instead of interfaces.
     let planner = planner!(
+        config = reuse_fragments_config(),
         Subgraph1: r#"
           type V @shareable {
             x: Int
@@ -1302,6 +1316,7 @@ fn it_handles_fragment_rebasing_in_a_subgraph_where_some_union_membership_relati
 #[test]
 fn it_preserves_nested_fragments_when_outer_one_has_directives_and_is_eliminated() {
     let planner = planner!(
+        config = reuse_fragments_config(),
         Subgraph1: r#"
           type Query {
             t: T

apollo-router-benchmarks/src/shared.rs

@@ -76,7 +76,7 @@ pub fn setup() -> TestHarness<'static> {
     }}).build();
 
     let review_service = MockSubgraph::builder().with_json(json!{{
-        "query": "query TopProducts__reviews__1($representations:[_Any!]!){_entities(representations:$representations){...on Product{reviews{id product{__typename upc}author{__typename id}}}}}",
+        "query": "query TopProducts__reviews__1($representations:[_Any!]!){_entities(representations:$representations){..._generated_onProduct1_0}}fragment _generated_onProduct1_0 on Product{reviews{id product{__typename upc}author{__typename id}}}",
         "operationName": "TopProducts__reviews__1",
         "variables": {
             "representations":[

apollo-router/src/axum_factory/snapshots/apollo_router__axum_factory__tests__defer_is_not_buffered.snap

@@ -20,7 +20,7 @@ expression: parts
     },
     "errors": [
       {
-        "message": "couldn't find mock for query {\"query\":\"query($representations: [_Any!]!) { _entities(representations: $representations) { ... on Product { reviews { __typename id product { __typename upc } } } } }\",\"variables\":{\"representations\":[{\"__typename\":\"Product\",\"upc\":\"1\"},{\"__typename\":\"Product\",\"upc\":\"2\"}]}}",
+        "message": "couldn't find mock for query {\"query\":\"query($representations: [_Any!]!) { _entities(representations: $representations) { ..._generated_onProduct1_0 } } fragment _generated_onProduct1_0 on Product { reviews { __typename id product { __typename upc } } }\",\"variables\":{\"representations\":[{\"__typename\":\"Product\",\"upc\":\"1\"},{\"__typename\":\"Product\",\"upc\":\"2\"}]}}",
         "path": [
           "topProducts",
           "@"

apollo-router/src/axum_factory/tests.rs

@@ -1847,7 +1847,7 @@ async fn http_compressed_service() -> impl Service<
                 "apollo.include_subgraph_errors": {
                     "all": true
                 }
-            }
+            },
         }))
         .unwrap()
         .supergraph_hook(move |service| {

apollo-router/src/configuration/mod.rs

@@ -681,7 +681,7 @@ pub(crate) struct Supergraph {
     pub(crate) reuse_query_fragments: Option<bool>,
 
     /// Enable QP generation of fragments for subgraph requests
-    /// Default: false
+    /// Default: true
     pub(crate) generate_query_fragments: bool,
 
     /// Set to false to disable defer support
@@ -701,6 +701,10 @@ pub(crate) struct Supergraph {
     pub(crate) experimental_log_on_broken_pipe: bool,
 }
 
+const fn default_generate_query_fragments() -> bool {
+    true
+}
+
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Deserialize, Serialize, JsonSchema)]
 #[serde(rename_all = "snake_case", untagged)]
 pub(crate) enum AvailableParallelism {
@@ -753,7 +757,7 @@ impl Supergraph {
                     Some(false)
                 } else { reuse_query_fragments }
             ),
-            generate_query_fragments: generate_query_fragments.unwrap_or_default(),
+            generate_query_fragments: generate_query_fragments.unwrap_or_else(default_generate_query_fragments),
             early_cancel: early_cancel.unwrap_or_default(),
             experimental_log_on_broken_pipe: experimental_log_on_broken_pipe.unwrap_or_default(),
         }
@@ -790,7 +794,7 @@ impl Supergraph {
                     Some(false)
                 } else { reuse_query_fragments }
             ),
-            generate_query_fragments: generate_query_fragments.unwrap_or_default(),
+            generate_query_fragments: generate_query_fragments.unwrap_or_else(default_generate_query_fragments),
             early_cancel: early_cancel.unwrap_or_default(),
             experimental_log_on_broken_pipe: experimental_log_on_broken_pipe.unwrap_or_default(),
         }

apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap

@@ -6604,8 +6604,8 @@ expression: "&schema"
           "type": "boolean"
         },
         "generate_query_fragments": {
-          "default": false,
-          "description": "Enable QP generation of fragments for subgraph requests Default: false",
+          "default": true,
+          "description": "Enable QP generation of fragments for subgraph requests Default: true",
           "type": "boolean"
         },
         "introspection": {

apollo-router/src/error.rs

@@ -1,4 +1,5 @@
 //! Router errors.
+use std::collections::HashMap;
 use std::sync::Arc;
 
 use apollo_compiler::validation::DiagnosticList;
@@ -412,6 +413,19 @@ impl IntoGraphQLErrors for QueryPlannerError {
     }
 }
 
+impl QueryPlannerError {
+    pub(crate) fn usage_reporting(&self) -> Option<UsageReporting> {
+        match self {
+            QueryPlannerError::PlanningErrors(pe) => Some(pe.usage_reporting.clone()),
+            QueryPlannerError::SpecError(e) => Some(UsageReporting {
+                stats_report_key: e.get_error_key().to_string(),
+                referenced_fields_by_type: HashMap::new(),
+            }),
+            _ => None,
+        }
+    }
+}
+
 #[derive(Clone, Debug, Error, Serialize, Deserialize)]
 /// Container for planner setup errors
 pub(crate) struct PlannerErrors(Arc<Vec<PlannerError>>);

apollo-router/src/plugin/test/mock/canned.rs

@@ -52,7 +52,7 @@ pub(crate) fn reviews_subgraph() -> MockSubgraph {
     let review_mocks = vec![
         (
             json! {{
-                    "query": "query TopProducts__reviews__1($representations:[_Any!]!){_entities(representations:$representations){...on Product{reviews{id product{__typename upc}author{__typename id}}}}}",
+                    "query": "query TopProducts__reviews__1($representations:[_Any!]!){_entities(representations:$representations){..._generated_onProduct1_0}}fragment _generated_onProduct1_0 on Product{reviews{id product{__typename upc}author{__typename id}}}",
                     "operationName": "TopProducts__reviews__1",
                     "variables": {
                         "representations":[

apollo-router/src/plugins/authorization/tests.rs

@@ -22,7 +22,7 @@ async fn authenticated_request() {
     let subgraphs = MockedSubgraphs([
     ("user", MockSubgraph::builder().with_json(
             serde_json::json!{{
-                "query": "query($representations:[_Any!]!){_entities(representations:$representations){...on User{name phone}}}",
+                "query": "query($representations:[_Any!]!){_entities(representations:$representations){..._generated_onUser2_0}}fragment _generated_onUser2_0 on User{name phone}",
                 "variables": {
                     "representations": [
                         { "__typename": "User", "id":0 }

apollo-router/src/plugins/cache/tests.rs

@@ -160,7 +160,7 @@ async fn insert() {
         ).with_header(CACHE_CONTROL, HeaderValue::from_static("public")).build()),
         ("orga", MockSubgraph::builder().with_json(
             serde_json::json!{{
-                "query": "query($representations:[_Any!]!){_entities(representations:$representations){...on Organization{creatorUser{__typename id}}}}",
+                "query": "query($representations:[_Any!]!){_entities(representations:$representations){..._generated_onOrganization1_0}}fragment _generated_onOrganization1_0 on Organization{creatorUser{__typename id}}",
             "variables": {
                 "representations": [
                     {
@@ -274,7 +274,7 @@ async fn no_cache_control() {
         ).build()),
         ("orga", MockSubgraph::builder().with_json(
             serde_json::json!{{
-                "query": "query($representations:[_Any!]!){_entities(representations:$representations){...on Organization{creatorUser{__typename id}}}}",
+                "query": "query($representations:[_Any!]!){_entities(representations:$representations){..._generated_onOrganization1_0}}fragment _generated_onOrganization1_0 on Organization{creatorUser{__typename id}}",
             "variables": {
                 "representations": [
                     {
@@ -365,7 +365,7 @@ async fn private() {
             .build()),
         ("orga", MockSubgraph::builder().with_json(
             serde_json::json!{{
-                "query": "query($representations:[_Any!]!){_entities(representations:$representations){...on Organization{creatorUser{__typename id}}}}",
+                "query": "query($representations:[_Any!]!){_entities(representations:$representations){..._generated_onOrganization1_0}}fragment _generated_onOrganization1_0 on Organization{creatorUser{__typename id}}",
             "variables": {
                 "representations": [
                     {