Skip to content

Commit 570b348

Browse files
committed
graphql: Use default value when an argument uses an undefined variable
1 parent 8aae775 commit 570b348

File tree

2 files changed

+94
-9
lines changed

2 files changed

+94
-9
lines changed

graphql/src/execution/query.rs

Lines changed: 66 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -697,39 +697,88 @@ impl<'s> RawQuery<'s> {
697697
}
698698
}
699699

700+
// graphql-bug-compat
701+
// Everything with looking up defaults here has to do with making queries
702+
// that use undefined variables work so that a query `{ things(first:
703+
// $first) { .. }}` where `$first` is not defined acts as if `$first` was
704+
// set to the default value 100
705+
trait DefaultLookup {
706+
fn find(&self, name: &str) -> Option<r::Value> {
707+
self.get(name)
708+
.map(|value| r::Value::try_from(value.clone()))
709+
.transpose()
710+
.expect("our API schema does not use variables as default values")
711+
}
712+
713+
fn get(&self, name: &str) -> Option<&q::Value>;
714+
}
715+
716+
struct DirectiveDefaults<'a>(Option<&'a Vec<(String, q::Value)>>);
717+
718+
impl<'a> DefaultLookup for DirectiveDefaults<'a> {
719+
fn get(&self, name: &str) -> Option<&q::Value> {
720+
self.0
721+
.and_then(|values| values.iter().find(|(n, _)| n == name).map(|(_, v)| v))
722+
}
723+
}
724+
725+
struct FieldArgumentDefaults<'a>(&'a s::Field);
726+
727+
impl<'a> DefaultLookup for FieldArgumentDefaults<'a> {
728+
fn get(&self, name: &str) -> Option<&q::Value> {
729+
self.0
730+
.arguments
731+
.iter()
732+
.find(|arg| arg.name == name)
733+
.and_then(|arg| arg.default_value.as_ref())
734+
}
735+
}
736+
700737
struct Transform {
701738
schema: Arc<ApiSchema>,
702739
variables: HashMap<String, r::Value>,
703740
fragments: HashMap<String, q::FragmentDefinition>,
704741
}
705742

