Skip to content

Commit a5d95c0

Browse files
tnineslingclaude
andcommitted
fix: restore origin_to_use() for link spec @link directive placement
The beta1 Component->Node migration replaced `Component { origin: schema_definition.origin_to_use(), ... }` with `Node::new(...)`, losing the logic that chose whether the link-to-link @link directive should go on the schema definition or an extension. Restore the origin_to_use() call in add_to_schema so the directive gets the correct ExtensionId, matching pre-upgrade serialization behavior. Also remove the promote_link_spec_directive workaround that was papering over the symptom in expand_schema. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 40ef934 commit a5d95c0

3 files changed

Lines changed: 12 additions & 32 deletions

File tree

‎apollo-federation/src/link/link_spec_definition.rs‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ use crate::link::spec::Version;
3030
use crate::link::spec_definition::SpecDefinition;
3131
use crate::link::spec_definition::SpecDefinitions;
3232
use crate::schema::FederationSchema;
33+
use crate::schema::SchemaElement;
3334
use crate::schema::position::SchemaDefinitionPosition;
3435
use crate::schema::type_and_directive_specification::ArgumentSpecification;
3536
use crate::schema::type_and_directive_specification::DirectiveArgumentSpecification;
@@ -586,10 +587,16 @@ impl LinkSpecDefinition {
586587
}));
587588
}
588589

590+
let directive = Node::new(Directive { name, arguments });
591+
let directive = match SchemaDefinitionPosition
592+
.get(schema.schema())
593+
.origin_to_use()
594+
{
595+
Some(ext_id) => directive.with_extension_id(ext_id),
596+
None => directive,
597+
};
589598
SchemaDefinitionPosition.insert_directive_at(
590-
schema,
591-
Node::new(Directive { name, arguments }),
592-
0, // @link to link spec should be first
599+
schema, directive, 0, // @link to link spec should be first
593600
)?;
594601
Ok(())
595602
}

‎apollo-federation/src/subgraph/typestate.rs‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -951,12 +951,6 @@ pub(crate) fn expand_schema(schema: Schema) -> Result<FederationSchema, Federati
951951
trace!("new_federation_subgraph_schema: collect_links_metadata");
952952
schema.collect_links_metadata()?;
953953

954-
// The link spec's @link directive should always live on the base schema definition
955-
// (not an extension) so it serializes as `schema @link(...) { ... }`. When the input
956-
// only has `extend schema @link(...) @link(...)`, the parser gives every directive an
957-
// ExtensionId. Promote the link-spec @link to the base definition here.
958-
promote_link_spec_directive(schema.schema_mut());
959-
960954
// Now we fill in the missing definitions
961955
trace!("expand_links: on_directive_definition_and_schema_parsed");
962956
FederationBlueprint::on_directive_definition_and_schema_parsed(&mut schema)?;
@@ -1014,27 +1008,6 @@ pub(crate) fn has_federation_spec_link(schema: &Schema) -> bool {
10141008
.any(|d| is_fed_spec_link_directive(schema, d))
10151009
}
10161010

1017-
/// If the `@link(url: "https://specs.apollo.dev/link/...")` directive on the schema definition
1018-
/// has an ExtensionId (because it came from `extend schema`), replace it with a copy that has
1019-
/// no ExtensionId so it serializes on the base `schema { ... }` definition.
1020-
fn promote_link_spec_directive(schema: &mut Schema) {
1021-
let link_identity_prefix = Identity::link_identity().to_string();
1022-
let idx = schema.schema_definition.directives.iter().position(|d| {
1023-
d.name == LINK_DIRECTIVE_NAME_IN_SPEC
1024-
&& d.extension_id().is_some()
1025-
&& d.arguments
1026-
.iter()
1027-
.find(|a| a.name == LINK_DIRECTIVE_URL_ARGUMENT_NAME.as_str())
1028-
.and_then(|a| a.value.as_str())
1029-
.is_some_and(|url| url.starts_with(&link_identity_prefix))
1030-
});
1031-
if let Some(idx) = idx {
1032-
let directive = &schema.schema_definition.directives[idx];
1033-
let promoted = directive.same_location((**directive).clone());
1034-
schema.schema_definition.make_mut().directives[idx] = promoted;
1035-
}
1036-
}
1037-
10381011
fn is_fed_spec_link_directive(schema: &Schema, directive: &Directive) -> bool {
10391012
if directive.name != LINK_DIRECTIVE_NAME_IN_SPEC {
10401013
return false;

‎apollo-federation/tests/subgraph/subgraph_validation_tests.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1056,11 +1056,11 @@ mod link_handling_tests {
10561056
// There are a few whitespace differences between this and the JS version, but the more important difference is that
10571057
// the links are added as a new extension instead of being attached to the top-level schema definition. We may need
10581058
// to revisit that later if we're doing strict comparisons of SDLs between versions.
1059-
const EXPECTED_FULL_SCHEMA: &str = r#"schema @link(url: "https://specs.apollo.dev/link/v1.0") {
1059+
const EXPECTED_FULL_SCHEMA: &str = r#"schema {
10601060
query: Query
10611061
}
10621062
1063-
extend schema @link(url: "https://specs.apollo.dev/federation/v2.0", import: ["@key"])
1063+
extend schema @link(url: "https://specs.apollo.dev/link/v1.0") @link(url: "https://specs.apollo.dev/federation/v2.0", import: ["@key"])
10641064
10651065
directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA
10661066

0 commit comments

Comments
 (0)