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
Fix more comma edge cases
  • Loading branch information
bonnici committed Apr 29, 2024
commit 8937aa542f08f20c79f530c11a97c1311fa9af98
8 changes: 4 additions & 4 deletions apollo-router/src/apollo_studio_interop/mod.rs
Original file line number Diff line number Diff line change
@@ -415,17 +415,17 @@ 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 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 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.
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.
let original_line_length =
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() * 2);
let separator = if original_line_length > 80 { " " } else { "," };

for (index, arg_string) in arg_strings.iter().enumerate() {
Original file line number Diff line number Diff line change
@@ -146,6 +146,10 @@ type Mutation
@join__type(graph: MAIN)
{
noInputMutation: EverythingResponse!
graph(id: ID!): TestGraphResponse!
mutation2(id: ID!): TestGraphResponse2!
mutation3(id: ID!): TestGraphResponse3!
mutation4(id1: ID!, id2: ID!): ID!
}

input NestedEnumInputType
@@ -203,6 +207,24 @@ type SortResponse
CCC: Int
}

type TestGraphResponse
@join__type(graph: MAIN)
{
updateCheckConfiguration(downgradeDefaultValueChange: Boolean, downgradeStaticChecks: Boolean): Int!
}

type TestGraphResponse2
@join__type(graph: MAIN)
{
updateCheckConfiguration(arg1: Boolean, arg2: Boolean): Int!
}

type TestGraphResponse3
@join__type(graph: MAIN)
{
updateCheckConfiguration(arg1: String, arg2: String, arg3: String): Int!
}

type Subscription
@join__type(graph: MAIN)
{
80 changes: 79 additions & 1 deletion apollo-router/src/apollo_studio_interop/tests.rs
Original file line number Diff line number Diff line change
@@ -1197,7 +1197,7 @@ async fn test_operation_arg_always_commas() {
}

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

let query_str = r#"query QueryCommaEdgeCase {
@@ -1319,6 +1319,84 @@ async fn test_nested_fragments() {
assert_bridge_results(schema_str, query_str, expected_sig, &expected_refs).await;
}

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

let query_str = "
mutation Test_Mutation_Space($arg1withalongnamegoeshere0123456789: Boolean) {
mutation2(id: \"x\") {
updateCheckConfiguration(arg1: $arg1withalongnamegoeshere0123456789, arg2: false)
}
}";

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("Test_Mutation_Space".into()), &schema);

let expected_sig = "# Test_Mutation_Space\nmutation Test_Mutation_Space($arg1withalongnamegoeshere0123456789:Boolean){mutation2(id:\"\"){updateCheckConfiguration(arg1:$arg1withalongnamegoeshere0123456789 arg2:false)}}";
let expected_refs: HashMap<String, ReferencedFieldsForType> = HashMap::from([
(
"Mutation".into(),
ReferencedFieldsForType {
field_names: vec!["mutation2".into()],
is_interface: false,
},
),
(
"TestGraphResponse2".into(),
ReferencedFieldsForType {
field_names: vec!["updateCheckConfiguration".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_mutation_comma() {
let schema_str = include_str!("testdata/schema_interop.graphql");

let query_str = "
mutation Test_Mutation_Comma($arg1withalongnamegoeshere012345678: Boolean) {
mutation2(id: \"x\") {
updateCheckConfiguration(arg1: $arg1withalongnamegoeshere012345678, arg2: false)
}
}";

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("Test_Mutation_Comma".into()), &schema);

let expected_sig = "# Test_Mutation_Comma\nmutation Test_Mutation_Comma($arg1withalongnamegoeshere012345678:Boolean){mutation2(id:\"\"){updateCheckConfiguration(arg1:$arg1withalongnamegoeshere012345678,arg2:false)}}";
let expected_refs: HashMap<String, ReferencedFieldsForType> = HashMap::from([
(
"Mutation".into(),
ReferencedFieldsForType {
field_names: vec!["mutation2".into()],
is_interface: false,
},
),
(
"TestGraphResponse2".into(),
ReferencedFieldsForType {
field_names: vec!["updateCheckConfiguration".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 {