Skip to content

Missing Pageable Sort interpretation #379

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

Closed
NiklasLoechte opened this issue Apr 11, 2023 · 3 comments
Closed

Missing Pageable Sort interpretation #379

NiklasLoechte opened this issue Apr 11, 2023 · 3 comments
Assignees
Labels
status: feedback-provided Feedback has been provided type: enhancement A general enhancement

Comments

@NiklasLoechte
Copy link

Currently you cannot sort by property only the sort direction is interpretated as the revision number.

AuditOrder sorting = RevisionSort.getRevisionDirection(pageable.getSort()).isDescending() //

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 11, 2023
@schauder
Copy link
Contributor

While it doesn't mean it has to stay this way, this seems to be intentional and somewhat in accordance to the relevant JavaDoc on RevisionRepository

Returns a Page of revisions for the entity with the given id. Note, that it's not guaranteed that implementations have to support sorting by all properties.

I guess the original idea was that sorting revisions by anything but timeline is of limited use.

Could you explain your use case of sorting by something else but the revision number?

@schauder schauder added status: waiting-for-feedback We need additional information before we can continue and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 17, 2023
@NiklasLoechte
Copy link
Author

NiklasLoechte commented Apr 17, 2023

Thank you very much for your answer. We have a distributed service that performs update operations for entities and updates a modified timestamp each time. We have encountered a problem when we use a cached sequence with an increment of 50 (to increase performance for large amounts of data). When multiple instances make updates to an entity, the temporal order is not guaranteed when using such a cache, so sorting by the modified timestamp of the revisioned entities seemed to be straight forward.
For now, overriding the current implementation of the repository with something like that did the job.

    var orderMapped = pageable.getSort().stream().findFirst().map(order ->
            order.getDirection().isAscending() ?
                AuditEntity.property(order.getProperty()).asc() :
                AuditEntity.property(order.getProperty()).desc())
        .orElse(AuditEntity.revisionNumber().asc());

But perhapse this can be adapted to enhance this great project.
If you think that this would be a benefit let me know and I could provide a pull request.
Thank you in advance.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Apr 17, 2023
@schauder
Copy link
Contributor

Yes, that does make sense and sounds like an improvement. A PR would be appreciated.

@schauder schauder added the type: enhancement A general enhancement label Apr 17, 2023
schauder pushed a commit that referenced this issue Apr 18, 2023
schauder added a commit that referenced this issue Apr 18, 2023
Support ordering by more than a single property.
Fixed formatting.
Added `@uthor` annotations.

See #379
Original Pull Request #381
@schauder schauder added this to the 2.7.12 (2021.2.12) milestone Apr 18, 2023
gregturn pushed a commit to spring-projects/spring-data-jpa that referenced this issue Apr 18, 2023
trayanus1026 pushed a commit to trayanus1026/spring-data-jpa-java that referenced this issue Aug 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants