Skip to content

Commit

Permalink
Merge pull request #31843 from Ladicek/arc-kotlin-suspend-functions-i…
Browse files Browse the repository at this point in the history
…nterception

ArC: Kotlin improvements
  • Loading branch information
Ladicek authored Mar 16, 2023
2 parents c451560 + 2f7c158 commit 4184c2e
Show file tree
Hide file tree
Showing 14 changed files with 365 additions and 81 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.quarkus.arc.deployment;

import static io.quarkus.arc.processor.KotlinUtils.isKotlinClass;
import static io.quarkus.deployment.annotations.ExecutionTime.RUNTIME_INIT;
import static io.quarkus.deployment.annotations.ExecutionTime.STATIC_INIT;

Expand Down Expand Up @@ -490,7 +491,7 @@ public ObserverRegistrationPhaseBuildItem registerSyntheticObservers(BeanRegistr
} else {
declaringClass = injectionPoint.getTarget().asMethod().declaringClass();
}
if (declaringClass.declaredAnnotation(DotNames.KOTLIN_METADATA_ANNOTATION) != null) {
if (isKotlinClass(declaringClass)) {
validationErrors.produce(new ValidationErrorBuildItem(
new DefinitionException(
"kotlin.collections.List cannot be used together with the @All qualifier, please use MutableList or java.util.List instead: "
Expand Down
2 changes: 2 additions & 0 deletions independent-projects/arc/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
<version.jta>2.0.1</version.jta>
<version.jandex>3.0.5</version.jandex>
<version.junit5>5.9.2</version.junit5>
<version.kotlin>1.8.10</version.kotlin>
<version.kotlin-coroutines>1.6.4</version.kotlin-coroutines>
<version.maven>3.8.7</version.maven>
<version.assertj>3.24.2</version.assertj>
<version.jboss-logging>3.5.0.Final</version.jboss-logging>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,9 +605,15 @@ private Map<MethodInfo, InterceptionInfo> initInterceptedMethods(List<Throwable>
Set<MethodInfo> finalMethods = Methods.addInterceptedMethodCandidates(beanDeployment, target.get().asClass(),
candidates, classLevelBindings, bytecodeTransformerConsumer, transformUnproxyableClasses);
if (!finalMethods.isEmpty()) {
String additionalError = "";
if (finalMethods.stream().anyMatch(KotlinUtils::isNoninterceptableKotlinMethod)) {
additionalError = "\n\tKotlin `suspend` functions must be `open` and declared in `open` classes, "
+ "otherwise they cannot be intercepted";
}
errors.add(new DeploymentException(String.format(
"Intercepted methods of the bean %s may not be declared final:\n\t- %s", getBeanClass(),
finalMethods.stream().map(Object::toString).sorted().collect(Collectors.joining("\n\t- ")))));
"Intercepted methods of the bean %s may not be declared final:\n\t- %s%s", getBeanClass(),
finalMethods.stream().map(Object::toString).sorted().collect(Collectors.joining("\n\t- ")),
additionalError)));
return Collections.emptyMap();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ public final class DotNames {
public static final DotName INSTANCE_HANDLE = create(InstanceHandle.class);
public static final DotName NO_CLASS_INTERCEPTORS = create(NoClassInterceptors.class);
public static final DotName DEPRECATED = create(Deprecated.class);

/**
* @deprecated use {@link KotlinUtils}; this constant will be removed at some time after Quarkus 3.6
*/
@Deprecated(forRemoval = true, since = "3.0")
public static final DotName KOTLIN_METADATA_ANNOTATION = create("kotlin.Metadata");

public static final DotName BOOLEAN = create(Boolean.class);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package io.quarkus.arc.processor;

import org.jboss.jandex.DotName;

class KotlinDotNames {
static final DotName METADATA = DotName.createSimple("kotlin.Metadata");

static final DotName CONTINUATION = DotName.createSimple("kotlin.coroutines.Continuation");
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package io.quarkus.arc.processor;

import java.lang.reflect.Modifier;

import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.MethodInfo;
import org.jboss.jandex.Type;

public class KotlinUtils {
public static boolean isKotlinClass(ClassInfo clazz) {
return clazz.hasDeclaredAnnotation(KotlinDotNames.METADATA);
}

public static boolean isKotlinMethod(MethodInfo method) {
return isKotlinClass(method.declaringClass());
}

public static boolean isKotlinSuspendMethod(MethodInfo method) {
if (!isKotlinMethod(method)) {
return false;
}

Type lastParameter = method.parameterType(method.parametersCount() - 1);
return KotlinDotNames.CONTINUATION.equals(lastParameter.name());
}

public static boolean isNoninterceptableKotlinMethod(MethodInfo method) {
// the Kotlin compiler generates somewhat streamlined bytecode when it determines
// that a `suspend` method cannot be overridden, and that bytecode doesn't work
// well with subclassing-based interception
//
// a `suspend` method can be overridden when it is `open` and is declared in an `open` class
return isKotlinSuspendMethod(method)
&& (Modifier.isFinal(method.flags()) || Modifier.isFinal(method.declaringClass().flags()));
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.quarkus.arc.processor;

import static io.quarkus.arc.processor.IndexClassLookupUtils.getClassByName;
import static io.quarkus.arc.processor.KotlinUtils.isNoninterceptableKotlinMethod;

import java.lang.reflect.Modifier;
import java.util.ArrayList;
Expand Down Expand Up @@ -178,7 +179,7 @@ private static Set<MethodInfo> addInterceptedMethodCandidates(BeanDeployment bea
}
boolean addToCandidates = true;
if (Modifier.isFinal(method.flags())) {
if (transformUnproxyableClasses) {
if (transformUnproxyableClasses && !isNoninterceptableKotlinMethod(method)) {
methodsFromWhichToRemoveFinal.add(NameAndDescriptor.fromMethodInfo(method));
} else {
addToCandidates = false;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.quarkus.arc.processor;

import static io.quarkus.arc.processor.IndexClassLookupUtils.getClassByName;
import static io.quarkus.arc.processor.KotlinUtils.isKotlinMethod;
import static org.objectweb.asm.Opcodes.ACC_FINAL;
import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
import static org.objectweb.asm.Opcodes.ACC_VOLATILE;
Expand Down Expand Up @@ -824,7 +825,6 @@ private void createInterceptedMethod(ClassOutput classOutput, BeanInfo bean, Met
// catch exceptions declared on the original method
boolean addCatchRuntimeException = true;
boolean addCatchException = true;
boolean isKotlin = method.declaringClass().declaredAnnotation(DotNames.KOTLIN_METADATA_ANNOTATION) != null;
Set<DotName> declaredExceptions = new LinkedHashSet<>(method.exceptions().size());
for (Type declaredException : method.exceptions()) {
declaredExceptions.add(declaredException.name());
Expand All @@ -849,7 +849,7 @@ private void createInterceptedMethod(ClassOutput classOutput, BeanInfo bean, Met
}
// now catch the rest (Exception e) if not already caught
// this catch is _not_ included for Kotlin methods because Kotlin has not checked exceptions contract
if (addCatchException && !isKotlin) {
if (addCatchException && !isKotlinMethod(method)) {
CatchBlockCreator catchOtherExceptions = tryCatch.addCatch(Exception.class);
// and wrap them in a new RuntimeException(e)
catchOtherExceptions.throwException(ArcUndeclaredThrowableException.class, "Error invoking subclass method",
Expand Down
93 changes: 92 additions & 1 deletion independent-projects/arc/tests/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,21 @@
<dependency>
<groupId>org.jetbrains.kotlin</groupId>
<artifactId>kotlin-stdlib</artifactId>
<version>1.8.10</version>
<version>${version.kotlin}</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.jetbrains.kotlin</groupId>
<artifactId>kotlin-test-junit5</artifactId>
<version>${version.kotlin}</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.jetbrains.kotlinx</groupId>
<artifactId>kotlinx-coroutines-core</artifactId>
<version>${version.kotlin-coroutines}</version>
<scope>test</scope>
</dependency>

Expand All @@ -67,4 +81,81 @@
</plugins>
</build>

<profiles>
<profile>
<id>kotlin-tests</id>
<activation>
<property>
<name>!no-kotlin-tests</name>
</property>
</activation>

<build>
<plugins>
<plugin>
<groupId>org.jetbrains.kotlin</groupId>
<artifactId>kotlin-maven-plugin</artifactId>
<version>${version.kotlin}</version>
<executions>
<execution>
<id>compile</id>
<goals>
<goal>compile</goal>
</goals>
<configuration>
<sourceDirs>
<sourceDir>${project.basedir}/src/main/kotlin</sourceDir>
<sourceDir>${project.basedir}/src/main/java</sourceDir>
</sourceDirs>
</configuration>
</execution>
<execution>
<id>test-compile</id>
<goals>
<goal>test-compile</goal>
</goals>
<configuration>
<sourceDirs>
<sourceDir>${project.basedir}/src/test/kotlin</sourceDir>
<sourceDir>${project.basedir}/src/test/java</sourceDir>
</sourceDirs>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<executions>
<!-- Replacing default-compile as it is treated specially by Maven -->
<execution>
<id>default-compile</id>
<phase>none</phase>
</execution>
<!-- Replacing default-testCompile as it is treated specially by Maven -->
<execution>
<id>default-testCompile</id>
<phase>none</phase>
</execution>
<execution>
<id>java-compile</id>
<phase>compile</phase>
<goals>
<goal>compile</goal>
</goals>
</execution>
<execution>
<id>java-test-compile</id>
<phase>test-compile</phase>
<goals>
<goal>testCompile</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
</profiles>

</project>

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
package io.quarkus.arc.test.interceptors

import io.quarkus.arc.Arc
import io.quarkus.arc.test.ArcTestContainer
import jakarta.annotation.Priority
import jakarta.inject.Singleton
import jakarta.interceptor.AroundInvoke
import jakarta.interceptor.Interceptor
import jakarta.interceptor.InterceptorBinding
import jakarta.interceptor.InvocationContext
import kotlinx.coroutines.delay
import kotlinx.coroutines.runBlocking
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.extension.RegisterExtension
import kotlin.test.assertEquals

class InterceptableSuspendMethodTest {
@RegisterExtension
val container = ArcTestContainer(MyInterceptorBinding::class.java, MyInterceptor::class.java,
MyService::class.java)

@Test
fun test() {
val service = Arc.container().instance(MyService::class.java).get()
val result = runBlocking {
service.hello()
}

assertEquals("hello", result)
assertEquals(1, MyInterceptor.intercepted)
}

@Target(AnnotationTarget.CLASS, AnnotationTarget.FUNCTION)
@Retention(AnnotationRetention.RUNTIME)
@InterceptorBinding
annotation class MyInterceptorBinding

@MyInterceptorBinding
@Priority(1)
@Interceptor
class MyInterceptor {
companion object {
var intercepted = 0
}

@AroundInvoke
fun intercept(ctx: InvocationContext): Any {
intercepted++
return ctx.proceed()
}
}

@Singleton
open class MyService {
@MyInterceptorBinding
open suspend fun hello(): String {
delay(10)
return "hello"
}
}
}
Loading

0 comments on commit 4184c2e

Please sign in to comment.