spring-projects / spring-data-rest

Simplifies building hypermedia-driven REST web services on top of Spring Data repositories
https://spring.io/projects/spring-data-rest
Apache License 2.0
916 stars 562 forks source link

It's very cumbersome to customize the defaultMethodArgumentResolvers defined in RepositoryRestMvcConfiguration. [DATAREST-1504] #1862

Open spring-projects-issues opened 4 years ago

spring-projects-issues commented 4 years ago

jvanheesch opened DATAREST-1504 and commented

This issue was discussed in https://gitter.im/spring-projects/spring-data.

It is really hard to customize the defaultMethodArgumentResolvers() defined in RepositoryRestMvcConfiguration. Multiple objects are instantiated with new in various places. In my case - I'd like to customize the behavior of SortTranslator, because it affects the following components:

As it stands, this is very cumbersome as SortTranslator is created in the constructor of JacksonMappingAwareSortTranslator


No further details from DATAREST-1504

spring-projects-issues commented 4 years ago

Oliver Drotbohm commented

Would you mind taking a step back and elaborate on what you're actually trying to achieve?

spring-projects-issues commented 4 years ago

jvanheesch commented

My problem is related to https://jira.spring.io/browse/DATAREST-976.

I currently have an angular frontend using an SDR backend. The frontend shows a table of books (/books?page=0&size=10&sort=title,desc&projection=summary). The table contains columns title and authorName (included in summary projection). The books can be sorted by title, and everything is working perfectly fine.

I'd now like to be able to sort the books by authorName. Spring Data REST currently does not support sorting a collection of books on book.author.name if book.author is exported as association resource. I figured I'd use a RepositoryRestController to manually handle this request. However, the Pageable argument is provided by MappingAwarePageableArgumentResolver, and its Sort no longer contains the user's requested author.name. If I could customize this SortTranslator, I could decide on a case by case basis whether or not it makes sense to support a certain sort order:

@Override
public Sort translateSort(Sort input, PersistentEntity<?, ?> rootEntity) {
    if (Book.class.equals(rootEntity.getType())) {
        List<Sort.Order> list = new ArrayList<>();
        input.iterator().forEachRemaining(list::add);
        if (list.size() == 1) {
            if (list.get(0).getProperty().equals("author.name")) {
                return input;
            }
        }
    }
    return super.translateSort(input, rootEntity);
}
spring-projects-issues commented 4 years ago

Oliver Drotbohm commented

That's an interesting arrangement. The reason for the cross-aggregate search not to work is that we cannot guarantee a backing store actually supports searching for that. E.g. in MongoDB, an aggregate reference is usually represented by a DBRef and you cannot search across those references but have to resort to application level joins. Or replicate the author's name into the Book aggregate.

I'd also argue that a client would naturally want to populate request parameters based on the field names it finds in the JSON documents. So in your case, the client would probably use authorName as value if that's what it gets presented through the rendering of the projection. The latter, especially projections using @Value unfortunately destroy the straight forward mapping to the underlying entity. That means, we need some mapping back to allow projection designers to declare which of the properties exposed map which property path on the target bean. I can imagine both an annotation based approach but also a programmatic one somewhere around ProjectionDefinitionConfiguration. Something along the lines of:

config.withSortMapping(BookSummary.class, BookSummary::getAuthorName, Sort.sort(Book.class).by(it -> it.getAuthor().getName()));

The very latter could also be provided a simple Sort created from a String.

WDYT?

spring-projects-issues commented 4 years ago

jvanheesch commented

My use case is indeed sorting on properties exported through a projection, and your approach would certainly enable that. I would prefer defining the Sort in the projection interface (e.g. using annotations) rather than using ProjectionDefinitionConfiguration. Ideally, I would not define the Sort property at all, and nested properties such as author.name (and their corresponding Sort) would be derived from the method name in the projection interface. Spring Data's query derivation mechanism is able to derive a correct query for findAllByAuthorName() in BookRepository, but SDR won't derive author.name from getAuthorName() in a projection interface (for Book), so I have to resort to @Value("#\{target.getAuthor().getName()\}").

I'd love to be able to sort on properties exported through a projection, but binding the Sort definition to a projection introduces some complexity. For example: should GET /books?projection=X&sort=someProperty,asc still return a sorted list in case someProperty is not present in projection X (e.g. someProperty is a property on Book, or on some other projection Y for which a Sort was defined)?

In addition, I think it would be useful if developers could still register a custom Sort without binding it to a projection property (although this may seem far-fetched). In theory, it's perfectly valid to support sorting by a certain property, without exposing the values of that property to the client. For example: items in a shopping cart are often sorted by timeAddedToShoppingCart (or creationTime of the shoppingCartItem), even though such timestamp is generally not shown in the UI. In this case, the timeAddedToShoppingCart property is not needed in the projection (and may even be unwanted). The sorting could still be properly supported if the shoppingCartItem collection resource link is sent with the sort param already expanded (/...?sort=timeAddedToShoppingCart). It's certainly not a great example, but I hope it gets my point across