Skip to content

Commit

Permalink
Add validation of execution model annotations
Browse files Browse the repository at this point in the history
The `@Blocking`, `@NonBlocking` and `@RunOnVirtualThread` annotations
may only be used on "entrypoint" methods (methods invoked by various
frameworks in Quarkus). Using these annotations on methods that can
only be invoked by application code is invalid.
  • Loading branch information
Ladicek committed Oct 9, 2023
1 parent 6be99a4 commit 7367ea5
Show file tree
Hide file tree
Showing 12 changed files with 424 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package io.quarkus.deployment.execannotations;

import java.util.Objects;
import java.util.function.Predicate;

import org.jboss.jandex.MethodInfo;

import io.quarkus.builder.item.MultiBuildItem;

/**
* Carries a predicate that identifies methods that can have annotations which affect
* the execution model ({@code @Blocking}, {@code @NonBlocking}, {@code @RunOnVirtualThread}).
* <p>
* Used to detect wrong usage of these annotations, as they are implemented directly
* by the various frameworks and may only be put on "entrypoint" methods. Placing these
* annotations on methods that can only be invoked by application code is always wrong.
*/
public final class ExecutionModelAnnotationsAllowedBuildItem extends MultiBuildItem {
private final Predicate<MethodInfo> predicate;

public ExecutionModelAnnotationsAllowedBuildItem(Predicate<MethodInfo> predicate) {
this.predicate = Objects.requireNonNull(predicate);
}

public boolean matches(MethodInfo method) {
return predicate.test(method);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package io.quarkus.deployment.execannotations;

import io.quarkus.runtime.annotations.ConfigPhase;
import io.quarkus.runtime.annotations.ConfigRoot;
import io.smallrye.config.ConfigMapping;
import io.smallrye.config.WithDefault;

@ConfigRoot(phase = ConfigPhase.BUILD_TIME)
@ConfigMapping(prefix = "quarkus.execution-model-annotations")
public interface ExecutionModelAnnotationsConfig {
/**
* Detection mode of invalid usage of execution model annotations.
* <p>
* An execution model annotation is {@code @Blocking}, {@code @NonBlocking} and {@code @RunOnVirtualThread}.
* These annotations may only be used on "entrypoint" methods (methods invoked by various frameworks in Quarkus);
* using them on methods that can only be invoked by application code is invalid.
*/
@WithDefault("fail")
Mode detectionMode();

enum Mode {
/**
* Invalid usage of execution model annotations causes build failure.
*/
FAIL,
/**
* Invalid usage of execution model annotations causes warning during build.
*/
WARN,
/**
* No detection of invalid usage of execution model annotations.
*/
DISABLED,
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package io.quarkus.deployment.execannotations;

import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.StringJoiner;
import java.util.function.Predicate;

import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.AnnotationTarget;
import org.jboss.jandex.DotName;
import org.jboss.jandex.IndexView;
import org.jboss.jandex.MethodInfo;
import org.jboss.jandex.Type;
import org.jboss.logging.Logger;

import io.quarkus.deployment.SuppressForbidden;
import io.quarkus.deployment.annotations.BuildStep;
import io.quarkus.deployment.annotations.Produce;
import io.quarkus.deployment.builditem.CombinedIndexBuildItem;
import io.quarkus.deployment.pkg.builditem.ArtifactResultBuildItem;
import io.smallrye.common.annotation.Blocking;
import io.smallrye.common.annotation.NonBlocking;
import io.smallrye.common.annotation.RunOnVirtualThread;

public class ExecutionModelAnnotationsProcessor {
private static final Logger log = Logger.getLogger(ExecutionModelAnnotationsProcessor.class);

private static final DotName BLOCKING = DotName.createSimple(Blocking.class);
private static final DotName NON_BLOCKING = DotName.createSimple(NonBlocking.class);
private static final DotName RUN_ON_VIRTUAL_THREAD = DotName.createSimple(RunOnVirtualThread.class);

@BuildStep
@Produce(ArtifactResultBuildItem.class) // only to make sure this build step is executed
void check(ExecutionModelAnnotationsConfig config, CombinedIndexBuildItem index,
List<ExecutionModelAnnotationsAllowedBuildItem> predicates) {

if (config.detectionMode() == ExecutionModelAnnotationsConfig.Mode.DISABLED) {
return;
}

StringBuilder message = new StringBuilder("\n");
doCheck(message, index.getIndex(), predicates, BLOCKING);
doCheck(message, index.getIndex(), predicates, NON_BLOCKING);
doCheck(message, index.getIndex(), predicates, RUN_ON_VIRTUAL_THREAD);

if (message.length() > 1) {
message.append("The @Blocking, @NonBlocking and @RunOnVirtualThread annotations may only be used "
+ "on \"entrypoint\" methods (methods invoked by various frameworks in Quarkus)\n");
message.append("Using the @Blocking, @NonBlocking and @RunOnVirtualThread annotations on methods "
+ "that can only be invoked by application code is invalid");
if (config.detectionMode() == ExecutionModelAnnotationsConfig.Mode.WARN) {
log.warn(message);
} else {
throw new IllegalStateException(message.toString());
}
}
}

private void doCheck(StringBuilder message, IndexView index,
List<ExecutionModelAnnotationsAllowedBuildItem> predicates, DotName annotationName) {

List<String> badMethods = new ArrayList<>();
for (AnnotationInstance annotation : index.getAnnotations(annotationName)) {
// these annotations may be put on classes too, but we'll ignore that for now
if (annotation.target() != null && annotation.target().kind() == AnnotationTarget.Kind.METHOD) {
MethodInfo method = annotation.target().asMethod();
for (ExecutionModelAnnotationsAllowedBuildItem predicate : predicates) {
if (!predicate.matches(method)) {
badMethods.add(methodToString(method));
break;
}
}
}
}

if (!badMethods.isEmpty()) {
message.append("Wrong usage(s) of @").append(annotationName.withoutPackagePrefix()).append(" found:\n");
for (String method : badMethods) {
message.append("\t- ").append(method).append("\n");
}
}
}

@BuildStep
ExecutionModelAnnotationsAllowedBuildItem devuiJsonRpcServices() {
return new ExecutionModelAnnotationsAllowedBuildItem(new Predicate<MethodInfo>() {
@Override
public boolean test(MethodInfo method) {
// gross hack to allow methods declared in Dev UI JSON RPC service classes,
// as the proper way (consuming `JsonRPCProvidersBuildItem`) only works in dev mode
String clazz = method.declaringClass().name().toString().toLowerCase(Locale.ROOT);
return clazz.startsWith("io.quarkus.")
|| clazz.startsWith("io.quarkiverse.")
|| clazz.endsWith("jsonrpcservice");
}
});
}

@SuppressForbidden(reason = "Using Type.toString() to build an informative message")
private String methodToString(MethodInfo method) {
StringBuilder result = new StringBuilder();
result.append(method.declaringClass().name()).append('.').append(method.name());
StringJoiner joiner = new StringJoiner(", ", "(", ")");
for (Type parameter : method.parameterTypes()) {
joiner.add(parameter.toString());
}
result.append(joiner);
return result.toString();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package io.quarkus.grpc.deployment;

import java.util.function.Predicate;

import org.jboss.jandex.MethodInfo;

import io.quarkus.deployment.annotations.BuildStep;
import io.quarkus.deployment.execannotations.ExecutionModelAnnotationsAllowedBuildItem;

public class GrpcMethodsProcessor {
@BuildStep
ExecutionModelAnnotationsAllowedBuildItem grpcMethods() {
return new ExecutionModelAnnotationsAllowedBuildItem(new Predicate<MethodInfo>() {
@Override
public boolean test(MethodInfo method) {
return method.declaringClass().hasDeclaredAnnotation(GrpcDotNames.GRPC_SERVICE);
}
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package io.quarkus.vertx.web.deployment;

import java.util.function.Predicate;

import org.jboss.jandex.MethodInfo;

import io.quarkus.deployment.annotations.BuildStep;
import io.quarkus.deployment.execannotations.ExecutionModelAnnotationsAllowedBuildItem;

public class ReactiveRoutesMethodsProcessor {
@BuildStep
ExecutionModelAnnotationsAllowedBuildItem reactiveRoutesMethods() {
return new ExecutionModelAnnotationsAllowedBuildItem(new Predicate<MethodInfo>() {
@Override
public boolean test(MethodInfo method) {
return method.hasDeclaredAnnotation(DotNames.ROUTE)
|| method.hasDeclaredAnnotation(DotNames.ROUTES);
}
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package io.quarkus.execannotations;

Check failure on line 1 in extensions/reactive-routes/deployment/src/test/java/io/quarkus/execannotations/ExecAnnotationInvalidTest.java

View workflow job for this annotation

GitHub Actions / Build summary for 7367ea51fefbac43686f64e678b0a73cf8f7b958

JVM Tests - JDK 11

org.opentest4j.AssertionFailedError: The build was expected to fail at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:662) at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
Raw output
org.opentest4j.AssertionFailedError: The build was expected to fail
	at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:662)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)

Check failure on line 1 in extensions/reactive-routes/deployment/src/test/java/io/quarkus/execannotations/ExecAnnotationInvalidTest.java

View workflow job for this annotation

GitHub Actions / Build summary for 7367ea51fefbac43686f64e678b0a73cf8f7b958

JVM Tests - JDK 17

org.opentest4j.AssertionFailedError: The build was expected to fail at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:662) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Raw output
org.opentest4j.AssertionFailedError: The build was expected to fail
	at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:662)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Check failure on line 1 in extensions/reactive-routes/deployment/src/test/java/io/quarkus/execannotations/ExecAnnotationInvalidTest.java

View workflow job for this annotation

GitHub Actions / Build summary for 7367ea51fefbac43686f64e678b0a73cf8f7b958

JVM Tests - JDK 17 Windows

org.opentest4j.AssertionFailedError: The build was expected to fail at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:662) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Raw output
org.opentest4j.AssertionFailedError: The build was expected to fail
	at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:662)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Check failure on line 1 in extensions/reactive-routes/deployment/src/test/java/io/quarkus/execannotations/ExecAnnotationInvalidTest.java

View workflow job for this annotation

GitHub Actions / Build summary for 7367ea51fefbac43686f64e678b0a73cf8f7b958

JVM Tests - JDK 20

org.opentest4j.AssertionFailedError: The build was expected to fail at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:662) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Raw output
org.opentest4j.AssertionFailedError: The build was expected to fail
	at io.quarkus.test.QuarkusUnitTest.beforeAll(QuarkusUnitTest.java:662)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;
import io.smallrye.common.annotation.Blocking;

public class ExecAnnotationInvalidTest {
@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withApplicationRoot(jar -> jar.addClasses(MyService.class))
.assertException(e -> {
assertInstanceOf(IllegalStateException.class, e);
assertTrue(e.getMessage().contains("Wrong usage"));
assertTrue(e.getMessage().contains("MyService.hello()"));
});

@Test
public void test() {
fail();
}

static class MyService {
@Blocking
String hello() {
return "Hello world!";
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package io.quarkus.execannotations;

import static io.restassured.RestAssured.when;
import static org.hamcrest.Matchers.is;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;
import io.quarkus.vertx.web.Route;
import io.smallrye.common.annotation.Blocking;

public class ExecAnnotationValidTest {
@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.withApplicationRoot(jar -> jar.addClasses(MyService.class));

@Test
public void test() {
when().get("/").then().statusCode(200).body(is("Hello world!"));
}

static class MyService {
@Route(path = "/")
@Blocking
String hello() {
return "Hello world!";
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package io.quarkus.resteasy.common.deployment;

import java.util.function.Predicate;

import org.jboss.jandex.MethodInfo;

import io.quarkus.deployment.annotations.BuildStep;
import io.quarkus.deployment.execannotations.ExecutionModelAnnotationsAllowedBuildItem;
import io.quarkus.resteasy.common.spi.ResteasyDotNames;

public class JaxrsMethodsProcessor {
@BuildStep
ExecutionModelAnnotationsAllowedBuildItem jaxrsMethods() {
return new ExecutionModelAnnotationsAllowedBuildItem(new Predicate<MethodInfo>() {
@Override
public boolean test(MethodInfo method) {
// looking for `@Path` on the declaring class is enough
// to avoid having to process inherited JAX-RS annotations
if (method.declaringClass().hasDeclaredAnnotation(ResteasyDotNames.PATH)) {
return true;
}

// we currently don't handle custom @HttpMethod annotations, should be fine most of the time
return method.hasDeclaredAnnotation(ResteasyDotNames.PATH)
|| method.hasDeclaredAnnotation(ResteasyDotNames.GET)
|| method.hasDeclaredAnnotation(ResteasyDotNames.POST)
|| method.hasDeclaredAnnotation(ResteasyDotNames.PUT)
|| method.hasDeclaredAnnotation(ResteasyDotNames.DELETE)
|| method.hasDeclaredAnnotation(ResteasyDotNames.PATCH)
|| method.hasDeclaredAnnotation(ResteasyDotNames.HEAD)
|| method.hasDeclaredAnnotation(ResteasyDotNames.OPTIONS);
}
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package io.quarkus.resteasy.reactive.common.deployment;

import java.util.function.Predicate;

import org.jboss.jandex.MethodInfo;
import org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames;

import io.quarkus.deployment.annotations.BuildStep;
import io.quarkus.deployment.execannotations.ExecutionModelAnnotationsAllowedBuildItem;

public class JaxrsMethodsProcessor {
@BuildStep
ExecutionModelAnnotationsAllowedBuildItem jaxrsMethods() {
return new ExecutionModelAnnotationsAllowedBuildItem(new Predicate<MethodInfo>() {
@Override
public boolean test(MethodInfo method) {
// looking for `@Path` on the declaring class is enough
// to avoid having to process inherited JAX-RS annotations
if (method.declaringClass().hasDeclaredAnnotation(ResteasyReactiveDotNames.PATH)) {
return true;
}

// we currently don't handle custom @HttpMethod annotations, should be fine most of the time
return method.hasDeclaredAnnotation(ResteasyReactiveDotNames.PATH)
|| method.hasDeclaredAnnotation(ResteasyReactiveDotNames.GET)
|| method.hasDeclaredAnnotation(ResteasyReactiveDotNames.POST)
|| method.hasDeclaredAnnotation(ResteasyReactiveDotNames.PUT)
|| method.hasDeclaredAnnotation(ResteasyReactiveDotNames.DELETE)
|| method.hasDeclaredAnnotation(ResteasyReactiveDotNames.PATCH)
|| method.hasDeclaredAnnotation(ResteasyReactiveDotNames.HEAD)
|| method.hasDeclaredAnnotation(ResteasyReactiveDotNames.OPTIONS);
}
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package io.quarkus.scheduler.deployment;

import java.util.function.Predicate;

import org.jboss.jandex.MethodInfo;

import io.quarkus.deployment.annotations.BuildStep;
import io.quarkus.deployment.execannotations.ExecutionModelAnnotationsAllowedBuildItem;

public class SchedulerMethodsProcessor {
@BuildStep
ExecutionModelAnnotationsAllowedBuildItem schedulerMethods() {
return new ExecutionModelAnnotationsAllowedBuildItem(new Predicate<MethodInfo>() {
@Override
public boolean test(MethodInfo method) {
return method.hasDeclaredAnnotation(SchedulerDotNames.SCHEDULED_NAME)
|| method.hasDeclaredAnnotation(SchedulerDotNames.SCHEDULES_NAME);
}
});
}
}
Loading

0 comments on commit 7367ea5

Please sign in to comment.