Skip to content

Commit b8650b4

Browse files
ardatankamilkisiela
ardatan
and
kamilkisiela
authored
fix(query-planner): inline nested fragment spreads (#502)
Ref ROUTER-160 In case of a following schema with nested abstract types ```graphql type Query { results: [Result] } interface Result { id: ID! } interface Tshirt implements Result { id: ID! name: String } type SingleColor implements Tshirt & Suggestion { id: ID! name: String colorOption: TshirtColorOption } type MultipleColor implements Tshirt & Result { id: ID! name: String colorOptions: [TshirtColorOption] } type TshirtColorOption { id: ID! price: Float color: Color } enum Color { RED GREEN BLUE } ``` The following query is normalized incorrectly; ```graphql query { results { __typename ...on Tshirt { ...TshirtResult } } } fragment TshirtResult on Tshirt { id name ...on MultipleColor { colorOptions { ...ColorDetails } } ...on SingleColor { colorOption { ...ColorDetails } } } fragment ColorDetails on TshirtColorOption { id color } ``` Some fields are removed incorrectly ```diff query { results { __typename ... on MultipleColor { - id - name - colorOptions { - id - color } } ... on SingleColor { - id - name - colorOption { - id - color } } } } ``` --------- Co-authored-by: Kamil Kisiela <[email protected]>
1 parent 7e64b2a commit b8650b4

File tree

3 files changed

+309
-17
lines changed

3 files changed

+309
-17
lines changed

‎lib/query-planner/fixture/tests/nested-fragments.supergraph.graphql‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
schema
2+
@link(url: "https://specs.apollo.dev/link/v1.0")
3+
@link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) {
4+
query: Query
5+
}
6+
7+
directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE
8+
9+
directive @join__field(
10+
graph: join__Graph
11+
requires: join__FieldSet
12+
provides: join__FieldSet
13+
type: String
14+
external: Boolean
15+
override: String
16+
usedOverridden: Boolean
17+
) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION
18+
19+
directive @join__graph(name: String!, url: String!) on ENUM_VALUE
20+
21+
directive @join__implements(
22+
graph: join__Graph!
23+
interface: String!
24+
) repeatable on OBJECT | INTERFACE
25+
26+
directive @join__type(
27+
graph: join__Graph!
28+
key: join__FieldSet
29+
extension: Boolean! = false
30+
resolvable: Boolean! = true
31+
isInterfaceObject: Boolean! = false
32+
) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR
33+
34+
directive @join__unionMember(
35+
graph: join__Graph!
36+
member: String!
37+
) repeatable on UNION
38+
39+
directive @link(
40+
url: String
41+
as: String
42+
for: link__Purpose
43+
import: [link__Import]
44+
) repeatable on SCHEMA
45+
46+
scalar join__FieldSet
47+
48+
enum join__Graph {
49+
TSHIRTS @join__graph(name: "products", url: "http://localhost/products")
50+
}
51+
52+
scalar link__Import
53+
54+
enum link__Purpose {
55+
"""
56+
`SECURITY` features provide metadata necessary to securely resolve fields.
57+
"""
58+
SECURITY
59+
60+
"""
61+
`EXECUTION` features provide metadata necessary for operation execution.
62+
"""
63+
EXECUTION
64+
}
65+
66+
type MultipleColor implements Tshirt & Result
67+
@join__implements(graph: TSHIRTS, interface: "Tshirt")
68+
@join__implements(graph: TSHIRTS, interface: "Result")
69+
@join__type(graph: TSHIRTS) {
70+
id: ID!
71+
name: String
72+
colorOptions: [TshirtColorOption]
73+
}
74+
75+
interface Tshirt implements Result
76+
@join__implements(graph: TSHIRTS, interface: "Result")
77+
@join__type(graph: TSHIRTS) {
78+
id: ID!
79+
name: String
80+
}
81+
82+
type Query @join__type(graph: TSHIRTS) {
83+
results: [Result]
84+
}
85+
86+
type SingleColor implements Tshirt & Result
87+
@join__implements(graph: TSHIRTS, interface: "Tshirt")
88+
@join__implements(graph: TSHIRTS, interface: "Result")
89+
@join__type(graph: TSHIRTS) {
90+
id: ID!
91+
name: String
92+
colorOption: TshirtColorOption
93+
}
94+
95+
interface Result @join__type(graph: TSHIRTS) {
96+
id: ID!
97+
}
98+
99+
type TshirtColorOption @join__type(graph: TSHIRTS) {
100+
id: ID!
101+
price: Float
102+
color: Color
103+
}
104+
105+
enum Color @join__type(graph: TSHIRTS) {
106+
RED
107+
GREEN
108+
BLUE
109+
}

‎lib/query-planner/src/ast/normalization/mod.rs‎

Lines changed: 159 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,10 @@ mod tests {
9292
use crate::utils::parsing::parse_schema;
9393

9494
fn pretty_query(query_str: String) -> String {
95-
format!("{}", parse_query::<&str>(&query_str).unwrap())
95+
format!(
96+
"{}",
97+
parse_query::<&str>(&query_str).expect(&format!("failed to parse: {}", query_str),)
98+
)
9699
}
97100

98101
// TODO: remove unused variables
@@ -1131,4 +1134,159 @@ mod tests {
11311134
",
11321135
);
11331136
}
1137+
#[test]
1138+
fn nested_fragment_spreads_1() {
1139+
let schema_str =
1140+
std::fs::read_to_string("./fixture/tests/nested-fragments.supergraph.graphql")
1141+
.expect("Unable to read supergraph");
1142+
let schema = parse_schema(&schema_str);
1143+
let supergraph = SupergraphState::new(&schema);
1144+
1145+
insta::assert_snapshot!(
1146+
pretty_query(
1147+
normalize_operation(
1148+
&supergraph,
1149+
&parse_query(
1150+
r#"
1151+
query {
1152+
results {
1153+
__typename
1154+
...on Tshirt {
1155+
...TshirtResult
1156+
}
1157+
}
1158+
}
1159+
1160+
fragment TshirtResult on Tshirt {
1161+
id
1162+
name
1163+
...on MultipleColor {
1164+
colorOptions {
1165+
...ColorDetails
1166+
}
1167+
}
1168+
...on SingleColor {
1169+
colorOption {
1170+
...ColorDetails
1171+
}
1172+
}
1173+
}
1174+
1175+
fragment ColorDetails on TshirtColorOption {
1176+
id
1177+
color
1178+
}
1179+
"#,
1180+
)
1181+
.expect("to parse"),
1182+
None,
1183+
)
1184+
.expect("to normalize")
1185+
.to_string()
1186+
),
1187+
@r"
1188+
query {
1189+
results {
1190+
__typename
1191+
... on MultipleColor {
1192+
id
1193+
name
1194+
colorOptions {
1195+
id
1196+
color
1197+
}
1198+
}
1199+
... on SingleColor {
1200+
id
1201+
name
1202+
colorOption {
1203+
id
1204+
color
1205+
}
1206+
}
1207+
}
1208+
}
1209+
",
1210+
);
1211+
}
1212+
1213+
#[test]
1214+
fn inlining_nested_fragments_with_same_type() {
1215+
let schema_str =
1216+
std::fs::read_to_string("./fixture/tests/nested-fragments.supergraph.graphql")
1217+
.expect("Unable to read supergraph");
1218+
let schema = parse_schema(&schema_str);
1219+
let supergraph = SupergraphState::new(&schema);
1220+
1221+
insta::assert_snapshot!(
1222+
&pretty_query(
1223+
normalize_operation(
1224+
&supergraph,
1225+
&parse_query(
1226+
r#"
1227+
query {
1228+
results {
1229+
__typename
1230+
... on Tshirt {
1231+
...TshirtResult1
1232+
}
1233+
}
1234+
}
1235+
1236+
fragment TshirtResult1 on Tshirt {
1237+
...TshirtResult2
1238+
}
1239+
1240+
fragment TshirtResult2 on Tshirt {
1241+
id
1242+
name
1243+
...on MultipleColor {
1244+
colorOptions {
1245+
...ColorDetails
1246+
}
1247+
}
1248+
...on SingleColor {
1249+
colorOption {
1250+
...ColorDetails
1251+
}
1252+
}
1253+
}
1254+
1255+
fragment ColorDetails on TshirtColorOption {
1256+
id
1257+
color
1258+
}
1259+
"#,
1260+
)
1261+
.expect("to parse"),
1262+
None,
1263+
)
1264+
.unwrap()
1265+
.to_string()
1266+
),
1267+
@r"
1268+
query {
1269+
results {
1270+
__typename
1271+
... on MultipleColor {
1272+
id
1273+
name
1274+
colorOptions {
1275+
id
1276+
color
1277+
}
1278+
}
1279+
... on SingleColor {
1280+
id
1281+
name
1282+
colorOption {
1283+
id
1284+
color
1285+
}
1286+
}
1287+
}
1288+
}
1289+
"
1290+
);
1291+
}
11341292
}

‎lib/query-planner/src/ast/normalization/pipeline/inline_fragment_spreads.rs‎

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::collections::HashMap;
22

33
use graphql_parser::query::{
44
Definition, FragmentDefinition, InlineFragment, Mutation, OperationDefinition, Query,
5-
Selection, SelectionSet, Subscription,
5+
Selection, SelectionSet, Subscription, TypeCondition,
66
};
77

88
use crate::ast::normalization::{context::NormalizationContext, error::NormalizationError};
@@ -20,20 +20,24 @@ pub fn inline_fragment_spreads(ctx: &mut NormalizationContext) -> Result<(), Nor
2020
match definition {
2121
Definition::Operation(op_def) => match op_def {
2222
OperationDefinition::SelectionSet(selection_set) => {
23-
handle_selection_set(selection_set, &fragment_map)?;
23+
handle_selection_set(selection_set, &fragment_map, None)?;
2424
}
2525
OperationDefinition::Query(Query { selection_set, .. }) => {
26-
handle_selection_set(selection_set, &fragment_map)?;
26+
handle_selection_set(selection_set, &fragment_map, None)?;
2727
}
2828
OperationDefinition::Mutation(Mutation { selection_set, .. }) => {
29-
handle_selection_set(selection_set, &fragment_map)?;
29+
handle_selection_set(selection_set, &fragment_map, None)?;
3030
}
3131
OperationDefinition::Subscription(Subscription { selection_set, .. }) => {
32-
handle_selection_set(selection_set, &fragment_map)?;
32+
handle_selection_set(selection_set, &fragment_map, None)?;
3333
}
3434
},
3535
Definition::Fragment(frag_def) => {
36-
handle_selection_set(&mut frag_def.selection_set, &fragment_map)?;
36+
handle_selection_set(
37+
&mut frag_def.selection_set,
38+
&fragment_map,
39+
Some(&frag_def.type_condition),
40+
)?;
3741
}
3842
}
3943
}
@@ -45,14 +49,19 @@ pub fn inline_fragment_spreads(ctx: &mut NormalizationContext) -> Result<(), Nor
4549
fn handle_selection_set<'a>(
4650
selection_set: &mut SelectionSet<'a, String>,
4751
fragment_map: &HashMap<String, FragmentDefinition<'a, String>>,
52+
parent_type_condition: Option<&TypeCondition<'a, String>>,
4853
) -> Result<(), NormalizationError> {
4954
let old_items = std::mem::take(&mut selection_set.items);
5055
let mut new_items = Vec::with_capacity(old_items.len());
5156

5257
for selection in old_items {
5358
match selection {
5459
Selection::Field(mut field) => {
55-
handle_selection_set(&mut field.selection_set, fragment_map)?;
60+
handle_selection_set(
61+
&mut field.selection_set,
62+
fragment_map,
63+
parent_type_condition,
64+
)?;
5665
new_items.push(Selection::Field(field));
5766
}
5867
Selection::FragmentSpread(spread) => {
@@ -62,19 +71,35 @@ fn handle_selection_set<'a>(
6271
}
6372
})?;
6473

65-
let mut inline_fragment = InlineFragment {
66-
position: spread.position,
67-
type_condition: Some(fragment_def.type_condition.clone()),
68-
directives: spread.directives.clone(),
69-
selection_set: fragment_def.selection_set.clone(),
70-
};
74+
if parent_type_condition == Some(&fragment_def.type_condition) {
75+
// If the fragment's type condition matches the top type condition,
76+
// we can inline its selections directly.
77+
let mut selection_set = fragment_def.selection_set.clone();
78+
handle_selection_set(&mut selection_set, fragment_map, parent_type_condition)?;
79+
new_items.extend(selection_set.items);
80+
} else {
81+
let mut inline_fragment = InlineFragment {
82+
position: spread.position,
83+
type_condition: Some(fragment_def.type_condition.clone()),
84+
directives: spread.directives.clone(),
85+
selection_set: fragment_def.selection_set.clone(),
86+
};
7187

72-
handle_selection_set(&mut inline_fragment.selection_set, fragment_map)?;
88+
handle_selection_set(
89+
&mut inline_fragment.selection_set,
90+
fragment_map,
91+
inline_fragment.type_condition.as_ref(),
92+
)?;
7393

74-
new_items.push(Selection::InlineFragment(inline_fragment));
94+
new_items.push(Selection::InlineFragment(inline_fragment));
95+
}
7596
}
7697
Selection::InlineFragment(mut inline_fragment) => {
77-
handle_selection_set(&mut inline_fragment.selection_set, fragment_map)?;
98+
handle_selection_set(
99+
&mut inline_fragment.selection_set,
100+
fragment_map,
101+
inline_fragment.type_condition.as_ref(),
102+
)?;
78103
new_items.push(Selection::InlineFragment(inline_fragment));
79104
}
80105
}

0 commit comments

Comments
 (0)