LimitOffset pagination crashes Browseable API when limit=0

[wiresurfer]

We are using LimitOffsetPagination on all our endpoints, and noticed that sending limit=0 to the browsable api causes a crash. Getting json output (either from an xmlhttprequest or from format=json) does not crash.

Steps to reproduce

• Install LimitOffsetPagination as the default pagination
• Open a list (ours are ModelViewSet) in the browser
• Set the query string to ?limit=0&offset=20
• boom

Expected behavior

Should show the browsable api for the endpoint, with the following data:

{
"count":148,
"next":"https://api.co/page/?limit=0&offset=20",
"previous":"https://api.co/page/?limit=0&offset=20",
"results":[]
}

Linked DRF issue

This issue was reported and resolved in DRF a year ago.
encode/django-rest-framework#4079
This was resolved in >3.6.0 of DRF.

Notes

We use a RqlLimitOffset pagination in our project.
Double checking the verision of DRF, we are at 3.14.0
Django-Rql is at 4.4.0

[maxipavlovic]

Hey @wiresurfer, redirecting you to Jonathan.
@jonatrios could you, please, check it out.

[jonatrios]

Hi @wiresurfer

The issue that you are facing is not related to the issue #4079 of the DRF that you mentioned. limit=0 works perfectly fine if you use the drf built-in LimitOffsetPagination class.
The problem with same scenario but using the dj_rql.drf.paginations.RQLLimitOffsetPagination as pagination_class (global or view level), is that this redefines the paginate_queryset method, leading to something more related to issue #8761 that was fixed in PR 8764, and that it is planned to be part of version 3.15 that has not yet been released, you can check drf v3.15 released notes.

Now, going back to the issue, we can comment the following:
rest_framework.pagination.LimitOffsetPagination.get_limit set the strict=True when use the rest_framework.pagination._positive_int helper function (https://github.com/encode/django-rest-framework/blob/3.14.0/rest_framework/pagination.py#L436), while dj_rql set this parameter to False, this translate to different return value if do not have the PAGE_SIZE defined either in your custom paginator class or your project settings. Drf will return you None and dj_rql will return 0. This will not change completely the behaviour of the code, but it have some implications when it comes to the redefined paginate_queryset method:

1. For Drf get_limit return value being None, paginate_queryset will also return None: https://github.com/encode/django-rest-framework/blob/3.14.0/rest_framework/pagination.py#L385
2. For dj_rql get_limit return value being 0, paginate_queryset will return an empty list [] : https://github.com/cloudblue/django-rql/blob/4.4.0/dj_rql/drf/paginations.py#L48

And there you have the problem if you do not have the incoming request object already available for the paginator.LimitOffsetPagination assumes that for returning an empty list, first you need to have the request as paginator instance attribute setted. And this is not true for the RQLLimitOffsetPagination. So the idea of make the request consistently available It's something that also worried someone else. Thanks for reporting, we are going to take care of this as soon as we can.

While you wait for the fix to be public and you can upgrade your dependency, you can make workaround, writing a custom class and redefine the paginate_queryset, something like:

class MyPaginator(RQLLimitOffsetPagination):
    def paginate_queryset(self, queryset, request, view=None):
        self.request = request
        return super().paginate_queryset(queryset, request, view)

[wiresurfer]

@jonatrios thank you for the update.
In our codebase, we have reached the same findings. Indeed the strict=true difference and a custom paginator class did the trick.
Will wait for the public release and deprecate the custom paginator then.
Closing the issue from my side.

---

Thank you for the detailed writeup. I appreciate you taking the time to put the references down.