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): match JS condition order with multiple skip/include on the same selection #6120

Merged
merged 3 commits into from
Oct 7, 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
18 changes: 1 addition & 17 deletions apollo-federation/src/operation/directive_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,13 +288,6 @@ impl DirectiveList {
.iter()
}

/// Iterate the directives in a consistent sort order.
pub(crate) fn iter_sorted(&self) -> DirectiveIterSorted<'_> {
self.inner
.as_ref()
.map_or_else(DirectiveIterSorted::empty, |inner| inner.iter_sorted())
}

/// Remove one directive application by name.
///
/// To remove a repeatable directive, you may need to call this multiple times.
Expand Down Expand Up @@ -336,7 +329,7 @@ impl DirectiveList {
}

/// Iterate over a [`DirectiveList`] in a consistent sort order.
pub(crate) struct DirectiveIterSorted<'a> {
struct DirectiveIterSorted<'a> {
directives: &'a [Node<executable::Directive>],
inner: std::slice::Iter<'a, usize>,
}
Expand All @@ -354,15 +347,6 @@ impl ExactSizeIterator for DirectiveIterSorted<'_> {
}
}

impl DirectiveIterSorted<'_> {
fn empty() -> Self {
Self {
directives: &[],
inner: [].iter(),
}
}
}

#[cfg(test)]
mod tests {
use std::collections::HashSet;
Expand Down
2 changes: 1 addition & 1 deletion apollo-federation/src/query_plan/conditions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl Conditions {

pub(crate) fn from_directives(directives: &DirectiveList) -> Result<Self, FederationError> {
let mut variables = IndexMap::default();
for directive in directives.iter_sorted() {
for directive in directives.iter() {
let negated = match directive.name.as_str() {
"include" => false,
"skip" => true,
Expand Down
40 changes: 40 additions & 0 deletions apollo-federation/tests/query_plan/build_query_plan_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1294,3 +1294,43 @@ fn handles_multiple_conditions_on_abstract_types() {
"###
);
}

#[test]
fn condition_order_router799() {
let planner = planner!(
books: r#"
type Query {
bookName: String!
}
type Mutation {
bookName(name: String!): Int!
}
"#,
);

assert_plan!(
&planner,
r#"
mutation($var0: Boolean! = true, $var1: Boolean!) {
... on Mutation @skip(if: $var0) @include(if: $var1) {
field0: __typename
}
}
"#,
@r###"
QueryPlan {
Include(if: $var1) {
Skip(if: $var0) {
Fetch(service: "books") {
{
... on Mutation {
field0: __typename
}
}
},
},
},
}
"###
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Composed from subgraphs with hash: 3ebe0e8c55ad21763879da6341617f14953e80d7
schema
@link(url: "https://specs.apollo.dev/link/v1.0")
@link(url: "https://specs.apollo.dev/join/v0.4", for: EXECUTION)
{
query: Query
mutation: Mutation
}

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

scalar join__DirectiveArguments

scalar join__FieldSet

enum join__Graph {
BOOKS @join__graph(name: "books", 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 Mutation
@join__type(graph: BOOKS)
{
bookName(name: String!): Int!
}

type Query
@join__type(graph: BOOKS)
{
bookName: String!
}
Loading