Skip to content

Commit 3da3ff8

Browse files
geoandgeoand
committed
Make placing JPA listener annotations a little safer
This essentially handles the case where such a listener is added on private method of a CDI EntityListener class. When the listener is a @singleton we can safely transform the private method to package private, but when it's @ApplicationScoped there is nothing we can do (as any transformation we apply is invisible to Arc when it creates the Client Proxy class), so we therefore fail the build (with an actionable error message). Closes: quarkusio#24340
1 parent 6e9bd4c commit 3da3ff8

File tree

3 files changed

+276
-0
lines changed

3 files changed

+276
-0
lines changed

‎extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmCdiProcessor.java‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22

33
import static org.apache.commons.lang3.BooleanUtils.isFalse;
44

5+
import java.lang.reflect.Modifier;
56
import java.util.ArrayList;
67
import java.util.Arrays;
78
import java.util.List;
89
import java.util.Locale;
910
import java.util.Set;
11+
import java.util.function.BiFunction;
1012
import java.util.function.Function;
1113
import java.util.stream.Collectors;
1214

@@ -24,26 +26,35 @@
2426
import org.hibernate.query.criteria.HibernateCriteriaBuilder;
2527
import org.hibernate.relational.SchemaManager;
2628
import org.jboss.jandex.AnnotationInstance;
29+
import org.jboss.jandex.AnnotationTarget;
2730
import org.jboss.jandex.AnnotationTarget.Kind;
2831
import org.jboss.jandex.AnnotationTransformation;
2932
import org.jboss.jandex.AnnotationValue;
33+
import org.jboss.jandex.ClassInfo;
3034
import org.jboss.jandex.ClassType;
35+
import org.jboss.jandex.CompositeIndex;
3136
import org.jboss.jandex.DotName;
3237
import org.jboss.jandex.FieldInfo;
38+
import org.jboss.jandex.MethodInfo;
3339
import org.jboss.jandex.ParameterizedType;
3440
import org.jboss.jandex.Type;
41+
import org.objectweb.asm.ClassVisitor;
3542

3643
import io.agroal.api.AgroalDataSource;
3744
import io.quarkus.agroal.spi.JdbcDataSourceBuildItem;
3845
import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
3946
import io.quarkus.arc.deployment.AnnotationsTransformerBuildItem;
4047
import io.quarkus.arc.deployment.BeanDefiningAnnotationBuildItem;
48+
import io.quarkus.arc.deployment.BeanDiscoveryFinishedBuildItem;
4149
import io.quarkus.arc.deployment.SyntheticBeanBuildItem;
4250
import io.quarkus.arc.deployment.SyntheticBeanBuildItem.ExtendedBeanConfigurator;
4351
import io.quarkus.arc.deployment.UnremovableBeanBuildItem;
4452
import io.quarkus.arc.deployment.ValidationPhaseBuildItem;
4553
import io.quarkus.arc.processor.AnnotationsTransformer;
54+
import io.quarkus.arc.processor.BeanInfo;
55+
import io.quarkus.arc.processor.BuiltinScope;
4656
import io.quarkus.arc.processor.DotNames;
57+
import io.quarkus.arc.processor.ScopeInfo;
4758
import io.quarkus.arc.processor.Transformation;
4859
import io.quarkus.deployment.Capabilities;
4960
import io.quarkus.deployment.Capability;
@@ -52,7 +63,10 @@
5263
import io.quarkus.deployment.annotations.BuildSteps;
5364
import io.quarkus.deployment.annotations.ExecutionTime;
5465
import io.quarkus.deployment.annotations.Record;
66+
import io.quarkus.deployment.builditem.BytecodeTransformerBuildItem;
5567
import io.quarkus.deployment.builditem.CombinedIndexBuildItem;
68+
import io.quarkus.gizmo.ClassTransformer;
69+
import io.quarkus.gizmo.MethodDescriptor;
5670
import io.quarkus.hibernate.orm.PersistenceUnit;
5771
import io.quarkus.hibernate.orm.runtime.HibernateOrmRecorder;
5872
import io.quarkus.hibernate.orm.runtime.HibernateOrmRuntimeConfig;
@@ -276,6 +290,62 @@ void registerBeans(HibernateOrmConfig hibernateOrmConfig,
276290
unremovableBeans.produce(UnremovableBeanBuildItem.beanTypes(jpaModel.getPotentialCdiBeanClassNames()));
277291
}
278292

