Merge remote-tracking branch 'origin/benjamn/JSONSelection-compute_ou…

…tput_shape' into dylan/string-template-type-checking # Conflicts: # Cargo.lock # apollo-federation/Cargo.toml # apollo-federation/src/sources/connect/json_selection/apply_to.rs # apollo-federation/src/sources/connect/json_selection/methods/future.rs # apollo-federation/src/sources/connect/json_selection/methods/public.rs
apollographql · Jan 9, 2025 · dcb717a · dcb717a
2 parents 27303ce + b45366b
commit dcb717a
Show file tree

Hide file tree

Showing 20 changed files with 400 additions and 37 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -510,7 +510,7 @@ commands:
             # TODO: remove this workaround once we update to Xcode >= 15.1.0
             # See: https://github.com/apollographql/router/pull/5462
             RUST_LIB_BACKTRACE: 0
-          command: xtask test --workspace --locked --features ci
+          command: xtask test --workspace --locked --features ci,snapshot
       - run:
           name: Delete large files from cache
           command: |

diff --git a/Cargo.lock b/Cargo.lock
@@ -6425,7 +6425,9 @@ dependencies = [
 
 [[package]]
 name = "shape"
-version = "0.3.2"
+version = "0.4.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "70179a0773695f4fc0b3e8e59f356064ed1532492e52bcf7e0dfef42934ec4c5"
 dependencies = [
  "apollo-compiler",
  "indexmap 2.7.0",

diff --git a/apollo-federation/Cargo.toml b/apollo-federation/Cargo.toml
@@ -43,7 +43,7 @@ url = "2"
 either = "1.13.0"
 tracing = "0.1.40"
 ron = { version = "0.8.1", optional = true }
-shape = { version = "0.3.1", path = "../../shape-rs" }
+shape = "0.4.1"
 
 [dev-dependencies]
 hex.workspace = true

diff --git a/apollo-federation/src/sources/connect/json_selection/methods/future.rs b/apollo-federation/src/sources/connect/json_selection/methods/future.rs
@@ -931,6 +931,7 @@ fn values_method(
         ),
     }
 }
