Skip to content
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

fix(federation): do not generate fragments without type condition #6111

Merged
merged 3 commits into from
Oct 3, 2024
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
14 changes: 14 additions & 0 deletions apollo-federation/src/operation/optimize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1684,6 +1684,20 @@ impl FragmentGenerator {
SelectionValue::InlineFragment(mut candidate) => {
self.visit_selection_set(candidate.get_selection_set_mut())?;

// XXX(@goto-bus-stop): This is temporary to support mismatch testing with JS!
// JS federation does not consider fragments without a type condition.
if candidate
.get()
.inline_fragment
.type_condition_position
.is_none()
{
new_selection_set.add_local_selection(&Selection::InlineFragment(
Arc::clone(candidate.get()),
))?;
continue;
}

let directives = &candidate.get().inline_fragment.directives;
let skip_include = directives
.iter()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,9 @@ fn it_handles_fragments_with_one_non_leaf_field() {
);
}

/// XXX(@goto-bus-stop): this test is meant to check that fragments with @skip and @include *are*
/// migrated. But we are currently matching JS behavior, where they are not. This test should be
/// updated when we remove JS compatibility.
#[test]
fn it_migrates_skip_include() {
let planner = planner!(
Expand Down Expand Up @@ -348,6 +351,50 @@ fn fragments_that_share_a_hash_but_are_not_identical_generate_their_own_fragment
);
}

#[test]
fn same_as_js_router798() {
let planner = planner!(
config = QueryPlannerConfig { generate_query_fragments: true, reuse_query_fragments: false, ..Default::default() },
Subgraph1: r#"
interface Interface { a: Int }
type Y implements Interface { a: Int b: Int }
type Z implements Interface { a: Int c: Int }

type Query {
interfaces(id: Int!): Interface
}
"#,
);
assert_plan!(
&planner,
r#"
query($var0: Boolean! = true) {
... @skip(if: $var0) {
field0: interfaces(id: 0) {
field1: __typename
}
}
}
"#,
@r###"
QueryPlan {
Skip(if: $var0) {
Fetch(service: "Subgraph1") {
{
... {
field0: interfaces(id: 0) {
__typename
field1: __typename
}
}
}
},
},
}
"###
);
}

#[test]
fn works_with_key_chains() {
let planner = planner!(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# Composed from subgraphs with hash: ee7fce9eb672edf9b036a25bcae0b056ccf5f451
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
@link(url: "https://specs.apollo.dev/join/v0.4", for: EXECUTION)
{
query: Query
}

directive @join__directive(graphs: [join__Graph!], name: String!, args: join__DirectiveArguments) repeatable on SCHEMA | OBJECT | INTERFACE | FIELD_DEFINITION

directive @join__enumValue(graph: join__Graph!) repeatable on ENUM_VALUE

directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean, overrideLabel: String) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION

directive @join__graph(name: String!, url: String!) on ENUM_VALUE

directive @join__implements(graph: join__Graph!, interface: String!) repeatable on OBJECT | INTERFACE

directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true, isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR

directive @join__unionMember(graph: join__Graph!, member: String!) repeatable on UNION

directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA

interface Interface
@join__type(graph: SUBGRAPH1)
{
a: Int
}

scalar join__DirectiveArguments

scalar join__FieldSet

enum join__Graph {
SUBGRAPH1 @join__graph(name: "Subgraph1", url: "none")
}

scalar link__Import

enum link__Purpose {
"""
`SECURITY` features provide metadata necessary to securely resolve fields.
"""
SECURITY

"""
`EXECUTION` features provide metadata necessary for operation execution.
"""
EXECUTION
}

type Query
@join__type(graph: SUBGRAPH1)
{
interfaces(id: Int!): Interface
}

type Y implements Interface
@join__implements(graph: SUBGRAPH1, interface: "Interface")
@join__type(graph: SUBGRAPH1)
{
a: Int
b: Int
}

type Z implements Interface
@join__implements(graph: SUBGRAPH1, interface: "Interface")
@join__type(graph: SUBGRAPH1)
{
a: Int
c: Int
}
Loading