Skip to content

Commit 1649506

Browse files
mp911demp911de
committed
Consider Sort override from Pageable using Fluent Query API.
We now consider properly a sort value from a given Pageable. Previously, the sort Parameter was not applied from Pageable but from a previous sort(…) call. Also, we apply the given Sort to the resulting Pageable to ensure sort continuity. Closes #3762
1 parent b90ab18 commit 1649506

File tree

5 files changed

+107
-25
lines changed

5 files changed

+107
-25
lines changed

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

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ public FetchableFluentQuery<R> project(Collection<String> properties) {
147147
@Override
148148
public R oneValue() {
149149

150-
List<?> results = createSortedAndProjectedQuery() //
150+
List<?> results = createSortedAndProjectedQuery(this.sort) //
151151
.limit(2) // Never need more than 2 values
152152
.fetch();
153153

@@ -161,7 +161,7 @@ public R oneValue() {
161161
@Override
162162
public R firstValue() {
163163

164-
List<?> results = createSortedAndProjectedQuery() //
164+
List<?> results = createSortedAndProjectedQuery(this.sort) //
165165
.limit(1) // Never need more than 1 value
166166
.fetch();
167167

@@ -170,7 +170,11 @@ public R firstValue() {
170170

171171
@Override
172172
public List<R> all() {
173-
return convert(createSortedAndProjectedQuery().fetch());
173+
return all(this.sort);
174+
}
175+
176+
private List<R> all(Sort sort) {
177+
return convert(createSortedAndProjectedQuery(sort).fetch());
174178
}
175179

176180
@Override
@@ -184,13 +188,13 @@ public Window<R> scroll(ScrollPosition scrollPosition) {
184188

185189
@Override
186190
public Page<R> page(Pageable pageable) {
187-
return pageable.isUnpaged() ? new PageImpl<>(all()) : readPage(pageable);
191+
return pageable.isUnpaged() ? new PageImpl<>(all(pageable.getSortOr(this.sort))) : readPage(pageable);
188192
}
189193

190194
@Override
191195
public Stream<R> stream() {
192196

193-
return createSortedAndProjectedQuery() //
197+
return createSortedAndProjectedQuery(this.sort) //
194198
.stream() //
195199
.map(getConversionFunction());
196200
}
@@ -205,7 +209,7 @@ public boolean exists() {
205209
return existsOperation.apply(predicate);
206210
}
207211

208-
private AbstractJPAQuery<?, ?> createSortedAndProjectedQuery() {
212+
private AbstractJPAQuery<?, ?> createSortedAndProjectedQuery(Sort sort) {
209213

210214
AbstractJPAQuery<?, ?> query = finder.apply(sort);
211215
applyQuerySettings(this.returnedType, this.limit, query, null);
@@ -247,6 +251,7 @@ private void applyQuerySettings(ReturnedType returnedType, int limit, AbstractJP
247251

248252
private Page<R> readPage(Pageable pageable) {
249253

254+
Sort sort = pageable.getSortOr(this.sort);
250255
AbstractJPAQuery<?, ?> query = pagedFinder.apply(sort, pageable);
251256

252257
if (!properties.isEmpty()) {
@@ -255,7 +260,8 @@ private Page<R> readPage(Pageable pageable) {
255260

256261
List<R> paginatedResults = convert(query.fetch());
257262

258-
return PageableExecutionUtils.getPage(paginatedResults, pageable, () -> countOperation.apply(predicate));
263+
return PageableExecutionUtils.getPage(paginatedResults, withSort(pageable, sort),
264+
() -> countOperation.apply(predicate));
259265
}
260266

261267
private List<R> convert(List<?> resultList) {

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

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,17 @@ public SpecificationFluentQuery<R> sortBy(Sort sort) {
9393

9494
Assert.notNull(sort, "Sort must not be null");
9595

96-
return new FetchableFluentQueryBySpecification<>(spec, entityType, resultType, this.sort.and(sort), limit,
97-
properties, finder, scroll, countOperation, existsOperation, entityManager, projectionFactory);
96+
return getSorted(this.sort.and(sort));
97+
}
98+
99+
private FetchableFluentQueryBySpecification<S, R> getSorted(Sort sort) {
100+
101+
if (this.sort == sort) {
102+
return this;
103+
}
104+
105+
return new FetchableFluentQueryBySpecification<>(spec, entityType, resultType, sort, limit, properties, finder,
106+
scroll, countOperation, existsOperation, entityManager, projectionFactory);
98107
}
99108

100109
@Override
@@ -125,7 +134,7 @@ public SpecificationFluentQuery<R> project(Collection<String> properties) {
125134
@Override
126135
public R oneValue() {
127136

128-
List<?> results = createSortedAndProjectedQuery() //
137+
List<?> results = createSortedAndProjectedQuery(this.sort) //
129138
.setMaxResults(2) // Never need more than 2 values
130139
.getResultList();
131140

@@ -139,7 +148,7 @@ public R oneValue() {
139148
@Override
140149
public R firstValue() {
141150

142-
List<?> results = createSortedAndProjectedQuery() //
151+
List<?> results = createSortedAndProjectedQuery(this.sort) //
143152
.setMaxResults(1) // Never need more than 1 value
144153
.getResultList();
145154

@@ -148,7 +157,11 @@ public R firstValue() {
148157

149158
@Override
150159
public List<R> all() {
151-
return convert(createSortedAndProjectedQuery().getResultList());
160+
return all(this.sort);
161+
}
162+
163+
private List<R> all(Sort sort) {
164+
return convert(createSortedAndProjectedQuery(sort).getResultList());
152165
}
153166

154167
@Override
@@ -161,24 +174,25 @@ public Window<R> scroll(ScrollPosition scrollPosition) {
161174

162175
@Override
163176
public Slice<R> slice(Pageable pageable) {
164-
return pageable.isUnpaged() ? new PageImpl<>(all()) : readSlice(pageable);
177+
return pageable.isUnpaged() ? new PageImpl<>(all(pageable.getSortOr(this.sort))) : readSlice(pageable);
165178
}
166179

167180
@Override
168181
public Page<R> page(Pageable pageable) {
169-
return pageable.isUnpaged() ? new PageImpl<>(all()) : readPage(pageable, spec);
182+
return pageable.isUnpaged() ? new PageImpl<>(all(pageable.getSortOr(this.sort))) : readPage(pageable, spec);
170183
}
171184

172185
@Override
173186
@SuppressWarnings({ "rawtypes", "unchecked" })
174187
public Page<R> page(Pageable pageable, Specification<?> countSpec) {
175-
return pageable.isUnpaged() ? new PageImpl<>(all()) : readPage(pageable, (Specification) countSpec);
188+
return pageable.isUnpaged() ? new PageImpl<>(all(pageable.getSortOr(this.sort)))
189+
: readPage(pageable, (Specification) countSpec);
176190
}
177191

178192
@Override
179193
public Stream<R> stream() {
180194

181-
return createSortedAndProjectedQuery() //
195+
return createSortedAndProjectedQuery(this.sort) //
182196
.getResultStream() //
183197
.map(getConversionFunction());
184198
}
@@ -193,9 +207,9 @@ public boolean exists() {
193207
return existsOperation.apply(spec);
194208
}
195209

196-
private TypedQuery<S> createSortedAndProjectedQuery() {
210+
private TypedQuery<S> createSortedAndProjectedQuery(Sort sort) {
197211

198-
TypedQuery<S> query = finder.apply(this);
212+
TypedQuery<S> query = finder.apply(getSorted(sort));
199213

200214
if (!properties.isEmpty()) {
201215
query.setHint(EntityGraphFactory.HINT, EntityGraphFactory.create(entityManager, entityType, properties));
@@ -210,7 +224,7 @@ private TypedQuery<S> createSortedAndProjectedQuery() {
210224

211225
private Slice<R> readSlice(Pageable pageable) {
212226

213-
TypedQuery<S> pagedQuery = createSortedAndProjectedQuery();
227+
TypedQuery<S> pagedQuery = createSortedAndProjectedQuery(pageable.getSort());
214228

215229
if (pageable.isPaged()) {
216230
pagedQuery.setFirstResult(PageableUtils.getOffsetAsInteger(pageable));
@@ -230,7 +244,8 @@ private Slice<R> readSlice(Pageable pageable) {
230244

231245
private Page<R> readPage(Pageable pageable, @Nullable Specification<S> countSpec) {
232246

233-
TypedQuery<S> pagedQuery = createSortedAndProjectedQuery();
247+
Sort sort = pageable.getSortOr(this.sort);
248+
TypedQuery<S> pagedQuery = createSortedAndProjectedQuery(sort);
234249

235250
if (pageable.isPaged()) {
236251
pagedQuery.setFirstResult(PageableUtils.getOffsetAsInteger(pageable));
@@ -239,7 +254,8 @@ private Page<R> readPage(Pageable pageable, @Nullable Specification<S> countSpec
239254

240255
List<R> paginatedResults = convert(pagedQuery.getResultList());
241256

242-
return PageableExecutionUtils.getPage(paginatedResults, pageable, () -> countOperation.apply(countSpec));
257+
return PageableExecutionUtils.getPage(paginatedResults, withSort(pageable, sort),
258+
() -> countOperation.apply(countSpec));
243259
}
244260

245261
private List<R> convert(List<S> resultList) {

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import java.util.function.Function;
2323

2424
import org.springframework.core.convert.support.DefaultConversionService;
25+
import org.springframework.data.domain.PageRequest;
26+
import org.springframework.data.domain.Pageable;
2527
import org.springframework.data.domain.ScrollPosition;
2628
import org.springframework.data.domain.Sort;
2729
import org.springframework.data.jpa.repository.query.AbstractJpaQuery;
@@ -50,7 +52,7 @@ abstract class FluentQuerySupport<S, R> {
5052
protected final ProjectionFactory projectionFactory;
5153

5254
FluentQuerySupport(Class<R> resultType, Sort sort, int limit, @Nullable Collection<String> properties,
53-
Class<S> entityType, ProjectionFactory projectionFactory) {
55+
Class<S> entityType, ProjectionFactory projectionFactory) {
5456

5557
this.returnedType = ReturnedType.of(resultType, entityType, projectionFactory);
5658
this.resultType = resultType;
@@ -94,6 +96,15 @@ final Function<Object, R> getConversionFunction(Class<S> inputType, Class<R> tar
9496
return o -> DefaultConversionService.getSharedInstance().convert(o, targetType);
9597
}
9698

99+
Pageable withSort(Pageable pageable, Sort sort) {
100+
101+
if (pageable instanceof PageRequest pr && pageable.getSort() != sort) {
102+
return pr.withSort(sort);
103+
}
104+
105+
return pageable;
106+
}
107+
97108
interface ScrollQueryFactory<Q> {
98109
Q createQuery(FluentQuerySupport<?, ?> query, ScrollPosition scrollPosition);
99110
}

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

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2485,6 +2485,24 @@ void findByFluentExamplePage() {
24852485
assertThat(page1.getContent()).containsExactly(fourthUser);
24862486
}
24872487

2488+
@Test // GH-3762
2489+
void findByFluentExamplePageSortOverride() {
2490+
2491+
flushTestUsers();
2492+
2493+
User prototype = new User();
2494+
prototype.setFirstname("v");
2495+
2496+
Example<User> userProbe = of(prototype, matching().withIgnorePaths("age", "createdAt", "active")
2497+
.withMatcher("firstname", GenericPropertyMatcher::contains));
2498+
2499+
Page<User> page = repository.findBy(userProbe, //
2500+
q -> q.sortBy(Sort.by("firstname")).page(PageRequest.of(0, 2, Sort.by(DESC, "firstname"))));
2501+
2502+
assertThat(page.getContent()).containsExactly(fourthUser, firstUser);
2503+
assertThat(repository.findAll(page.nextPageable())).containsExactly(secondUser, thirdUser);
2504+
}
2505+
24882506
@Test // GH-2294
24892507
void findByFluentExampleWithInterfaceBasedProjection() {
24902508

@@ -2706,20 +2724,38 @@ void findByFluentSpecificationPage() {
27062724
assertThat(page1.getContent()).containsExactly(fourthUser);
27072725
}
27082726

2709-
@Test // GH-2274
2727+
@Test // GH-3762
2728+
void findByFluentSpecificationSortOverridePage() {
2729+
2730+
flushTestUsers();
2731+
2732+
Page<User> page = repository.findBy(userHasFirstnameLike("v"),
2733+
q -> q.sortBy(Sort.by("firstname")).page(PageRequest.of(0, 2, Sort.by(DESC, "firstname"))));
2734+
2735+
assertThat(page.getContent()).containsExactly(fourthUser, firstUser);
2736+
assertThat(repository.findAll(page.nextPageable())).containsExactly(secondUser, thirdUser);
2737+
2738+
Slice<User> slice = repository.findBy(userHasFirstnameLike("v"),
2739+
q -> q.sortBy(Sort.by("firstname")).slice(PageRequest.of(0, 2, Sort.by(DESC, "firstname"))));
2740+
2741+
assertThat(slice.getContent()).containsExactly(fourthUser, firstUser);
2742+
assertThat(repository.findAll(slice.nextPageable())).containsExactly(secondUser, thirdUser);
2743+
}
2744+
2745+
@Test // GH-2274, 3762
27102746
void findByFluentSpecificationSlice() {
27112747

27122748
flushTestUsers();
27132749

27142750
Slice<User> slice = repository.findBy(userHasFirstnameLike("v"),
2715-
q -> q.sortBy(Sort.by("firstname")).slice(PageRequest.of(0, 2)));
2751+
q -> q.sortBy(Sort.by(DESC, "firstname")).slice(PageRequest.of(0, 2, Sort.by("firstname"))));
27162752

27172753
assertThat(slice).isNotInstanceOf(Page.class);
27182754
assertThat(slice.getContent()).containsExactly(thirdUser, firstUser);
27192755
assertThat(slice.hasNext()).isTrue();
27202756

27212757
slice = repository.findBy(userHasFirstnameLike("v"),
2722-
q -> q.sortBy(Sort.by("firstname")).slice(PageRequest.of(0, 3)));
2758+
q -> q.sortBy(Sort.by("firstname")).slice(PageRequest.of(0, 3, Sort.by("firstname"))));
27232759

27242760
assertThat(slice).isNotInstanceOf(Page.class);
27252761
assertThat(slice).hasSize(3);

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,19 @@ void findByFluentPredicatePage() {
399399
assertThat(page1.getContent()).containsExactly(oliver);
400400
}
401401

402+
@Test // GH-3762
403+
void findByFluentPredicateSortOverridePage() {
404+
405+
Predicate predicate = user.firstname.contains("v");
406+
407+
Page<User> page = predicateExecutor.findBy(predicate,
408+
q -> q.sortBy(Sort.by("firstname")).page(PageRequest.of(0, 1, Sort.by(Direction.DESC, "firstname"))));
409+
410+
assertThat(page.getContent()).containsOnly(oliver);
411+
assertThat(predicateExecutor.findAll(predicate, page.nextPageable())).containsOnly(dave);
412+
413+
}
414+
402415
@Test // GH-2294
403416
void findByFluentPredicateWithInterfaceBasedProjection() {
404417

0 commit comments

Comments
 (0)