From 8ee7334dbb986f5ef36bb630e33438e85377f5d9 Mon Sep 17 00:00:00 2001 From: Nick Marsh Date: Mon, 22 Apr 2024 13:29:20 +1000 Subject: [PATCH 1/6] Fix nested named fragments --- .../src/apollo_studio_interop/mod.rs | 1 + .../src/apollo_studio_interop/tests.rs | 85 +++++++++++++++++++ .../src/query_planner/bridge_query_planner.rs | 6 +- 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/apollo-router/src/apollo_studio_interop/mod.rs b/apollo-router/src/apollo_studio_interop/mod.rs index b1afad3f34..5d89d569fc 100644 --- a/apollo-router/src/apollo_studio_interop/mod.rs +++ b/apollo-router/src/apollo_studio_interop/mod.rs @@ -167,6 +167,7 @@ impl UsageReportingGenerator<'_> { .get(&fragment_node.fragment_name) { e.insert(fragment.clone()); + self.extract_signature_fragments(&fragment.selection_set); } } } diff --git a/apollo-router/src/apollo_studio_interop/tests.rs b/apollo-router/src/apollo_studio_interop/tests.rs index c3ae55ce6c..0660de4221 100644 --- a/apollo-router/src/apollo_studio_interop/tests.rs +++ b/apollo-router/src/apollo_studio_interop/tests.rs @@ -1196,6 +1196,91 @@ async fn test_operation_arg_always_commas() { assert_bridge_results(schema_str, query_str, expected_sig, &expected_refs).await; } +#[test(tokio::test)] +async fn test_nested_fragments() { + let schema_str = include_str!("testdata/schema_interop.graphql"); + + let query_str = " + fragment UnionType1Fragment on UnionType1 { + unionType1Field + } + + fragment ObjectResponseFragment on ObjectTypeResponse { + intField + } + + fragment EverythingResponseFragment on EverythingResponse { + listOfObjects { + ...ObjectResponseFragment + ... on ObjectTypeResponse { + stringField + } + } + } + + query NestedFragmentQuery { + noInputQuery { + ...EverythingResponseFragment + ... on EverythingResponse { + listOfUnions { + ...UnionType1Fragment + ... on UnionType2 { + unionType2Field + } + } + } + } + }"; + + 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("NestedFragmentQuery".into()), &schema); + + let expected_sig = "# NestedFragmentQuery\nfragment EverythingResponseFragment on EverythingResponse{listOfObjects{...ObjectResponseFragment...on ObjectTypeResponse{stringField}}}fragment ObjectResponseFragment on ObjectTypeResponse{intField}fragment UnionType1Fragment on UnionType1{unionType1Field}query NestedFragmentQuery{noInputQuery{...EverythingResponseFragment...on EverythingResponse{listOfUnions{...UnionType1Fragment...on UnionType2{unionType2Field}}}}}"; + let expected_refs: HashMap = HashMap::from([ + ( + "Query".into(), + ReferencedFieldsForType { + field_names: vec!["noInputQuery".into()], + is_interface: false, + }, + ), + ( + "EverythingResponse".into(), + ReferencedFieldsForType { + field_names: vec!["listOfObjects".into(), "listOfUnions".into()], + is_interface: false, + }, + ), + ( + "ObjectTypeResponse".into(), + ReferencedFieldsForType { + field_names: vec!["intField".into(), "stringField".into()], + is_interface: false, + }, + ), + ( + "UnionType1".into(), + ReferencedFieldsForType { + field_names: vec!["unionType1Field".into()], + is_interface: false, + }, + ), + ( + "UnionType2".into(), + ReferencedFieldsForType { + field_names: vec!["unionType2Field".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 { diff --git a/apollo-router/src/query_planner/bridge_query_planner.rs b/apollo-router/src/query_planner/bridge_query_planner.rs index 4825ce57a3..6231c3fc15 100644 --- a/apollo-router/src/query_planner/bridge_query_planner.rs +++ b/apollo-router/src/query_planner/bridge_query_planner.rs @@ -472,7 +472,8 @@ impl BridgeQueryPlanner { "Mismatch between the Apollo usage reporting signature generated in router and router-bridge" ); tracing::debug!( - "Different signatures generated between router and router-bridge:\n{}\n{}", + "Different signatures generated between router and router-bridge.\nQuery:\n{}\nRouter:\n{}\nRouter Bridge:\n{}", + filtered_query, generated_usage_reporting.result.stats_report_key, usage_reporting.stats_report_key, ); @@ -500,7 +501,8 @@ impl BridgeQueryPlanner { "Mismatch between the Apollo usage report referenced fields generated in router and router-bridge" ); tracing::debug!( - "Different referenced fields generated between router and router-bridge:\n{:?}\n{:?}", + "Different referenced fields generated between router and router-bridge.\nQuery:\n{}\nRouter:\n{:?}\nRouter Bridge:\n{:?}", + filtered_query, generated_usage_reporting.result.referenced_fields_by_type, usage_reporting.referenced_fields_by_type, ); From 32c77ff7c81cf859a158cc8d772c22ddbbd50a59 Mon Sep 17 00:00:00 2001 From: Nick Marsh Date: Mon, 29 Apr 2024 15:04:57 +1000 Subject: [PATCH 2/6] Re-add test --- .../src/apollo_studio_interop/tests.rs | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/apollo-router/src/apollo_studio_interop/tests.rs b/apollo-router/src/apollo_studio_interop/tests.rs index ac2dd78215..5b8c0bd9d9 100644 --- a/apollo-router/src/apollo_studio_interop/tests.rs +++ b/apollo-router/src/apollo_studio_interop/tests.rs @@ -1234,6 +1234,91 @@ async fn test_comma_edge_case() { assert_bridge_results(schema_str, query_str, expected_sig, &expected_refs).await; } +#[test(tokio::test)] +async fn test_nested_fragments() { + let schema_str = include_str!("testdata/schema_interop.graphql"); + + let query_str = " + fragment UnionType1Fragment on UnionType1 { + unionType1Field + } + + fragment ObjectResponseFragment on ObjectTypeResponse { + intField + } + + fragment EverythingResponseFragment on EverythingResponse { + listOfObjects { + ...ObjectResponseFragment + ... on ObjectTypeResponse { + stringField + } + } + } + + query NestedFragmentQuery { + noInputQuery { + ...EverythingResponseFragment + ... on EverythingResponse { + listOfUnions { + ...UnionType1Fragment + ... on UnionType2 { + unionType2Field + } + } + } + } + }"; + + 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("NestedFragmentQuery".into()), &schema); + + let expected_sig = "# NestedFragmentQuery\nfragment EverythingResponseFragment on EverythingResponse{listOfObjects{...ObjectResponseFragment...on ObjectTypeResponse{stringField}}}fragment ObjectResponseFragment on ObjectTypeResponse{intField}fragment UnionType1Fragment on UnionType1{unionType1Field}query NestedFragmentQuery{noInputQuery{...EverythingResponseFragment...on EverythingResponse{listOfUnions{...UnionType1Fragment...on UnionType2{unionType2Field}}}}}"; + let expected_refs: HashMap = HashMap::from([ + ( + "Query".into(), + ReferencedFieldsForType { + field_names: vec!["noInputQuery".into()], + is_interface: false, + }, + ), + ( + "EverythingResponse".into(), + ReferencedFieldsForType { + field_names: vec!["listOfObjects".into(), "listOfUnions".into()], + is_interface: false, + }, + ), + ( + "ObjectTypeResponse".into(), + ReferencedFieldsForType { + field_names: vec!["intField".into(), "stringField".into()], + is_interface: false, + }, + ), + ( + "UnionType1".into(), + ReferencedFieldsForType { + field_names: vec!["unionType1Field".into()], + is_interface: false, + }, + ), + ( + "UnionType2".into(), + ReferencedFieldsForType { + field_names: vec!["unionType2Field".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 { From 507c802b6d1ba85c35b4706f339952fcb3288d4b Mon Sep 17 00:00:00 2001 From: Nick Marsh Date: Mon, 29 Apr 2024 15:05:31 +1000 Subject: [PATCH 3/6] Fix tagging of metrics --- apollo-router/src/query_planner/bridge_query_planner.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apollo-router/src/query_planner/bridge_query_planner.rs b/apollo-router/src/query_planner/bridge_query_planner.rs index 72bd3c5d01..2604ab58bc 100644 --- a/apollo-router/src/query_planner/bridge_query_planner.rs +++ b/apollo-router/src/query_planner/bridge_query_planner.rs @@ -715,7 +715,7 @@ impl BridgeQueryPlanner { "apollo.router.operations.telemetry.studio.signature", "The match status of the Apollo reporting signature generated by the JS implementation vs the Rust implementation", 1, - "generation.is_matched" = "false" + "generation.is_matched" = false ); tracing::debug!( "Different signatures generated between router and router-bridge.\nQuery:\n{}\nRouter:\n{}\nRouter Bridge:\n{}", @@ -728,7 +728,7 @@ impl BridgeQueryPlanner { "apollo.router.operations.telemetry.studio.signature", "The match status of the Apollo reporting signature generated by the JS implementation vs the Rust implementation", 1, - "generation.is_matched" = "true" + "generation.is_matched" = true ); } @@ -741,7 +741,7 @@ impl BridgeQueryPlanner { "apollo.router.operations.telemetry.studio.references", "The match status of the Apollo reporting references generated by the JS implementation vs the Rust implementation", 1, - "generation.is_matched" = "false" + "generation.is_matched" = false ); tracing::debug!( "Different referenced fields generated between router and router-bridge.\nQuery:\n{}\nRouter:\n{:?}\nRouter Bridge:\n{:?}", @@ -754,7 +754,7 @@ impl BridgeQueryPlanner { "apollo.router.operations.telemetry.studio.references", "The match status of the Apollo reporting references generated by the JS implementation vs the Rust implementation", 1, - "generation.is_matched" = "true" + "generation.is_matched" = true ); } } else if matches!( From 8937aa542f08f20c79f530c11a97c1311fa9af98 Mon Sep 17 00:00:00 2001 From: Nick Marsh Date: Mon, 29 Apr 2024 16:31:07 +1000 Subject: [PATCH 4/6] Fix more comma edge cases --- .../src/apollo_studio_interop/mod.rs | 8 +- .../testdata/schema_interop.graphql | 22 +++++ .../src/apollo_studio_interop/tests.rs | 80 ++++++++++++++++++- 3 files changed, 105 insertions(+), 5 deletions(-) diff --git a/apollo-router/src/apollo_studio_interop/mod.rs b/apollo-router/src/apollo_studio_interop/mod.rs index 794a990e6e..e240f309b6 100644 --- a/apollo-router/src/apollo_studio_interop/mod.rs +++ b/apollo-router/src/apollo_studio_interop/mod.rs @@ -415,9 +415,9 @@ fn format_field(field: &Node, 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 = sorted_args .iter() .map(|a| ApolloReportingSignatureFormatter::Argument(a).to_string()) @@ -425,7 +425,7 @@ fn format_field(field: &Node, f: &mut fmt::Formatter) -> fmt::Result { // 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::() + (arg_strings.len() * 2); + field.name.len() + 2 + arg_strings.iter().map(|s| s.len()).sum::() + (arg_strings.len() * 2); let separator = if original_line_length > 80 { " " } else { "," }; for (index, arg_string) in arg_strings.iter().enumerate() { diff --git a/apollo-router/src/apollo_studio_interop/testdata/schema_interop.graphql b/apollo-router/src/apollo_studio_interop/testdata/schema_interop.graphql index e41e500782..4f598b9497 100644 --- a/apollo-router/src/apollo_studio_interop/testdata/schema_interop.graphql +++ b/apollo-router/src/apollo_studio_interop/testdata/schema_interop.graphql @@ -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) { diff --git a/apollo-router/src/apollo_studio_interop/tests.rs b/apollo-router/src/apollo_studio_interop/tests.rs index 5b8c0bd9d9..e616c8a61b 100644 --- a/apollo-router/src/apollo_studio_interop/tests.rs +++ b/apollo-router/src/apollo_studio_interop/tests.rs @@ -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 = 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 = 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 { From 0423a97fc2e5da6df5561a0457361677662e0380 Mon Sep 17 00:00:00 2001 From: Nick Marsh Date: Tue, 30 Apr 2024 17:08:13 +1000 Subject: [PATCH 5/6] More signature fixes --- .../src/apollo_studio_interop/mod.rs | 22 ++-- .../testdata/schema_interop.graphql | 11 ++ .../src/apollo_studio_interop/tests.rs | 121 ++++++++++++++++++ 3 files changed, 146 insertions(+), 8 deletions(-) diff --git a/apollo-router/src/apollo_studio_interop/mod.rs b/apollo-router/src/apollo_studio_interop/mod.rs index e240f309b6..c0dcd81861 100644 --- a/apollo-router/src/apollo_studio_interop/mod.rs +++ b/apollo-router/src/apollo_studio_interop/mod.rs @@ -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, 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 = 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::() + (arg_strings.len() * 2); + field.name.len() + 2 + arg_strings.iter().map(|s| s.len()).sum::() + 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, 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)?; } diff --git a/apollo-router/src/apollo_studio_interop/testdata/schema_interop.graphql b/apollo-router/src/apollo_studio_interop/testdata/schema_interop.graphql index 4f598b9497..1467d12abe 100644 --- a/apollo-router/src/apollo_studio_interop/testdata/schema_interop.graphql +++ b/apollo-router/src/apollo_studio_interop/testdata/schema_interop.graphql @@ -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) { diff --git a/apollo-router/src/apollo_studio_interop/tests.rs b/apollo-router/src/apollo_studio_interop/tests.rs index e616c8a61b..f53b740f16 100644 --- a/apollo-router/src/apollo_studio_interop/tests.rs +++ b/apollo-router/src/apollo_studio_interop/tests.rs @@ -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 = 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 = 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 = 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 { From 60e56fa4b6f4d272d9ccaa6ed83c623de374c880 Mon Sep 17 00:00:00 2001 From: Gary Pennington Date: Tue, 30 Apr 2024 10:52:02 +0100 Subject: [PATCH 6/6] Fix lint complaints --- apollo-router/src/apollo_studio_interop/mod.rs | 11 +++++++---- apollo-router/src/apollo_studio_interop/tests.rs | 4 +--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/apollo-router/src/apollo_studio_interop/mod.rs b/apollo-router/src/apollo_studio_interop/mod.rs index c0dcd81861..7b4e9caa73 100644 --- a/apollo-router/src/apollo_studio_interop/mod.rs +++ b/apollo-router/src/apollo_studio_interop/mod.rs @@ -417,7 +417,7 @@ fn format_field(field: &Node, 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 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 + + // 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 = sorted_args .iter() @@ -428,10 +428,13 @@ fn format_field(field: &Node, f: &mut fmt::Formatter) -> fmt::Result { // * 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 + // * 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::() + arg_strings.len() + ((arg_strings.len() - 1) * 2); + let original_line_length = field.name.len() + + 2 + + arg_strings.iter().map(|s| s.len()).sum::() + + 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() { diff --git a/apollo-router/src/apollo_studio_interop/tests.rs b/apollo-router/src/apollo_studio_interop/tests.rs index f53b740f16..06f526d909 100644 --- a/apollo-router/src/apollo_studio_interop/tests.rs +++ b/apollo-router/src/apollo_studio_interop/tests.rs @@ -1475,7 +1475,6 @@ async fn test_comma_upper_bound() { 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"); @@ -1493,8 +1492,7 @@ async fn test_underscore() { 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 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 = HashMap::from([