706743
impl Transform {
707-
fn variable(&self, name: &str, pos: &Pos) -> Result<r::Value, QueryExecutionError> {
744+
fn variable(
745+
&self,
746+
name: &str,
747+
default: Option<r::Value>,
748+
pos: &Pos,
749+
) -> Result<r::Value, QueryExecutionError> {
708750
self.variables
709751
.get(name)
710752
.map(|value| value.clone())
753+
.or(default)
711754
.ok_or_else(|| QueryExecutionError::MissingVariableError(pos.clone(), name.to_string()))
712755
}
713756

714757
/// Interpolate variable references in the arguments `args`
715758
fn interpolate_arguments(
716759
&self,
717760
args: Vec<(String, q::Value)>,
761+
defaults: &dyn DefaultLookup,
718762
pos: &Pos,
719763
) -> Result<Vec<(String, r::Value)>, QueryExecutionError> {
720764
args.into_iter()
721-
.map(|(name, val)| self.interpolate_value(val, pos).map(|val| (name, val)))
765+
.map(|(name, val)| {
766+
let default = defaults.find(&name);
767+
self.interpolate_value(val, default, pos)
768+
.map(|val| (name, val))
769+
})
722770
.collect()
723771
}
724772

725773
/// Turn `value` into an `r::Value` by resolving variable references
726774
fn interpolate_value(
727775
&self,
728776
value: q::Value,
777+
default: Option<r::Value>,
729778
pos: &Pos,
730779
) -> Result<r::Value, QueryExecutionError> {
731780
match value {
732-
q::Value::Variable(var) => self.variable(&var, pos),
781+
q::Value::Variable(var) => self.variable(&var, default, pos),
733782
q::Value::Int(ref num) => Ok(r::Value::Int(
734783
num.as_i64().expect("q::Value::Int contains an i64"),
735784
)),
@@ -741,14 +790,14 @@ impl Transform {
741790
q::Value::List(vals) => {
742791
let vals: Vec<_> = vals
743792
.into_iter()
744-
.map(|val| self.interpolate_value(val, pos))
793+
.map(|val| self.interpolate_value(val, None, pos))
745794
.collect::<Result<Vec<_>, _>>()?;
746795
Ok(r::Value::List(vals))
747796
}
748797
q::Value::Object(map) => {
749798
let mut rmap = BTreeMap::new();
750799
for (key, value) in map.into_iter() {
751-
let value = self.interpolate_value(value, pos)?;
800+
let value = self.interpolate_value(value, None, pos)?;
752801
rmap.insert(key, value);
753802
}
754803
Ok(r::Value::object(rmap))
@@ -762,6 +811,7 @@ impl Transform {
762811
fn interpolate_directives(
763812
&self,
764813
dirs: Vec<q::Directive>,
814+
defns: &Vec<s::Directive>,
765815
) -> Result<(Vec<a::Directive>, bool), QueryExecutionError> {
766816
let dirs = dirs
767817
.into_iter()
@@ -771,7 +821,13 @@ impl Transform {
771821
position,
772822
arguments,
773823
} = dir;
774-
self.interpolate_arguments(arguments, &position)
824+
let defaults = DirectiveDefaults(
825+
defns
826+
.iter()
827+
.find(|defn| defn.name == name)
828+
.map(|defn| &defn.arguments),
829+
);
830+
self.interpolate_arguments(arguments, &defaults, &position)
775831
.map(|arguments| a::Directive {
776832
name,
777833
position,
@@ -882,12 +938,13 @@ impl Transform {
882938
)]
883939
})?;
884940

885-
let (directives, skip) = self.interpolate_directives(directives)?;
941+
let (directives, skip) = self.interpolate_directives(directives, &field_type.directives)?;
886942
if skip {
887943
return Ok(None);
888944
}
889945

890-
let mut arguments = self.interpolate_arguments(arguments, &position)?;
946+
let mut arguments =
947+
self.interpolate_arguments(arguments, &FieldArgumentDefaults(&field_type), &position)?;
891948
self.coerce_argument_values(&mut arguments, parent_type, &name)?;
892949

893950
let is_leaf_type = self.schema.document().is_leaf_type(&field_type.field_type);
@@ -1003,7 +1060,7 @@ impl Transform {
10031060
ty: ObjectOrInterface,
10041061
newset: &mut a::SelectionSet,
10051062
) -> Result<(), Vec<QueryExecutionError>> {
1006-
let (directives, skip) = self.interpolate_directives(directives)?;
1063+
let (directives, skip) = self.interpolate_directives(directives, &vec![])?;
10071064
// Field names in fragment spreads refer to this type, which will
10081065
// usually be different from the outer type
10091066
let ty = match frag_cond {

graphql/tests/query.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1487,6 +1487,34 @@ fn leaf_selection_mismatch() {
14871487
})
14881488
}
14891489

1490+
// see: graphql-bug-compat
1491+
#[test]
1492+
fn missing_var_uses_default() {
1493+
run_test_sequentially(|store| async move {
1494+
let deployment = setup(store.as_ref());
1495+
let result = execute_query_document(
1496+
&deployment.hash,
1497+
// '$first' is not defined, use its default from the schema
1498+
graphql_parser::parse_query("query { musicians(first: $first, skip: $skip) { id } }")
1499+
.expect("invalid test query")
1500+
.into_static(),
1501+
)
1502+
.await;
1503+
// We silently set `$first` to 100 and `$skip` to 0, and therefore
1504+
// get everything
1505+
let exp = object! {
1506+
musicians: vec![
1507+
object! { id: "m1" },
1508+
object! { id: "m2" },
1509+
object! { id: "m3" },
1510+
object! { id: "m4" },
1511+
]
1512+
};
1513+
let data = extract_data!(result).unwrap();
1514+
assert_eq!(exp, data);
1515+
})
1516+
}
1517+
14901518
async fn check_musicians_at(
14911519
id: &DeploymentHash,
14921520
query: &str,

0 commit comments

Comments
 (0)