Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust Apollo signature generation fixes #5049

Closed
Prev Previous commit
Next Next commit
More signature fixes
bonnici committed Apr 30, 2024
commit 0423a97fc2e5da6df5561a0457361677662e0380
22 changes: 14 additions & 8 deletions apollo-router/src/apollo_studio_interop/mod.rs
Original file line number Diff line number Diff line change
@@ -32,6 +32,7 @@ pub(crate) struct ComparableUsageReporting {
}

/// Enum specifying the result of a comparison.
#[derive(Debug)]
pub(crate) enum UsageReportingComparisonResult {
/// The UsageReporting instances are the same
Equal,
@@ -370,7 +371,7 @@ fn format_selection_set(selection_set: &SelectionSet, f: &mut fmt::Formatter) ->
&& field_str
.chars()
.last()
.map_or(false, |c| c.is_alphanumeric())
.map_or(false, |c| c.is_alphanumeric() || c == '_')
{
f.write_str(" ")?;
}
@@ -415,17 +416,22 @@ fn format_field(field: &Node<Field>, f: &mut fmt::Formatter) -> fmt::Result {

// The graphql-js implementation will use newlines and indentation instead of commas if the length of the "arg line" is
// over 80 characters. This "arg line" includes the alias followed by ": " if the field has an alias (which is never
// the case for now), followed by the field name, followed by all argument names and values separated by ": ", surrounded
// with brackets. Our usage reporting plugin replaces all newlines + indentation with a single space, so we have to replace
// commas with spaces if the line length is too long.
// the case for any signatures that the JS implementation formatted), followed by the field name, followed by all argument
// names and values separated by ": ", surrounded with brackets. Our usage reporting plugin replaces all newlines +
// indentation with a single space, so we have to replace commas with spaces if the line length is too long.
let arg_strings: Vec<String> = sorted_args
.iter()
.map(|a| ApolloReportingSignatureFormatter::Argument(a).to_string())
.collect();
// Adjust for incorrect spacing generated by the argument formatter - 2 extra characters for the surrounding brackets, plus
// 2 extra characters per argument for the separating space and the space between the argument name and type.
// Adjust for incorrect spacing generated by the argument formatter. We end summing up:
// * the length of field name
// * 2 extra characters for the surrounding brackets
// * the length of all formatted arguments
// * one extra character per argument since the JS implementation inserts a space between the argument name and value
// * two extra character per argument except the last one since the JS implementation inserts a separating comma and space
// between arguments (but not the last one)
let original_line_length =
field.name.len() + 2 + arg_strings.iter().map(|s| s.len()).sum::<usize>() + (arg_strings.len() * 2);
field.name.len() + 2 + arg_strings.iter().map(|s| s.len()).sum::<usize>() + arg_strings.len() + ((arg_strings.len() - 1) * 2);
let separator = if original_line_length > 80 { " " } else { "," };

for (index, arg_string) in arg_strings.iter().enumerate() {
@@ -438,7 +444,7 @@ fn format_field(field: &Node<Field>, f: &mut fmt::Formatter) -> fmt::Result {
|| arg_string
.chars()
.last()
.map_or(true, |c| c.is_alphanumeric()))
.map_or(true, |c| c.is_alphanumeric() || c == '_'))
{
f.write_str(separator)?;
}
Original file line number Diff line number Diff line change
@@ -187,6 +187,8 @@ type Query
defaultArgQuery(stringInput: String! = "default", inputType: AnotherInputType = {anotherInput: "inputDefault"}): BasicResponse!
inputTypeDefaultQuery(input: InputTypeWithDefault): BasicResponse!
sortQuery(listInput: [String!]!, stringInput: String!, nullableStringInput: String, INTInput: Int!, floatInput: Float!, boolInput: Boolean!, enumInput: SomeEnum, idInput: ID!): SortResponse!
manyArgsQuery(arg1: String, arg2: String, arg3: String, arg4: String, arg5: String, arg6: String, arg7: String): EverythingResponse!
underscoreQuery(arg_: String, _arg2: String, _arg3_: String): UnderscoreResponse
}

enum SomeEnum
@@ -225,6 +227,15 @@ type TestGraphResponse3
updateCheckConfiguration(arg1: String, arg2: String, arg3: String): Int!
}

type UnderscoreResponse
@join__type(graph: MAIN)
{
_: String
_name: String
_name_: String
name_: String
}

type Subscription
@join__type(graph: MAIN)
{
121 changes: 121 additions & 0 deletions apollo-router/src/apollo_studio_interop/tests.rs
Original file line number Diff line number Diff line change
@@ -1397,6 +1397,127 @@ async fn test_mutation_comma() {
assert_bridge_results(schema_str, query_str, expected_sig, &expected_refs).await;
}

#[test(tokio::test)]
async fn test_comma_lower_bound() {
let schema_str = include_str!("testdata/schema_interop.graphql");

let query_str = "
query TestCommaLowerBound($arg: String, $slightlyTooLongName1234: String) {
manyArgsQuery(arg1: $arg, arg2: $arg, arg3: $arg, arg4: $slightlyTooLongName1234) {
enumResponse
}
}";

let schema = Schema::parse_and_validate(schema_str, "schema.graphql").unwrap();
let doc = ExecutableDocument::parse(&schema, query_str, "query.graphql").unwrap();

let generated =
generate_usage_reporting(&doc, &doc, &Some("TestCommaLowerBound".into()), &schema);

let expected_sig = "# TestCommaLowerBound\nquery TestCommaLowerBound($arg:String,$slightlyTooLongName1234:String){manyArgsQuery(arg1:$arg arg2:$arg arg3:$arg arg4:$slightlyTooLongName1234){enumResponse}}";
let expected_refs: HashMap<String, ReferencedFieldsForType> = HashMap::from([
(
"Query".into(),
ReferencedFieldsForType {
field_names: vec!["manyArgsQuery".into()],
is_interface: false,
},
),
(
"EverythingResponse".into(),
ReferencedFieldsForType {
field_names: vec!["enumResponse".into()],
is_interface: false,
},
),
]);

assert_expected_results(&generated, expected_sig, &expected_refs);
assert_bridge_results(schema_str, query_str, expected_sig, &expected_refs).await;
}

#[test(tokio::test)]
async fn test_comma_upper_bound() {
let schema_str = include_str!("testdata/schema_interop.graphql");

let query_str = "
query TestCommaUpperBound($arg: String, $slightlyTooLongName12345: String) {
manyArgsQuery(arg1: $arg, arg2: $arg, arg3: $arg, arg4: $slightlyTooLongName12345) {
enumResponse
}
}";

let schema = Schema::parse_and_validate(schema_str, "schema.graphql").unwrap();
let doc = ExecutableDocument::parse(&schema, query_str, "query.graphql").unwrap();

let generated =
generate_usage_reporting(&doc, &doc, &Some("TestCommaUpperBound".into()), &schema);

let expected_sig = "# TestCommaUpperBound\nquery TestCommaUpperBound($arg:String,$slightlyTooLongName12345:String){manyArgsQuery(arg1:$arg arg2:$arg arg3:$arg arg4:$slightlyTooLongName12345){enumResponse}}";
let expected_refs: HashMap<String, ReferencedFieldsForType> = HashMap::from([
(
"Query".into(),
ReferencedFieldsForType {
field_names: vec!["manyArgsQuery".into()],
is_interface: false,
},
),
(
"EverythingResponse".into(),
ReferencedFieldsForType {
field_names: vec!["enumResponse".into()],
is_interface: false,
},
),
]);

assert_expected_results(&generated, expected_sig, &expected_refs);
assert_bridge_results(schema_str, query_str, expected_sig, &expected_refs).await;
}


#[test(tokio::test)]
async fn test_underscore() {
let schema_str = include_str!("testdata/schema_interop.graphql");

let query_str = "
query UnderscoreQuery($arg2_: String, $_arg3_: String) {
underscoreQuery(arg_: \"x\", _arg2: $arg2_, _arg3_: $_arg3_) {
_
_name
_name_
name_
}
}";

let schema = Schema::parse_and_validate(schema_str, "schema.graphql").unwrap();
let doc = ExecutableDocument::parse(&schema, query_str, "query.graphql").unwrap();

let generated =
generate_usage_reporting(&doc, &doc, &Some("UnderscoreQuery".into()), &schema);

let expected_sig = "# UnderscoreQuery\nquery UnderscoreQuery($_arg3_:String,$arg2_:String){underscoreQuery(_arg2:$arg2_,_arg3_:$_arg3_,arg_:\"\"){_ _name _name_ name_}}";
let expected_refs: HashMap<String, ReferencedFieldsForType> = HashMap::from([
(
"Query".into(),
ReferencedFieldsForType {
field_names: vec!["underscoreQuery".into()],
is_interface: false,
},
),
(
"UnderscoreResponse".into(),
ReferencedFieldsForType {
field_names: vec!["_".into(), "_name".into(), "_name_".into(), "name_".into()],
is_interface: false,
},
),
]);

assert_expected_results(&generated, expected_sig, &expected_refs);
assert_bridge_results(schema_str, query_str, expected_sig, &expected_refs).await;
}

#[test(tokio::test)]
async fn test_compare() {
let source = ComparableUsageReporting {