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 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
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.AnnotationTarget;
Expand Down Expand Up @@ -76,7 +77,7 @@ private static Map<DotName, AnnotationInstance> getParentAnnotations(FieldInfo f
private static Map<DotName, AnnotationInstance> getParentAnnotations(ClassInfo classInfo) {
Map<DotName, AnnotationInstance> parentAnnotations = new HashMap<>();

for (AnnotationInstance classAnnotation : classInfo.classAnnotations()) {
for (AnnotationInstance classAnnotation : classInfo.declaredAnnotations()) {
parentAnnotations.putIfAbsent(classAnnotation.name(), classAnnotation);
}

Expand All @@ -95,7 +96,7 @@ private static Map<DotName, AnnotationInstance> getPackageAnnotations(ClassInfo
if (packageName != null) {
ClassInfo packageInfo = ScanningContext.getIndex().getClassByName(packageName);
if (packageInfo != null) {
for (AnnotationInstance packageAnnotation : packageInfo.classAnnotations()) {
for (AnnotationInstance packageAnnotation : packageInfo.declaredAnnotations()) {
packageAnnotations.putIfAbsent(packageAnnotation.name(), packageAnnotation);
}
}
Expand Down Expand Up @@ -178,7 +179,7 @@ public static Annotations getAnnotationsForClass(ClassInfo classInfo) {

Map<DotName, AnnotationInstance> annotationMap = new HashMap<>();

for (AnnotationInstance annotationInstance : classInfo.classAnnotations()) {
for (AnnotationInstance annotationInstance : classInfo.declaredAnnotations()) {
DotName name = annotationInstance.name();
annotationMap.put(name, annotationInstance);
}
Expand Down Expand Up @@ -390,6 +391,25 @@ public Optional<String> getOneOfTheseMethodParameterAnnotationsValue(DotName...
return Optional.empty();
}

/**
* Get a stream of that annotation, maybe empty if not present, maybe a stream of one, or maybe several, if it's repeatable.
*/
public Stream<AnnotationInstance> resolve(DotName name) {
var annotationInstance = annotationsMap.get(name);
if (annotationInstance == null) {
var repeatableType = ScanningContext.getIndex().getClassByName(name);
if (repeatableType.hasAnnotation(REPEATABLE)) {
DotName containerName = repeatableType.annotation(REPEATABLE).value().asClass().name();
AnnotationInstance containerAnnotation = annotationsMap.get(containerName);
if (containerAnnotation != null) {
return Stream.of(containerAnnotation.value().asNestedArray());
}
}
return Stream.of();
}
return Stream.of(annotationInstance);
}

@Override
public String toString() {
return annotationsMap.toString();
Expand Down Expand Up @@ -558,6 +578,8 @@ private static Map<DotName, AnnotationInstance> getAnnotationsWithFilter(org.jbo

private static final short ZERO = 0;

public static final DotName REPEATABLE = DotName.createSimple("java.lang.annotation.Repeatable");

// SmallRye Common Annotations
public static final DotName BLOCKING = DotName.createSimple("io.smallrye.common.annotation.Blocking");
public static final DotName NON_BLOCKING = DotName.createSimple("io.smallrye.common.annotation.NonBlocking");
Expand Down Expand Up @@ -629,5 +651,4 @@ private static Map<DotName, AnnotationInstance> getAnnotationsWithFilter(org.jbo

//Kotlin NotNull
public static final DotName KOTLIN_NOT_NULL = DotName.createSimple("org.jetbrains.annotations.NotNull");

}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ public DirectiveType create(ClassInfo classInfo) {
directiveType.setClassName(classInfo.name().toString());
directiveType.setName(toDirectiveName(classInfo, annotations));
directiveType.setDescription(DescriptionHelper.getDescriptionForType(annotations).orElse(null));
directiveType.setLocations(getLocations(classInfo.classAnnotation(DIRECTIVE)));
directiveType.setLocations(getLocations(classInfo.declaredAnnotation(DIRECTIVE)));
directiveType.setRepeatable(classInfo.hasAnnotation(Annotations.REPEATABLE));

for (MethodInfo method : classInfo.methods()) {
DirectiveArgument argument = new DirectiveArgument();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,7 @@ protected static Type getReturnType(MethodInfo methodInfo) {
}

/**
* The the return type.This is usually the method return type, but can also be adapted to something else
*
* @param fieldInfo
* @return the return type
* The return type. This is usually the method return type, but can also be adapted to something else
*/
protected static Type getReturnType(FieldInfo fieldInfo) {
return fieldInfo.type();
Expand Down Expand Up @@ -100,8 +97,7 @@ private void doPopulateField(Direction direction, Field field, Type type, Annota

// Directives
if (directives != null) { // this happens while scanning for the directive types
field.addDirectiveInstances(
directives.buildDirectiveInstances(name -> annotations.getOneOfTheseAnnotations(name).orElse(null)));
field.addDirectiveInstances(directives.buildDirectiveInstances(annotations));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -74,15 +74,15 @@ public Type create(ClassInfo classInfo, Reference reference) {
addOperations(type, classInfo);

// Directives
addDirectives(type, classInfo);
addDirectives(type, annotations);

return type;
}

protected abstract void addFields(Type type, ClassInfo classInfo, Reference reference);

private void addDirectives(Type type, ClassInfo classInfo) {
type.setDirectiveInstances(directives.buildDirectiveInstances(classInfo::classAnnotation));
private void addDirectives(Type type, Annotations annotations) {
type.setDirectiveInstances(directives.buildDirectiveInstances(annotations));
}

private void addPolymorphicTypes(Type type, ClassInfo classInfo, Reference reference) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public EnumType create(ClassInfo classInfo, Reference reference) {
}

private List<DirectiveInstance> getDirectiveInstances(Annotations annotations) {
return directives.buildDirectiveInstances(dotName -> annotations.getOneOfTheseAnnotations(dotName).orElse(null));
return directives.buildDirectiveInstances(annotations);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

/**
* This creates an input type object.
*
*
* The input object has fields that might reference other types
* that should still be created.
*
Expand Down Expand Up @@ -138,7 +138,7 @@ public void setDirectives(Directives directives) {
}

private List<DirectiveInstance> getDirectiveInstances(Annotations annotations) {
return directives.buildDirectiveInstances(dotName -> annotations.getOneOfTheseAnnotations(dotName).orElse(null));
return directives.buildDirectiveInstances(annotations);
}

private void addFields(InputType inputType, ClassInfo classInfo, Reference reference) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
package io.smallrye.graphql.schema.helper;

import static java.util.stream.Collectors.toList;
import static org.jboss.jandex.AnnotationValue.Kind.ARRAY;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Function;

import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.AnnotationValue;
import org.jboss.jandex.DotName;

import io.smallrye.graphql.schema.Annotations;
import io.smallrye.graphql.schema.model.DirectiveInstance;
import io.smallrye.graphql.schema.model.DirectiveType;

Expand All @@ -37,21 +38,13 @@ public Directives(List<DirectiveType> directiveTypes) {
}
}

public List<DirectiveInstance> buildDirectiveInstances(Function<DotName, AnnotationInstance> getAnnotation) {
List<DirectiveInstance> result = null;
public List<DirectiveInstance> buildDirectiveInstances(Annotations annotations) {
// only build directive instances from `@Directive` annotations here (that means the `directiveTypes` map),
// because `directiveTypesOther` directives get their instances added on-the-go by classes that extend `ModelCreator`
for (DotName directiveTypeName : directiveTypes.keySet()) {
AnnotationInstance annotationInstance = getAnnotation.apply(directiveTypeName);
if (annotationInstance == null) {
continue;
}
if (result == null) {
result = new ArrayList<>();
}
result.add(toDirectiveInstance(annotationInstance));
}
return result;
return directiveTypes.keySet().stream()
.flatMap(annotations::resolve)
.map(this::toDirectiveInstance)
.collect(toList());
}

private DirectiveInstance toDirectiveInstance(AnnotationInstance annotationInstance) {
Expand Down
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.
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,15 @@
import io.smallrye.common.annotation.Experimental;
import io.smallrye.graphql.api.Directive;

/** <b><code>directive @extends on OBJECT | INTERFACE</code></b> */
/**
* <b><code>directive @extends on OBJECT | INTERFACE</code></b>
*
* @see <a href="https://www.apollographql.com/docs/federation/federated-types/federated-directives/#extends">federation
* spec</a>
*/
@Directive(on = { OBJECT, INTERFACE })
@Description("Some libraries such as graphql-java don't have native support for type extensions in their printer. " +
"Apollo Federation supports using an @extends directive in place of extend type to annotate type references.")
@Description("Indicates that an object or interface definition is an extension of another definition of that same type.\n" +
"If your subgraph library supports GraphQL's built-in extend keyword, do not use this directive! Instead, use extend.")
@Retention(RUNTIME)
@Experimental("SmallRye GraphQL Federation is still subject to change. " +
"Additionally, this annotation is currently only a directive without explicit support from the extension.")
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.smallrye.graphql.api.federation;

import static io.smallrye.graphql.api.DirectiveLocation.FIELD_DEFINITION;
import static io.smallrye.graphql.api.DirectiveLocation.OBJECT;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

import java.lang.annotation.Retention;
Expand All @@ -10,10 +11,17 @@
import io.smallrye.common.annotation.Experimental;
import io.smallrye.graphql.api.Directive;

/** <b><code>directive @external on FIELD_DEFINITION</code></b> */
@Directive(on = FIELD_DEFINITION)
@Description("The @external directive is used to mark a field as owned by another service. " +
"This allows service A to use fields from service B while also knowing at runtime the types of that field.")
/**
* <b><code>directive @external on FIELD_DEFINITION | OBJECT</code></b>
*
* @see <a href="https://www.apollographql.com/docs/federation/federated-types/federated-directives/#external">federation
* spec</a>
*/
@Directive(on = { FIELD_DEFINITION, OBJECT })
@Description("Indicates that this subgraph usually can't resolve a particular object field, but it still needs to define " +
"that field for other purposes.\n" +
"This directive is always used in combination with another directive that references object fields, " +
"such as @provides or @requires.")
@Retention(RUNTIME)
@Experimental("SmallRye GraphQL Federation is still subject to change.")
public @interface External {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,40 @@
import static io.smallrye.graphql.api.DirectiveLocation.OBJECT;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

import java.lang.annotation.Repeatable;
import java.lang.annotation.Retention;

import org.eclipse.microprofile.graphql.Description;
import org.eclipse.microprofile.graphql.NonNull;

import io.smallrye.common.annotation.Experimental;
import io.smallrye.graphql.api.Directive;
import io.smallrye.graphql.api.federation.Key.Keys;

/** <b><code>directive @key(fields: _FieldSet!) on OBJECT | INTERFACE</code></b> */
/**
* <b><code>directive @key(fields: FieldSet!) repeatable on OBJECT | INTERFACE</code></b>
*
* @see <a href="https://www.apollographql.com/docs/federation/federated-types/federated-directives/#key">federation spec</a>
*/
@Directive(on = { OBJECT, INTERFACE })
@Description("The @key directive is used to indicate a combination of fields that can be used to uniquely identify " +
"and fetch an object or interface.")
@Description("Designates an object type as an entity and specifies its key fields (a set of fields that the subgraph " +
"can use to uniquely identify any instance of the entity). You can apply multiple @key directives to " +
"a single entity (to specify multiple valid sets of key fields).")
@Retention(RUNTIME)
@Repeatable(Keys.class)
@Experimental("SmallRye GraphQL Federation is still subject to change.")
public @interface Key {
@NonNull
String[] fields();
@Description("A GraphQL selection set (provided as a string) of fields and subfields that contribute " +
"to the entity's primary key.\n" +
"Examples:\n" +
"\"id\"\n" +
"\"username region\"\n" +
"\"name organization { id }\"")
String fields();

@Retention(RUNTIME)
@interface Keys {
Key[] value();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,27 @@
import io.smallrye.common.annotation.Experimental;
import io.smallrye.graphql.api.Directive;

/** <b><code>directive @provides(fields: _FieldSet!) on FIELD_DEFINITION</code></b> */
/**
* <b><code>directive @provides(fields: FieldSet!) on FIELD_DEFINITION</code></b>
*
* @see <a href="https://www.apollographql.com/docs/federation/federated-types/federated-directives/#provides">federation
* spec</a>
*/
@Directive(on = FIELD_DEFINITION)
@Description("When resolving the annotated field, this service can provide additional, normally `@external` fields.")
@Description("Specifies a set of entity fields that a subgraph can resolve, but only at a particular schema path " +
"(at other paths, the subgraph can't resolve those fields).\n" +
"If a subgraph can always resolve a particular entity field, do not apply this directive.\n" +
"Using this directive is always an optional optimization. It can reduce the total number of subgraphs " +
"that your graph router needs to communicate with to resolve certain operations, which can improve performance.")
@Retention(RUNTIME)
@Experimental("SmallRye GraphQL Federation is still subject to change. " +
"Additionally, this annotation is currently only a directive without explicit support from the extension.")
@Experimental("SmallRye GraphQL Federation is still subject to change.")
public @interface Provides {
@NonNull
String[] fields();
@Description("A GraphQL selection set (provided as a string) of object fields and subfields that the subgraph " +
"can resolve only at this query path.\n" +
"Examples:\n" +
"\"name\"\n" +
"\"name address\"\n" +
"\"... on Person { name address }\" (valid for fields that return a union or interface)")
String fields();
}
Loading