Skip to content

Commit

Permalink
Set up a place for body validations & add the first new check
Browse files Browse the repository at this point in the history
  • Loading branch information
dylan-apollo committed Oct 11, 2024
1 parent 9e3219c commit 619b794
Show file tree
Hide file tree
Showing 12 changed files with 318 additions and 111 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub use apply_to::*;
// remove the `#[cfg(test)]`.
pub(crate) use known_var::*;
pub(crate) use location::Ranged;
pub(crate) use location::WithRange;
pub use parser::*;
#[cfg(test)]
pub(crate) use pretty::*;
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ impl ExternalVarPaths for NamedSelection {

#[derive(Debug, PartialEq, Eq, Clone)]
pub struct PathSelection {
pub(super) path: WithRange<PathList>,
pub(crate) path: WithRange<PathList>,
}

// Like NamedSelection, PathSelection is an AST structure that takes its range
Expand Down Expand Up @@ -378,7 +378,7 @@ impl From<PathList> for PathSelection {
}

#[derive(Debug, PartialEq, Eq, Clone)]
pub(super) enum PathList {
pub(crate) enum PathList {
// A VarPath must start with a variable (either $identifier, $, or @),
// followed by any number of PathStep items (the WithRange<PathList>).
// Because we represent the @ quasi-variable using PathList::Var, this
Expand Down Expand Up @@ -1079,7 +1079,7 @@ pub(crate) fn parse_string_literal(input: Span) -> IResult<Span, WithRange<Strin
}

#[derive(Debug, PartialEq, Eq, Clone, Default)]
pub(super) struct MethodArgs {
pub(crate) struct MethodArgs {
pub(super) args: Vec<WithRange<LitExpr>>,
pub(super) range: OffsetRange,
}
Expand Down
35 changes: 29 additions & 6 deletions apollo-federation/src/sources/connect/validation/coordinates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,35 @@ impl Display for BaseUrlCoordinate<'_> {
}
}

pub(super) fn connect_directive_http_body_coordinate(
connect_directive_name: &Name,
object: &Node<ObjectType>,
field: &Name,
) -> String {
format!("`@{connect_directive_name}({HTTP_ARGUMENT_NAME}: {{{CONNECT_BODY_ARGUMENT_NAME}:}})` on `{object_name}.{field}`", object_name = object.name)
/// The coordinate of a `@connect(http:body:)`.
#[derive(Clone, Copy)]
pub(super) struct BodyCoordinate<'a> {
pub(super) connect_directive_name: &'a Name,
pub(super) field_coordinate: FieldCoordinate<'a>,
}

impl Display for BodyCoordinate<'_> {
fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> {
let BodyCoordinate {
connect_directive_name,
field_coordinate,
} = self;
write!(f, "`@{connect_directive_name}({HTTP_ARGUMENT_NAME}: {{{CONNECT_BODY_ARGUMENT_NAME}:}})` on {field_coordinate}")
}
}

impl<'a> From<ConnectDirectiveCoordinate<'a>> for BodyCoordinate<'a> {
fn from(connect_directive_coordinate: ConnectDirectiveCoordinate<'a>) -> Self {
let ConnectDirectiveCoordinate {
directive: _,
connect_directive_name,
field_coordinate,
} = connect_directive_coordinate;
Self {
connect_directive_name,
field_coordinate,
}
}
}

pub(super) fn source_http_argument_coordinate(source_directive_name: &DirectiveName) -> String {
Expand Down
14 changes: 3 additions & 11 deletions apollo-federation/src/sources/connect/validation/extended_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,13 @@ use super::coordinates::HttpHeadersCoordinate;
use super::entity::validate_entity_arg;
use super::http::headers;
use super::http::method;
use super::http::validate_body;
use super::resolvable_key_fields;
use super::selection::validate_body_selection;
use super::selection::validate_selection;
use super::source_name::validate_source_name_arg;
use super::source_name::SourceName;
use super::Code;
use super::Message;
use crate::sources::connect::spec::schema::CONNECT_BODY_ARGUMENT_NAME;
use crate::sources::connect::spec::schema::CONNECT_SOURCE_ARGUMENT_NAME;
use crate::sources::connect::spec::schema::HTTP_ARGUMENT_NAME;
use crate::sources::connect::validation::graphql::SchemaInfo;
Expand Down Expand Up @@ -250,15 +249,8 @@ fn validate_field(
}
};

