Skip to content

Commit 7245389

Browse files
gregturngregturn
committed
Make getPredicate return null when there are no predicates.
This does integration testing with both EclipseLink and Hibernate, verifying that queries are run properly. I also inspected the generated queries in the debugger, verifying the "where true=1" was no longer generated. Closes #2282.
1 parent f5ddb7a commit 7245389

File tree

5 files changed

+205
-36
lines changed

5 files changed

+205
-36
lines changed

‎spring-data-jpa/src/main/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilder.java‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,6 @@
1515
*/
1616
package org.springframework.data.jpa.convert;
1717

18-
import java.util.ArrayList;
19-
import java.util.EnumSet;
20-
import java.util.List;
21-
import java.util.Optional;
22-
import java.util.Set;
23-
2418
import jakarta.persistence.criteria.CriteriaBuilder;
2519
import jakarta.persistence.criteria.Expression;
2620
import jakarta.persistence.criteria.From;
@@ -32,6 +26,12 @@
3226
import jakarta.persistence.metamodel.ManagedType;
3327
import jakarta.persistence.metamodel.SingularAttribute;
3428

29+
import java.util.ArrayList;
30+
import java.util.EnumSet;
31+
import java.util.List;
32+
import java.util.Optional;
33+
import java.util.Set;
34+
3535
import org.springframework.dao.InvalidDataAccessApiUsageException;
3636
import org.springframework.data.domain.Example;
3737
import org.springframework.data.domain.ExampleMatcher;
@@ -76,8 +76,9 @@ public class QueryByExamplePredicateBuilder {
7676
* @param root must not be {@literal null}.
7777
* @param cb must not be {@literal null}.
7878
* @param example must not be {@literal null}.
79-
* @return never {@literal null}.
79+
* @return {@literal null} indicates no {@link Predicate}.
8080
*/
81+
@Nullable
8182
public static <T> Predicate getPredicate(Root<T> root, CriteriaBuilder cb, Example<T> example) {
8283
return getPredicate(root, cb, example, EscapeCharacter.DEFAULT);
8384
}
@@ -89,8 +90,9 @@ public static <T> Predicate getPredicate(Root<T> root, CriteriaBuilder cb, Examp
8990
* @param cb must not be {@literal null}.
9091
* @param example must not be {@literal null}.
9192
* @param escapeCharacter Must not be {@literal null}.
92-
* @return never {@literal null}.
93+
* @return {@literal null} indicates no constraints
9394
*/
95+
@Nullable
9496
public static <T> Predicate getPredicate(Root<T> root, CriteriaBuilder cb, Example<T> example,
9597
EscapeCharacter escapeCharacter) {
9698

@@ -105,7 +107,7 @@ public static <T> Predicate getPredicate(Root<T> root, CriteriaBuilder cb, Examp
105107
escapeCharacter);
106108

107109
if (predicates.isEmpty()) {
108-
return cb.isTrue(cb.literal(true));
110+
return null;
109111
}
110112

111113
if (predicates.size() == 1) {

‎spring-data-jpa/src/test/java/org/springframework/data/jpa/convert/QueryByExamplePredicateBuilderUnitTests.java‎

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,6 @@
2020
import static org.mockito.Mockito.*;
2121
import static org.springframework.data.domain.Example.*;
2222

23-
import java.lang.reflect.Member;
24-
import java.util.LinkedHashSet;
25-
import java.util.Set;
26-
2723
import jakarta.persistence.Id;
2824
import jakarta.persistence.criteria.CriteriaBuilder;
2925
import jakarta.persistence.criteria.Expression;
@@ -37,6 +33,10 @@
3733
import jakarta.persistence.metamodel.SingularAttribute;
3834
import jakarta.persistence.metamodel.Type;
3935

36+
import java.lang.reflect.Member;
37+
import java.util.LinkedHashSet;
38+
import java.util.Set;
39+
4040
import org.junit.jupiter.api.BeforeEach;
4141
import org.junit.jupiter.api.Test;
4242
import org.junit.jupiter.api.extension.ExtendWith;
@@ -45,7 +45,6 @@
4545
import org.mockito.junit.jupiter.MockitoExtension;
4646
import org.mockito.junit.jupiter.MockitoSettings;
4747
import org.mockito.quality.Strictness;
48-
4948
import org.springframework.data.domain.Example;
5049
import org.springframework.data.domain.ExampleMatcher;
5150
import org.springframework.data.domain.ExampleMatcher.GenericPropertyMatcher;
@@ -90,19 +89,16 @@ class QueryByExamplePredicateBuilderUnitTests {
9089
void setUp() {
9190

9291
personIdAttribute = new SingularAttributeStub<>("id", PersistentAttributeType.BASIC, Long.class);
93-
personFirstnameAttribute = new SingularAttributeStub<>("firstname", PersistentAttributeType.BASIC,
94-
String.class);
92+
personFirstnameAttribute = new SingularAttributeStub<>("firstname", PersistentAttributeType.BASIC, String.class);
9593
personAgeAttribute = new SingularAttributeStub<>("age", PersistentAttributeType.BASIC, Long.class);
96-
personFatherAttribute = new SingularAttributeStub<>("father", PersistentAttributeType.MANY_TO_ONE,
97-
Person.class, personEntityType);
98-
personSkillAttribute = new SingularAttributeStub<>("skill", PersistentAttributeType.EMBEDDED,
99-
Skill.class, skillEntityType);
100-
personAddressAttribute = new SingularAttributeStub<>("address", PersistentAttributeType.EMBEDDED,
101-
Address.class);
102-
skillNameAttribute = new SingularAttributeStub<>("name", PersistentAttributeType.BASIC,
103-
String.class);
104-
skillNestedAttribute = new SingularAttributeStub<>("nested", PersistentAttributeType.MANY_TO_ONE,
105-
Skill.class, skillEntityType);
94+
personFatherAttribute = new SingularAttributeStub<>("father", PersistentAttributeType.MANY_TO_ONE, Person.class,
95+
personEntityType);
96+
personSkillAttribute = new SingularAttributeStub<>("skill", PersistentAttributeType.EMBEDDED, Skill.class,
97+
skillEntityType);
98+
personAddressAttribute = new SingularAttributeStub<>("address", PersistentAttributeType.EMBEDDED, Address.class);
99+
skillNameAttribute = new SingularAttributeStub<>("name", PersistentAttributeType.BASIC, String.class);
100+
skillNestedAttribute = new SingularAttributeStub<>("nested", PersistentAttributeType.MANY_TO_ONE, Skill.class,
101+
skillEntityType);
106102

107103
personEntityAttribtues = new LinkedHashSet<>();
108104
personEntityAttribtues.add(personIdAttribute);
@@ -153,9 +149,9 @@ void getPredicateShouldThrowExceptionOnNullExample() {
153149
}
154150

155151
@Test // DATAJPA-218
156-
void emptyCriteriaListShouldResultTruePredicate() {
152+
void emptyCriteriaListShouldResultInNullPredicate() {
157153
assertThat(QueryByExamplePredicateBuilder.getPredicate(root, cb, of(new Person()), EscapeCharacter.DEFAULT))
158-
.isEqualTo(truePredicate);
154+
.isNull();
159155
}
160156

161157
@Test // DATAJPA-218
@@ -306,13 +302,11 @@ static class SingularAttributeStub<X, T> implements SingularAttribute<X, T> {
306302
private Class<T> javaType;
307303
private Type<T> type;
308304

309-
SingularAttributeStub(String name,
310-
jakarta.persistence.metamodel.Attribute.PersistentAttributeType attributeType, Class<T> javaType) {
305+
SingularAttributeStub(String name, PersistentAttributeType attributeType, Class<T> javaType) {
311306
this(name, attributeType, javaType, null);
312307
}
313308

314-
SingularAttributeStub(String name,
315-
jakarta.persistence.metamodel.Attribute.PersistentAttributeType attributeType, Class<T> javaType, Type<T> type) {
309+
SingularAttributeStub(String name, PersistentAttributeType attributeType, Class<T> javaType, Type<T> type) {
316310
this.name = name;
317311
this.attributeType = attributeType;
318312
this.javaType = javaType;
@@ -325,7 +319,7 @@ public String getName() {
325319
}
326320

327321
@Override
328-
public jakarta.persistence.metamodel.Attribute.PersistentAttributeType getPersistentAttributeType() {
322+
public PersistentAttributeType getPersistentAttributeType() {
329323
return attributeType;
330324
}
331325

‎spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/QueryByExampleEclipseLinkIntegrationTests.java‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* Copyright 2008-2022 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.jpa.repository;
17+
18+
import static org.assertj.core.api.Assertions.*;
19+
20+
import jakarta.persistence.EntityManager;
21+
import jakarta.persistence.criteria.CriteriaBuilder;
22+
import jakarta.persistence.criteria.CriteriaQuery;
23+
import jakarta.persistence.criteria.Predicate;
24+
import jakarta.persistence.criteria.Root;
25+
26+
import org.junit.jupiter.api.AfterEach;
27+
import org.junit.jupiter.api.BeforeEach;
28+
import org.junit.jupiter.api.Test;
29+
import org.junit.jupiter.api.extension.ExtendWith;
30+
import org.springframework.beans.factory.annotation.Autowired;
31+
import org.springframework.data.domain.Example;
32+
import org.springframework.data.jpa.convert.QueryByExamplePredicateBuilder;
33+
import org.springframework.data.jpa.domain.sample.Role;
34+
import org.springframework.data.jpa.repository.sample.RoleRepository;
35+
import org.springframework.test.context.ContextConfiguration;
36+
import org.springframework.test.context.junit.jupiter.SpringExtension;
37+
import org.springframework.transaction.annotation.Transactional;
38+
39+
/**
40+
* @author Greg Turnquist
41+
* @since 3.0
42+
*/
43+
@ExtendWith(SpringExtension.class)
44+
@ContextConfiguration({ "classpath:eclipselink.xml", "classpath:config/namespace-application-context.xml" })
45+
@Transactional
46+
public class QueryByExampleEclipseLinkIntegrationTests {
47+
48+
@Autowired RoleRepository repository;
49+
@Autowired EntityManager em;
50+
51+
private Role drummer;
52+
private Role guitarist;
53+
private Role singer;
54+
55+
@BeforeEach
56+
void setUp() {
57+
58+
drummer = repository.save(new Role("drummer"));
59+
guitarist = repository.save(new Role("guitarist"));
60+
singer = repository.save(new Role("singer"));
61+
}
62+
63+
@AfterEach
64+
void clearUp() {
65+
repository.deleteAll();
66+
}
67+
68+
@Test // GH-2283
69+
void queryByExampleWithNoPredicatesShouldHaveNoWhereClause() {
70+
71+
// given
72+
Role probe = new Role();
73+
Example<Role> example = Example.of(probe);
74+
75+
CriteriaBuilder builder = em.getCriteriaBuilder();
76+
CriteriaQuery<Role> query = builder.createQuery(Role.class);
77+
Root<Role> root = query.from(Role.class);
78+
79+
// when
80+
Predicate predicate = QueryByExamplePredicateBuilder.getPredicate(root, builder, example);
81+
82+
// then
83+
assertThat(predicate).isNull();
84+
assertThat(repository.findAll(example)).containsExactlyInAnyOrder(drummer, guitarist, singer);
85+
}
86+
}

‎spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/QueryByExampleHibernateIntegrationTests.java‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/*
2+
* Copyright 2008-2022 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.jpa.repository;
17+
18+
import static org.assertj.core.api.Assertions.*;
19+
20+
import jakarta.persistence.EntityManager;
21+
import jakarta.persistence.criteria.CriteriaBuilder;
22+
import jakarta.persistence.criteria.CriteriaQuery;
23+
import jakarta.persistence.criteria.Predicate;
24+
import jakarta.persistence.criteria.Root;
25+
26+
import org.junit.jupiter.api.AfterEach;
27+
import org.junit.jupiter.api.BeforeEach;
28+
import org.junit.jupiter.api.Test;
29+
import org.junit.jupiter.api.extension.ExtendWith;
30+
import org.springframework.beans.factory.annotation.Autowired;
31+
import org.springframework.data.domain.Example;
32+
import org.springframework.data.jpa.convert.QueryByExamplePredicateBuilder;
33+
import org.springframework.data.jpa.domain.sample.Role;
34+
import org.springframework.data.jpa.repository.sample.RoleRepository;
35+
import org.springframework.test.context.ContextConfiguration;
36+
import org.springframework.test.context.junit.jupiter.SpringExtension;
37+
import org.springframework.transaction.annotation.Transactional;
38+
39+
/**
40+
* @author Greg Turnquist
41+
* @since 3.0
42+
*/
43+
@ExtendWith(SpringExtension.class)
44+
@ContextConfiguration({ "classpath:hibernate.xml", "classpath:config/namespace-application-context.xml" })
45+
@Transactional
46+
public class QueryByExampleHibernateIntegrationTests {
47+
48+
@Autowired RoleRepository repository;
49+
@Autowired EntityManager em;
50+
51+
private Role drummer;
52+
private Role guitarist;
53+
private Role singer;
54+
55+
@BeforeEach
56+
void setUp() {
57+
58+
drummer = repository.save(new Role("drummer"));
59+
guitarist = repository.save(new Role("guitarist"));
60+
singer = repository.save(new Role("singer"));
61+
}
62+
63+
@AfterEach
64+
void clearUp() {
65+
repository.deleteAll();
66+
}
67+
68+
@Test // GH-2283
69+
void queryByExampleWithNoPredicatesShouldHaveNoWhereClause() {
70+
71+
// given
72+
Role probe = new Role();
73+
Example<Role> example = Example.of(probe);
74+
75+
CriteriaBuilder builder = em.getCriteriaBuilder();
76+
CriteriaQuery<Role> query = builder.createQuery(Role.class);
77+
Root<Role> root = query.from(Role.class);
78+
79+
// when
80+
Predicate predicate = QueryByExamplePredicateBuilder.getPredicate(root, builder, example);
81+
82+
// then
83+
assertThat(predicate).isNull();
84+
assertThat(repository.findAll(example)).containsExactlyInAnyOrder(drummer, guitarist, singer);
85+
}
86+
}

‎spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/sample/RoleRepository.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,14 @@
1818
import jakarta.persistence.LockModeType;
1919
import jakarta.persistence.QueryHint;
2020

21+
import java.util.List;
2122
import java.util.Optional;
2223

2324
import org.springframework.data.jpa.domain.sample.Role;
25+
import org.springframework.data.jpa.repository.JpaRepository;
2426
import org.springframework.data.jpa.repository.Lock;
2527
import org.springframework.data.jpa.repository.QueryHints;
2628
import org.springframework.data.querydsl.QuerydslPredicateExecutor;
27-
import org.springframework.data.repository.CrudRepository;
2829

2930
import com.querydsl.core.types.Predicate;
3031

@@ -35,12 +36,12 @@
3536
* @author Thomas Darimont
3637
* @author Yanming Zhou
3738
*/
38-
public interface RoleRepository extends CrudRepository<Role, Integer>, QuerydslPredicateExecutor<Role> {
39+
public interface RoleRepository extends JpaRepository<Role, Integer>, QuerydslPredicateExecutor<Role> {
3940

4041
@Override
4142
@Lock(LockModeType.READ)
4243
@QueryHints(@QueryHint(name = "foo", value = "bar"))
43-
Iterable<Role> findAll();
44+
List<Role> findAll();
4445

4546
@Override
4647
@Lock(LockModeType.READ)

0 commit comments

Comments
 (0)