Skip to content

Commit b186a87

Browse files
manovotnmanovotn
authored
Merge pull request #50452 from jonasrutishauser/arc-fix-subclass-generator-with-not-visible-decorators
ArC: fix subclass generation with not visible decorators
2 parents abdbece + 6e15235 commit b186a87

File tree

9 files changed

+254
-38
lines changed

9 files changed

+254
-38
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
public class DecoratorInfo extends BeanInfo implements Comparable<DecoratorInfo> {
1818

1919
private final InjectionPointInfo delegateInjectionPoint;
20-
private final Set<Type> decoratedTypes;
20+
private final Set<Type> decoratedTypes; // interfaces only
2121

2222
DecoratorInfo(AnnotationTarget target, BeanDeployment beanDeployment, InjectionPointInfo delegateInjectionPoint,
2323
Set<Type> decoratedTypes, List<Injection> injections, int priority) {

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

Lines changed: 14 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,6 @@ protected FieldDescriptor createConstructor(ClassOutput classOutput, BeanInfo be
318318
}
319319

320320
MethodDescriptor methodDescriptor = MethodDescriptor.of(method);
321-
MethodDescriptor originalMethodDescriptor = MethodDescriptor.of(method);
322321
InterceptionInfo interception = bean.getInterceptedMethods().get(method);
323322
DecorationInfo decoration = bean.getDecoratedMethods().get(method);
324323
MethodDescriptor forwardDescriptor = forwardingMethods.get(methodDescriptor);
@@ -392,20 +391,10 @@ protected FieldDescriptor createConstructor(ClassOutput classOutput, BeanInfo be
392391
if (decoratorMethod != null) {
393392
AssignableResultHandle funDecoratorInstance = funcBytecode.createVariable(Object.class);
394393
funcBytecode.assign(funDecoratorInstance, decoratorHandle);
395-
String declaringClass = decoratorMethod.decorator.getBeanClass().toString();
396-
if (decoratorMethod.decorator.isAbstract()) {
397-
String baseName = DecoratorGenerator
398-
.createBaseName(decoratorMethod.decorator.getTarget().get().asClass());
399-
String targetPackage = DotNames.packageName(decoratorMethod.decorator.getProviderType().name());
400-
declaringClass = generatedNameFromTarget(targetPackage, baseName,
401-
DecoratorGenerator.ABSTRACT_IMPL_SUFFIX);
402-
}
403-
// We need to use the decorator method in order to support generic decorators
394+
// We need to use the decorator method in order to support not visible or generic decorators
404395
MethodDescriptor decoratorMethodDescriptor = MethodDescriptor.of(decoratorMethod.method);
405-
MethodDescriptor virtualMethodDescriptor = MethodDescriptor.ofMethod(declaringClass,
406-
originalMethodDescriptor.getName(),
407-
decoratorMethodDescriptor.getReturnType(), decoratorMethodDescriptor.getParameterTypes());
408-
ResultHandle superResult = funcBytecode.invokeVirtualMethod(virtualMethodDescriptor, funDecoratorInstance,
396+
ResultHandle superResult = funcBytecode.invokeInterfaceMethod(decoratorMethodDescriptor,
397+
funDecoratorInstance,
409398
superParamHandles);
410399
funcBytecode.returnValue(superResult != null ? superResult : funcBytecode.loadNull());
411400
} else {
@@ -474,19 +463,11 @@ protected FieldDescriptor createConstructor(ClassOutput classOutput, BeanInfo be
474463
ResultHandle decoratorInstance = decoratedMethod.readInstanceField(FieldDescriptor.of(subclass.getClassName(),
475464
firstDecorator.getIdentifier(), Object.class.getName()), decoratedMethod.getThis());
476465

477-
String declaringClass = firstDecorator.getBeanClass().toString();
478-
if (firstDecorator.isAbstract()) {
479-
String baseName = DecoratorGenerator.createBaseName(firstDecorator.getTarget().get().asClass());
480-
String targetPackage = DotNames.packageName(firstDecorator.getProviderType().name());
481-
declaringClass = generatedNameFromTarget(targetPackage, baseName, DecoratorGenerator.ABSTRACT_IMPL_SUFFIX);
482-
}
483-
// We need to use the decorator method in order to support generic decorators
466+
// We need to use the decorator method in order to support not visible or generic decorators
484467
MethodDescriptor decoratorMethodDescriptor = MethodDescriptor.of(decoratorMethod.method);
485-
MethodDescriptor virtualMethodDescriptor = MethodDescriptor.ofMethod(
486-
declaringClass, methodDescriptor.getName(),
487-
decoratorMethodDescriptor.getReturnType(), decoratorMethodDescriptor.getParameterTypes());
488468
decoratedMethod
489-
.returnValue(decoratedMethod.invokeVirtualMethod(virtualMethodDescriptor, decoratorInstance, params));
469+
.returnValue(
470+
decoratedMethod.invokeInterfaceMethod(decoratorMethodDescriptor, decoratorInstance, params));
490471
}
491472
}
492473

@@ -610,7 +591,7 @@ private void processDecorator(DecoratorInfo decorator, BeanInfo bean, Type provi
610591
int paramIndex, Map<String, ResultHandle> decoratorToResultHandle,
611592
ResultHandle creationalContextHandle, Map<MethodDescriptor, MethodDescriptor> forwardingMethods) {
612593

613-
// First genetare the delegate subclass
594+
// First generate the delegate subclass
614595
// An instance of this subclass is injected in the delegate injection point of a decorator instance
615596
ClassInfo decoratorClass = decorator.getTarget().get().asClass();
616597
String baseName;
@@ -656,10 +637,14 @@ private void processDecorator(DecoratorInfo decorator, BeanInfo bean, Type provi
656637
constructorParameterTypes.add(subclass.getClassName());
657638
Map<DecoratorInfo, FieldDescriptor> nextDecoratorToField = new HashMap<>();
658639
for (DecoratorInfo nextDecorator : decoratorParameters) {
640+
// this can be always of type `Object`, because decorated types are always interfaces
641+
// and their methods are always invoked via `invokeinterface` (see `SubclassGenerator`)
642+
// and the JVM verifier doesn't care about the receiver type of interface method calls
643+
// (see e.g. https://wiki.openjdk.org/display/HotSpot/InterfaceCalls)
659644
FieldCreator nextDecoratorField = delegateSubclass
660-
.getFieldCreator(nextDecorator.getIdentifier(), nextDecorator.getBeanClass().toString())
645+
.getFieldCreator(nextDecorator.getIdentifier(), Object.class)
661646
.setModifiers(ACC_PRIVATE | ACC_FINAL);
662-
constructorParameterTypes.add(nextDecorator.getBeanClass().toString());
647+
constructorParameterTypes.add(Object.class.getName());
663648
nextDecoratorToField.put(nextDecorator, nextDecoratorField.getFieldDescriptor());
664649
}
665650

@@ -764,15 +749,7 @@ && isDecorated(decoratedMethodDescriptors, methodDescriptor, resolvedMethodDescr
764749
// This method is decorated by this decorator and there is a next decorator in the chain
765750
// Just delegate to the next decorator
766751
delegateTo = forward.readInstanceField(nextDecoratorToField.get(nextDecorator.decorator), forward.getThis());
767-
if (delegateTypeIsInterface) {
768-
ret = forward.invokeInterfaceMethod(methodDescriptor, delegateTo, params);
769-
} else {
770-
MethodDescriptor virtualMethod = MethodDescriptor.ofMethod(providerTypeName,
771-
methodDescriptor.getName(),
772-
nextDecorator.method.returnType(),
773-
nextDecorator.method.parameterTypes());
774-
ret = forward.invokeVirtualMethod(virtualMethod, delegateTo, params);
775-
}
752+
ret = forward.invokeInterfaceMethod(MethodDescriptor.of(nextDecorator.method), delegateTo, params);
776753

777754
} else {
778755
// This method is not decorated or no next decorator was found in the chain

‎independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/NonPublicDecoratorTest.java‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
package io.quarkus.arc.test.decorators;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertTrue;
5+
6+
import java.util.concurrent.atomic.AtomicBoolean;
7+
8+
import jakarta.annotation.PostConstruct;
9+
import jakarta.annotation.PreDestroy;
10+
import jakarta.annotation.Priority;
11+
import jakarta.decorator.Decorator;
12+
import jakarta.decorator.Delegate;
13+
import jakarta.enterprise.context.Dependent;
14+
import jakarta.inject.Inject;
15+
16+
import org.junit.jupiter.api.Test;
17+
import org.junit.jupiter.api.extension.RegisterExtension;
18+
19+
import io.quarkus.arc.Arc;
20+
import io.quarkus.arc.InstanceHandle;
21+
import io.quarkus.arc.test.ArcTestContainer;
22+
import io.quarkus.arc.test.decorators.other.Converter;
23+
import io.quarkus.arc.test.decorators.other.ToUpperCaseConverter;
24+
25+
public class NonPublicDecoratorTest {
26+
27+
@RegisterExtension
28+
public ArcTestContainer container = new ArcTestContainer(Converter.class, ToUpperCaseConverter.class,
29+
TrimConverterDecorator.class, AdditionalConverterDecorator.class, LastConverterDecorator.class);
30+
31+
@Test
32+
public void testDecoration() {
33+
InstanceHandle<ToUpperCaseConverter> handle = Arc.container().instance(ToUpperCaseConverter.class);
34+
ToUpperCaseConverter converter = handle.get();
35+
assertEquals("HOLA! test", converter.convert(" holA!"));
36+
assertEquals(" HOLA!", converter.convertNoDelegation(" holA!"));
37+
handle.destroy();
38+
assertTrue(TrimConverterDecorator.CONSTRUCTED.get());
39+
assertTrue(TrimConverterDecorator.DESTROYED.get());
40+
}
41+
42+
@Dependent
43+
@Priority(1)
44+
@Decorator
45+
static class TrimConverterDecorator implements Converter<String> {
46+
47+
static final AtomicBoolean CONSTRUCTED = new AtomicBoolean();
48+
static final AtomicBoolean DESTROYED = new AtomicBoolean();
49+
50+
@Inject
51+
@Delegate
52+
Converter<String> delegate;
53+
54+
@Override
55+
public String convert(String value) {
56+
return delegate.convert(value.trim());
57+
}
58+
59+
@PostConstruct
60+
void init() {
61+
CONSTRUCTED.set(true);
62+
}
63+
64+
@PreDestroy
65+
void destroy() {
66+
DESTROYED.set(true);
67+
}
68+
69+
}
70+
71+
@Priority(2)
72+
@Decorator
73+
static class AdditionalConverterDecorator implements Converter<String> {
74+
75+
@Inject
76+
@Delegate
77+
ToUpperCaseConverter delegate;
78+
79+
@Override
80+
public String convert(String value) {
81+
return delegate.convert(value + " ");
82+
}
83+
84+
}
85+
86+
@Priority(3)
87+
@Decorator
88+
static class LastConverterDecorator implements Converter<String> {
89+
90+
@Inject
91+
@Delegate
92+
Converter<String> delegate;
93+
94+
@Override
95+
public String convert(String value) {
96+
return delegate.convert(value) + "test";
97+
}
98+
99+
}
100+
101+
}

‎independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/interceptor/InterceptorAndNonPublicDecoratorTest.java‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
package io.quarkus.arc.test.decorators.interceptor;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
5+
import java.util.concurrent.atomic.AtomicReference;
6+
7+
import jakarta.annotation.Priority;
8+
import jakarta.decorator.Decorator;
9+
import jakarta.decorator.Delegate;
10+
import jakarta.inject.Inject;
11+
import jakarta.interceptor.AroundInvoke;
12+
import jakarta.interceptor.Interceptor;
13+
import jakarta.interceptor.InvocationContext;
14+
15+
import org.junit.jupiter.api.Test;
16+
import org.junit.jupiter.api.extension.RegisterExtension;
17+
18+
import io.quarkus.arc.Arc;
19+
import io.quarkus.arc.test.ArcTestContainer;
20+
import io.quarkus.arc.test.decorators.interceptor.other.Converter;
21+
import io.quarkus.arc.test.decorators.interceptor.other.Logging;
22+
import io.quarkus.arc.test.decorators.interceptor.other.ToUpperCaseConverter;
23+
24+
public class InterceptorAndNonPublicDecoratorTest {
25+
26+
@RegisterExtension
27+
public ArcTestContainer container = new ArcTestContainer(Converter.class, ToUpperCaseConverter.class,
28+
TrimConverterDecorator.class, LoggingInterceptor.class, Logging.class);
29+
30+
@Test
31+
public void testInterceptionAndDecoration() {
32+
LoggingInterceptor.LOG.set(null);
33+
ToUpperCaseConverter converter = Arc.container().instance(ToUpperCaseConverter.class).get();
34+
assertEquals("HOLA!", converter.convert(" holA!"));
35+
assertEquals("HOLA!", LoggingInterceptor.LOG.get());
36+
assertEquals(" HOLA!", converter.convertNoDelegation(" holA!"));
37+
assertEquals(" HOLA!", LoggingInterceptor.LOG.get());
38+
}
39+
40+
@Priority(1)
41+
@Decorator
42+
static class TrimConverterDecorator implements Converter<String> {
43+
44+
@Inject
45+
@Delegate
46+
Converter<String> delegate;
47+
48+
@Override
49+
public String convert(String value) {
50+
return delegate.convert(value.trim());
51+
}
52+
53+
}
54+
55+
@Logging
56+
@Priority(10)
57+
@Interceptor
58+
static class LoggingInterceptor {
59+
60+
static final AtomicReference<Object> LOG = new AtomicReference<Object>();
61+
62+
@AroundInvoke
63+
Object log(InvocationContext ctx) throws Exception {
64+
Object ret = ctx.proceed();
65+
LOG.set(ret);
66+
return ret;
67+
}
68+
}
69+
70+
}

‎independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/interceptor/other/Converter.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.decorators.interceptor.other;
2+
3+
public interface Converter<T> {
4+
5+
T convert(T value);
6+
7+
}

‎independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/interceptor/other/Logging.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
package io.quarkus.arc.test.decorators.interceptor.other;
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+
7+
import java.lang.annotation.Documented;
8+
import java.lang.annotation.Retention;
9+
import java.lang.annotation.Target;
10+
11+
import jakarta.interceptor.InterceptorBinding;
12+
13+
@Target({ TYPE, METHOD })
14+
@Retention(RUNTIME)
15+
@Documented
16+
@InterceptorBinding
17+
public @interface Logging {
18+
19+
}

‎independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/interceptor/other/ToUpperCaseConverter.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package io.quarkus.arc.test.decorators.interceptor.other;
2+
3+
import jakarta.enterprise.context.ApplicationScoped;
4+
5+
@Logging
6+
@ApplicationScoped
7+
public class ToUpperCaseConverter implements Converter<String> {
8+
9+
@Override
10+
public String convert(String value) {
11+
return value.toUpperCase();
12+
}
13+
14+
public String convertNoDelegation(String value) {
15+
return value.toUpperCase();
16+
}
17+
18+
}

‎independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/other/Converter.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.decorators.other;
2+
3+
public interface Converter<T> {
4+
5+
T convert(T value);
6+
7+
}

‎independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/decorators/other/ToUpperCaseConverter.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package io.quarkus.arc.test.decorators.other;
2+
3+
import jakarta.enterprise.context.ApplicationScoped;
4+
5+
@ApplicationScoped
6+
public class ToUpperCaseConverter implements Converter<String> {
7+
8+
@Override
9+
public String convert(String value) {
10+
return value.toUpperCase();
11+
}
12+
13+
public String convertNoDelegation(String value) {
14+
return value.toUpperCase();
15+
}
16+
17+
}

0 commit comments

Comments
 (0)