diff --git a/apollo-federation/cli/src/main.rs b/apollo-federation/cli/src/main.rs index 1b2c17665e..a92f9ffa36 100644 --- a/apollo-federation/cli/src/main.rs +++ b/apollo-federation/cli/src/main.rs @@ -307,6 +307,7 @@ fn compose_from_config_inner( error: SingleFederationError::Internal { message: format!("Failed to parse YAML config: {}", e), }, + locations: Vec::new(), }] })?; @@ -337,6 +338,7 @@ fn compose_from_config_inner( name ), }, + locations: Vec::new(), }]); }; diff --git a/apollo-federation/src/error/mod.rs b/apollo-federation/src/error/mod.rs index 1151381ea3..d20846e67d 100644 --- a/apollo-federation/src/error/mod.rs +++ b/apollo-federation/src/error/mod.rs @@ -131,6 +131,17 @@ pub struct SubgraphLocation { pub type Locations = Vec; +/// Supergraph coordinates carried on `@inaccessible` API schema errors so composition can attach +/// subgraph source locations (port of JS `inaccessible_elements` / `inaccessible_referencers` +/// extensions and `updateInaccessibleErrorsWithLinkToSubgraphs`). Those locations are surfaced on +/// [`CompositionError::MergeError`] as [`SubgraphLocation`] entries (Rust equivalent of JS +/// `withModifiedErrorNodes`). +#[derive(Clone, Debug, Default)] +pub struct InaccessibleCompositionLinks { + pub elements: Vec, + pub referencers: Vec, +} + pub(crate) trait HasLocations { fn locations(&self, subgraph: &Subgraph) -> Locations; } @@ -150,7 +161,10 @@ pub enum CompositionError { locations: Locations, }, #[error("{error}")] - MergeError { error: SingleFederationError }, + MergeError { + error: SingleFederationError, + locations: Locations, + }, #[error("{error}")] MergeValidationError { error: SingleFederationError }, #[error("{message}")] @@ -498,6 +512,7 @@ impl CompositionError { | Self::RequiredInputFieldMissingInSomeSubgraph { locations, .. } | Self::EmptyMergedInputType { locations, .. } | Self::InvalidFieldSharing { locations, .. } + | Self::MergeError { locations, .. } | Self::ArgumentDefaultMismatch { locations, .. } | Self::InputFieldDefaultMismatch { locations, .. } => locations.extend(new_locations), // Remaining errors do not have an obvious way to appending locations, so we do nothing @@ -516,6 +531,7 @@ impl CompositionError { | Self::RequiredInputFieldMissingInSomeSubgraph { locations, .. } | Self::EmptyMergedInputType { locations, .. } | Self::InvalidFieldSharing { locations, .. } + | Self::MergeError { locations, .. } | Self::ArgumentDefaultMismatch { locations, .. } | Self::InputFieldDefaultMismatch { locations, .. } => locations, _ => &[], @@ -771,19 +787,40 @@ pub enum SingleFederationError { #[error("{message}")] InvalidLinkIdentifier { message: String }, #[error("{message}")] - ReferencedInaccessible { message: String }, + ReferencedInaccessible { + message: String, + links: InaccessibleCompositionLinks, + }, #[error("{message}")] - DefaultValueUsesInaccessible { message: String }, + DefaultValueUsesInaccessible { + message: String, + links: InaccessibleCompositionLinks, + }, #[error("{message}")] - QueryRootTypeInaccessible { message: String }, + QueryRootTypeInaccessible { + message: String, + links: InaccessibleCompositionLinks, + }, #[error("{message}")] - RequiredInaccessible { message: String }, + RequiredInaccessible { + message: String, + links: InaccessibleCompositionLinks, + }, #[error("{message}")] - ImplementedByInaccessible { message: String }, + ImplementedByInaccessible { + message: String, + links: InaccessibleCompositionLinks, + }, #[error("{message}")] - DisallowedInaccessible { message: String }, + DisallowedInaccessible { + message: String, + links: InaccessibleCompositionLinks, + }, #[error("{message}")] - OnlyInaccessibleChildren { message: String }, + OnlyInaccessibleChildren { + message: String, + links: InaccessibleCompositionLinks, + }, #[error("{message}")] RequiredInputFieldMissingInSomeSubgraph { message: String }, #[error("{message}")] diff --git a/apollo-federation/src/link/inaccessible_spec_definition.rs b/apollo-federation/src/link/inaccessible_spec_definition.rs index 158a72d573..764413411b 100644 --- a/apollo-federation/src/link/inaccessible_spec_definition.rs +++ b/apollo-federation/src/link/inaccessible_spec_definition.rs @@ -16,6 +16,7 @@ use apollo_compiler::schema::InputValueDefinition; use apollo_compiler::schema::Value; use crate::error::FederationError; +use crate::error::InaccessibleCompositionLinks; use crate::error::MultipleFederationErrors; use crate::error::SingleFederationError; use crate::link::Purpose; @@ -249,7 +250,7 @@ fn type_uses_inaccessible( } /// Check if a directive uses the @inaccessible directive anywhere in its definition. -fn directive_uses_inaccessible( +pub(crate) fn directive_uses_inaccessible( inaccessible_directive: &Name, directive: &DirectiveDefinition, ) -> bool { @@ -300,6 +301,10 @@ fn validate_inaccessible_in_default_value( if input_field_position.is_inaccessible(schema, inaccessible_directive)? { errors.push(SingleFederationError::DefaultValueUsesInaccessible { message: format!("Input field `{input_field_position}` is @inaccessible but is used in the default value of `{value_position}`, which is in the API schema."), + links: InaccessibleCompositionLinks { + elements: vec![input_field_position.to_string()], + referencers: vec![value_position.clone()], + }, }.into()); } @@ -351,6 +356,10 @@ fn validate_inaccessible_in_default_value( if enum_value_position.is_inaccessible(schema, inaccessible_directive)? { errors.push(SingleFederationError::DefaultValueUsesInaccessible { message: format!("Enum value `{enum_value_position}` is @inaccessible but is used in the default value of `{value_position}`, which is in the API schema."), + links: InaccessibleCompositionLinks { + elements: vec![enum_value_position.to_string()], + referencers: vec![value_position.clone()], + }, }.into()); } } @@ -380,6 +389,10 @@ fn validate_inaccessible_in_arguments( }; errors.push(SingleFederationError::RequiredInaccessible { message: format!("Argument `{usage_position}({arg_name}:)` is @inaccessible but is a required argument of its {kind}."), + links: InaccessibleCompositionLinks { + elements: vec![format!("{usage_position}({arg_name}:)")], + referencers: vec![], + }, }.into()); } @@ -438,6 +451,10 @@ fn validate_inaccessible_in_fields( errors.push( SingleFederationError::ImplementedByInaccessible { message: format!("Field `{type_position}.{field_name}` is @inaccessible but implements the interface field `{super_position}`, which is in the API schema."), + links: InaccessibleCompositionLinks { + elements: vec![format!("{type_position}.{field_name}")], + referencers: vec![super_position.to_string()], + }, } .into(), ); @@ -475,6 +492,12 @@ fn validate_inaccessible_in_fields( for accessible_reference in accessible_super_references { errors.push(SingleFederationError::ImplementedByInaccessible { message: format!("Argument `{type_position}.{field_name}({arg_name}:)` is @inaccessible but implements the interface argument `{accessible_reference}` which is in the API schema."), + links: InaccessibleCompositionLinks { + elements: vec![format!( + "{type_position}.{field_name}({arg_name}:)" + )], + referencers: vec![accessible_reference.to_string()], + }, }.into()); } } else if arg.is_required() { @@ -520,6 +543,12 @@ fn validate_inaccessible_in_fields( for inaccessible_reference in inaccessible_super_references { errors.push(SingleFederationError::RequiredInaccessible { message: format!("Argument `{inaccessible_reference}` is @inaccessible but is implemented by the argument `{type_position}.{field_name}({arg_name}:)` which is in the API schema."), + links: InaccessibleCompositionLinks { + elements: vec![inaccessible_reference.to_string()], + referencers: vec![format!( + "{type_position}.{field_name}({arg_name}:)" + )], + }, }.into()); } } @@ -549,6 +578,10 @@ fn validate_inaccessible_in_fields( if has_inaccessible_field && !has_accessible_field { errors.push(SingleFederationError::OnlyInaccessibleChildren { message: format!("Type `{type_position}` is in the API schema but all of its members are @inaccessible."), + links: InaccessibleCompositionLinks { + elements: vec![type_position.to_string()], + referencers: vec![], + }, }.into()); } @@ -751,6 +784,10 @@ fn validate_inaccessible_type( if !$ref.is_inaccessible(schema, inaccessible_directive)? { errors.push(SingleFederationError::ReferencedInaccessible { message: format!("Type `{}` is @inaccessible but is referenced by `{}`, which is in the API schema.", $ty, $ref), + links: InaccessibleCompositionLinks { + elements: vec![$ty.to_string()], + referencers: vec![$ref.to_string()], + }, }.into()) } } @@ -805,6 +842,10 @@ fn validate_inaccessible_type( { errors.push(SingleFederationError::QueryRootTypeInaccessible { message: format!("Type `{position}` is @inaccessible but is the query root type, which must be in the API schema."), + links: InaccessibleCompositionLinks { + elements: vec![position.to_string()], + referencers: vec![], + }, }.into()); } check_inaccessible_referencers!( @@ -899,6 +940,10 @@ fn validate_inaccessible( message: format!( "Core feature type `{position}` cannot use @inaccessible." ), + links: InaccessibleCompositionLinks { + elements: vec![position.to_string()], + referencers: vec![], + }, } .into(), ) @@ -947,6 +992,10 @@ fn validate_inaccessible( if field_inaccessible && field.is_required() { errors.push(SingleFederationError::RequiredInaccessible{ message: format!("Input field `{position}` is @inaccessible but is a required input field of its type."), + links: InaccessibleCompositionLinks { + elements: vec![input_object_position.field(field.name.clone()).to_string()], + referencers: vec![], + }, }.into()); } @@ -970,6 +1019,10 @@ fn validate_inaccessible( if has_inaccessible_field && !has_accessible_field { errors.push(SingleFederationError::OnlyInaccessibleChildren { message: format!("Type `{position}` is in the API schema but all of its input fields are @inaccessible."), + links: InaccessibleCompositionLinks { + elements: vec![position.to_string()], + referencers: vec![], + }, }.into()); } } @@ -985,6 +1038,10 @@ fn validate_inaccessible( if !any_accessible_member { errors.push(SingleFederationError::OnlyInaccessibleChildren { message: format!("Type `{position}` is in the API schema but all of its members are @inaccessible."), + links: InaccessibleCompositionLinks { + elements: vec![position.to_string()], + referencers: vec![], + }, }.into()); } } @@ -1001,6 +1058,10 @@ fn validate_inaccessible( if has_inaccessible_value && !has_accessible_value { errors.push(SingleFederationError::OnlyInaccessibleChildren { message: format!("Type `{enum_position}` is in the API schema but all of its members are @inaccessible."), + links: InaccessibleCompositionLinks { + elements: vec![enum_position.to_string()], + referencers: vec![], + }, }.into()); } } @@ -1028,6 +1089,10 @@ fn validate_inaccessible( message: format!( "Core feature directive `{position}` cannot use @inaccessible.", ), + links: InaccessibleCompositionLinks { + elements: vec![position.to_string()], + referencers: vec![], + }, } .into(), ); @@ -1042,6 +1107,10 @@ fn validate_inaccessible( .join(", "); errors.push(SingleFederationError::DisallowedInaccessible { message: format!("Directive `{position}` cannot use @inaccessible because it may be applied to these type-system locations: {type_system_locations}"), + links: InaccessibleCompositionLinks { + elements: vec![position.to_string()], + referencers: vec![], + }, }.into()); } } else { diff --git a/apollo-federation/src/merger/merger.rs b/apollo-federation/src/merger/merger.rs index 46d09f16fa..50b542384a 100644 --- a/apollo-federation/src/merger/merger.rs +++ b/apollo-federation/src/merger/merger.rs @@ -560,7 +560,8 @@ impl Merger { hints, }); } - match Self::validate_supergraph_schema(self.merged) { + let merged = self.merged; + match Self::validate_supergraph_schema(merged, &self.subgraphs) { Ok(supergraph) => Ok(MergeResult { supergraph: Some(supergraph), errors: Vec::default(), @@ -579,6 +580,7 @@ impl Merger { /// computed. fn validate_supergraph_schema( merged: FederationSchema, + subgraphs: &[Subgraph], ) -> Result> { // TODO: Errors thrown by the `validate` below are likely to be confusing for users, // because they refer to a document they don't know about (the merged-but-not-returned @@ -597,8 +599,11 @@ impl Merger { // other errors in theory, but if there are, better to find it now rather than later). api_schema::to_api_schema(supergraph_schema.clone(), Default::default()).map_err( |err| { - // TODO: port `updateInaccessibleErrorsWithLinkToSubgraphs` from JS (FED-882) - Self::convert_to_merge_errors(err) + super::supergraph_coordinate::update_inaccessible_errors_with_link_to_subgraphs( + &supergraph_schema, + subgraphs, + err, + ) }, )?; @@ -610,7 +615,10 @@ impl Merger { error .into_errors() .into_iter() - .map(|e| CompositionError::MergeError { error: e }) + .map(|e| CompositionError::MergeError { + error: e, + locations: Vec::new(), + }) .collect() } diff --git a/apollo-federation/src/merger/mod.rs b/apollo-federation/src/merger/mod.rs index d174f003b6..655da142b0 100644 --- a/apollo-federation/src/merger/mod.rs +++ b/apollo-federation/src/merger/mod.rs @@ -12,3 +12,4 @@ mod merge_interface; mod merge_links; mod merge_type; mod merge_union; +mod supergraph_coordinate; diff --git a/apollo-federation/src/merger/supergraph_coordinate.rs b/apollo-federation/src/merger/supergraph_coordinate.rs new file mode 100644 index 0000000000..1a21cbff04 --- /dev/null +++ b/apollo-federation/src/merger/supergraph_coordinate.rs @@ -0,0 +1,548 @@ +//! Resolve supergraph element coordinates to subgraph locations for composition diagnostics. +//! Ports the behavior of `updateInaccessibleErrorsWithLinkToSubgraphs` in Apollo Federation's +//! [`merge.ts`](https://github.com/apollographql/federation/blob/f3ab499eaf62b1a1c0f08b838d2cbde5accb303a/composition-js/src/merging/merge.ts). + +use apollo_compiler::Name; +use apollo_compiler::schema::ExtendedType; + +use crate::error::ErrorCode; +use crate::error::HasLocations; +use crate::error::Locations; +use crate::error::SingleFederationError; +use crate::error::SubgraphLocation; +use crate::link::inaccessible_spec_definition::IsInaccessibleExt; +use crate::link::inaccessible_spec_definition::directive_uses_inaccessible; +use crate::schema::FederationSchema; +use crate::schema::position::DirectiveArgumentDefinitionPosition; +use crate::schema::position::DirectiveDefinitionPosition; +use crate::schema::position::DirectiveTargetPosition; +use crate::schema::position::EnumTypeDefinitionPosition; +use crate::schema::position::EnumValueDefinitionPosition; +use crate::schema::position::InputObjectFieldDefinitionPosition; +use crate::schema::position::InputObjectTypeDefinitionPosition; +use crate::schema::position::InterfaceFieldArgumentDefinitionPosition; +use crate::schema::position::InterfaceFieldDefinitionPosition; +use crate::schema::position::InterfaceTypeDefinitionPosition; +use crate::schema::position::ObjectFieldArgumentDefinitionPosition; +use crate::schema::position::ObjectFieldDefinitionPosition; +use crate::schema::position::ObjectTypeDefinitionPosition; +use crate::schema::position::ScalarTypeDefinitionPosition; +use crate::schema::position::UnionTypeDefinitionPosition; +use crate::subgraph::typestate::HasMetadata; +use crate::subgraph::typestate::Subgraph; + +/// A coordinate string from the merged supergraph, resolved against that schema. +#[derive(Clone, Debug)] +pub(crate) enum ParsedSupergraphCoordinate { + Target(DirectiveTargetPosition), + DirectiveDefinition(DirectiveDefinitionPosition), +} + +impl ParsedSupergraphCoordinate { + fn exists_in(&self, schema: &FederationSchema) -> bool { + match self { + Self::Target(p) => p.exists_in(schema), + Self::DirectiveDefinition(p) => p.try_get(schema.schema()).is_some(), + } + } + + fn locations(&self, subgraph: &Subgraph) -> Locations { + match self { + Self::Target(p) => p.locations(subgraph), + Self::DirectiveDefinition(p) => p.locations(subgraph), + } + } + + /// Whether this coordinate is marked `@inaccessible` in the given subgraph (not the supergraph). + fn subgraph_marks_inaccessible_element( + &self, + subgraph: &Subgraph, + inaccessible: &Name, + ) -> Result { + let schema = subgraph.schema(); + match self { + Self::Target(p) => directive_target_is_inaccessible(p, schema, inaccessible), + Self::DirectiveDefinition(p) => { + let def = p.get(schema.schema())?; + Ok(directive_uses_inaccessible(inaccessible, def)) + } + } + } +} + +fn parse_name(segment: &str) -> Option { + Name::new(segment).ok() +} + +fn type_level_target( + schema: &FederationSchema, + type_name: Name, +) -> Option { + let ext = schema.schema().types.get(&type_name)?; + Some(match ext { + ExtendedType::Scalar(_) => { + DirectiveTargetPosition::ScalarType(ScalarTypeDefinitionPosition { type_name }) + } + ExtendedType::Object(_) => { + DirectiveTargetPosition::ObjectType(ObjectTypeDefinitionPosition { type_name }) + } + ExtendedType::Interface(_) => { + DirectiveTargetPosition::InterfaceType(InterfaceTypeDefinitionPosition { type_name }) + } + ExtendedType::Union(_) => { + DirectiveTargetPosition::UnionType(UnionTypeDefinitionPosition { type_name }) + } + ExtendedType::Enum(_) => { + DirectiveTargetPosition::EnumType(EnumTypeDefinitionPosition { type_name }) + } + ExtendedType::InputObject(_) => { + DirectiveTargetPosition::InputObjectType(InputObjectTypeDefinitionPosition { + type_name, + }) + } + }) +} + +fn field_or_member_target( + schema: &FederationSchema, + type_name: Name, + member_name: Name, +) -> Option { + let ext = schema.schema().types.get(&type_name)?; + match ext { + ExtendedType::Object(o) if o.fields.contains_key(&member_name) => Some( + DirectiveTargetPosition::ObjectField(ObjectFieldDefinitionPosition { + type_name, + field_name: member_name, + }), + ), + ExtendedType::Interface(i) if i.fields.contains_key(&member_name) => Some( + DirectiveTargetPosition::InterfaceField(InterfaceFieldDefinitionPosition { + type_name, + field_name: member_name, + }), + ), + ExtendedType::InputObject(io) if io.fields.contains_key(&member_name) => Some( + DirectiveTargetPosition::InputObjectField(InputObjectFieldDefinitionPosition { + type_name, + field_name: member_name, + }), + ), + ExtendedType::Enum(e) if e.values.contains_key(&member_name) => Some( + DirectiveTargetPosition::EnumValue(EnumValueDefinitionPosition { + type_name, + value_name: member_name, + }), + ), + _ => None, + } +} + +/// Parse a supergraph element coordinate (same string forms as [`std::fmt::Display`] on positions). +pub(crate) fn parse_supergraph_coordinate( + schema: &FederationSchema, + coordinate: &str, +) -> Option { + let coordinate = coordinate.trim(); + if coordinate.is_empty() { + return None; + } + + // `@directive` (directive definition), e.g. `@tag` + if coordinate.starts_with('@') && !coordinate.contains('(') && coordinate.len() > 1 { + let directive_name = parse_name(&coordinate[1..])?; + let pos = DirectiveDefinitionPosition { directive_name }; + return pos + .try_get(schema.schema()) + .is_some() + .then(|| ParsedSupergraphCoordinate::DirectiveDefinition(pos)); + } + + // `@directive(arg:)` — directive definition argument + if coordinate.starts_with('@') { + let rest = coordinate.strip_prefix('@')?; + let open_paren = rest.find('(')?; + if !rest.ends_with(')') { + return None; + } + let directive_name = parse_name(&rest[..open_paren])?; + let inner = &rest[open_paren + 1..rest.len() - 1]; + let arg_name = inner.strip_suffix(':')?; + let arg_name = parse_name(arg_name)?; + let pos = DirectiveArgumentDefinitionPosition { + directive_name, + argument_name: arg_name, + }; + return pos.try_get(schema.schema()).is_some().then(|| { + ParsedSupergraphCoordinate::Target(DirectiveTargetPosition::DirectiveArgument(pos)) + }); + } + + // `Type.field(arg:)` — field argument + if let Some(open_paren) = coordinate.rfind('(') + && coordinate.ends_with(')') + { + let inner = &coordinate[open_paren + 1..coordinate.len() - 1]; + if let Some(arg_name) = inner.strip_suffix(':') { + let arg_name = parse_name(arg_name)?; + let before = &coordinate[..open_paren]; + let dot = before.rfind('.')?; + let type_name = parse_name(&before[..dot])?; + let field_name = parse_name(&before[dot + 1..])?; + let ext = schema.schema().types.get(&type_name)?; + return match ext { + ExtendedType::Object(o) if o.fields.contains_key(&field_name) => { + let pos = ObjectFieldArgumentDefinitionPosition { + type_name, + field_name, + argument_name: arg_name, + }; + pos.try_get(schema.schema()).is_some().then(|| { + ParsedSupergraphCoordinate::Target( + DirectiveTargetPosition::ObjectFieldArgument(pos), + ) + }) + } + ExtendedType::Interface(i) if i.fields.contains_key(&field_name) => { + let pos = InterfaceFieldArgumentDefinitionPosition { + type_name, + field_name, + argument_name: arg_name, + }; + pos.try_get(schema.schema()).is_some().then(|| { + ParsedSupergraphCoordinate::Target( + DirectiveTargetPosition::InterfaceFieldArgument(pos), + ) + }) + } + _ => None, + }; + } + } + + if let Some(dot) = coordinate.rfind('.') { + let left = &coordinate[..dot]; + let right = &coordinate[dot + 1..]; + let type_name = parse_name(left)?; + let member_name = parse_name(right)?; + return field_or_member_target(schema, type_name, member_name) + .map(ParsedSupergraphCoordinate::Target); + } + + let type_name = parse_name(coordinate)?; + type_level_target(schema, type_name).map(ParsedSupergraphCoordinate::Target) +} + +fn directive_target_is_inaccessible( + pos: &DirectiveTargetPosition, + schema: &FederationSchema, + inaccessible_directive: &Name, +) -> Result { + match pos { + DirectiveTargetPosition::Schema(p) => Ok(p + .get(schema.schema()) + .directives + .has(inaccessible_directive)), + DirectiveTargetPosition::ScalarType(p) => p.is_inaccessible(schema, inaccessible_directive), + DirectiveTargetPosition::ObjectType(p) => p.is_inaccessible(schema, inaccessible_directive), + DirectiveTargetPosition::ObjectField(p) => { + p.is_inaccessible(schema, inaccessible_directive) + } + DirectiveTargetPosition::ObjectFieldArgument(p) => { + p.is_inaccessible(schema, inaccessible_directive) + } + DirectiveTargetPosition::InterfaceType(p) => { + p.is_inaccessible(schema, inaccessible_directive) + } + DirectiveTargetPosition::InterfaceField(p) => { + p.is_inaccessible(schema, inaccessible_directive) + } + DirectiveTargetPosition::InterfaceFieldArgument(p) => { + p.is_inaccessible(schema, inaccessible_directive) + } + DirectiveTargetPosition::UnionType(p) => p.is_inaccessible(schema, inaccessible_directive), + DirectiveTargetPosition::EnumType(p) => p.is_inaccessible(schema, inaccessible_directive), + DirectiveTargetPosition::EnumValue(p) => p.is_inaccessible(schema, inaccessible_directive), + DirectiveTargetPosition::InputObjectType(p) => { + p.is_inaccessible(schema, inaccessible_directive) + } + DirectiveTargetPosition::InputObjectField(p) => { + p.is_inaccessible(schema, inaccessible_directive) + } + DirectiveTargetPosition::DirectiveArgument(p) => { + p.is_inaccessible(schema, inaccessible_directive) + } + } +} + +fn referencer_base_type_name( + pos: &DirectiveTargetPosition, + schema: &FederationSchema, +) -> Option { + match pos { + DirectiveTargetPosition::ObjectField(p) => p + .get(schema.schema()) + .ok() + .map(|f| f.ty.inner_named_type().clone()), + DirectiveTargetPosition::InterfaceField(p) => p + .get(schema.schema()) + .ok() + .map(|f| f.ty.inner_named_type().clone()), + DirectiveTargetPosition::ObjectFieldArgument(p) => p + .get(schema.schema()) + .ok() + .map(|a| a.ty.inner_named_type().clone()), + DirectiveTargetPosition::InterfaceFieldArgument(p) => p + .get(schema.schema()) + .ok() + .map(|a| a.ty.inner_named_type().clone()), + DirectiveTargetPosition::InputObjectField(p) => p + .get(schema.schema()) + .ok() + .map(|f| f.ty.inner_named_type().clone()), + DirectiveTargetPosition::DirectiveArgument(p) => p + .get(schema.schema()) + .ok() + .map(|a| a.ty.inner_named_type().clone()), + _ => None, + } +} + +fn referencer_matches_required_inaccessible( + pos: &DirectiveTargetPosition, + schema: &FederationSchema, +) -> bool { + match pos { + DirectiveTargetPosition::ObjectFieldArgument(p) => p + .get(schema.schema()) + .is_ok_and(|a| a.ty.is_non_null() || a.default_value.is_none()), + DirectiveTargetPosition::InterfaceFieldArgument(p) => p + .get(schema.schema()) + .is_ok_and(|a| a.ty.is_non_null() || a.default_value.is_none()), + DirectiveTargetPosition::InputObjectField(p) => p + .get(schema.schema()) + .is_ok_and(|f| f.ty.is_non_null() || f.default_value.is_none()), + DirectiveTargetPosition::DirectiveArgument(p) => p + .get(schema.schema()) + .is_ok_and(|a| a.ty.is_non_null() || a.default_value.is_none()), + _ => false, + } +} + +/// Port of JS `isRelevantSubgraphReferencer` for `@inaccessible` API schema errors. +fn is_relevant_subgraph_referencer( + code: &ErrorCode, + referencer: &DirectiveTargetPosition, + referencer_subgraph_schema: &FederationSchema, + inaccessible_element_type_names: &[String], + subgraph_has_inaccessible_elements: bool, +) -> bool { + match code { + ErrorCode::ReferencedInaccessible => { + let Some(first) = inaccessible_element_type_names.first() else { + return false; + }; + let Ok(expected) = Name::new(first.as_str()) else { + return false; + }; + referencer_base_type_name(referencer, referencer_subgraph_schema) + .is_some_and(|ty| ty == expected) + } + ErrorCode::DefaultValueUsesInaccessible | ErrorCode::ImplementedByInaccessible => true, + ErrorCode::RequiredInaccessible => { + referencer_matches_required_inaccessible(referencer, referencer_subgraph_schema) + } + ErrorCode::DisallowedInaccessible | ErrorCode::OnlyInaccessibleChildren => { + subgraph_has_inaccessible_elements + } + _ => false, + } +} + +/// Port of `Merger.updateInaccessibleErrorsWithLinkToSubgraphs`: map a federation error from API +/// schema validation into merge errors with subgraph source locations for `@inaccessible` issues. +/// +/// In JS, subgraph hints are attached by rewriting the GraphQL error's AST nodes +/// (`withModifiedErrorNodes`). In Rust, the same information is carried on +/// [`crate::error::CompositionError::MergeError`] as [`crate::error::Locations`]. +pub(crate) fn update_inaccessible_errors_with_link_to_subgraphs( + supergraph: &FederationSchema, + subgraphs: &[Subgraph], + err: crate::error::FederationError, +) -> Vec { + err.into_errors() + .into_iter() + .map(|error| { + let locations = + subgraph_locations_for_single_inaccessible_error(supergraph, subgraphs, &error); + crate::error::CompositionError::MergeError { error, locations } + }) + .collect() +} + +/// Subgraph source locations for one error (JS `withModifiedErrorNodes` per error). +fn subgraph_locations_for_single_inaccessible_error( + supergraph: &FederationSchema, + subgraphs: &[Subgraph], + error: &SingleFederationError, +) -> Locations { + let (code, links) = match error { + SingleFederationError::ReferencedInaccessible { links, .. } + | SingleFederationError::DefaultValueUsesInaccessible { links, .. } + | SingleFederationError::RequiredInaccessible { links, .. } + | SingleFederationError::ImplementedByInaccessible { links, .. } + | SingleFederationError::DisallowedInaccessible { links, .. } + | SingleFederationError::OnlyInaccessibleChildren { links, .. } + | SingleFederationError::QueryRootTypeInaccessible { links, .. } => (error.code(), links), + _ => return Vec::new(), + }; + let code = &code; + + let mut out: Vec = Vec::new(); + let mut subgraph_has_inaccessible: Vec = vec![false; subgraphs.len()]; + + for coordinate in &links.elements { + let Some(parsed) = parse_supergraph_coordinate(supergraph, coordinate) else { + continue; + }; + for (idx, subgraph) in subgraphs.iter().enumerate() { + let Ok(Some(inaccessible_name)) = subgraph.inaccessible_directive_name() else { + continue; + }; + if !parsed.exists_in(subgraph.schema()) { + continue; + } + let Ok(marked) = + parsed.subgraph_marks_inaccessible_element(subgraph, &inaccessible_name) + else { + continue; + }; + if marked { + subgraph_has_inaccessible[idx] = true; + out.extend(parsed.locations(subgraph)); + } + } + } + + for coordinate in &links.referencers { + let Some(parsed) = parse_supergraph_coordinate(supergraph, coordinate) else { + continue; + }; + if !parsed.exists_in(supergraph) { + continue; + } + for (idx, subgraph) in subgraphs.iter().enumerate() { + if !parsed.exists_in(subgraph.schema()) { + continue; + } + match &parsed { + ParsedSupergraphCoordinate::Target(target_pos) => { + if is_relevant_subgraph_referencer( + code, + target_pos, + subgraph.schema(), + &links.elements, + subgraph_has_inaccessible[idx], + ) { + out.extend(target_pos.locations(subgraph)); + } + } + ParsedSupergraphCoordinate::DirectiveDefinition(dir_pos) => { + if is_relevant_directive_definition_referencer( + code, + subgraph_has_inaccessible[idx], + ) { + out.extend(dir_pos.locations(subgraph)); + } + } + } + } + } + + out +} + +/// `isRelevantSubgraphReferencer` for coordinates that resolve to a directive definition (`@name`). +/// JS `elementByCoordinate` can return a directive definition; those referencers are not field, +/// argument, or input-field definitions, so `REFERENCED_INACCESSIBLE` and `REQUIRED_INACCESSIBLE` +/// never apply (same as the `default` branch for unrelated element kinds in JS). +fn is_relevant_directive_definition_referencer( + code: &ErrorCode, + subgraph_has_inaccessible_elements: bool, +) -> bool { + match code { + ErrorCode::ReferencedInaccessible | ErrorCode::RequiredInaccessible => false, + ErrorCode::DefaultValueUsesInaccessible | ErrorCode::ImplementedByInaccessible => true, + ErrorCode::DisallowedInaccessible | ErrorCode::OnlyInaccessibleChildren => { + subgraph_has_inaccessible_elements + } + _ => false, + } +} + +#[cfg(test)] +mod tests { + use super::parse_supergraph_coordinate; + use crate::Supergraph; + use crate::subgraph::Subgraph; + + fn sample_supergraph() -> Supergraph { + let s1 = Subgraph::parse_and_expand( + "S1", + "http://s1", + r#" + type Query { + t: T + f(a: Int): T + } + + type T @key(fields: "k") { + k: ID + } + "#, + ) + .unwrap(); + let s2 = Subgraph::parse_and_expand( + "S2", + "http://s2", + r#" + type T @key(fields: "k") { + k: ID + a: Int + } + "#, + ) + .unwrap(); + Supergraph::compose(vec![&s1, &s2]).unwrap() + } + + #[test] + fn parse_supergraph_coordinate_rejects_empty_or_whitespace() { + let sg = sample_supergraph(); + assert!(parse_supergraph_coordinate(&sg.schema, "").is_none()); + assert!(parse_supergraph_coordinate(&sg.schema, " ").is_none()); + } + + #[test] + fn parse_supergraph_coordinate_type_and_field() { + let sg = sample_supergraph(); + assert!(parse_supergraph_coordinate(&sg.schema, "Query.t").is_some()); + assert!(parse_supergraph_coordinate(&sg.schema, "Query.f").is_some()); + assert!(parse_supergraph_coordinate(&sg.schema, "T").is_some()); + assert!(parse_supergraph_coordinate(&sg.schema, "Unknown").is_none()); + } + + #[test] + fn parse_supergraph_coordinate_field_argument() { + let sg = sample_supergraph(); + assert!(parse_supergraph_coordinate(&sg.schema, "Query.f(a:)").is_some()); + assert!(parse_supergraph_coordinate(&sg.schema, "Query.f(").is_none()); + } + + #[test] + fn parse_supergraph_coordinate_known_directive_definition() { + let sg = sample_supergraph(); + // Present on composed supergraph schemas (federation join spec). + assert!(parse_supergraph_coordinate(&sg.schema, "@join__graph").is_some()); + } +} diff --git a/apollo-federation/tests/composition/compose_inaccessible.rs b/apollo-federation/tests/composition/compose_inaccessible.rs index 3ffde68d6c..5105d06fe9 100644 --- a/apollo-federation/tests/composition/compose_inaccessible.rs +++ b/apollo-federation/tests/composition/compose_inaccessible.rs @@ -2,6 +2,7 @@ use apollo_compiler::coord; use apollo_compiler::name; use apollo_compiler::schema::ExtendedType; use apollo_federation::composition::compose; +use apollo_federation::error::CompositionError; use apollo_federation::subgraph::typestate::Subgraph; use test_log::test; @@ -297,6 +298,85 @@ fn inaccessible_errors_if_subgraph_misuses_inaccessible() { ); } +/// API-schema `@inaccessible` validation runs after merge; [`CompositionError::MergeError`] should +/// carry subgraph source ranges (Rust port of JS `updateInaccessibleErrorsWithLinkToSubgraphs`). +#[test] +fn referenced_inaccessible_merge_error_includes_subgraph_locations() { + let subgraph_a = ServiceDefinition { + name: "subgraphA", + type_defs: r#" + type Query { + q1: Int + q2: A + } + + type A @shareable { + x: Int + y: Int + } + "#, + }; + + let subgraph_b = ServiceDefinition { + name: "subgraphB", + type_defs: r#" + type A @shareable @inaccessible { + x: Int + y: Int + } + "#, + }; + + let result = compose_as_fed2_subgraphs(&[subgraph_a, subgraph_b]); + let errors = result.expect_err("Expected composition to fail"); + + let merge_errors: Vec<_> = errors + .iter() + .filter_map(|e| match e { + CompositionError::MergeError { error, locations } => { + Some((error, locations.as_slice())) + } + _ => None, + }) + .collect(); + + assert!( + !merge_errors.is_empty(), + "Expected at least one MergeError from API schema validation, got {errors:#?}" + ); + + let mut seen_subgraphs: Vec<&str> = merge_errors + .iter() + .flat_map(|(_, locs)| locs.iter().map(|l| l.subgraph.as_str())) + .collect(); + seen_subgraphs.sort_unstable(); + seen_subgraphs.dedup(); + + assert!( + seen_subgraphs.contains(&"subgraphA"), + "Expected referencer location in subgraphA, saw subgraphs {seen_subgraphs:?}. Errors: {merge_errors:#?}" + ); + assert!( + seen_subgraphs.contains(&"subgraphB"), + "Expected @inaccessible definition location in subgraphB, saw subgraphs {seen_subgraphs:?}. Errors: {merge_errors:#?}" + ); + + for (err, locs) in &merge_errors { + assert!( + !locs.is_empty(), + "Each MergeError from this path should include at least one SubgraphLocation: {err}" + ); + for loc in *locs { + let (sl, sc) = (loc.range.start.line, loc.range.start.column); + let (el, ec) = (loc.range.end.line, loc.range.end.column); + assert!( + el > sl || (el == sl && ec > sc), + "Expected non-empty source range for {loc:?}" + ); + } + } +} + #[test] fn inaccessible_uses_security_core_purpose_in_supergraph() { let subgraph_a = ServiceDefinition {