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

[spring-projects-issues]

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 #190

1 votes, 3 watchers

[spring-projects-issues]

Kazuki Shimizu commented

I've submitted the PR

[spring-projects-issues]

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]

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]

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]

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.

[OrangeDog]

I implemented this using JEE validation:

    @Override
    public boolean isValid(Object value, ConstraintValidatorContext context) {
        if (value == null) {
            return true;
        }

        Sort sort;
        if (value instanceof Sort) {
            sort = (Sort) value;
        } else if (value instanceof Pageable) {
            sort = ((Pageable) value).getSort();
        } else {
            throw new UnexpectedTypeException(
                    "Expected org.springframework.data.domain.Sort or org.springframework.data.domain.Pageable"
            );
        }

        return sort.stream().allMatch(order -> properties.contains(order.getProperty()));
    }