From 2098b42c4c8e06f69edd95210f14d8e57c02410e Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 14 Nov 2022 16:53:36 +0100 Subject: [PATCH 1/3] missing skip and include implementation for root operations --- apollo-router/src/spec/query.rs | 37 ++++++++- apollo-router/src/spec/query/tests.rs | 114 ++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 2 deletions(-) diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index d21df6a82f..986f1fed56 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -735,6 +735,8 @@ impl Query { Selection::InlineFragment { type_condition, selection_set, + skip, + include, .. } => { // top level objects will not provide a __typename field @@ -744,6 +746,14 @@ impl Query { return Err(InvalidValue); } + if skip.should_skip(parameters.variables).unwrap_or(false) { + continue; + } + + if !include.should_include(parameters.variables).unwrap_or(true) { + continue; + } + self.apply_selection_set( selection_set, parameters, @@ -756,10 +766,33 @@ impl Query { Selection::FragmentSpread { name, known_type: _, - skip: _, - include: _, + skip, + include, } => { + if skip.should_skip(parameters.variables).unwrap_or(false) { + continue; + } + + if !include.should_include(parameters.variables).unwrap_or(true) { + continue; + } + if let Some(fragment) = self.fragments.get(name) { + if fragment + .skip + .should_skip(parameters.variables) + .unwrap_or(false) + { + continue; + } + if !fragment + .include + .should_include(parameters.variables) + .unwrap_or(true) + { + continue; + } + let operation_type_name = parameters.schema.root_operation_name(operation.kind); let is_apply = { diff --git a/apollo-router/src/spec/query/tests.rs b/apollo-router/src/spec/query/tests.rs index 7bf5aa13fa..3214d7a194 100644 --- a/apollo-router/src/spec/query/tests.rs +++ b/apollo-router/src/spec/query/tests.rs @@ -4102,6 +4102,120 @@ fn include() { }, }}) .test(); + + FormatTest::builder() + .schema(schema) + .query( + "query Example($shouldInclude: Boolean) { + get { + name + } + ...test @include(if: $shouldInclude) + } + + fragment test on Query { + get { + id + } + }", + ) + .response(json! {{ + "get": { + "name": "a", + }, + }}) + .operation("Example") + .variables(json! {{ + "shouldInclude": false + }}) + .expected(json! {{ + "get": { + "name": "a", + }, + }}) + .test(); + + FormatTest::builder() + .schema(schema) + .query( + "query Example($shouldInclude: Boolean) { + get { + name + } + ...test @include(if: $shouldInclude) + } + + fragment test on Query { + get { + id + } + }", + ) + .response(json! {{ + "get": { + "name": "a", + }, + }}) + .operation("Example") + .expected(json! {{ + "get": null, + }}) + .test(); + + FormatTest::builder() + .schema(schema) + .query( + "query Example($shouldInclude: Boolean) { + get { + name + } + ... @include(if: $shouldInclude) { + get { + id + } + } + }", + ) + .response(json! {{ + "get": { + "name": "a", + }, + }}) + .operation("Example") + .variables(json! {{ + "shouldInclude": false + }}) + .expected(json! {{ + "get": { + "name": "a", + }, + }}) + .test(); + + FormatTest::builder() + .schema(schema) + .query( + "query Example($shouldInclude: Boolean) { + get { + name + } + ... @include(if: $shouldInclude) { + get { + id + } + } + }", + ) + .response(json! {{ + "get": { + "name": "a", + }, + }}) + .operation("Example") + .expected(json! {{ + "get": null, + }}) + .test(); } #[test] From 9d51dc8170fdd41774a30e42db58e227d0f8440c Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Mon, 14 Nov 2022 17:16:50 +0100 Subject: [PATCH 2/3] changelog --- NEXT_CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index d6aab17885..85535658e2 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -63,6 +63,12 @@ the `Accept` header means `*/*` when it is absent. By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/2078 +### Missing `@skip` and `@include` implementation for root operations ([Issue #2072](https://github.com/apollographql/router/issues/2072)) + +`@skip` and `@include` were not implemented for inline fratgments and fragment spreads on top level operations. + +By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/2096 + ## 🛠 Maintenance ## 📚 Documentation From 95be815438fb16fa4e799205865f57b5f9ae5feb Mon Sep 17 00:00:00 2001 From: Geoffroy Couprie Date: Tue, 15 Nov 2022 10:29:27 +0100 Subject: [PATCH 3/3] Update NEXT_CHANGELOG.md Co-authored-by: Bryn Cooke --- NEXT_CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 85535658e2..b7240fef20 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -65,7 +65,7 @@ By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router ### Missing `@skip` and `@include` implementation for root operations ([Issue #2072](https://github.com/apollographql/router/issues/2072)) -`@skip` and `@include` were not implemented for inline fratgments and fragment spreads on top level operations. +`@skip` and `@include` were not implemented for inline fragments and fragment spreads on top level operations. By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/2096