commercetools-jvm-sdk icon indicating copy to clipboard operation
commercetools-jvm-sdk copied to clipboard

QueryAllImpl uses offset-based pagination

Open heshamMassoud opened this issue 7 years ago • 1 comments

QueryAllImpl uses an offset based pagination for fetching all products which is against what is recommended by the docs: http://dev.commercetools.com/http-api.html#offset

This offset-based pagination does not work with resources more than 100,000. Since the offset value cannot be more than 100,000.

Malformed parameter: offset: The offset parameter must be in the range of [0..100000]"

More on this here and here.

Proposed Solution

Use the solution proposed in the docs (limit based pagination with sorted ids). We already applied the solution on a custom queryAll used in the sync: https://github.com/commercetools/commercetools-sync-java/pull/211

However, to do this solution fetching all pages won't be done in parallel. As it used to be, but rather sequential since we need to use information (last resource id) in the result of every page to be able to get the next page.

Which means that it could also be better to be an option to be chosen by the user whether they want to fetchAll sequentially (for resources greater than 100,000) or in parallel otherwise.

Would be happy to provide a PR for it here but I guess we need to first discuss the final solution first.

heshamMassoud avatar Dec 15 '17 11:12 heshamMassoud

@heshamMassoud I would suggest that @acbeni takes a look at your code and proposal. And then we can discuss this further with you.

katmatt avatar Jan 19 '18 09:01 katmatt