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

IndexError for CursorPagination when queryset changes #6504

Closed
6 tasks done
readevalprint opened this issue Mar 11, 2019 · 4 comments · Fixed by #6593
Closed
6 tasks done

IndexError for CursorPagination when queryset changes #6504

readevalprint opened this issue Mar 11, 2019 · 4 comments · Fixed by #6593

Comments

@readevalprint
Copy link
Contributor

Checklist

  • I have verified that that issue exists against the master branch of Django REST framework.
  • I have searched for similar issues in both open and closed tickets and cannot find a duplicate.
  • This is not a usage question. (Those should be directed to the discussion group instead.)
  • This cannot be dealt with as a third party library. (We prefer new functionality to be in the form of third party libraries where possible.)
  • I have reduced the issue to the simplest possible case.
  • I have included a failing test as a pull request. (If you are unable to do so we can still accept the issue.)

Steps to reproduce

  1. Enable CursorPagination
  2. Make enough items that there is pagination
  3. Go to page 2
  4. Delete all items within the shell
  5. Refresh the page
  6. See error

Expected behavior

To be honest I'm not sure, maybe an empty page.

Actual behavior

Traceback (most recent call last):
  File "/var/env/lib/python3.6/site-packages/django/contrib/staticfiles/handlers.py", line 65, in __call__
    return self.application(environ, start_response)
  File "/var/env/lib/python3.6/site-packages/django/core/handlers/wsgi.py", line 142, in __call__
    response = self.get_response(request)
  File "/var/env/lib/python3.6/site-packages/django/core/handlers/base.py", line 78, in get_response
    response = self._middleware_chain(request)
  File "/var/env/lib/python3.6/site-packages/django/core/handlers/exception.py", line 36, in inner
    response = response_for_exception(request, exc)
  File "/var/env/lib/python3.6/site-packages/django/core/handlers/exception.py", line 90, in response_for_exception
    response = handle_uncaught_exception(request, get_resolver(get_urlconf()), sys.exc_info())
  File "/var/env/lib/python3.6/site-packages/django/core/handlers/exception.py", line 125, in handle_uncaught_exception
    return debug.technical_500_response(request, *exc_info)
  File "/var/env/lib/python3.6/site-packages/django_extensions/management/technical_response.py", line 37, in null_technical_500_response
    six.reraise(exc_type, exc_value, tb)
  File "/var/env/lib/python3.6/site-packages/six.py", line 692, in reraise
    raise value.with_traceback(tb)
  File "/var/env/lib/python3.6/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/var/env/lib/python3.6/site-packages/django/core/handlers/base.py", line 126, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/var/env/lib/python3.6/site-packages/django/core/handlers/base.py", line 124, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/var/env/lib/python3.6/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
    return view_func(*args, **kwargs)
  File "/var/env/lib/python3.6/site-packages/rest_framework/viewsets.py", line 116, in view
    return self.dispatch(request, *args, **kwargs)
  File "/var/env/lib/python3.6/site-packages/rest_framework/views.py", line 495, in dispatch
    response = self.handle_exception(exc)
  File "/var/env/lib/python3.6/site-packages/rest_framework/views.py", line 455, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/var/env/lib/python3.6/site-packages/rest_framework/views.py", line 492, in dispatch
    response = handler(request, *args, **kwargs)
  File "/var/env/lib/python3.6/site-packages/rest_framework/mixins.py", line 45, in list
    return self.get_paginated_response(serializer.data)
  File "/var/env/lib/python3.6/site-packages/rest_framework/generics.py", line 180, in get_paginated_response
    return self.paginator.get_paginated_response(data)
  File "/var/env/lib/python3.6/site-packages/rest_framework/pagination.py", line 781, in get_paginated_response
    ('previous', self.get_previous_link()),
  File "/var/env/lib/python3.6/site-packages/rest_framework/pagination.py", line 643, in get_previous_link
    compare = self._get_position_from_instance(self.page[0], self.ordering)
IndexError: list index out of range
@ewjoachim
Copy link
Contributor

ewjoachim commented Apr 13, 2019

I'm going to try and have a go at it. If there's no news from me by April 15, consider I'm not on it anymore.

@ewjoachim
Copy link
Contributor

For the record I can't repoduce when following the exact steps of the ticket, but if I try to go back to page 1 then I have the same traceback.

@ewjoachim
Copy link
Contributor

ewjoachim commented Apr 13, 2019

Adding some info: the cursor pagination may have 3 parameters: position, offset and cursor. The bug is triggered when:

  • You're on a page that contains no objects anymore, because they were deleted (this works and returns an empty list)
  • The link to the previous page, even after reloading, is including a cursor with an offset and no position.
  • Following this link leads to a 500

This is because the self.page attribute is empty, no "unique" element is assumed to be in the page, the page is assumed to contain only element that have the same position, and thus the algorithm resolves to using a offset-based ordering.

The following comment:

# There were no unique positions in the page.

... encompasses the error. This assumption is wrong if the page is empty.

Fixing the logic and telling the algorithm to check if the page is empty and then set the position to previous_position/next_position is enough to get rid of this bug.

PR will follow.

ewjoachim added a commit to ewjoachim/django-rest-framework that referenced this issue Apr 14, 2019
ewjoachim added a commit to ewjoachim/django-rest-framework that referenced this issue Apr 14, 2019
ewjoachim added a commit to ewjoachim/django-rest-framework that referenced this issue Apr 14, 2019
Co-Authored-By: Tom Quinonero <tq@3yourmind.com>
ewjoachim added a commit to ewjoachim/django-rest-framework that referenced this issue Apr 14, 2019
)

Co-Authored-By: Tom Quinonero <tq@3yourmind.com>
ewjoachim added a commit to ewjoachim/django-rest-framework that referenced this issue Apr 16, 2019
Co-Authored-By: Tom Quinonero <tq@3yourmind.com>
ewjoachim added a commit to ewjoachim/django-rest-framework that referenced this issue Apr 16, 2019
)

Co-Authored-By: Tom Quinonero <tq@3yourmind.com>
tomchristie pushed a commit that referenced this issue May 20, 2019
…6593)

* Added regression tests (#6504)

Co-Authored-By: Tom Quinonero <tq@3yourmind.com>

* Fix CursorPagination when objects get deleted between calls (#6504)

Co-Authored-By: Tom Quinonero <tq@3yourmind.com>
@readevalprint
Copy link
Contributor Author

Thanks @ewjoachim and @tomchristie 👍

pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this issue Nov 17, 2020
…6504)  (encode#6593)

* Added regression tests (encode#6504)

Co-Authored-By: Tom Quinonero <tq@3yourmind.com>

* Fix CursorPagination when objects get deleted between calls (encode#6504)

Co-Authored-By: Tom Quinonero <tq@3yourmind.com>
sigvef pushed a commit to sigvef/django-rest-framework that referenced this issue Dec 3, 2022
…6504)  (encode#6593)

* Added regression tests (encode#6504)

Co-Authored-By: Tom Quinonero <tq@3yourmind.com>

* Fix CursorPagination when objects get deleted between calls (encode#6504)

Co-Authored-By: Tom Quinonero <tq@3yourmind.com>
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

Successfully merging a pull request may close this issue.

2 participants