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

Detect Kotlin nullability for type arguments #1873

Merged
merged 1 commit into from
Aug 2, 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
40 changes: 39 additions & 1 deletion common/schema-builder/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,14 @@
<groupId>io.smallrye</groupId>
<artifactId>jandex</artifactId>
</dependency>


<!-- Kotlin support -->
<dependency>
<groupId>org.jetbrains.kotlinx</groupId>
<artifactId>kotlinx-metadata-jvm</artifactId>
<version>${version.kotlinx.metadata.jvm}</version>
</dependency>

<!-- Logging -->
<dependency>
<groupId>org.jboss.logging</groupId>
Expand Down Expand Up @@ -61,6 +68,16 @@
<artifactId>jakarta.validation-api</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.smallrye.reactive</groupId>
<artifactId>mutiny</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>jakarta.json</groupId>
<artifactId>jakarta.json-api</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand All @@ -73,6 +90,27 @@
<trimStackTrace>false</trimStackTrace>
</configuration>
</plugin>

<plugin>
<groupId>org.jetbrains.kotlin</groupId>
<artifactId>kotlin-maven-plugin</artifactId>
<version>${version.kotlin.compiler}</version>
<executions>
<execution>
<id>test-compile</id>
<goals>
<goal>test-compile</goal>
</goals>
<configuration>
<sourceDirs>
<dir>${project.basedir}/src/test/kotlin</dir>
</sourceDirs>
</configuration>
</execution>
</executions>
</plugin>