293+
@BuildStep
294+
void transformBeans(JpaModelBuildItem jpaModel, JpaModelIndexBuildItem indexBuildItem,
295+
BeanDiscoveryFinishedBuildItem beans,
296+
BuildProducer<BytecodeTransformerBuildItem> producer) {
297+
if (!HibernateOrmProcessor.hasEntities(jpaModel)) {
298+
return;
299+
}
300+
301+
// the idea here is to remove the 'private' modifier from all methods that are annotated with JPA Listener methods
302+
// and don't belong to entities
303+
CompositeIndex index = indexBuildItem.getIndex();
304+
for (DotName dotName : jpaModel.getPotentialCdiBeanClassNames()) {
305+
if (jpaModel.getManagedClassNames().contains(dotName.toString())) {
306+
continue;
307+
}
308+
ClassInfo classInfo = index.getClassByName(dotName);
309+
List<BeanInfo> matchingBeans = beans.getBeans().stream().filter(bi -> bi.getBeanClass().equals(dotName)).toList();
310+
if (matchingBeans.size() == 1) {
311+
ScopeInfo beanScope = matchingBeans.get(0).getScope();
312+
for (DotName jpaListenerDotName : ClassNames.JPA_LISTENER_ANNOTATIONS) {
313+
for (AnnotationInstance annotationInstance : classInfo.annotations(jpaListenerDotName)) {
314+
AnnotationTarget target = annotationInstance.target();
315+
if (target.kind() != AnnotationTarget.Kind.METHOD) {
316+
continue;
317+
}
318+
MethodInfo method = target.asMethod();
319+
if (Modifier.isPrivate(method.flags())) {
320+
if (beanScope.getDotName().equals(BuiltinScope.SINGLETON.getName())) {
321+
// we can safely transform in this case
322+
producer.produce(new BytecodeTransformerBuildItem(method.declaringClass().name().toString(),
323+
new BiFunction<>() {
324+
@Override
325+
public ClassVisitor apply(String cls, ClassVisitor clsVisitor) {
326+
var classTransformer = new ClassTransformer(cls);
327+
classTransformer.modifyMethod(MethodDescriptor.of(method))
328+
.removeModifiers(Modifier.PRIVATE);
329+
return classTransformer.applyTo(clsVisitor);
330+
}
331+
}));
332+
} else {
333+
// we can't transform because the client proxy does not know about the transformation and
334+
// will therefore simply copy the private method which will then likely fail because it does
335+
// not contain the injected fields
336+
throw new IllegalArgumentException(
337+
"Methods that are annotated with JPA Listener annotations should not be private. Offending method is '"
338+
+ method.declaringClass().name() + "#" + method.name() + "'");
339+
}
340+
}
341+
}
342+
}
343+
} else {
344+
// we don't really know what to do here, just bail and CDI will figure it out
345+
}
346+
}
347+
}
348+
279349
@BuildStep
280350
void registerAnnotations(BuildProducer<AdditionalBeanBuildItem> additionalBeans,
281351
BuildProducer<BeanDefiningAnnotationBuildItem> beanDefiningAnnotations) {

‎extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/JpaListenerOnPrivateMethodOfApplicationScopedCdiBeanTest.java‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package io.quarkus.hibernate.orm;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.assertj.core.api.Assertions.fail;
5+
6+
import jakarta.enterprise.context.ApplicationScoped;
7+
import jakarta.persistence.Entity;
8+
import jakarta.persistence.EntityListeners;
9+
import jakarta.persistence.GeneratedValue;
10+
import jakarta.persistence.GenerationType;
11+
import jakarta.persistence.Id;
12+
import jakarta.persistence.PostPersist;
13+
import jakarta.transaction.Transactional;
14+
15+
import org.junit.jupiter.api.Test;
16+
import org.junit.jupiter.api.extension.RegisterExtension;
17+
18+
import io.quarkus.test.QuarkusUnitTest;
19+
20+
public class JpaListenerOnPrivateMethodOfApplicationScopedCdiBeanTest {
21+
22+
@RegisterExtension
23+
static QuarkusUnitTest runner = new QuarkusUnitTest()
24+
.withApplicationRoot((jar) -> jar
25+
.addAsResource("application.properties"))
26+
.assertException(e -> {
27+
assertThat(e).isInstanceOf(IllegalArgumentException.class)
28+
.hasMessageContaining("SomeEntityListener#postPersist");
29+
});
30+
31+
@Test
32+
@Transactional
33+
public void test() {
34+
fail("should never be called");
35+
}
36+
37+
@Entity
38+
@EntityListeners(SomeEntityListener.class)
39+
public static class SomeEntity {
40+
private long id;
41+
private String name;
42+
43+
public SomeEntity() {
44+
}
45+
46+
public SomeEntity(String name) {
47+
this.name = name;
48+
}
49+
50+
@Id
51+
@GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "myEntitySeq")
52+
public long getId() {
53+
return id;
54+
}
55+
56+
public void setId(long id) {
57+
this.id = id;
58+
}
59+
60+
public String getName() {
61+
return name;
62+
}
63+
64+
public void setName(String name) {
65+
this.name = name;
66+
}
67+
68+
@Override
69+
public String toString() {
70+
return "SomeEntity:" + name;
71+
}
72+
}
73+
74+
@ApplicationScoped
75+
public static class SomeEntityListener {
76+
77+
@PostPersist
78+
private void postPersist(SomeEntity someEntity) {
79+
fail("should not reach here");
80+
}
81+
}
82+
}

‎extensions/hibernate-orm/deployment/src/test/java/io/quarkus/hibernate/orm/JpaListenerOnPrivateMethodOfSingletonCdiBeanTest.java‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
package io.quarkus.hibernate.orm;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
5+
import java.util.List;
6+
import java.util.concurrent.CopyOnWriteArrayList;
7+
8+
import jakarta.enterprise.context.ApplicationScoped;
9+
import jakarta.inject.Inject;
10+
import jakarta.inject.Singleton;
11+
import jakarta.persistence.Entity;
12+
import jakarta.persistence.EntityListeners;
13+
import jakarta.persistence.EntityManager;
14+
import jakarta.persistence.GeneratedValue;
15+
import jakarta.persistence.GenerationType;
16+
import jakarta.persistence.Id;
17+
import jakarta.persistence.PostPersist;
18+
import jakarta.persistence.PrePersist;
19+
import jakarta.transaction.Transactional;
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.test.QuarkusUnitTest;
26+
27+
public class JpaListenerOnPrivateMethodOfSingletonCdiBeanTest {
28+
29+
@RegisterExtension
30+
static QuarkusUnitTest runner = new QuarkusUnitTest()
31+
.withApplicationRoot((jar) -> jar
32+
.addAsResource("application.properties"));
33+
34+
@Inject
35+
EventStore eventStore;
36+
37+
@Inject
38+
EntityManager em;
39+
40+
@Test
41+
@Transactional
42+
public void test() {
43+
Arc.container().requestContext().activate();
44+
try {
45+
SomeEntity entity = new SomeEntity("test");
46+
em.persist(entity);
47+
em.flush();
48+
} finally {
49+
Arc.container().requestContext().terminate();
50+
}
51+
52+
assertThat(eventStore.getEvents()).containsExactly("prePersist", "postPersist");
53+
}
54+
55+
@Entity
56+
@EntityListeners(SomeEntityListener.class)
57+
public static class SomeEntity {
58+
private long id;
59+
private String name;
60+
61+
public SomeEntity() {
62+
}
63+
64+
public SomeEntity(String name) {
65+
this.name = name;
66+
}
67+
68+
@Id
69+
@GeneratedValue(strategy = GenerationType.SEQUENCE, generator = "myEntitySeq")
70+
public long getId() {
71+
return id;
72+
}
73+
74+
public void setId(long id) {
75+
this.id = id;
76+
}
77+
78+
public String getName() {
79+
return name;
80+
}
81+
82+
public void setName(String name) {
83+
this.name = name;
84+
}
85+
86+
@Override
87+
public String toString() {
88+
return "SomeEntity:" + name;
89+
}
90+
}
91+
92+
@Singleton
93+
public static class SomeEntityListener {
94+
95+
private final EventStore eventStore;
96+
97+
public SomeEntityListener(EventStore eventStore) {
98+
this.eventStore = eventStore;
99+
}
100+
101+
@PrePersist
102+
void prePersist(SomeEntity someEntity) {
103+
eventStore.addEvent("prePersist");
104+
}
105+
106+
@PostPersist
107+
private void postPersist(SomeEntity someEntity) {
108+
eventStore.addEvent("postPersist");
109+
}
110+
}
111+
112+
@ApplicationScoped
113+
public static class EventStore {
114+
private final List<String> events = new CopyOnWriteArrayList<>();
115+
116+
public void addEvent(String event) {
117+
events.add(event);
118+
}
119+
120+
public List<String> getEvents() {
121+
return events;
122+
}
123+
}
124+
}

0 commit comments

Comments
 (0)