Skip to content

Commit 01156c3

Browse files
LadicekLadicek
committed
ArC: skip intercepting method calls during bean injection
Previously, method interception was skipped only during bean instance creation. With this commit, method interception is skipped during the entire injection process (that is, during bean constructor invocation and initializer methods invocations). This is the easiest way to prevent interception of initializer methods called by the container, as demanded by the CDI specification. It also makes sure the following provision of the Interceptors specification is honored: > With the exception of `@AroundConstruct` lifecycle callback interceptor > methods, no interceptor methods are invoked until after dependency > injection has been completed on both the interceptor instances and > the target.
1 parent 2b32749 commit 01156c3

File tree

6 files changed

+140
-18
lines changed

6 files changed

+140
-18
lines changed

‎independent-projects/arc/cdi-tck-runner/src/test/resources/testng.xml‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -145,15 +145,6 @@
145145
<exclude name="testExceptions"/>
146146
</methods>
147147
</class>
148-
<class name="org.jboss.cdi.tck.tests.interceptors.invocation.InterceptorInvocationTest">
149-
<methods>
150-
<exclude name="testDisposerMethodsAreIntercepted"/>
151-
<exclude name="testInitializerMethodsNotIntercepted"/>
152-
<exclude name="testProducerMethodsAreIntercepted"/>
153-
<exclude name="testLifecycleCallbacksAreIntercepted"/>
154-
<exclude name="testObjectMethodsAreNotIntercepted"/>
155-
</methods>
156-
</class>
157148
<class name="org.jboss.cdi.tck.interceptors.tests.bindings.broken.InvalidTransitiveInterceptorBindingAnnotationsTest">
158149
<methods>
159150
<exclude name="testInterceptorBindingsWithConflictingAnnotationMembersNotOk"/>

‎independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1734,6 +1734,22 @@ void implementCreateForClassBean(ClassOutput classOutput, ClassCreator beanCreat
17341734
}
17351735
}
17361736

1737+
if (bean.isSubclassRequired()) {
1738+
// marking the *_Subclass instance as constructed not when its constructor finishes,
1739+
// but only after injection is complete, to satisfy the Interceptors specification:
1740+
//
1741+
// > With the exception of `@AroundConstruct` lifecycle callback interceptor methods,
1742+
// > no interceptor methods are invoked until after dependency injection has been completed
1743+
// > on both the interceptor instances and the target.
1744+
//
1745+
// and also the CDI specification:
1746+
//
1747+
// > Invocations of initializer methods by the container are not business method invocations.
1748+
String subclassName = SubclassGenerator.generatedName(bean.getProviderType().name(), baseName);
1749+
create.invokeVirtualMethod(MethodDescriptor.ofMethod(subclassName,
1750+
SubclassGenerator.MARK_CONSTRUCTED_METHOD_NAME, void.class), instanceHandle);
1751+
}
1752+
17371753
// PostConstruct lifecycle callback interceptors
17381754
InterceptionInfo postConstructs = bean.getLifecycleInterceptors(InterceptionType.POST_CONSTRUCT);
17391755
if (!postConstructs.isEmpty()) {
@@ -1785,6 +1801,7 @@ void implementCreateForClassBean(ClassOutput classOutput, ClassCreator beanCreat
17851801
}
17861802
}
17871803
}
1804+
17881805
create.returnValue(instanceHandle);
17891806
}
17901807

‎independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/SubclassGenerator.java‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import static io.quarkus.arc.processor.IndexClassLookupUtils.getClassByName;
44
import static org.objectweb.asm.Opcodes.ACC_FINAL;
55
import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
6+
import static org.objectweb.asm.Opcodes.ACC_VOLATILE;
67

