-
Notifications
You must be signed in to change notification settings - Fork 345
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
generator: support optional directive arguments #1830
Labels
module: generator
Issue affects the schema generator and federation code
type: enhancement
New feature or request
Comments
dariuszkuc
added a commit
to dariuszkuc/graphql-kotlin
that referenced
this issue
Sep 11, 2023
Adds support for [Federation v2.5](https://www.apollographql.com/docs/federation/federation-versions#v25). Adds new `willApplyDirective`/`didApplyDirective` hooks that can be used to customize transformation of directive definition to applied directive. JVM does not support nested arrays as annotation arguments so we need to apply custom hooks to generate valid `@requiresScopes` directive. New hooks can also be used to filter out default arguments (ExpediaGroup#1830). New federation directives - `@authenticated` - indicates that target element is only accessible to the authenticated supergraph users - `@requiresScopes` - indicates that target element is only accessible to the authenticated supergraph users with the appropriate JWT scopes
dariuszkuc
added a commit
to dariuszkuc/graphql-kotlin
that referenced
this issue
Sep 11, 2023
Adds support for [Federation v2.5](https://www.apollographql.com/docs/federation/federation-versions#v25). Adds new `willApplyDirective`/`didApplyDirective` hooks that can be used to customize transformation of directive definition to applied directive. JVM does not support nested arrays as annotation arguments so we need to apply custom hooks to generate valid `@requiresScopes` directive. New hooks can also be used to filter out default arguments (ExpediaGroup#1830). New federation directives - `@authenticated` - indicates that target element is only accessible to the authenticated supergraph users - `@requiresScopes` - indicates that target element is only accessible to the authenticated supergraph users with the appropriate JWT scopes
dariuszkuc
added a commit
to dariuszkuc/graphql-kotlin
that referenced
this issue
Sep 11, 2023
Adds support for [Apollo Federation v2.5](https://www.apollographql.com/docs/federation/federation-versions#v25). Adds new `willApplyDirective`/`didApplyDirective` hooks that can be used to customize transformation of directive definition to applied directive. JVM does not support nested arrays as annotation arguments so we need to apply custom hooks to generate valid `@requiresScopes` directive. New hooks can also be used to filter out default arguments (ExpediaGroup#1830). New federation directives - `@authenticated` - indicates that target element is only accessible to the authenticated supergraph users - `@requiresScopes` - indicates that target element is only accessible to the authenticated supergraph users with the appropriate JWT scopes Note: due to the potential conflict on directive names we will no longer auto import federation directives. New directives will be auto-namespaced to the target spec. For backwards compatibility, we will continue auto-importing directives up to Federation version 2.3.
samuelAndalon
pushed a commit
that referenced
this issue
Sep 12, 2023
### 📝 Description Adds support for [Apollo Federation v2.5](https://www.apollographql.com/docs/federation/federation-versions#v25). Adds new `willApplyDirective`/`didApplyDirective` hooks that can be used to customize transformation of directive definition to applied directive. JVM does not support nested arrays as annotation arguments so we need to apply custom hooks to generate valid `@requiresScopes` directive. New hooks can also be used to filter out default arguments (#1830). New federation directives - `@authenticated` - indicates that target element is only accessible to the authenticated supergraph users - `@requiresScopes` - indicates that target element is only accessible to the authenticated supergraph users with the appropriate JWT scopes Note: due to the potential conflict on directive names we will no longer auto import federation directives. New directives will be auto-namespaced to the target spec. For backwards compatibility, we will continue auto-importing directives up to Federation version 2.3. ### 🔗 Related Issues #1830
This is partially resolved by #1839. Keeping this issue open to track potential usability improvements in the future. |
dariuszkuc
added
the
module: generator
Issue affects the schema generator and federation code
label
Sep 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
module: generator
Issue affects the schema generator and federation code
type: enhancement
New feature or request
Is your feature request related to a problem? Please describe.
Directives are defined as annotations which works great in many cases but we are also constrained by the JVM limitations. Currently JVM does not support
null
value for annotation attribute so it is not possible to define directive with nullable arguments by using annotation only approach. While we can create custom directive definition usingwillGenerateDirective
hook, annotation arguments cannot be null so it is not possible to apply the annotation with nullable attributes.We cannot use the existing
OptionalInput
approach either as JVM has constraints on annotation attributes - attributes can only be a primitive value, String, Enum, Class, Annotation or an array of any of the above. Furthermore, we cannot just recreate genericOptionalInput
as an annotation because of A) lack of inheritance support in annotations and B) generic annotation attributes are not supported.Describe the solution you'd like
Schema generator should provide support for creating and applying directives with nullable arguments. Ideally we wouldn't need any custom hooks to do it and everything would be derived from annotation attributes.
Describe alternatives you've considered
--- Alternative # 1 ---
We could potentially create specific wrappers for each supported attribute type (e.g.
@OptionalStringArg(val value = "", val defined: Boolean = false)
).Drawbacks:
--- Alternative # 2 ---
Since we can provide our own directive definition using hooks, we could potentially introduce other hooks to allow for filtering directive arguments , e.g.
Drawbacks:
willGenerateDirective
directiveName
would have to be the original one before the renames--- Alternative # 3 ---
OR we could create a hook to transform final applied directive (currently hooks only execute around generation of directive definitions), e.g.
Drawbacks:
willGenerateDirective
isValidDirectiveArgumentValue
hook) but it is a very flexible approachAdditional context
Example use case from Apollo Federation spec -
@link
directive defines single required argumentbut we cannot apply the directive without providing values for all optional fields, i.e. we cannot represent the basic link when there are no imports and namespace is derived. Ideally we would like to be able to specify just
Since this is not possible, currently users will always have to provide ALL optional values
The text was updated successfully, but these errors were encountered: