Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FluentQuery.page(Pagable pagable) does not consider Pageable.sort #3762

Closed
Mrakobec opened this issue Jan 24, 2025 · 4 comments
Closed

FluentQuery.page(Pagable pagable) does not consider Pageable.sort #3762

Mrakobec opened this issue Jan 24, 2025 · 4 comments
Assignees
Labels
type: bug A general bug

Comments

@Mrakobec
Copy link

Mrakobec commented Jan 24, 2025

it possible to implement the method fluentQuery.page(Pageble pagable) for automatically call sortBy(Sort sort) if pagable.sort is not null?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 24, 2025
@mp911de
Copy link
Member

mp911de commented Jan 24, 2025

As per our Javadoc:

pageable - the pageable to request a paged result, can be Pageable.unpaged(), must not be null. The given Pageable will override any previously specified sort if the Sort object is not Sort.isUnsorted(). Any potentially specified limit(int) will be overridden by Pageable.getPageSize().

So we expect that Pageable.sort takes precedence over any previously configured Sort if Pageable is sorted.

Care to elaborate what Spring Data module and which API (Querydsl, Specifications, Query by Example) you're using?

@mp911de mp911de added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 27, 2025
@mp911de mp911de self-assigned this Jan 27, 2025
@mp911de
Copy link
Member

mp911de commented Feb 5, 2025

We need to strengthen our Sort override. With the original Pageable used for subsequent pagination, we should not apply a previous Sort set on the fluent query as subsequent Page requests would use Pageable.sort when using Page.nextPageable().

@mp911de mp911de transferred this issue from spring-projects/spring-data-commons Feb 5, 2025
@mp911de mp911de changed the title FluentQuery when call page(Pagable pagable) automatically call sortBy(Sort sort) if Pageble sort is not null FluentQuery.page(Pagable pagable) does not consider Pageable.sort Feb 5, 2025
@mp911de mp911de added this to the 3.4.3 (2024.1.3) milestone Feb 5, 2025
mp911de added a commit that referenced this issue Feb 5, 2025
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
mp911de added a commit that referenced this issue Feb 5, 2025
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
@mp911de mp911de closed this as completed in 1649506 Feb 5, 2025
@Mrakobec
Copy link
Author

Mrakobec commented Feb 6, 2025

Amazing! Thank you a lot!
Am I correct in seeing that I can specify only .page() for <S extends T, R> R findBy(Specification<T> spec, Function<FluentQuery.FetchableFluentQuery<S>, R> queryFunction), and the sorting will be applied? Additionally, if there was a .sortBy() specified, it will be overridden if sorting is present in the pageRequest?

Examples:

  1. Explicitly specifying .sortBy() before .page():

    public Page<ExampleEntity> findAll(Specification<ExampleEntity> filters, PageRequest pageRequest) {
        return voucherRepository.findBy(filters, fetchable ->
                fetchable.project("someAttribute")
                        .sortBy(pageRequest.getSort()) 
                        .page(pageRequest));
    }
  2. Only specifying .page() without .sortBy():

    public Page<ExampleEntity> findAll(Specification<ExampleEntity> filters, PageRequest pageRequest) {
        return voucherRepository.findBy(filters, fetchable ->
                fetchable.project("someAttribute")
                        .page(pageRequest));
    }

@mp911de
Copy link
Member

mp911de commented Feb 6, 2025

Yes, that's right. We entirely override any previous sortBy(…) once a Pageable parameter is given.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants