diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index de1a0f3d0a..6cf67046ca 100644 --- a/NEXT_CHANGELOG.md +++ b/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. diff --git a/apollo-router-core/src/spec/query.rs b/apollo-router-core/src/spec/query.rs index 781db246d6..6563f0ab64 100644 --- a/apollo-router-core/src/spec/query.rs +++ b/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] diff --git a/docker-compose-federation2.yml b/docker-compose-federation2.yml index 860cdf5cd1..ad7fbce842 100644 --- a/docker-compose-federation2.yml +++ b/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 diff --git a/fuzz/fuzz_targets/router.rs b/fuzz/fuzz_targets/router.rs index 7cc01ac465..882dcf2233 100644 --- a/fuzz/fuzz_targets/router.rs +++ b/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 { diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs index 8a2e8a0ddd..a0f4660284 100644 --- a/fuzz/src/lib.rs +++ b/fuzz/src/lib.rs @@ -10,7 +10,7 @@ pub fn generate_valid_operation(input: &[u8]) -> Result { 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 { diff --git a/fuzz/supergraph-fed2.graphql b/fuzz/supergraph-fed2.graphql new file mode 100644 index 0000000000..f588155d30 --- /dev/null +++ b/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) +} diff --git a/fuzz/supergraph.graphql b/fuzz/supergraph.graphql index 5c85026632..b0d984195c 100644 --- a/fuzz/supergraph.graphql +++ b/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