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

Value of sort attribute in @SortDefault should parse the direction from the property #2810

Closed
onacit opened this issue Apr 2, 2023 · 4 comments
Assignees
Labels
in: web Integration with Spring MVC status: invalid An issue that we don't feel is valid

Comments

@onacit
Copy link

onacit commented Apr 2, 2023

When requested with (property,direction)+, the Sort seems not handle it.

with,

some?sort=a,asc&sort=b,desc

The PropertyPath class takes the whole value(e.g. a,asc) as a property name.

@onacit
Copy link
Author

onacit commented Apr 2, 2023

It seems that,

following codes goes wrong,

            @SortDefault(sort = {
                 "a,desc", "b,desc"
            })
            final Pageable pageable

while the following codes works

            @SortDefault(sort = "a", direction = Sort.Direction.DESC)
            @SortDefault(sort = "b", direction = Sort.Direction.DESC)
            final Pageable pageable

@schauder schauder changed the title Sort should parse the direction from the proprty Sort should parse the direction from the property Apr 3, 2023
@schauder schauder removed the status: waiting-for-triage An issue we've not yet triaged label Apr 3, 2023
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Apr 3, 2023
@odrotbohm
Copy link
Member

Can you elaborate why you expect the annotation attribute to be parsed? The Javadoc clearly states that sort takes, the property name, not a sort expression. The latter is an implementation detail of the way we build URIs. I'm inclined to close this as "works as designed" but would love to hear back from you first.

@odrotbohm odrotbohm 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 4, 2023
@odrotbohm odrotbohm changed the title Sort should parse the direction from the property Value of sort attribute in @SortDefault should parse the direction from the property Apr 4, 2023
@onacit
Copy link
Author

onacit commented Apr 4, 2023

https://docs.spring.io/spring-data/jpa/docs/current/reference/html/#core.web.basic.paging-and-sorting

sort Properties that should be sorted by in the format property,property(,ASC|DESC)(,IgnoreCase). The default sort direction is case-sensitive ascending. Use multiple sort parameters if you want to switch direction or case sensitivity — for example, ?sort=firstname&sort=lastname,asc&sort=city,ignorecase.

Maybe I'm confused, in which case, the @SortDefault#sort which is String[] might led me that way.

@SortDefault(
        sort = {"a", "b"},
        direction = Sort.Direction.DESC
)

Is above annotation is effectively equivalent to

@SortDefault(sort = "a", direction = Sort.Direction.DESC)
@SortDefault(sort = "b", direction = Sort.Direction.DESC)

?

Thanks.

@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 4, 2023
@odrotbohm
Copy link
Member

Yes, it is. You're referencing two different things here. One is the documentation of our default URI parameter value format and the other is the declaration of the annotation on the controller method parameter.

@odrotbohm odrotbohm added in: web Integration with Spring MVC status: invalid An issue that we don't feel is valid and removed status: feedback-provided Feedback has been provided labels Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Integration with Spring MVC status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

4 participants