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

Adjust Paging's sort in order to make it easy to build link based on it #2952

Open
benzen opened this issue Oct 12, 2023 · 4 comments
Open
Assignees
Labels
status: waiting-for-triage An issue we've not yet triaged

Comments

@benzen
Copy link

benzen commented Oct 12, 2023

I'm trying to write link like this

/resources?page=11&size=100&sort=name,asc.

var pageRessources = ressourceRepository.findAll(pageable);

    model.addAllAttributes(
        Map.of(
            "pageResources", pageResources,
            "pageable", pageable
            ));

    return "resource/list";
  }

And my thymleaf templates looks like this

<a
              class="item icon"
              th:href="@{/ressoures(page=1, size=${pageable.pageSize}, sort=${pageable.sort})}"
            >
              <i class="ui icon angle double left"></i>
              &nbsp;
            </a>
            

My issue is that this template, will produce the following link

/ressources?page=1&size=100&sort=name:ASC

But this is not handeled correctly by spring mvc. I could not found a way to easely build the given link with the current API.

So I ended up writing this

var currentSort = pageable.getSort().stream().map((s) -> "%s,%s".formatted(s.getProperty(), s.getDirection())).findFirst().orElse("");

Maybe the best solution is not in spring-data-core but in spring mvc. Or maybe there is a solution that I wasnt able to find.

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

mp911de commented Oct 13, 2023

Using Sort.toString() is by no means intended to render something you can use to parse it back via PageableHandlerMethodArgumentResolver or SortHandlerMethodArgumentResolver.

I'm not fully sure we have an utility to render sort query string options. Paging @odrotbohm

@odrotbohm
Copy link
Member

odrotbohm commented Oct 13, 2023

Mark is right. That said, given the popularity of Thymeleaf, I wonder if it makes sense to investigate options how we can make such link creation easier.

If in doubt, I would start with a dedicated model object in application code that would use the Spring MVC / HATEOAS APIs. Those allow creating full links by pointing at controller methods to use the rendered URIs in the template instead of constructing the URI in the template in the first place.

@benzen
Copy link
Author

benzen commented Oct 13, 2023

I've have a different opinion on this.
Since thoses links are only relevant to the template itself, I find it more natural to build it inside template.
Also, Thymleaf, and others have nice feature to build link into your app.

But that's not the point to me. My point is that, getting informations (page size and current page) from pageable is easy.
But getting sort, direction, is way harder than it needs to be. Granted it allow to sort on multiple column at the same time, which is great.

Maybe from sort, a method to get easy access sort and sort order would help.

Maybe somthing along thoses lines:

pageable.getSort().first().getProperty()
pageable.getSort().first().getDirection()

First would give the first sort or unsorted.
This would keep the full capacity of the current architecture and make the easy case easier

@odrotbohm
Copy link
Member

odrotbohm commented Oct 13, 2023

There's been a Spring Data-specific Thymeleaf dialect once. It looks pretty dated, but I wonder if we could / should provide something like this ourselves. EDIT: It looks like I oversaw the latest releases. I'd just recommend to use that then.

Btw. I am not arguing that an application-level model should be the ultimate solution. All I am saying is that there is already an API in place to create links to controller methods in a type-safe way. I'd recommend using those over concatenating arbitrary strings in HTML snippets any time, until we find a better solution. That said, changing Sort etc. is not that.

Sort, Page and Pageable are domain types. They cannot and must not care about any specific representation that could be useful in some context. Otherwise, we'd overload them with responsibilities. The translation into particular technologies needs to be handled by dedicated adapters, especially as there's symmetry involved (what's rendered by the adapter needs to be parsable by the adapter).

Finally, I don't buy the “this should be in the template” argument as the template is not what governs the URI structure, the controller method is. That's why it makes sense to keep the creation of links pointing to those close to them, and also type safe as you would want to realize your link creation breaks in case you change the signature of the controller method.

@mp911de mp911de added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged status: invalid An issue that we don't feel is valid labels Oct 16, 2023
@mp911de mp911de self-assigned this Oct 16, 2023
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

No branches or pull requests

4 participants