Spring Cloud Openfeign Pageable sort params cannot be encoded correctly

Hi all, I have also met this issue, the suggested workaround by @OlgaMaciaszek is useful in the single sort property situation, but not gonna work in multiple sort properties with directions.

For example: we have two sort properties createdAt and startDate with sort direction DESC. The query string by default looks like /api/v1/xxx/?sort=createdAt&sort=DESC&sort=startDate&sort=DESC, while it may look like /api/v1/xxx/?sort=createdAt,DESC,startDate,DESC when @CollectionFormat(feign.CollectionFormat.CSV) annotation is used. Apparently, neither of the query string is correct.

We found a workaround for this case. We reimplement the PageableSpringEncoder like this:

    public class CustomizedPageableSpringEncoder implements Encoder {

        private final Encoder delegate;

        /**
         * Page index parameter name.
         */
        private static final String PAGE_PARAMETER = "page";

        /**
         * Page size parameter name.
         */
        private static final String SIZE_PARAMETER = "size";

        /**
         * Sort parameter name.
         */
        private static final String SORT_PARAMETER = "sort";

        /**
         * Creates a new PageableSpringEncoder with the given delegate for fallback. If no
         * delegate is provided and this encoder cant handle the request, an EncodeException
         * is thrown.
         *
         * @param delegate The optional delegate.
         */
        public CustomizedPageableSpringEncoder(Encoder delegate) {
            this.delegate = delegate;
        }

        @Override
        public void encode(Object object, Type bodyType, RequestTemplate template) throws EncodeException {
            if (supports(object)) {
                if (object instanceof Pageable) {
                    Pageable pageable = (Pageable) object;

                    if (pageable.isPaged()) {
                        template.query(PAGE_PARAMETER, pageable.getPageNumber() + "");
                        template.query(SIZE_PARAMETER, pageable.getPageSize() + "");
                    }

                    applySort(template, pageable.getSort());
                } else if (object instanceof Sort) {
                    Sort sort = (Sort) object;
                    applySort(template, sort);
                }
            } else {
                if (delegate != null) {
                    delegate.encode(object, bodyType, template);
                } else {
                    throw new EncodeException(
                            "PageableSpringEncoder does not support the given object "
                                    + object.getClass()
                                    + " and no delegate was provided for fallback!");
                }
            }
        }

        private void applySort(RequestTemplate template, Sort sort) {
            Collection<String> existingSorts = template.queries().get(SORT_PARAMETER);
            List<String> sortQueries = existingSorts != null ? new ArrayList<>(existingSorts)
                    : new ArrayList<>();
            for (Sort.Order order : sort) {
                sortQueries.add(order.getProperty() + "%2C" + order.getDirection());
            }
            if (!sortQueries.isEmpty()) {
                template.query(SORT_PARAMETER, sortQueries);
            }
        }
    }

    protected boolean supports(Object object) {
        return object instanceof Pageable || object instanceof Sort;
    }
}

The only difference is to separate the sort property and sort direction by %2C instead of , so that the sort pair would not be splitted. (see QueryTemplate.java in feign-core)

I'd like to ask if there is any concern for this workaround, or if there is any better solution for this case. Thanks in advance.

Originally posted by @TokenJan in https://github.com/spring-cloud/spring-cloud-openfeign/issues/392#issuecomment-737199814

Comment From: OlgaMaciaszek

Thanks, @TokenJan ; would you like to provide a Pull Request?

Comment From: o2-mesmer

While the above works for Pageable params in GET calls, unfortunately it doesn't help for Pageables in POST calls.

As per my understanding (and see also this issue), to have the pageable as query params in a POST, one has to annotate it with @SpringQueryMap. Here, the fix shown by @TokenJan doesn't apply, since the conversion happens before the encoder has a chance to work.

A solution, then, is to create a custom QueryMapEncoder (and link it through the FeignConfiguration) like so:

import feign.querymap.BeanQueryMapEncoder;
import lombok.extern.slf4j.Slf4j;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;

import java.util.HashMap;
import java.util.Map;
import java.util.stream.Collectors;

public class PageableQueryMapEncoder
    extends BeanQueryMapEncoder
{
    @Override
    public Map<String, Object> encode(Object object) {
        if(object instanceof Pageable){
            return appendTo((Pageable) object, new HashMap<>());
        }
        else if(object instanceof Sort){
            return appendTo((Sort) object, new HashMap<>());
        } else {
            return super.encode(object);
        }
    }

    private Map<String, Object> appendTo(Pageable pageable, Map<String, Object> queryMap){
        if(pageable.isPaged()){
            queryMap.put("page", pageable.getPageNumber());
            queryMap.put("size", pageable.getPageSize());
        }

        if(pageable.getSort().isSorted()){
            return appendTo(pageable.getSort(), queryMap);
        } else {
            return queryMap;
        }
    }

    private Map<String, Object> appendTo(Sort sort, Map<String, Object> queryMap){
        var encoded = sort.stream()
            .map(_o -> _o.getProperty() + "%2C" + _o.getDirection())
            .collect(Collectors.toList());

        queryMap.put("sort", encoded);

        return queryMap;
    }
}

I love Feign and all, but boy, is it tedious. :-/