From f41361572088d8c970a39951210854d9f1101dba Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Mon, 17 Jun 2024 16:29:05 -0500 Subject: [PATCH 1/5] Adding validity condition for entity arg to only be used with root Query --- ...s@invalid_entity_arg_on_field.graphql.snap | 6 +++ ...sts@valid_entity_arg_on_field.graphql.snap | 6 +++ .../invalid_entity_arg_on_field.graphql | 17 +++++++ .../valid_entity_arg_on_field.graphql | 15 ++++++ .../src/sources/connect/models/validation.rs | 51 +++++++++++++++++++ 5 files changed, 95 insertions(+) create mode 100644 apollo-federation/src/sources/connect/models/snapshots/apollo_federation__sources__connect__models__validation__test_validate_source__validation_tests@invalid_entity_arg_on_field.graphql.snap create mode 100644 apollo-federation/src/sources/connect/models/snapshots/apollo_federation__sources__connect__models__validation__test_validate_source__validation_tests@valid_entity_arg_on_field.graphql.snap create mode 100644 apollo-federation/src/sources/connect/models/test_data/invalid_entity_arg_on_field.graphql create mode 100644 apollo-federation/src/sources/connect/models/test_data/valid_entity_arg_on_field.graphql diff --git a/apollo-federation/src/sources/connect/models/snapshots/apollo_federation__sources__connect__models__validation__test_validate_source__validation_tests@invalid_entity_arg_on_field.graphql.snap b/apollo-federation/src/sources/connect/models/snapshots/apollo_federation__sources__connect__models__validation__test_validate_source__validation_tests@invalid_entity_arg_on_field.graphql.snap new file mode 100644 index 0000000000..9421ecfb93 --- /dev/null +++ b/apollo-federation/src/sources/connect/models/snapshots/apollo_federation__sources__connect__models__validation__test_validate_source__validation_tests@invalid_entity_arg_on_field.graphql.snap @@ -0,0 +1,6 @@ +--- +source: apollo-federation/src/sources/connect/models/validation.rs +expression: "format!(\"{:?}\", errors)" +input_file: apollo-federation/src/sources/connect/models/test_data/invalid_entity_arg_on_field.graphql +--- +[Message { code: EntityNotOnRootQuery, message: "`@connect(entity:)` on `User.bestFriend` is invalid. Entities can only be declared on root `Query` fields.", locations: [Location { line: 15, column: 61 }..Location { line: 15, column: 73 }] }] diff --git a/apollo-federation/src/sources/connect/models/snapshots/apollo_federation__sources__connect__models__validation__test_validate_source__validation_tests@valid_entity_arg_on_field.graphql.snap b/apollo-federation/src/sources/connect/models/snapshots/apollo_federation__sources__connect__models__validation__test_validate_source__validation_tests@valid_entity_arg_on_field.graphql.snap new file mode 100644 index 0000000000..fa331c7877 --- /dev/null +++ b/apollo-federation/src/sources/connect/models/snapshots/apollo_federation__sources__connect__models__validation__test_validate_source__validation_tests@valid_entity_arg_on_field.graphql.snap @@ -0,0 +1,6 @@ +--- +source: apollo-federation/src/sources/connect/models/validation.rs +expression: "format!(\"{:?}\", errors)" +input_file: apollo-federation/src/sources/connect/models/test_data/valid_entity_arg_on_field.graphql +--- +[] diff --git a/apollo-federation/src/sources/connect/models/test_data/invalid_entity_arg_on_field.graphql b/apollo-federation/src/sources/connect/models/test_data/invalid_entity_arg_on_field.graphql new file mode 100644 index 0000000000..2d3277baec --- /dev/null +++ b/apollo-federation/src/sources/connect/models/test_data/invalid_entity_arg_on_field.graphql @@ -0,0 +1,17 @@ +extend schema +@link( + url: "https://specs.apollo.dev/connect/v0.1" + import: ["@connect"] +) + +type Query { + user: User + @connect(http: {GET: "http://127.0.0.1:8000/resources"}, entity: true) +} + +type User { + id: ID! + name: String! + bestFriend: User + @connect(http: {GET: "http://127.0.0.1:8000/resources"}, entity: true) +} diff --git a/apollo-federation/src/sources/connect/models/test_data/valid_entity_arg_on_field.graphql b/apollo-federation/src/sources/connect/models/test_data/valid_entity_arg_on_field.graphql new file mode 100644 index 0000000000..318d6332c7 --- /dev/null +++ b/apollo-federation/src/sources/connect/models/test_data/valid_entity_arg_on_field.graphql @@ -0,0 +1,15 @@ +extend schema +@link( + url: "https://specs.apollo.dev/connect/v0.1" + import: ["@connect"] +) + +type Query { + user: User + @connect(http: {GET: "http://127.0.0.1:8000/resources"}, entity: true) +} + +type User { + id: ID! + name: String! +} diff --git a/apollo-federation/src/sources/connect/models/validation.rs b/apollo-federation/src/sources/connect/models/validation.rs index 139f871d24..8cdc00a2de 100644 --- a/apollo-federation/src/sources/connect/models/validation.rs +++ b/apollo-federation/src/sources/connect/models/validation.rs @@ -67,6 +67,7 @@ use itertools::Itertools; use url::Url; use crate::link::Link; +use crate::sources::connect::spec::schema::CONNECT_ENTITY_ARGUMENT_NAME; use crate::sources::connect::spec::schema::CONNECT_HTTP_ARGUMENT_DELETE_METHOD_NAME; use crate::sources::connect::spec::schema::CONNECT_HTTP_ARGUMENT_GET_METHOD_NAME; use crate::sources::connect::spec::schema::CONNECT_HTTP_ARGUMENT_NAME; @@ -369,6 +370,28 @@ fn validate_field( ), ) }); + + if let Some(entity_arg) = connect_directive + .arguments + .iter() + .find(|arg| arg.name == CONNECT_ENTITY_ARGUMENT_NAME) + { + if entity_arg + .value + .to_bool() + .is_some_and(|entity_arg_value| entity_arg_value) + && category != ObjectCategory::Query + { + errors.push(Message { + code: Code::EntityNotOnRootQuery, + message: format!("{coordinate} is invalid. Entities can only be declared on root `Query` fields.", coordinate = connect_directive_entity_argument_coordinate(connect_directive_name, object_name, &field.name)), + locations: Location::from_node(entity_arg.location(), source_map) + .into_iter() + .collect(), + }) + } + } + if let Some(source_name) = connect_directive .arguments .iter() @@ -500,6 +523,14 @@ fn connect_directive_name_coordinate( format!("`@{connect_directive_name}({CONNECT_SOURCE_ARGUMENT_NAME}: {source})` on `{object}.{field}`") } +fn connect_directive_entity_argument_coordinate( + connect_directive_entity_argument: &Name, + object: &Name, + field: &Name, +) -> String { + format!("`@{connect_directive_entity_argument}({CONNECT_ENTITY_ARGUMENT_NAME}:)` on `{object}.{field}`") +} + fn connect_directive_http_coordinate( connect_directive_name: &Name, object: &Name, @@ -730,6 +761,8 @@ pub enum Code { MultipleHttpMethods, /// The `@connect` directive is missing an HTTP method. MissingHttpMethod, + // The `entity` argument should only be used on the root `Query` field + EntityNotOnRootQuery, } impl Code { @@ -768,4 +801,22 @@ mod test_validate_source { assert_snapshot!(format!("{:?}", errors)); }); } + + // #[test] + // fn invalid_entity_arg_on_field() { + // let schema = include_str!("test_data/invalid_entity_arg_on_field.graphql"); + // let schema = Schema::parse(schema, "test.graphql").unwrap(); + // let errors = validate(schema); + // assert_eq!(errors.len(), 1); + // assert_eq!(errors[0].code, Code::EntityNotOnRootQuery); + // assert_snapshot!(errors[0].message, @r###"`@connect(entity:)` on `User.bestFriend` is invalid. Entities can only be declared on root `Query` fields."###); + // } + + // #[test] + // fn valid_entity_arg_on_field() { + // let schema = include_str!("test_data/valid_entity_arg_on_field.graphql"); + // let schema = Schema::parse(schema, "test.graphql").unwrap(); + // let errors = validate(schema); + // assert_eq!(errors.len(), 0); + // } } From 7930ecb8285fef1ac720fa439486c5cc5f45d421 Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Mon, 17 Jun 2024 17:50:06 -0500 Subject: [PATCH 2/5] changeset --- .changesets/feat_taylor_cnn_140.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changesets/feat_taylor_cnn_140.md diff --git a/.changesets/feat_taylor_cnn_140.md b/.changesets/feat_taylor_cnn_140.md new file mode 100644 index 0000000000..408313c00a --- /dev/null +++ b/.changesets/feat_taylor_cnn_140.md @@ -0,0 +1,5 @@ +### Add validation check for `entity` arg on root `Query` field ([PR #5468](https://github.com/apollographql/router/pull/5468)) + +*Description here* + +By [@tayrrible](https://github.com/tayrrible) in https://github.com/apollographql/router/pull/5468 \ No newline at end of file From 25a94df0063396dd13e4c0931ad3b9b1a657e47d Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Tue, 18 Jun 2024 08:52:49 -0500 Subject: [PATCH 3/5] deleting changelog --- .changesets/feat_taylor_cnn_140.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changesets/feat_taylor_cnn_140.md diff --git a/.changesets/feat_taylor_cnn_140.md b/.changesets/feat_taylor_cnn_140.md deleted file mode 100644 index 408313c00a..0000000000 --- a/.changesets/feat_taylor_cnn_140.md +++ /dev/null @@ -1,5 +0,0 @@ -### Add validation check for `entity` arg on root `Query` field ([PR #5468](https://github.com/apollographql/router/pull/5468)) - -*Description here* - -By [@tayrrible](https://github.com/tayrrible) in https://github.com/apollographql/router/pull/5468 \ No newline at end of file From 3dcb4ec170d8a8de51e456e18de343a8ea3341ed Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Tue, 18 Jun 2024 09:09:11 -0500 Subject: [PATCH 4/5] rebased so fixing merge conflicts & updating tests --- .../src/sources/connect/models/validation.rs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/apollo-federation/src/sources/connect/models/validation.rs b/apollo-federation/src/sources/connect/models/validation.rs index 8cdc00a2de..3f5c18273b 100644 --- a/apollo-federation/src/sources/connect/models/validation.rs +++ b/apollo-federation/src/sources/connect/models/validation.rs @@ -801,22 +801,4 @@ mod test_validate_source { assert_snapshot!(format!("{:?}", errors)); }); } - - // #[test] - // fn invalid_entity_arg_on_field() { - // let schema = include_str!("test_data/invalid_entity_arg_on_field.graphql"); - // let schema = Schema::parse(schema, "test.graphql").unwrap(); - // let errors = validate(schema); - // assert_eq!(errors.len(), 1); - // assert_eq!(errors[0].code, Code::EntityNotOnRootQuery); - // assert_snapshot!(errors[0].message, @r###"`@connect(entity:)` on `User.bestFriend` is invalid. Entities can only be declared on root `Query` fields."###); - // } - - // #[test] - // fn valid_entity_arg_on_field() { - // let schema = include_str!("test_data/valid_entity_arg_on_field.graphql"); - // let schema = Schema::parse(schema, "test.graphql").unwrap(); - // let errors = validate(schema); - // assert_eq!(errors.len(), 0); - // } } From 238799ec78a46a152d507015e08c9dd2632ddabb Mon Sep 17 00:00:00 2001 From: Taylor Jones Date: Tue, 18 Jun 2024 10:01:09 -0500 Subject: [PATCH 5/5] fixing indentation format --- .../src/sources/connect/models/validation.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/apollo-federation/src/sources/connect/models/validation.rs b/apollo-federation/src/sources/connect/models/validation.rs index 3f5c18273b..0ca679815c 100644 --- a/apollo-federation/src/sources/connect/models/validation.rs +++ b/apollo-federation/src/sources/connect/models/validation.rs @@ -383,12 +383,12 @@ fn validate_field( && category != ObjectCategory::Query { errors.push(Message { - code: Code::EntityNotOnRootQuery, - message: format!("{coordinate} is invalid. Entities can only be declared on root `Query` fields.", coordinate = connect_directive_entity_argument_coordinate(connect_directive_name, object_name, &field.name)), - locations: Location::from_node(entity_arg.location(), source_map) - .into_iter() - .collect(), - }) + code: Code::EntityNotOnRootQuery, + message: format!("{coordinate} is invalid. Entities can only be declared on root `Query` fields.", coordinate = connect_directive_entity_argument_coordinate(connect_directive_name, object_name, &field.name)), + locations: Location::from_node(entity_arg.location(), source_map) + .into_iter() + .collect(), + }) } }