diff --git a/.changesets/fix_tninesling_cost_name_handling.md b/.changesets/fix_tninesling_cost_name_handling.md new file mode 100644 index 0000000000..fe911c43ed --- /dev/null +++ b/.changesets/fix_tninesling_cost_name_handling.md @@ -0,0 +1,22 @@ +### Ensure cost directives are picked up when not explicitly imported ([PR #6328](https://github.com/apollographql/router/pull/6328)) + +With the recent composition changes, importing `@cost` results in a supergraph schema with the cost specification import at the top. The `@cost` directive itself is not explicitly imported, as it's expected to be available as the default export from the cost link. In contrast, uses of `@listSize` to translate to an explicit import in the supergraph. + +Old SDL link + +``` +@link( + url: "https://specs.apollo.dev/cost/v0.1" + import: ["@cost", "@listSize"] +) +``` + +New SDL link + +``` +@link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"]) +``` + +Instead of using the directive names from the import list in the link, the directive names now come from `SpecDefinition::directive_name_in_schema`, which is equivalent to the change we made on the composition side. + +By [@tninesling](https://github.com/tninesling) in https://github.com/apollographql/router/pull/6328 diff --git a/apollo-federation/src/link/cost_spec_definition.rs b/apollo-federation/src/link/cost_spec_definition.rs index 9e2aed07cc..bd0894c392 100644 --- a/apollo-federation/src/link/cost_spec_definition.rs +++ b/apollo-federation/src/link/cost_spec_definition.rs @@ -1,16 +1,20 @@ +use std::collections::HashSet; + use apollo_compiler::ast::Argument; use apollo_compiler::ast::Directive; -use apollo_compiler::collections::IndexMap; +use apollo_compiler::ast::DirectiveList; +use apollo_compiler::ast::FieldDefinition; +use apollo_compiler::ast::InputValueDefinition; use apollo_compiler::name; use apollo_compiler::schema::Component; -use apollo_compiler::schema::EnumType; -use apollo_compiler::schema::ObjectType; -use apollo_compiler::schema::ScalarType; +use apollo_compiler::schema::ExtendedType; use apollo_compiler::Name; use apollo_compiler::Node; use lazy_static::lazy_static; use crate::error::FederationError; +use crate::internal_error; +use crate::link::federation_spec_definition::get_federation_spec_definition_from_subgraph; use crate::link::spec::Identity; use crate::link::spec::Url; use crate::link::spec::Version; @@ -21,14 +25,17 @@ use crate::schema::position::ObjectTypeDefinitionPosition; use crate::schema::position::ScalarTypeDefinitionPosition; use crate::schema::FederationSchema; -pub(crate) const COST_DIRECTIVE_NAME_IN_SPEC: Name = name!("cost"); -pub(crate) const COST_DIRECTIVE_NAME_DEFAULT: Name = name!("federation__cost"); - -pub(crate) const LIST_SIZE_DIRECTIVE_NAME_IN_SPEC: Name = name!("listSize"); -pub(crate) const LIST_SIZE_DIRECTIVE_NAME_DEFAULT: Name = name!("federation__listSize"); +const COST_DIRECTIVE_NAME: Name = name!("cost"); +const COST_DIRECTIVE_WEIGHT_ARGUMENT_NAME: Name = name!("weight"); +const LIST_SIZE_DIRECTIVE_NAME: Name = name!("listSize"); +const LIST_SIZE_DIRECTIVE_ASSUMED_SIZE_ARGUMENT_NAME: Name = name!("assumedSize"); +const LIST_SIZE_DIRECTIVE_SLICING_ARGUMENTS_ARGUMENT_NAME: Name = name!("slicingArguments"); +const LIST_SIZE_DIRECTIVE_SIZED_FIELDS_ARGUMENT_NAME: Name = name!("sizedFields"); +const LIST_SIZE_DIRECTIVE_REQUIRE_ONE_SLICING_ARGUMENT_ARGUMENT_NAME: Name = + name!("requireOneSlicingArgument"); #[derive(Clone)] -pub(crate) struct CostSpecDefinition { +pub struct CostSpecDefinition { url: Url, minimum_federation_version: Option, } @@ -36,27 +43,24 @@ pub(crate) struct CostSpecDefinition { macro_rules! propagate_demand_control_directives { ($func_name:ident, $directives_ty:ty, $wrap_ty:expr) => { pub(crate) fn $func_name( - &self, - subgraph_schema: &FederationSchema, + supergraph_schema: &FederationSchema, source: &$directives_ty, + subgraph_schema: &FederationSchema, dest: &mut $directives_ty, - original_directive_names: &IndexMap, ) -> Result<(), FederationError> { - let cost_directive_name = original_directive_names.get(&COST_DIRECTIVE_NAME_IN_SPEC); - let cost_directive = cost_directive_name.and_then(|name| source.get(name.as_str())); + let cost_directive = Self::cost_directive_name(supergraph_schema)? + .and_then(|name| source.get(name.as_str())); if let Some(cost_directive) = cost_directive { - dest.push($wrap_ty(self.cost_directive( + dest.push($wrap_ty(Self::cost_directive( subgraph_schema, cost_directive.arguments.clone(), )?)); } - let list_size_directive_name = - original_directive_names.get(&LIST_SIZE_DIRECTIVE_NAME_IN_SPEC); - let list_size_directive = - list_size_directive_name.and_then(|name| source.get(name.as_str())); + let list_size_directive = Self::list_size_directive_name(supergraph_schema)? + .and_then(|name| source.get(name.as_str())); if let Some(list_size_directive) = list_size_directive { - dest.push($wrap_ty(self.list_size_directive( + dest.push($wrap_ty(Self::list_size_directive( subgraph_schema, list_size_directive.arguments.clone(), )?)); @@ -68,34 +72,31 @@ macro_rules! propagate_demand_control_directives { } macro_rules! propagate_demand_control_directives_to_position { - ($func_name:ident, $source_ty:ty, $dest_ty:ty) => { + ($func_name:ident, $source_ty:ty, $pos_ty:ty) => { pub(crate) fn $func_name( - &self, + supergraph_schema: &FederationSchema, subgraph_schema: &mut FederationSchema, - source: &Node<$source_ty>, - dest: &$dest_ty, - original_directive_names: &IndexMap, + pos: &$pos_ty, ) -> Result<(), FederationError> { - let cost_directive_name = original_directive_names.get(&COST_DIRECTIVE_NAME_IN_SPEC); - let cost_directive = - cost_directive_name.and_then(|name| source.directives.get(name.as_str())); + let source = pos.get(supergraph_schema.schema())?; + let cost_directive = Self::cost_directive_name(supergraph_schema)? + .and_then(|name| source.directives.get(name.as_str())); if let Some(cost_directive) = cost_directive { - dest.insert_directive( + pos.insert_directive( subgraph_schema, - Component::from( - self.cost_directive(subgraph_schema, cost_directive.arguments.clone())?, - ), + Component::from(Self::cost_directive( + subgraph_schema, + cost_directive.arguments.clone(), + )?), )?; } - let list_size_directive_name = - original_directive_names.get(&LIST_SIZE_DIRECTIVE_NAME_IN_SPEC); - let list_size_directive = - list_size_directive_name.and_then(|name| source.directives.get(name.as_str())); + let list_size_directive = Self::list_size_directive_name(supergraph_schema)? + .and_then(|name| source.directives.get(name.as_str())); if let Some(list_size_directive) = list_size_directive { - dest.insert_directive( + pos.insert_directive( subgraph_schema, - Component::from(self.list_size_directive( + Component::from(Self::list_size_directive( subgraph_schema, list_size_directive.arguments.clone(), )?), @@ -119,35 +120,32 @@ impl CostSpecDefinition { } pub(crate) fn cost_directive( - &self, schema: &FederationSchema, arguments: Vec>, ) -> Result { - let name = self - .directive_name_in_schema(schema, &COST_DIRECTIVE_NAME_IN_SPEC)? - .unwrap_or(COST_DIRECTIVE_NAME_DEFAULT); + let name = Self::cost_directive_name(schema)?.ok_or_else(|| { + internal_error!("The \"@cost\" directive is undefined in the target schema") + })?; Ok(Directive { name, arguments }) } pub(crate) fn list_size_directive( - &self, schema: &FederationSchema, arguments: Vec>, ) -> Result { - let name = self - .directive_name_in_schema(schema, &LIST_SIZE_DIRECTIVE_NAME_IN_SPEC)? - .unwrap_or(LIST_SIZE_DIRECTIVE_NAME_DEFAULT); + let name = Self::list_size_directive_name(schema)?.ok_or_else(|| { + internal_error!("The \"@listSize\" directive is undefined in the target schema") + })?; Ok(Directive { name, arguments }) } propagate_demand_control_directives!( propagate_demand_control_directives, - apollo_compiler::ast::DirectiveList, + DirectiveList, Node::new ); - propagate_demand_control_directives_to_position!( propagate_demand_control_directives_for_enum, EnumType, @@ -163,6 +161,81 @@ impl CostSpecDefinition { ScalarType, ScalarTypeDefinitionPosition ); + + fn for_federation_schema(schema: &FederationSchema) -> Option<&'static Self> { + let link = schema + .metadata()? + .for_identity(&Identity::cost_identity())?; + COST_VERSIONS.find(&link.url.version) + } + + /// Returns the name of the `@cost` directive in the given schema, accounting for import aliases or specification name + /// prefixes such as `@federation__cost`. This checks the linked cost specification, if there is one, and falls back + /// to the federation spec. + fn cost_directive_name(schema: &FederationSchema) -> Result, FederationError> { + if let Some(spec) = Self::for_federation_schema(schema) { + spec.directive_name_in_schema(schema, &COST_DIRECTIVE_NAME) + } else if let Ok(fed_spec) = get_federation_spec_definition_from_subgraph(schema) { + fed_spec.directive_name_in_schema(schema, &COST_DIRECTIVE_NAME) + } else { + Ok(None) + } + } + + /// Returns the name of the `@listSize` directive in the given schema, accounting for import aliases or specification name + /// prefixes such as `@federation__listSize`. This checks the linked cost specification, if there is one, and falls back + /// to the federation spec. + fn list_size_directive_name( + schema: &FederationSchema, + ) -> Result, FederationError> { + if let Some(spec) = Self::for_federation_schema(schema) { + spec.directive_name_in_schema(schema, &LIST_SIZE_DIRECTIVE_NAME) + } else if let Ok(fed_spec) = get_federation_spec_definition_from_subgraph(schema) { + fed_spec.directive_name_in_schema(schema, &LIST_SIZE_DIRECTIVE_NAME) + } else { + Ok(None) + } + } + + pub fn cost_directive_from_argument( + schema: &FederationSchema, + argument: &InputValueDefinition, + ty: &ExtendedType, + ) -> Result, FederationError> { + let directive_name = Self::cost_directive_name(schema)?; + if let Some(name) = directive_name.as_ref() { + Ok(CostDirective::from_directives(name, &argument.directives) + .or(CostDirective::from_schema_directives(name, ty.directives()))) + } else { + Ok(None) + } + } + + pub fn cost_directive_from_field( + schema: &FederationSchema, + field: &FieldDefinition, + ty: &ExtendedType, + ) -> Result, FederationError> { + let directive_name = Self::cost_directive_name(schema)?; + if let Some(name) = directive_name.as_ref() { + Ok(CostDirective::from_directives(name, &field.directives) + .or(CostDirective::from_schema_directives(name, ty.directives()))) + } else { + Ok(None) + } + } + + pub fn list_size_directive_from_field_definition( + schema: &FederationSchema, + field: &FieldDefinition, + ) -> Result, FederationError> { + let directive_name = Self::list_size_directive_name(schema)?; + if let Some(name) = directive_name.as_ref() { + Ok(ListSizeDirective::from_field_definition(name, field)) + } else { + Ok(None) + } + } } impl SpecDefinition for CostSpecDefinition { @@ -185,3 +258,96 @@ lazy_static! { definitions }; } + +pub struct CostDirective { + weight: i32, +} + +impl CostDirective { + pub fn weight(&self) -> f64 { + self.weight as f64 + } + + fn from_directives(directive_name: &Name, directives: &DirectiveList) -> Option { + directives + .get(directive_name)? + .specified_argument_by_name(&COST_DIRECTIVE_WEIGHT_ARGUMENT_NAME)? + .to_i32() + .map(|weight| Self { weight }) + } + + fn from_schema_directives( + directive_name: &Name, + directives: &apollo_compiler::schema::DirectiveList, + ) -> Option { + directives + .get(directive_name)? + .specified_argument_by_name(&COST_DIRECTIVE_WEIGHT_ARGUMENT_NAME)? + .to_i32() + .map(|weight| Self { weight }) + } +} + +pub struct ListSizeDirective { + pub assumed_size: Option, + pub slicing_argument_names: Option>, + pub sized_fields: Option>, + pub require_one_slicing_argument: bool, +} + +impl ListSizeDirective { + pub fn from_field_definition( + directive_name: &Name, + definition: &FieldDefinition, + ) -> Option { + let directive = definition.directives.get(directive_name)?; + let assumed_size = Self::assumed_size(directive); + let slicing_argument_names = Self::slicing_argument_names(directive); + let sized_fields = Self::sized_fields(directive); + let require_one_slicing_argument = + Self::require_one_slicing_argument(directive).unwrap_or(true); + + Some(Self { + assumed_size, + slicing_argument_names, + sized_fields, + require_one_slicing_argument, + }) + } + + fn assumed_size(directive: &Directive) -> Option { + directive + .specified_argument_by_name(&LIST_SIZE_DIRECTIVE_ASSUMED_SIZE_ARGUMENT_NAME)? + .to_i32() + } + + fn slicing_argument_names(directive: &Directive) -> Option> { + let names = directive + .specified_argument_by_name(&LIST_SIZE_DIRECTIVE_SLICING_ARGUMENTS_ARGUMENT_NAME)? + .as_list()? + .iter() + .flat_map(|arg| arg.as_str()) + .map(String::from) + .collect(); + Some(names) + } + + fn sized_fields(directive: &Directive) -> Option> { + let fields = directive + .specified_argument_by_name(&LIST_SIZE_DIRECTIVE_SIZED_FIELDS_ARGUMENT_NAME)? + .as_list()? + .iter() + .flat_map(|arg| arg.as_str()) + .map(String::from) + .collect(); + Some(fields) + } + + fn require_one_slicing_argument(directive: &Directive) -> Option { + directive + .specified_argument_by_name( + &LIST_SIZE_DIRECTIVE_REQUIRE_ONE_SLICING_ARGUMENT_ARGUMENT_NAME, + )? + .to_bool() + } +} diff --git a/apollo-federation/src/link/federation_spec_definition.rs b/apollo-federation/src/link/federation_spec_definition.rs index 5e3e09512c..e89ce9f7ff 100644 --- a/apollo-federation/src/link/federation_spec_definition.rs +++ b/apollo-federation/src/link/federation_spec_definition.rs @@ -14,8 +14,6 @@ use crate::error::SingleFederationError; use crate::link::argument::directive_optional_boolean_argument; use crate::link::argument::directive_optional_string_argument; use crate::link::argument::directive_required_string_argument; -use crate::link::cost_spec_definition::CostSpecDefinition; -use crate::link::cost_spec_definition::COST_VERSIONS; use crate::link::spec::Identity; use crate::link::spec::Url; use crate::link::spec::Version; @@ -539,17 +537,6 @@ impl FederationSpecDefinition { )?, }) } - - pub(crate) fn get_cost_spec_definition( - &self, - schema: &FederationSchema, - ) -> Option<&'static CostSpecDefinition> { - schema - .metadata() - .and_then(|metadata| metadata.for_identity(&Identity::cost_identity())) - .and_then(|link| COST_VERSIONS.find(&link.url.version)) - .or_else(|| COST_VERSIONS.find_for_federation_version(self.version())) - } } impl SpecDefinition for FederationSpecDefinition { diff --git a/apollo-federation/src/link/mod.rs b/apollo-federation/src/link/mod.rs index 971428257d..1f59986d93 100644 --- a/apollo-federation/src/link/mod.rs +++ b/apollo-federation/src/link/mod.rs @@ -23,7 +23,7 @@ use crate::link::spec::Url; pub(crate) mod argument; pub(crate) mod context_spec_definition; -pub(crate) mod cost_spec_definition; +pub mod cost_spec_definition; pub mod database; pub(crate) mod federation_spec_definition; pub(crate) mod graphql_definition; diff --git a/apollo-federation/src/link/spec_definition.rs b/apollo-federation/src/link/spec_definition.rs index 1fb084afe5..5826f8f4d9 100644 --- a/apollo-federation/src/link/spec_definition.rs +++ b/apollo-federation/src/link/spec_definition.rs @@ -182,17 +182,6 @@ impl SpecDefinitions { self.definitions.get(requested) } - pub(crate) fn find_for_federation_version(&self, federation_version: &Version) -> Option<&T> { - for definition in self.definitions.values() { - if let Some(minimum_federation_version) = definition.minimum_federation_version() { - if minimum_federation_version >= federation_version { - return Some(definition); - } - } - } - None - } - pub(crate) fn versions(&self) -> Keys { self.definitions.keys() } diff --git a/apollo-federation/src/supergraph/mod.rs b/apollo-federation/src/supergraph/mod.rs index a5547c27e6..d6d05a5d68 100644 --- a/apollo-federation/src/supergraph/mod.rs +++ b/apollo-federation/src/supergraph/mod.rs @@ -38,7 +38,6 @@ use itertools::Itertools; use lazy_static::lazy_static; use time::OffsetDateTime; -use self::schema::get_apollo_directive_names; pub(crate) use self::schema::new_empty_fed_2_subgraph_schema; use self::subgraph::FederationSubgraph; use self::subgraph::FederationSubgraphs; @@ -265,8 +264,6 @@ fn extract_subgraphs_from_fed_2_supergraph( context_spec_definition: Option<&'static ContextSpecDefinition>, filtered_types: &Vec, ) -> Result<(), FederationError> { - let original_directive_names = get_apollo_directive_names(supergraph_schema)?; - let TypeInfos { object_types, interface_types, @@ -281,7 +278,6 @@ fn extract_subgraphs_from_fed_2_supergraph( join_spec_definition, context_spec_definition, filtered_types, - &original_directive_names, )?; extract_object_type_content( @@ -291,7 +287,6 @@ fn extract_subgraphs_from_fed_2_supergraph( federation_spec_definitions, join_spec_definition, &object_types, - &original_directive_names, )?; extract_interface_type_content( supergraph_schema, @@ -300,7 +295,6 @@ fn extract_subgraphs_from_fed_2_supergraph( federation_spec_definitions, join_spec_definition, &interface_types, - &original_directive_names, )?; extract_union_type_content( supergraph_schema, @@ -313,19 +307,15 @@ fn extract_subgraphs_from_fed_2_supergraph( supergraph_schema, subgraphs, graph_enum_value_name_to_subgraph_name, - federation_spec_definitions, join_spec_definition, &enum_types, - &original_directive_names, )?; extract_input_object_type_content( supergraph_schema, subgraphs, graph_enum_value_name_to_subgraph_name, - federation_spec_definitions, join_spec_definition, &input_object_types, - &original_directive_names, )?; extract_join_directives( @@ -404,7 +394,6 @@ fn add_all_empty_subgraph_types( join_spec_definition: &'static JoinSpecDefinition, context_spec_definition: Option<&'static ContextSpecDefinition>, filtered_types: &Vec, - original_directive_names: &IndexMap, ) -> Result { let type_directive_definition = join_spec_definition.type_directive_definition(supergraph_schema)?; @@ -434,12 +423,6 @@ fn add_all_empty_subgraph_types( graph_enum_value_name_to_subgraph_name, &type_directive_application.graph, )?; - let federation_spec_definition = federation_spec_definitions - .get(&type_directive_application.graph) - .ok_or_else(|| SingleFederationError::InvalidFederationSupergraph { - message: "Subgraph unexpectedly does not use federation spec" - .to_owned(), - })?; pos.pre_insert(&mut subgraph.schema)?; pos.insert( @@ -451,16 +434,11 @@ fn add_all_empty_subgraph_types( }), )?; - if let Some(cost_spec_definition) = - federation_spec_definition.get_cost_spec_definition(&subgraph.schema) - { - cost_spec_definition.propagate_demand_control_directives_for_scalar( - &mut subgraph.schema, - pos.get(supergraph_schema.schema())?, - pos, - original_directive_names, - )?; - } + CostSpecDefinition::propagate_demand_control_directives_for_scalar( + supergraph_schema, + &mut subgraph.schema, + pos, + )?; } None } @@ -740,7 +718,6 @@ fn extract_object_type_content( federation_spec_definitions: &IndexMap, join_spec_definition: &JoinSpecDefinition, info: &[TypeInfo], - original_directive_names: &IndexMap, ) -> Result<(), FederationError> { let field_directive_definition = join_spec_definition.field_directive_definition(supergraph_schema)?; @@ -796,21 +773,12 @@ fn extract_object_type_content( graph_enum_value_name_to_subgraph_name, graph_enum_value, )?; - let federation_spec_definition = federation_spec_definitions - .get(graph_enum_value) - .ok_or_else(|| SingleFederationError::InvalidFederationSupergraph { - message: "Subgraph unexpectedly does not use federation spec".to_owned(), - })?; - if let Some(cost_spec_definition) = - federation_spec_definition.get_cost_spec_definition(&subgraph.schema) - { - cost_spec_definition.propagate_demand_control_directives_for_object( - &mut subgraph.schema, - type_, - &pos, - original_directive_names, - )?; - } + + CostSpecDefinition::propagate_demand_control_directives_for_object( + supergraph_schema, + &mut subgraph.schema, + &pos, + )?; } for (field_name, field) in type_.fields.iter() { @@ -836,17 +804,14 @@ fn extract_object_type_content( message: "Subgraph unexpectedly does not use federation spec" .to_owned(), })?; - let cost_spec_definition = - federation_spec_definition.get_cost_spec_definition(&subgraph.schema); add_subgraph_field( field_pos.clone().into(), field, + supergraph_schema, subgraph, federation_spec_definition, is_shareable, None, - cost_spec_definition, - original_directive_names, )?; } } else { @@ -877,8 +842,6 @@ fn extract_object_type_content( message: "Subgraph unexpectedly does not use federation spec" .to_owned(), })?; - let cost_spec_definition = - federation_spec_definition.get_cost_spec_definition(&subgraph.schema); if !subgraph_info.contains_key(graph_enum_value) { return Err( SingleFederationError::InvalidFederationSupergraph { @@ -894,12 +857,11 @@ fn extract_object_type_content( add_subgraph_field( field_pos.clone().into(), field, + supergraph_schema, subgraph, federation_spec_definition, is_shareable, Some(field_directive_application), - cost_spec_definition, - original_directive_names, )?; } } @@ -916,7 +878,6 @@ fn extract_interface_type_content( federation_spec_definitions: &IndexMap, join_spec_definition: &JoinSpecDefinition, info: &[TypeInfo], - original_directive_names: &IndexMap, ) -> Result<(), FederationError> { let field_directive_definition = join_spec_definition.field_directive_definition(supergraph_schema)?; @@ -1039,17 +1000,14 @@ fn extract_interface_type_content( message: "Subgraph unexpectedly does not use federation spec" .to_owned(), })?; - let cost_spec_definition = - federation_spec_definition.get_cost_spec_definition(&subgraph.schema); add_subgraph_field( pos.field(field_name.clone()), field, + supergraph_schema, subgraph, federation_spec_definition, false, None, - cost_spec_definition, - original_directive_names, )?; } } else { @@ -1073,8 +1031,6 @@ fn extract_interface_type_content( message: "Subgraph unexpectedly does not use federation spec" .to_owned(), })?; - let cost_spec_definition = - federation_spec_definition.get_cost_spec_definition(&subgraph.schema); if !subgraph_info.contains_key(graph_enum_value) { return Err( SingleFederationError::InvalidFederationSupergraph { @@ -1090,12 +1046,11 @@ fn extract_interface_type_content( add_subgraph_field( pos.field(field_name.clone()), field, + supergraph_schema, subgraph, federation_spec_definition, false, Some(field_directive_application), - cost_spec_definition, - original_directive_names, )?; } } @@ -1201,10 +1156,8 @@ fn extract_enum_type_content( supergraph_schema: &FederationSchema, subgraphs: &mut FederationSubgraphs, graph_enum_value_name_to_subgraph_name: &IndexMap>, - federation_spec_definitions: &IndexMap, join_spec_definition: &JoinSpecDefinition, info: &[TypeInfo], - original_directive_names: &IndexMap, ) -> Result<(), FederationError> { // This was added in join 0.3, so it can genuinely be None. let enum_value_directive_definition = @@ -1226,21 +1179,12 @@ fn extract_enum_type_content( graph_enum_value_name_to_subgraph_name, graph_enum_value, )?; - let federation_spec_definition = federation_spec_definitions - .get(graph_enum_value) - .ok_or_else(|| SingleFederationError::InvalidFederationSupergraph { - message: "Subgraph unexpectedly does not use federation spec".to_owned(), - })?; - if let Some(cost_spec_definition) = - federation_spec_definition.get_cost_spec_definition(&subgraph.schema) - { - cost_spec_definition.propagate_demand_control_directives_for_enum( - &mut subgraph.schema, - type_, - &pos, - original_directive_names, - )?; - } + + CostSpecDefinition::propagate_demand_control_directives_for_enum( + supergraph_schema, + &mut subgraph.schema, + &pos, + )?; } for (value_name, value) in type_.values.iter() { @@ -1310,10 +1254,8 @@ fn extract_input_object_type_content( supergraph_schema: &FederationSchema, subgraphs: &mut FederationSubgraphs, graph_enum_value_name_to_subgraph_name: &IndexMap>, - federation_spec_definitions: &IndexMap, join_spec_definition: &JoinSpecDefinition, info: &[TypeInfo], - original_directive_names: &IndexMap, ) -> Result<(), FederationError> { let field_directive_definition = join_spec_definition.field_directive_definition(supergraph_schema)?; @@ -1345,21 +1287,12 @@ fn extract_input_object_type_content( graph_enum_value_name_to_subgraph_name, graph_enum_value, )?; - let federation_spec_definition = federation_spec_definitions - .get(graph_enum_value) - .ok_or_else(|| SingleFederationError::InvalidFederationSupergraph { - message: "Subgraph unexpectedly does not use federation spec" - .to_owned(), - })?; - let cost_spec_definition = - federation_spec_definition.get_cost_spec_definition(&subgraph.schema); add_subgraph_input_field( input_field_pos.clone(), input_field, + supergraph_schema, subgraph, None, - cost_spec_definition, - original_directive_names, )?; } } else { @@ -1375,14 +1308,6 @@ fn extract_input_object_type_content( graph_enum_value_name_to_subgraph_name, graph_enum_value, )?; - let federation_spec_definition = federation_spec_definitions - .get(graph_enum_value) - .ok_or_else(|| SingleFederationError::InvalidFederationSupergraph { - message: "Subgraph unexpectedly does not use federation spec" - .to_owned(), - })?; - let cost_spec_definition = - federation_spec_definition.get_cost_spec_definition(&subgraph.schema); if !subgraph_info.contains_key(graph_enum_value) { return Err( SingleFederationError::InvalidFederationSupergraph { @@ -1398,10 +1323,9 @@ fn extract_input_object_type_content( add_subgraph_input_field( input_field_pos.clone(), input_field, + supergraph_schema, subgraph, Some(field_directive_application), - cost_spec_definition, - original_directive_names, )?; } } @@ -1415,12 +1339,11 @@ fn extract_input_object_type_content( fn add_subgraph_field( object_or_interface_field_definition_position: ObjectOrInterfaceFieldDefinitionPosition, field: &FieldDefinition, + supergraph_schema: &FederationSchema, subgraph: &mut FederationSubgraph, federation_spec_definition: &'static FederationSpecDefinition, is_shareable: bool, field_directive_application: Option<&FieldDirectiveArguments>, - cost_spec_definition: Option<&'static CostSpecDefinition>, - original_directive_names: &IndexMap, ) -> Result<(), FederationError> { let field_directive_application = field_directive_application.unwrap_or_else(|| &FieldDirectiveArguments { @@ -1456,14 +1379,13 @@ fn add_subgraph_field( default_value: argument.default_value.clone(), directives: Default::default(), }; - if let Some(cost_spec_definition) = cost_spec_definition { - cost_spec_definition.propagate_demand_control_directives( - &subgraph.schema, - &argument.directives, - &mut destination_argument.directives, - original_directive_names, - )?; - } + + CostSpecDefinition::propagate_demand_control_directives( + supergraph_schema, + &argument.directives, + &subgraph.schema, + &mut destination_argument.directives, + )?; subgraph_field .arguments @@ -1509,14 +1431,12 @@ fn add_subgraph_field( )); } - if let Some(cost_spec_definition) = cost_spec_definition { - cost_spec_definition.propagate_demand_control_directives( - &subgraph.schema, - &field.directives, - &mut subgraph_field.directives, - original_directive_names, - )?; - } + CostSpecDefinition::propagate_demand_control_directives( + supergraph_schema, + &field.directives, + &subgraph.schema, + &mut subgraph_field.directives, + )?; if let Some(context_arguments) = &field_directive_application.context_arguments { for args in context_arguments { @@ -1563,10 +1483,9 @@ fn add_subgraph_field( fn add_subgraph_input_field( input_object_field_definition_position: InputObjectFieldDefinitionPosition, input_field: &InputValueDefinition, + supergraph_schema: &FederationSchema, subgraph: &mut FederationSubgraph, field_directive_application: Option<&FieldDirectiveArguments>, - cost_spec_definition: Option<&'static CostSpecDefinition>, - original_directive_names: &IndexMap, ) -> Result<(), FederationError> { let field_directive_application = field_directive_application.unwrap_or_else(|| &FieldDirectiveArguments { @@ -1592,14 +1511,12 @@ fn add_subgraph_input_field( directives: Default::default(), }; - if let Some(cost_spec_definition) = cost_spec_definition { - cost_spec_definition.propagate_demand_control_directives( - &subgraph.schema, - &input_field.directives, - &mut subgraph_input_field.directives, - original_directive_names, - )?; - } + CostSpecDefinition::propagate_demand_control_directives( + supergraph_schema, + &input_field.directives, + &subgraph.schema, + &mut subgraph_input_field.directives, + )?; input_object_field_definition_position .insert(&mut subgraph.schema, Component::from(subgraph_input_field))?; diff --git a/apollo-federation/src/supergraph/schema.rs b/apollo-federation/src/supergraph/schema.rs index 46aa19618e..700a52e0e4 100644 --- a/apollo-federation/src/supergraph/schema.rs +++ b/apollo-federation/src/supergraph/schema.rs @@ -1,49 +1,8 @@ -use apollo_compiler::collections::IndexMap; use apollo_compiler::schema::SchemaBuilder; -use apollo_compiler::Name; use crate::error::FederationError; -use crate::link::spec::APOLLO_SPEC_DOMAIN; -use crate::link::Link; use crate::schema::FederationSchema; -/// Builds a map of original name to new name for Apollo feature directives. This is -/// used to handle cases where a directive is renamed via an import statement. For -/// example, importing a directive with a custom name like -/// ```graphql -/// @link(url: "https://specs.apollo.dev/cost/v0.1", import: [{ name: "@cost", as: "@renamedCost" }]) -/// ``` -/// results in a map entry of `cost -> renamedCost` with the `@` prefix removed. -/// -/// If the directive is imported under its default name, that also results in an entry. So, -/// ```graphql -/// @link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@cost"]) -/// ``` -/// results in a map entry of `cost -> cost`. This duals as a way to check if a directive -/// is included in the supergraph schema. -/// -/// **Important:** This map does _not_ include directives imported from identities other -/// than `specs.apollo.dev`. This helps us avoid extracting directives to subgraphs -/// when a custom directive's name conflicts with that of a default one. -pub(super) fn get_apollo_directive_names( - supergraph_schema: &FederationSchema, -) -> Result, FederationError> { - let mut hm: IndexMap = IndexMap::default(); - for directive in &supergraph_schema.schema().schema_definition.directives { - if directive.name.as_str() == "link" { - if let Ok(link) = Link::from_directive_application(directive) { - if link.url.identity.domain != APOLLO_SPEC_DOMAIN { - continue; - } - for import in link.imports { - hm.insert(import.element.clone(), import.imported_name().clone()); - } - } - } - } - Ok(hm) -} - /// TODO: Use the JS/programmatic approach instead of hard-coding definitions. pub(crate) fn new_empty_fed_2_subgraph_schema() -> Result { let builder = SchemaBuilder::new().adopt_orphan_extensions(); diff --git a/apollo-router/src/plugins/demand_control/cost_calculator/directives.rs b/apollo-router/src/plugins/demand_control/cost_calculator/directives.rs index cf819478e1..0b3c6738ae 100644 --- a/apollo-router/src/plugins/demand_control/cost_calculator/directives.rs +++ b/apollo-router/src/plugins/demand_control/cost_calculator/directives.rs @@ -1,114 +1,20 @@ use ahash::HashMap; use ahash::HashMapExt; use ahash::HashSet; -use apollo_compiler::ast::DirectiveList; use apollo_compiler::ast::FieldDefinition; -use apollo_compiler::ast::InputValueDefinition; use apollo_compiler::ast::NamedType; use apollo_compiler::executable::Field; use apollo_compiler::executable::SelectionSet; -use apollo_compiler::name; use apollo_compiler::parser::Parser; -use apollo_compiler::schema::ExtendedType; use apollo_compiler::validation::Valid; -use apollo_compiler::Name; use apollo_compiler::Schema; -use apollo_federation::link::spec::APOLLO_SPEC_DOMAIN; -use apollo_federation::link::Link; +use apollo_federation::link::cost_spec_definition::ListSizeDirective as ParsedListSizeDirective; use tower::BoxError; use crate::json_ext::Object; use crate::json_ext::ValueExt; use crate::plugins::demand_control::DemandControlError; -const COST_DIRECTIVE_NAME: Name = name!("cost"); -const COST_DIRECTIVE_DEFAULT_NAME: Name = name!("federation__cost"); -const COST_DIRECTIVE_WEIGHT_ARGUMENT_NAME: Name = name!("weight"); - -const LIST_SIZE_DIRECTIVE_NAME: Name = name!("listSize"); -const LIST_SIZE_DIRECTIVE_DEFAULT_NAME: Name = name!("federation__listSize"); -const LIST_SIZE_DIRECTIVE_ASSUMED_SIZE_ARGUMENT_NAME: Name = name!("assumedSize"); -const LIST_SIZE_DIRECTIVE_SLICING_ARGUMENTS_ARGUMENT_NAME: Name = name!("slicingArguments"); -const LIST_SIZE_DIRECTIVE_SIZED_FIELDS_ARGUMENT_NAME: Name = name!("sizedFields"); -const LIST_SIZE_DIRECTIVE_REQUIRE_ONE_SLICING_ARGUMENT_ARGUMENT_NAME: Name = - name!("requireOneSlicingArgument"); - -pub(in crate::plugins::demand_control) fn get_apollo_directive_names( - schema: &Schema, -) -> HashMap { - let mut hm: HashMap = HashMap::new(); - for directive in &schema.schema_definition.directives { - if directive.name.as_str() == "link" { - if let Ok(link) = Link::from_directive_application(directive) { - if link.url.identity.domain != APOLLO_SPEC_DOMAIN { - continue; - } - for import in link.imports { - hm.insert(import.element.clone(), import.imported_name().clone()); - } - } - } - } - hm -} - -pub(in crate::plugins::demand_control) struct CostDirective { - weight: i32, -} - -impl CostDirective { - pub(in crate::plugins::demand_control) fn weight(&self) -> f64 { - self.weight as f64 - } - - pub(in crate::plugins::demand_control) fn from_argument( - directive_name_map: &HashMap, - argument: &InputValueDefinition, - ) -> Option { - Self::from_directives(directive_name_map, &argument.directives) - } - - pub(in crate::plugins::demand_control) fn from_field( - directive_name_map: &HashMap, - field: &FieldDefinition, - ) -> Option { - Self::from_directives(directive_name_map, &field.directives) - } - - pub(in crate::plugins::demand_control) fn from_type( - directive_name_map: &HashMap, - ty: &ExtendedType, - ) -> Option { - Self::from_schema_directives(directive_name_map, ty.directives()) - } - - fn from_directives( - directive_name_map: &HashMap, - directives: &DirectiveList, - ) -> Option { - directive_name_map - .get(&COST_DIRECTIVE_NAME) - .and_then(|name| directives.get(name)) - .or(directives.get(&COST_DIRECTIVE_DEFAULT_NAME)) - .and_then(|cost| cost.specified_argument_by_name(&COST_DIRECTIVE_WEIGHT_ARGUMENT_NAME)) - .and_then(|weight| weight.to_i32()) - .map(|weight| Self { weight }) - } - - pub(in crate::plugins::demand_control) fn from_schema_directives( - directive_name_map: &HashMap, - directives: &apollo_compiler::schema::DirectiveList, - ) -> Option { - directive_name_map - .get(&COST_DIRECTIVE_NAME) - .and_then(|name| directives.get(name)) - .or(directives.get(&COST_DIRECTIVE_DEFAULT_NAME)) - .and_then(|cost| cost.specified_argument_by_name(&COST_DIRECTIVE_WEIGHT_ARGUMENT_NAME)) - .and_then(|weight| weight.to_i32()) - .map(|weight| Self { weight }) - } -} - pub(in crate::plugins::demand_control) struct IncludeDirective { pub(in crate::plugins::demand_control) is_included: bool, } @@ -134,86 +40,13 @@ pub(in crate::plugins::demand_control) struct ListSizeDirective<'schema> { } impl<'schema> ListSizeDirective<'schema> { - pub(in crate::plugins::demand_control) fn size_of(&self, field: &Field) -> Option { - if self - .sized_fields - .as_ref() - .is_some_and(|sf| sf.contains(field.name.as_str())) - { - self.expected_size - } else { - None - } - } -} - -/// The `@listSize` directive from a field definition, which can be converted to -/// `ListSizeDirective` with a concrete field from a request. -pub(in crate::plugins::demand_control) struct DefinitionListSizeDirective { - assumed_size: Option, - slicing_argument_names: Option>, - sized_fields: Option>, - require_one_slicing_argument: bool, -} - -impl DefinitionListSizeDirective { - pub(in crate::plugins::demand_control) fn from_field_definition( - directive_name_map: &HashMap, - definition: &FieldDefinition, - ) -> Result, DemandControlError> { - let directive = directive_name_map - .get(&LIST_SIZE_DIRECTIVE_NAME) - .and_then(|name| definition.directives.get(name)) - .or(definition.directives.get(&LIST_SIZE_DIRECTIVE_DEFAULT_NAME)); - if let Some(directive) = directive { - let assumed_size = directive - .specified_argument_by_name(&LIST_SIZE_DIRECTIVE_ASSUMED_SIZE_ARGUMENT_NAME) - .and_then(|arg| arg.to_i32()); - let slicing_argument_names = directive - .specified_argument_by_name(&LIST_SIZE_DIRECTIVE_SLICING_ARGUMENTS_ARGUMENT_NAME) - .and_then(|arg| arg.as_list()) - .map(|arg_list| { - arg_list - .iter() - .flat_map(|arg| arg.as_str()) - .map(String::from) - .collect() - }); - let sized_fields = directive - .specified_argument_by_name(&LIST_SIZE_DIRECTIVE_SIZED_FIELDS_ARGUMENT_NAME) - .and_then(|arg| arg.as_list()) - .map(|arg_list| { - arg_list - .iter() - .flat_map(|arg| arg.as_str()) - .map(String::from) - .collect() - }); - let require_one_slicing_argument = directive - .specified_argument_by_name( - &LIST_SIZE_DIRECTIVE_REQUIRE_ONE_SLICING_ARGUMENT_ARGUMENT_NAME, - ) - .and_then(|arg| arg.to_bool()) - .unwrap_or(true); - - Ok(Some(Self { - assumed_size, - slicing_argument_names, - sized_fields, - require_one_slicing_argument, - })) - } else { - Ok(None) - } - } - - pub(in crate::plugins::demand_control) fn with_field_and_variables( - &self, + pub(in crate::plugins::demand_control) fn new( + parsed: &'schema ParsedListSizeDirective, field: &Field, variables: &Object, - ) -> Result { + ) -> Result { let mut slicing_arguments: HashMap<&str, i32> = HashMap::new(); - if let Some(slicing_argument_names) = self.slicing_argument_names.as_ref() { + if let Some(slicing_argument_names) = parsed.slicing_argument_names.as_ref() { // First, collect the default values for each argument for argument in &field.definition.arguments { if slicing_argument_names.contains(argument.name.as_str()) { @@ -240,7 +73,7 @@ impl DefinitionListSizeDirective { } } - if self.require_one_slicing_argument && slicing_arguments.len() != 1 { + if parsed.require_one_slicing_argument && slicing_arguments.len() != 1 { return Err(DemandControlError::QueryParseFailure(format!( "Exactly one slicing argument is required, but found {}", slicing_arguments.len() @@ -252,16 +85,28 @@ impl DefinitionListSizeDirective { .values() .max() .cloned() - .or(self.assumed_size); + .or(parsed.assumed_size); - Ok(ListSizeDirective { + Ok(Self { expected_size, - sized_fields: self + sized_fields: parsed .sized_fields .as_ref() .map(|set| set.iter().map(|s| s.as_str()).collect()), }) } + + pub(in crate::plugins::demand_control) fn size_of(&self, field: &Field) -> Option { + if self + .sized_fields + .as_ref() + .is_some_and(|sf| sf.contains(field.name.as_str())) + { + self.expected_size + } else { + None + } + } } pub(in crate::plugins::demand_control) struct RequiresDirective { diff --git a/apollo-router/src/plugins/demand_control/cost_calculator/fixtures/custom_cost_schema.graphql b/apollo-router/src/plugins/demand_control/cost_calculator/fixtures/custom_cost_schema.graphql index d966512be1..02184164a9 100644 --- a/apollo-router/src/plugins/demand_control/cost_calculator/fixtures/custom_cost_schema.graphql +++ b/apollo-router/src/plugins/demand_control/cost_calculator/fixtures/custom_cost_schema.graphql @@ -1,10 +1,7 @@ schema @link(url: "https://specs.apollo.dev/link/v1.0") @link(url: "https://specs.apollo.dev/join/v0.5", for: EXECUTION) - @link( - url: "https://specs.apollo.dev/cost/v0.1" - import: ["@cost", "@listSize"] - ) { + @link(url: "https://specs.apollo.dev/cost/v0.1", import: ["@listSize"]) { query: Query } @@ -12,13 +9,6 @@ directive @cost( weight: Int! ) on ARGUMENT_DEFINITION | ENUM | FIELD_DEFINITION | INPUT_FIELD_DEFINITION | OBJECT | SCALAR -directive @cost__listSize( - assumedSize: Int - slicingArguments: [String!] - sizedFields: [String!] - requireOneSlicingArgument: Boolean = true -) on FIELD_DEFINITION - directive @join__directive( graphs: [join__Graph!] name: String! diff --git a/apollo-router/src/plugins/demand_control/cost_calculator/schema.rs b/apollo-router/src/plugins/demand_control/cost_calculator/schema.rs index 6a46ee9fe9..d59243f4d5 100644 --- a/apollo-router/src/plugins/demand_control/cost_calculator/schema.rs +++ b/apollo-router/src/plugins/demand_control/cost_calculator/schema.rs @@ -3,20 +3,21 @@ use std::sync::Arc; use ahash::HashMap; use ahash::HashMapExt; +use apollo_compiler::ast::InputValueDefinition; use apollo_compiler::schema::ExtendedType; use apollo_compiler::validation::Valid; use apollo_compiler::Name; use apollo_compiler::Schema; +use apollo_federation::link::cost_spec_definition::CostDirective; +use apollo_federation::link::cost_spec_definition::CostSpecDefinition; +use apollo_federation::link::cost_spec_definition::ListSizeDirective; +use apollo_federation::schema::ValidFederationSchema; -use super::directives::get_apollo_directive_names; -use super::directives::CostDirective; -use super::directives::DefinitionListSizeDirective as ListSizeDirective; use super::directives::RequiresDirective; use crate::plugins::demand_control::DemandControlError; pub(crate) struct DemandControlledSchema { - directive_name_map: HashMap, - inner: Arc>, + inner: ValidFederationSchema, type_field_cost_directives: HashMap>, type_field_list_size_directives: HashMap>, type_field_requires_directives: HashMap>, @@ -24,8 +25,7 @@ pub(crate) struct DemandControlledSchema { impl DemandControlledSchema { pub(crate) fn new(schema: Arc>) -> Result { - let directive_name_map = get_apollo_directive_names(&schema); - + let fed_schema = ValidFederationSchema::new((*schema).clone())?; let mut type_field_cost_directives: HashMap> = HashMap::new(); let mut type_field_list_size_directives: HashMap> = @@ -55,17 +55,20 @@ impl DemandControlledSchema { )) })?; - if let Some(cost_directive) = - CostDirective::from_field(&directive_name_map, field_definition) - .or(CostDirective::from_type(&directive_name_map, field_type)) - { + if let Some(cost_directive) = CostSpecDefinition::cost_directive_from_field( + &fed_schema, + field_definition, + field_type, + )? { field_cost_directives.insert(field_name.clone(), cost_directive); } - if let Some(list_size_directive) = ListSizeDirective::from_field_definition( - &directive_name_map, - field_definition, - )? { + if let Some(list_size_directive) = + CostSpecDefinition::list_size_directive_from_field_definition( + &fed_schema, + field_definition, + )? + { field_list_size_directives .insert(field_name.clone(), list_size_directive); } @@ -90,17 +93,20 @@ impl DemandControlledSchema { )) })?; - if let Some(cost_directive) = - CostDirective::from_field(&directive_name_map, field_definition) - .or(CostDirective::from_type(&directive_name_map, field_type)) - { + if let Some(cost_directive) = CostSpecDefinition::cost_directive_from_field( + &fed_schema, + field_definition, + field_type, + )? { field_cost_directives.insert(field_name.clone(), cost_directive); } - if let Some(list_size_directive) = ListSizeDirective::from_field_definition( - &directive_name_map, - field_definition, - )? { + if let Some(list_size_directive) = + CostSpecDefinition::list_size_directive_from_field_definition( + &fed_schema, + field_definition, + )? + { field_list_size_directives .insert(field_name.clone(), list_size_directive); } @@ -122,18 +128,13 @@ impl DemandControlledSchema { } Ok(Self { - directive_name_map, - inner: schema, + inner: fed_schema, type_field_cost_directives, type_field_list_size_directives, type_field_requires_directives, }) } - pub(in crate::plugins::demand_control) fn directive_name_map(&self) -> &HashMap { - &self.directive_name_map - } - pub(in crate::plugins::demand_control) fn type_field_cost_directive( &self, type_name: &str, @@ -163,11 +164,24 @@ impl DemandControlledSchema { .get(type_name)? .get(field_name) } + + pub(in crate::plugins::demand_control) fn argument_cost_directive( + &self, + definition: &InputValueDefinition, + ty: &ExtendedType, + ) -> Option { + // For now, we ignore FederationError and return None because this should not block the whole scoring + // process at runtime. Later, this should be pushed into the constructor and propagate any federation + // errors encountered when parsing. + CostSpecDefinition::cost_directive_from_argument(&self.inner, definition, ty) + .ok() + .flatten() + } } impl AsRef> for DemandControlledSchema { fn as_ref(&self) -> &Valid { - &self.inner + self.inner.schema() } } @@ -175,6 +189,6 @@ impl Deref for DemandControlledSchema { type Target = Schema; fn deref(&self) -> &Self::Target { - &self.inner + self.inner.schema() } } diff --git a/apollo-router/src/plugins/demand_control/cost_calculator/static_cost.rs b/apollo-router/src/plugins/demand_control/cost_calculator/static_cost.rs index 397f33e9b6..ca7217ba7f 100644 --- a/apollo-router/src/plugins/demand_control/cost_calculator/static_cost.rs +++ b/apollo-router/src/plugins/demand_control/cost_calculator/static_cost.rs @@ -22,7 +22,6 @@ use super::DemandControlError; use crate::graphql::Response; use crate::graphql::ResponseVisitor; use crate::json_ext::Object; -use crate::plugins::demand_control::cost_calculator::directives::CostDirective; use crate::plugins::demand_control::cost_calculator::directives::ListSizeDirective; use crate::query_planner::fetch::SubgraphOperation; use crate::query_planner::DeferredNode; @@ -59,9 +58,7 @@ fn score_argument( argument_definition.ty.inner_named_type() )) })?; - let cost_directive = - CostDirective::from_argument(schema.directive_name_map(), argument_definition) - .or(CostDirective::from_type(schema.directive_name_map(), ty)); + let cost_directive = schema.argument_cost_directive(argument_definition, ty); match (argument, ty) { (_, ExtendedType::Interface(_)) @@ -124,9 +121,7 @@ fn score_variable( argument_definition.ty.inner_named_type() )) })?; - let cost_directive = - CostDirective::from_argument(schema.directive_name_map(), argument_definition) - .or(CostDirective::from_type(schema.directive_name_map(), ty)); + let cost_directive = schema.argument_cost_directive(argument_definition, ty); match (variable, ty) { (_, ExtendedType::Interface(_)) @@ -221,7 +216,7 @@ impl StaticCostCalculator { .schema .type_field_list_size_directive(parent_type, &field.name) { - Some(dir) => dir.with_field_and_variables(field, ctx.variables).map(Some), + Some(dir) => ListSizeDirective::new(dir, field, ctx.variables).map(Some), None => Ok(None), }?; let instance_count = if !field.ty().is_list() { diff --git a/apollo-router/src/plugins/demand_control/mod.rs b/apollo-router/src/plugins/demand_control/mod.rs index 5e3ba587f8..8e8d419ab8 100644 --- a/apollo-router/src/plugins/demand_control/mod.rs +++ b/apollo-router/src/plugins/demand_control/mod.rs @@ -11,6 +11,7 @@ use apollo_compiler::schema::FieldLookupError; use apollo_compiler::validation::Valid; use apollo_compiler::validation::WithErrors; use apollo_compiler::ExecutableDocument; +use apollo_federation::error::FederationError; use displaydoc::Display; use futures::future::Either; use futures::stream; @@ -123,6 +124,8 @@ pub(crate) enum DemandControlError { SubgraphOperationNotInitialized(crate::query_planner::fetch::SubgraphOperationNotInitialized), /// {0} ContextSerializationError(String), + /// {0} + FederationError(FederationError), } impl IntoGraphQLErrors for DemandControlError { @@ -163,6 +166,10 @@ impl IntoGraphQLErrors for DemandControlError { .extension_code(self.code()) .message(self.to_string()) .build()]), + DemandControlError::FederationError(_) => Ok(vec![graphql::Error::builder() + .extension_code(self.code()) + .message(self.to_string()) + .build()]), } } } @@ -175,6 +182,7 @@ impl DemandControlError { DemandControlError::QueryParseFailure(_) => "COST_QUERY_PARSE_FAILURE", DemandControlError::SubgraphOperationNotInitialized(e) => e.code(), DemandControlError::ContextSerializationError(_) => "COST_CONTEXT_SERIALIZATION_ERROR", + DemandControlError::FederationError(_) => "FEDERATION_ERROR", } } } @@ -201,6 +209,12 @@ impl<'a> From> for DemandControlError { } } +impl From for DemandControlError { + fn from(value: FederationError) -> Self { + DemandControlError::FederationError(value) + } +} + #[derive(Clone)] pub(crate) struct DemandControlContext { pub(crate) strategy: Strategy,