Fix missing validation for global variables in client-edges (#4513)

Summary: run `validate_global_variables` validation before `remove_client_edge_selections`, as these client edges may reference global variables. Pull Request resolved: #4513 Test Plan: - Added new unit-tests (validation with server schema and resolvers) - both of these cases throwing the same validation error for undefined variable on the query. - Testing on WWW (no existing validation errors!) Running `phps GraphQLDumpSchemas`... Querying files to compile... [WARN] Unable to load saved state, falling back to full build: unable to load [ads_frameworks_lab_intern] compiling... [ar_intern] compiling... [anna] compiling... [ads_manager] compiling... [enterprise] compiling... [calibra_care] compiling... [lightspeed_relay_reactive_executor] compiling... [intern] compiling... [oculus] compiling... [lightspeed_resolvers] compiling... [oversight_board] compiling... [unified_intake] compiling... [oversight_board_intern] compiling... [f2_care] compiling... [facebook] compiling... [oculus_intern] compiling... [novi_toolbox] compiling... [candidate_portal] compiling... [meta_dot_com] compiling... [mobile] compiling... [figma] compiling... [frl] compiling... [novi_risk] compiling... [instagram] compiling... [anna] compiled documents: 3 reader, 3 normalization, 3 operation text [f2_care] compiled documents: 100 reader, 52 normalization, 100 operation text [figma] compiled documents: 142 reader, 134 normalization, 134 operation text [calibra_care] compiled documents: 220 reader, 91 normalization, 216 operation text [novi_toolbox] compiled documents: 10 reader, 10 normalization, 10 operation text [novi_risk] compiled documents: 17 reader, 17 normalization, 17 operation text [lightspeed_relay_reactive_executor] compiled documents: 25 reader, 13 normalization, 0 operation text [unified_intake] compiled documents: 23 reader, 13 normalization, 25 operation text [ar_intern] compiled documents: 30 reader, 19 normalization, 30 operation text [ads_frameworks_lab_intern] compiled documents: 64 reader, 29 normalization, 30 operation text [oversight_board] compiled documents: 125 reader, 39 normalization, 124 operation text [lightspeed_resolvers] compiled documents: 272 reader, 99 normalization, 7 operation text [candidate_portal] compiled documents: 704 reader, 261 normalization, 695 operation text [meta_dot_com] compiled documents: 806 reader, 315 normalization, 838 operation text [frl] compiled documents: 1417 reader, 913 normalization, 1375 operation text [oversight_board_intern] compiled documents: 955 reader, 387 normalization, 953 operation text [ads_manager] compiled documents: 1605 reader, 866 normalization, 1309 operation text [enterprise] compiled documents: 2191 reader, 1304 normalization, 2235 operation text [oculus_intern] compiled documents: 1711 reader, 1098 normalization, 1877 operation text [mobile] compiled documents: 1942 reader, 1257 normalization, 1841 operation text [instagram] compiled documents: 2966 reader, 1391 normalization, 2914 operation text [oculus] compiled documents: 3328 reader, 1452 normalization, 3403 operation text [facebook] compiled documents: 59825 reader, 30162 normalization, 61630 operation text [intern] compiled documents: 129111 reader, 66562 normalization, 134882 operation text Compilation completed. --- Tested with the D50806253: [ERROR] Error in the project `ads_manager`: ✖︎ Operation 'AdsDuplicationDialogWrapperTestQuery' references undefined variables: '$client_ads_promoted_object_type_ui', '$family_line_campaign_ids', '$table_selected_adgroup_ids', '$table_selected_campaign_group_ids', '$table_selected_campaign_ids', '$zero_prediction_gk_enabled'. html/js/ads/ads_manager/duplication/product/zero_conversion_warning/AdsDuplicationZeroConversionWarningNotice.react.js:56:48 55 │ campign_group_ids: $table_selected_campaign_group_ids 56 │ promotedObjectUIStateByCampaign: $client_ads_promoted_object_type_ui │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 57 │ ) include(if: $zero_prediction_gk_enabled) ℹ︎ related location html/js/ads/ads_manager/duplication/product/zero_conversion_warning/AdsDuplicationZeroConversionWarningNotice.react.js:42:20 41 │ ad_campaigns: combined_campaigns( 42 │ ids: $family_line_campaign_ids │ ^^^^^^^^^^^^^^^^^^^^^^^^^ 43 │ hostID: $hostID ℹ︎ related location html/js/ads/ueditor/core/selectors/hooks/useAdsUEditorGetDerivedPromotedObjectTypeForSelectedAdgroups.js:31:18 30 │ ad_table_selected_adgroups: combined_adgroups( 31 │ ids: $table_selected_adgroup_ids │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ 32 │ hostID: $hostID ℹ︎ related location html/js/ads/ads_manager/duplication/product/zero_conversion_warning/AdsDuplicationZeroConversionWarningNotice.react.js:55:34 54 │ host_id: $hostID 55 │ campign_group_ids: $table_selected_campaign_group_ids │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 56 │ promotedObjectUIStateByCampaign: $client_ads_promoted_object_type_ui ℹ︎ related location html/js/ads/ads_manager/duplication/product/zero_conversion_warning/AdsDuplicationZeroConversionWarningNotice.react.js:53:28 52 │ zero_conversion_prediction: override_zero_conversion_prediction( 53 │ campign_ids: $table_selected_campaign_ids │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 54 │ host_id: $hostID ℹ︎ related location html/js/ads/ads_manager/duplication/product/zero_conversion_warning/AdsDuplicationZeroConversionWarningNotice.react.js:57:28 56 │ promotedObjectUIStateByCampaign: $client_ads_promoted_object_type_ui 57 │ ) include(if: $zero_prediction_gk_enabled) │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ 58 │ } [ERROR] Compilation failed. Reviewed By: monicatang Differential Revision: D50975000 Pulled By: alunyov fbshipit-source-id: 4bf3299d1633949bd85aba8a3f9aa09db3478c6d
facebook · Nov 3, 2023 · 5ffcdbc · 5ffcdbc
1 parent 8fc5000
commit 5ffcdbc
Show file tree

Hide file tree

Showing 8 changed files with 118 additions and 261 deletions.
diff --git a/...relay-resolver-with-args-fragment-spread-using-undefined-global-variable.invalid.expected b/...relay-resolver-with-args-fragment-spread-using-undefined-global-variable.invalid.expected
@@ -0,0 +1,38 @@
+==================================== INPUT ====================================
+# expected-to-throw
+
+fragment relayResolverWithArgsFragmentSpreadUsingUndefinedGlobalVariable_PopStarNameResolverFragment_name on ClientType @argumentDefinitions(scale: {type: "Float!"}) {
+  name
+  profile_picture(scale: $scale)
+}
+
+query relayResolverWithArgsFragmentSpreadUsingUndefinedGlobalVariable_Query {
+  ...relayResolverWithArgsFragmentSpreadUsingUndefinedGlobalVariable_Fragment
+}
+
+fragment relayResolverWithArgsFragmentSpreadUsingUndefinedGlobalVariable_Fragment on Query {
+  pop_star {
+    pop_star_name(scale: $scale)
+  }
+}
+
+
+# %extensions%
+
+type ClientType {
+  name: String
+  profile_picture(scale: Float!): String
+  pop_star_name(scale: Float!): String @relay_resolver(fragment_name: "relayResolverWithArgsFragmentSpreadUsingUndefinedGlobalVariable_PopStarNameResolverFragment_name", import_path: "./path/to/PopStarNameResolver.js", , import_name: "pop_star_name")
+}
+
+extend type Query {
+  pop_star: ClientType @relay_resolver(import_path: "./path/to/PopStarNameResolver.js", import_name: "pop_star")
+}
+==================================== ERROR ====================================
+✖︎ Operation 'relayResolverWithArgsFragmentSpreadUsingUndefinedGlobalVariable_Query' references undefined variable: '$scale'.
+
+  relay-resolver-with-args-fragment-spread-using-undefined-global-variable.invalid.graphql:14:26
+   13 │   pop_star {
+   14 │     pop_star_name(scale: $scale)
+      │                          ^^^^^^
+   15 │   }
diff --git a/.../relay-resolver-with-args-fragment-spread-using-undefined-global-variable.invalid.graphql b/.../relay-resolver-with-args-fragment-spread-using-undefined-global-variable.invalid.graphql
@@ -0,0 +1,29 @@
+# expected-to-throw
+
+fragment relayResolverWithArgsFragmentSpreadUsingUndefinedGlobalVariable_PopStarNameResolverFragment_name on ClientType @argumentDefinitions(scale: {type: "Float!"}) {
+  name
+  profile_picture(scale: $scale)
+}
+
+query relayResolverWithArgsFragmentSpreadUsingUndefinedGlobalVariable_Query {
+  ...relayResolverWithArgsFragmentSpreadUsingUndefinedGlobalVariable_Fragment
+}
+
+fragment relayResolverWithArgsFragmentSpreadUsingUndefinedGlobalVariable_Fragment on Query {
+  pop_star {
+    pop_star_name(scale: $scale)
+  }
+}
+
+
+# %extensions%
+
+type ClientType {
+  name: String
+  profile_picture(scale: Float!): String
+  pop_star_name(scale: Float!): String @relay_resolver(fragment_name: "relayResolverWithArgsFragmentSpreadUsingUndefinedGlobalVariable_PopStarNameResolverFragment_name", import_path: "./path/to/PopStarNameResolver.js", , import_name: "pop_star_name")
+}
+
+extend type Query {
+  pop_star: ClientType @relay_resolver(import_path: "./path/to/PopStarNameResolver.js", import_name: "pop_star")
+}
diff --git a/.../tests/compile_relay_artifacts/fixtures/relay-resolver-with-args-fragment-spread.expected b/.../tests/compile_relay_artifacts/fixtures/relay-resolver-with-args-fragment-spread.expected
diff --git a/...r/tests/compile_relay_artifacts/fixtures/relay-resolver-with-args-fragment-spread.graphql b/...r/tests/compile_relay_artifacts/fixtures/relay-resolver-with-args-fragment-spread.graphql
diff --git a/...sts/compile_relay_artifacts/fixtures/validate-global-variables-undefined.invalid.expected b/...sts/compile_relay_artifacts/fixtures/validate-global-variables-undefined.invalid.expected
@@ -0,0 +1,22 @@
+==================================== INPUT ====================================
+# expected-to-throw
+query validateGlobalVariablesUndefinedQuery {
+  me {
+    ...validateGlobalVariablesUndefined_user
+  }
+}
+
+fragment validateGlobalVariablesUndefined_user on User {
+  id
+  ... @include(if: $condition) {
+    lastName
+  }
+}
+==================================== ERROR ====================================
+✖︎ Operation 'validateGlobalVariablesUndefinedQuery' references undefined variable: '$condition'.
+
+  validate-global-variables-undefined.invalid.graphql:10:20
+    9 │   id
+   10 │   ... @include(if: $condition) {
+      │                    ^^^^^^^^^^
+   11 │     lastName
diff --git a/...ests/compile_relay_artifacts/fixtures/validate-global-variables-undefined.invalid.graphql b/...ests/compile_relay_artifacts/fixtures/validate-global-variables-undefined.invalid.graphql
@@ -0,0 +1,13 @@
+# expected-to-throw
+query validateGlobalVariablesUndefinedQuery {
+  me {
+    ...validateGlobalVariablesUndefined_user
+  }
+}
+
+fragment validateGlobalVariablesUndefined_user on User {
+  id
+  ... @include(if: $condition) {
+    lastName
+  }
+}
diff --git a/compiler/crates/relay-compiler/tests/compile_relay_artifacts_test.rs b/compiler/crates/relay-compiler/tests/compile_relay_artifacts_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<<543023a93cfd81f327040c9b43f96de3>>
+ * @generated SignedSource<<06e1b766183e3717178a63a288c23273>>
  */
 
 mod compile_relay_artifacts;
@@ -1224,10 +1224,10 @@ async fn relay_resolver_with_args_and_alias_no_fragment() {
 }
 
 #[tokio::test]
-async fn relay_resolver_with_args_fragment_spread() {
-    let input = include_str!("compile_relay_artifacts/fixtures/relay-resolver-with-args-fragment-spread.graphql");
-    let expected = include_str!("compile_relay_artifacts/fixtures/relay-resolver-with-args-fragment-spread.expected");
-    test_fixture(transform_fixture, "relay-resolver-with-args-fragment-spread.graphql", "compile_relay_artifacts/fixtures/relay-resolver-with-args-fragment-spread.expected", input, expected).await;
+async fn relay_resolver_with_args_fragment_spread_using_undefined_global_variable_invalid() {
+    let input = include_str!("compile_relay_artifacts/fixtures/relay-resolver-with-args-fragment-spread-using-undefined-global-variable.invalid.graphql");
+    let expected = include_str!("compile_relay_artifacts/fixtures/relay-resolver-with-args-fragment-spread-using-undefined-global-variable.invalid.expected");
+    test_fixture(transform_fixture, "relay-resolver-with-args-fragment-spread-using-undefined-global-variable.invalid.graphql", "compile_relay_artifacts/fixtures/relay-resolver-with-args-fragment-spread-using-undefined-global-variable.invalid.expected", input, expected).await;
 }
 
 #[tokio::test]
@@ -1846,6 +1846,13 @@ async fn validate_global_variables_shared_fragment_invalid() {
     test_fixture(transform_fixture, "validate-global-variables-shared-fragment.invalid.graphql", "compile_relay_artifacts/fixtures/validate-global-variables-shared-fragment.invalid.expected", input, expected).await;
 }
 
+#[tokio::test]
+async fn validate_global_variables_undefined_invalid() {
+    let input = include_str!("compile_relay_artifacts/fixtures/validate-global-variables-undefined.invalid.graphql");
+    let expected = include_str!("compile_relay_artifacts/fixtures/validate-global-variables-undefined.invalid.expected");
+    test_fixture(transform_fixture, "validate-global-variables-undefined.invalid.graphql", "compile_relay_artifacts/fixtures/validate-global-variables-undefined.invalid.expected", input, expected).await;
+}
+
 #[tokio::test]
 async fn viewer_query() {
     let input = include_str!("compile_relay_artifacts/fixtures/viewer-query.graphql");