if let Some((_, body)) = http_arg
.iter()
.find(|(name, _)| name == &CONNECT_BODY_ARGUMENT_NAME)
{
if let Err(err) =
validate_body_selection(connect_directive, object, field, schema, body)
{
errors.push(err);
}
if let Err(err) = validate_body(http_arg, connect_coordinate, schema) {
errors.push(err);
}

if let Some(source_name) = connect_directive
Expand Down
3 changes: 3 additions & 0 deletions apollo-federation/src/sources/connect/validation/http.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
mod body;
pub(super) mod headers;
pub(super) mod method;
pub(super) mod url;

pub(super) use body::validate_body;
156 changes: 156 additions & 0 deletions apollo-federation/src/sources/connect/validation/http/body.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
use std::ops::Range;

use apollo_compiler::ast::Value;
use apollo_compiler::parser::LineColumn;
use apollo_compiler::Name;
use apollo_compiler::Node;

use crate::sources::connect::json_selection::KnownVariable;
use crate::sources::connect::json_selection::PathList;
use crate::sources::connect::json_selection::Ranged;
use crate::sources::connect::json_selection::WithRange;
use crate::sources::connect::spec::schema::CONNECT_BODY_ARGUMENT_NAME;
use crate::sources::connect::validation::coordinates::BodyCoordinate;
use crate::sources::connect::validation::coordinates::ConnectDirectiveCoordinate;
use crate::sources::connect::validation::graphql::GraphQLString;
use crate::sources::connect::validation::graphql::SchemaInfo;
use crate::sources::connect::validation::selection::get_json_selection;
use crate::sources::connect::validation::Code;
use crate::sources::connect::validation::Message;
use crate::sources::connect::JSONSelection;
use crate::sources::connect::PathSelection;

pub(crate) fn validate_body(
http_arg: &[(Name, Node<Value>)],
connect_coordinate: ConnectDirectiveCoordinate,
schema: &SchemaInfo,
) -> Result<(), Message> {
let Some(body) = http_arg
.iter()
.find_map(|(name, value)| (name == &CONNECT_BODY_ARGUMENT_NAME).then_some(value))
else {
return Ok(());
};
let coordinate = BodyCoordinate::from(connect_coordinate);
let (value, json_selection) = get_json_selection(coordinate, &schema.sources, body)?;
let arg = Arg {
value,
coordinate,
schema,
};

match json_selection {
JSONSelection::Named(_) => Ok(()), // TODO: validate_sub_selection(sub_selection, arg),
JSONSelection::Path(path_selection) => validate_path_selection(path_selection, arg),
}
}

fn validate_path_selection(path_selection: PathSelection, arg: Arg) -> Result<(), Message> {
let PathSelection { path } = path_selection;
match path.as_ref() {
PathList::Var(variable, trailing) => {
match variable.as_ref() {
KnownVariable::This => {
// TODO: Verify that the name & shape matches arg.coordinate.field_coordinate.object
Ok(())
}
KnownVariable::Args => validate_args_path(trailing, arg)
.map_err(|err| err.with_fallback_locations(arg.get_selection_location(&path))),
KnownVariable::Config => Ok(()), // We have no way of knowing is this is valid, yet
KnownVariable::Dollar => {
// TODO: Validate that this is followed only by a method
Ok(())
}
KnownVariable::AtSign => {
// TODO: This is probably just not allowed?
Ok(())
}
}
}
PathList::Key(_, _) => {
// TODO: Make sure this is aliased & built from data we have
Ok(())
}
PathList::Expr(_, _) => {
Ok(()) // We don't know what shape to expect, so any is fine
}
PathList::Method(_, _, _) => {
// TODO: This is a parse error, but return an error here just in case
Ok(())
}
PathList::Selection(_) => {
// TODO: This is a parse error, but return an error here just in case
Ok(())
}
PathList::Empty => {
// TODO: This is a parse error, but return an error here just in case
Ok(())
}
}
}

// Validate a reference to `$args`
fn validate_args_path(path: &WithRange<PathList>, arg: Arg) -> Result<(), Message> {
match path.as_ref() {
PathList::Var(var_type, _) => {
// This is probably caught by the parser, but we can't type-safely guarantee that yet
Err(Message {
code: Code::InvalidJsonSelection,
message: format!(
"Can't reference a path within another path. `$args.{var_type}` is invalid.",
var_type = var_type.as_str()
),
locations: arg.get_selection_location(var_type).collect(),
})
}
PathList::Key(_, _) => {
// TODO: Make sure that the path matches an argument, then validate the shape of that path
Ok(())
}
PathList::Expr(_, _) => Err(Message {
code: Code::InvalidJsonSelection,
message: "Can't use a literal expression after `$args`.".to_string(),
locations: arg.get_selection_location(path).collect(),
}),
PathList::Method(_, _, _) => {
// TODO: Validate that the method can be called directly on `$args`
Ok(())
}
PathList::Selection(_) => {
// TODO: Validate that the `SubSelection` is valid for `$args`
Ok(())
}
PathList::Empty => {
// They're selecting the entirety of `$args`, this is okay as long as there are any args!
if arg.coordinate.field_coordinate.field.arguments.is_empty() {
Err(Message {
code: Code::InvalidJsonSelection,
message: "Can't use `$args` when there are no arguments.".to_string(),
locations: vec![],
})
} else {
Ok(())
}
}
}
}

/// The `@connect(http.body:)` argument.
#[derive(Clone, Copy)]
struct Arg<'schema> {
value: GraphQLString<'schema>,
coordinate: BodyCoordinate<'schema>,
schema: &'schema SchemaInfo<'schema>,
}

impl Arg<'_> {
fn get_selection_location<T>(
&self,
selection: &impl Ranged<T>,
) -> impl Iterator<Item = Range<LineColumn>> {
selection
.range()
.and_then(|range| self.value.line_col_for_subslice(range, self.schema))
.into_iter()
}
}
19 changes: 18 additions & 1 deletion apollo-federation/src/sources/connect/validation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,23 @@ pub struct Message {
pub locations: Vec<Range<LineColumn>>,
}

impl Message {
/// If there was no location for this message yet, add some. Otherwise, do nothing.
pub(crate) fn with_fallback_locations(
self,
fallback_locations: impl Iterator<Item = Range<LineColumn>>,
) -> Self {
if self.locations.is_empty() {
Self {
locations: fallback_locations.collect(),
..self
}
} else {
self
}
}
}

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum Code {
/// A problem with GraphQL syntax or semantics was found. These will usually be caught before
Expand Down Expand Up @@ -574,7 +591,7 @@ mod test_validate_source {
#[test]
fn validation_tests() {
insta::with_settings!({prepend_module_to_snapshot => false}, {
glob!("test_data", "*.graphql", |path| {
glob!("test_data", "**/*.graphql", |path| {
let schema = read_to_string(path).unwrap();
let errors = validate(&schema, path.to_str().unwrap());
assert_snapshot!(format!("{:#?}", errors));
Expand Down
Loading

0 comments on commit 619b794

Please sign in to comment.