Skip to content

Commit baca9e1

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 412e5ee commit baca9e1

File tree

5 files changed

+91
-20
lines changed

5 files changed

+91
-20
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
@@ -132,7 +132,7 @@ public FetchableFluentQuery<R> project(Collection<String> properties) {
132132
@Override
133133
public R oneValue() {
134134

135-
List<?> results = createSortedAndProjectedQuery() //
135+
List<?> results = createSortedAndProjectedQuery(this.sort) //
136136
.limit(2) // Never need more than 2 values
137137
.fetch();
138138

@@ -146,7 +146,7 @@ public R oneValue() {
146146
@Override
147147
public R firstValue() {
148148

149-
List<?> results = createSortedAndProjectedQuery() //
149+
List<?> results = createSortedAndProjectedQuery(this.sort) //
150150
.limit(1) // Never need more than 1 value
151151
.fetch();
152152

@@ -155,7 +155,11 @@ public R firstValue() {
155155

156156
@Override
157157
public List<R> all() {
158-
return convert(createSortedAndProjectedQuery().fetch());
158+
return all(this.sort);
159+
}
160+
161+
private List<R> all(Sort sort) {
162+
return convert(createSortedAndProjectedQuery(sort).fetch());
159163
}
160164

161165
@Override
@@ -168,13 +172,13 @@ public Window<R> scroll(ScrollPosition scrollPosition) {
168172

169173
@Override
170174
public Page<R> page(Pageable pageable) {
171-
return pageable.isUnpaged() ? new PageImpl<>(all()) : readPage(pageable);
175+
return pageable.isUnpaged() ? new PageImpl<>(all(pageable.getSortOr(this.sort))) : readPage(pageable);
172176
}
173177

174178
@Override
175179
public Stream<R> stream() {
176180

177-
return createSortedAndProjectedQuery() //
181+
return createSortedAndProjectedQuery(this.sort) //
178182
.stream() //
179183
.map(getConversionFunction());
180184
}
@@ -189,7 +193,7 @@ public boolean exists() {
189193
return existsOperation.apply(predicate);
190194
}
191195

192-
private AbstractJPAQuery<?, ?> createSortedAndProjectedQuery() {
196+
private AbstractJPAQuery<?, ?> createSortedAndProjectedQuery(Sort sort) {
193197

194198
AbstractJPAQuery<?, ?> query = finder.apply(sort);
195199

@@ -206,6 +210,7 @@ public boolean exists() {
206210

207211
private Page<R> readPage(Pageable pageable) {
208212

213+
Sort sort = pageable.getSortOr(this.sort);
209214
AbstractJPAQuery<?, ?> query = pagedFinder.apply(sort, pageable);
210215

211216
if (!properties.isEmpty()) {
@@ -214,7 +219,8 @@ private Page<R> readPage(Pageable pageable) {
214219

215220
List<R> paginatedResults = convert(query.fetch());
216221

217-
return PageableExecutionUtils.getPage(paginatedResults, pageable, () -> countOperation.apply(predicate));
222+
return PageableExecutionUtils.getPage(paginatedResults, withSort(pageable, sort),
223+
() -> countOperation.apply(predicate));
218224
}
219225

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

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

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -89,17 +89,23 @@ public FetchableFluentQuery<R> sortBy(Sort sort) {
8989

9090
Assert.notNull(sort, "Sort must not be null");
9191

92-
return new FetchableFluentQueryBySpecification<>(spec, entityType, resultType, this.sort.and(sort), limit,
93-
properties, finder, scroll, countOperation, existsOperation, entityManager, projectionFactory);
92+
Sort sort1 = this.sort.and(sort);
93+
94+
if (this.sort == sort1) {
95+
return this;
96+
}
97+
98+
return new FetchableFluentQueryBySpecification<>(spec, entityType, resultType, sort1, limit, properties, finder,
99+
scroll, countOperation, existsOperation, entityManager, projectionFactory);
94100
}
95101

96102
@Override
97103
public FetchableFluentQuery<R> limit(int limit) {
98104

99105
Assert.isTrue(limit >= 0, "Limit must not be negative");
100106

101-
return new FetchableFluentQueryBySpecification<>(spec, entityType, resultType, sort, limit,
102-
properties, finder, scroll, countOperation, existsOperation, entityManager, projectionFactory);
107+
return new FetchableFluentQueryBySpecification<>(spec, entityType, resultType, sort, limit, properties, finder,
108+
scroll, countOperation, existsOperation, entityManager, projectionFactory);
103109
}
104110

105111
@Override
@@ -124,7 +130,7 @@ public FetchableFluentQuery<R> project(Collection<String> properties) {
124130
@Override
125131
public R oneValue() {
126132

127-
List<?> results = createSortedAndProjectedQuery() //
133+
List<?> results = createSortedAndProjectedQuery(this.sort) //
128134
.setMaxResults(2) // Never need more than 2 values
129135
.getResultList();
130136

@@ -138,7 +144,7 @@ public R oneValue() {
138144
@Override
139145
public R firstValue() {
140146

141-
List<?> results = createSortedAndProjectedQuery() //
147+
List<?> results = createSortedAndProjectedQuery(this.sort) //
142148
.setMaxResults(1) // Never need more than 1 value
143149
.getResultList();
144150

@@ -147,7 +153,11 @@ public R firstValue() {
147153

148154
@Override
149155
public List<R> all() {
150-
return convert(createSortedAndProjectedQuery().getResultList());
156+
return all(this.sort);
157+
}
158+
159+
private List<R> all(Sort sort) {
160+
return convert(createSortedAndProjectedQuery(sort).getResultList());
151161
}
152162

153163
@Override
@@ -160,13 +170,13 @@ public Window<R> scroll(ScrollPosition scrollPosition) {
160170

161171
@Override
162172
public Page<R> page(Pageable pageable) {
163-
return pageable.isUnpaged() ? new PageImpl<>(all()) : readPage(pageable);
173+
return pageable.isUnpaged() ? new PageImpl<>(all(pageable.getSortOr(this.sort))) : readPage(pageable);
164174
}
165175

166176
@Override
167177
public Stream<R> stream() {
168178

169-
return createSortedAndProjectedQuery() //
179+
return createSortedAndProjectedQuery(this.sort) //
170180
.getResultStream() //
171181
.map(getConversionFunction());
172182
}
@@ -181,7 +191,7 @@ public boolean exists() {
181191
return existsOperation.apply(spec);
182192
}
183193

184-
private TypedQuery<S> createSortedAndProjectedQuery() {
194+
private TypedQuery<S> createSortedAndProjectedQuery(Sort sort) {
185195

186196
TypedQuery<S> query = finder.apply(sort);
187197

@@ -198,7 +208,8 @@ private TypedQuery<S> createSortedAndProjectedQuery() {
198208

199209
private Page<R> readPage(Pageable pageable) {
200210

201-
TypedQuery<S> pagedQuery = createSortedAndProjectedQuery();
211+
Sort sort = pageable.getSortOr(this.sort);
212+
TypedQuery<S> pagedQuery = createSortedAndProjectedQuery(sort);
202213

203214
if (pageable.isPaged()) {
204215
pagedQuery.setFirstResult(PageableUtils.getOffsetAsInteger(pageable));
@@ -207,7 +218,7 @@ private Page<R> readPage(Pageable pageable) {
207218

208219
List<R> paginatedResults = convert(pagedQuery.getResultList());
209220

210-
return PageableExecutionUtils.getPage(paginatedResults, pageable, () -> countOperation.apply(spec));
221+
return PageableExecutionUtils.getPage(paginatedResults, withSort(pageable, sort), () -> countOperation.apply(spec));
211222
}
212223

213224
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
@@ -24,6 +24,8 @@
2424
import java.util.function.Function;
2525

2626
import org.springframework.core.convert.support.DefaultConversionService;
27+
import org.springframework.data.domain.PageRequest;
28+
import org.springframework.data.domain.Pageable;
2729
import org.springframework.data.domain.ScrollPosition;
2830
import org.springframework.data.domain.Sort;
2931
import org.springframework.data.projection.ProjectionFactory;
@@ -49,7 +51,7 @@ abstract class FluentQuerySupport<S, R> {
4951
protected final ProjectionFactory projectionFactory;
5052

5153
FluentQuerySupport(Class<R> resultType, Sort sort, int limit, @Nullable Collection<String> properties,
52-
Class<S> entityType, ProjectionFactory projectionFactory) {
54+
Class<S> entityType, ProjectionFactory projectionFactory) {
5355

5456
this.resultType = resultType;
5557
this.sort = sort;
@@ -87,6 +89,15 @@ final Function<Object, R> getConversionFunction(Class<S> inputType, Class<R> tar
8789
return o -> DefaultConversionService.getSharedInstance().convert(o, targetType);
8890
}
8991

92+
Pageable withSort(Pageable pageable, Sort sort) {
93+
94+
if (pageable instanceof PageRequest pr && pageable.getSort() != sort) {
95+
return pr.withSort(sort);
96+
}
97+
98+
return pageable;
99+
}
100+
90101
interface ScrollQueryFactory {
91102
Query createQuery(Sort sort, ScrollPosition scrollPosition);
92103
}

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2434,6 +2434,24 @@ void findByFluentExamplePage() {
24342434
assertThat(page1.getContent()).containsExactly(fourthUser);
24352435
}
24362436

2437+
@Test // GH-3762
2438+
void findByFluentExamplePageSortOverride() {
2439+
2440+
flushTestUsers();
2441+
2442+
User prototype = new User();
2443+
prototype.setFirstname("v");
2444+
2445+
Example<User> userProbe = of(prototype, matching().withIgnorePaths("age", "createdAt", "active")
2446+
.withMatcher("firstname", GenericPropertyMatcher::contains));
2447+
2448+
Page<User> page = repository.findBy(userProbe, //
2449+
q -> q.sortBy(Sort.by("firstname")).page(PageRequest.of(0, 2, Sort.by(DESC, "firstname"))));
2450+
2451+
assertThat(page.getContent()).containsExactly(fourthUser, firstUser);
2452+
assertThat(repository.findAll(page.nextPageable())).containsExactly(secondUser, thirdUser);
2453+
}
2454+
24372455
@Test // GH-2294
24382456
void findByFluentExampleWithInterfaceBasedProjection() {
24392457

@@ -2689,6 +2707,18 @@ void findByFluentSpecificationPage() {
26892707
assertThat(page1.getContent()).containsExactly(fourthUser);
26902708
}
26912709

2710+
@Test // GH-3762
2711+
void findByFluentSpecificationSortOverridePage() {
2712+
2713+
flushTestUsers();
2714+
2715+
Page<User> page = repository.findBy(userHasFirstnameLike("v"),
2716+
q -> q.sortBy(Sort.by("firstname")).page(PageRequest.of(0, 2, Sort.by(DESC, "firstname"))));
2717+
2718+
assertThat(page.getContent()).containsExactly(fourthUser, firstUser);
2719+
assertThat(repository.findAll(page.nextPageable())).containsExactly(secondUser, thirdUser);
2720+
}
2721+
26922722
@Test // GH-2274
26932723
void findByFluentSpecificationWithInterfaceBasedProjection() {
26942724

‎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)