fix(response_cache): entity fields returning null with cache-control: no-cache

apollo-router/src/plugins/response_cache/plugin.rs

@@ -1504,41 +1504,39 @@ async fn cache_lookup_entities(
         .iter()
         .map(|k| k.cache_key.as_str())
         .collect::<Vec<&str>>();
-    let cache_result = cache.fetch_multiple(&cache_keys, &name).await;
     Span::current().set_span_dyn_attribute(
         "cache.keys".into(),
         opentelemetry::Value::Array(Array::String(
             cache_keys
-                .into_iter()
+                .iter()
                 .map(|ck| StringValue::from(ck.to_string()))
                 .collect(),
         )),
     );
 
-    if cache_control.is_some_and(|c| c.is_no_cache()) {
-        // skip cache lookup if no-cache is set - we have no means of revalidating entries without
-        // just performing the query, so there's no benefit to hitting the cache
-        return Ok(ControlFlow::Continue((
-            request,
-            ResponseCacheResults::default(),
-        )));
-    }
+    // When no-cache is set, skip using any cached values: treat every entity as a cache miss
+    // so that all representations are fetched fresh from the subgraph. We still build the
+    // IntermediateResult list (all with cache_entry = None) so that insert_entities_in_result
+    // can properly assemble the response in the correct order.
+    let cache_result: Vec<Option<CacheEntry>> = if cache_control.is_some_and(|c| c.is_no_cache()) {
+        std::iter::repeat_n(None, keys_len).collect()
+    } else {
+        match cache.fetch_multiple(&cache_keys, &name).await {
+            Ok(res) => res
+                .into_iter()
+                .map(|v| match v {
+                    Some(v) if v.control.can_use() => Some(v),
+                    _ => None,
+                })
+                .collect(),
+            Err(err) => {
+                if !err.is_row_not_found() {
+                    let span = Span::current();
+                    span.mark_as_error(format!("cannot get cache entry: {err}"));
+                }
 
-    let cache_result: Vec<Option<CacheEntry>> = match cache_result {
-        Ok(res) => res
-            .into_iter()
-            .map(|v| match v {
-                Some(v) if v.control.can_use() => Some(v),
-                _ => None,
-            })
-            .collect(),
-        Err(err) => {
-            if !err.is_row_not_found() {
-                let span = Span::current();
-                span.mark_as_error(format!("cannot get cache entry: {err}"));
+                std::iter::repeat_n(None, keys_len).collect()
             }
-
-            std::iter::repeat_n(None, keys_len).collect()
         }
     };
     let body = request.subgraph_request.body_mut();

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

@@ -1482,6 +1482,144 @@ async fn no_store_from_request() {
     assert_eq!(invalidations_by_subgraph.into_values().sum::<u64>(), 0);
 }
 
+// Regression test for ROUTER-1689:
+// When `cache-control: no-cache` is sent by the client and response_cache is enabled,
+// entity fields resolved via `_entities` queries must not be discarded.
+// Previously, the no-cache fast-path returned an empty IntermediateResult list,
+// causing insert_entities_in_result to produce `_entities: []` and entity fields to be null.
+#[tokio::test]
+async fn no_cache_from_request() {
+    let valid_schema = Arc::new(Schema::parse_and_validate(SCHEMA, "test.graphql").unwrap());
+    let query = "query { currentUser { activeOrganization { id creatorUser { __typename id } } } }";
+
+    let subgraphs = serde_json::json!({
+        "user": {
+            "query": {
+                "currentUser": {
+                    "activeOrganization": {
+                        "__typename": "Organization",
+                        "id": "1",
+                    }
+                }
+            }
+        },
+        "orga": {
+            "entities": [
+                {
+                    "__typename": "Organization",
+                    "id": "1",
+                    "creatorUser": {
+                        "__typename": "User",
+                        "id": 2
+                    }
+                }
+            ]
+        },
+    });
+
+    let (drop_tx, drop_rx) = tokio::sync::broadcast::channel(2);
+    let storage = Storage::new(&Config::test(false, &Uuid::new_v4().to_string()), drop_rx)
+        .await
+        .unwrap();
+    let response_cache = ResponseCache::for_test(
+        storage.clone(),
+        Default::default(),
+        valid_schema.clone(),
+        false,
+        drop_tx,
+    )
+    .await
+    .unwrap();
+
+    // Phase 1: Warm up the cache with a normal request (no cache-control header)
+    let service = TestHarness::builder()
+        .configuration_json(serde_json::json!({"include_subgraph_errors": { "all": true }, "experimental_mock_subgraphs": subgraphs.clone(), "headers": {
+            "all": {
+                "request": [{
+                    "propagate": {
+                        "named": "cache-control"
+                    }
+                }]
+            }
+        } }))
+        .unwrap()
+        .schema(SCHEMA)
+        .extra_private_plugin(response_cache.clone())
+        .build_supergraph()
+        .await
+        .unwrap();
+
+    let request = supergraph::Request::fake_builder()
+        .query(query)
+        .context(Context::new())
+        .build()
+        .unwrap();
+    let mut response = service.oneshot(request).await.unwrap();
+    let response = response.next_response().await.unwrap();
+
+    // Sanity-check: normal request returns entity data
+    insta::assert_json_snapshot!(response, @r#"
+    {
+      "data": {
+        "currentUser": {
+          "activeOrganization": {
+            "id": "1",
+            "creatorUser": {
+              "__typename": "User",
+              "id": 2
+            }
+          }
+        }
+      }
+    }
+    "#);
+
+    // Phase 2: Request with `no-cache` — cache must be bypassed for lookup but entity data
+    // from the subgraph must still be returned correctly (regression for ROUTER-1689).
+    let service = TestHarness::builder()
+        .configuration_json(serde_json::json!({"include_subgraph_errors": { "all": true }, "experimental_mock_subgraphs": subgraphs.clone(), "headers": {
+            "all": {
+                "request": [{
+                    "propagate": {
+                        "named": "cache-control"
+                    }
+                }]
+            }
+        } }))
+        .unwrap()
+        .schema(SCHEMA)
+        .extra_private_plugin(response_cache.clone())
+        .build_supergraph()
+        .await
+        .unwrap();
+
+    let request = supergraph::Request::fake_builder()
+        .query(query)
+        .context(Context::new())
+        .header(CACHE_CONTROL, HeaderValue::from_static("no-cache"))
+        .build()
+        .unwrap();
+    let mut response = service.oneshot(request).await.unwrap();
+    let response = response.next_response().await.unwrap();
+
+    // Entity fields must NOT be null — this was the regression
+    insta::assert_json_snapshot!(response, @r#"
+    {
+      "data": {
+        "currentUser": {
+          "activeOrganization": {
+            "id": "1",
+            "creatorUser": {
+              "__typename": "User",
+              "id": 2
+            }
+          }
+        }
+      }
+    }
+    "#);
+}
+
 #[tokio::test]
 async fn private_only() {
     async {