spring-projects / spring-data-commons

Spring Data Commons. Interfaces and code shared between the various datastore specific implementations.
https://spring.io/projects/spring-data
Apache License 2.0
778 stars 675 forks source link

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

Open benzen opened 1 year ago

benzen commented 1 year ago

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.

mp911de commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.