Skip to content

Commit 3c9becc

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

File tree

6 files changed

+42
-41
lines changed

6 files changed

+42
-41
lines changed

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

Lines changed: 9 additions & 8 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

@@ -67,29 +67,30 @@ public <E, P> P createPredicate(KeysetScrollPosition keyset, Sort sort, QueryAda
6767
for (Order order : sort) {
6868

6969
if (!keysetValues.containsKey(order.getProperty())) {
70-
throw new IllegalStateException("KeysetScrollPosition does not contain all keyset values");
70+
throw new IllegalStateException(String
71+
.format("KeysetScrollPosition does not contain all keyset values. Missing key: %s", order.getProperty()));
7172
}
7273

7374
List<P> sortConstraint = new ArrayList<>();
7475

7576
int j = 0;
7677
for (Order inner : sort) {
7778

78-
E propertyExpression = queryAdapter.createExpression(inner.getProperty());
79+
E propertyExpression = strategy.createExpression(inner.getProperty());
7980
Object o = keysetValues.get(inner.getProperty());
8081

8182
if (j >= i) { // tail segment
8283

83-
sortConstraint.add(queryAdapter.compare(inner, propertyExpression, o));
84+
sortConstraint.add(strategy.compare(inner, propertyExpression, o));
8485
break;
8586
}
8687

87-
sortConstraint.add(queryAdapter.compare(propertyExpression, o));
88+
sortConstraint.add(strategy.compare(propertyExpression, o));
8889
j++;
8990
}
9091

9192
if (!sortConstraint.isEmpty()) {
92-
or.add(queryAdapter.and(sortConstraint));
93+
or.add(strategy.and(sortConstraint));
9394
}
9495

9596
i++;
@@ -99,7 +100,7 @@ public <E, P> P createPredicate(KeysetScrollPosition keyset, Sort sort, QueryAda
99100
return null;
100101
}
101102

102-
return queryAdapter.or(or);
103+
return strategy.or(or);
103104
}
104105

105106
protected Sort getSortOrders(Sort sort) {
@@ -150,7 +151,7 @@ protected <T> List<T> getResultWindow(List<T> list, int limit) {
150151
* @param <E> property path expression type.
151152
* @param <P> predicate type.
152153
*/
153-
public interface QueryAdapter<E, P> {
154+
public interface QueryStrategy<E, P> {
154155

155156
/**
156157
* Create an expression object from the given {@code property} path.

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

Lines changed: 7 additions & 7 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;
@@ -60,7 +60,7 @@ public KeysetScrollSpecification(KeysetScrollPosition position, Sort sort, JpaEn
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,17 +80,17 @@ 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) {
9494
this.from = from;
9595
this.cb = cb;
9696
}

‎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/QuerydslJpaPredicateExecutor.java

Lines changed: 6 additions & 6 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;
@@ -75,7 +75,7 @@ public class QuerydslJpaPredicateExecutor<T> implements QuerydslPredicateExecuto
7575
private final JpaEntityInformation<T, ?> entityInformation;
7676
private final EntityPath<T> path;
7777
private final Querydsl querydsl;
78-
private final QuerydslQueryAdapter scrollQueryAdapter;
78+
private final QuerydslQueryStrategy scrollQueryAdapter;
7979
private final EntityManager entityManager;
8080
private final CrudMethodMetadata metadata;
8181

@@ -96,7 +96,7 @@ public QuerydslJpaPredicateExecutor(JpaEntityInformation<T, ?> entityInformation
9696
this.path = resolver.createPath(entityInformation.getJavaType());
9797
this.querydsl = new Querydsl(entityManager, new PathBuilder<T>(path.getType(), path.getMetadata()));
9898
this.entityManager = entityManager;
99-
this.scrollQueryAdapter = new QuerydslQueryAdapter();
99+
this.scrollQueryAdapter = new QuerydslQueryStrategy();
100100
}
101101

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

181181
if (scrollPosition instanceof KeysetScrollPosition keyset) {
182182

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

187187
if (keysetPredicate != null) {
188188
predicateToUse = predicate instanceof BooleanExpression be ? be.and(keysetPredicate)
@@ -328,7 +328,7 @@ private List<T> executeSorted(JPQLQuery<T> query, Sort sort) {
328328
return querydsl.applySorting(sort, query).fetch();
329329
}
330330

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

333333
@Override
334334
public Expression<?> createExpression(String property) {

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,13 +1245,13 @@ void scrollByExampleOffset() {
12451245
Window<User> firstWindow = repository.findBy(example,
12461246
q -> q.limit(2).sortBy(Sort.by("firstname")).scroll(OffsetScrollPosition.initial()));
12471247

1248-
assertThat(firstWindow).hasSize(2).containsOnly(jane1, jane2);
1248+
assertThat(firstWindow).containsExactly(jane1, jane2);
12491249
assertThat(firstWindow.hasNext()).isTrue();
12501250

12511251
Window<User> nextWindow = repository.findBy(example,
12521252
q -> q.limit(2).sortBy(Sort.by("firstname")).scroll(firstWindow.positionAt(1)));
12531253

1254-
assertThat(nextWindow).hasSize(2).containsOnly(john1, john2);
1254+
assertThat(nextWindow).containsExactly(john1, john2);
12551255
assertThat(nextWindow.hasNext()).isFalse();
12561256
}
12571257

@@ -1277,7 +1277,7 @@ void scrollByExampleKeyset() {
12771277
Window<User> nextWindow = repository.findBy(example,
12781278
q -> q.limit(2).sortBy(Sort.by("firstname", "emailAddress")).scroll(firstWindow.positionAt(0)));
12791279

1280-
assertThat(nextWindow).hasSize(2).containsOnly(jane2, john1);
1280+
assertThat(nextWindow).containsExactly(jane2, john1);
12811281
assertThat(nextWindow.hasNext()).isTrue();
12821282
}
12831283

@@ -1319,13 +1319,13 @@ void scrollByPredicateOffset() {
13191319
Window<User> firstWindow = repository.findBy(QUser.user.firstname.startsWith("J"),
13201320
q -> q.limit(2).sortBy(Sort.by("firstname")).scroll(OffsetScrollPosition.initial()));
13211321

1322-
assertThat(firstWindow).hasSize(2).containsOnly(jane1, jane2);
1322+
assertThat(firstWindow).containsExactly(jane1, jane2);
13231323
assertThat(firstWindow.hasNext()).isTrue();
13241324

13251325
Window<User> nextWindow = repository.findBy(QUser.user.firstname.startsWith("J"),
13261326
q -> q.limit(2).sortBy(Sort.by("firstname")).scroll(firstWindow.positionAt(1)));
13271327

1328-
assertThat(nextWindow).hasSize(2).containsOnly(john1, john2);
1328+
assertThat(nextWindow).containsExactly(john1, john2);
13291329
assertThat(nextWindow.hasNext()).isFalse();
13301330
}
13311331

@@ -1348,7 +1348,7 @@ void scrollByPredicateKeyset() {
13481348
Window<User> nextWindow = repository.findBy(QUser.user.firstname.startsWith("J"),
13491349
q -> q.limit(2).sortBy(Sort.by("firstname", "emailAddress")).scroll(firstWindow.positionAt(0)));
13501350

1351-
assertThat(nextWindow).hasSize(2).containsOnly(jane2, john1);
1351+
assertThat(nextWindow).containsExactly(jane2, john1);
13521352
assertThat(nextWindow.hasNext()).isTrue();
13531353
}
13541354

@@ -1365,7 +1365,7 @@ void scrollByPredicateKeysetBackward() {
13651365
Window<User> firstWindow = repository.findBy(QUser.user.firstname.startsWith("J"),
13661366
q -> q.limit(3).sortBy(Sort.by("firstname", "emailAddress")).scroll(KeysetScrollPosition.initial()));
13671367

1368-
assertThat(firstWindow).containsSequence(jane1, jane2, john1);
1368+
assertThat(firstWindow).containsExactly(jane1, jane2, john1);
13691369
assertThat(firstWindow.hasNext()).isTrue();
13701370

13711371
KeysetScrollPosition scrollPosition = (KeysetScrollPosition) firstWindow.positionAt(2);
@@ -1374,7 +1374,7 @@ void scrollByPredicateKeysetBackward() {
13741374
Window<User> previousWindow = repository.findBy(QUser.user.firstname.startsWith("J"),
13751375
q -> q.limit(3).sortBy(Sort.by("firstname", "emailAddress")).scroll(backward));
13761376

1377-
assertThat(previousWindow).hasSize(2).containsSequence(jane1, jane2);
1377+
assertThat(previousWindow).containsExactly(jane1, jane2);
13781378

13791379
// no more items before this window
13801380
assertThat(previousWindow.hasNext()).isFalse();
@@ -1393,7 +1393,7 @@ void scrollByPartTreeKeysetBackward() {
13931393
Window<User> firstWindow = repository.findTop3ByFirstnameStartingWithOrderByFirstnameAscEmailAddressAsc("J",
13941394
KeysetScrollPosition.initial());
13951395

1396-
assertThat(firstWindow).containsSequence(jane1, jane2, john1);
1396+
assertThat(firstWindow).containsExactly(jane1, jane2, john1);
13971397
assertThat(firstWindow.hasNext()).isTrue();
13981398

13991399
KeysetScrollPosition scrollPosition = (KeysetScrollPosition) firstWindow.positionAt(2);
@@ -1402,7 +1402,7 @@ void scrollByPartTreeKeysetBackward() {
14021402
Window<User> previousWindow = repository.findTop3ByFirstnameStartingWithOrderByFirstnameAscEmailAddressAsc("J",
14031403
backward);
14041404

1405-
assertThat(previousWindow).hasSize(2).containsSequence(jane1, jane2);
1405+
assertThat(previousWindow).containsExactly(jane1, jane2);
14061406

14071407
// no more items before this window
14081408
assertThat(previousWindow.hasNext()).isFalse();

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,16 @@ class CollectionUtilsUnitTests {
3131
@Test // GH-2878
3232
void shouldReturnFirstItems() {
3333

34-
assertThat(CollectionUtils.getFirst(2, List.of(1, 2, 3))).hasSize(2).containsSequence(1, 2);
35-
assertThat(CollectionUtils.getFirst(2, List.of(1, 2))).hasSize(2).containsSequence(1, 2);
36-
assertThat(CollectionUtils.getFirst(2, List.of(1))).hasSize(1).containsSequence(1);
34+
assertThat(CollectionUtils.getFirst(2, List.of(1, 2, 3))).containsExactly(1, 2);
35+
assertThat(CollectionUtils.getFirst(2, List.of(1, 2))).containsExactly(1, 2);
36+
assertThat(CollectionUtils.getFirst(2, List.of(1))).containsExactly(1);
3737
}
3838

3939
@Test // GH-2878
4040
void shouldReturnLastItems() {
4141

42-
assertThat(CollectionUtils.getLast(2, List.of(1, 2, 3))).hasSize(2).containsSequence(2, 3);
43-
assertThat(CollectionUtils.getLast(2, List.of(1, 2))).hasSize(2).containsSequence(1, 2);
44-
assertThat(CollectionUtils.getLast(2, List.of(1))).hasSize(1).containsSequence(1);
42+
assertThat(CollectionUtils.getLast(2, List.of(1, 2, 3))).containsExactly(2, 3);
43+
assertThat(CollectionUtils.getLast(2, List.of(1, 2))).containsExactly(1, 2);
44+
assertThat(CollectionUtils.getLast(2, List.of(1))).containsExactly(1);
4545
}
4646
}

0 commit comments

Comments
 (0)