Skip to content

Commit 7876d3d

Browse files
chadongminchadongmin
authored
Fix: Improve handling of contains() on JPA collections mapped with @Converter to basic types (#1199)
* Fix: Early error for .contains() on @converter mapped JPA collections * Test : add test code * Add test cases to improve test coverage * style: revert formatting change based on review feedback for consistency
1 parent 63b4a04 commit 7876d3d

File tree

5 files changed

+277
-17
lines changed

5 files changed

+277
-17
lines changed

‎querydsl-libraries/querydsl-jpa/src/main/java/com/querydsl/jpa/JPQLSerializer.java‎

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,28 +17,14 @@
1717
import com.querydsl.core.JoinType;
1818
import com.querydsl.core.QueryMetadata;
1919
import com.querydsl.core.support.SerializerBase;
20-
import com.querydsl.core.types.CollectionExpression;
21-
import com.querydsl.core.types.Constant;
22-
import com.querydsl.core.types.ConstantImpl;
23-
import com.querydsl.core.types.EntityPath;
24-
import com.querydsl.core.types.Expression;
25-
import com.querydsl.core.types.ExpressionUtils;
26-
import com.querydsl.core.types.FactoryExpression;
27-
import com.querydsl.core.types.MapExpression;
28-
import com.querydsl.core.types.Operation;
29-
import com.querydsl.core.types.Operator;
30-
import com.querydsl.core.types.Ops;
31-
import com.querydsl.core.types.Order;
32-
import com.querydsl.core.types.OrderSpecifier;
33-
import com.querydsl.core.types.Path;
34-
import com.querydsl.core.types.PathType;
35-
import com.querydsl.core.types.Predicate;
36-
import com.querydsl.core.types.SubQueryExpression;
20+
import com.querydsl.core.types.*;
3721
import com.querydsl.core.types.dsl.Expressions;
3822
import com.querydsl.core.util.MathUtils;
3923
import jakarta.persistence.Entity;
4024
import jakarta.persistence.EntityManager;
25+
import jakarta.persistence.metamodel.Attribute;
4126
import jakarta.persistence.metamodel.EntityType;
27+
import jakarta.persistence.metamodel.Metamodel;
4228
import jakarta.persistence.metamodel.SingularAttribute;
4329
import java.util.ArrayList;
4430
import java.util.Arrays;
@@ -596,8 +582,57 @@ private void visitPathInCollection(
596582
return null;
597583
}
598584

585+
private boolean isCollectionPathWithConverterToBasicType(Path<?> path, Metamodel metamodel) {
586+
587+
PathMetadata metadata = path.getMetadata();
588+
if (metadata.getPathType() != PathType.PROPERTY
589+
|| metadata.getParent() == null
590+
|| !metadata.getParent().getMetadata().isRoot()) {
591+
return false;
592+
}
593+
Class<?> owningEntityJavaType = metadata.getParent().getType();
594+
String attributeName = metadata.getName();
595+
try {
596+
EntityType<?> entityType = metamodel.entity(owningEntityJavaType);
597+
Attribute<?, ?> attribute = entityType.getAttribute(attributeName);
598+
boolean isJavaCollection =
599+
java.util.Collection.class.isAssignableFrom(attribute.getJavaType());
600+
boolean isPersistentBasic =
601+
attribute.getPersistentAttributeType() == Attribute.PersistentAttributeType.BASIC;
602+
return isJavaCollection && isPersistentBasic;
603+
} catch (IllegalArgumentException e) {
604+
System.err.println(
605+
"QueryDSL-JPA (DEBUG): Metamodel lookup failed for " + path + ": " + e.getMessage());
606+
return false;
607+
}
608+
}
609+
599610
private void visitAnyInPath(
600611
Class<?> type, Operator operator, List<? extends Expression<?>> args) {
612+
Expression<?> collectionExpression = args.get(1);
613+
614+
if (this.entityManager != null && collectionExpression instanceof Path<?>) {
615+
Path<?> collectionPath = (Path<?>) collectionExpression;
616+
jakarta.persistence.metamodel.Metamodel metamodel = this.entityManager.getMetamodel();
617+
if (isCollectionPathWithConverterToBasicType(collectionPath, metamodel)) {
618+
String effectiveOperatorForMessage =
619+
(operator == Ops.IN)
620+
? "MEMBER OF (translated from IN)"
621+
: "NOT MEMBER OF (translated from NOT IN)";
622+
throw new com.querydsl.core.QueryException(
623+
"QueryDSL Error: Path '"
624+
+ collectionPath
625+
+ "' is a Java collection mapped via a JPA @Converter to a basic database type. "
626+
+ "The QueryDSL operation '"
627+
+ operator.toString()
628+
+ "' on this path, which would typically translate to JPQL '"
629+
+ effectiveOperatorForMessage
630+
+ "', "
631+
+ "is not supported by JPA/Hibernate for such converted attributes as they are not treated as queryable 'plural paths'. "
632+
+ "Consider alternative query strategies (e.g., native queries with database-specific functions) "
633+
+ "or re-evaluate your entity mapping strategy.");
634+
}
635+
}
601636
super.visitOperation(
602637
type, operator == Ops.IN ? JPQLOps.MEMBER_OF : JPQLOps.NOT_MEMBER_OF, args);
603638
}

‎querydsl-libraries/querydsl-jpa/src/test/java/com/querydsl/jpa/JPAQueryConverterCollectionContainsTest.java‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
package com.querydsl.jpa;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
5+
6+
import com.querydsl.jpa.domain.Alchemist;
7+
import com.querydsl.jpa.domain.PotionEffect;
8+
import com.querydsl.jpa.domain.QAlchemist;
9+
import com.querydsl.jpa.impl.JPAQueryFactory;
10+
import com.querydsl.jpa.testutil.JPATestRunner;
11+
import jakarta.persistence.EntityManager;
12+
import java.util.Collections;
13+
import java.util.EnumSet;
14+
import java.util.List;
15+
import org.junit.After;
16+
import org.junit.Before;
17+
import org.junit.Test;
18+
import org.junit.runner.RunWith;
19+
20+
@RunWith(JPATestRunner.class)
21+
public class JPAQueryConverterCollectionContainsTest implements JPATest {
22+
23+
private EntityManager em;
24+
private JPAQueryFactory queryFactory;
25+
26+
@Override
27+
public void setEntityManager(EntityManager em) {
28+
this.em = em;
29+
if (this.em != null) {
30+
this.queryFactory = new JPAQueryFactory(this.em);
31+
}
32+
}
33+
34+
private static final QAlchemist qAlchemist = QAlchemist.alchemist;
35+
36+
@Before
37+
public void setUpData() {
38+
if (em == null) {
39+
throw new IllegalStateException("EntityManager has not been set by JPATestRunner.");
40+
}
41+
if (queryFactory == null) {
42+
queryFactory = new JPAQueryFactory(em);
43+
}
44+
45+
if (!em.getTransaction().isActive()) {
46+
em.getTransaction().begin();
47+
}
48+
49+
queryFactory.delete(qAlchemist).where(qAlchemist.alchemistName.isNotNull()).execute();
50+
51+
em.persist(
52+
new Alchemist(
53+
"Merlin",
54+
EnumSet.of(PotionEffect.INVISIBILITY, PotionEffect.STRENGTH),
55+
Collections.emptyList()));
56+
em.persist(new Alchemist("Circe", EnumSet.of(PotionEffect.HEALING), Collections.emptyList()));
57+
em.persist(
58+
new Alchemist(
59+
"Paracelsus",
60+
EnumSet.of(PotionEffect.STRENGTH, PotionEffect.SPEED),
61+
Collections.emptyList()));
62+
em.persist(
63+
new Alchemist("Zosimos", EnumSet.of(PotionEffect.INVISIBILITY), Collections.emptyList()));
64+
65+
em.flush();
66+
em.clear();
67+
}
68+
69+
@After
70+
public void tearDown() {
71+
if (em != null && em.getTransaction().isActive()) {
72+
em.getTransaction().rollback();
73+
}
74+
}
75+
76+
@Test
77+
public void knownEffectsContains_ShouldThrowQueryDSLException_WhenConverterIsUsed() {
78+
if (queryFactory == null) {
79+
throw new IllegalStateException("JPAQueryFactory not initialized.");
80+
}
81+
PotionEffect searchEffect = PotionEffect.INVISIBILITY;
82+
83+
assertThatThrownBy(
84+
() -> {
85+
queryFactory
86+
.selectFrom(qAlchemist)
87+
.where(qAlchemist.knownEffects.contains(searchEffect))
88+
.fetch();
89+
})
90+
.isInstanceOf(com.querydsl.core.QueryException.class)
91+
.hasMessageContaining(
92+
"QueryDSL Error: Path 'alchemist.knownEffects' is a Java collection mapped via a JPA @Converter to a basic database type.")
93+
.hasMessageContaining("The QueryDSL operation 'IN' on this path")
94+
.hasMessageContaining("is not supported by JPA/Hibernate for such converted attributes");
95+
}
96+
97+
@Test
98+
public void learnedSpellsContains_ShouldNOTThrowQueryDSLException_ForStandardCollection() {
99+
if (queryFactory == null) {
100+
throw new IllegalStateException("JPAQueryFactory not initialized.");
101+
}
102+
String searchSpell = "Fireball";
103+
104+
Alchemist gandalf =
105+
new Alchemist(
106+
"Gandalf", EnumSet.of(PotionEffect.STRENGTH), List.of("Fireball", "Lightning Bolt"));
107+
em.persist(gandalf);
108+
em.flush();
109+
110+
List<Alchemist> results =
111+
queryFactory
112+
.selectFrom(qAlchemist)
113+
.where(
114+
qAlchemist
115+
.learnedSpells
116+
.contains(searchSpell)
117+
.and(qAlchemist.alchemistName.eq("Gandalf")))
118+
.fetch();
119+
120+
assertThat(results).isNotNull();
121+
assertThat(results).hasSize(1);
122+
assertThat(results.get(0).getAlchemistName()).isEqualTo("Gandalf");
123+
}
124+
}

‎querydsl-libraries/querydsl-jpa/src/test/java/com/querydsl/jpa/domain/Alchemist.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package com.querydsl.jpa.domain;
2+
3+
import jakarta.persistence.*;
4+
import java.util.ArrayList;
5+
import java.util.List;
6+
import java.util.Set;
7+
8+
@Entity(name = "AlchemistEntity")
9+
@Table(name = "alchemist_effects_test")
10+
public class Alchemist {
11+
@Id
12+
@GeneratedValue(strategy = GenerationType.IDENTITY)
13+
private Integer id;
14+
15+
private String alchemistName;
16+
17+
@Convert(converter = PotionEffectSetConverter.class)
18+
@Column(columnDefinition = "TEXT")
19+
private Set<PotionEffect> knownEffects;
20+
21+
@ElementCollection
22+
@CollectionTable(name = "alchemist_spells", joinColumns = @JoinColumn(name = "alchemist_id"))
23+
@Column(name = "spell_name")
24+
private List<String> learnedSpells = new ArrayList<>();
25+
26+
public Alchemist() {}
27+
28+
public Alchemist(
29+
String alchemistName, Set<PotionEffect> knownEffects, List<String> learnedSpells) {
30+
this.alchemistName = alchemistName;
31+
this.knownEffects = knownEffects;
32+
this.learnedSpells = learnedSpells;
33+
}
34+
35+
public Integer getId() {
36+
return id;
37+
}
38+
39+
public String getAlchemistName() {
40+
return alchemistName;
41+
}
42+
43+
public List<String> getLearnedSpells() {
44+
return learnedSpells;
45+
}
46+
47+
public Set<PotionEffect> getKnownEffects() {
48+
return knownEffects;
49+
}
50+
}

‎querydsl-libraries/querydsl-jpa/src/test/java/com/querydsl/jpa/domain/PotionEffect.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package com.querydsl.jpa.domain;
2+
3+
public enum PotionEffect {
4+
HEALING,
5+
STRENGTH,
6+
INVISIBILITY,
7+
SPEED
8+
}

‎querydsl-libraries/querydsl-jpa/src/test/java/com/querydsl/jpa/domain/PotionEffectSetConverter.java‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package com.querydsl.jpa.domain;
2+
3+
import jakarta.persistence.AttributeConverter;
4+
import jakarta.persistence.Converter;
5+
import java.io.Serializable;
6+
import java.util.Collections;
7+
import java.util.EnumSet;
8+
import java.util.Set;
9+
import java.util.stream.Collectors;
10+
11+
@Converter
12+
public class PotionEffectSetConverter
13+
implements AttributeConverter<Set<PotionEffect>, String>, Serializable {
14+
private static final long serialVersionUID = 4L;
15+
private static final String DELIMITER = ";";
16+
17+
@Override
18+
public String convertToDatabaseColumn(Set<PotionEffect> attribute) {
19+
if (attribute == null || attribute.isEmpty()) {
20+
return null;
21+
}
22+
return attribute.stream().map(PotionEffect::name).collect(Collectors.joining(DELIMITER));
23+
}
24+
25+
@Override
26+
public Set<PotionEffect> convertToEntityAttribute(String dbData) {
27+
if (dbData == null || dbData.trim().isEmpty()) {
28+
return Collections.emptySet();
29+
}
30+
String[] effectNames = dbData.split(DELIMITER);
31+
Set<PotionEffect> effects = EnumSet.noneOf(PotionEffect.class);
32+
for (String effectName : effectNames) {
33+
try {
34+
if (!effectName.trim().isEmpty()) {
35+
effects.add(PotionEffect.valueOf(effectName.trim()));
36+
}
37+
} catch (IllegalArgumentException e) {
38+
System.err.println("Warning: Could not convert '" + effectName + "' to PotionEffect enum.");
39+
}
40+
}
41+
return effects;
42+
}
43+
}

0 commit comments

Comments
 (0)