-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Dissallow some readtime fatures in mutation responses
Reviewed By: alunyov Differential Revision: D52644165 fbshipit-source-id: ab8d8326e16d85b6f62754bd6c30fb7e340b9e8b
- Loading branch information
1 parent
2f9e88a
commit ada971e
Showing
29 changed files
with
517 additions
and
48 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
153 changes: 153 additions & 0 deletions
153
compiler/crates/relay-transforms/src/validations/disallow_readtime_features_in_mutations.rs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,153 @@ | ||
/* | ||
* Copyright (c) Meta Platforms, Inc. and affiliates. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
|
||
use common::Diagnostic; | ||
use common::DiagnosticsResult; | ||
use common::FeatureFlag; | ||
use common::NamedItem; | ||
use docblock_shared::RELAY_RESOLVER_DIRECTIVE_NAME; | ||
use graphql_ir::ConstantValue; | ||
use graphql_ir::Field; | ||
use graphql_ir::FragmentDefinition; | ||
use graphql_ir::FragmentSpread; | ||
use graphql_ir::LinkedField; | ||
use graphql_ir::OperationDefinition; | ||
use graphql_ir::Program; | ||
use graphql_ir::ScalarField; | ||
use graphql_ir::ValidationMessage; | ||
use graphql_ir::Validator; | ||
use schema::Schema; | ||
|
||
use crate::ACTION_ARGUMENT; | ||
use crate::REQUIRED_DIRECTIVE_NAME; | ||
use crate::THROW_ACTION; | ||
|
||
/// Some Relay features will cause a field to throw or suspend at read time. | ||
/// These behaviors are incompatible with our mutation APIs. | ||
/// This validator checks that no such features are used in mutations. | ||
pub fn disallow_readtime_features_in_mutations( | ||
program: &Program, | ||
allow_resolvers_mutation_response: &FeatureFlag, | ||
allow_required_in_mutation_response: &FeatureFlag, | ||
enable_relay_resolver_mutations: bool, | ||
) -> DiagnosticsResult<()> { | ||
let mut validator = DisallowReadtimeFeaturesInMutations::new( | ||
program, | ||
allow_resolvers_mutation_response.clone(), | ||
allow_required_in_mutation_response.clone(), | ||
enable_relay_resolver_mutations, | ||
); | ||
validator.validate_program(program) | ||
} | ||
|
||
struct DisallowReadtimeFeaturesInMutations<'program> { | ||
program: &'program Program, | ||
allow_resolvers_mutation_response: FeatureFlag, | ||
allow_required_in_mutation_response: FeatureFlag, | ||
enable_relay_resolver_mutations: bool, | ||
allow_resolvers_for_this_mutation: bool, | ||
allow_required_for_this_mutation: bool, | ||
} | ||
|
||
impl<'program> DisallowReadtimeFeaturesInMutations<'program> { | ||
fn new( | ||
program: &'program Program, | ||
allow_resolvers_mutation_response: FeatureFlag, | ||
allow_required_in_mutation_response: FeatureFlag, | ||
enable_relay_resolver_mutations: bool, | ||
) -> Self { | ||
Self { | ||
program, | ||
allow_resolvers_mutation_response, | ||
allow_required_in_mutation_response, | ||
enable_relay_resolver_mutations, | ||
allow_resolvers_for_this_mutation: false, | ||
allow_required_for_this_mutation: false, | ||
} | ||
} | ||
|
||
fn validate_field(&self, field: &impl Field) -> DiagnosticsResult<()> { | ||
if !self.allow_required_for_this_mutation { | ||
if let Some(directive) = field.directives().named(*REQUIRED_DIRECTIVE_NAME) { | ||
let action = directive | ||
.arguments | ||
.named(*ACTION_ARGUMENT) | ||
.and_then(|arg| arg.value.item.get_constant()); | ||
if let Some(ConstantValue::Enum(action)) = action { | ||
if *action == *THROW_ACTION { | ||
return Err(vec![Diagnostic::error( | ||
ValidationMessage::RequiredInMutation, | ||
directive.name.location, | ||
)]); | ||
} | ||
} | ||
} | ||
} | ||
if !self.allow_resolvers_for_this_mutation | ||
&& self | ||
.program | ||
.schema | ||
.field(field.definition().item) | ||
.directives | ||
.named(*RELAY_RESOLVER_DIRECTIVE_NAME) | ||
.is_some() | ||
{ | ||
return Err(vec![Diagnostic::error( | ||
ValidationMessage::ResolverInMutation, | ||
field.alias_or_name_location(), | ||
)]); | ||
} | ||
|
||
Ok(()) | ||
} | ||
} | ||
|
||
impl Validator for DisallowReadtimeFeaturesInMutations<'_> { | ||
const NAME: &'static str = "disallow_readtime_features_in_mutations"; | ||
const VALIDATE_ARGUMENTS: bool = false; | ||
const VALIDATE_DIRECTIVES: bool = false; | ||
|
||
fn validate_operation(&mut self, operation: &OperationDefinition) -> DiagnosticsResult<()> { | ||
if !operation.is_mutation() { | ||
// No need to traverse into non-mutation operations | ||
return Ok(()); | ||
} | ||
self.allow_resolvers_for_this_mutation = self.enable_relay_resolver_mutations | ||
|| self | ||
.allow_resolvers_mutation_response | ||
.is_enabled_for(operation.name.item.0); | ||
self.allow_required_for_this_mutation = self | ||
.allow_required_in_mutation_response | ||
.is_enabled_for(operation.name.item.0); | ||
let result = self.default_validate_operation(operation); | ||
|
||
// Reset state | ||
self.allow_resolvers_for_this_mutation = false; | ||
self.allow_required_for_this_mutation = false; | ||
|
||
result | ||
} | ||
|
||
fn validate_fragment(&mut self, _fragment: &FragmentDefinition) -> DiagnosticsResult<()> { | ||
// We only care about mutations | ||
Ok(()) | ||
} | ||
|
||
fn validate_fragment_spread(&mut self, _spread: &FragmentSpread) -> DiagnosticsResult<()> { | ||
// Values nested within fragment spreads are fine since they are not read as part of the | ||
// mutation response. | ||
Ok(()) | ||
} | ||
|
||
fn validate_scalar_field(&mut self, field: &ScalarField) -> DiagnosticsResult<()> { | ||
self.validate_field(field) | ||
} | ||
fn validate_linked_field(&mut self, field: &LinkedField) -> DiagnosticsResult<()> { | ||
self.validate_field(field)?; | ||
self.default_validate_linked_field(field) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
14 changes: 14 additions & 0 deletions
14
...eadtime_features_in_mutations/fixtures/fragment_with_required_spread_in_fragment.expected
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
==================================== INPUT ==================================== | ||
mutation MyMutation { | ||
setName(name: "Alice") { | ||
...myFragment | ||
} | ||
} | ||
|
||
fragment myFragment on User { | ||
name @required(action: THROW) | ||
} | ||
|
||
# %extensions% | ||
==================================== OUTPUT =================================== | ||
OK |
11 changes: 11 additions & 0 deletions
11
...readtime_features_in_mutations/fixtures/fragment_with_required_spread_in_fragment.graphql
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
mutation MyMutation { | ||
setName(name: "Alice") { | ||
...myFragment | ||
} | ||
} | ||
|
||
fragment myFragment on User { | ||
name @required(action: THROW) | ||
} | ||
|
||
# %extensions% |
21 changes: 21 additions & 0 deletions
21
...ow_readtime_features_in_mutations/fixtures/mutation_with_linked_resolver.invalid.expected
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
==================================== INPUT ==================================== | ||
# expected-to-throw | ||
mutation MyMutation { | ||
some_resolver | ||
} | ||
|
||
# %extensions% | ||
type SomeType { | ||
id: ID | ||
} | ||
extend type Mutation { | ||
some_resolver: SomeType @relay_resolver | ||
} | ||
==================================== ERROR ==================================== | ||
✖︎ Expected selections on field `some_resolver` of type `Mutation` | ||
|
||
mutation_with_linked_resolver.invalid.graphql:3:3 | ||
2 │ mutation MyMutation { | ||
3 │ some_resolver | ||
│ ^^^^^^^^^^^^^ | ||
4 │ } |
12 changes: 12 additions & 0 deletions
12
...low_readtime_features_in_mutations/fixtures/mutation_with_linked_resolver.invalid.graphql
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
# expected-to-throw | ||
mutation MyMutation { | ||
some_resolver | ||
} | ||
|
||
# %extensions% | ||
type SomeType { | ||
id: ID | ||
} | ||
extend type Mutation { | ||
some_resolver: SomeType @relay_resolver | ||
} |
17 changes: 17 additions & 0 deletions
17
...low_readtime_features_in_mutations/fixtures/mutation_with_required_field.invalid.expected
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
==================================== INPUT ==================================== | ||
# expected-to-throw | ||
mutation MyMutation { | ||
setName(name: "Alice") { | ||
name @required(action: THROW) | ||
} | ||
} | ||
|
||
# %extensions% | ||
==================================== ERROR ==================================== | ||
✖︎ Unexpected `@required(action: THROW)` directive in mutation response. The use of `@required(action: THROW)` is not supported in mutations. | ||
|
||
mutation_with_required_field.invalid.graphql:4:10 | ||
3 │ setName(name: "Alice") { | ||
4 │ name @required(action: THROW) | ||
│ ^^^^^^^^^ | ||
5 │ } |
8 changes: 8 additions & 0 deletions
8
...llow_readtime_features_in_mutations/fixtures/mutation_with_required_field.invalid.graphql
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
# expected-to-throw | ||
mutation MyMutation { | ||
setName(name: "Alice") { | ||
name @required(action: THROW) | ||
} | ||
} | ||
|
||
# %extensions% |
Oops, something went wrong.