From 13972414d77e4bc8a1930937bc8c637ba9f9e32b Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Wed, 14 Feb 2024 17:05:53 +0100 Subject: [PATCH] ArC: fix interception when some methods return void This commit fixes 2 cases when invalid bytecode is generated: - when a `void`-returning method is intercepted and also decorated - when an interceptor declared on a target class returns `void` --- .../arc/processor/SubclassGenerator.java | 8 +- .../decorators/VoidMethodDecoratorTest.java | 69 ++++++++++++ ...VoidMethodInterceptorAndDecoratorTest.java | 101 ++++++++++++++++++ 3 files changed, 174 insertions(+), 4 deletions(-) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/VoidMethodDecoratorTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/interceptor/VoidMethodInterceptorAndDecoratorTest.java diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/SubclassGenerator.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/SubclassGenerator.java index d5b558580a9ff..1574f72bc58be 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/SubclassGenerator.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/SubclassGenerator.java @@ -341,7 +341,7 @@ public String apply(Set bindings) { applicationClassPredicate.test(bean.getBeanClass()), funBytecode.getMethodParam(1), funBytecode.getMethodParam(0)); - funBytecode.returnValue(ret); + funBytecode.returnValue(ret != null ? ret : funBytecode.loadNull()); constructor.invokeInterfaceMethod(MethodDescriptors.LIST_ADD, methodsList, fun.getInstance()); } constructor.writeInstanceField(field.getFieldDescriptor(), constructor.getThis(), methodsList); @@ -461,9 +461,9 @@ public String apply(Set bindings) { MethodDescriptor virtualMethodDescriptor = MethodDescriptor.ofMethod(declaringClass, originalMethodDescriptor.getName(), decoratorMethodDescriptor.getReturnType(), decoratorMethodDescriptor.getParameterTypes()); - funcBytecode - .returnValue(funcBytecode.invokeVirtualMethod(virtualMethodDescriptor, funDecoratorInstance, - superParamHandles)); + ResultHandle superResult = funcBytecode.invokeVirtualMethod(virtualMethodDescriptor, funDecoratorInstance, + superParamHandles); + funcBytecode.returnValue(superResult != null ? superResult : funcBytecode.loadNull()); } else { ResultHandle superResult = funcBytecode.invokeVirtualMethod(forwardDescriptor, targetHandle, superParamHandles); diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/VoidMethodDecoratorTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/VoidMethodDecoratorTest.java new file mode 100644 index 0000000000000..f39ed5510e120 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/VoidMethodDecoratorTest.java @@ -0,0 +1,69 @@ +package io.quarkus.arc.test.decorators; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.concurrent.atomic.AtomicBoolean; + +import jakarta.annotation.Priority; +import jakarta.decorator.Decorator; +import jakarta.decorator.Delegate; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.context.Dependent; +import jakarta.inject.Inject; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.test.ArcTestContainer; + +public class VoidMethodDecoratorTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(Performer.class, MainPerformer.class, + PerformerDecorator.class); + + @Test + public void testDecoration() { + MainPerformer performer = Arc.container().instance(MainPerformer.class).get(); + + assertFalse(MainPerformer.DONE.get()); + assertFalse(PerformerDecorator.DONE.get()); + + performer.doSomething(); + + assertTrue(MainPerformer.DONE.get()); + assertTrue(PerformerDecorator.DONE.get()); + } + + interface Performer { + void doSomething(); + } + + @ApplicationScoped + static class MainPerformer implements Performer { + static final AtomicBoolean DONE = new AtomicBoolean(); + + @Override + public void doSomething() { + DONE.set(true); + } + } + + @Dependent + @Priority(1) + @Decorator + static class PerformerDecorator implements Performer { + static final AtomicBoolean DONE = new AtomicBoolean(); + + @Inject + @Delegate + Performer delegate; + + @Override + public void doSomething() { + DONE.set(true); + delegate.doSomething(); + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/interceptor/VoidMethodInterceptorAndDecoratorTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/interceptor/VoidMethodInterceptorAndDecoratorTest.java new file mode 100644 index 0000000000000..b17de441bd387 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/interceptor/VoidMethodInterceptorAndDecoratorTest.java @@ -0,0 +1,101 @@ +package io.quarkus.arc.test.decorators.interceptor; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.util.concurrent.atomic.AtomicBoolean; + +import jakarta.annotation.Priority; +import jakarta.decorator.Decorator; +import jakarta.decorator.Delegate; +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.context.Dependent; +import jakarta.inject.Inject; +import jakarta.interceptor.AroundInvoke; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.arc.Arc; +import io.quarkus.arc.test.ArcTestContainer; + +public class VoidMethodInterceptorAndDecoratorTest { + @RegisterExtension + public ArcTestContainer container = new ArcTestContainer(Performer.class, MainPerformer.class, + PerformerDecorator.class, MyInterceptorBinding.class, MyInterceptor.class); + + @Test + public void testDecoration() { + MainPerformer performer = Arc.container().instance(MainPerformer.class).get(); + + assertFalse(MainPerformer.DONE.get()); + assertFalse(PerformerDecorator.DONE.get()); + assertFalse(MyInterceptor.INTERCEPTED.get()); + + performer.doSomething(); + + assertTrue(MainPerformer.DONE.get()); + assertTrue(PerformerDecorator.DONE.get()); + assertTrue(MyInterceptor.INTERCEPTED.get()); + } + + interface Performer { + void doSomething(); + } + + @ApplicationScoped + @MyInterceptorBinding + static class MainPerformer implements Performer { + static final AtomicBoolean DONE = new AtomicBoolean(); + + @Override + public void doSomething() { + DONE.set(true); + } + } + + @Dependent + @Priority(1) + @Decorator + static class PerformerDecorator implements Performer { + static final AtomicBoolean DONE = new AtomicBoolean(); + + @Inject + @Delegate + Performer delegate; + + @Override + public void doSomething() { + DONE.set(true); + delegate.doSomething(); + } + } + + @Target({ ElementType.TYPE, ElementType.METHOD }) + @Retention(RetentionPolicy.RUNTIME) + @Documented + @InterceptorBinding + public @interface MyInterceptorBinding { + } + + @MyInterceptorBinding + @Priority(1) + @Interceptor + static class MyInterceptor { + static final AtomicBoolean INTERCEPTED = new AtomicBoolean(); + + @AroundInvoke + Object log(InvocationContext ctx) throws Exception { + INTERCEPTED.set(true); + return ctx.proceed(); + } + } +}