diff --git a/apollo-federation/src/sources/connect/expand/mod.rs b/apollo-federation/src/sources/connect/expand/mod.rs index ef998c6028..22e7d4aa36 100644 --- a/apollo-federation/src/sources/connect/expand/mod.rs +++ b/apollo-federation/src/sources/connect/expand/mod.rs @@ -522,7 +522,7 @@ mod helpers { to_schema, &self.directive_deny_list, ); - let (_, parsed) = JSONSelection::parse(&selection).map_err(|e| { + let parsed = JSONSelection::parse(&selection).map_err(|e| { FederationError::internal(format!( "could not parse fake selection for sibling field: {e}" )) diff --git a/apollo-federation/src/sources/connect/expand/visitors/mod.rs b/apollo-federation/src/sources/connect/expand/visitors/mod.rs index 2d2c46fa7d..9420d56456 100644 --- a/apollo-federation/src/sources/connect/expand/visitors/mod.rs +++ b/apollo-federation/src/sources/connect/expand/visitors/mod.rs @@ -275,8 +275,7 @@ mod tests { fn it_iterates_over_empty_path() { let mut visited = Vec::new(); let visitor = TestVisitor::new(&mut visited); - let (unmatched, selection) = JSONSelection::parse("").unwrap(); - assert!(unmatched.is_empty()); + let selection = JSONSelection::parse("").unwrap(); visitor .walk(selection.next_subselection().cloned().unwrap()) @@ -288,8 +287,7 @@ mod tests { fn it_iterates_over_simple_selection() { let mut visited = Vec::new(); let visitor = TestVisitor::new(&mut visited); - let (unmatched, selection) = JSONSelection::parse("a b c d").unwrap(); - assert!(unmatched.is_empty()); + let selection = JSONSelection::parse("a b c d").unwrap(); visitor .walk(selection.next_subselection().cloned().unwrap()) @@ -306,9 +304,7 @@ mod tests { fn it_iterates_over_aliased_selection() { let mut visited = Vec::new(); let visitor = TestVisitor::new(&mut visited); - let (unmatched, selection) = - JSONSelection::parse("a: one b: two c: three d: four").unwrap(); - assert!(unmatched.is_empty()); + let selection = JSONSelection::parse("a: one b: two c: three d: four").unwrap(); visitor .walk(selection.next_subselection().cloned().unwrap()) @@ -325,8 +321,7 @@ mod tests { fn it_iterates_over_nested_selection() { let mut visited = Vec::new(); let visitor = TestVisitor::new(&mut visited); - let (unmatched, selection) = JSONSelection::parse("a { b { c { d { e } } } } f").unwrap(); - assert!(unmatched.is_empty()); + let selection = JSONSelection::parse("a { b { c { d { e } } } } f").unwrap(); visitor .walk(selection.next_subselection().cloned().unwrap()) @@ -345,7 +340,7 @@ mod tests { fn it_iterates_over_paths() { let mut visited = Vec::new(); let visitor = TestVisitor::new(&mut visited); - let (unmatched, selection) = JSONSelection::parse( + let selection = JSONSelection::parse( "a $.b { c @@ -357,7 +352,6 @@ mod tests { j", ) .unwrap(); - assert!(unmatched.is_empty()); visitor .walk(selection.next_subselection().cloned().unwrap()) @@ -376,7 +370,7 @@ mod tests { fn it_iterates_over_complex_selection() { let mut visited = Vec::new(); let visitor = TestVisitor::new(&mut visited); - let (unmatched, selection) = JSONSelection::parse( + let selection = JSONSelection::parse( "id name username @@ -400,7 +394,6 @@ mod tests { }", ) .unwrap(); - assert!(unmatched.is_empty()); visitor .walk(selection.next_subselection().cloned().unwrap()) diff --git a/apollo-federation/src/sources/connect/json_selection/helpers.rs b/apollo-federation/src/sources/connect/json_selection/helpers.rs index bb03779d4c..831a0d5d94 100644 --- a/apollo-federation/src/sources/connect/json_selection/helpers.rs +++ b/apollo-federation/src/sources/connect/json_selection/helpers.rs @@ -13,10 +13,7 @@ use super::ParseResult; #[macro_export] macro_rules! selection { ($input:expr) => { - if let Ok((remainder, parsed)) = - $crate::sources::connect::json_selection::JSONSelection::parse($input) - { - assert_eq!(remainder, ""); + if let Ok(parsed) = $crate::sources::connect::json_selection::JSONSelection::parse($input) { parsed } else { panic!("invalid selection: {:?}", $input); diff --git a/apollo-federation/src/sources/connect/json_selection/location.rs b/apollo-federation/src/sources/connect/json_selection/location.rs index a8c16e1c25..8f359dcd49 100644 --- a/apollo-federation/src/sources/connect/json_selection/location.rs +++ b/apollo-federation/src/sources/connect/json_selection/location.rs @@ -315,15 +315,13 @@ mod tests { #[test] fn test_arrow_path_ranges() { - let (remainder, parsed) = - JSONSelection::parse(" __typename: @ -> echo ( \"Frog\" , ) ").unwrap(); - assert_eq!(remainder, ""); + let parsed = JSONSelection::parse(" __typename: @ -> echo ( \"Frog\" , ) ").unwrap(); assert_debug_snapshot!(parsed); } #[test] fn test_parse_with_range_snapshots() { - let (remainder, parsed) = JSONSelection::parse( + let parsed = JSONSelection::parse( r#" path: some.nested.path { isbn author { name }} alias: "not an identifier" { @@ -337,7 +335,6 @@ mod tests { "#, ) .unwrap(); - assert_eq!(remainder, ""); assert_snapshot!(format!("{:#?}", parsed)); } } diff --git a/apollo-federation/src/sources/connect/json_selection/parser.rs b/apollo-federation/src/sources/connect/json_selection/parser.rs index 3af4bd2292..4e8f1bf997 100644 --- a/apollo-federation/src/sources/connect/json_selection/parser.rs +++ b/apollo-federation/src/sources/connect/json_selection/parser.rs @@ -139,19 +139,24 @@ impl JSONSelection { // if we drastically change implementation details. That's why we use &str // as the input type and a custom JSONSelectionParseError type as the error // type, rather than using Span or nom::error::Error directly. - pub fn parse(input: &str) -> IResult<&str, Self, JSONSelectionParseError> { + pub fn parse(input: &str) -> Result { match JSONSelection::parse_span(new_span(input)) { Ok((remainder, selection)) => { - // To avoid exposing the implementation details of nom_locate's - // LocatedSpan, we report the remainder as a &str instead. - Ok((remainder.fragment(), selection)) + let fragment = remainder.fragment(); + if fragment.is_empty() { + Ok(selection) + } else { + Err(JSONSelectionParseError { + message: "Unexpected trailing characters".to_string(), + fragment: fragment.to_string(), + offset: remainder.location_offset(), + }) + } } Err(e) => match e { nom::Err::Error(e) | nom::Err::Failure(e) => { - // If a non-fatal nom::Err::Error bubbles all the way up - // here, then it becomes a fatal nom::Err::Failure. - Err(nom::Err::Failure(JSONSelectionParseError { + Err(JSONSelectionParseError { message: if let Some(message_str) = e.input.extra { message_str.to_string() } else { @@ -162,7 +167,7 @@ impl JSONSelection { }, fragment: e.input.fragment().to_string(), offset: e.input.location_offset(), - })) + }) } nom::Err::Incomplete(_) => unreachable!("nom::Err::Incomplete not expected here"), @@ -2808,12 +2813,7 @@ mod tests { #[test] fn test_ranged_locations() { fn check(input: &str, expected: JSONSelection) { - let (remainder, parsed) = JSONSelection::parse(input).unwrap(); - // We can't (and don't want to) use span_is_all_spaces_or_comments - // here because remainder is a &str not a Span, and - // JSONSelection::parse is responsible for consuming the entire - // string when possible (see all_consuming). - assert_eq!(remainder, ""); + let parsed = JSONSelection::parse(input).unwrap(); assert_eq!(parsed, expected); } diff --git a/apollo-federation/src/sources/connect/json_selection/pretty.rs b/apollo-federation/src/sources/connect/json_selection/pretty.rs index 8500b85973..43408625fe 100644 --- a/apollo-federation/src/sources/connect/json_selection/pretty.rs +++ b/apollo-federation/src/sources/connect/json_selection/pretty.rs @@ -448,9 +448,7 @@ mod tests { #[test] fn it_prints_root_selection() { - let (unmatched, root_selection) = JSONSelection::parse("id name").unwrap(); - assert!(unmatched.is_empty()); - + let root_selection = JSONSelection::parse("id name").unwrap(); test_permutations(root_selection, "id\nname"); } } diff --git a/apollo-federation/src/sources/connect/json_selection/selection_set.rs b/apollo-federation/src/sources/connect/json_selection/selection_set.rs index 5bb317c87b..be71659000 100644 --- a/apollo-federation/src/sources/connect/json_selection/selection_set.rs +++ b/apollo-federation/src/sources/connect/json_selection/selection_set.rs @@ -299,8 +299,7 @@ mod tests { } "###, ) - .unwrap() - .1; + .unwrap(); let schema = Schema::parse_and_validate( r###" @@ -368,8 +367,7 @@ mod tests { } "###, ) - .unwrap() - .1; + .unwrap(); let schema = Schema::parse_and_validate( r###" @@ -474,8 +472,7 @@ mod tests { } "###, ) - .unwrap() - .1; + .unwrap(); let schema = Schema::parse_and_validate( r###" @@ -545,8 +542,7 @@ mod tests { } "###, ) - .unwrap() - .1; + .unwrap(); let schema = Schema::parse_and_validate( r###" @@ -595,8 +591,7 @@ mod tests { } "###, ) - .unwrap() - .1; + .unwrap(); let schema = Schema::parse_and_validate( r###" diff --git a/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__error_snapshots-2.snap b/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__error_snapshots-2.snap index 9f8b0bc552..31ee30f07f 100644 --- a/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__error_snapshots-2.snap +++ b/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__error_snapshots-2.snap @@ -3,11 +3,9 @@ source: apollo-federation/src/sources/connect/json_selection/parser.rs expression: "JSONSelection::parse(\"$bogus\")" --- Err( - Failure( - JSONSelectionParseError { - message: "Unknown variable", - fragment: "$bogus", - offset: 0, - }, - ), + JSONSelectionParseError { + message: "Unknown variable", + fragment: "$bogus", + offset: 0, + }, ) diff --git a/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__error_snapshots-3.snap b/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__error_snapshots-3.snap index 5c4848988e..a38ba61010 100644 --- a/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__error_snapshots-3.snap +++ b/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__error_snapshots-3.snap @@ -3,11 +3,9 @@ source: apollo-federation/src/sources/connect/json_selection/parser.rs expression: "JSONSelection::parse(\"id $.object\")" --- Err( - Failure( - JSONSelectionParseError { - message: "Named path selection must either begin with alias or end with subselection", - fragment: "$.object", - offset: 3, - }, - ), + JSONSelectionParseError { + message: "Named path selection must either begin with alias or end with subselection", + fragment: "$.object", + offset: 3, + }, ) diff --git a/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__error_snapshots.snap b/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__error_snapshots.snap index 1569ecdcfb..a3d6191722 100644 --- a/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__error_snapshots.snap +++ b/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__error_snapshots.snap @@ -3,11 +3,9 @@ source: apollo-federation/src/sources/connect/json_selection/parser.rs expression: "JSONSelection::parse(\".data\")" --- Err( - Failure( - JSONSelectionParseError { - message: "Key paths cannot start with just .key (use $.key instead)", - fragment: ".data", - offset: 0, - }, - ), + JSONSelectionParseError { + message: "Key paths cannot start with just .key (use $.key instead)", + fragment: ".data", + offset: 0, + }, ) diff --git a/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-4.snap b/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-4.snap index 13668f7b1d..fdb02f5f99 100644 --- a/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-4.snap +++ b/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-4.snap @@ -3,11 +3,9 @@ source: apollo-federation/src/sources/connect/json_selection/parser.rs expression: "JSONSelection::parse(r#\"\n id\n created\n choices->first.message\n model\n \"#)" --- Err( - Failure( - JSONSelectionParseError { - message: "Named path selection must either begin with alias or end with subselection", - fragment: "choices->first.message\n model\n ", - offset: 48, - }, - ), + JSONSelectionParseError { + message: "Named path selection must either begin with alias or end with subselection", + fragment: "choices->first.message\n model\n ", + offset: 48, + }, ) diff --git a/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-5.snap b/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-5.snap index fee1908412..5eb4503516 100644 --- a/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-5.snap +++ b/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-5.snap @@ -3,140 +3,137 @@ source: apollo-federation/src/sources/connect/json_selection/parser.rs expression: "JSONSelection::parse(r#\"\n id: $this.id\n $args.input {\n title\n body\n }\n \"#)" --- Ok( - ( - "", - Named( - SubSelection { - selections: [ - Path( - Some( - Alias { - name: WithRange { - node: Field( - "id", - ), - range: Some( - 13..15, - ), - }, - range: Some( - 13..16, - ), - }, - ), - PathSelection { - path: WithRange { - node: Var( - WithRange { - node: $this, - range: Some( - 17..22, - ), - }, - WithRange { - node: Key( - WithRange { - node: Field( - "id", - ), - range: Some( - 23..25, - ), - }, - WithRange { - node: Empty, - range: Some( - 25..25, - ), - }, - ), - range: Some( - 22..25, - ), - }, + Named( + SubSelection { + selections: [ + Path( + Some( + Alias { + name: WithRange { + node: Field( + "id", ), range: Some( - 17..25, + 13..15, ), }, + range: Some( + 13..16, + ), }, ), - Path( - None, - PathSelection { - path: WithRange { - node: Var( - WithRange { - node: $args, - range: Some( - 38..43, - ), - }, - WithRange { - node: Key( - WithRange { - node: Field( - "input", - ), - range: Some( - 44..49, - ), - }, - WithRange { - node: Selection( - SubSelection { - selections: [ - Field( - None, - WithRange { - node: Field( - "title", - ), - range: Some( - 68..73, - ), - }, - None, - ), - Field( - None, - WithRange { - node: Field( - "body", - ), - range: Some( - 90..94, - ), - }, - None, - ), - ], - range: Some( - 50..108, + PathSelection { + path: WithRange { + node: Var( + WithRange { + node: $this, + range: Some( + 17..22, + ), + }, + WithRange { + node: Key( + WithRange { + node: Field( + "id", + ), + range: Some( + 23..25, + ), + }, + WithRange { + node: Empty, + range: Some( + 25..25, + ), + }, + ), + range: Some( + 22..25, + ), + }, + ), + range: Some( + 17..25, + ), + }, + }, + ), + Path( + None, + PathSelection { + path: WithRange { + node: Var( + WithRange { + node: $args, + range: Some( + 38..43, + ), + }, + WithRange { + node: Key( + WithRange { + node: Field( + "input", + ), + range: Some( + 44..49, + ), + }, + WithRange { + node: Selection( + SubSelection { + selections: [ + Field( + None, + WithRange { + node: Field( + "title", + ), + range: Some( + 68..73, + ), + }, + None, ), - }, - ), - range: Some( - 50..108, - ), - }, - ), - range: Some( - 43..108, - ), - }, - ), - range: Some( - 38..108, - ), - }, + Field( + None, + WithRange { + node: Field( + "body", + ), + range: Some( + 90..94, + ), + }, + None, + ), + ], + range: Some( + 50..108, + ), + }, + ), + range: Some( + 50..108, + ), + }, + ), + range: Some( + 43..108, + ), + }, + ), + range: Some( + 38..108, + ), }, - ), - ], - range: Some( - 13..108, + }, ), - }, - ), + ], + range: Some( + 13..108, + ), + }, ), ) diff --git a/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-6.snap b/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-6.snap index 9351e02fd7..78f820f487 100644 --- a/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-6.snap +++ b/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-6.snap @@ -3,163 +3,160 @@ source: apollo-federation/src/sources/connect/json_selection/parser.rs expression: "JSONSelection::parse(r#\"\n $this { id }\n $args { $.input { title body } }\n \"#)" --- Ok( - ( - "", - Named( - SubSelection { - selections: [ - Path( - None, - PathSelection { - path: WithRange { - node: Var( - WithRange { - node: $this, - range: Some( - 13..18, - ), - }, - WithRange { - node: Selection( - SubSelection { - selections: [ - Field( - None, - WithRange { - node: Field( - "id", - ), - range: Some( - 21..23, - ), - }, - None, - ), - ], - range: Some( - 19..25, + Named( + SubSelection { + selections: [ + Path( + None, + PathSelection { + path: WithRange { + node: Var( + WithRange { + node: $this, + range: Some( + 13..18, + ), + }, + WithRange { + node: Selection( + SubSelection { + selections: [ + Field( + None, + WithRange { + node: Field( + "id", + ), + range: Some( + 21..23, + ), + }, + None, ), - }, - ), - range: Some( - 19..25, - ), - }, - ), - range: Some( - 13..25, - ), - }, + ], + range: Some( + 19..25, + ), + }, + ), + range: Some( + 19..25, + ), + }, + ), + range: Some( + 13..25, + ), }, - ), - Path( - None, - PathSelection { - path: WithRange { - node: Var( - WithRange { - node: $args, - range: Some( - 38..43, - ), - }, - WithRange { - node: Selection( - SubSelection { - selections: [ - Path( - None, - PathSelection { - path: WithRange { - node: Var( - WithRange { - node: $, - range: Some( - 46..47, - ), - }, - WithRange { - node: Key( - WithRange { - node: Field( - "input", - ), - range: Some( - 48..53, - ), - }, - WithRange { - node: Selection( - SubSelection { - selections: [ - Field( - None, - WithRange { - node: Field( - "title", - ), - range: Some( - 56..61, - ), - }, - None, - ), - Field( - None, - WithRange { - node: Field( - "body", - ), - range: Some( - 62..66, - ), - }, - None, - ), - ], - range: Some( - 54..68, + }, + ), + Path( + None, + PathSelection { + path: WithRange { + node: Var( + WithRange { + node: $args, + range: Some( + 38..43, + ), + }, + WithRange { + node: Selection( + SubSelection { + selections: [ + Path( + None, + PathSelection { + path: WithRange { + node: Var( + WithRange { + node: $, + range: Some( + 46..47, + ), + }, + WithRange { + node: Key( + WithRange { + node: Field( + "input", + ), + range: Some( + 48..53, + ), + }, + WithRange { + node: Selection( + SubSelection { + selections: [ + Field( + None, + WithRange { + node: Field( + "title", + ), + range: Some( + 56..61, + ), + }, + None, ), - }, - ), - range: Some( - 54..68, - ), - }, - ), - range: Some( - 47..68, - ), - }, - ), - range: Some( - 46..68, - ), - }, + Field( + None, + WithRange { + node: Field( + "body", + ), + range: Some( + 62..66, + ), + }, + None, + ), + ], + range: Some( + 54..68, + ), + }, + ), + range: Some( + 54..68, + ), + }, + ), + range: Some( + 47..68, + ), + }, + ), + range: Some( + 46..68, + ), }, - ), - ], - range: Some( - 44..70, + }, ), - }, - ), - range: Some( - 44..70, - ), - }, - ), - range: Some( - 38..70, - ), - }, + ], + range: Some( + 44..70, + ), + }, + ), + range: Some( + 44..70, + ), + }, + ), + range: Some( + 38..70, + ), }, - ), - ], - range: Some( - 13..70, + }, ), - }, - ), + ], + range: Some( + 13..70, + ), + }, ), ) diff --git a/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-7.snap b/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-7.snap index 65eca95ca2..1d58e549d9 100644 --- a/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-7.snap +++ b/apollo-federation/src/sources/connect/json_selection/snapshots/apollo_federation__sources__connect__json_selection__parser__tests__path_with_subselection-7.snap @@ -3,268 +3,265 @@ source: apollo-federation/src/sources/connect/json_selection/parser.rs expression: "JSONSelection::parse(r#\"\n # Equivalent to id: $this.id\n $this { id }\n\n $args {\n __typename: $(\"Args\")\n\n # Using $. instead of just . prevents .input from\n # parsing as a key applied to the $(\"Args\") string.\n $.input { title body }\n\n extra\n }\n\n from: $.from\n \"#)" --- Ok( - ( - "", - Named( - SubSelection { - selections: [ - Path( - None, - PathSelection { - path: WithRange { - node: Var( - WithRange { - node: $this, - range: Some( - 54..59, - ), - }, - WithRange { - node: Selection( - SubSelection { - selections: [ - Field( - None, - WithRange { - node: Field( - "id", - ), + Named( + SubSelection { + selections: [ + Path( + None, + PathSelection { + path: WithRange { + node: Var( + WithRange { + node: $this, + range: Some( + 54..59, + ), + }, + WithRange { + node: Selection( + SubSelection { + selections: [ + Field( + None, + WithRange { + node: Field( + "id", + ), + range: Some( + 62..64, + ), + }, + None, + ), + ], + range: Some( + 60..66, + ), + }, + ), + range: Some( + 60..66, + ), + }, + ), + range: Some( + 54..66, + ), + }, + }, + ), + Path( + None, + PathSelection { + path: WithRange { + node: Var( + WithRange { + node: $args, + range: Some( + 80..85, + ), + }, + WithRange { + node: Selection( + SubSelection { + selections: [ + Path( + Some( + Alias { + name: WithRange { + node: Field( + "__typename", + ), + range: Some( + 104..114, + ), + }, range: Some( - 62..64, + 104..115, ), }, - None, ), - ], - range: Some( - 60..66, - ), - }, - ), - range: Some( - 60..66, - ), - }, - ), - range: Some( - 54..66, - ), - }, - }, - ), - Path( - None, - PathSelection { - path: WithRange { - node: Var( - WithRange { - node: $args, - range: Some( - 80..85, - ), - }, - WithRange { - node: Selection( - SubSelection { - selections: [ - Path( - Some( - Alias { - name: WithRange { - node: Field( - "__typename", + PathSelection { + path: WithRange { + node: Expr( + WithRange { + node: String( + "Args", ), range: Some( - 104..114, + 118..124, ), }, - range: Some( - 104..115, - ), - }, - ), - PathSelection { - path: WithRange { - node: Expr( - WithRange { - node: String( - "Args", - ), - range: Some( - 118..124, - ), - }, - WithRange { - node: Empty, - range: Some( - 125..125, - ), - }, - ), - range: Some( - 116..125, - ), - }, + WithRange { + node: Empty, + range: Some( + 125..125, + ), + }, + ), + range: Some( + 116..125, + ), }, - ), - Path( - None, - PathSelection { - path: WithRange { - node: Var( - WithRange { - node: $, - range: Some( - 277..278, - ), - }, - WithRange { - node: Key( - WithRange { - node: Field( - "input", - ), - range: Some( - 279..284, - ), - }, - WithRange { - node: Selection( - SubSelection { - selections: [ - Field( - None, - WithRange { - node: Field( - "title", - ), - range: Some( - 287..292, - ), - }, - None, - ), - Field( - None, - WithRange { - node: Field( - "body", - ), - range: Some( - 293..297, - ), - }, - None, - ), - ], - range: Some( - 285..299, + }, + ), + Path( + None, + PathSelection { + path: WithRange { + node: Var( + WithRange { + node: $, + range: Some( + 277..278, + ), + }, + WithRange { + node: Key( + WithRange { + node: Field( + "input", + ), + range: Some( + 279..284, + ), + }, + WithRange { + node: Selection( + SubSelection { + selections: [ + Field( + None, + WithRange { + node: Field( + "title", + ), + range: Some( + 287..292, + ), + }, + None, ), - }, - ), - range: Some( - 285..299, - ), - }, - ), - range: Some( - 278..299, - ), - }, - ), - range: Some( - 277..299, - ), - }, - }, - ), - Field( - None, - WithRange { - node: Field( - "extra", + Field( + None, + WithRange { + node: Field( + "body", + ), + range: Some( + 293..297, + ), + }, + None, + ), + ], + range: Some( + 285..299, + ), + }, + ), + range: Some( + 285..299, + ), + }, + ), + range: Some( + 278..299, + ), + }, ), range: Some( - 317..322, + 277..299, ), }, - None, - ), - ], - range: Some( - 86..336, + }, + ), + Field( + None, + WithRange { + node: Field( + "extra", + ), + range: Some( + 317..322, + ), + }, + None, ), - }, - ), - range: Some( - 86..336, - ), - }, + ], + range: Some( + 86..336, + ), + }, + ), + range: Some( + 86..336, + ), + }, + ), + range: Some( + 80..336, + ), + }, + }, + ), + Path( + Some( + Alias { + name: WithRange { + node: Field( + "from", ), range: Some( - 80..336, + 350..354, ), }, + range: Some( + 350..355, + ), }, ), - Path( - Some( - Alias { - name: WithRange { - node: Field( - "from", + PathSelection { + path: WithRange { + node: Var( + WithRange { + node: $, + range: Some( + 356..357, + ), + }, + WithRange { + node: Key( + WithRange { + node: Field( + "from", + ), + range: Some( + 358..362, + ), + }, + WithRange { + node: Empty, + range: Some( + 362..362, + ), + }, ), range: Some( - 350..354, + 357..362, ), }, - range: Some( - 350..355, - ), - }, - ), - PathSelection { - path: WithRange { - node: Var( - WithRange { - node: $, - range: Some( - 356..357, - ), - }, - WithRange { - node: Key( - WithRange { - node: Field( - "from", - ), - range: Some( - 358..362, - ), - }, - WithRange { - node: Empty, - range: Some( - 362..362, - ), - }, - ), - range: Some( - 357..362, - ), - }, - ), - range: Some( - 356..362, - ), - }, + ), + range: Some( + 356..362, + ), }, - ), - ], - range: Some( - 54..362, + }, ), - }, - ), + ], + range: Some( + 54..362, + ), + }, ), ) diff --git a/apollo-federation/src/sources/connect/spec/directives.rs b/apollo-federation/src/sources/connect/spec/directives.rs index 1da002ed4e..893d590c72 100644 --- a/apollo-federation/src/sources/connect/spec/directives.rs +++ b/apollo-federation/src/sources/connect/spec/directives.rs @@ -287,15 +287,8 @@ impl ConnectDirectiveArguments { let selection_value = arg.value.as_str().ok_or(internal!( "`selection` field in `@connect` directive is not a string" ))?; - let (remainder, selection_value) = JSONSelection::parse(selection_value) - .map_err(|_| internal!("invalid JSON selection"))?; - if !remainder.is_empty() { - return Err(internal!(format!( - "`selection` field in `@connect` directive could not be fully parsed: the following was left over: {remainder}" - ))); - } - - selection = Some(selection_value); + selection = + Some(JSONSelection::parse(selection_value).map_err(|e| internal!(e.message))?); } else if arg_name == CONNECT_ENTITY_ARGUMENT_NAME.as_str() { let entity_value = arg.value.to_bool().ok_or(internal!( "`entity` field in `@connect` directive is not a boolean" @@ -337,15 +330,7 @@ impl TryFrom<&ObjectNode> for ConnectHTTPArguments { let body_value = value.as_str().ok_or(internal!( "`body` field in `@connect` directive's `http` field is not a string" ))?; - let (remainder, body_value) = JSONSelection::parse(body_value) - .map_err(|_| internal!("invalid JSON selection"))?; - if !remainder.is_empty() { - return Err(internal!(format!( - "`body` field in `@connect` directive could not be fully parsed: the following was left over: {remainder}" - ))); - } - - body = Some(body_value); + body = Some(JSONSelection::parse(body_value).map_err(|e| internal!(e.message))?); } else if name == HEADERS_ARGUMENT_NAME.as_str() { // TODO: handle a single object since the language spec allows it headers = value.as_list().map(nodes_to_headers).transpose()?; diff --git a/apollo-federation/src/sources/connect/validation/selection.rs b/apollo-federation/src/sources/connect/validation/selection.rs index 9cc4665b31..ebbb6eb143 100644 --- a/apollo-federation/src/sources/connect/validation/selection.rs +++ b/apollo-federation/src/sources/connect/validation/selection.rs @@ -76,7 +76,7 @@ pub(super) fn validate_body_selection( let selection_str = require_value_is_str(selection_node, &coordinate, &schema.sources)?; - let (_rest, selection) = JSONSelection::parse(selection_str).map_err(|err| Message { + let selection = JSONSelection::parse(selection_str).map_err(|err| Message { code: Code::InvalidJsonSelection, message: format!("{coordinate} is not a valid JSONSelection: {err}"), locations: selection_node @@ -129,16 +129,15 @@ fn get_json_selection<'a>( .collect(), })?; - let (_rest, selection) = - JSONSelection::parse(selection_str.as_str()).map_err(|err| Message { - code: Code::InvalidJsonSelection, - message: format!("{coordinate} is not a valid JSONSelection: {err}",), - locations: selection_arg - .value - .line_column_range(source_map) - .into_iter() - .collect(), - })?; + let selection = JSONSelection::parse(selection_str.as_str()).map_err(|err| Message { + code: Code::InvalidJsonSelection, + message: format!("{coordinate} is not a valid JSONSelection: {err}",), + locations: selection_arg + .value + .line_column_range(source_map) + .into_iter() + .collect(), + })?; if selection.is_empty() { return Err(Message { diff --git a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@invalid_selection_syntax.graphql.snap b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@invalid_selection_syntax.graphql.snap index ae80600189..ba10fe51b9 100644 --- a/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@invalid_selection_syntax.graphql.snap +++ b/apollo-federation/src/sources/connect/validation/snapshots/validation_tests@invalid_selection_syntax.graphql.snap @@ -6,7 +6,7 @@ input_file: apollo-federation/src/sources/connect/validation/test_data/invalid_s [ Message { code: InvalidJsonSelection, - message: "`@connect(selection:)` on `Query.something` is not a valid JSONSelection: Parsing Failure: JSONSelectionParseError { message: \"nom::error::ErrorKind::Eof\", fragment: \"&how\", offset: 0 }", + message: "`@connect(selection:)` on `Query.something` is not a valid JSONSelection: nom::error::ErrorKind::Eof at offset 0: &how", locations: [ 8:86..8:92, ], diff --git a/apollo-router/src/plugins/connectors/handle_responses.rs b/apollo-router/src/plugins/connectors/handle_responses.rs index d7276318e2..8a12a66ca5 100644 --- a/apollo-router/src/plugins/connectors/handle_responses.rs +++ b/apollo-router/src/plugins/connectors/handle_responses.rs @@ -297,7 +297,7 @@ mod tests { headers: Default::default(), body: Default::default(), }, - selection: JSONSelection::parse("$.data").unwrap().1, + selection: JSONSelection::parse("$.data").unwrap(), entity_resolver: None, config: Default::default(), max_requests: None, @@ -310,7 +310,7 @@ mod tests { name: "hello".to_string(), inputs: Default::default(), typename: ResponseTypeName::Concrete("String".to_string()), - selection: Arc::new(JSONSelection::parse("$.data").unwrap().1), + selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; let response2 = http::Response::builder() @@ -320,7 +320,7 @@ mod tests { name: "hello2".to_string(), inputs: Default::default(), typename: ResponseTypeName::Concrete("String".to_string()), - selection: Arc::new(JSONSelection::parse("$.data").unwrap().1), + selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; let res = super::handle_responses( @@ -391,7 +391,7 @@ mod tests { headers: Default::default(), body: Default::default(), }, - selection: JSONSelection::parse("$.data { id }").unwrap().1, + selection: JSONSelection::parse("$.data { id }").unwrap(), entity_resolver: Some(EntityResolver::Explicit), config: Default::default(), max_requests: None, @@ -404,7 +404,7 @@ mod tests { index: 0, inputs: Default::default(), typename: ResponseTypeName::Concrete("User".to_string()), - selection: Arc::new(JSONSelection::parse("$.data").unwrap().1), + selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; let response2 = http::Response::builder() @@ -414,7 +414,7 @@ mod tests { index: 1, inputs: Default::default(), typename: ResponseTypeName::Concrete("User".to_string()), - selection: Arc::new(JSONSelection::parse("$.data").unwrap().1), + selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; let res = super::handle_responses( @@ -497,7 +497,7 @@ mod tests { headers: Default::default(), body: Default::default(), }, - selection: JSONSelection::parse("$.data").unwrap().1, + selection: JSONSelection::parse("$.data").unwrap(), entity_resolver: Some(EntityResolver::Implicit), config: Default::default(), max_requests: None, @@ -511,7 +511,7 @@ mod tests { inputs: Default::default(), field_name: "field".to_string(), typename: ResponseTypeName::Concrete("User".to_string()), - selection: Arc::new(JSONSelection::parse("$.data").unwrap().1), + selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; let response2 = http::Response::builder() @@ -522,7 +522,7 @@ mod tests { inputs: Default::default(), field_name: "field".to_string(), typename: ResponseTypeName::Concrete("User".to_string()), - selection: Arc::new(JSONSelection::parse("$.data").unwrap().1), + selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; let res = super::handle_responses( @@ -605,7 +605,7 @@ mod tests { headers: Default::default(), body: Default::default(), }, - selection: JSONSelection::parse("$.data").unwrap().1, + selection: JSONSelection::parse("$.data").unwrap(), entity_resolver: Some(EntityResolver::Explicit), config: Default::default(), max_requests: None, @@ -619,7 +619,7 @@ mod tests { index: 0, inputs: Default::default(), typename: ResponseTypeName::Concrete("User".to_string()), - selection: Arc::new(JSONSelection::parse("$.data").unwrap().1), + selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; let response2 = http::Response::builder() @@ -629,7 +629,7 @@ mod tests { index: 1, inputs: Default::default(), typename: ResponseTypeName::Concrete("User".to_string()), - selection: Arc::new(JSONSelection::parse("$.data").unwrap().1), + selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; let response3 = http::Response::builder() @@ -640,7 +640,7 @@ mod tests { index: 2, inputs: Default::default(), typename: ResponseTypeName::Concrete("User".to_string()), - selection: Arc::new(JSONSelection::parse("$.data").unwrap().1), + selection: Arc::new(JSONSelection::parse("$.data").unwrap()), }; let res = super::handle_responses( diff --git a/apollo-router/src/plugins/connectors/make_requests.rs b/apollo-router/src/plugins/connectors/make_requests.rs index 9beccbff2a..9ff545cd1e 100644 --- a/apollo-router/src/plugins/connectors/make_requests.rs +++ b/apollo-router/src/plugins/connectors/make_requests.rs @@ -570,7 +570,7 @@ mod tests { headers: Default::default(), body: Default::default(), }, - selection: JSONSelection::parse("f").unwrap().1, + selection: JSONSelection::parse("f").unwrap(), entity_resolver: None, config: Default::default(), max_requests: None, @@ -704,7 +704,7 @@ mod tests { headers: Default::default(), body: Default::default(), }, - selection: JSONSelection::parse("$").unwrap().1, + selection: JSONSelection::parse("$").unwrap(), entity_resolver: None, config: Default::default(), max_requests: None, @@ -866,7 +866,7 @@ mod tests { headers: Default::default(), body: Default::default(), }, - selection: JSONSelection::parse("$.data").unwrap().1, + selection: JSONSelection::parse("$.data").unwrap(), entity_resolver: None, config: Default::default(), max_requests: None, @@ -1098,7 +1098,7 @@ mod tests { headers: Default::default(), body: Default::default(), }, - selection: JSONSelection::parse("field").unwrap().1, + selection: JSONSelection::parse("field").unwrap(), entity_resolver: Some(super::EntityResolver::Explicit), config: Default::default(), max_requests: None, @@ -1417,7 +1417,7 @@ mod tests { headers: Default::default(), body: Default::default(), }, - selection: JSONSelection::parse("field").unwrap().1, + selection: JSONSelection::parse("field").unwrap(), entity_resolver: Some(super::EntityResolver::Explicit), config: Default::default(), max_requests: None, @@ -1717,7 +1717,7 @@ mod tests { headers: Default::default(), body: Default::default(), }, - selection: JSONSelection::parse("field { field }").unwrap().1, + selection: JSONSelection::parse("field { field }").unwrap(), entity_resolver: None, config: Default::default(), max_requests: None, @@ -1939,7 +1939,7 @@ mod tests { headers: Default::default(), body: Default::default(), }, - selection: JSONSelection::parse("selected").unwrap().1, + selection: JSONSelection::parse("selected").unwrap(), entity_resolver: None, config: Default::default(), max_requests: None, @@ -2214,7 +2214,7 @@ mod tests { headers: Default::default(), body: Default::default(), }, - selection: JSONSelection::parse("selected").unwrap().1, + selection: JSONSelection::parse("selected").unwrap(), entity_resolver: None, config: Default::default(), max_requests: None, @@ -2486,7 +2486,7 @@ mod tests { headers: Default::default(), body: Default::default(), }, - selection: JSONSelection::parse("selected").unwrap().1, + selection: JSONSelection::parse("selected").unwrap(), entity_resolver: None, config: Default::default(), max_requests: None, @@ -2623,7 +2623,7 @@ mod tests { headers: Default::default(), body: Default::default(), }, - selection: JSONSelection::parse("$.data").unwrap().1, + selection: JSONSelection::parse("$.data").unwrap(), entity_resolver: None, config: Default::default(), max_requests: None,