Skip to content

Commit ba37131

Browse files
mp911demp911de
committed
Address review comments.
1 parent 053cbef commit ba37131

File tree

12 files changed

+97
-89
lines changed

12 files changed

+97
-89
lines changed

‎spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/CollectionUtils.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ class CollectionUtils {
2727
/**
2828
* Return the first {@code count} items from the list.
2929
*
30-
* @param count
31-
* @param list
32-
* @return
30+
* @param count the number of first elements to be included in the returned list.
31+
* @param list must not be {@literal null}
32+
* @return the returned sublist if the {@code list} is greater {@code count}.
3333
* @param <T>
3434
*/
3535
public static <T> List<T> getFirst(int count, List<T> list) {
@@ -44,9 +44,9 @@ public static <T> List<T> getFirst(int count, List<T> list) {
4444
/**
4545
* Return the last {@code count} items from the list.
4646
*
47-
* @param count
48-
* @param list
49-
* @return
47+
* @param count the number of last elements to be included in the returned list.
48+
* @param list must not be {@literal null}
49+
* @return the returned sublist if the {@code list} is greater {@code count}.
5050
* @param <T>
5151
*/
5252
public static <T> List<T> getLast(int count, List<T> list) {

‎spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaKeysetScrollQueryCreator.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.springframework.data.jpa.repository.support.JpaEntityInformation;
3030
import org.springframework.data.repository.query.ReturnedType;
3131
import org.springframework.data.repository.query.parser.PartTree;
32+
import org.springframework.lang.Nullable;
3233

3334
/**
3435
* Extension to {@link JpaQueryCreator} to create queries considering {@link KeysetScrollPosition keyset scrolling}.
@@ -50,7 +51,7 @@ public JpaKeysetScrollQueryCreator(PartTree tree, ReturnedType type, CriteriaBui
5051
}
5152

5253
@Override
53-
protected CriteriaQuery<? extends Object> complete(Predicate predicate, Sort sort, CriteriaQuery<?> query,
54+
protected CriteriaQuery<?> complete(@Nullable Predicate predicate, Sort sort, CriteriaQuery<?> query,
5455
CriteriaBuilder builder, Root<?> root) {
5556

5657
KeysetScrollSpecification<Object> keysetSpec = new KeysetScrollSpecification<>(scrollPosition, sort,

‎spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollDelegate.java‎

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public static KeysetScrollDelegate of(Direction direction) {
4848
}
4949

5050
@Nullable
51-
public <E, P> P createPredicate(KeysetScrollPosition keyset, Sort sort, QueryAdapter<E, P> queryAdapter) {
51+
public <E, P> P createPredicate(KeysetScrollPosition keyset, Sort sort, QueryStrategy<E, P> strategy) {
5252

5353
Map<String, Object> keysetValues = keyset.getKeys();
5454

@@ -57,39 +57,37 @@ public <E, P> P createPredicate(KeysetScrollPosition keyset, Sort sort, QueryAda
5757
return null;
5858
}
5959

60-
// build matrix query for keyset paging that contains sort^2 queries
61-
// reflecting a query that follows sort order semantics starting from the last returned keyset
62-
6360
List<P> or = new ArrayList<>();
64-
6561
int i = 0;
66-
// progressive query building
62+
63+
// progressive query building to reconstruct a query matching sorting rules
6764
for (Order order : sort) {
6865

6966
if (!keysetValues.containsKey(order.getProperty())) {
70-
throw new IllegalStateException("KeysetScrollPosition does not contain all keyset values");
67+
throw new IllegalStateException(String
68+
.format("KeysetScrollPosition does not contain all keyset values. Missing key: %s", order.getProperty()));
7169
}
7270

7371
List<P> sortConstraint = new ArrayList<>();
7472

7573
int j = 0;
7674
for (Order inner : sort) {
7775

78-
E propertyExpression = queryAdapter.createExpression(inner.getProperty());
76+
E propertyExpression = strategy.createExpression(inner.getProperty());
7977
Object o = keysetValues.get(inner.getProperty());
8078

8179
if (j >= i) { // tail segment
8280

83-
sortConstraint.add(queryAdapter.compare(inner, propertyExpression, o));
81+
sortConstraint.add(strategy.compare(inner, propertyExpression, o));
8482
break;
8583
}
8684

87-
sortConstraint.add(queryAdapter.compare(propertyExpression, o));
85+
sortConstraint.add(strategy.compare(propertyExpression, o));
8886
j++;
8987
}
9088

9189
if (!sortConstraint.isEmpty()) {
92-
or.add(queryAdapter.and(sortConstraint));
90+
or.add(strategy.and(sortConstraint));
9391
}
9492

9593
i++;
@@ -99,7 +97,7 @@ public <E, P> P createPredicate(KeysetScrollPosition keyset, Sort sort, QueryAda
9997
return null;
10098
}
10199

102-
return queryAdapter.or(or);
100+
return strategy.or(or);
103101
}
104102

105103
protected Sort getSortOrders(Sort sort) {
@@ -116,9 +114,9 @@ protected <T> List<T> getResultWindow(List<T> list, int limit) {
116114
}
117115

118116
/**
119-
* Reverse scrolling director variant applying {@link Direction#Backward}. In reverse scrolling, we need to flip
120-
* directions for the actual query so that we do not get everything from the top position and apply the limit but
121-
* rather flip the sort direction, apply the limit and then reverse the result to restore the actual sort order.
117+
* Reverse scrolling variant applying {@link Direction#Backward}. In reverse scrolling, we need to flip directions for
118+
* the actual query so that we do not get everything from the top position and apply the limit but rather flip the
119+
* sort direction, apply the limit and then reverse the result to restore the actual sort order.
122120
*/
123121
private static class ReverseKeysetScrollDelegate extends KeysetScrollDelegate {
124122

@@ -150,34 +148,34 @@ protected <T> List<T> getResultWindow(List<T> list, int limit) {
150148
* @param <E> property path expression type.
151149
* @param <P> predicate type.
152150
*/
153-
public interface QueryAdapter<E, P> {
151+
public interface QueryStrategy<E, P> {
154152

155153
/**
156154
* Create an expression object from the given {@code property} path.
157155
*
158-
* @param property
156+
* @param property must not be {@literal null}.
159157
* @return
160158
*/
161159
E createExpression(String property);
162160

163161
/**
164162
* Create a comparison object according to the {@link Order}.
165163
*
166-
* @param order
167-
* @param propertyExpression
168-
* @param o
164+
* @param order must not be {@literal null}.
165+
* @param propertyExpression must not be {@literal null}.
166+
* @param value
169167
* @return
170168
*/
171-
P compare(Order order, E propertyExpression, Object o);
169+
P compare(Order order, E propertyExpression, Object value);
172170

173171
/**
174172
* Create an equals-comparison object.
175173
*
176-
* @param propertyExpression
177-
* @param o
174+
* @param propertyExpression must not be {@literal null}.
175+
* @param value
178176
* @return
179177
*/
180-
P compare(E propertyExpression, Object o);
178+
P compare(E propertyExpression, @Nullable Object value);
181179

182180
/**
183181
* AND-combine the {@code intermediate} predicates.

‎spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/KeysetScrollSpecification.java‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
import org.springframework.data.domain.Sort;
2929
import org.springframework.data.domain.Sort.Order;
3030
import org.springframework.data.jpa.domain.Specification;
31-
import org.springframework.data.jpa.repository.query.KeysetScrollDelegate.QueryAdapter;
31+
import org.springframework.data.jpa.repository.query.KeysetScrollDelegate.QueryStrategy;
3232
import org.springframework.data.jpa.repository.support.JpaEntityInformation;
3333
import org.springframework.data.mapping.PropertyPath;
3434
import org.springframework.lang.Nullable;
@@ -53,14 +53,14 @@ public KeysetScrollSpecification(KeysetScrollPosition position, Sort sort, JpaEn
5353
/**
5454
* Create a {@link Sort} object to be used with the actual query.
5555
*
56-
* @param position
57-
* @param sort
58-
* @param entity
56+
* @param position must not be {@literal null}.
57+
* @param sort must not be {@literal null}.
58+
* @param entity must not be {@literal null}.
5959
* @return
6060
*/
6161
public static Sort createSort(KeysetScrollPosition position, Sort sort, JpaEntityInformation<?, ?> entity) {
6262

63-
KeysetScrollDelegate director = KeysetScrollDelegate.of(position.getDirection());
63+
KeysetScrollDelegate delegate = KeysetScrollDelegate.of(position.getDirection());
6464

6565
Sort sortToUse;
6666
if (entity.hasCompositeId()) {
@@ -69,7 +69,7 @@ public static Sort createSort(KeysetScrollPosition position, Sort sort, JpaEntit
6969
sortToUse = sort.and(Sort.by(entity.getRequiredIdAttribute().getName()));
7070
}
7171

72-
return director.getSortOrders(sortToUse);
72+
return delegate.getSortOrders(sortToUse);
7373
}
7474

7575
@Override
@@ -80,36 +80,39 @@ public Predicate toPredicate(Root<T> root, CriteriaQuery<?> query, CriteriaBuild
8080
@Nullable
8181
public Predicate createPredicate(Root<?> root, CriteriaBuilder criteriaBuilder) {
8282

83-
KeysetScrollDelegate director = KeysetScrollDelegate.of(position.getDirection());
84-
return director.createPredicate(position, sort, new JpaQueryAdapter(root, criteriaBuilder));
83+
KeysetScrollDelegate delegate = KeysetScrollDelegate.of(position.getDirection());
84+
return delegate.createPredicate(position, sort, new JpaQueryStrategy(root, criteriaBuilder));
8585
}
8686

8787
@SuppressWarnings("rawtypes")
88-
private static class JpaQueryAdapter implements QueryAdapter<Expression<Comparable>, Predicate> {
88+
private static class JpaQueryStrategy implements QueryStrategy<Expression<Comparable>, Predicate> {
8989

9090
private final From<?, ?> from;
9191
private final CriteriaBuilder cb;
9292

93-
public JpaQueryAdapter(From<?, ?> from, CriteriaBuilder cb) {
93+
public JpaQueryStrategy(From<?, ?> from, CriteriaBuilder cb) {
94+
9495
this.from = from;
9596
this.cb = cb;
9697
}
9798

9899
@Override
99100
public Expression<Comparable> createExpression(String property) {
101+
100102
PropertyPath path = PropertyPath.from(property, from.getJavaType());
101103
return QueryUtils.toExpressionRecursively(from, path);
102104
}
103105

104106
@Override
105-
public Predicate compare(Order order, Expression<Comparable> propertyExpression, Object o) {
106-
return order.isAscending() ? cb.greaterThan(propertyExpression, (Comparable) o)
107-
: cb.lessThan(propertyExpression, (Comparable) o);
107+
public Predicate compare(Order order, Expression<Comparable> propertyExpression, Object value) {
108+
109+
return order.isAscending() ? cb.greaterThan(propertyExpression, (Comparable) value)
110+
: cb.lessThan(propertyExpression, (Comparable) value);
108111
}
109112

110113
@Override
111-
public Predicate compare(Expression<Comparable> propertyExpression, Object o) {
112-
return cb.equal(propertyExpression, o);
114+
public Predicate compare(Expression<Comparable> propertyExpression, @Nullable Object value) {
115+
return value == null ? cb.isNull(propertyExpression) : cb.equal(propertyExpression, value);
113116
}
114117

115118
@Override

‎spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ScrollDelegate.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public class ScrollDelegate<T> {
4141

4242
private final JpaEntityInformation<T, ?> entity;
4343

44-
public ScrollDelegate(JpaEntityInformation<T, ?> entity) {
44+
protected ScrollDelegate(JpaEntityInformation<T, ?> entity) {
4545
this.entity = entity;
4646
}
4747

@@ -79,8 +79,8 @@ public Window<T> scroll(Query query, Sort sort, ScrollPosition scrollPosition) {
7979
private static <T> Window<T> createWindow(Sort sort, int limit, Direction direction,
8080
JpaEntityInformation<T, ?> entity, List<T> result) {
8181

82-
KeysetScrollDelegate director = KeysetScrollDelegate.of(direction);
83-
List<T> resultsToUse = director.postProcessResults(result);
82+
KeysetScrollDelegate delegate = KeysetScrollDelegate.of(direction);
83+
List<T> resultsToUse = delegate.postProcessResults(result);
8484

8585
IntFunction<KeysetScrollPosition> positionFunction = value -> {
8686

@@ -90,7 +90,7 @@ private static <T> Window<T> createWindow(Sort sort, int limit, Direction direct
9090
return KeysetScrollPosition.of(keys);
9191
};
9292

93-
return Window.from(director.getResultWindow(resultsToUse, limit), positionFunction, hasMoreElements(result, limit));
93+
return Window.from(delegate.getResultWindow(resultsToUse, limit), positionFunction, hasMoreElements(result, limit));
9494
}
9595

9696
private static <T> Window<T> createWindow(List<T> result, int limit,

‎spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/FetchableFluentQueryBySpecification.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,22 +60,22 @@ class FetchableFluentQueryBySpecification<S, R> extends FluentQuerySupport<S, R>
6060
private final EntityManager entityManager;
6161

6262
public FetchableFluentQueryBySpecification(Specification<S> spec, Class<S> entityType,
63-
Function<Sort, TypedQuery<S>> finder, SpecificationScrollDelegate<S> scrollDirector,
63+
Function<Sort, TypedQuery<S>> finder, SpecificationScrollDelegate<S> scrollDelegate,
6464
Function<Specification<S>, Long> countOperation, Function<Specification<S>, Boolean> existsOperation,
6565
EntityManager entityManager) {
66-
this(spec, entityType, (Class<R>) entityType, Sort.unsorted(), 0, Collections.emptySet(), finder, scrollDirector,
66+
this(spec, entityType, (Class<R>) entityType, Sort.unsorted(), 0, Collections.emptySet(), finder, scrollDelegate,
6767
countOperation, existsOperation, entityManager);
6868
}
6969

7070
private FetchableFluentQueryBySpecification(Specification<S> spec, Class<S> entityType, Class<R> resultType,
7171
Sort sort, int limit, Collection<String> properties, Function<Sort, TypedQuery<S>> finder,
72-
SpecificationScrollDelegate<S> scrollDirector, Function<Specification<S>, Long> countOperation,
72+
SpecificationScrollDelegate<S> scrollDelegate, Function<Specification<S>, Long> countOperation,
7373
Function<Specification<S>, Boolean> existsOperation, EntityManager entityManager) {
7474

7575
super(resultType, sort, limit, properties, entityType);
7676
this.spec = spec;
7777
this.finder = finder;
78-
this.scroll = scrollDirector;
78+
this.scroll = scrollDelegate;
7979
this.countOperation = countOperation;
8080
this.existsOperation = existsOperation;
8181
this.entityManager = entityManager;

‎spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/JpaMetamodelEntityInformation.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,8 @@ public boolean isNew(T entity) {
228228
@Override
229229
public Map<String, Object> getKeyset(Iterable<String> propertyPaths, T entity) {
230230

231-
// TODO: proxy business?
231+
// TODO: Proxy handling requires more elaborate refactoring, see
232+
// https://github.com/spring-projects/spring-data-jpa/issues/2784
232233
BeanWrapper entityWrapper = new DirectFieldAccessFallbackBeanWrapper(entity);
233234

234235
Map<String, Object> keyset = new LinkedHashMap<>();

‎spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/support/QuerydslJpaPredicateExecutor.java‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
import org.springframework.data.domain.Sort.Order;
3333
import org.springframework.data.jpa.repository.EntityGraph;
3434
import org.springframework.data.jpa.repository.query.KeysetScrollDelegate;
35-
import org.springframework.data.jpa.repository.query.KeysetScrollDelegate.QueryAdapter;
35+
import org.springframework.data.jpa.repository.query.KeysetScrollDelegate.QueryStrategy;
3636
import org.springframework.data.jpa.repository.query.KeysetScrollSpecification;
3737
import org.springframework.data.jpa.repository.support.FetchableFluentQueryByPredicate.PredicateScrollDelegate;
3838
import org.springframework.data.jpa.repository.support.FluentQuerySupport.ScrollQueryFactory;
@@ -48,6 +48,7 @@
4848
import com.querydsl.core.types.ConstantImpl;
4949
import com.querydsl.core.types.EntityPath;
5050
import com.querydsl.core.types.Expression;
51+
import com.querydsl.core.types.NullExpression;
5152
import com.querydsl.core.types.Ops;
5253
import com.querydsl.core.types.OrderSpecifier;
5354
import com.querydsl.core.types.Predicate;
@@ -75,7 +76,7 @@ public class QuerydslJpaPredicateExecutor<T> implements QuerydslPredicateExecuto
7576
private final JpaEntityInformation<T, ?> entityInformation;
7677
private final EntityPath<T> path;
7778
private final Querydsl querydsl;
78-
private final QuerydslQueryAdapter scrollQueryAdapter;
79+
private final QuerydslQueryStrategy scrollQueryAdapter;
7980
private final EntityManager entityManager;
8081
private final CrudMethodMetadata metadata;
8182

@@ -96,7 +97,7 @@ public QuerydslJpaPredicateExecutor(JpaEntityInformation<T, ?> entityInformation
9697
this.path = resolver.createPath(entityInformation.getJavaType());
9798
this.querydsl = new Querydsl(entityManager, new PathBuilder<T>(path.getType(), path.getMetadata()));
9899
this.entityManager = entityManager;
99-
this.scrollQueryAdapter = new QuerydslQueryAdapter();
100+
this.scrollQueryAdapter = new QuerydslQueryStrategy();
100101
}
101102

102103
@Override
@@ -180,9 +181,9 @@ public <S extends T, R> R findBy(Predicate predicate, Function<FetchableFluentQu
180181

181182
if (scrollPosition instanceof KeysetScrollPosition keyset) {
182183

183-
KeysetScrollDelegate director = KeysetScrollDelegate.of(keyset.getDirection());
184+
KeysetScrollDelegate delegate = KeysetScrollDelegate.of(keyset.getDirection());
184185
sort = KeysetScrollSpecification.createSort(keyset, sort, entityInformation);
185-
BooleanExpression keysetPredicate = director.createPredicate(keyset, sort, scrollQueryAdapter);
186+
BooleanExpression keysetPredicate = delegate.createPredicate(keyset, sort, scrollQueryAdapter);
186187

187188
if (keysetPredicate != null) {
188189
predicateToUse = predicate instanceof BooleanExpression be ? be.and(keysetPredicate)
@@ -328,22 +329,23 @@ private List<T> executeSorted(JPQLQuery<T> query, Sort sort) {
328329
return querydsl.applySorting(sort, query).fetch();
329330
}
330331

331-
class QuerydslQueryAdapter implements QueryAdapter<Expression<?>, BooleanExpression> {
332+
class QuerydslQueryStrategy implements QueryStrategy<Expression<?>, BooleanExpression> {
332333

333334
@Override
334335
public Expression<?> createExpression(String property) {
335336
return querydsl.createExpression(property);
336337
}
337338

338339
@Override
339-
public BooleanExpression compare(Order order, Expression<?> propertyExpression, Object o) {
340+
public BooleanExpression compare(Order order, Expression<?> propertyExpression, Object value) {
340341
return Expressions.booleanOperation(order.isAscending() ? Ops.GT : Ops.LT, propertyExpression,
341-
ConstantImpl.create(o));
342+
ConstantImpl.create(value));
342343
}
343344

344345
@Override
345-
public BooleanExpression compare(Expression<?> propertyExpression, Object o) {
346-
return Expressions.booleanOperation(Ops.EQ, propertyExpression, ConstantImpl.create(o));
346+
public BooleanExpression compare(Expression<?> propertyExpression, @Nullable Object value) {
347+
return Expressions.booleanOperation(Ops.EQ, propertyExpression,
348+
value == null ? NullExpression.DEFAULT : ConstantImpl.create(value));
347349
}
348350

349351
@Override

0 commit comments

Comments
 (0)