Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DataTablesRepositoryImpl.findAll(...) sometimes not returns the requested rows #23

Closed
ppermezel-solucom opened this issue Aug 17, 2016 · 4 comments

Comments

@ppermezel-solucom
Copy link

This method does not returns enough rows in some cases:

public <R> DataTablesOutput<R> findAll(DataTablesInput input,
      Specification<T> additionalSpecification, Specification<T> preFilteringSpecification,
      Converter<T, R> converter)

For example: I use the jQuery DataTables with the Scroller plugin for displaying a list of 830 items. The scrolling zone has a dynamic height, thus a dynamic number of displayed rows. When I scroll down to the very bottom of the list, this request is sent to the server:

http://localhost:8080/myapp/users/results?draw=2&columns%5B0%5D.data=name&columns%5B0%5D.name=&columns%5B0%5D.searchable=true&columns%5B0%5D.orderable=true&columns%5B0%5D.search.value=&columns%5B0%5D.search.regex=false&order%5B0%5D.column=0&order%5B0%5D.dir=asc&start=749&length=81&search.value=&search.regex=false

in which we notice these parameters:

  • start=749
  • length=81

so the web client should receive the last 81 results, from indexes 749 to 829 (=749+81-1, because the first result's index is 0). Actually, it receives 81 results, but from indexes 729 to 810 ! Therefore, the displayed items are incorrect.

This bug seems to be located in the DataTablesUtils.getPageable method, at this row:

return new PageRequest(input.getStart() / input.getLength(), input.getLength(), sort);
@darrachequesne
Copy link
Owner

What's happening is that your request from 749 to 829 does not match an actual page, in Spring Data terms.

The (quite fancy) behaviour of the Scroller plugin (the start is not a multiple of the length) is indeed not correctly handled.

@ppermezel-solucom
Copy link
Author

ppermezel-solucom commented Aug 17, 2016

Yes this behavior is exactely the problem! :-(

However, even with a "normal" pagination, the start should not supposed to be a multiple of the length. For example: if you have a total number of 123 items and a page length of 10, the request for the last page could be:

  • start=120
  • length=3 (and not 10 since there's only 3 items on the last page because 123 % 10 = 3)

Fortunately, in the case of "normal" pagination with server-side processing, DataTables is always requesting the full length of the page, even if the last page contains less remaining items. :-)

At this point, I see 2 solutions to make spring-data-jpa-datatables working with the Scroller plugin:

  1. Client-side: use the ajax.data option to "correct" the start and length parameters so that start % length == 0 while avoiding loading the full set of data.
  2. Server-side: Adapt the DataTablesUtils.getPageable(...) method so that it instantiate a custom implementation of Pageable, instead of org.springframework.data.domain.PageRequest.
    For example:
package org.springframework.data.jpa.datatables.domain;

import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Sort;

public class DataTablesPageRequest implements Pageable {

    private int offset;
    private int limit;
    private Sort sort;

    /**
     * Constructor for DataTablesPageRequest.
     *
     * @param limit
     *            maximum number of results
     * @param offset
     *            index of first result
     * @param sort
     *            sort option
     */
    public DataTablesPageRequest(final int offset, final int limit, final Sort sort) {
        this.offset = offset;
        this.limit = limit;
        this.sort = sort;
    }

    @Override
    public int getPageSize() {
        return limit;
    }

    public void setLimit(final int limit) {
        this.limit = limit;
    }

    public int getLimit() {
        return limit;
    }

    public void setOffset(final int offset) {
        this.offset = offset;
    }

    @Override
    public int getOffset() {
        return offset;
    }

    public void setSort(final Sort sort) {
        this.sort = sort;
    }

    @Override
    public Sort getSort() {
        return sort;
    }

    @Override
    public Pageable next() {
        throw new RuntimeException("Method not available in this implementation.");
    }

    @Override
    public Pageable previousOrFirst() {
        throw new RuntimeException("Method not available in this implementation.");
    }

    @Override
    public Pageable first() {
        throw new RuntimeException("Method not available in this implementation.");
    }

    @Override
    public boolean hasPrevious() {
        throw new RuntimeException("Method not available in this implementation.");
    }

    @Override
    public int getPageNumber() {
        throw new RuntimeException("Method not available in this implementation.");
    }
}

(note that the 5 last overloaded methods should never be called in our context).

What do you think about that?

@darrachequesne
Copy link
Owner

Thanks for the detailed analysis! I've just published v2.5 with your suggested change, could you try it and tell me if I got something wrong?

@ppermezel-solucom
Copy link
Author

I've just tested the new version 2.5 and it works very well with the Scroller plugin. :-)
Congratulations for sharing your extension of the Spring Data JPA project and for your responsiveness in the implementation of this fix. Merci beaucoup ! ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants