do not perform nested fetches if the parent one returned null

NEXT_CHANGELOG.md

@@ -117,6 +117,21 @@ The benchmarks show that this change brings a 23% gain in requests per second co
 
 By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/1423
 
+### do not perform nested fetches if the parent one returned null ([PR #1332](https://github.com/apollographql/router/pull/1332)
+
+In a query of the form:
+```graphql
+mutation {
+	mutationA {
+		mutationB
+	}
+}
+```
+
+If `mutationA` returned null, we should not execute `mutationB`.
+
+By [@Ty3uK](https://github.com/Ty3uK) in https://github.com/apollographql/router/pull/1332
+
 ## 🛠 Maintenance
 
 ## 📚 Documentation

apollo-router/src/json_ext.rs

@@ -64,10 +64,14 @@ pub trait ValueExt {
     #[track_caller]
     fn from_path(path: &Path, value: Value) -> Value;
 
-    /// Insert a `value` at a `Path`
+    /// Insert a `Value` at a `Path`
     #[track_caller]
     fn insert(&mut self, path: &Path, value: Value) -> Result<(), FetchError>;
 
+    /// Get a `Value` from a `Path`
+    #[track_caller]
+    fn get_path<'a>(&'a self, path: &'a Path) -> Result<&'a Value, FetchError>;
+
     /// Select all values matching a `Path`.
     ///
     /// the function passed as argument will be called with the values found and their Path
@@ -241,7 +245,7 @@ impl ValueExt for Value {
         res_value
     }
 
-    /// Insert a `value` at a `Path`
+    /// Insert a `Value` at a `Path`
     #[track_caller]
     fn insert(&mut self, path: &Path, value: Value) -> Result<(), FetchError> {
         let mut current_node = self;
@@ -318,6 +322,18 @@ impl ValueExt for Value {
         Ok(())
     }
 
+    /// Get a `Value` from a `Path`
+    #[track_caller]
+    fn get_path<'a>(&'a self, path: &'a Path) -> Result<&'a Value, FetchError> {
+        let mut res = Err(FetchError::ExecutionPathNotFound {
+            reason: "value not found".to_string(),
+        });
+        iterate_path(&mut Path::default(), &path.0, self, &mut |_path, value| {
+            res = Ok(value);
+        });
+        res
+    }
+
     #[track_caller]
     fn select_values_and_paths<'a, F>(&'a self, path: &'a Path, mut f: F)
     where

apollo-router/src/query_planner/mod.rs

@@ -889,6 +889,20 @@ pub(crate) mod fetch {
 
                 Some(Variables { variables, paths })
             } else {
+                // with nested operations (Query or Mutation has an operation returning a Query or Mutation),
+                // when the first fetch fails, the query plan wwill still execute up until the second fetch,
+                // where `requires` is empty (not a federated fetch), the current dir is not emmpty (child of
+                // the previous operation field) and the data is null. In that case, we recognize that we
+                // should not perform the next fetch
+                if !current_dir.is_empty()
+                    && data
+                        .get_path(current_dir)
+                        .map(|value| value.is_null())
+                        .unwrap_or(true)
+                {
+                    return None;
+                }
+
                 Some(Variables {
                     variables: variable_usages
                         .iter()
@@ -1560,4 +1574,123 @@ mod tests {
             r#"{"data":{"t":{"y":"Y","__typename":"T","id":1234,"x":"X"}},"path":["t"]}"#
         );
     }
+
+    #[tokio::test]
+    async fn dependent_mutations() {
+        let schema = r#"schema
+        @core(feature: "https://specs.apollo.dev/core/v0.1"),
+        @core(feature: "https://specs.apollo.dev/join/v0.1")
+      {
+        query: Query
+        mutation: Mutation
+      }
+
+      directive @core(feature: String!) repeatable on SCHEMA
+      directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet) on FIELD_DEFINITION
+      directive @join__type(graph: join__Graph!, key: join__FieldSet) repeatable on OBJECT | INTERFACE
+      directive @join__owner(graph: join__Graph!) on OBJECT | INTERFACE
+      directive @join__graph(name: String!, url: String!) on ENUM_VALUE
+      scalar join__FieldSet
+
+      enum join__Graph {
+        A @join__graph(name: "A" url: "http://localhost:4001")
+        B @join__graph(name: "B" url: "http://localhost:4004")
+      }
+
+      type Mutation {
+          mutationA: Mutation @join__field(graph: A)
+          mutationB: Boolean @join__field(graph: B)
+      }
+
+      type Query {
+          query: Boolean @join__field(graph: A)
+      }"#;
+
+        let query_plan: QueryPlan = QueryPlan {
+            // generated from:
+            // mutation {
+            //   mutationA {
+            //     mutationB
+            //   }
+            // }
+            root: serde_json::from_str(
+                r#"{
+                "kind": "Sequence",
+                "nodes": [
+                    {
+                        "kind": "Fetch",
+                        "serviceName": "A",
+                        "variableUsages": [],
+                        "operation": "mutation{mutationA{__typename}}",
+                        "operationKind": "mutation"
+                    },
+                    {
+                        "kind": "Flatten",
+                        "path": [
+                            "mutationA"
+                        ],
+                        "node": {
+                            "kind": "Fetch",
+                            "serviceName": "B",
+                            "variableUsages": [],
+                            "operation": "mutation{...on Mutation{mutationB}}",
+                            "operationKind": "mutation"
+                        }
+                    }
+                ]
+            }"#,
+            )
+            .unwrap(),
+            usage_reporting: UsageReporting {
+                stats_report_key: "this is a test report key".to_string(),
+                referenced_fields_by_type: Default::default(),
+            },
+            options: QueryPlanOptions::default(),
+        };
+
+        let mut mock_a_service = plugin::test::MockSubgraphService::new();
+        mock_a_service
+            .expect_call()
+            .times(1)
+            .returning(|_| Ok(SubgraphResponse::fake_builder().build()));
+
+        // the first fetch returned null, so there should never be a call to B
+        let mut mock_b_service = plugin::test::MockSubgraphService::new();
+        mock_b_service.expect_call().never();
+
+        let sf = Arc::new(MockSubgraphFactory {
+            subgraphs: HashMap::from([
+                (
+                    "A".into(),
+                    ServiceBuilder::new()
+                        .buffer(1)
+                        .service(mock_a_service.build().boxed()),
+                ),
+                (
+                    "B".into(),
+                    ServiceBuilder::new()
+                        .buffer(1)
+                        .service(mock_b_service.build().boxed()),
+                ),
+            ]),
+            plugins: Default::default(),
+        });
+
+        let (sender, _) = futures::channel::mpsc::channel(10);
+        let _response = query_plan
+            .execute(
+                &Context::new(),
+                &sf,
+                &Arc::new(
+                    http_ext::Request::fake_builder()
+                        .headers(Default::default())
+                        .body(Default::default())
+                        .build()
+                        .expect("fake builds should always work; qed"),
+                ),
+                &Schema::from_str(schema).unwrap(),
+                sender,
+            )
+            .await;
+    }
 }