From e5a688f5cfaaaa3550b439afc279cc56c90d1a85 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Tue, 24 Aug 2021 09:47:27 +0200 Subject: [PATCH] Fix SmallRye Fault Tolerance build-time class loading The Fault Tolerance extension performs discovery at build time. It finds bean classes that use fault tolerance annotations and passes that information to runtime (where SmallRye Fault Tolerance applies runtime configuration). To represent types, SmallRye Fault Tolerance uses `Class` objects, and so the discovery process used to load bean classes from the deployment classloader. This works fine, unless one of the bean classes is generated by another Quarkus extension. These generated classes are stored in memory and the deployment classloader doesn't know about them. Naturally, attempting to load the class fails and aborts the build. This commit fixes the problem by not loading the classes at all. Instead, a class proxy is generated for each class. This still lets us use `Class` objects to represent types, but they are never loaded during build, only at runtime. The class proxy construct is marked as deprecated, because loading classes at build time is possible (it wasn't in the past). In this situation, though, where the class potentially isn't available, it's a perfect fit. --- .../quarkus/deployment/util/JandexUtil.java | 52 -------------- .../deployment/util/JandexUtilTest.java | 69 ------------------- .../rest-client-reactive/deployment/pom.xml | 5 ++ .../ft/AsyncRestClientFallbackTest.java | 68 ++++++++++++++++++ .../reactive/ft/RestClientFallbackTest.java | 63 +++++++++++++++++ .../ft/UniRestClientFallbackTest.java | 63 +++++++++++++++++ .../deployment/FaultToleranceScanner.java | 32 +++++---- .../SmallRyeFaultToleranceProcessor.java | 5 +- 8 files changed, 223 insertions(+), 134 deletions(-) create mode 100644 extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/ft/AsyncRestClientFallbackTest.java create mode 100644 extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/ft/RestClientFallbackTest.java create mode 100644 extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/ft/UniRestClientFallbackTest.java diff --git a/core/deployment/src/main/java/io/quarkus/deployment/util/JandexUtil.java b/core/deployment/src/main/java/io/quarkus/deployment/util/JandexUtil.java index c79c8285ea7c4..743d4a24155e6 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/util/JandexUtil.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/util/JandexUtil.java @@ -1,6 +1,5 @@ package io.quarkus.deployment.util; -import java.lang.reflect.Array; import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collections; @@ -377,55 +376,4 @@ public ClassNotIndexedException(DotName dotName) { } } - public static Class loadRawType(Type type) { - switch (type.kind()) { - case VOID: - return void.class; - case PRIMITIVE: - switch (type.asPrimitiveType().primitive()) { - case BOOLEAN: - return boolean.class; - case CHAR: - return char.class; - case BYTE: - return byte.class; - case SHORT: - return short.class; - case INT: - return int.class; - case LONG: - return long.class; - case FLOAT: - return float.class; - case DOUBLE: - return double.class; - default: - throw new IllegalArgumentException("Unknown primitive type: " + type); - } - case CLASS: - return load(type.asClassType().name()); - case PARAMETERIZED_TYPE: - return load(type.asParameterizedType().name()); - case ARRAY: - Class component = loadRawType(type.asArrayType().component()); - int dimensions = type.asArrayType().dimensions(); - return Array.newInstance(component, new int[dimensions]).getClass(); - case WILDCARD_TYPE: - return loadRawType(type.asWildcardType().extendsBound()); - case TYPE_VARIABLE: - return load(type.asTypeVariable().name()); - case UNRESOLVED_TYPE_VARIABLE: - return Object.class; // can't do better here - default: - throw new IllegalArgumentException("Unknown type: " + type); - } - } - - private static Class load(DotName name) { - try { - return Thread.currentThread().getContextClassLoader().loadClass(name.toString()); - } catch (ClassNotFoundException e) { - throw new RuntimeException(e); - } - } } diff --git a/core/deployment/src/test/java/io/quarkus/deployment/util/JandexUtilTest.java b/core/deployment/src/test/java/io/quarkus/deployment/util/JandexUtilTest.java index 3b86dd1d18e84..e2b8a47b0130c 100644 --- a/core/deployment/src/test/java/io/quarkus/deployment/util/JandexUtilTest.java +++ b/core/deployment/src/test/java/io/quarkus/deployment/util/JandexUtilTest.java @@ -2,16 +2,13 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; -import static org.junit.jupiter.api.Assertions.assertEquals; import java.io.IOException; import java.io.InputStream; -import java.io.Serializable; import java.util.Collection; import java.util.List; import java.util.Map; -import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; import org.jboss.jandex.Index; import org.jboss.jandex.Indexer; @@ -305,72 +302,6 @@ public static class MultiBoundedRepo implements Repo { public static class ErasedRepo2 extends MultiBoundedRepo { } - @Test - public void testLoadRawType() { - Index index = index(TestMethods.class); - ClassInfo clazz = index.getClassByName(DotName.createSimple(TestMethods.class.getName())); - - assertEquals(void.class, JandexUtil.loadRawType(clazz.method("aaa").returnType())); - assertEquals(int.class, JandexUtil.loadRawType(clazz.method("bbb").returnType())); - assertEquals(String.class, JandexUtil.loadRawType(clazz.method("ccc").returnType())); - assertEquals(List.class, JandexUtil.loadRawType(clazz.method("ddd").returnType())); - assertEquals(String[][].class, JandexUtil.loadRawType(clazz.method("eee").returnType())); - assertEquals(Object.class, JandexUtil.loadRawType(clazz.method("fff").returnType())); - assertEquals(Number.class, JandexUtil.loadRawType(clazz.method("ggg").returnType())); - assertEquals(Number.class, JandexUtil.loadRawType(clazz.method("hhh").returnType())); - assertEquals(Comparable.class, JandexUtil.loadRawType(clazz.method("iii").returnType())); - assertEquals(Comparable.class, JandexUtil.loadRawType(clazz.method("jjj").returnType())); - assertEquals(Serializable.class, JandexUtil.loadRawType(clazz.method("kkk").returnType())); - assertEquals(Object.class, JandexUtil.loadRawType(clazz.method("lll").returnType() - .asParameterizedType().arguments().get(0))); - assertEquals(Number.class, JandexUtil.loadRawType(clazz.method("mmm").returnType() - .asParameterizedType().arguments().get(0))); - assertEquals(Object.class, JandexUtil.loadRawType(clazz.method("nnn").returnType() - .asParameterizedType().arguments().get(0))); - assertEquals(Object.class, JandexUtil.loadRawType(clazz.method("ooo").returnType() - .asParameterizedType().arguments().get(0))); - assertEquals(Number.class, JandexUtil.loadRawType(clazz.method("ppp").returnType() - .asParameterizedType().arguments().get(0))); - assertEquals(Object.class, JandexUtil.loadRawType(clazz.method("qqq").returnType() - .asParameterizedType().arguments().get(0))); - } - - public interface TestMethods { - void aaa(); - - int bbb(); - - String ccc(); - - List ddd(); - - String[][] eee(); - - X fff(); - - Y ggg(); - - > Y hhh(); - - > Y iii(); - - & Serializable> Y jjj(); - - > Y kkk(); - - List lll(); - - List mmm(); - - List nnn(); - - List ooo(); - - List ppp(); - - List qqq(); - } - private static Index index(Class... classes) { Indexer indexer = new Indexer(); for (Class clazz : classes) { diff --git a/extensions/resteasy-reactive/rest-client-reactive/deployment/pom.xml b/extensions/resteasy-reactive/rest-client-reactive/deployment/pom.xml index d60bc5768a1a9..207e881efd431 100644 --- a/extensions/resteasy-reactive/rest-client-reactive/deployment/pom.xml +++ b/extensions/resteasy-reactive/rest-client-reactive/deployment/pom.xml @@ -26,6 +26,11 @@ quarkus-resteasy-reactive-jackson-deployment test + + io.quarkus + quarkus-smallrye-fault-tolerance-deployment + test + io.quarkus quarkus-junit5-internal diff --git a/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/ft/AsyncRestClientFallbackTest.java b/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/ft/AsyncRestClientFallbackTest.java new file mode 100644 index 0000000000000..2a46b22922e5b --- /dev/null +++ b/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/ft/AsyncRestClientFallbackTest.java @@ -0,0 +1,68 @@ +package io.quarkus.rest.client.reactive.ft; + +import static io.quarkus.rest.client.reactive.RestClientTestUtil.setUrlForClass; +import static java.util.concurrent.CompletableFuture.completedFuture; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.concurrent.CompletionStage; +import java.util.concurrent.ExecutionException; + +import javax.inject.Inject; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.WebApplicationException; + +import org.eclipse.microprofile.faulttolerance.Asynchronous; +import org.eclipse.microprofile.faulttolerance.ExecutionContext; +import org.eclipse.microprofile.faulttolerance.Fallback; +import org.eclipse.microprofile.faulttolerance.FallbackHandler; +import org.eclipse.microprofile.rest.client.inject.RegisterRestClient; +import org.eclipse.microprofile.rest.client.inject.RestClient; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.asset.StringAsset; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusUnitTest; + +public class AsyncRestClientFallbackTest { + @RegisterExtension + static final QuarkusUnitTest TEST = new QuarkusUnitTest() + .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) + .addClasses(TestEndpoint.class, Client.class, MyFallback.class) + .addAsResource(new StringAsset(setUrlForClass(Client.class)), "application.properties")); + + @Inject + @RestClient + Client client; + + @Test + public void testFallbackWasUsed() throws ExecutionException, InterruptedException { + assertEquals("pong", client.ping().toCompletableFuture().get()); + } + + @Path("/test") + public static class TestEndpoint { + @GET + public String get() { + throw new WebApplicationException(404); + } + } + + @RegisterRestClient + public interface Client { + @GET + @Path("/test") + @Asynchronous + @Fallback(MyFallback.class) + CompletionStage ping(); + } + + public static class MyFallback implements FallbackHandler> { + @Override + public CompletionStage handle(ExecutionContext context) { + return completedFuture("pong"); + } + } +} diff --git a/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/ft/RestClientFallbackTest.java b/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/ft/RestClientFallbackTest.java new file mode 100644 index 0000000000000..da3dfd95e1b8f --- /dev/null +++ b/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/ft/RestClientFallbackTest.java @@ -0,0 +1,63 @@ +package io.quarkus.rest.client.reactive.ft; + +import static io.quarkus.rest.client.reactive.RestClientTestUtil.setUrlForClass; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import javax.inject.Inject; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.WebApplicationException; + +import org.eclipse.microprofile.faulttolerance.ExecutionContext; +import org.eclipse.microprofile.faulttolerance.Fallback; +import org.eclipse.microprofile.faulttolerance.FallbackHandler; +import org.eclipse.microprofile.rest.client.inject.RegisterRestClient; +import org.eclipse.microprofile.rest.client.inject.RestClient; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.asset.StringAsset; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusUnitTest; + +public class RestClientFallbackTest { + @RegisterExtension + static final QuarkusUnitTest TEST = new QuarkusUnitTest() + .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) + .addClasses(TestEndpoint.class, Client.class, MyFallback.class) + .addAsResource(new StringAsset(setUrlForClass(Client.class)), "application.properties")); + + @Inject + @RestClient + Client client; + + @Test + public void testFallbackWasUsed() { + assertEquals("pong", client.ping()); + } + + @Path("/test") + public static class TestEndpoint { + @GET + public String get() { + throw new WebApplicationException(404); + } + } + + @RegisterRestClient + public interface Client { + @GET + @Path("/test") + @Fallback(MyFallback.class) + String ping(); + } + + public static class MyFallback implements FallbackHandler { + @Override + public String handle(ExecutionContext context) { + return "pong"; + } + + } +} diff --git a/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/ft/UniRestClientFallbackTest.java b/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/ft/UniRestClientFallbackTest.java new file mode 100644 index 0000000000000..360eefb016c0b --- /dev/null +++ b/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/ft/UniRestClientFallbackTest.java @@ -0,0 +1,63 @@ +package io.quarkus.rest.client.reactive.ft; + +import static io.quarkus.rest.client.reactive.RestClientTestUtil.setUrlForClass; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import javax.inject.Inject; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.WebApplicationException; + +import org.eclipse.microprofile.faulttolerance.ExecutionContext; +import org.eclipse.microprofile.faulttolerance.Fallback; +import org.eclipse.microprofile.faulttolerance.FallbackHandler; +import org.eclipse.microprofile.rest.client.inject.RegisterRestClient; +import org.eclipse.microprofile.rest.client.inject.RestClient; +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.asset.StringAsset; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusUnitTest; +import io.smallrye.mutiny.Uni; + +public class UniRestClientFallbackTest { + @RegisterExtension + static final QuarkusUnitTest TEST = new QuarkusUnitTest() + .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) + .addClasses(TestEndpoint.class, Client.class, MyFallback.class) + .addAsResource(new StringAsset(setUrlForClass(Client.class)), "application.properties")); + + @Inject + @RestClient + Client client; + + @Test + public void testFallbackWasUsed() { + assertEquals("pong", client.ping().await().indefinitely()); + } + + @Path("/test") + public static class TestEndpoint { + @GET + public String get() { + throw new WebApplicationException(404); + } + } + + @RegisterRestClient + public interface Client { + @GET + @Path("/test") + @Fallback(MyFallback.class) + Uni ping(); + } + + public static class MyFallback implements FallbackHandler> { + @Override + public Uni handle(ExecutionContext context) { + return Uni.createFrom().item("pong"); + } + } +} diff --git a/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/FaultToleranceScanner.java b/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/FaultToleranceScanner.java index e787009138a8e..849d004c98b55 100644 --- a/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/FaultToleranceScanner.java +++ b/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/FaultToleranceScanner.java @@ -16,10 +16,11 @@ import org.jboss.jandex.DotName; import org.jboss.jandex.IndexView; import org.jboss.jandex.MethodInfo; +import org.jboss.jandex.Type; import io.quarkus.arc.processor.AnnotationStore; import io.quarkus.deployment.builditem.AnnotationProxyBuildItem; -import io.quarkus.deployment.util.JandexUtil; +import io.quarkus.deployment.recording.RecorderContext; import io.quarkus.gizmo.ClassOutput; import io.smallrye.common.annotation.Blocking; import io.smallrye.common.annotation.NonBlocking; @@ -37,12 +38,15 @@ final class FaultToleranceScanner { private final AnnotationProxyBuildItem proxy; private final ClassOutput output; + private final RecorderContext recorderContext; + FaultToleranceScanner(IndexView index, AnnotationStore annotationStore, AnnotationProxyBuildItem proxy, - ClassOutput output) { + ClassOutput output, RecorderContext recorderContext) { this.index = index; this.annotationStore = annotationStore; this.proxy = proxy; this.output = output; + this.recorderContext = recorderContext; } boolean hasFTAnnotations(ClassInfo clazz) { @@ -100,7 +104,7 @@ FaultToleranceMethod createFaultToleranceMethod(ClassInfo beanClass, MethodInfo FaultToleranceMethod result = new FaultToleranceMethod(); - result.beanClass = load(beanClass.name()); + result.beanClass = getClassProxy(beanClass); result.method = createMethodDescriptor(method); result.asynchronous = getAnnotation(Asynchronous.class, method, beanClass, annotationsPresentDirectly); @@ -125,13 +129,13 @@ FaultToleranceMethod createFaultToleranceMethod(ClassInfo beanClass, MethodInfo private MethodDescriptor createMethodDescriptor(MethodInfo method) { MethodDescriptor result = new MethodDescriptor(); - result.declaringClass = load(method.declaringClass().name()); + result.declaringClass = getClassProxy(method.declaringClass()); result.name = method.name(); result.parameterTypes = method.parameters() .stream() - .map(JandexUtil::loadRawType) + .map(this::getClassProxy) .toArray(Class[]::new); - result.returnType = JandexUtil.loadRawType(method.returnType()); + result.returnType = getClassProxy(method.returnType()); return result; } @@ -171,11 +175,15 @@ private A createAnnotation(Class annotationType, Annot return proxy.builder(instance, annotationType).build(output); } - private static Class load(DotName name) { - try { - return Thread.currentThread().getContextClassLoader().loadClass(name.toString()); - } catch (ClassNotFoundException e) { - throw new RuntimeException(e); - } + // using class proxies instead of attempting to load the class, because the class + // doesn't have to exist in the deployment classloader at all -- instead, it may be + // generated by another Quarkus extension (such as RestClient Reactive) + private Class getClassProxy(ClassInfo clazz) { + return recorderContext.classProxy(clazz.name().toString()); + } + + private Class getClassProxy(Type type) { + // Type.name() returns the right thing + return recorderContext.classProxy(type.name().toString()); } } diff --git a/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/SmallRyeFaultToleranceProcessor.java b/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/SmallRyeFaultToleranceProcessor.java index 97fca2a96cfd1..42c2f9a6ebe79 100644 --- a/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/SmallRyeFaultToleranceProcessor.java +++ b/extensions/smallrye-fault-tolerance/deployment/src/main/java/io/quarkus/smallrye/faulttolerance/deployment/SmallRyeFaultToleranceProcessor.java @@ -54,6 +54,7 @@ import io.quarkus.deployment.builditem.nativeimage.ReflectiveMethodBuildItem; import io.quarkus.deployment.builditem.nativeimage.ServiceProviderBuildItem; import io.quarkus.deployment.metrics.MetricsCapabilityBuildItem; +import io.quarkus.deployment.recording.RecorderContext; import io.quarkus.gizmo.ClassOutput; import io.quarkus.smallrye.faulttolerance.runtime.QuarkusAsyncExecutorProvider; import io.quarkus.smallrye.faulttolerance.runtime.QuarkusExistingCircuitBreakerNames; @@ -243,6 +244,7 @@ public void transform(TransformationContext ctx) { // needs to be RUNTIME_INIT because we need to read MP Config @Record(ExecutionTime.RUNTIME_INIT) void validateFaultToleranceAnnotations(SmallRyeFaultToleranceRecorder recorder, + RecorderContext recorderContext, ValidationPhaseBuildItem validationPhase, BeanArchiveIndexBuildItem beanArchiveIndexBuildItem, AnnotationProxyBuildItem annotationProxy, @@ -255,7 +257,8 @@ void validateFaultToleranceAnnotations(SmallRyeFaultToleranceRecorder recorder, // none of them are application classes ClassOutput classOutput = new GeneratedClassGizmoAdaptor(generatedClasses, false); - FaultToleranceScanner scaner = new FaultToleranceScanner(index, annotationStore, annotationProxy, classOutput); + FaultToleranceScanner scaner = new FaultToleranceScanner(index, annotationStore, annotationProxy, classOutput, + recorderContext); List ftMethods = new ArrayList<>(); List exceptions = new ArrayList<>();