validate @connection filters argument

Reviewed By: josephsavona Differential Revision: D45989512 fbshipit-source-id: 6b7f48312d762c8698b9cfdeefe2d5fe206688f1
facebook · May 19, 2023 · 2dd7954 · 2dd7954
1 parent 423d6ba
commit 2dd7954
Show file tree

Hide file tree

Showing 7 changed files with 156 additions and 12 deletions.
diff --git a/compiler/crates/graphql-ir/src/errors.rs b/compiler/crates/graphql-ir/src/errors.rs
@@ -334,6 +334,16 @@ pub enum ValidationMessage {
         filters_arg_name: ArgumentName,
     },
 
+    #[error(
+        "Expected the `{filters_arg_name}` argument to `@{connection_directive_name}` to be a list of argument names to the connection field to use to identify the connection, got `{invalid_name}`. Not specifying `filters` is often recommended and will use all fields."
+    )]
+    InvalidConnectionFiltersArgNotAnArgument {
+        connection_directive_name: DirectiveName,
+        connection_field_name: StringKey,
+        filters_arg_name: ArgumentName,
+        invalid_name: StringKey,
+    },
+
     #[error("@stream_connection does not support aliasing the '{field_name}' field.")]
     UnsupportedAliasingInStreamConnection { field_name: StringKey },
 

diff --git a/compiler/crates/relay-transforms/src/validations/validate_connections.rs b/compiler/crates/relay-transforms/src/validations/validate_connections.rs
@@ -5,6 +5,7 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+use common::ArgumentName;
 use common::Diagnostic;
 use common::DiagnosticsResult;
 use common::NamedItem;
