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

1670 repeatable directives #1675

Merged
merged 6 commits into from
Jan 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
17 changes: 16 additions & 1 deletion docs/directives.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,20 @@
# Directives

## Custom Directives

You can add your own [GraphQL Directives](https://spec.graphql.org/draft/#sec-Language.Directives) by writing
a corresponding Java Annotation and annotate it as `@Directive`, e.g.:

```java
@Directive(on = { OBJECT, INTERFACE })
@Description("Just a test")
@Retention(RUNTIME)
public @interface MyDirective {
}
```

Directives can be repeatable, see the `@Key` annotation for an example.

## Directives generated from Bean Validation annotations

If your project uses Bean Validation to validate fields on input types and operation arguments, and you enable
Expand All @@ -23,4 +38,4 @@ BV annotations are listed here):

Note: The `@NotNull` annotation does not map to a directive, instead it makes the GraphQL type non-nullable.

Constraints will only appear on fields of input types and operation arguments.
Constraints will only appear on fields of input types and operation arguments.
8 changes: 5 additions & 3 deletions docs/federation.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Federation

To enable support for [GraphQL Federation](https://www.apollographql.com/docs/federation), simply set the `smallrye.graphql.federation.enabled` config key to `true`.
Support for [GraphQL Federation](https://www.apollographql.com/docs/federation) is enabled by default. If you add one of the federation annotations, the corresponding directives will be declared to your schema and the additional Federation queries will be added automatically. You can also disable Federation completely by setting the `smallrye.graphql.federation.enabled` config key to `false`.
Copy link
Member

@jmartisk jmartisk Jan 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabled by default is bad, IMO.
On the Quarkus side, I've already had a user complaining that they suddenly started seeing

type _Service {
  sdl: String!
}

in their application's schema after upgrading the Quarkus version, even though they don't have anything related to Federation in it, and it broke some of their tooling.
My plan is to make Quarkus detect automatically whether federation should be enabled by scanning through the application for any annotations from io.smallrye.graphql.api.federation, and enabling if found. Of course, you will be able to override this automatic enabling using configuration.

This behavior is Quarkus specific though. WildFly can probably implement something similar if we want. So I'm not sure what to write in the general SmallRye documentation, because it potentially depends on the runtime that you're using. If we make both Quarkus and WildFly behave this way, I would say we can describe this automatic enabling in the SmallRye documentation. WDYT?

Copy link
Member

@jmartisk jmartisk Jan 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So your current text is kinda correct in the sense that if you add a Federation annotation, it will appear in the schema, but it's wrong to say that "Federation is enabled by default", because we don't want to include the type _Service stuff by default. And we won't include federation directive type declarations in the schema either- I have submitted #1678 to address this (don't worry that the PR is to a different branch, I will forward/back-port everything once we merge this and that).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original suggestion to "enable" it by default was from @phillip-kruger, actually... where "enable" means automatic scanning, so the _service and _entities queries only appear when necessary. I thought this was already so... also in JEE containers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now, with Quarkus, that stuff appears always by default, even when the application doesn't have any Federation in it. I am going to fix that.
Ok, re-reading your text after this clarification, I think it is fine. It will be enabled in the sense that if you add an annotation, you will get directives and queries. At least this is how it will work after some adjustments in Quarkus+WildFly.


You can add the Federation directives by using the equivalent Java annotation, e.g. to extend a `Product` entity with a `price` field, you can write a class:

Expand Down Expand Up @@ -37,15 +37,15 @@ import org.eclipse.microprofile.graphql.Query;
public class Prices {
@Query
public Product product(@Id String id) {
return ...;
return ...
}
}
```

The GraphQL Schema then contains:

```graphql
type Product @extends @key(fields : ["id"]) {
type Product @extends @key(fields : "id") {
id: ID
price: Int
}
Expand All @@ -58,3 +58,5 @@ type Query {
product(id: ID): Product
}
```

If you can resolve, e.g., the product with different types of ids, you can add multiple `@Key` annotations.