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

Updating Federation directives #2010

Merged
merged 2 commits into from
Jan 30, 2024
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
13 changes: 7 additions & 6 deletions common/schema-builder/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,13 @@
<description>Creates the model from a Jandex index</description>

<dependencies>


<!-- The API -->
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>smallrye-graphql-api</artifactId>
</dependency>

<!-- The model -->
<dependency>
<groupId>${project.groupId}</groupId>
Expand Down Expand Up @@ -50,11 +56,6 @@
<artifactId>yasson</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>smallrye-graphql-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.jetbrains</groupId>
<artifactId>annotations</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,8 @@ private void addDirectiveTypes(Schema schema) {
boolean federationEnabled = Boolean.getBoolean("smallrye.graphql.federation.enabled");
// only add federation-related directive types to the schema if federation is enabled
DotName packageName = classInfo.name().packagePrefixName();
if (packageName == null || !packageName.equals(FEDERATION_ANNOTATIONS_PACKAGE) || federationEnabled) {
if (packageName == null || !packageName.toString().startsWith(FEDERATION_ANNOTATIONS_PACKAGE.toString())
|| federationEnabled) {
schema.addDirectiveType(directiveTypeCreator.create(classInfo));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
package io.smallrye.graphql.schema.creator;

import static io.smallrye.graphql.schema.Annotations.DIRECTIVE;
import static io.smallrye.graphql.schema.Annotations.NON_NULL;
import static java.util.stream.Collectors.toSet;

import java.util.Collections;
import java.util.Set;
import java.util.stream.Stream;

import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.ArrayType;
import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.ClassType;
import org.jboss.jandex.DotName;
import org.jboss.jandex.MethodInfo;
import org.jboss.jandex.Type;
import org.jboss.logging.Logger;

import io.smallrye.graphql.api.federation.policy.Policy;
import io.smallrye.graphql.api.federation.requiresscopes.RequiresScopes;
import io.smallrye.graphql.schema.Annotations;
import io.smallrye.graphql.schema.helper.DescriptionHelper;
import io.smallrye.graphql.schema.helper.Direction;
Expand All @@ -20,6 +27,10 @@
import io.smallrye.graphql.schema.model.DirectiveType;

public class DirectiveTypeCreator extends ModelCreator {
private static final DotName POLICY = DotName.createSimple(Policy.class.getName());
private static final DotName REQUIRES_SCOPES = DotName.createSimple(RequiresScopes.class.getName());
private static final DotName STRING = DotName.createSimple(String.class.getName());

private static final Logger LOG = Logger.getLogger(DirectiveTypeCreator.class.getName());

public DirectiveTypeCreator(ReferenceCreator referenceCreator) {
Expand All @@ -32,8 +43,6 @@ public String getDirectiveLocation() {
"This method should never be called since 'DirectiveType' cannot have another directives");
}

private static DotName NON_NULL = DotName.createSimple("org.eclipse.microprofile.graphql.NonNull");

public DirectiveType create(ClassInfo classInfo) {
LOG.debug("Creating directive from " + classInfo.name().toString());

Expand All @@ -48,10 +57,22 @@ public DirectiveType create(ClassInfo classInfo) {

for (MethodInfo method : classInfo.methods()) {
DirectiveArgument argument = new DirectiveArgument();
argument.setReference(referenceCreator.createReferenceForOperationArgument(method.returnType(), null));
Type argumentType;
if (classInfo.name().equals(POLICY) || classInfo.name().equals(REQUIRES_SCOPES)) {
// For both of these directives, we need to override the argument type to be an array of nested arrays
// of strings, where none of the nested elements can be null
AnnotationInstance nonNullAnnotation = AnnotationInstance.create(NON_NULL, null,
Collections.emptyList());
Type stringType = ClassType.createWithAnnotations(STRING, Type.Kind.CLASS,
new AnnotationInstance[] { nonNullAnnotation });
argumentType = buildArrayType(stringType, 2, nonNullAnnotation);
} else {
argumentType = method.returnType();
}
argument.setReference(referenceCreator.createReferenceForOperationArgument(argumentType, null));
argument.setName(method.name());
Annotations annotationsForMethod = Annotations.getAnnotationsForInterfaceField(method);
populateField(Direction.IN, argument, method.returnType(), annotationsForMethod);
populateField(Direction.IN, argument, argumentType, annotationsForMethod);
if (annotationsForMethod.containsOneOfTheseAnnotations(NON_NULL)) {
argument.setNotNull(true);
}
Expand All @@ -72,4 +93,14 @@ private Set<String> getLocations(AnnotationInstance directiveAnnotation) {
return Stream.of(directiveAnnotation.value("on").asEnumArray())
.collect(toSet());
}

private static Type buildArrayType(Type baseType, int dimensions, AnnotationInstance annotation) {
Type currentType = baseType;
for (int i = 0; i < dimensions; i++) {
ArrayType.Builder builder = ArrayType.builder(currentType, 1);
builder.addAnnotation(annotation);
currentType = builder.build();
}
return currentType;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ private void checkWrappedTypeKotlinNullability(MethodInfo methodInfo,
KmType returnType = function.get().getReturnType();
var nullable = isKotlinWrappedTypeNullable(returnType);
if (operation.getWrapper().isCollectionOrArrayOrMap()) {
operation.getWrapper().setNotEmpty(!nullable);
operation.getWrapper().setWrappedTypeNotNull(!nullable);
// workaround: consistent behavior to java: if wrapped type is non null, collection will be marked as non null
if (!nullable && !operation.isNotNull()) {
operation.setNotNull(true);
Expand All @@ -159,7 +159,7 @@ private void checkWrappedTypeKotlinNullability(MethodInfo methodInfo,
if (argument.hasWrapper() && argument.getWrapper().isCollectionOrArrayOrMap()) {
KmValueParameter typeParameter = valueParameters.get(i);
var paramNullable = isKotlinWrappedTypeNullable(typeParameter.getType());
argument.getWrapper().setNotEmpty(!paramNullable);
argument.getWrapper().setWrappedTypeNotNull(!paramNullable);
// workaround: consistent behavior to java: if wrapped type is not null, collection will be marked as not null
if (!paramNullable && !argument.isNotNull()) {
argument.setNotNull(true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public static Optional<Wrapper> createWrapper(Type fieldType, Type methodType) {
Wrapper wrapper = new Wrapper(getWrapperType(methodType), methodType.name().toString());
// NotNull
if (markParameterizedTypeNonNull(fieldType, methodType)) {
wrapper.setNotEmpty(true);
wrapper.setWrappedTypeNotNull(true);
}
// Wrapper of wrapper
Optional<Wrapper> wrapperOfWrapper = getWrapperOfWrapper(methodType);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static org.jboss.jandex.AnnotationValue.Kind.ARRAY;

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -13,6 +14,8 @@
import org.jboss.jandex.DotName;
import org.jboss.logging.Logger;

import io.smallrye.graphql.api.federation.policy.Policy;
import io.smallrye.graphql.api.federation.requiresscopes.RequiresScopes;
import io.smallrye.graphql.schema.Annotations;
import io.smallrye.graphql.schema.model.DirectiveInstance;
import io.smallrye.graphql.schema.model.DirectiveType;
Expand Down Expand Up @@ -64,10 +67,20 @@ public List<DirectiveInstance> buildDirectiveInstances(Annotations annotations,

private DirectiveInstance toDirectiveInstance(AnnotationInstance annotationInstance) {
DirectiveInstance directiveInstance = new DirectiveInstance();
directiveInstance.setType(directiveTypes.get(annotationInstance.name()));
DirectiveType directiveType = directiveTypes.get(annotationInstance.name());
directiveInstance.setType(directiveType);

for (AnnotationValue annotationValue : annotationInstance.values()) {
directiveInstance.setValue(annotationValue.name(), valueObject(annotationValue));
if (directiveType.getClassName().equals(Policy.class.getName()) ||
directiveType.getClassName().equals(RequiresScopes.class.getName())) {
// For both of these directives, we need to process the annotation values as nested arrays of strings
List<List<String>> valueList = processAnnotationValues((AnnotationValue[]) annotationValue.value());
directiveInstance.setValue(annotationValue.name(), valueList);
} else {
directiveInstance.setValue(annotationValue.name(), valueObject(annotationValue));
}
}

return directiveInstance;
}

Expand All @@ -86,4 +99,20 @@ private Object valueObject(AnnotationValue annotationValue) {
public Map<DotName, DirectiveType> getDirectiveTypes() {
return directiveTypes;
}

private List<List<String>> processAnnotationValues(AnnotationValue[] annotationValues) {
if (annotationValues == null || annotationValues.length == 0) {
return Collections.emptyList();
}

List<List<String>> valuesList = new ArrayList<>();
for (AnnotationValue nestedValue : annotationValues) {
List<String> values = new ArrayList<>();
for (AnnotationValue value : (AnnotationValue[]) nestedValue.asNested().values().get(0).value()) {
values.add(value.asString());
}
valuesList.add(values);
}
return valuesList;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -333,15 +333,15 @@ public void testKotlinTypeWrappedInCollectionNullability() {

var yyy1 = getQueryByName(schema, "yyy1");
assertTrue(yyy1.isNotNull());
assertTrue(yyy1.getWrapper().isNotEmpty());
assertTrue(yyy1.getWrapper().isWrappedTypeNotNull());

var yyy2 = getQueryByName(schema, "yyy2");
assertTrue(yyy2.isNotNull());
assertFalse(yyy2.getWrapper().isNotEmpty());
assertFalse(yyy2.getWrapper().isWrappedTypeNotNull());

var yyy3 = getQueryByName(schema, "yyy3");
assertTrue(yyy3.isNotNull());
assertTrue(yyy3.getWrapper().isNotEmpty());
assertTrue(yyy3.getWrapper().isWrappedTypeNotNull());

var yyy4 = getQueryByName(schema, "yyy4");
assertFalse(yyy4.isNotNull());
Expand All @@ -351,22 +351,22 @@ public void testKotlinTypeWrappedInCollectionNullability() {
assertFalse(arguments.get(1).isNotNull()); // i1

assertTrue(arguments.get(2).isNotNull()); // i2
assertTrue(arguments.get(2).getWrapper().isNotEmpty()); // i2
assertTrue(arguments.get(2).getWrapper().isWrappedTypeNotNull()); // i2

assertTrue(arguments.get(3).isNotNull()); // i3
assertFalse(arguments.get(3).getWrapper().isNotEmpty()); // i3
assertFalse(arguments.get(3).getWrapper().isWrappedTypeNotNull()); // i3

assertFalse(arguments.get(4).isNotNull()); // i4
assertFalse(arguments.get(4).getWrapper().isNotEmpty()); // i4
assertFalse(arguments.get(4).getWrapper().isWrappedTypeNotNull()); // i4

// it is not consistent with java: if in java the wrapped list element is not null, the list will be marked as non-null in any case.
assertTrue(arguments.get(5).isNotNull()); // i5
assertTrue(arguments.get(5).getWrapper().isNotEmpty()); // i5
assertTrue(arguments.get(5).getWrapper().isWrappedTypeNotNull()); // i5

var yyy5 = getQueryByName(schema, "yyy5");
// it is not consistent with java: if in java the wrapped list element is not null, the list will be marked as non-null in any case.
assertTrue(yyy5.isNotNull());
assertTrue(yyy5.getWrapper().isNotEmpty());
assertTrue(yyy5.getWrapper().isWrappedTypeNotNull());
}

private Operation getQueryByName(Schema schema, String name) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
public class Wrapper implements Serializable {

private String wrapperClassName;
private boolean notEmpty = false; // Mark this to be not empty
private boolean wrappedTypeNotNull = false;
private WrapperType wrapperType = WrapperType.UNKNOWN;

private Wrapper wrapper = null;
Expand All @@ -24,10 +24,10 @@ public Wrapper(WrapperType wrapperType, String wrapperClassName) {
this.wrapperClassName = wrapperClassName;
}

public Wrapper(WrapperType wrapperType, String wrapperClassName, boolean notEmpty) {
public Wrapper(WrapperType wrapperType, String wrapperClassName, boolean wrappedTypeNotNull) {
this.wrapperType = wrapperType;
this.wrapperClassName = wrapperClassName;
this.notEmpty = notEmpty;
this.wrappedTypeNotNull = wrappedTypeNotNull;
}

public WrapperType getWrapperType() {
Expand All @@ -46,12 +46,12 @@ public void setWrapperClassName(String wrapperClassName) {
this.wrapperClassName = wrapperClassName;
}

public void setNotEmpty(boolean notEmpty) {
this.notEmpty = notEmpty;
public void setWrappedTypeNotNull(boolean wrappedTypeNotNull) {
this.wrappedTypeNotNull = wrappedTypeNotNull;
}

public boolean isNotEmpty() {
return this.notEmpty;
public boolean isWrappedTypeNotNull() {
return this.wrappedTypeNotNull;
}

public Wrapper getWrapper() {
Expand Down Expand Up @@ -92,15 +92,15 @@ public boolean isUnknown() {

@Override
public String toString() {
return "Wrapper{" + "wrapperClassName=" + wrapperClassName + ", notEmpty=" + notEmpty + ", wrapperType=" + wrapperType
+ ", wrapper=" + wrapper + '}';
return "Wrapper{" + "wrapperClassName=" + wrapperClassName + ", wrappedTypeNotNull=" + wrappedTypeNotNull +
", wrapperType=" + wrapperType + ", wrapper=" + wrapper + '}';
}

@Override
public int hashCode() {
int hash = 7;
hash = 59 * hash + Objects.hashCode(this.wrapperClassName);
hash = 59 * hash + (this.notEmpty ? 1 : 0);
hash = 59 * hash + (this.wrappedTypeNotNull ? 1 : 0);
hash = 59 * hash + Objects.hashCode(this.wrapperType);
hash = 59 * hash + Objects.hashCode(this.wrapper);
return hash;
Expand All @@ -118,7 +118,7 @@ public boolean equals(Object obj) {
return false;
}
final Wrapper other = (Wrapper) obj;
if (this.notEmpty != other.notEmpty) {
if (this.wrappedTypeNotNull != other.wrappedTypeNotNull) {
return false;
}
if (!Objects.equals(this.wrapperClassName, other.wrapperClassName)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package io.smallrye.graphql.api.federation;

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

import java.lang.annotation.Retention;

import org.eclipse.microprofile.graphql.Description;

import io.smallrye.common.annotation.Experimental;
import io.smallrye.graphql.api.Directive;

/**
* <b><code>directive @authenticated on ENUM | FIELD_DEFINITION | INTERFACE | OBJECT | SCALAR</code></b>
*
* @see <a href="https://www.apollographql.com/docs/federation/federated-types/federated-directives/#authenticated">federation
* spec</a>
*/
@Directive(on = { ENUM, FIELD_DEFINITION, INTERFACE, OBJECT, SCALAR })
@Description("Indicates to composition that the target element is accessible only to the authenticated supergraph " +
"users.")
@Retention(RUNTIME)
@Experimental("SmallRye GraphQL Federation is still subject to change.")
public @interface Authenticated {
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
import io.smallrye.graphql.api.Directive;

/**
* <b><code>directive @inaccessible on FIELD_DEFINITION | INTERFACE | OBJECT | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION</code></b>
* <b><code>directive @inaccessible on FIELD_DEFINITION | INTERFACE | OBJECT | UNION | ARGUMENT_DEFINITION | SCALAR |
* ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION</code></b>
*
* @see <a href="https://www.apollographql.com/docs/federation/federated-types/federated-directives/#inaccessible">federation
* spec</a>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.lang.annotation.Repeatable;
import java.lang.annotation.Retention;

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

Expand All @@ -15,7 +16,7 @@
import io.smallrye.graphql.api.federation.Key.Keys;

/**
* <b><code>directive @key(fields: FieldSet!) repeatable on OBJECT | INTERFACE</code></b>
* <b><code>directive @key(fields: FieldSet!, resolvable: Boolean = true) repeatable on OBJECT | INTERFACE</code></b>
*
* @see <a href="https://www.apollographql.com/docs/federation/federated-types/federated-directives/#key">federation spec</a>
*/
Expand All @@ -36,6 +37,12 @@
"\"name organization { id }\"")
String fields();

@DefaultValue("true")
@Description("If false, indicates to the router that this subgraph doesn't define a reference resolver for this " +
"entity. This means that router query plans can't \"jump to\" this subgraph to resolve fields that aren't" +
" defined in another subgraph.")
boolean resolvable() default true;

@Retention(RUNTIME)
@interface Keys {
Key[] value();
Expand Down
Loading
Loading