78
import java.lang.reflect.Modifier;
89
import java.util.ArrayList;
@@ -69,6 +70,7 @@ public class SubclassGenerator extends AbstractGenerator {
6970
private static final DotName JAVA_LANG_RUNTIME_EXCEPTION = DotNames.create(RuntimeException.class.getName());
7071

7172
static final String SUBCLASS_SUFFIX = "_Subclass";
73+
static final String MARK_CONSTRUCTED_METHOD_NAME = "arc$markConstructed";
7274
static final String DESTROY_METHOD_NAME = "arc$destroy";
7375

7476
protected static final String FIELD_NAME_PREDESTROYS = "arc$preDestroys";
@@ -179,10 +181,9 @@ protected FieldDescriptor createConstructor(ClassOutput classOutput, BeanInfo be
179181

180182
Map<MethodDescriptor, MethodDescriptor> forwardingMethods = new HashMap<>();
181183
List<MethodInfo> interceptedOrDecoratedMethods = bean.getInterceptedOrDecoratedMethods();
182-
int methodIdx = 1;
183184
for (MethodInfo method : interceptedOrDecoratedMethods) {
184185
forwardingMethods.put(MethodDescriptor.of(method),
185-
createForwardingMethod(subclass, providerTypeName, method, methodIdx));
186+
createForwardingMethod(subclass, providerTypeName, method));
186187
}
187188

188189
// If a decorator is associated:
@@ -225,8 +226,15 @@ protected FieldDescriptor createConstructor(ClassOutput classOutput, BeanInfo be
225226
}
226227
}
227228

229+
// `volatile` is perhaps not best, this field is monotonic (once `true`, it never becomes `false` again),
230+
// so maybe making the `markConstructed` method `synchronized` would be enough (?)
228231
FieldCreator constructedField = subclass.getFieldCreator(FIELD_NAME_CONSTRUCTED, boolean.class)
229-
.setModifiers(ACC_PRIVATE | ACC_FINAL);
232+
.setModifiers(ACC_PRIVATE | ACC_VOLATILE);
233+
234+
MethodCreator markConstructed = subclass.getMethodCreator(MARK_CONSTRUCTED_METHOD_NAME, void.class);
235+
markConstructed.writeInstanceField(constructedField.getFieldDescriptor(), markConstructed.getThis(),
236+
markConstructed.load(true));
237+
markConstructed.returnVoid();
230238

231239
// Initialize maps of shared interceptor chains and interceptor bindings
232240
IntegerHolder chainIdx = new IntegerHolder();
@@ -302,7 +310,7 @@ public String apply(List<BindingKey> keys) {
302310
}
303311
};
304312

305-
methodIdx = 1;
313+
int methodIdx = 1;
306314
for (MethodInfo method : interceptedOrDecoratedMethods) {
307315
InterceptionInfo interception = bean.getInterceptedMethods().get(method);
308316
if (interception != null) {
@@ -498,8 +506,6 @@ public String apply(List<BindingKey> keys) {
498506
interceptorChainMap, bindingsMap);
499507
}
500508

501-
constructor.writeInstanceField(constructedField.getFieldDescriptor(), constructor.getThis(), constructor.load(true));
502-
503509
constructor.returnValue(null);
504510
return preDestroysField != null ? preDestroysField.getFieldDescriptor() : null;
505511
}
@@ -758,10 +764,9 @@ private boolean isDecorated(Set<MethodDescriptor> decoratedMethodDescriptors, Me
758764
return false;
759765
}
760766

