-
Notifications
You must be signed in to change notification settings - Fork 66
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
feat: federation 2.5 support #347
Conversation
Adds support for Federation specification v2.5. Changes: * new `@authenticated` directive ```graphql directive @authenticated on ENUM | FIELD_DEFINITION | INTERFACE | OBJECT | SCALAR ``` * new `@requiresScopes` directive ```graphql directive @requiresScopes(scopes: [[Scope!]!]!) on ENUM | FIELD_DEFINITION | INTERFACE | OBJECT | SCALAR scalar Scope ```
e71e47b
to
3ad69af
Compare
extend schema @link(url: "https://specs.apollo.dev/federation/v2.5", import: ["@key"]) | ||
|
||
directive @authenticated(role: [String!]!) on FIELD_DEFINITION | ||
|
||
type Product @key(fields: "id") { | ||
id: ID! | ||
name: String! | ||
supplier: String @authenticated(role: ["manager"]) | ||
} | ||
|
||
type Query { | ||
product(id: ID!): Product | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sure it's less of a concern for requiresScopes
but might be nice to have a similar test for completeness.
| OBJECT | ||
| SCALAR | ||
|
||
scalar Scope |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure this needs to be federation__Scope
everywhere it's defined and referenced by @requiresScopes
. I would expect a composition error / incompatible definition for this.
Invalid definition for directive "@requiresScopes": argument "scopes" should have type "[[federation__Scope!]!]!" but found type "[[Scope!]!]!"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
federation__
namespace prefix should only be required if given directive/type is not explicitly imported (see the diff between the custom authenticated directive with no imports and default authorization with imports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I'm not really privy to the java-related changes but they seem reasonable.
Adds support for [Federation specification v2.5](https://www.apollographql.com/docs/federation/federation-versions#v25). Changes: * new `@authenticated` directive ```graphql directive @authenticated on ENUM | FIELD_DEFINITION | INTERFACE | OBJECT | SCALAR ``` * new `@requiresScopes` directive ```graphql directive @requiresScopes(scopes: [[Scope!]!]!) on ENUM | FIELD_DEFINITION | INTERFACE | OBJECT | SCALAR scalar Scope ```
Adds support for Federation specification v2.5.
Changes:
@authenticated
directive@requiresScopes
directive