Skip to content

Clean up Connector to enable listing IDs via supergraph: #7918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 19 additions & 39 deletions apollo-federation/src/connectors/expand/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use crate::ValidFederationSubgraph;
use crate::connectors::ConnectSpec;
use crate::connectors::Connector;
use crate::error::FederationError;
use crate::link::Link;
use crate::merge::merge_subgraphs;
use crate::schema::FederationSchema;
use crate::subgraph::Subgraph;
Expand All @@ -24,6 +23,8 @@ mod carryover;
pub(crate) mod visitors;
use visitors::filter_directives;

use crate::connectors::spec::ConnectLink;

pub struct Connectors {
pub by_service_name: Arc<IndexMap<Arc<str>, Connector>>,
pub labels_by_service_name: Arc<IndexMap<Arc<str>, String>>,
Expand Down Expand Up @@ -67,22 +68,18 @@ pub fn expand_connectors(
let (connect_subgraphs, graphql_subgraphs): (Vec<_>, Vec<_>) = supergraph
.extract_subgraphs()?
.into_iter()
.partition_map(
|(_, sub)| match ConnectSpec::get_from_schema(sub.schema.schema()) {
Some((spec, link)) if contains_connectors(&link, &sub) => {
either::Either::Left((spec, link, sub))
}
_ => either::Either::Right(ValidSubgraph::from(sub)),
},
);
.partition_map(|(_, sub)| match ConnectLink::new(sub.schema.schema()) {
Some(Ok(link)) if contains_connectors(&link, &sub) => either::Either::Left((link, sub)),
_ => either::Either::Right(ValidSubgraph::from(sub)),
});

// Expand just the connector subgraphs
let mut expanded_subgraphs = Vec::new();
let mut spec_versions = HashSet::new();

for (spec, link, sub) in connect_subgraphs {
expanded_subgraphs.extend(split_subgraph(&link, sub, spec)?);
spec_versions.insert(spec);
for (link, sub) in connect_subgraphs {
expanded_subgraphs.extend(split_subgraph(&link, sub)?);
spec_versions.insert(link.spec);
}

// Merge the subgraphs into one supergraph
Expand Down Expand Up @@ -120,7 +117,7 @@ pub fn expand_connectors(

let labels_by_service_name = connectors_by_service_name
.iter()
.map(|(service_name, connector)| (service_name.clone(), connector.id.label.clone()))
.map(|(service_name, connector)| (service_name.clone(), connector.label.0.clone()))
.collect();

let source_config_keys = connectors_by_service_name
Expand All @@ -139,27 +136,24 @@ pub fn expand_connectors(
})
}

fn contains_connectors(link: &Link, subgraph: &ValidFederationSubgraph) -> bool {
let connect_name = ConnectSpec::connect_directive_name(link);
let source_name = ConnectSpec::source_directive_name(link);

fn contains_connectors(link: &ConnectLink, subgraph: &ValidFederationSubgraph) -> bool {
subgraph
.schema
.get_directive_definitions()
.any(|directive| {
directive.directive_name == connect_name || directive.directive_name == source_name
directive.directive_name == link.connect_directive_name
|| directive.directive_name == link.source_directive_name
})
}

/// Split up a subgraph so that each connector directive becomes its own subgraph.
///
/// Subgraphs passed to this function should contain connector directives.
fn split_subgraph(
link: &Link,
link: &ConnectLink,
subgraph: ValidFederationSubgraph,
spec: ConnectSpec,
) -> Result<Vec<(Connector, ValidSubgraph)>, FederationError> {
let connector_map = Connector::from_schema(subgraph.schema.schema(), &subgraph.name, spec)?;
let connector_map = Connector::from_schema(subgraph.schema.schema(), &subgraph.name)?;

let expander = helpers::Expander::new(link, &subgraph);
connector_map
Expand Down Expand Up @@ -219,14 +213,13 @@ mod helpers {
use super::visitors::try_insert;
use super::visitors::try_pre_insert;
use crate::ValidFederationSubgraph;
use crate::connectors::ConnectSpec;
use crate::connectors::Connector;
use crate::connectors::EntityResolver;
use crate::connectors::JSONSelection;
use crate::connectors::id::ConnectedElement;
use crate::connectors::spec::ConnectLink;
use crate::error::FederationError;
use crate::internal_error;
use crate::link::Link;
use crate::link::spec::Identity;
use crate::schema::FederationSchema;
use crate::schema::ValidFederationSchema;
Expand All @@ -243,14 +236,6 @@ mod helpers {

/// A helper struct for expanding a subgraph into one per connect directive.
pub(super) struct Expander<'a> {
/// The name of the connect directive, possibly aliased.
#[allow(unused)]
connect_name: Name,

/// The name of the connect directive, possibly aliased.
#[allow(unused)]
source_name: Name,

/// The name of the @key directive, as known in the subgraph
key_name: Name,

Expand All @@ -266,10 +251,7 @@ mod helpers {
}

impl<'a> Expander<'a> {
pub(super) fn new(link: &Link, subgraph: &'a ValidFederationSubgraph) -> Self {
let connect_name = ConnectSpec::connect_directive_name(link);
let source_name = ConnectSpec::source_directive_name(link);

pub(super) fn new(link: &ConnectLink, subgraph: &'a ValidFederationSubgraph) -> Self {
// When we go to expand all output types, we'll need to make sure that we don't carry over
// any connect-related directives. The following directives are also special because they
// influence planning and satisfiability:
Expand Down Expand Up @@ -307,13 +289,11 @@ mod helpers {
});
let directive_deny_list = IndexSet::from_iter(extra_excluded.chain([
key_name.clone(),
connect_name.clone(),
source_name.clone(),
link.connect_directive_name.clone(),
link.source_directive_name.clone(),
]));

Self {
connect_name,
source_name,
key_name,
interface_object_name,
original_schema: &subgraph.schema,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/batch.g
{
"one_Query_users_0": Connector {
id: ConnectId {
label: "one.json http: GET /users",
subgraph_name: "one",
source_name: Some(
"json",
Expand Down Expand Up @@ -87,10 +86,12 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/batch.g
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"one.json http: GET /users",
),
},
"one_User_0": Connector {
id: ConnectId {
label: "[BATCH] one.json http: POST /users-batch",
subgraph_name: "one",
source_name: Some(
"json",
Expand Down Expand Up @@ -265,5 +266,8 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/batch.g
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"[BATCH] one.json http: POST /users-batch",
),
},
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/carryov
{
"one_Query_ts_0": Connector {
id: ConnectId {
label: "one.json http: GET /t",
subgraph_name: "one",
source_name: Some(
"json",
Expand Down Expand Up @@ -171,10 +170,12 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/carryov
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"one.json http: GET /t",
),
},
"one_Query_t_0": Connector {
id: ConnectId {
label: "one.json http: GET /t/{$args.id}",
subgraph_name: "one",
source_name: Some(
"json",
Expand Down Expand Up @@ -388,10 +389,12 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/carryov
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"one.json http: GET /t/{$args.id}",
),
},
"one_T_r_0": Connector {
id: ConnectId {
label: "one.json http: GET /t/{$this.id}/r",
subgraph_name: "one",
source_name: Some(
"json",
Expand Down Expand Up @@ -527,5 +530,8 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/carryov
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"one.json http: GET /t/{$this.id}/r",
),
},
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/interfa
{
"connectors_Itf_e_0": Connector {
id: ConnectId {
label: "connectors.json http: GET /itfs/{$this.id}/e",
subgraph_name: "connectors",
source_name: Some(
"json",
Expand Down Expand Up @@ -144,10 +143,12 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/interfa
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"connectors.json http: GET /itfs/{$this.id}/e",
),
},
"connectors_Query_itfs_0": Connector {
id: ConnectId {
label: "connectors.json http: GET /itfs",
subgraph_name: "connectors",
source_name: Some(
"json",
Expand Down Expand Up @@ -240,10 +241,12 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/interfa
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"connectors.json http: GET /itfs",
),
},
"connectors_Query_itf_0": Connector {
id: ConnectId {
label: "connectors.json http: GET /itfs/{$args.id}",
subgraph_name: "connectors",
source_name: Some(
"json",
Expand Down Expand Up @@ -397,5 +400,8 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/interfa
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"connectors.json http: GET /itfs/{$args.id}",
),
},
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/keys.gr
{
"one_Query_t_0": Connector {
id: ConnectId {
label: "one. http: GET http://localhost/ts/{$args.id}",
subgraph_name: "one",
source_name: None,
named: None,
Expand Down Expand Up @@ -147,10 +146,12 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/keys.gr
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"one. http: GET http://localhost/ts/{$args.id}",
),
},
"one_Query_t2_0": Connector {
id: ConnectId {
label: "one. http: GET http://localhost/ts/{$args.id}?id2={$args.id2}",
subgraph_name: "one",
source_name: None,
named: None,
Expand Down Expand Up @@ -341,10 +342,12 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/keys.gr
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"one. http: GET http://localhost/ts/{$args.id}?id2={$args.id2}",
),
},
"one_Query_unselected_0": Connector {
id: ConnectId {
label: "one. http: GET http://localhost/ts/{$args.unselected}",
subgraph_name: "one",
source_name: None,
named: None,
Expand Down Expand Up @@ -485,10 +488,12 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/keys.gr
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"one. http: GET http://localhost/ts/{$args.unselected}",
),
},
"one_T_r1_0": Connector {
id: ConnectId {
label: "one. http: GET http://localhost/rs/{$this.id}",
subgraph_name: "one",
source_name: None,
named: None,
Expand Down Expand Up @@ -617,10 +622,12 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/keys.gr
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"one. http: GET http://localhost/rs/{$this.id}",
),
},
"one_T_r2_0": Connector {
id: ConnectId {
label: "one. http: GET http://localhost/rs/{$this.id}?id2={$this.id2}",
subgraph_name: "one",
source_name: None,
named: None,
Expand Down Expand Up @@ -799,10 +806,12 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/keys.gr
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"one. http: GET http://localhost/rs/{$this.id}?id2={$this.id2}",
),
},
"one_T_r3_0": Connector {
id: ConnectId {
label: "one. http: GET http://localhost/rs/{$this.id}",
subgraph_name: "one",
source_name: None,
named: None,
Expand Down Expand Up @@ -977,10 +986,12 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/keys.gr
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"one. http: GET http://localhost/rs/{$this.id}",
),
},
"one_T_r4_0": Connector {
id: ConnectId {
label: "one. http: POST http://localhost/rs",
subgraph_name: "one",
source_name: None,
named: None,
Expand Down Expand Up @@ -1130,10 +1141,12 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/keys.gr
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"one. http: POST http://localhost/rs",
),
},
"one_T_r5_0": Connector {
id: ConnectId {
label: "one. http: POST http://localhost/rs",
subgraph_name: "one",
source_name: None,
named: None,
Expand Down Expand Up @@ -1329,5 +1342,8 @@ input_file: apollo-federation/src/connectors/expand/tests/schemas/expand/keys.gr
connect_extensions: None,
connect_is_success: None,
},
label: Label(
"one. http: POST http://localhost/rs",
),
},
}
Loading