Skip to content

Commit

Permalink
graphql: Use default value when an argument uses an undefined variable
Browse files Browse the repository at this point in the history
  • Loading branch information
lutter committed Jan 19, 2022
1 parent 8aae775 commit c98485d
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 9 deletions.
75 changes: 66 additions & 9 deletions graphql/src/execution/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -697,39 +697,88 @@ impl<'s> RawQuery<'s> {
}
}

// gql-bug-compat
// Everything with looking up defaults here has to do with making queries
// that use undefined variables work so that a query `{ things(first:
// $first) { .. }}` where `$first` is not defined acts as if `$first` was
// set to the default value 100
trait DefaultLookup {
fn find(&self, name: &str) -> Option<r::Value> {
self.get(name)
.map(|value| r::Value::try_from(value.clone()))
.transpose()
.expect("our API schema does not use variables as default values")
}

fn get(&self, name: &str) -> Option<&q::Value>;
}

struct DirectiveDefaults<'a>(Option<&'a Vec<(String, q::Value)>>);

impl<'a> DefaultLookup for DirectiveDefaults<'a> {
fn get(&self, name: &str) -> Option<&q::Value> {
self.0
.and_then(|values| values.iter().find(|(n, _)| n == name).map(|(_, v)| v))
}
}

struct FieldArgumentDefaults<'a>(&'a s::Field);

impl<'a> DefaultLookup for FieldArgumentDefaults<'a> {
fn get(&self, name: &str) -> Option<&q::Value> {
self.0
.arguments
.iter()
.find(|arg| arg.name == name)
.and_then(|arg| arg.default_value.as_ref())
}
}

struct Transform {
schema: Arc<ApiSchema>,
variables: HashMap<String, r::Value>,
fragments: HashMap<String, q::FragmentDefinition>,
}

impl Transform {
fn variable(&self, name: &str, pos: &Pos) -> Result<r::Value, QueryExecutionError> {
fn variable(
&self,
name: &str,
default: Option<r::Value>,
pos: &Pos,
) -> Result<r::Value, QueryExecutionError> {
self.variables
.get(name)
.map(|value| value.clone())
.or(default)
.ok_or_else(|| QueryExecutionError::MissingVariableError(pos.clone(), name.to_string()))
}

/// Interpolate variable references in the arguments `args`
fn interpolate_arguments(
&self,
args: Vec<(String, q::Value)>,
defaults: &dyn DefaultLookup,
pos: &Pos,
) -> Result<Vec<(String, r::Value)>, QueryExecutionError> {
args.into_iter()
.map(|(name, val)| self.interpolate_value(val, pos).map(|val| (name, val)))
.map(|(name, val)| {
let default = defaults.find(&name);
self.interpolate_value(val, default, pos)
.map(|val| (name, val))
})
.collect()
}

/// Turn `value` into an `r::Value` by resolving variable references
fn interpolate_value(
&self,
value: q::Value,
default: Option<r::Value>,
pos: &Pos,
) -> Result<r::Value, QueryExecutionError> {
match value {
q::Value::Variable(var) => self.variable(&var, pos),
q::Value::Variable(var) => self.variable(&var, default, pos),
q::Value::Int(ref num) => Ok(r::Value::Int(
num.as_i64().expect("q::Value::Int contains an i64"),
)),
Expand All @@ -741,14 +790,14 @@ impl Transform {
q::Value::List(vals) => {
let vals: Vec<_> = vals
.into_iter()
.map(|val| self.interpolate_value(val, pos))
.map(|val| self.interpolate_value(val, None, pos))
.collect::<Result<Vec<_>, _>>()?;
Ok(r::Value::List(vals))
}
q::Value::Object(map) => {
let mut rmap = BTreeMap::new();
for (key, value) in map.into_iter() {
let value = self.interpolate_value(value, pos)?;
let value = self.interpolate_value(value, None, pos)?;
rmap.insert(key, value);
}
Ok(r::Value::object(rmap))
Expand All @@ -762,6 +811,7 @@ impl Transform {
fn interpolate_directives(
&self,
dirs: Vec<q::Directive>,
defns: &Vec<s::Directive>,
) -> Result<(Vec<a::Directive>, bool), QueryExecutionError> {
let dirs = dirs
.into_iter()
Expand All @@ -771,7 +821,13 @@ impl Transform {
position,
arguments,
} = dir;
self.interpolate_arguments(arguments, &position)
let defaults = DirectiveDefaults(
defns
.iter()
.find(|defn| defn.name == name)
.map(|defn| &defn.arguments),
);
self.interpolate_arguments(arguments, &defaults, &position)
.map(|arguments| a::Directive {
name,
position,
Expand Down Expand Up @@ -882,12 +938,13 @@ impl Transform {
)]
})?;

let (directives, skip) = self.interpolate_directives(directives)?;
let (directives, skip) = self.interpolate_directives(directives, &field_type.directives)?;
if skip {
return Ok(None);
}

let mut arguments = self.interpolate_arguments(arguments, &position)?;
let mut arguments =
self.interpolate_arguments(arguments, &FieldArgumentDefaults(&field_type), &position)?;
self.coerce_argument_values(&mut arguments, parent_type, &name)?;

let is_leaf_type = self.schema.document().is_leaf_type(&field_type.field_type);
Expand Down Expand Up @@ -1003,7 +1060,7 @@ impl Transform {
ty: ObjectOrInterface,
newset: &mut a::SelectionSet,
) -> Result<(), Vec<QueryExecutionError>> {
let (directives, skip) = self.interpolate_directives(directives)?;
let (directives, skip) = self.interpolate_directives(directives, &vec![])?;
// Field names in fragment spreads refer to this type, which will
// usually be different from the outer type
let ty = match frag_cond {
Expand Down
28 changes: 28 additions & 0 deletions graphql/tests/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1487,6 +1487,34 @@ fn leaf_selection_mismatch() {
})
}

// see: graphql-bug-compat
#[test]
fn missing_var_uses_default() {
run_test_sequentially(|store| async move {
let deployment = setup(store.as_ref());
let result = execute_query_document(
&deployment.hash,
// '$first' is not defined, use its default from the schema
graphql_parser::parse_query("query { musicians(first: $first, skip: $skip) { id } }")
.expect("invalid test query")
.into_static(),
)
.await;
// We silently set `$first` to 100 and `$skip` to 0, and therefore
// get everything
let exp = object! {
musicians: vec![
object! { id: "m1" },
object! { id: "m2" },
object! { id: "m3" },
object! { id: "m4" },
]
};
let data = extract_data!(result).unwrap();
assert_eq!(exp, data);
})
}

async fn check_musicians_at(
id: &DeploymentHash,
query: &str,
Expand Down

0 comments on commit c98485d

Please sign in to comment.