Set correctly hasNext for the last chunk of a deferred response (#1736

) close #1687 Signed-off-by: Benjamin Coenen <[email protected]>
apollographql · Sep 9, 2022 · 24a00e6 · 24a00e6
1 parent 514bb86
commit 24a00e6
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 14 deletions.
diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md
@@ -66,6 +66,13 @@ By [@BrynCooke](https://github.com/BrynCooke) & [@bnjjj](https://github.com/bnjj
 
 
 ## 🐛 Fixes
+
+### Set correctly hasNext for the last chunk of a deferred response ([#1687](https://github.com/apollographql/router/issues/1687))
+
+You no longer will receive a last chunk `{"hasNext": false}` in a deferred response.
+
+By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/1736
+
 ## 🛠 Maintenance
 
 ### Add errors vec in `QueryPlannerResponse` to handle errors in `query_planning_service` ([PR #1504](https://github.com/apollographql/router/pull/1504))

diff --git a/apollo-router/src/query_planner/mod.rs b/apollo-router/src/query_planner/mod.rs
@@ -495,6 +495,7 @@ impl PlanNode {
                         let mut primary_receiver = primary_sender.subscribe();
                         let mut value = parent_value.clone();
                         let fut = async move {
+                            let mut has_next = true;
                             let mut errors = Vec::new();
 
                             if is_depends_empty {
@@ -539,13 +540,15 @@ impl PlanNode {
                                 if !is_depends_empty {
                                     let primary_value =
                                         primary_receiver.recv().await.unwrap_or_default();
+                                    has_next = false;
                                     v.deep_merge(primary_value);
                                 }
 
                                 if let Err(e) = tx
                                     .send(
                                         Response::builder()
                                             .data(v)
+                                            .has_next(has_next)
                                             .errors(err)
                                             .and_path(Some(deferred_path.clone()))
                                             .and_subselection(subselection.or(node_subselection))
@@ -563,12 +566,13 @@ impl PlanNode {
                             } else {
                                 let primary_value =
                                     primary_receiver.recv().await.unwrap_or_default();
+                                has_next = false;
                                 value.deep_merge(primary_value);
-
                                 if let Err(e) = tx
                                     .send(
                                         Response::builder()
                                             .data(value)
+                                            .has_next(has_next)
                                             .errors(errors)
                                             .and_path(Some(deferred_path.clone()))
                                             .and_subselection(subselection)
@@ -1565,7 +1569,7 @@ mod tests {
             serde_json::to_value(&response).unwrap(),
             // the primary response appears there because the deferred response gets data from it
             // unneeded parts are removed in response formatting
-            serde_json::json! {{"data":{"t":{"y":"Y","__typename":"T","id":1234,"x":"X"}},"path":["t"]}}
+            serde_json::json! {{"data":{"t":{"y":"Y","__typename":"T","id":1234,"x":"X"}}, "hasNext": false, "path":["t"]}}
         );
     }
 

diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs
@@ -3,9 +3,7 @@
 use std::sync::Arc;
 use std::task::Poll;
 
-use futures::future::ready;
 use futures::future::BoxFuture;
-use futures::stream::once;
 use futures::stream::StreamExt;
 use futures::TryFutureExt;
 use http::header::ACCEPT;
@@ -299,8 +297,8 @@ fn process_execution_response(
     let ExecutionResponse { response, context } = execution_response;
 
     let (parts, response_stream) = response.into_parts();
-
     let stream = response_stream.map(move |mut response: Response| {
+        let has_next = response.has_next.unwrap_or(true);
         tracing::debug_span!("format_response").in_scope(|| {
             query.format_response(
                 &mut response,
@@ -313,7 +311,7 @@ fn process_execution_response(
         match (response.path.as_ref(), response.data.as_ref()) {
             (None, _) | (_, None) => {
                 if can_be_deferred {
-                    response.has_next = Some(true);
+                    response.has_next = Some(has_next);
                 }
 
                 response
@@ -335,7 +333,7 @@ fn process_execution_response(
                 });
 
                 Response::builder()
-                    .has_next(true)
+                    .has_next(has_next)
                     .incremental(
                         sub_responses
                             .into_iter()
@@ -360,9 +358,7 @@ fn process_execution_response(
         response: http::Response::from_parts(
             parts,
             if can_be_deferred {
-                stream
-                    .chain(once(ready(Response::builder().has_next(false).build())))
-                    .left_stream()
+                stream.left_stream()
             } else {
                 stream.right_stream()
             }

diff --git a/apollo-router/tests/snapshots/integration_tests__defer_path-2.snap b/apollo-router/tests/snapshots/integration_tests__defer_path-2.snap
@@ -1,10 +1,10 @@
 ---
 source: apollo-router/tests/integration_tests.rs
-assertion_line: 679
+assertion_line: 599
 expression: second
 ---
 {
-  "hasNext": true,
+  "hasNext": false,
   "incremental": [
     {
       "label": "name",

diff --git a/apollo-router/tests/snapshots/integration_tests__defer_path_in_array-2.snap b/apollo-router/tests/snapshots/integration_tests__defer_path_in_array-2.snap
@@ -1,10 +1,10 @@
 ---
 source: apollo-router/tests/integration_tests.rs
-assertion_line: 726
+assertion_line: 639
 expression: second
 ---
 {
-  "hasNext": true,
+  "hasNext": false,
   "incremental": [
     {
       "label": "author name",