federation/composition: fix coercion rules when we expand supergraphs

apollo-federation/src/compat.rs

@@ -11,9 +11,11 @@ use apollo_compiler::ExecutableDocument;
 use apollo_compiler::Name;
 use apollo_compiler::Node;
 use apollo_compiler::Schema;
+use apollo_compiler::ast::DirectiveDefinition;
 use apollo_compiler::ast::Value;
 use apollo_compiler::collections::IndexMap;
 use apollo_compiler::executable;
+use apollo_compiler::schema;
 use apollo_compiler::schema::Directive;
 use apollo_compiler::schema::ExtendedType;
 use apollo_compiler::schema::InputValueDefinition;
@@ -53,7 +55,7 @@ fn standardize_deprecated(directive: &mut Directive) {
 }
 
 /// Retain only semantic directives in a directive list from the high-level schema representation.
-fn retain_semantic_directives(directives: &mut apollo_compiler::schema::DirectiveList) {
+fn retain_semantic_directives(directives: &mut schema::DirectiveList) {
     directives
         .0
         .retain(|directive| is_semantic_directive_application(directive));
@@ -178,6 +180,13 @@ fn coerce_value(
                 coerce_value(types, element, ty.item_type())?;
             }
         }
+        // Coerce single-element list to a non-list type.
+        // This is the reverse of the scalar-to-list coercion below.
+        (Value::List(list), Some(_)) if !ty.is_list() && list.len() == 1 => {
+            let element = list.pop().unwrap();
+            *target.make_mut() = element.as_ref().clone();
+            coerce_value(types, target, ty)?;
+        }
         // Coerce single values (except null) to a list.
         (
             Value::Object(_)
@@ -287,6 +296,63 @@ pub(crate) fn coerce_schema_default_values(schema: &mut Schema) {
     }
 }
 
+pub(crate) fn coerce_schema_values(schema: &mut Schema) {
+    // Keep a copy of the types in the schema so we can mutate the schema while walking it.
+    let types = schema.types.clone();
+
+    let directive_definitions = schema.directive_definitions.clone();
+
+    for ty in schema.types.values_mut() {
+        match ty {
+            ExtendedType::Object(object) => {
+                let object = object.make_mut();
+                coerce_directive_application_values_schema(
+                    &directive_definitions,
+                    &types,
+                    &mut object.directives,
+                );
+                for field in object.fields.values_mut() {
+                    let field = field.make_mut();
+                    coerce_arguments_default_values(&types, &mut field.arguments);
+                    coerce_directive_application_values_ast(
+                        &directive_definitions,
+                        &types,
+                        &mut field.directives,
+                    );
+                }
+            }
+            ExtendedType::Interface(interface) => {
+                let interface = interface.make_mut();
+                for field in interface.fields.values_mut() {
+                    let field = field.make_mut();
+                    coerce_arguments_default_values(&types, &mut field.arguments);
+                }
+            }
+            ExtendedType::InputObject(input_object) => {
+                let input_object = input_object.make_mut();
+                for field in input_object.fields.values_mut() {
+                    let field = field.make_mut();
+                    let Some(default_value) = &mut field.default_value else {
+                        continue;
+                    };
+
+                    if coerce_value(&types, default_value, &field.ty).is_err() {
+                        field.default_value = None;
+                    }
+                }
+            }
+            ExtendedType::Union(_) | ExtendedType::Scalar(_) | ExtendedType::Enum(_) => {
+                // Nothing to do
+            }
+        }
+    }
+
+    for directive in schema.directive_definitions.values_mut() {
+        let directive = directive.make_mut();
+        coerce_arguments_default_values(&types, &mut directive.arguments);
+    }
+}
+
 fn coerce_directive_application_values(
     schema: &Valid<Schema>,
     directives: &mut executable::DirectiveList,
@@ -306,6 +372,46 @@ fn coerce_directive_application_values(
     }
 }
 
+fn coerce_directive_application_values_schema(
+    directive_definitions: &IndexMap<Name, Node<DirectiveDefinition>>,
+    type_definitions: &IndexMap<Name, ExtendedType>,
+    directives: &mut schema::DirectiveList,
+) {
+    for directive in directives {
+        let Some(definition) = directive_definitions.get(&directive.name) else {
+            continue;
+        };
+        let directive = directive.make_mut();
+        for arg in &mut directive.arguments {
+            let Some(definition) = definition.argument_by_name(&arg.name) else {
+                continue;
+            };
+            let arg = arg.make_mut();
+            _ = coerce_value(type_definitions, &mut arg.value, &definition.ty);
+        }
+    }
+}
+
+fn coerce_directive_application_values_ast(
+    directive_definitions: &IndexMap<Name, Node<DirectiveDefinition>>,
+    type_definitions: &IndexMap<Name, ExtendedType>,
+    directives: &mut apollo_compiler::ast::DirectiveList,
+) {
+    for directive in directives {
+        let Some(definition) = directive_definitions.get(&directive.name) else {
+            continue;
+        };
+        let directive = directive.make_mut();
+        for arg in &mut directive.arguments {
+            let Some(definition) = definition.argument_by_name(&arg.name) else {
+                continue;
+            };
+            let arg = arg.make_mut();
+            _ = coerce_value(type_definitions, &mut arg.value, &definition.ty);
+        }
+    }
+}
+
 fn coerce_selection_set_values(
     schema: &Valid<Schema>,
     selection_set: &mut executable::SelectionSet,

apollo-federation/src/subgraph/typestate.rs

@@ -16,6 +16,7 @@ use tracing::trace;
 use crate::LinkSpecDefinition;
 use crate::ValidFederationSchema;
 use crate::bail;
+use crate::compat::coerce_schema_values;
 use crate::ensure;
 use crate::error::FederationError;
 use crate::error::Locations;
@@ -224,6 +225,9 @@ impl Subgraph<Initial> {
         // Simulate graphql-js behavior accepting duplicate argument definitions.
         parser_backward_compatibility::remove_duplicate_arguments(&mut schema);
 
+        // Coerce directive argument values based on directive definitions.
+        coerce_schema_values(&mut schema);
+
         Self::new(name, url, schema, orphan_extension_types)
     }
 

apollo-federation/tests/composition/compose_directive.rs

@@ -1089,7 +1089,7 @@ mod composition {
             directive @auth(scope: [String!]) repeatable on FIELD_DEFINITION
 
             type Query {
-              shared: String @shareable @auth(scope: "VIEWER")
+              shared: String @shareable @auth(scope: ["VIEWER"])
             }
         "#).unwrap();
         let subgraph_b = Subgraph::parse("subgraphB", "", r#"
@@ -1119,7 +1119,10 @@ mod composition {
             "Expected 2 @auth directives on Query.shared"
         );
 
-        assert_eq!(auth_directives[0].to_string(), r#"@auth(scope: "VIEWER")"#);
+        assert_eq!(
+            auth_directives[0].to_string(),
+            r#"@auth(scope: ["VIEWER"])"#
+        );
         assert_eq!(auth_directives[1].to_string(), "@auth");
     }
 }

apollo-federation/tests/composition/directive_argument_merge_strategies.rs

@@ -122,7 +122,7 @@ mod tests {
 
             type T
               @key(fields: "k")
-              @requiresScopes(scopes: ["foo", "bar"])
+              @requiresScopes(scopes: [["foo"], ["bar"]])
             {
               k: ID @requiresScopes(scopes: [])
             }
@@ -134,11 +134,11 @@ mod tests {
             type_defs: r#"
             type T
               @key(fields: "k")
-              @requiresScopes(scopes: ["foo"])
+              @requiresScopes(scopes: [["foo"]])
             {
-              k: ID @requiresScopes(scopes: ["v1", "v2"])
+              k: ID @requiresScopes(scopes: [["v1"], ["v2"]])
               a: Int
-              b: String @requiresScopes(scopes: ["x"])
+              b: String @requiresScopes(scopes: [["x"]])
             }
             "#,
         };
@@ -182,7 +182,7 @@ mod tests {
             .expect("@requiresScopes directive should be present on T");
         assert_eq!(
             t_requires_scopes_directive.to_string(),
-            r#"@requiresScopes(scopes: ["foo", "bar"])"#
+            r#"@requiresScopes(scopes: [["foo"], ["bar"]])"#
         );
 
         let k = coord!(T.k)
@@ -195,7 +195,7 @@ mod tests {
             .expect("@requiresScopes directive should be present on T.k");
         assert_eq!(
             k_requires_scopes_directive.to_string(),
-            r#"@requiresScopes(scopes: ["v1", "v2"])"#
+            r#"@requiresScopes(scopes: [["v1"], ["v2"]])"#
         );
 
         let b = coord!(T.b)
@@ -208,7 +208,7 @@ mod tests {
             .expect("@requiresScopes directive should be present on T.b");
         assert_eq!(
             b_requires_scopes_directive.to_string(),
-            r#"@requiresScopes(scopes: ["x"])"#
+            r#"@requiresScopes(scopes: [["x"]])"#
         )
     }
 

apollo-federation/tests/subgraph/coercion_tests.rs

@@ -0,0 +1,65 @@
+use apollo_federation::subgraph::test_utils::build_and_validate;
+
+#[test]
+fn coerces_directive_argument_values() {
+    // Test that directive argument values are coerced correctly.
+    let schema = r#"
+        extend schema @link(url: "https://specs.apollo.dev/federation/v2.0", import: ["@key"])
+
+        type Query {
+            test: T!
+        }
+
+        type T @key(fields: "id") {
+            id: ID!
+            x: Int!
+        }
+    "#;
+
+    let _subgraph = build_and_validate(schema);
+    // Success: schema validated after coercion
+}
+
+#[test]
+fn coerces_field_argument_default_values() {
+    // Test that field argument default values are coerced correctly.
+    // The field argument expects String! but the default is a list ["id"]
+    // which should be coerced to "id".
+    let schema = r#"
+        extend schema @link(url: "https://specs.apollo.dev/federation/v2.0", import: ["@key"])
+
+        type Query {
+            test: T!
+        }
+
+        type T @key(fields: "id") {
+            id: ID!
+            name(arg: String! = ["id"]): String!
+            x: Int!
+        }
+    "#;
+
+    let _subgraph = build_and_validate(schema);
+    // Success: schema validated after coercion
+}
+
+#[test]
+fn coerces_input_field_default_values() {
+    // Test that input object field default values are coerced correctly.
+    // - `name` has an enum-like default value `Anonymous` which should be coerced for custom scalars
+    // - `age` expects Int but the default is a list [18] which should be coerced
+    let schema = r#"
+        extend schema @link(url: "https://specs.apollo.dev/federation/v2.0", import: ["@key"])
+
+        type Query {
+            test(input: UserInput): String
+        }
+
+        input UserInput {
+            name: String = Anonymous
+            age: Int = [18]
+        }
+    "#;
+    let _subgraph = build_and_validate(schema);
+    // Success: schema validated after coercion
+}

apollo-federation/tests/subgraph/mod.rs

@@ -1,2 +1,3 @@
+mod coercion_tests;
 mod parse_expand_tests;
 mod subgraph_validation_tests;