</plugins>
</build>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,7 @@ private static Map<DotName, AnnotationInstance> getAnnotationsWithFilter(org.jbo
public static final DotName DIRECTIVE = DotName.createSimple("io.smallrye.graphql.api.Directive");
public static final DotName DEFAULT_NON_NULL = DotName.createSimple("io.smallrye.graphql.api.DefaultNonNull");
public static final DotName NULLABLE = DotName.createSimple("io.smallrye.graphql.api.Nullable");
public static final DotName KOTLIN_METADATA = DotName.createSimple("kotlin.Metadata");

// MicroProfile GraphQL Annotations
public static final DotName GRAPHQL_API = DotName.createSimple("org.eclipse.microprofile.graphql.GraphQLApi");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.Objects;
import java.util.Optional;

import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.DotName;
import org.jboss.jandex.MethodInfo;
import org.jboss.jandex.Type;
Expand All @@ -21,6 +22,14 @@
import io.smallrye.graphql.schema.model.Operation;
import io.smallrye.graphql.schema.model.OperationType;
import io.smallrye.graphql.schema.model.Reference;
import kotlinx.metadata.Flag;
import kotlinx.metadata.KmClassifier;
import kotlinx.metadata.KmFunction;
import kotlinx.metadata.KmType;
import kotlinx.metadata.KmTypeProjection;
import kotlinx.metadata.KmValueParameter;
import kotlinx.metadata.jvm.KotlinClassHeader;
import kotlinx.metadata.jvm.KotlinClassMetadata;

/**
* Creates a Operation object
Expand Down Expand Up @@ -94,9 +103,73 @@ public Operation createOperation(MethodInfo methodInfo, OperationType operationT
addDirectivesForRolesAllowed(annotationsForMethod, annotationsForClass, operation, reference);
populateField(Direction.OUT, operation, fieldType, annotationsForMethod);

if (operation.hasWrapper()) {
checkWrappedTypeKotlinNullability(methodInfo, annotationsForClass, operation);
}
return operation;
}

// If the operation return type is a wrapper and is written in Kotlin,
// this checks whether the wrapped type is nullable.
// Nullability metadata is stored in the kotlin.Metadata annotation
// on the class that contains the operation.
private void checkWrappedTypeKotlinNullability(MethodInfo methodInfo,
Annotations annotationsForClass,
Operation operation) {
Optional<AnnotationInstance> kotlinMetadataAnnotation = annotationsForClass
.getOneOfTheseAnnotations(Annotations.KOTLIN_METADATA);
if (kotlinMetadataAnnotation.isPresent()) {
KotlinClassMetadata.Class kotlinClass = toKotlinClassMetadata(kotlinMetadataAnnotation.get());
// We need to find the corresponding function inside
// the KotlinClassMetadata to check its IS_NULLABLE flag.
Optional<KmFunction> function = kotlinClass.getKmClass().getFunctions()
.stream()
.filter(f -> f.getName().equals(methodInfo.name()))
.filter(f -> compareParameterLists(f.getValueParameters(), methodInfo.parameterTypes()))
.findAny();
if (function.isPresent()) {
KmType returnType = function.get().getReturnType();
KmTypeProjection arg = returnType.getArguments().get(0);
int flags = arg.getType().getFlags();
boolean nullable = Flag.Type.IS_NULLABLE.invoke(flags);
if (nullable) {
operation.setNotNull(false);
}
}
}
}

private boolean compareParameterLists(List<KmValueParameter> kotlinParameters,
List<Type> jandexParameters) {
if (kotlinParameters.size() != jandexParameters.size()) {
return false;
}
for (int i = 0; i < kotlinParameters.size(); i++) {
// TODO: the matching of parameter types could use some improvements
// For example, it won't work for primitives.
// An Int parameter will be represented as kotlin.Int in the KotlinClassMetadata,
// but as "int" in the Jandex MethodInfo.
if (!((KmClassifier.Class) kotlinParameters.get(i).getType().classifier)
.getName().replace("/", ".")
.equals(jandexParameters.get(i).name().toString())) {
return false;
}
}
return true;
}

private KotlinClassMetadata.Class toKotlinClassMetadata(AnnotationInstance metadata) {
KotlinClassHeader classHeader = new KotlinClassHeader(
metadata.value("k").asInt(),
metadata.value("mv").asIntArray(),
metadata.value("d1").asStringArray(),
metadata.value("d2").asStringArray(),
metadata.value("xs") != null ? metadata.value("xs").asString() : null,
metadata.value("pn") != null ? metadata.value("pn").asString() : null,
metadata.value("xi").asInt());
return (KotlinClassMetadata.Class) KotlinClassMetadata.read(classHeader);
}

private static void validateFieldType(MethodInfo methodInfo, OperationType operationType) {
Type returnType = methodInfo.returnType();
if (!operationType.equals(OperationType.MUTATION) && returnType.kind().equals(Type.Kind.VOID)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.nio.file.Paths;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Set;
import java.util.concurrent.ExecutorService;
Expand Down Expand Up @@ -269,6 +270,36 @@ public void testGenericSchemaBuilding() {

}

@Test
public void testKotlinTypeNullability() {
Indexer indexer = new Indexer();
indexDirectory(indexer, "io/smallrye/graphql/kotlin");
IndexView index = indexer.complete();
Schema schema = SchemaBuilder.build(index);

assertTrue(getQueryByName(schema, "notNullable").isNotNull());
assertFalse(getQueryByName(schema, "nullable").isNotNull());
assertTrue(getQueryByName(schema, "notNullableItemInUni").isNotNull());
assertFalse(getQueryByName(schema, "nullableItemInUni").isNotNull());

Map<String, Operation> fooSubfields = schema.getTypes().get("Foo").getOperations();
assertTrue(fooSubfields.get("notNullableNestedItem").isNotNull());
assertTrue(fooSubfields.get("notNullableNestedItemInUni").isNotNull());
assertFalse(fooSubfields.get("nullableNestedItem").isNotNull());
assertFalse(fooSubfields.get("nullableNestedItemInUni").isNotNull());

assertFalse(getQueryByName(schema, "zzz1").isNotNull());
assertFalse(getQueryByName(schema, "zzz2").isNotNull());
assertTrue(getQueryByName(schema, "zzz3").isNotNull());
assertTrue(getQueryByName(schema, "zzz4").isNotNull());
}

private Operation getQueryByName(Schema schema, String name) {
return schema.getQueries()
.stream().filter(q -> q.getName().equals(name))
.findFirst().orElseThrow();
}

static IndexView getTCKIndex() {
Indexer indexer = new Indexer();
indexDirectory(indexer, "org/eclipse/microprofile/graphql/tck/apps/basic/api");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package io.smallrye.graphql.kotlin

import io.smallrye.mutiny.Uni
import org.eclipse.microprofile.graphql.GraphQLApi
import org.eclipse.microprofile.graphql.Query
import org.eclipse.microprofile.graphql.Source
import jakarta.json.bind.annotation.JsonbCreator

data class Foo @JsonbCreator constructor(val bar: String?)
data class Foo2 @JsonbCreator constructor(val bar: String?)
data class Foo3 @JsonbCreator constructor(val bar: String?)
data class Foo4 @JsonbCreator constructor(val bar: String?)

@GraphQLApi
class Example {
@Query
fun nullable(): Foo? = null

@Query
fun notNullable(): Foo = Foo("blabla")

@Query
fun nullableItemInUni(): Uni<Foo?> = Uni.createFrom().nullItem()

@Query
fun notNullableItemInUni(): Uni<Foo> = Uni.createFrom().item(Foo("blabla"))

fun nullableNestedItem(@Source foo: Foo): String? = foo.bar

fun notNullableNestedItem(@Source foo: Foo): String = "bar"

fun nullableNestedItemInUni(@Source foo: Foo): Uni<String?> = Uni.createFrom().nullItem()

fun notNullableNestedItemInUni(@Source foo: Foo): Uni<String> = Uni.createFrom().item("bar")

// some overloaded methods to make sure we correctly find the function inside KotlinClassMetadata
@Query("zzz1")
fun zzz(x: Foo): Uni<Foo?> = Uni.createFrom().nullItem()

@Query("zzz2")
fun zzz(x: Foo2): Uni<Foo?> = Uni.createFrom().nullItem()

@Query("zzz3")
fun zzz(x: Foo3): Uni<Foo> = Uni.createFrom().nullItem()

@Query("zzz4")
fun zzz(x: Foo4): Uni<Foo> = Uni.createFrom().nullItem()

}
2 changes: 2 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@
<maven.compiler.source>11</maven.compiler.source>
<maven.compiler.target>11</maven.compiler.target>
<version.impsort.plugin>1.8.0</version.impsort.plugin>
<version.kotlinx.metadata.jvm>0.7.0</version.kotlinx.metadata.jvm>
<version.kotlin.compiler>1.9.0</version.kotlin.compiler>

</properties>

Expand Down
Loading