761-
private MethodDescriptor createForwardingMethod(ClassCreator subclass, String providerTypeName, MethodInfo method,
762-
int index) {
767+
private MethodDescriptor createForwardingMethod(ClassCreator subclass, String providerTypeName, MethodInfo method) {
763768
MethodDescriptor methodDescriptor = MethodDescriptor.of(method);
764-
String forwardMethodName = method.name() + "$$superforward" + index;
769+
String forwardMethodName = method.name() + "$$superforward";
765770
MethodDescriptor forwardDescriptor = MethodDescriptor.ofMethod(subclass.getClassName(), forwardMethodName,
766771
methodDescriptor.getReturnType(),
767772
methodDescriptor.getParameterTypes());

‎independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/initializer/InitializerMethodInterceptionTest.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package io.quarkus.arc.test.interceptors.initializer;
2+
3+
import static java.lang.annotation.ElementType.METHOD;
4+
import static java.lang.annotation.ElementType.TYPE;
5+
import static java.lang.annotation.RetentionPolicy.RUNTIME;
6+
import static org.junit.jupiter.api.Assertions.assertFalse;
7+
import static org.junit.jupiter.api.Assertions.assertNotNull;
8+
import static org.junit.jupiter.api.Assertions.assertTrue;
9+
10+
import java.lang.annotation.Documented;
11+
import java.lang.annotation.Retention;
12+
import java.lang.annotation.Target;
13+
14+
import jakarta.annotation.Priority;
15+
import jakarta.inject.Singleton;
16+
import jakarta.interceptor.AroundInvoke;
17+
import jakarta.interceptor.Interceptor;
18+
import jakarta.interceptor.InterceptorBinding;
19+
import jakarta.interceptor.InvocationContext;
20+
21+
import org.junit.jupiter.api.Test;
22+
import org.junit.jupiter.api.extension.RegisterExtension;
23+
24+
import io.quarkus.arc.Arc;
25+
import io.quarkus.arc.test.ArcTestContainer;
26+
import io.quarkus.arc.test.interceptors.initializer.subpkg.MyDependency;
27+
import io.quarkus.arc.test.interceptors.initializer.subpkg.MySuperclass;
28+
29+
public class InitializerMethodInterceptionTest {
30+
@RegisterExtension
31+
public ArcTestContainer container = new ArcTestContainer(MyDependency.class, MyBean.class, MyInterceptorBinding.class,
32+
MyInterceptor.class);
33+
34+
@Test
35+
public void testInterception() {
36+
MyBean bean = Arc.container().instance(MyBean.class).get();
37+
assertNotNull(bean);
38+
assertTrue(bean.publicInitializerCalled);
39+
assertTrue(bean.protectedInitializerCalled);
40+
assertTrue(bean.packagePrivateInitializerCalled);
41+
assertTrue(bean.privateInitializerCalled);
42+
43+
// initializer methods invoked by the container are not intercepted
44+
assertFalse(MyInterceptor.intercepted);
45+
}
46+
47+
// note that `MySuperclass` is intentionally in a different package
48+
@Singleton
49+
@MyInterceptorBinding
50+
static class MyBean extends MySuperclass {
51+
}
52+
53+
@Target({ TYPE, METHOD })
54+
@Retention(RUNTIME)
55+
@Documented
56+
@InterceptorBinding
57+
@interface MyInterceptorBinding {
58+
}
59+
60+
@MyInterceptorBinding
61+
@Interceptor
62+
@Priority(1)
63+
static class MyInterceptor {
64+
static boolean intercepted = false;
65+
66+
@AroundInvoke
67+
Object intercept(InvocationContext ctx) throws Exception {
68+
intercepted = true;
69+
return ctx.proceed();
70+
}
71+
}
72+
}

‎independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/initializer/subpkg/MyDependency.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package io.quarkus.arc.test.interceptors.initializer.subpkg;
2+
3+
import jakarta.enterprise.context.Dependent;
4+
5+
@Dependent
6+
public class MyDependency {
7+
}

‎independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/initializer/subpkg/MySuperclass.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package io.quarkus.arc.test.interceptors.initializer.subpkg;
2+
3+
import jakarta.inject.Inject;
4+
5+
public class MySuperclass {
6+
public boolean publicInitializerCalled;
7+
public boolean protectedInitializerCalled;
8+
public boolean packagePrivateInitializerCalled;
9+
public boolean privateInitializerCalled;
10+
11+
@Inject
12+
public void publicInject(MyDependency ignored) {
13+
publicInitializerCalled = true;
14+
}
15+
16+
@Inject
17+
protected void protectedInject(MyDependency ignored) {
18+
protectedInitializerCalled = true;
19+
}
20+
21+
@Inject
22+
void packagePrivateInject(MyDependency ignored) {
23+
packagePrivateInitializerCalled = true;
24+
}
25+
26+
@Inject
27+
private void privateInject(MyDependency ignored) {
28+
privateInitializerCalled = true;
29+
}
30+
}

0 commit comments

Comments
 (0)