Skip to content

Commit 53d772d

Browse files
committed
graphql: Fix duplicated scalar fields
1 parent 4a47533 commit 53d772d

File tree

2 files changed

+37
-8
lines changed

2 files changed

+37
-8
lines changed

core/tests/interfaces.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,17 @@ fn alias() {
566566
parent: Legged
567567
}";
568568

569-
let query = "query { l: legged(id: \"child\") { ... on Animal { p: parent { i: id, t: __typename } } } }";
569+
let query = "query {
570+
l: legged(id: \"child\") {
571+
... on Animal {
572+
p: parent {
573+
i: id,
574+
t: __typename,
575+
__typename
576+
}
577+
}
578+
}
579+
}";
570580

571581
let parent = (
572582
Entity::from(vec![
@@ -594,7 +604,8 @@ fn alias() {
594604
l: object! {
595605
p: object! {
596606
i: "parent",
597-
t: "Animal"
607+
t: "Animal",
608+
__typename: "Animal"
598609
}
599610
}
600611
}

graphql/src/execution/execution.rs

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,18 @@ fn execute_selection_set_to_map(
286286
// Group fields with the same response key, so we can execute them together
287287
let grouped_field_set = collect_fields(ctx, object_type, selection_set, None);
288288

289+
// Gather fields that appear more than once with the same response key.
290+
let multiple_response_keys = {
291+
let mut multiple_response_keys = HashSet::new();
292+
let mut fields = HashSet::new();
293+
for field in grouped_field_set.iter().map(|(_, f)| f.iter()).flatten() {
294+
if !fields.insert(field.name.as_str()) {
295+
multiple_response_keys.insert(field.name.as_str());
296+
}
297+
}
298+
multiple_response_keys
299+
};
300+
289301
// Process all field groups in order
290302
for (response_key, fields) in grouped_field_set {
291303
match ctx.deadline {
@@ -298,15 +310,21 @@ fn execute_selection_set_to_map(
298310

299311
// If the field exists on the object, execute it and add its result to the result map
300312
if let Some(ref field) = sast::get_field(object_type, &fields[0].name) {
301-
// If we have the value already, because it's a scalar or a prefetched object, we want
302-
// to use it and avoid cloning, so we take it from the object value.
313+
// Check if we have the value already.
303314
let field_value = prefetched_object
304315
.as_mut()
305316
.map(|o| {
306-
// Prefetched objects associated to `prefetch:response_key`,
307-
// while scalars are associated to the field name.
308-
o.remove(&format!("prefetch:{}", response_key))
309-
.or(o.remove(&fields[0].name))
317+
// Prefetched objects are associated to `prefetch:response_key`.
318+
if let Some(val) = o.remove(&format!("prefetch:{}", response_key)) {
319+
return Some(val);
320+
}
321+
322+
// Scalars and scalar lists are associated to the field name.
323+
// If the field has more than one response key, we have to clone.
324+
match multiple_response_keys.contains(fields[0].name.as_str()) {
325+
false => o.remove(&fields[0].name),
326+
true => o.get(&fields[0].name).cloned(),
327+
}
310328
})
311329
.flatten();
312330
match execute_field(&ctx, object_type, field_value, &fields[0], field, fields) {

0 commit comments

Comments
 (0)