From aae7d30d56874930813cbdb9162e43aa076be9d3 Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Mon, 6 Jul 2020 17:16:29 +0300 Subject: [PATCH] Allow Arc to deal with final methods of beans that needs to be proxied This is done in the same manner that is already present for interceptors and is very useful for Kotlin code where methods are final by default Fixes: #10290 --- .../quarkus/arc/deployment/ArcProcessor.java | 12 +- .../RequestScopedFinalMethodsTest.java | 106 ++++++++++++++++++ .../quarkus/arc/processor/BeanProcessor.java | 12 +- .../arc/processor/ClientProxyGenerator.java | 33 ++++-- .../io/quarkus/arc/processor/Methods.java | 66 +++++++---- 5 files changed, 192 insertions(+), 37 deletions(-) create mode 100644 extensions/arc/deployment/src/test/java/io/quarkus/arc/test/unproxyable/RequestScopedFinalMethodsTest.java diff --git a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java index 78e252b890e9b5..ff9cd0d9e86f63 100644 --- a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java +++ b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/ArcProcessor.java @@ -410,7 +410,8 @@ public BeanContainerBuildItem generateResources(ArcRecorder recorder, ShutdownCo BuildProducer reflectiveFields, BuildProducer generatedClass, LiveReloadBuildItem liveReloadBuildItem, - BuildProducer generatedResource) throws Exception { + BuildProducer generatedResource, + BuildProducer bytecodeTransformer) throws Exception { for (ValidationErrorBuildItem validationError : validationErrors) { for (Throwable error : validationError.getValues()) { @@ -426,6 +427,13 @@ public BeanContainerBuildItem generateResources(ArcRecorder recorder, ShutdownCo liveReloadBuildItem.setContextObject(ExistingClasses.class, existingClasses); } + Consumer bytecodeTransformerConsumer = new Consumer() { + @Override + public void accept(BytecodeTransformer t) { + bytecodeTransformer.produce(new BytecodeTransformerBuildItem(t.getClassToTransform(), t.getVisitorFunction())); + } + }; + long start = System.currentTimeMillis(); List resources = beanProcessor.generateResources(new ReflectionRegistration() { @Override @@ -437,7 +445,7 @@ public void registerMethod(MethodInfo methodInfo) { public void registerField(FieldInfo fieldInfo) { reflectiveFields.produce(new ReflectiveFieldBuildItem(fieldInfo)); } - }, existingClasses.existingClasses); + }, existingClasses.existingClasses, bytecodeTransformerConsumer); for (ResourceOutput.Resource resource : resources) { switch (resource.getType()) { case JAVA_CLASS: diff --git a/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/unproxyable/RequestScopedFinalMethodsTest.java b/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/unproxyable/RequestScopedFinalMethodsTest.java new file mode 100644 index 00000000000000..1b9f8760025418 --- /dev/null +++ b/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/unproxyable/RequestScopedFinalMethodsTest.java @@ -0,0 +1,106 @@ +package io.quarkus.arc.test.unproxyable; + +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; + +import javax.annotation.PostConstruct; +import javax.enterprise.context.Dependent; +import javax.enterprise.context.RequestScoped; +import javax.enterprise.event.Event; +import javax.enterprise.event.Observes; +import javax.inject.Inject; +import javax.inject.Singleton; + +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.ArcContainer; +import io.quarkus.arc.InstanceHandle; +import io.quarkus.arc.ManagedContext; +import io.quarkus.test.QuarkusUnitTest; + +public class RequestScopedFinalMethodsTest { + + @RegisterExtension + public static QuarkusUnitTest container = new QuarkusUnitTest() + .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class) + .addClasses(RequestScopedBean.class)); + + @Test + public void testRequestScopedBeanWorksProperly() { + ArcContainer container = Arc.container(); + ManagedContext requestContext = container.requestContext(); + requestContext.activate(); + + InstanceHandle handle = container.instance(RequestScopedBean.class); + Assertions.assertTrue(handle.isAvailable()); + + RequestScopedBean bean = handle.get(); + Assertions.assertNull(bean.getProp()); + bean.setProp(100); + Assertions.assertEquals(100, bean.getProp()); + + requestContext.terminate(); + requestContext.activate(); + + handle = container.instance(RequestScopedBean.class); + bean = handle.get(); + Assertions.assertTrue(handle.isAvailable()); + Assertions.assertNull(bean.getProp()); + } + + @RequestScoped + static class RequestScopedBean { + private Integer prop = null; + + public final Integer getProp() { + return prop; + } + + public final void setProp(Integer prop) { + this.prop = prop; + } + } + + @Dependent + static class StringProducer { + + @Inject + Event event; + + void fire(String value) { + event.fire(value); + } + + } + + @Singleton + static class StringObserver { + + private List events; + + @Inject + RequestScopedBean requestScopedBean; + + @PostConstruct + void init() { + events = new CopyOnWriteArrayList<>(); + } + + void observeSync(@Observes Integer value) { + Integer oldValue = requestScopedBean.getProp(); + Integer newValue = oldValue == null ? value : value + oldValue; + requestScopedBean.setProp(newValue); + events.add(newValue); + } + + List getEvents() { + return events; + } + + } +} diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanProcessor.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanProcessor.java index e4e15cd244c12f..1b6a56b256da93 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanProcessor.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanProcessor.java @@ -38,7 +38,7 @@ *
  • {@link #initialize(Consumer)}
    • *
  • {@link #validate(Consumer)}
    • *
  • {@link #processValidationErrors(io.quarkus.arc.processor.BeanDeploymentValidator.ValidationContext)}
    • - *
  • {@link #generateResources(ReflectionRegistration, Set)}
    • + *
  • {@link #generateResources(ReflectionRegistration, Set, Consumer)}
    • * */ public class BeanProcessor { @@ -64,6 +64,7 @@ public static Builder builder() { private final BeanDeployment beanDeployment; private final boolean generateSources; private final boolean allowMocking; + private final boolean transformUnproxyableClasses; // This predicate is used to filter annotations for InjectionPoint metadata // Note that we do create annotation literals for all annotations for an injection point that resolves to a @Dependent bean that injects the InjectionPoint metadata @@ -79,6 +80,7 @@ private BeanProcessor(Builder builder) { this.annotationLiterals = new AnnotationLiteralProcessor(builder.sharedAnnotationLiterals, applicationClassPredicate); this.generateSources = builder.generateSources; this.allowMocking = builder.allowMocking; + this.transformUnproxyableClasses = builder.transformUnproxyableClasses; // Initialize all build processors buildContext = new BuildContextImpl(); @@ -133,7 +135,8 @@ public void processValidationErrors(BeanDeploymentValidator.ValidationContext va BeanDeployment.processErrors(validationContext.getDeploymentProblems()); } - public List generateResources(ReflectionRegistration reflectionRegistration, Set existingClasses) + public List generateResources(ReflectionRegistration reflectionRegistration, Set existingClasses, + Consumer bytecodeTransformerConsumer) throws IOException { if (reflectionRegistration == null) { reflectionRegistration = this.reflectionRegistration; @@ -174,7 +177,8 @@ public List generateResources(ReflectionRegistration reflectionRegistr if (bean.getScope().isNormal()) { // Generate client proxy resources.addAll( - clientProxyGenerator.generate(bean, resource.getFullyQualifiedName())); + clientProxyGenerator.generate(bean, resource.getFullyQualifiedName(), + bytecodeTransformerConsumer, transformUnproxyableClasses)); } if (bean.isSubclassRequired()) { resources.addAll( @@ -234,7 +238,7 @@ public void accept(BytecodeTransformer transformer) { initialize(unsupportedBytecodeTransformer); ValidationContext validationContext = validate(unsupportedBytecodeTransformer); processValidationErrors(validationContext); - generateResources(null, new HashSet<>()); + generateResources(null, new HashSet<>(), unsupportedBytecodeTransformer); return beanDeployment; } diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/ClientProxyGenerator.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/ClientProxyGenerator.java index d4507c7b0183d9..08bd3a582b9faa 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/ClientProxyGenerator.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/ClientProxyGenerator.java @@ -25,9 +25,11 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Consumer; import java.util.function.Predicate; import javax.enterprise.context.ContextNotActiveException; import javax.enterprise.context.spi.Contextual; @@ -70,9 +72,12 @@ public ClientProxyGenerator(Predicate applicationClassPredicate, boolea * * @param bean * @param beanClassName Fully qualified class name + * @param bytecodeTransformerConsumer + * @param transformUnproxyableClasses whether or not unproxyable classes should be transformed * @return a collection of resources */ - Collection generate(BeanInfo bean, String beanClassName) { + Collection generate(BeanInfo bean, String beanClassName, + Consumer bytecodeTransformerConsumer, boolean transformUnproxyableClasses) { ResourceClassOutput classOutput = new ResourceClassOutput(applicationClassPredicate.test(bean.getBeanClass()), generateSources); @@ -127,7 +132,7 @@ Collection generate(BeanInfo bean, String beanClassName) { implementMockMethods(clientProxy); } - for (MethodInfo method : getDelegatingMethods(bean)) { + for (MethodInfo method : getDelegatingMethods(bean, bytecodeTransformerConsumer, transformUnproxyableClasses)) { MethodDescriptor originalMethodDescriptor = MethodDescriptor.of(method); MethodCreator forward = clientProxy.getMethodCreator(originalMethodDescriptor); @@ -302,23 +307,35 @@ void implementGetBean(ClassCreator clientProxy, FieldDescriptor beanField) { creator.returnValue(creator.readInstanceField(beanField, creator.getThis())); } - Collection getDelegatingMethods(BeanInfo bean) { + Collection getDelegatingMethods(BeanInfo bean, Consumer bytecodeTransformerConsumer, + boolean transformUnproxyableClasses) { Map methods = new HashMap<>(); if (bean.isClassBean()) { - Methods.addDelegatingMethods(bean.getDeployment().getIndex(), bean.getTarget().get().asClass(), - methods); + Set methodsFromWhichToRemoveFinal = new HashSet<>(); + ClassInfo classInfo = bean.getTarget().get().asClass(); + Methods.addDelegatingMethods(bean.getDeployment().getIndex(), classInfo, + methods, methodsFromWhichToRemoveFinal, transformUnproxyableClasses); + if (!methodsFromWhichToRemoveFinal.isEmpty()) { + String className = classInfo.name().toString(); + bytecodeTransformerConsumer.accept(new BytecodeTransformer(className, + new Methods.RemoveFinalFromMethod(className, methodsFromWhichToRemoveFinal))); + } } else if (bean.isProducerMethod()) { MethodInfo producerMethod = bean.getTarget().get().asMethod(); ClassInfo returnTypeClass = getClassByName(bean.getDeployment().getIndex(), producerMethod.returnType()); - Methods.addDelegatingMethods(bean.getDeployment().getIndex(), returnTypeClass, methods); + Methods.addDelegatingMethods(bean.getDeployment().getIndex(), returnTypeClass, methods, null, + transformUnproxyableClasses); } else if (bean.isProducerField()) { FieldInfo producerField = bean.getTarget().get().asField(); ClassInfo fieldClass = getClassByName(bean.getDeployment().getIndex(), producerField.type()); - Methods.addDelegatingMethods(bean.getDeployment().getIndex(), fieldClass, methods); + Methods.addDelegatingMethods(bean.getDeployment().getIndex(), fieldClass, methods, null, + transformUnproxyableClasses); } else if (bean.isSynthetic()) { - Methods.addDelegatingMethods(bean.getDeployment().getIndex(), bean.getImplClazz(), methods); + Methods.addDelegatingMethods(bean.getDeployment().getIndex(), bean.getImplClazz(), methods, null, + transformUnproxyableClasses); } + return methods.values(); } diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java index 1ace72fdb77637..52bde54593077f 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java @@ -58,11 +58,12 @@ static boolean isSynthetic(MethodInfo method) { return (method.flags() & SYNTHETIC) != 0; } - static void addDelegatingMethods(IndexView index, ClassInfo classInfo, Map methods) { + static void addDelegatingMethods(IndexView index, ClassInfo classInfo, Map methods, + Set methodsFromWhichToRemoveFinal, boolean transformUnproxyableClasses) { // TODO support interfaces default methods if (classInfo != null) { for (MethodInfo method : classInfo.methods()) { - if (skipForClientProxy(method)) { + if (skipForClientProxy(method, transformUnproxyableClasses, methodsFromWhichToRemoveFinal)) { continue; } methods.computeIfAbsent(new Methods.MethodKey(method), key -> { @@ -82,19 +83,22 @@ static void addDelegatingMethods(IndexView index, ClassInfo classInfo, Map methodsFromWhichToRemoveFinal) { if (Modifier.isStatic(method.flags()) || Modifier.isPrivate(method.flags())) { return true; } @@ -108,6 +112,11 @@ private static boolean skipForClientProxy(MethodInfo method) { if (Modifier.isFinal(method.flags())) { String className = method.declaringClass().name().toString(); if (!className.startsWith("java.")) { + if (transformUnproxyableClasses && (methodsFromWhichToRemoveFinal != null)) { + methodsFromWhichToRemoveFinal.add(NameAndDescriptor.fromMethodInfo(method)); + return false; + } + LOGGER.warn(String.format( "Final method %s.%s() is ignored during proxy generation and should never be invoked upon the proxy instance!", className, method.name())); @@ -161,23 +170,8 @@ static Set addInterceptedMethodCandidates(BeanDeployment beanDeploym } if (!methodsFromWhichToRemoveFinal.isEmpty()) { bytecodeTransformerConsumer.accept( - new BytecodeTransformer(classInfo.name().toString(), new BiFunction() { - @Override - public ClassVisitor apply(String s, ClassVisitor classVisitor) { - return new ClassVisitor(Gizmo.ASM_API_VERSION, classVisitor) { - @Override - public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, - String[] exceptions) { - if (methodsFromWhichToRemoveFinal.contains(new NameAndDescriptor(name, descriptor))) { - access = access & (~Opcodes.ACC_FINAL); - LOGGER.debug("final modifier removed from method " + name + " of class " - + classInfo.name().toString()); - } - return super.visitMethod(access, name, descriptor, signature, exceptions); - } - }; - } - })); + new BytecodeTransformer(classInfo.name().toString(), + new RemoveFinalFromMethod(classInfo.name().toString(), methodsFromWhichToRemoveFinal))); } if (classInfo.superClassType() != null) { ClassInfo superClassInfo = getClassByName(beanDeployment.getIndex(), classInfo.superName()); @@ -197,7 +191,7 @@ public MethodVisitor visitMethod(int access, String name, String descriptor, Str return finalMethodsFoundAndNotChanged; } - private static class NameAndDescriptor { + static class NameAndDescriptor { private final String name; private final String descriptor; @@ -349,4 +343,30 @@ static DotName toRawType(Type a) { } } + static class RemoveFinalFromMethod implements BiFunction { + + private final String classToTransform; + private final Set methodsFromWhichToRemoveFinal; + + public RemoveFinalFromMethod(String classToTransform, Set methodsFromWhichToRemoveFinal) { + this.classToTransform = classToTransform; + this.methodsFromWhichToRemoveFinal = methodsFromWhichToRemoveFinal; + } + + @Override + public ClassVisitor apply(String s, ClassVisitor classVisitor) { + return new ClassVisitor(Gizmo.ASM_API_VERSION, classVisitor) { + @Override + public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, + String[] exceptions) { + if (methodsFromWhichToRemoveFinal.contains(new NameAndDescriptor(name, descriptor))) { + access = access & (~Opcodes.ACC_FINAL); + LOGGER.debug("final modifier removed from method " + name + " of class " + classToTransform); + } + return super.visitMethod(access, name, descriptor, signature, exceptions); + } + }; + } + } + }