diff --git a/Cargo.lock b/Cargo.lock index 913b9ece62..a47274c453 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -189,6 +189,7 @@ dependencies = [ "insta", "itertools 0.13.0", "lazy_static", + "line-col", "multimap 0.10.0", "nom", "nom_locate", @@ -3984,6 +3985,12 @@ dependencies = [ "vcpkg", ] +[[package]] +name = "line-col" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9e69cdf6b85b5c8dce514f694089a2cf8b1a702f6cd28607bcb3cf296c9778db" + [[package]] name = "linked-hash-map" version = "0.5.6" diff --git a/apollo-federation/Cargo.toml b/apollo-federation/Cargo.toml index 32e49485fc..f02f1971e4 100644 --- a/apollo-federation/Cargo.toml +++ b/apollo-federation/Cargo.toml @@ -26,6 +26,7 @@ http.workspace = true indexmap = { version = "2.2.6", features = ["serde"] } itertools = "0.13.0" lazy_static = "1.4.0" +line-col = "0.2.1" multimap = "0.10.0" nom = "7.1.3" petgraph = { version = "0.6.4", features = ["serde-1"] } diff --git a/apollo-federation/src/sources/connect/validation/entity.rs b/apollo-federation/src/sources/connect/validation/entity.rs index 625aae21b8..a3823956db 100644 --- a/apollo-federation/src/sources/connect/validation/entity.rs +++ b/apollo-federation/src/sources/connect/validation/entity.rs @@ -4,7 +4,6 @@ use apollo_compiler::ast::InputValueDefinition; use apollo_compiler::ast::Value; use apollo_compiler::executable::FieldSet; use apollo_compiler::executable::Selection; -use apollo_compiler::parser::SourceMap; use apollo_compiler::schema::Component; use apollo_compiler::schema::Directive; use apollo_compiler::schema::ExtendedType; @@ -12,7 +11,6 @@ use apollo_compiler::schema::InputObjectType; use apollo_compiler::schema::ObjectType; use apollo_compiler::Name; use apollo_compiler::Node; -use apollo_compiler::Schema; use super::coordinates::connect_directive_entity_argument_coordinate; use super::coordinates::field_with_connect_directive_entity_true_coordinate; @@ -23,13 +21,13 @@ use super::Message; use crate::sources::connect::expand::visitors::FieldVisitor; use crate::sources::connect::expand::visitors::GroupVisitor; use crate::sources::connect::spec::schema::CONNECT_ENTITY_ARGUMENT_NAME; +use crate::sources::connect::validation::graphql::SchemaInfo; pub(super) fn validate_entity_arg( field: &Component, connect_directive: &Node, object: &Node, - schema: &Schema, - source_map: &SourceMap, + schema: &SchemaInfo, category: ObjectCategory, ) -> Vec { let mut messages = vec![]; @@ -52,7 +50,7 @@ pub(super) fn validate_entity_arg( "{coordinate} is invalid. Entity resolvers can only be declared on root `Query` fields.", coordinate = connect_directive_entity_argument_coordinate(connect_directive_name, entity_arg_value.as_ref(), object, &field.name) ), - locations: entity_arg.line_column_range(source_map) + locations: entity_arg.line_column_range(&schema.sources) .into_iter() .collect(), }) @@ -73,7 +71,7 @@ pub(super) fn validate_entity_arg( ) ), locations: entity_arg - .line_column_range(source_map) + .line_column_range(&schema.sources) .into_iter() .collect(), }) @@ -95,7 +93,7 @@ pub(super) fn validate_entity_arg( ), ), locations: entity_arg - .line_column_range(source_map) + .line_column_range(&schema.sources) .into_iter() .collect(), }); @@ -104,11 +102,9 @@ pub(super) fn validate_entity_arg( if let Some(message) = (ArgumentVisitor { schema, - connect_directive_name, entity_arg, entity_arg_value, object, - source_map, field: &field.name, key_fields, }) @@ -160,12 +156,10 @@ struct Field<'schema> { /// Since input types may contain fields with subtypes, and the fields of those subtypes can be /// part of composite keys, this potentially requires visiting a tree. struct ArgumentVisitor<'schema> { - schema: &'schema Schema, - connect_directive_name: &'schema Name, + schema: &'schema SchemaInfo<'schema>, entity_arg: &'schema Node, entity_arg_value: &'schema Node, object: &'schema Node, - source_map: &'schema SourceMap, field: &'schema Name, key_fields: Vec
, } @@ -229,7 +223,7 @@ impl<'schema> FieldVisitor> for ArgumentVisitor<'schema> { message: format!( "{coordinate} has invalid arguments. Mismatched type on field `{field_name}` - expected `{entity_type}` but found `{input_type}`.", coordinate = field_with_connect_directive_entity_true_coordinate( - self.connect_directive_name, + self.schema.connect_directive_name, self.entity_arg_value.as_ref(), self.object, self.field, @@ -239,9 +233,9 @@ impl<'schema> FieldVisitor> for ArgumentVisitor<'schema> { entity_type = field.entity_type.name(), ), locations: field.node - .line_column_range(self.source_map) + .line_column_range(&self.schema.sources) .into_iter() - .chain(self.entity_arg.line_column_range(self.source_map)) + .chain(self.entity_arg.line_column_range(&self.schema.sources)) .collect(), }) } @@ -292,7 +286,7 @@ impl<'schema> ArgumentVisitor<'schema> { message: format!( "{coordinate} has invalid arguments. Argument `{arg_name}` does not have a matching field `{arg_name}` on type `{entity_type}`.", coordinate = field_with_connect_directive_entity_true_coordinate( - self.connect_directive_name, + self.schema.connect_directive_name, self.entity_arg_value.as_ref(), self.object, &field.name @@ -301,9 +295,9 @@ impl<'schema> ArgumentVisitor<'schema> { entity_type = entity_type.name, ), locations: arg - .line_column_range(self.source_map) + .line_column_range(&self.schema.sources) .into_iter() - .chain(self.entity_arg.line_column_range(self.source_map)) + .chain(self.entity_arg.line_column_range(&self.schema.sources)) .collect(), })) } @@ -364,7 +358,7 @@ impl<'schema> ArgumentVisitor<'schema> { message: format!( "{coordinate} has invalid arguments. Field `{name}` on `{input_type}` does not have a matching field `{name}` on `{entity_type}`.", coordinate = field_with_connect_directive_entity_true_coordinate( - self.connect_directive_name, + self.schema.connect_directive_name, self.entity_arg_value.as_ref(), self.object, self.field, @@ -373,9 +367,9 @@ impl<'schema> ArgumentVisitor<'schema> { entity_type = entity_object_type.name, ), locations: input_field - .line_column_range(self.source_map) + .line_column_range(&self.schema.sources) .into_iter() - .chain(self.entity_arg.line_column_range(self.source_map)) + .chain(self.entity_arg.line_column_range(&self.schema.sources)) .collect(), })) } @@ -419,16 +413,16 @@ impl<'schema> ArgumentVisitor<'schema> { None => format!("Argument `{name}`"), }, coordinate = field_with_connect_directive_entity_true_coordinate( - self.connect_directive_name, + self.schema.connect_directive_name, self.entity_arg_value.as_ref(), self.object, self.field, ), ), locations: node - .line_column_range(self.source_map) + .line_column_range(&self.schema.sources) .into_iter() - .chain(self.entity_arg.line_column_range(self.source_map)) + .chain(self.entity_arg.line_column_range(&self.schema.sources)) .collect(), }) } else { diff --git a/apollo-federation/src/sources/connect/validation/extended_type.rs b/apollo-federation/src/sources/connect/validation/extended_type.rs index e04c8e32d2..c3d1e385ac 100644 --- a/apollo-federation/src/sources/connect/validation/extended_type.rs +++ b/apollo-federation/src/sources/connect/validation/extended_type.rs @@ -1,11 +1,6 @@ -use std::sync::Arc; - use apollo_compiler::ast::FieldDefinition; -use apollo_compiler::collections::IndexMap; use apollo_compiler::collections::IndexSet; use apollo_compiler::executable::Selection; -use apollo_compiler::parser::FileId; -use apollo_compiler::parser::SourceFile; use apollo_compiler::parser::SourceMap; use apollo_compiler::parser::SourceSpan; use apollo_compiler::schema::Component; @@ -13,7 +8,6 @@ use apollo_compiler::schema::ExtendedType; use apollo_compiler::schema::ObjectType; use apollo_compiler::Name; use apollo_compiler::Node; -use apollo_compiler::Schema; use itertools::Itertools; use super::coordinates::ConnectDirectiveCoordinate; @@ -33,33 +27,26 @@ 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; pub(super) fn validate_extended_type( extended_type: &ExtendedType, - schema: &Schema, - connect_directive_name: &Name, - source_directive_name: &Name, + schema: &SchemaInfo, all_source_names: &[SourceName], - source_map: &Arc>>, seen_fields: &mut IndexSet<(Name, Name)>, ) -> Vec { match extended_type { - ExtendedType::Object(object) => validate_object_fields( - object, - schema, - connect_directive_name, - source_directive_name, - all_source_names, - seen_fields, - ), + ExtendedType::Object(object) => { + validate_object_fields(object, schema, all_source_names, seen_fields) + } ExtendedType::Union(union_type) => vec![validate_abstract_type( SourceSpan::recompose(union_type.location(), union_type.name.location()), - source_map, + &schema.sources, "union", )], ExtendedType::Interface(interface) => vec![validate_abstract_type( SourceSpan::recompose(interface.location(), interface.name.location()), - source_map, + &schema.sources, "interface", )], _ => Vec::new(), @@ -77,9 +64,7 @@ pub enum ObjectCategory { /// are resolvable by some combination of `@connect` directives. fn validate_object_fields( object: &Node, - schema: &Schema, - connect_directive_name: &Name, - source_directive_name: &Name, + schema: &SchemaInfo, source_names: &[SourceName], seen_fields: &mut IndexSet<(Name, Name)>, ) -> Vec { @@ -122,7 +107,8 @@ fn validate_object_fields( return vec![Message { code: Code::SubscriptionInConnectors, message: format!( - "A subscription root type is not supported when using `@{connect_directive_name}`." + "A subscription root type is not supported when using `@{connect_directive_name}`.", + connect_directive_name = schema.connect_directive_name, ), locations: object.line_column_range(source_map).into_iter().collect(), }]; @@ -154,8 +140,6 @@ fn validate_object_fields( object_category, source_names, object, - connect_directive_name, - source_directive_name, schema, seen_fields, ) @@ -163,20 +147,17 @@ fn validate_object_fields( .collect() } -#[allow(clippy::too_many_arguments)] fn validate_field( field: &Component, category: ObjectCategory, source_names: &[SourceName], object: &Node, - connect_directive_name: &Name, - source_directive_name: &Name, - schema: &Schema, + schema: &SchemaInfo, seen_fields: &mut IndexSet<(Name, Name)>, ) -> Vec { let field_coordinate = FieldCoordinate { object, field }; let connect_coordinate = ConnectDirectiveCoordinate { - connect_directive_name, + connect_directive_name: schema.connect_directive_name, field_coordinate, }; let source_map = &schema.sources; @@ -184,7 +165,7 @@ fn validate_field( let connect_directives = field .directives .iter() - .filter(|directive| directive.name == *connect_directive_name) + .filter(|directive| directive.name == *schema.connect_directive_name) .collect_vec(); if connect_directives.is_empty() { @@ -194,14 +175,14 @@ fn validate_field( field, object, source_map, - connect_directive_name, + schema.connect_directive_name, )), ObjectCategory::Mutation => errors.push(get_missing_connect_directive_message( Code::MutationFieldMissingConnect, field, object, source_map, - connect_directive_name, + schema.connect_directive_name, )), _ => (), } @@ -240,7 +221,6 @@ fn validate_field( connect_directive, object, schema, - source_map, category, )); @@ -294,10 +274,8 @@ fn validate_field( &field.name, &object.name, source_name, - source_map, source_names, - source_directive_name, - &connect_directive.name, + schema, )); if let Some((template, coordinate)) = url_template { @@ -320,7 +298,8 @@ fn validate_field( code: Code::RelativeConnectUrlWithoutSource, message: format!( "{coordinate} specifies the relative URL {raw_value}, but no `{CONNECT_SOURCE_ARGUMENT_NAME}` is defined. Either use an absolute URL including scheme (e.g. https://), or add a `@{source_directive_name}`.", - raw_value = coordinate.node + raw_value = coordinate.node, + source_directive_name = schema.source_directive_name, ), locations: coordinate.node.line_column_range(source_map).into_iter().collect() }) @@ -332,7 +311,7 @@ fn validate_field( http_arg, source_map, HttpHeadersCoordinate::Connect { - directive_name: connect_directive_name, + directive_name: schema.connect_directive_name, object: &object.name, field: &field.name, }, diff --git a/apollo-federation/src/sources/connect/validation/graphql.rs b/apollo-federation/src/sources/connect/validation/graphql.rs new file mode 100644 index 0000000000..b5b202c6dc --- /dev/null +++ b/apollo-federation/src/sources/connect/validation/graphql.rs @@ -0,0 +1,77 @@ +//! Helper structs & functions for dealing with GraphQL schemas +use std::ops::Deref; + +use apollo_compiler::Name; +use apollo_compiler::Schema; +use line_col::LineColLookup; + +mod strings; + +pub(super) use strings::GraphQLString; + +pub(super) struct SchemaInfo<'schema> { + pub(crate) schema: &'schema Schema, + len: usize, + lookup: LineColLookup<'schema>, + pub(crate) connect_directive_name: &'schema Name, + pub(crate) source_directive_name: &'schema Name, +} + +impl<'schema> SchemaInfo<'schema> { + pub(crate) fn new( + schema: &'schema Schema, + src: &'schema str, + connect_directive_name: &'schema Name, + source_directive_name: &'schema Name, + ) -> Self { + Self { + schema, + len: src.len(), + lookup: LineColLookup::new(src), + connect_directive_name, + source_directive_name, + } + } + + /// Get the 1-based line and column values for an offset into this schema. + /// + /// # Returns + /// The line and column, or `None` if the offset is not within the schema. + pub(crate) fn line_col(&self, offset: usize) -> Option<(usize, usize)> { + if offset > self.len { + None + } else { + Some(self.lookup.get(offset)) + } + } +} + +impl Deref for SchemaInfo<'_> { + type Target = Schema; + + fn deref(&self) -> &Self::Target { + self.schema + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn line_col_lookup() { + let src = r#" + type Query { + foo: String + } + "#; + let schema = Schema::parse(src, "testSchema").unwrap(); + + let name = "unused".try_into().unwrap(); + let schema_info = SchemaInfo::new(&schema, src, &name, &name); + + assert_eq!(schema_info.line_col(0), Some((1, 1))); + assert_eq!(schema_info.line_col(4), Some((2, 4))); + assert_eq!(schema_info.line_col(200), None); + } +} diff --git a/apollo-federation/src/sources/connect/validation/graphql/strings.rs b/apollo-federation/src/sources/connect/validation/graphql/strings.rs new file mode 100644 index 0000000000..863bfe5489 --- /dev/null +++ b/apollo-federation/src/sources/connect/validation/graphql/strings.rs @@ -0,0 +1,164 @@ +//! Helpers for dealing with GraphQL literal strings and getting locations within them. +//! +//! Specifically, working around these issues with `apollo-compiler` which make determining the +//! line/column of locations _within_ a string impossible: +//! +//! 1. Quotes are included in locations (i.e., `line_column_range`) but not the values from `Node`. +//! 2. String values in `Node` are stripped of insignificant whitespace + +use std::ops::Range; + +use apollo_compiler::ast::Value; +use apollo_compiler::parser::LineColumn; +use apollo_compiler::parser::SourceMap; +use apollo_compiler::Node; + +use crate::sources::connect::validation::graphql::SchemaInfo; + +#[derive(Clone, Copy)] +pub(crate) struct GraphQLString<'schema> { + /// The GraphQL String literal without quotes, but with all whitespace intact + raw_string: &'schema str, + /// Where `raw_string` _starts_ in the source text + offset: usize, +} + +impl<'schema> GraphQLString<'schema> { + /// Get the raw string value of this GraphQL string literal + /// + /// Returns `None` if the value was not a string literal or if location data is messed up + pub fn new(value: &'schema Node, sources: &'schema SourceMap) -> Result { + let value_without_quotes = value.as_str().ok_or(())?; + let source_span = value.location().ok_or(())?; + let file = sources.get(&source_span.file_id()).ok_or(())?; + let source_text = file.source_text(); + let start_of_quotes = source_span.offset(); + let end_of_quotes = source_span.end_offset(); + let value_with_quotes = source_text.get(start_of_quotes..end_of_quotes).ok_or(())?; + + // On each line, the whitespace gets messed up for multi-line strings + // So we find the first and last lines of the parsed value (no whitespace) within + // the raw value (with whitespace) to get our raw string. + let first_line_of_value = value_without_quotes.lines().next().ok_or(())?; + let start_of_value = value_with_quotes.find(first_line_of_value).ok_or(())?; + let last_line_of_value = value_without_quotes.lines().last().ok_or(())?; + let end_of_value = + value_with_quotes.rfind(last_line_of_value).ok_or(())? + last_line_of_value.len(); + let raw_string = value_with_quotes + .get(start_of_value..end_of_value) + .ok_or(())?; + + Ok(Self { + raw_string, + offset: start_of_quotes + start_of_value, + }) + } + + pub fn as_str(&self) -> &str { + self.raw_string + } + + pub fn line_col_for_subslice( + &self, + substring_location: Range, + schema_info: &SchemaInfo, + ) -> Option> { + let start_offset = self.offset + substring_location.start; + let end_offset = self.offset + substring_location.end; + + let (line, column) = schema_info.line_col(start_offset)?; + let start = LineColumn { line, column }; + let (line, column) = schema_info.line_col(end_offset)?; + let end = LineColumn { line, column }; + + Some(start..end) + } +} + +#[cfg(test)] +mod tests { + use apollo_compiler::ast::Value; + use apollo_compiler::parser::LineColumn; + use apollo_compiler::schema::ExtendedType; + use apollo_compiler::Node; + use apollo_compiler::Schema; + use pretty_assertions::assert_eq; + + use crate::sources::connect::validation::graphql::GraphQLString; + use crate::sources::connect::validation::graphql::SchemaInfo; + + const SCHEMA: &str = r#" + type Query { + field: String @connect( + http: { + GET: "https://example.com" + }, + selection: """ + something + somethingElse { + nested + } + """ + ) + } + "#; + + fn connect_argument<'schema>(schema: &'schema Schema, name: &str) -> &'schema Node { + let ExtendedType::Object(query) = schema.types.get("Query").unwrap() else { + panic!("Query type not found"); + }; + let field = query.fields.get("field").unwrap(); + let directive = field.directives.get("connect").unwrap(); + directive.argument_by_name(name).unwrap() + } + + #[test] + fn single_quoted_string() { + let schema = Schema::parse(SCHEMA, "test.graphql").unwrap(); + let http = connect_argument(&schema, "http").as_object().unwrap(); + let value = &http[0].1; + + let string = GraphQLString::new(value, &schema.sources).unwrap(); + assert_eq!(string.as_str(), "https://example.com"); + let name = "unused".try_into().unwrap(); + let schema_info = SchemaInfo::new(&schema, SCHEMA, &name, &name); + assert_eq!( + string.line_col_for_subslice(2..5, &schema_info), + Some( + LineColumn { + line: 5, + column: 25 + }..LineColumn { + line: 5, + column: 28 + } + ) + ); + } + + #[test] + fn multi_line_string() { + let schema = Schema::parse(SCHEMA, "test.graphql").unwrap(); + let value = connect_argument(&schema, "selection"); + + let string = GraphQLString::new(value, &schema.sources).unwrap(); + assert_eq!( + string.as_str(), + r#"something + somethingElse { + nested + }"# + ); + let name = "unused".try_into().unwrap(); + let schema_info = SchemaInfo::new(&schema, SCHEMA, &name, &name); + assert_eq!( + string.line_col_for_subslice(8..16, &schema_info), + Some( + LineColumn { + line: 8, + column: 21 + }..LineColumn { line: 9, column: 7 } + ) + ); + } +} diff --git a/apollo-federation/src/sources/connect/validation/http/method.rs b/apollo-federation/src/sources/connect/validation/http/method.rs index 27c8b1a546..808d8df655 100644 --- a/apollo-federation/src/sources/connect/validation/http/method.rs +++ b/apollo-federation/src/sources/connect/validation/http/method.rs @@ -1,7 +1,6 @@ use apollo_compiler::ast::Value; use apollo_compiler::Name; use apollo_compiler::Node; -use apollo_compiler::Schema; use super::url::validate_template; use crate::sources::connect::spec::schema::CONNECT_HTTP_ARGUMENT_DELETE_METHOD_NAME; @@ -11,6 +10,7 @@ use crate::sources::connect::spec::schema::CONNECT_HTTP_ARGUMENT_POST_METHOD_NAM use crate::sources::connect::spec::schema::CONNECT_HTTP_ARGUMENT_PUT_METHOD_NAME; use crate::sources::connect::validation::coordinates::ConnectHTTPCoordinate; use crate::sources::connect::validation::coordinates::HttpMethodCoordinate; +use crate::sources::connect::validation::graphql::SchemaInfo; use crate::sources::connect::validation::Code; use crate::sources::connect::validation::Message; use crate::sources::connect::URLTemplate; @@ -19,7 +19,7 @@ pub(crate) fn validate<'schema>( http_arg: &'schema [(Name, Node)], coordinate: ConnectHTTPCoordinate<'schema>, http_arg_node: &Node, - schema: &Schema, + schema: &SchemaInfo, ) -> Result<(URLTemplate, HttpMethodCoordinate<'schema>), Vec> { let source_map = &schema.sources; let mut methods = http_arg diff --git a/apollo-federation/src/sources/connect/validation/http/url.rs b/apollo-federation/src/sources/connect/validation/http/url.rs index a7f1ba8cd7..68e9683d29 100644 --- a/apollo-federation/src/sources/connect/validation/http/url.rs +++ b/apollo-federation/src/sources/connect/validation/http/url.rs @@ -1,12 +1,9 @@ use std::fmt::Display; -use std::ops::Range; use std::str::FromStr; use apollo_compiler::ast::FieldDefinition; use apollo_compiler::ast::Type; use apollo_compiler::ast::Value; -use apollo_compiler::parser::LineColumn; -use apollo_compiler::parser::SourceMap; use apollo_compiler::schema::Component; use apollo_compiler::schema::ExtendedType; use apollo_compiler::Node; @@ -16,7 +13,8 @@ use url::Url; use crate::sources::connect::url_template; use crate::sources::connect::url_template::VariableType; use crate::sources::connect::validation::coordinates::HttpMethodCoordinate; -use crate::sources::connect::validation::require_value_is_str; +use crate::sources::connect::validation::graphql::GraphQLString; +use crate::sources::connect::validation::graphql::SchemaInfo; use crate::sources::connect::validation::Code; use crate::sources::connect::validation::Message; use crate::sources::connect::URLTemplate; @@ -24,24 +22,16 @@ use crate::sources::connect::Variable; pub(crate) fn validate_template( coordinate: HttpMethodCoordinate, - schema: &Schema, + schema: &SchemaInfo, ) -> Result> { - let (template, str_value) = match parse_template(coordinate, &schema.sources) { + let (template, str_value) = match parse_template(coordinate, schema) { Ok(tuple) => tuple, Err(message) => return Err(vec![message]), }; let mut messages = Vec::new(); if let Some(base) = template.base.as_ref() { - messages.extend( - validate_base_url( - base, - coordinate, - coordinate.node, - str_value, - &schema.sources, - ) - .err(), - ); + messages + .extend(validate_base_url(base, coordinate, coordinate.node, str_value, schema).err()); } for variable in template.path_variables() { @@ -54,11 +44,10 @@ pub(crate) fn validate_template( "Variables in path parameters should be non-null, but {coordinate} contains `{{{variable}}}` which is nullable. \ If a null value is provided at runtime, the request will fail.", ), - locations: select_substring_location( - coordinate.node.line_column_range(&schema.sources), - str_value, - Some(variable.location.clone()), - ), + locations: str_value.line_col_for_subslice( + variable.location.clone(), + schema, + ).into_iter().collect(), }); } Ok(_) => {} // Type is non-null, or unknowable @@ -80,21 +69,27 @@ pub(crate) fn validate_template( fn parse_template<'schema>( coordinate: HttpMethodCoordinate<'schema>, - sources: &SourceMap, -) -> Result<(URLTemplate, &'schema str), Message> { - let str_value = require_value_is_str(coordinate.node, coordinate, sources)?; - let template = - URLTemplate::from_str(str_value).map_err(|url_template::Error { message, location }| { - Message { - code: Code::InvalidUrl, - message: format!("{coordinate} must be a valid URL template. {message}"), - locations: select_substring_location( - coordinate.node.line_column_range(sources), - str_value, - location, - ), - } - })?; + schema: &'schema SchemaInfo, +) -> Result<(URLTemplate, GraphQLString<'schema>), Message> { + let str_value = GraphQLString::new(coordinate.node, &schema.sources).map_err(|_| Message { + code: Code::GraphQLError, + message: format!("The value for {coordinate} must be a string."), + locations: coordinate + .node + .line_column_range(&schema.sources) + .into_iter() + .collect(), + })?; + let template = URLTemplate::from_str(str_value.as_str()).map_err( + |url_template::Error { message, location }| Message { + code: Code::InvalidUrl, + message: format!("{coordinate} must be a valid URL template. {message}"), + locations: location + .and_then(|location| str_value.line_col_for_subslice(location, schema)) + .into_iter() + .collect(), + }, + )?; Ok((template, str_value)) } @@ -102,57 +97,32 @@ pub(crate) fn validate_base_url( url: &Url, coordinate: impl Display, value: &Node, - str_value: &str, - sources: &SourceMap, + str_value: GraphQLString, + schema: &SchemaInfo, ) -> Result<(), Message> { let scheme = url.scheme(); if scheme != "http" && scheme != "https" { - let scheme_location = Some(0..scheme.len()); + let scheme_location = 0..scheme.len(); Err(Message { code: Code::InvalidUrlScheme, message: format!( "The value {value} for {coordinate} must be http or https, got {scheme}.", ), - locations: select_substring_location( - value.line_column_range(sources), - str_value, - scheme_location, - ), + locations: str_value + .line_col_for_subslice(scheme_location, schema) + .into_iter() + .collect(), }) } else { Ok(()) } } -fn select_substring_location( - line_column: Option>, - full_url: &str, - substring_location: Option>, -) -> Vec> { - line_column - .map(|mut template_location| { - // The default location includes the parameter name, we just want the value, - // so we need to calculate that. - template_location.end.column -= 1; // Get rid of the end quote - template_location.start.column = template_location.end.column - full_url.len(); - - if let Some(location) = substring_location { - // We can point to a substring of the URL template! do it. - template_location.start.column += location.start; - template_location.end.column = - template_location.start.column + location.end - location.start; - } - template_location - }) - .into_iter() - .collect() -} - fn validate_variable<'schema>( variable: &'schema Variable, - url_value: &str, + url_value: GraphQLString, coordinate: HttpMethodCoordinate<'schema>, - schema: &'schema Schema, + schema: &'schema SchemaInfo, ) -> Result, Message> { let field_coordinate = coordinate.connect.field_coordinate; let field = field_coordinate.field; @@ -171,11 +141,10 @@ fn validate_variable<'schema>( message: format!( "{coordinate} contains `{{{variable}}}`, but {field_coordinate} does not have an argument named `{path_root}`.", ), - locations: select_substring_location( - coordinate.node.line_column_range(&schema.sources), - url_value, - Some(path_component_start..path_component_end), - ) + locations: url_value.line_col_for_subslice( + path_component_start..path_component_end, + schema, + ).into_iter().collect() } }).map(|arg| arg.ty.as_ref().clone())? } @@ -186,11 +155,10 @@ fn validate_variable<'schema>( "{coordinate} contains `{{{variable}}}`, but {object} does not have a field named `{path_root}`.", object = field_coordinate.object.name, ), - locations: select_substring_location( - coordinate.node.line_column_range(&schema.sources), - url_value, - Some(path_component_start..path_component_end), - ) + locations: url_value.line_col_for_subslice( + path_component_start..path_component_end, + schema, + ).into_iter().collect() }).map(|field| field.ty.clone())? } }; @@ -226,11 +194,10 @@ fn validate_variable<'schema>( message: format!( "{coordinate} contains `{{{variable}}}`, but `{variable_type}` does not have a field named `{nested_field_name}`.", ), - locations: select_substring_location( - coordinate.node.line_column_range(&schema.sources), - url_value, - Some(path_component_start..path_component_end), - ) + locations: url_value.line_col_for_subslice( + path_component_start..path_component_end, + schema, + ).into_iter().collect(), }) })?.clone(); if parent_is_nullable && variable_type.is_non_null() { diff --git a/apollo-federation/src/sources/connect/validation/mod.rs b/apollo-federation/src/sources/connect/validation/mod.rs index 53abf5e532..9864439e53 100644 --- a/apollo-federation/src/sources/connect/validation/mod.rs +++ b/apollo-federation/src/sources/connect/validation/mod.rs @@ -17,6 +17,7 @@ mod coordinates; mod entity; mod extended_type; +mod graphql; mod http; mod selection; mod source_name; @@ -59,6 +60,8 @@ use crate::sources::connect::spec::schema::SOURCE_DIRECTIVE_NAME_IN_SPEC; use crate::sources::connect::spec::schema::SOURCE_NAME_ARGUMENT_NAME; use crate::sources::connect::validation::coordinates::BaseUrlCoordinate; use crate::sources::connect::validation::coordinates::HttpHeadersCoordinate; +use crate::sources::connect::validation::graphql::GraphQLString; +use crate::sources::connect::validation::graphql::SchemaInfo; use crate::sources::connect::validation::http::headers; use crate::sources::connect::ConnectSpecDefinition; use crate::subgraph::spec::CONTEXT_DIRECTIVE_NAME; @@ -70,7 +73,11 @@ use crate::subgraph::spec::INTF_OBJECT_DIRECTIVE_NAME; /// /// This function attempts to collect as many validation errors as possible, so it does not bail /// out as soon as it encounters one. -pub fn validate(schema: Schema) -> Vec { +pub fn validate(source_text: &str, file_name: &str) -> Vec { + // TODO: Use parse_and_validate (adding in directives as needed) + // TODO: Handle schema errors rather than relying on JavaScript to catch it later + let schema = Schema::parse(source_text, file_name) + .unwrap_or_else(|schema_with_errors| schema_with_errors.partial); let connect_identity = ConnectSpecDefinition::identity(); let Some((link, link_directive)) = Link::for_identity(&schema, &connect_identity) else { return Vec::new(); // There are no connectors-related directives to validate @@ -83,15 +90,21 @@ pub fn validate(schema: Schema) -> Vec { let mut messages = check_conflicting_directives(&schema); - let source_map = &schema.sources; let source_directive_name = ConnectSpecDefinition::source_directive_name(&link); let connect_directive_name = ConnectSpecDefinition::connect_directive_name(&link); + let schema_info = SchemaInfo::new( + &schema, + source_text, + &connect_directive_name, + &source_directive_name, + ); + let source_directives: Vec = schema .schema_definition .directives .iter() .filter(|directive| directive.name == source_directive_name) - .map(|directive| validate_source(directive, source_map)) + .map(|directive| validate_source(directive, &schema_info)) .collect(); let mut valid_source_names = HashMap::new(); @@ -101,12 +114,17 @@ pub fn validate(schema: Schema) -> Vec { .collect_vec(); for directive in source_directives { messages.extend(directive.errors); - match directive.name.into_value_or_error(source_map) { + match directive.name.into_value_or_error(&schema_info.sources) { Err(error) => messages.push(error), Ok(name) => valid_source_names .entry(name) .or_insert_with(Vec::new) - .extend(directive.directive.node.line_column_range(source_map)), + .extend( + directive + .directive + .node + .line_column_range(&schema_info.sources), + ), } } for (name, locations) in valid_source_names { @@ -124,11 +142,8 @@ pub fn validate(schema: Schema) -> Vec { let connect_errors = schema.types.values().flat_map(|extended_type| { validate_extended_type( extended_type, - &schema, - &connect_directive_name, - &source_directive_name, + &schema_info, &all_source_names, - source_map, &mut seen_fields, ) }); @@ -136,10 +151,8 @@ pub fn validate(schema: Schema) -> Vec { if should_check_seen_fields(&messages) { messages.extend(check_seen_fields( - &schema, + &schema_info, &seen_fields, - source_map, - &connect_directive_name, &external_directive_name, )); } @@ -152,7 +165,7 @@ pub fn validate(schema: Schema) -> Vec { messages.push(Message { code: Code::NoSourceImport, message: format!("The `@{SOURCE_DIRECTIVE_NAME_IN_SPEC}` directive is not imported. Try adding `@{SOURCE_DIRECTIVE_NAME_IN_SPEC}` to `import` for `@{link_name}(url: \"{connect_identity}\")`", link_name=link_directive.name), - locations: link_directive.line_column_range(source_map) + locations: link_directive.line_column_range(&schema.sources) .into_iter() .collect(), }); @@ -179,10 +192,8 @@ fn should_check_seen_fields(messages: &[Message]) -> bool { /// Check that all fields defined in the schema are resolved by a connector. fn check_seen_fields( - schema: &Schema, + schema: &SchemaInfo, seen_fields: &IndexSet<(Name, Name)>, - source_map: &SourceMap, - connect_directive_name: &Name, external_directive_name: &Name, ) -> Vec { let all_fields: IndexSet<_> = schema @@ -235,8 +246,9 @@ fn check_seen_fields( code: Code::UnresolvedField, message: format!( "No connector resolves field `{parent_type}.{field_name}`. It must have a `@{connect_directive_name}` directive or appear in `@{connect_directive_name}(selection:)`.", + connect_directive_name = schema.connect_directive_name ), - locations: field_def.line_column_range(source_map).into_iter().collect(), + locations: field_def.line_column_range(&schema.sources).into_iter().collect(), } }).collect() } @@ -290,7 +302,7 @@ fn check_conflicting_directives(schema: &Schema) -> Vec { const DEFAULT_SOURCE_DIRECTIVE_NAME: &str = "connect__source"; const DEFAULT_CONNECT_DIRECTIVE_NAME: &str = "connect__connect"; -fn validate_source(directive: &Component, sources: &SourceMap) -> SourceDirective { +fn validate_source(directive: &Component, schema: &SchemaInfo) -> SourceDirective { let name = SourceName::from_directive(directive); let mut errors = Vec::new(); @@ -308,7 +320,7 @@ fn validate_source(directive: &Component, sources: &SourceMap) -> Sou BaseUrlCoordinate { source_directive_name: &directive.name, }, - sources, + schema, ) .err() { @@ -319,7 +331,7 @@ fn validate_source(directive: &Component, sources: &SourceMap) -> Sou errors.extend( headers::validate_arg( http_arg, - sources, + &schema.sources, HttpHeadersCoordinate::Source { directive_name: &directive.name, }, @@ -334,7 +346,10 @@ fn validate_source(directive: &Component, sources: &SourceMap) -> Sou "{coordinate} must have a `{HTTP_ARGUMENT_NAME}` argument.", coordinate = source_http_argument_coordinate(&directive.name), ), - locations: directive.line_column_range(sources).into_iter().collect(), + locations: directive + .line_column_range(&schema.sources) + .into_iter() + .collect(), }) } @@ -355,15 +370,25 @@ struct SourceDirective { fn parse_url( value: &Node, coordinate: Coordinate, - sources: &SourceMap, + schema: &SchemaInfo, ) -> Result<(), Message> { - let str_value = require_value_is_str(value, coordinate, sources)?; - let url = Url::parse(str_value).map_err(|inner| Message { + let str_value = GraphQLString::new(value, &schema.sources).map_err(|_| Message { + code: Code::GraphQLError, + message: format!("The value for {coordinate} must be a string."), + locations: value + .line_column_range(&schema.sources) + .into_iter() + .collect(), + })?; + let url = Url::parse(str_value.as_str()).map_err(|inner| Message { code: Code::InvalidUrl, message: format!("The value {value} for {coordinate} is not a valid URL: {inner}."), - locations: value.line_column_range(sources).into_iter().collect(), + locations: value + .line_column_range(&schema.sources) + .into_iter() + .collect(), })?; - http::url::validate_base_url(&url, coordinate, value, str_value, sources) + http::url::validate_base_url(&url, coordinate, value, str_value, schema) } fn require_value_is_str<'a, Coordinate: Display>( @@ -550,8 +575,7 @@ mod test_validate_source { insta::with_settings!({prepend_module_to_snapshot => false}, { glob!("test_data", "*.graphql", |path| { let schema = read_to_string(path).unwrap(); - let schema = Schema::parse(schema, path).unwrap(); - let errors = validate(schema); + let errors = validate(&schema, path.to_str().unwrap()); assert_snapshot!(format!("{:#?}", errors)); }); }); diff --git a/apollo-federation/src/sources/connect/validation/selection.rs b/apollo-federation/src/sources/connect/validation/selection.rs index 0f4f7d6115..17ba23fc92 100644 --- a/apollo-federation/src/sources/connect/validation/selection.rs +++ b/apollo-federation/src/sources/connect/validation/selection.rs @@ -24,6 +24,7 @@ use crate::sources::connect::expand::visitors::GroupVisitor; use crate::sources::connect::json_selection::NamedSelection; use crate::sources::connect::spec::schema::CONNECT_SELECTION_ARGUMENT_NAME; use crate::sources::connect::validation::coordinates::connect_directive_http_body_coordinate; +use crate::sources::connect::validation::graphql::SchemaInfo; use crate::sources::connect::JSONSelection; use crate::sources::connect::SubSelection; @@ -31,7 +32,7 @@ pub(super) fn validate_selection( field: &Component, connect_directive: &Node, parent_type: &Node, - schema: &Schema, + schema: &SchemaInfo, seen_fields: &mut IndexSet<(Name, Name)>, ) -> Option { let (selection_value, json_selection) = diff --git a/apollo-federation/src/sources/connect/validation/source_name.rs b/apollo-federation/src/sources/connect/validation/source_name.rs index e735d52040..9da9e86bdd 100644 --- a/apollo-federation/src/sources/connect/validation/source_name.rs +++ b/apollo-federation/src/sources/connect/validation/source_name.rs @@ -15,22 +15,21 @@ use super::Code; use super::DirectiveName; use super::Message; use crate::sources::connect::spec::schema::SOURCE_NAME_ARGUMENT_NAME; +use crate::sources::connect::validation::graphql::SchemaInfo; pub(super) fn validate_source_name_arg( field_name: &Name, object_name: &Name, source_name: &Node, - source_map: &SourceMap, source_names: &[SourceName], - source_directive_name: &Name, - connect_directive_name: &Name, + schema: &SchemaInfo, ) -> Vec { let mut messages = vec![]; if source_names.iter().all(|name| name != &source_name.value) { // TODO: Pick a suggestion that's not just the first defined source let qualified_directive = connect_directive_name_coordinate( - connect_directive_name, + schema.connect_directive_name, &source_name.value, object_name, field_name, @@ -41,7 +40,7 @@ pub(super) fn validate_source_name_arg( message: format!( "{qualified_directive} does not match any defined sources. Did you mean {first_source_name}?", ), - locations: source_name.line_column_range(source_map) + locations: source_name.line_column_range(&schema.sources) .into_iter() .collect(), }); @@ -50,9 +49,9 @@ pub(super) fn validate_source_name_arg( code: Code::NoSourcesDefined, message: format!( "{qualified_directive} specifies a source, but none are defined. Try adding {coordinate} to the schema.", - coordinate = source_name_value_coordinate(source_directive_name, &source_name.value), + coordinate = source_name_value_coordinate(schema.source_directive_name, &source_name.value), ), - locations: source_name.line_column_range(source_map) + locations: source_name.line_column_range(&schema.sources) .into_iter() .collect(), });