Skip to content

Commit

Permalink
Allow Arc to deal with final methods of beans that needs to be proxied
Browse files Browse the repository at this point in the history
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: quarkusio#10290
  • Loading branch information
geoand committed Jul 6, 2020
1 parent 30baf78 commit aae7d30
Show file tree
Hide file tree
Showing 5 changed files with 192 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,8 @@ public BeanContainerBuildItem generateResources(ArcRecorder recorder, ShutdownCo
BuildProducer<ReflectiveFieldBuildItem> reflectiveFields,
BuildProducer<GeneratedClassBuildItem> generatedClass,
LiveReloadBuildItem liveReloadBuildItem,
BuildProducer<GeneratedResourceBuildItem> generatedResource) throws Exception {
BuildProducer<GeneratedResourceBuildItem> generatedResource,
BuildProducer<BytecodeTransformerBuildItem> bytecodeTransformer) throws Exception {

for (ValidationErrorBuildItem validationError : validationErrors) {
for (Throwable error : validationError.getValues()) {
Expand All @@ -426,6 +427,13 @@ public BeanContainerBuildItem generateResources(ArcRecorder recorder, ShutdownCo
liveReloadBuildItem.setContextObject(ExistingClasses.class, existingClasses);
}

Consumer<BytecodeTransformer> bytecodeTransformerConsumer = new Consumer<BytecodeTransformer>() {
@Override
public void accept(BytecodeTransformer t) {
bytecodeTransformer.produce(new BytecodeTransformerBuildItem(t.getClassToTransform(), t.getVisitorFunction()));
}
};

long start = System.currentTimeMillis();
List<ResourceOutput.Resource> resources = beanProcessor.generateResources(new ReflectionRegistration() {
@Override
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -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<RequestScopedBean> 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<String> event;

void fire(String value) {
event.fire(value);
}

}

@Singleton
static class StringObserver {

private List<Integer> 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<Integer> getEvents() {
return events;
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
* <li>{@link #initialize(Consumer)}</li>
* <li>{@link #validate(Consumer)}</li>
* <li>{@link #processValidationErrors(io.quarkus.arc.processor.BeanDeploymentValidator.ValidationContext)}</li>
* <li>{@link #generateResources(ReflectionRegistration, Set)}</li>
* <li>{@link #generateResources(ReflectionRegistration, Set, Consumer)}</li>
* </ol>
*/
public class BeanProcessor {
Expand All @@ -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
Expand All @@ -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();
Expand Down Expand Up @@ -133,7 +135,8 @@ public void processValidationErrors(BeanDeploymentValidator.ValidationContext va
BeanDeployment.processErrors(validationContext.getDeploymentProblems());
}

public List<Resource> generateResources(ReflectionRegistration reflectionRegistration, Set<String> existingClasses)
public List<Resource> generateResources(ReflectionRegistration reflectionRegistration, Set<String> existingClasses,
Consumer<BytecodeTransformer> bytecodeTransformerConsumer)
throws IOException {
if (reflectionRegistration == null) {
reflectionRegistration = this.reflectionRegistration;
Expand Down Expand Up @@ -174,7 +177,8 @@ public List<Resource> 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(
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -70,9 +72,12 @@ public ClientProxyGenerator(Predicate<DotName> 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<Resource> generate(BeanInfo bean, String beanClassName) {
Collection<Resource> generate(BeanInfo bean, String beanClassName,
Consumer<BytecodeTransformer> bytecodeTransformerConsumer, boolean transformUnproxyableClasses) {

ResourceClassOutput classOutput = new ResourceClassOutput(applicationClassPredicate.test(bean.getBeanClass()),
generateSources);
Expand Down Expand Up @@ -127,7 +132,7 @@ Collection<Resource> 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);
Expand Down Expand Up @@ -302,23 +307,35 @@ void implementGetBean(ClassCreator clientProxy, FieldDescriptor beanField) {
creator.returnValue(creator.readInstanceField(beanField, creator.getThis()));
}

Collection<MethodInfo> getDelegatingMethods(BeanInfo bean) {
Collection<MethodInfo> getDelegatingMethods(BeanInfo bean, Consumer<BytecodeTransformer> bytecodeTransformerConsumer,
boolean transformUnproxyableClasses) {
Map<Methods.MethodKey, MethodInfo> methods = new HashMap<>();

if (bean.isClassBean()) {
Methods.addDelegatingMethods(bean.getDeployment().getIndex(), bean.getTarget().get().asClass(),
methods);
Set<Methods.NameAndDescriptor> 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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,12 @@ static boolean isSynthetic(MethodInfo method) {
return (method.flags() & SYNTHETIC) != 0;
}

static void addDelegatingMethods(IndexView index, ClassInfo classInfo, Map<Methods.MethodKey, MethodInfo> methods) {
static void addDelegatingMethods(IndexView index, ClassInfo classInfo, Map<MethodKey, MethodInfo> methods,
Set<NameAndDescriptor> 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 -> {
Expand All @@ -82,19 +83,22 @@ static void addDelegatingMethods(IndexView index, ClassInfo classInfo, Map<Metho
for (Type interfaceType : classInfo.interfaceTypes()) {
ClassInfo interfaceClassInfo = getClassByName(index, interfaceType.name());
if (interfaceClassInfo != null) {
addDelegatingMethods(index, interfaceClassInfo, methods);
addDelegatingMethods(index, interfaceClassInfo, methods, methodsFromWhichToRemoveFinal,
transformUnproxyableClasses);
}
}
if (classInfo.superClassType() != null) {
ClassInfo superClassInfo = getClassByName(index, classInfo.superName());
if (superClassInfo != null) {
addDelegatingMethods(index, superClassInfo, methods);
addDelegatingMethods(index, superClassInfo, methods, methodsFromWhichToRemoveFinal,
transformUnproxyableClasses);
}
}
}
}

private static boolean skipForClientProxy(MethodInfo method) {
private static boolean skipForClientProxy(MethodInfo method, boolean transformUnproxyableClasses,
Set<NameAndDescriptor> methodsFromWhichToRemoveFinal) {
if (Modifier.isStatic(method.flags()) || Modifier.isPrivate(method.flags())) {
return true;
}
Expand All @@ -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()));
Expand Down Expand Up @@ -161,23 +170,8 @@ static Set<MethodInfo> addInterceptedMethodCandidates(BeanDeployment beanDeploym
}
if (!methodsFromWhichToRemoveFinal.isEmpty()) {
bytecodeTransformerConsumer.accept(
new BytecodeTransformer(classInfo.name().toString(), new BiFunction<String, ClassVisitor, ClassVisitor>() {
@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());
Expand All @@ -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;

Expand Down Expand Up @@ -349,4 +343,30 @@ static DotName toRawType(Type a) {
}
}

static class RemoveFinalFromMethod implements BiFunction<String, ClassVisitor, ClassVisitor> {

private final String classToTransform;
private final Set<NameAndDescriptor> methodsFromWhichToRemoveFinal;

public RemoveFinalFromMethod(String classToTransform, Set<NameAndDescriptor> 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);
}
};
}
}

}

0 comments on commit aae7d30

Please sign in to comment.