fix unskipped fields at the root of the selection set #930

NEXT_CHANGELOG.md

@@ -66,6 +66,9 @@ telemetry:
 ```
 
 ## 🐛 Fixes
+### Fields in the root selection set of a query are now correctly skipped and included [PR #931](https://github.com/apollographql/router/pull/931)
+The `@skip` and `@include` directives are now executed for the fields in the root selection set.
+
 ###  Configuration errors on hot-reload are output [PR #850](https://github.com/apollographql/router/pull/850)
 If a configuration file had errors on reload these were silently swallowed. These are now added to the logs.
 

apollo-router-core/src/spec/query.rs

@@ -479,9 +479,27 @@ impl Query {
                     alias,
                     selection_set,
                     field_type,
-                    skip: _,
-                    include: _,
+                    skip,
+                    include,
                 } => {
+                    if skip
+                        .should_skip(variables)
+                        // validate_variables should have already checked that
+                        // the variable is present and it is of the correct type
+                        .unwrap_or_default()
+                    {
+                        continue;
+                    }
+
+                    if !include
+                        .should_include(variables)
+                        // validate_variables should have already checked that
+                        // the variable is present and it is of the correct type
+                        .unwrap_or(true)
+                    {
+                        continue;
+                    }
+
                     let field_name = alias.as_ref().unwrap_or(name);
                     if let Some(input_value) = input.get_mut(field_name.as_str()) {
                         // if there's already a value for that key in the output it means either:
@@ -3674,6 +3692,7 @@ mod tests {
         type Product {
             id: String!
             name: String
+            bar: String
         }";
 
         // combine skip and include
@@ -3724,6 +3743,32 @@ mod tests {
                 },
             }},
         );
+
+        assert_format_response!(
+            schema,
+            "query  {
+                get @skip(if: false) @include(if: false) {
+                    a:name
+                    bar
+                }
+                get @skip(if: false) {
+                    a:name
+                    a:name
+                }
+            }",
+            json! {{
+                "get": {
+                    "a": "a",
+                    "bar": "foo",
+                },
+            }},
+            None,
+            json! {{
+                "get": {
+                    "a": "a",
+                },
+            }},
+        );
     }
 
     #[test]

docker-compose-federation2.yml

@@ -1,6 +1,19 @@
 version: "3.9"
 services:
 
+  gateway:
+    container_name: apollo-gateway
+    build: dockerfiles/federation2-demo/gateway
+    environment:
+      - APOLLO_OTEL_EXPORTER_TYPE=collector
+      - APOLLO_OTEL_EXPORTER_HOST=collector
+      - APOLLO_OTEL_EXPORTER_PORT=4318
+      - APOLLO_SCHEMA_CONFIG_EMBEDDED=true
+    volumes:
+      - ./dockerfiles/federation2-demo/supergraph.graphql:/etc/config/supergraph.graphql
+    ports:
+      - "4100:4000"
+
   users:
     container_name: users
     build: dockerfiles/federation2-demo/subgraphs/users

fuzz/fuzz_targets/router.rs

@@ -47,6 +47,10 @@ fuzz_target!(|data: &[u8]| {
         } else {
             None
         };
+        if router_error.is_some() && gateway_error.is_some() {
+            // Do not check errors for now
+            return;
+        }
         let mut file = OpenOptions::new()
             .read(true)
             .write(true)
@@ -77,6 +81,26 @@ fuzz_target!(|data: &[u8]| {
 
         panic!()
     } else if router_response.is_ok() {
+        let gateway_errors_detected = gateway_response
+            .as_ref()
+            .unwrap()
+            .as_object()
+            .unwrap()
+            .get("errors")
+            .map(|e| !e.as_array().unwrap().len())
+            .unwrap_or(0);
+        let router_errors_detected = router_response
+            .as_ref()
+            .unwrap()
+            .as_object()
+            .unwrap()
+            .get("errors")
+            .map(|e| !e.as_array().unwrap().len())
+            .unwrap_or(0);
+        if gateway_errors_detected > 0 && gateway_errors_detected == router_errors_detected {
+            // Do not check the shape of errors right now
+            return;
+        }
         let router_response = serde_json::to_string_pretty(&router_response.unwrap()).unwrap();
         let gateway_response = serde_json::to_string_pretty(&gateway_response.unwrap()).unwrap();
         if router_response != gateway_response {

fuzz/src/lib.rs

@@ -10,7 +10,7 @@ pub fn generate_valid_operation(input: &[u8]) -> Result<String> {
     drop(env_logger::try_init());
 
     let parser =
-        Parser::new(&fs::read_to_string("fuzz/supergraph.graphql").expect("cannot read file"));
+        Parser::new(&fs::read_to_string("fuzz/supergraph-fed2.graphql").expect("cannot read file"));
 
     let tree = parser.parse();
     if tree.errors().len() > 0 {

fuzz/supergraph-fed2.graphql

@@ -0,0 +1,141 @@
+schema
+  @core(feature: "https://specs.apollo.dev/core/v0.2")
+  @core(feature: "https://specs.apollo.dev/join/v0.2", for: EXECUTION) {
+  query: Query
+}
+
+directive @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
+directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
+
+directive @core(
+  feature: String!
+  as: String
+  for: core__Purpose
+) repeatable on SCHEMA
+
+directive @join__field(
+  graph: join__Graph!
+  requires: join__FieldSet
+  provides: join__FieldSet
+  type: String
+  external: Boolean
+) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION
+
+directive @join__graph(name: String!, url: String!) on ENUM_VALUE
+
+directive @join__implements(
+  graph: join__Graph!
+  interface: String!
+) repeatable on OBJECT | INTERFACE
+
+directive @join__type(
+  graph: join__Graph!
+  key: join__FieldSet
+  extension: Boolean! = false
+  resolvable: Boolean! = true
+) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR
+
+enum core__Purpose {
+  """
+  `SECURITY` features provide metadata necessary to securely resolve fields.
+  """
+  SECURITY
+
+  """
+  `EXECUTION` features provide metadata necessary for operation execution.
+  """
+  EXECUTION
+}
+
+type DeliveryEstimates @join__type(graph: INVENTORY) {
+  estimatedDelivery: String
+  fastestDelivery: String
+}
+
+scalar join__FieldSet
+
+enum join__Graph {
+  INVENTORY
+    @join__graph(name: "inventory", url: "http://localhost:4004/graphql")
+  PANDAS @join__graph(name: "pandas", url: "http://localhost:4002/graphql")
+  PRODUCTS @join__graph(name: "products", url: "http://localhost:4003/graphql")
+  USERS @join__graph(name: "users", url: "http://localhost:4001/graphql")
+}
+
+type Panda @join__type(graph: PANDAS) {
+  name: ID!
+  favoriteFood: String
+}
+
+type Product implements ProductItf & SkuItf
+  @join__implements(graph: INVENTORY, interface: "ProductItf")
+  @join__implements(graph: PRODUCTS, interface: "ProductItf")
+  @join__implements(graph: PRODUCTS, interface: "SkuItf")
+  @join__type(graph: INVENTORY, key: "id")
+  @join__type(graph: PRODUCTS, key: "id")
+  @join__type(graph: PRODUCTS, key: "sku package")
+  @join__type(graph: PRODUCTS, key: "sku variation { id }") {
+  id: ID!
+  dimensions: ProductDimension
+    @join__field(graph: INVENTORY, external: true)
+    @join__field(graph: PRODUCTS)
+  delivery(zip: String): DeliveryEstimates
+    @join__field(graph: INVENTORY, requires: "dimensions { size weight }")
+  sku: String @join__field(graph: PRODUCTS)
+  package: String @join__field(graph: PRODUCTS)
+  variation: ProductVariation @join__field(graph: PRODUCTS)
+  createdBy: User @join__field(graph: PRODUCTS)
+}
+
+type ProductDimension
+  @join__type(graph: INVENTORY)
+  @join__type(graph: PRODUCTS) {
+  size: String
+  weight: Float
+}
+
+interface ProductItf implements SkuItf
+  @join__implements(graph: PRODUCTS, interface: "SkuItf")
+  @join__type(graph: INVENTORY)
+  @join__type(graph: PRODUCTS) {
+  id: ID!
+  dimensions: ProductDimension
+  delivery(zip: String): DeliveryEstimates @join__field(graph: INVENTORY)
+  sku: String @join__field(graph: PRODUCTS)
+  package: String @join__field(graph: PRODUCTS)
+  variation: ProductVariation @join__field(graph: PRODUCTS)
+  createdBy: User @join__field(graph: PRODUCTS)
+}
+
+type ProductVariation @join__type(graph: PRODUCTS) {
+  id: ID!
+}
+
+type Query
+  @join__type(graph: INVENTORY)
+  @join__type(graph: PANDAS)
+  @join__type(graph: PRODUCTS)
+  @join__type(graph: USERS) {
+  allPandas: [Panda] @join__field(graph: PANDAS)
+  panda(name: ID!): Panda @join__field(graph: PANDAS)
+  allProducts: [ProductItf] @join__field(graph: PRODUCTS)
+  product(id: ID!): ProductItf @join__field(graph: PRODUCTS)
+}
+
+enum ShippingClass @join__type(graph: INVENTORY) @join__type(graph: PRODUCTS) {
+  STANDARD
+  EXPRESS
+  OVERNIGHT
+}
+
+interface SkuItf @join__type(graph: PRODUCTS) {
+  sku: String
+}
+
+type User
+  @join__type(graph: PRODUCTS, key: "email")
+  @join__type(graph: USERS, key: "email") {
+  email: ID!
+  totalProductsCreated: Int
+  name: String @join__field(graph: USERS)
+}

fuzz/supergraph.graphql

@@ -26,8 +26,8 @@ directive @deprecated(
   reason: String = "No longer supported"
 ) on FIELD_DEFINITION | ENUM_VALUE
 
-# Uncomment this directive when this issue https://github.com/apollographql/router/issues/76 is resolved
-# directive @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
+directive @skip(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
+directive @include(if: Boolean!) on FIELD | FRAGMENT_SPREAD | INLINE_FRAGMENT
 
 scalar join__FieldSet