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
770 stars 670 forks source link

Add annotation that specify allowed sort properties [DATACMNS-966] #1422

Open spring-projects-issues opened 7 years ago

spring-projects-issues commented 7 years ago

Kazuki Shimizu opened DATACMNS-966 and commented

I'll suggest to add annotation that specify allowed sort properties to limit an injection as follow:

@Documented
@Target(ElementType.PARAMETER)
@Retention(RetentionPolicy.RUNTIME)
public @interface AllowedSortProperties {
    String[] value();
}
@RequestMapping("/accounts")
@RestController
public class AccountsRestController {
    @GetMapping
    public Page<Account> findPage(@AllowedSortProperties({"id", "name"}) Pageable pageable) {
        // ...
    }
}

When an invalid property is detected, i think better it is ignore from sort property. What do you think for this suggestion ? I will submit pull request at later.

Thanks.


Affects: 1.12.6 (Hopper SR6)

Referenced from: pull request https://github.com/spring-projects/spring-data-commons/pull/190

1 votes, 3 watchers

spring-projects-issues commented 7 years ago

Kazuki Shimizu commented

I've submitted the PR

spring-projects-issues commented 7 years ago

Oliver Drotbohm commented

I like the idea, I just wonder whether an annotation is a good idea here as I guess the list of properties could become a bit longer. Wondering whether some API on Sort like ….whitelist(String... properties) could be a better alternative. We could even expose a method to take a (lambda style) callback so that one could e.g. easily inspect the type that the sort is supposed to be applied to eventually etc

spring-projects-issues commented 7 years ago

Marcel Overdijk commented

I like the idea of allowed sort properties as well.

Another thing that I would be interested in is to have aliases for sort fields.

In our api we use snake case and I want users to provide something like sort=full_name;desc which in fact is an alias for the fullName property. It's not just converting snake to lower camel case as sometimes the aliases are completely different.

spring-projects-issues commented 7 years ago

Marcel Overdijk commented

I'm using the @QuesydslPredicate as well so I'm looking for something like:

@RequestMapping(method = GET, path = "/books", produces = HAL_JSON_VALUE)
public ResponseEntity<?> list(
        @QuerydslPredicate(root = Book.class, bindings = BookPredicateBindings.class) Predicate predicate,
        @PageableDefault(sort = "full_name", direction = Sort.Direction.ASC) @SortBindings(bindings = BookSortBindings.class) final Pageable pageable,
        final PagedResourcesAssembler<Book> pagedResourcesAssembler) {

The @QuerydslPredicate already supports a bindings class and I'm thinking about introducing a similar @SortBindings in my project (have to come up with a better name). This @SortBindings then points to a binds class which would provide an API to whitelist, blacklist and alias properties.

What do you think?

spring-projects-issues commented 3 years ago

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.