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

Directives on QUERY, MUTATION, or SUBSCRIPTION are not passed to subgraphs #2961

Closed
lleadbet opened this issue Mar 18, 2024 · 2 comments
Closed
Assignees

Comments

@lleadbet
Copy link
Contributor

lleadbet commented Mar 18, 2024

Describe the bug

When using a directive that uses one of QUERY, MUTATION, or SUBSCRIPTION, the router strips it before sending the request to the subgraph.

To Reproduce

Given the below two subgraphs:

Subgraph 1:

directive @operation on MUTATION | QUERY | SUBSCRIPTION
directive @field on FIELD

type Foo @key(fields: "id") {
  id: ID!
}
type Query {
  foo: [Foo]
}

Subgraph 2:

directive @operation on MUTATION | QUERY | SUBSCRIPTION
directive @field on FIELD

type Foo @key(fields: "id") {
  id: ID!
  bar: String
}

It will generate the following query plan:

QueryPlan {
  Sequence {
    Fetch(service: "foo") {
      {
        foo @field {
          __typename
          id
        }
      }
    },
    Flatten(path: "foo.@") {
      Fetch(service: "bar") {
        {
          ... on Foo {
            __typename
            id
          }
        } =>
        {
          ... on Foo {
            bar @field
          }
        }
      },
    },
  },
}

With this query:

query Operation @operation{
  foo @field {
    bar @field
  }
}

We can see that the FIELD directive works, however the one using the operation-level location is not.

Expected behavior

The operation-level directives are persisted to the initial subgraph.

@dariuszkuc dariuszkuc transferred this issue from apollographql/router Mar 19, 2024
@abernix
Copy link
Member

abernix commented Mar 20, 2024

@lleadbet We moved this to the federation repository as this appears to be a query planner issue (i know you know this, but for broad context: query planner runs inside the router, but lives in this repo right now)

duckki added a commit to duckki/federation that referenced this issue Mar 23, 2024
duckki added a commit that referenced this issue Mar 25, 2024
…subgraph queries (#2967)

- added "directives" field to the class `Operation`.
- updated DocumentNode <-> Operation conversion functions to pass
directives.
- added plumbing of directives through query planner logic.

#2961
@duckki duckki self-assigned this Mar 25, 2024
@duckki
Copy link
Contributor

duckki commented Mar 25, 2024

I merged the PR (#2967) that fixes this issue.

@duckki duckki closed this as completed Mar 25, 2024
o0Ignition0o pushed a commit that referenced this issue Apr 16, 2024
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @apollo/composition@2.7.3

### Patch Changes

- Fix a query planning bug where invalid subgraph queries are generated
with `reuseQueryFragments` set true.
([#2952](#2952))
([#2963](#2963))

- Updated dependencies
\[[`ec04c50b4fb832bfd281ecf9c0c2dd7656431b96`](ec04c50),
[`a494631918156f0431ceace74281c076cf1d5d51`](a494631)]:
    -   @apollo/federation-internals@2.7.3
    -   @apollo/query-graphs@2.7.3

## @apollo/gateway@2.7.3

### Patch Changes

- Updated dependencies
\[[`ec04c50b4fb832bfd281ecf9c0c2dd7656431b96`](ec04c50),
[`3e2c845c74407a136b9e0066e44c1ad1467d3013`](3e2c845),
[`a494631918156f0431ceace74281c076cf1d5d51`](a494631)]:
    -   @apollo/query-planner@2.7.3
    -   @apollo/composition@2.7.3
    -   @apollo/federation-internals@2.7.3

## @apollo/federation-internals@2.7.3

### Patch Changes

- Fix a query planning bug where invalid subgraph queries are generated
with `reuseQueryFragments` set true.
([#2952](#2952))
([#2963](#2963))

- Fixed query planner to pass the directives from original query to
subgraph operations (#2961)
([#2967](#2967))

## @apollo/query-graphs@2.7.3

### Patch Changes

- Updated dependencies
\[[`ec04c50b4fb832bfd281ecf9c0c2dd7656431b96`](ec04c50),
[`a494631918156f0431ceace74281c076cf1d5d51`](a494631)]:
    -   @apollo/federation-internals@2.7.3

## @apollo/query-planner@2.7.3

### Patch Changes

- Fix a query planning bug where invalid subgraph queries are generated
with `reuseQueryFragments` set true.
([#2952](#2952))
([#2963](#2963))

- Type conditioned fetching
([#2949](#2949))

When querying a field that is in a path of 2 or more unions, the query
planner was not able to handle different selections and would
aggressively collapse selections in fetches yielding an incorrect plan.

This change introduces new syntax to express type conditions in (key and
flatten) paths. Type conditioned fetching can be enabled through a flag,
and execution is supported in the router only. (#2938)

- Fixed query planner to pass the directives from original query to
subgraph operations (#2961)
([#2967](#2967))

- Updated dependencies
\[[`ec04c50b4fb832bfd281ecf9c0c2dd7656431b96`](ec04c50),
[`a494631918156f0431ceace74281c076cf1d5d51`](a494631)]:
    -   @apollo/federation-internals@2.7.3
    -   @apollo/query-graphs@2.7.3

## @apollo/subgraph@2.7.3

### Patch Changes

- Updated dependencies
\[[`ec04c50b4fb832bfd281ecf9c0c2dd7656431b96`](ec04c50),
[`a494631918156f0431ceace74281c076cf1d5d51`](a494631)]:
    -   @apollo/federation-internals@2.7.3

## apollo-federation-integration-testsuite@2.7.3

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
duckki added a commit to duckki/federation that referenced this issue Apr 17, 2024
- Previously, directives applied on operations weren't properly attached to their parent elements.
- Changed the `Operation` class to inherit from `DirectiveTargetElement` class, which attaches directives properly.

Related: apollographql#2961
duckki added a commit that referenced this issue Apr 17, 2024
- Changed the `Operation` class to inherit from `DirectiveTargetElement`
class, which attaches directives properly.
- Previously, directives applied on operations weren't properly attached
to their parent elements.

Related: #2961
dariuszkuc pushed a commit that referenced this issue Apr 19, 2024
…n level are retained in subgraph queries (#2986)

This fixes another regression created by PR #2967.

Related: #2961
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants