Skip to content

Commit cd840a0

Browse files
committed
fix: validate subscription with conditional selections
Adds validation for subscription operations that specify `@skip`/`@include` conditionals on root selections. See: graphql/graphql-spec#860
1 parent 3a938df commit cd840a0

File tree

4 files changed

+148
-0
lines changed

4 files changed

+148
-0
lines changed

crates/apollo-compiler/src/executable/mod.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,16 @@ pub(crate) enum BuildError {
266266
/// Name of the introspection field
267267
field: Name,
268268
},
269+
#[error(
270+
"{} can not specify @skip or @include on root fields",
271+
subscription_name_or_anonymous(name)
272+
)]
273+
SubscriptionUsesConditionalSelection {
274+
/// Name of the operation
275+
name: Option<Name>,
276+
/// Field name that specify @skip or @include directives.
277+
field: Name,
278+
},
269279

270280
#[error("{0}")]
271281
ConflictingFieldType(Box<ConflictingFieldType>),

crates/apollo-compiler/src/validation/mod.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,9 @@ impl DiagnosticData {
340340
ExecutableBuildError::SubscriptionUsesIntrospection { .. } => {
341341
"SubscriptionUsesIntrospection"
342342
}
343+
ExecutableBuildError::SubscriptionUsesConditionalSelection { .. } => {
344+
"SubscriptionUsesConditionalSelection"
345+
}
343346
ExecutableBuildError::ConflictingFieldType(_) => "ConflictingFieldType",
344347
ExecutableBuildError::ConflictingFieldName(_) => "ConflictingFieldName",
345348
ExecutableBuildError::ConflictingFieldArgument(_) => "ConflictingFieldArgument",
@@ -588,6 +591,18 @@ impl DiagnosticData {
588591
.to_string())
589592
}
590593
}
594+
ExecutableBuildError::SubscriptionUsesConditionalSelection { name, .. } => {
595+
if let Some(name) = name {
596+
Some(format!(
597+
r#"Subscription "{name}" can not specify @skip or @include on root fields."#
598+
))
599+
} else {
600+
Some(
601+
"Anonymous Subscription can not specify @skip or @include on root fields."
602+
.to_string(),
603+
)
604+
}
605+
}
591606
ExecutableBuildError::ConflictingFieldType(inner) => {
592607
let ConflictingFieldType {
593608
alias,
@@ -841,6 +856,12 @@ impl ToCliReport for DiagnosticData {
841856
format_args!("{field} is an introspection field"),
842857
);
843858
}
859+
ExecutableBuildError::SubscriptionUsesConditionalSelection { field, .. } => {
860+
report.with_label_opt(
861+
self.location,
862+
format_args!("{field} specifies @skip or @include condition"),
863+
);
864+
}
844865
ExecutableBuildError::ConflictingFieldType(inner) => {
845866
let ConflictingFieldType {
846867
alias,

crates/apollo-compiler/src/validation/operation.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,27 @@ pub(crate) fn validate_subscription(
4646
},
4747
);
4848
}
49+
50+
// first rule validates that we only have single selection
51+
let has_conditional_field = fields
52+
.iter()
53+
.find(|field| {
54+
field
55+
.field
56+
.directives
57+
.iter()
58+
.any(|d| matches!(d.name.as_str(), "skip" | "include"))
59+
})
60+
.map(|field| field.field);
61+
if let Some(conditional_field) = has_conditional_field {
62+
diagnostics.push(
63+
conditional_field.location(),
64+
executable::BuildError::SubscriptionUsesConditionalSelection {
65+
name: operation.name.clone(),
66+
field: conditional_field.name.clone(),
67+
},
68+
);
69+
}
4970
}
5071
}
5172

crates/apollo-compiler/tests/validation/operation.rs

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,3 +218,99 @@ type Product {
218218
"{errors}"
219219
);
220220
}
221+
222+
#[test]
223+
fn it_validates_subscription_cannot_specify_multiple_fields() {
224+
let input = r#"
225+
subscription MultipleSubs {
226+
ticker1
227+
ticker2
228+
}
229+
230+
type Query {
231+
hello: String
232+
}
233+
234+
type Subscription {
235+
ticker1: String
236+
ticker2: String
237+
}
238+
"#;
239+
240+
let errors = Parser::new()
241+
.parse_mixed_validate(input, "schema.graphql")
242+
.unwrap_err()
243+
.to_string();
244+
assert!(
245+
errors.contains("subscription `MultipleSubs` can only have one root field"),
246+
"{errors}"
247+
);
248+
assert!(
249+
errors.contains("There are 2 root fields: ticker1, ticker2. This is not allowed."),
250+
"{errors}"
251+
);
252+
}
253+
254+
#[test]
255+
fn it_validates_subscription_cannot_select_introspection_fields() {
256+
let input = r#"
257+
subscription IntrospectionSub {
258+
__typename
259+
}
260+
261+
type Query {
262+
hello: String
263+
}
264+
265+
type Subscription {
266+
ticker: String
267+
}
268+
"#;
269+
270+
let errors = Parser::new()
271+
.parse_mixed_validate(input, "schema.graphql")
272+
.unwrap_err()
273+
.to_string();
274+
assert!(
275+
errors.contains(
276+
"subscription `IntrospectionSub` can not have an introspection field as a root field"
277+
),
278+
"{errors}"
279+
);
280+
assert!(
281+
errors.contains("__typename is an introspection field"),
282+
"{errors}"
283+
);
284+
}
285+
286+
#[test]
287+
fn it_validates_subscription_cannot_select_conditional_fields() {
288+
let input = r#"
289+
subscription ConditionalSub($condition: Boolean = true) {
290+
ticker @include(if: $condition)
291+
}
292+
293+
type Query {
294+
hello: String
295+
}
296+
297+
type Subscription {
298+
ticker: String
299+
}
300+
"#;
301+
302+
let errors = Parser::new()
303+
.parse_mixed_validate(input, "schema.graphql")
304+
.unwrap_err()
305+
.to_string();
306+
assert!(
307+
errors.contains(
308+
"subscription `ConditionalSub` can not specify @skip or @include on root fields"
309+
),
310+
"{errors}"
311+
);
312+
assert!(
313+
errors.contains("ticker specifies @skip or @include condition"),
314+
"{errors}"
315+
);
316+
}

0 commit comments

Comments
 (0)