+
 fn values_shape(
     _method_name: &WithRange<String>,
     _method_args: Option<&MethodArgs>,
@@ -940,8 +941,7 @@ fn values_shape(
 ) -> Shape {
     match input_shape.case() {
         ShapeCase::Object { fields, rest, .. } => {
-            let values_vec = fields.values().cloned().collect::<Vec<_>>();
-            Shape::array(values_vec, rest.clone())
+            Shape::array(fields.values().cloned(), rest.clone())
         }
         _ => Shape::error("Method ->values requires an object input"),
     }

diff --git a/apollo-federation/src/sources/connect/validation/mod.rs b/apollo-federation/src/sources/connect/validation/mod.rs
@@ -264,19 +264,31 @@ fn check_seen_fields(
                 return None;
             }
             let coord = |(name, _): (&Name, _)| (extended_type.name().clone(), name.clone());
+
+            // ignore all fields on objects marked @external
+            if extended_type
+                .directives()
+                .iter()
+                .any(|dir| &dir.name == external_directive_name)
+            {
+                return None;
+            }
+
             match extended_type {
-                ExtendedType::Object(object) => Some(
-                    // ignore @external fields
-                    object
-                        .fields
-                        .iter()
-                        .filter(|(_, def)| {
-                            !def.directives
-                                .iter()
-                                .any(|dir| &dir.name == external_directive_name)
-                        })
-                        .map(coord),
-                ),
+                ExtendedType::Object(object) => {
+                    // ignore fields marked @external
+                    Some(
+                        object
+                            .fields
+                            .iter()
+                            .filter(|(_, def)| {
+                                !def.directives
+                                    .iter()
+                                    .any(|dir| &dir.name == external_directive_name)
+                            })
+                            .map(coord),
+                    )
+                }
                 ExtendedType::Interface(_) => None, // TODO: when interfaces are supported (probably should include fields from implementing/member types as well)
                 _ => None,
             }

diff --git a/...rc/sources/connect/validation/snapshots/validation_tests@all_fields_selected.graphql.snap b/...rc/sources/connect/validation/snapshots/validation_tests@all_fields_selected.graphql.snap
@@ -1,6 +1,6 @@
 ---
 source: apollo-federation/src/sources/connect/validation/mod.rs
-expression: "format!(\"{:#?}\", errors)"
+expression: "format!(\"{:#?}\", result.errors)"
 input_file: apollo-federation/src/sources/connect/validation/test_data/all_fields_selected.graphql
 ---
 [
@@ -15,28 +15,28 @@ input_file: apollo-federation/src/sources/connect/validation/test_data/all_field
         code: ConnectorsUnresolvedField,
         message: "No connector resolves field `T.secondUnused`. It must have a `@connect` directive or appear in `@connect(selection:)`.",
         locations: [
-            48:3..48:23,
+            49:3..49:23,
         ],
     },
     Message {
         code: ConnectorsUnresolvedField,
         message: "No connector resolves field `C.unselected`. It must have a `@connect` directive or appear in `@connect(selection:)`.",
         locations: [
-            54:3..54:21,
+            55:3..55:21,
         ],
     },
     Message {
         code: ConnectorsUnresolvedField,
         message: "No connector resolves field `D.unselected`. It must have a `@connect` directive or appear in `@connect(selection:)`.",
         locations: [
-            59:3..59:21,
+            60:3..60:21,
         ],
     },
     Message {
         code: ConnectorsUnresolvedField,
         message: "No connector resolves field `Unused.unselected`. It must have a `@connect` directive or appear in `@connect(selection:)`.",
         locations: [
-            63:3..63:18,
+            64:3..64:18,
         ],
     },
 ]
diff --git a/apollo-federation/src/sources/connect/validation/test_data/all_fields_selected.graphql b/apollo-federation/src/sources/connect/validation/test_data/all_fields_selected.graphql
@@ -36,11 +36,12 @@ type T @key(fields: "id") {
   wrapped: D
   unwrapped: String!
   external: External @external
+  external2: External2 @external
   computed: String!
     @requires(fields: "external")
     @connect(
       http: {
-        GET: "http://test/computed?id={$this.id}&external={$this.external.id}"
+        GET: "http://test/computed?id={$this.id}&external={$this.external.id}&external2={$this.external2.id}"
       }
       selection: "$"
     )
@@ -66,3 +67,7 @@ type Unused {
 type External {
   id: ID! @external
 }
+
+type External2 @external {
+  id: ID!
+}
diff --git a/apollo-router/src/plugins/connectors/tests/mod.rs b/apollo-router/src/plugins/connectors/tests/mod.rs
@@ -1579,6 +1579,7 @@ mod quickstart_tests {
             true,
             false,
             Some(vec![CONTENT_TYPE.to_string()]),
+            None,
         )
         .await;
 

diff --git a/apollo-router/src/test_harness/http_snapshot.rs b/apollo-router/src/test_harness/http_snapshot.rs
@@ -312,15 +312,20 @@ impl SnapshotServer {
         offline: bool,
         update: bool,
         include_headers: Option<Vec<String>>,
+        port: Option<u16>,
     ) -> Self {
+        let listener = port.map(|port| {
+            TcpListener::bind(format!("127.0.0.1:{port}"))
+                .expect("Failed to bind an OS port for snapshot server")
+        });
         Self::inner_start(
             snapshot_path,
             base_url,
             true,
             offline,
             update,
             include_headers,
-            None,
+            listener,
         )
         .await
     }

diff --git a/apollo-router/tests/samples/enterprise/connectors-debugging/README.md b/apollo-router/tests/samples/enterprise/connectors-debugging/README.md
@@ -0,0 +1 @@
+Example testing the connectors debugging extensions feature
diff --git a/apollo-router/tests/samples/enterprise/connectors-debugging/configuration.yaml b/apollo-router/tests/samples/enterprise/connectors-debugging/configuration.yaml
@@ -0,0 +1,16 @@
+include_subgraph_errors:
+  all: true
+
+preview_connectors:
+  debug_extensions: true
+  subgraphs:
+    connectors:
+      sources:
+        jsonPlaceholder:
+          override_url: http://localhost:4007
+
+telemetry:
+  exporters:
+    logging:
+      stdout:
+        format: text
diff --git a/apollo-router/tests/samples/enterprise/connectors-debugging/http_snapshots.json b/apollo-router/tests/samples/enterprise/connectors-debugging/http_snapshots.json
@@ -0,0 +1,65 @@
+[
+  {
+    "request": {
+      "method": "GET",
+      "path": "posts/1",
+      "body": null,
+      "headers": {
+        "x-test-from": "client-value"
+      }
+    },
+    "response": {
+      "status": 200,
+      "headers": {
+        "content-type": ["application/json; charset=utf-8"],
+        "date": ["Tue, 07 Jan 2025 18:34:52 GMT"]
+      },
+      "body": {
+        "userId": 1,
+        "id": 1,
+        "title": "sunt aut facere repellat provident occaecati excepturi optio reprehenderit",
+        "body": "quia et suscipit\nsuscipit recusandae consequuntur expedita et cum\nreprehenderit molestiae ut ut quas totam\nnostrum rerum est autem sunt rem eveniet architecto"
+      }
+    }
+  },
+  {
+    "request": {
+      "method": "GET",
+      "path": "users/1",
+      "body": null,
+      "headers": {
+        "x-test-from": "client-value"
+      }
+    },
+    "response": {
+      "status": 200,
+      "headers": {
+        "content-type": ["application/json; charset=utf-8"],
+        "date": ["Tue, 07 Jan 2025 18:34:52 GMT"]
+      },
+      "body": {
+        "id": 1,
+        "name": "Leanne Graham",
+        "username": "Bret",
+        "email": "[email protected]",
+        "address": {
+          "street": "Kulas Light",
+          "suite": "Apt. 556",
+          "city": "Gwenborough",
+          "zipcode": "92998-3874",
+          "geo": {
+            "lat": "-37.3159",
+            "lng": "81.1496"
+          }
+        },
+        "phone": "1-770-736-8031 x56442",
+        "website": "hildegard.org",
+        "company": {
+          "name": "Romaguera-Crona",
+          "catchPhrase": "Multi-layered client-server neural-net",
+          "bs": "harness real-time e-markets"
+        }
+      }
+    }
+  }
+]
-Original file line number
+Diff line change
@@ Expand Up / @@ -1579,6 +1579,7 @@ mod quickstart_tests { @@
                 true,
                 false,
                 Some(vec![CONTENT_TYPE.to_string()]),
+                None,
             )
             .await;
@@ Expand Down @@
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		Example testing the connectors debugging extensions feature