@@ -477,23 +478,36 @@ impl<'s> ConnectionValidation<'s> {
         if let Some((arg, filters_val)) = constant_filters_arg {
             match filters_val {
                 ConstantValue::List(list_val) => {
-                    let non_string_value = list_val
-                        .iter()
-                        .find(|val| !matches!(val, ConstantValue::String(_)));
-
-                    if non_string_value.is_some() {
-                        return Err(vec![
-                            Diagnostic::error(
+                    validate_map(list_val, |filter_val| {
+                        if let ConstantValue::String(filter_val) = filter_val {
+                            if connection_field
+                                .arguments
+                                .named(ArgumentName(*filter_val))
+                                .is_none()
+                            {
+                                Err(vec![Diagnostic::error(
+                                    ValidationMessage::InvalidConnectionFiltersArgNotAnArgument {
+                                        connection_directive_name: connection_directive.name.item,
+                                        connection_field_name: connection_schema_field.name.item,
+                                        filters_arg_name: *FILTERS_ARG_NAME,
+                                        invalid_name: *filter_val,
+                                    },
+                                    arg.value.location,
+                                )])
+                            } else {
+                                Ok(())
+                            }
+                        } else {
+                            Err(vec![Diagnostic::error(
                                 ValidationMessage::InvalidConnectionFiltersArg {
                                     connection_directive_name: connection_directive.name.item,
                                     connection_field_name: connection_schema_field.name.item,
                                     filters_arg_name: *FILTERS_ARG_NAME,
                                 },
                                 arg.value.location,
-                            )
-                            .annotate("related location", connection_field.definition.location),
-                        ]);
-                    }
+                            )])
+                        }
+                    })?;
                 }
                 _ => {
                     return Err(vec![

diff --git a/...y-transforms/tests/validate_connections/fixtures/connection-filters-not-a-string.expected b/...y-transforms/tests/validate_connections/fixtures/connection-filters-not-a-string.expected
@@ -0,0 +1,27 @@
+==================================== INPUT ====================================
+# expected-to-throw
+query NodeQuery($id: ID!, $orderBy: String) {
+  node(id: $id) {
+    id
+    ... on Story {
+      comments(first: 10, orderby: $orderBy)
+        @connection(key: "NodeQuery_comments", filters: [123]) {
+        edges {
+          node {
+            actor {
+              name
+            }
+          }
+        }
+      }
+    }
+  }
+}
+==================================== ERROR ====================================
+✖︎ Expected a value of type 'String'
+
+  connection-filters-not-a-string.graphql:7:58
+    6 │       comments(first: 10, orderby: $orderBy)
+    7 │         @connection(key: "NodeQuery_comments", filters: [123]) {
+      │                                                          ^^^
+    8 │         edges {
diff --git a/...ay-transforms/tests/validate_connections/fixtures/connection-filters-not-a-string.graphql b/...ay-transforms/tests/validate_connections/fixtures/connection-filters-not-a-string.graphql
@@ -0,0 +1,18 @@
+# expected-to-throw
+query NodeQuery($id: ID!, $orderBy: String) {
+  node(id: $id) {
+    id
+    ... on Story {
+      comments(first: 10, orderby: $orderBy)
+        @connection(key: "NodeQuery_comments", filters: [123]) {
+        edges {
+          node {
+            actor {
+              name
+            }
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/...lay-transforms/tests/validate_connections/fixtures/connection-filters-not-an-arg.expected b/...lay-transforms/tests/validate_connections/fixtures/connection-filters-not-an-arg.expected
@@ -0,0 +1,35 @@
+==================================== INPUT ====================================
+# expected-to-throw
+
+query NodeQuery($id: ID!, $ordering: String) {
+  node(id: $id) {
+    id
+    ... on Story {
+      comments(first: 10, orderby: $ordering)
+        @connection(key: "NodeQuery_comments", filters: ["ordering"]) {
+        edges {
+          node {
+            actor {
+              name
+              friends(first: 10) @connection(key: "NodeQuery_friends") {
+                edges {
+                  node {
+                    name
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+}
+==================================== ERROR ====================================
+✖︎ Expected the `filters` argument to `@connection` to be a list of argument names to the connection field to use to identify the connection, got `ordering`. Not specifying `filters` is often recommended and will use all fields.
+
+  connection-filters-not-an-arg.graphql:8:57
+    7 │       comments(first: 10, orderby: $ordering)
+    8 │         @connection(key: "NodeQuery_comments", filters: ["ordering"]) {
+      │                                                         ^^^^^^^^^^^^
+    9 │         edges {
diff --git a/...elay-transforms/tests/validate_connections/fixtures/connection-filters-not-an-arg.graphql b/...elay-transforms/tests/validate_connections/fixtures/connection-filters-not-an-arg.graphql
@@ -0,0 +1,26 @@
+# expected-to-throw
+
+query NodeQuery($id: ID!, $ordering: String) {
+  node(id: $id) {
+    id
+    ... on Story {
+      comments(first: 10, orderby: $ordering)
+        @connection(key: "NodeQuery_comments", filters: ["ordering"]) {
+        edges {
+          node {
+            actor {
+              name
+              friends(first: 10) @connection(key: "NodeQuery_friends") {
+                edges {
+                  node {
+                    name
+                  }
+                }
+              }
+            }
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/compiler/crates/relay-transforms/tests/validate_connections_test.rs b/compiler/crates/relay-transforms/tests/validate_connections_test.rs
@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<f5841c35143706bf9cd6631d1a139778>>
+ * @generated SignedSource<<5700a721e31a6f0171af5d09f9bf1592>>
  */
 
 mod validate_connections;
@@ -40,6 +40,20 @@ fn connection_filters() {
     test_fixture(transform_fixture, "connection-filters.graphql", "validate_connections/fixtures/connection-filters.expected", input, expected);
 }
 
+#[test]
+fn connection_filters_not_a_string() {
+    let input = include_str!("validate_connections/fixtures/connection-filters-not-a-string.graphql");
+    let expected = include_str!("validate_connections/fixtures/connection-filters-not-a-string.expected");
+    test_fixture(transform_fixture, "connection-filters-not-a-string.graphql", "validate_connections/fixtures/connection-filters-not-a-string.expected", input, expected);
+}
+
+#[test]
+fn connection_filters_not_an_arg() {
+    let input = include_str!("validate_connections/fixtures/connection-filters-not-an-arg.graphql");
+    let expected = include_str!("validate_connections/fixtures/connection-filters-not-an-arg.expected");
+    test_fixture(transform_fixture, "connection-filters-not-an-arg.graphql", "validate_connections/fixtures/connection-filters-not-an-arg.expected", input, expected);
+}
+
 #[test]
 fn connection_filters_null_invalid() {
     let input = include_str!("validate_connections/fixtures/connection-filters-null.invalid.graphql");