diff --git a/config/settings/base.py b/config/settings/base.py index 3596681e..4a1a5d67 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -421,7 +421,7 @@ "DEFAULT_PERMISSION_CLASSES": [ "documentcloud.core.permissions.DjangoObjectPermissionsOrAnonReadOnly" ], - "DEFAULT_PAGINATION_CLASS": "documentcloud.core.pagination.VersionedPagination", + "DEFAULT_PAGINATION_CLASS": "documentcloud.core.pagination.CursorPagination", "DEFAULT_FILTER_BACKENDS": ["django_filters.rest_framework.DjangoFilterBackend"], "HTML_SELECT_CUTOFF": 20, "DEFAULT_AUTHENTICATION_CLASSES": ( @@ -431,7 +431,7 @@ ), "DEFAULT_VERSIONING_CLASS": "documentcloud.core.versioning.QueryParameterVersioning", "DEFAULT_VERSION": env("REST_FRAMEWORK_DEFAULT_VERSION", default="2.0"), - "ALLOWED_VERSIONS": ["1.0", "2.0"], + "ALLOWED_VERSIONS": ["2.0"], "DEFAULT_SCHEMA_CLASS": "drf_spectacular.openapi.AutoSchema", } @@ -448,9 +448,6 @@ ], } -AUTH_PAGE_LIMIT = env.int("AUTH_PAGE_LIMIT", default=1000) -ANON_PAGE_LIMIT = env.int("ANON_PAGE_LIMIT", default=100) - DEFAULT_PAGE_SIZE = env.int("DEFAULT_PAGE_SIZE", default=25) MAX_PAGE_SIZE = env.int("MAX_PAGE_SIZE", default=1000) diff --git a/docs/api/api.md b/docs/api/api.md index 21b962eb..9e9a860e 100644 --- a/docs/api/api.md +++ b/docs/api/api.md @@ -85,17 +85,14 @@ returned by the API, as random access is not available. This system also restricts arbitrary ordering of the results, except for the document search route, which will still allow re-ordering with cursor based pagination. -If the cursor based pagination breaks your workflow, you may continue to use -the old page-offset based pagination system for now. In the future, this will -be disabled completely, and you will be forced to use the cursor based -pagination. To use the page-offset based pagination, which also has a top -level `count` key with a total count of the objects returned for all list -queries, add a `version=1.0` query parameter to your API queries. Be aware -that this will make your queries less performant, possibly to the point of them -being unusable. This should only be used as a stop-gap solution while you -update your workflow to use the new cursor based pagination. Please reach out -to [info@documentcloud.org](mailto:info@documentcloud.org) if you need -assistance moving to the new version. +The page-offset based pagination system, previously available by adding a +`version=1.0` query parameter, has been removed. Requests that include +`version=1.0` will now return a 404. All list routes use cursor based +pagination; use the `next` and `previous` links returned by the API to page +through results. Please reach out to +[info@documentcloud.org](mailto:info@documentcloud.org) if you need assistance +moving to cursor based pagination. + ### Sub Resources diff --git a/documentcloud/core/pagination.py b/documentcloud/core/pagination.py index 7a61499a..b7f6b3e7 100644 --- a/documentcloud/core/pagination.py +++ b/documentcloud/core/pagination.py @@ -1,71 +1,15 @@ # Django from django.conf import settings -from django.core.paginator import InvalidPage, Paginator +from django.core.paginator import Paginator from django.db import OperationalError, connection, transaction from django.utils.functional import cached_property from rest_framework import pagination -from rest_framework.exceptions import NotFound from rest_framework.response import Response # Standard Library from collections import OrderedDict -class PageNumberPagination(pagination.PageNumberPagination): - """For DRF""" - - page_size = 25 - page_size_query_param = "per_page" - max_page_size = 100 - auth_page_limit = settings.AUTH_PAGE_LIMIT - anon_page_limit = settings.ANON_PAGE_LIMIT - - def paginate_queryset(self, queryset, request, view=None): - """ - Paginate a queryset if required, either returning a - page object, or `None` if pagination is not configured for this view. - """ - page_size = self.get_page_size(request) - if not page_size: - return None - - paginator = self.django_paginator_class(queryset, page_size) - page_limit = ( - self.auth_page_limit - if request.user.is_authenticated - else self.anon_page_limit - ) - page_number = request.query_params.get(self.page_query_param, 1) - try: - page_number = int(page_number) - except (TypeError, ValueError): - msg = self.invalid_page_message.format( - page_number=page_number, message="That page number is not an integer" - ) - raise NotFound(msg) - - if page_number > page_limit: - msg = self.invalid_page_message.format( - page_number=page_number, message="That page is past the limit" - ) - raise NotFound(msg) - - try: - self.page = paginator.page(page_number) - except InvalidPage as exc: - msg = self.invalid_page_message.format( - page_number=page_number, message=str(exc) - ) - raise NotFound(msg) - - if paginator.num_pages > 1 and self.template is not None: - # The browsable API should display pagination controls. - self.display_page_controls = True - - self.request = request - return list(self.page) - - class CursorPagination(pagination.CursorPagination): ordering = "pk" page_size = settings.DEFAULT_PAGE_SIZE @@ -106,56 +50,6 @@ def _get_position_from_instance(self, instance, ordering): return str(attr) -class VersionedPagination(pagination.BasePagination): - """Proxies calls to different paginators depending on the version of the - API in use - """ - - # pylint: disable=abstract-method - - def __init__(self): - self.paginators = {"1.0": PageNumberPagination(), "2.0": CursorPagination()} - self.default_paginator = self.paginators[ - settings.REST_FRAMEWORK["DEFAULT_VERSION"] - ] - - def paginate_queryset(self, queryset, request, view=None): - self.request = request - paginator = self.paginators.get(request.version, self.default_paginator) - return paginator.paginate_queryset(queryset, request, view) - - def __getattribute__(self, attr): - """Redirect all other attribute accesses to the given paginator based on - the request version - """ - attributes = [ - "__init__", - "paginate_queryset", - "default_paginator", - "paginators", - "request", - ] - if attr in attributes: - return super().__getattribute__(attr) - if hasattr(self, "request"): - paginator = self.paginators.get( - self.request.version, self.default_paginator - ) - else: - paginator = self.default_paginator - return getattr(paginator, attr) - - -class VersionedCountPagination(VersionedPagination): - """Used for views where you want to use the cursor count pagination""" - - # pylint: disable=abstract-method - - def __init__(self): - super().__init__() - self.paginators["2.0"] = CursorCountPagination() - - class NoCountPaginator(Paginator): """For the admin where count is too slow""" diff --git a/documentcloud/documents/search.py b/documentcloud/documents/search.py index e8faf3a2..f294d2ab 100644 --- a/documentcloud/documents/search.py +++ b/documentcloud/documents/search.py @@ -3,7 +3,6 @@ from django.http.request import QueryDict # Standard Library -import math import re from collections import defaultdict from datetime import datetime @@ -15,7 +14,7 @@ from luqum.utils import LuceneTreeTransformer, LuceneTreeVisitor # DocumentCloud -from documentcloud.core.pagination import CursorPagination, PageNumberPagination +from documentcloud.core.pagination import CursorPagination from documentcloud.documents.constants import DATA_KEY_REGEX from documentcloud.documents.models import Document from documentcloud.documents.search_escape import escape @@ -118,7 +117,7 @@ def search(user, query_params): else: sort = SORT_MAP.get(sort_param, SORT_MAP["score"]) - page_query_data, page_response_data = _paginate(query_params, user) + page_query_data = _paginate_cursor(query_params, user) # allow explicit enabling of highlighting if query_params.get("hl", "").lower() == "true" and settings.SOLR_USE_HL: @@ -167,9 +166,7 @@ def search(user, query_params): ) results = SOLR.search(text_query, **kwargs) - response = _format_response( - results, query_params, user, escaped, page_response_data - ) + response = _format_response(results, query_params, user, escaped) if use_hl and query_notes: response = _highlight_notes(response, original_text_query) @@ -549,19 +546,6 @@ def _access_filter(user): return ["filter(access:public AND status:(success readable) AND !noindex:true)"] -def _paginate(query_params, user): # pylint: disable=inconsistent-return-statements - """Emulate the Django Rest Framework pagination style""" - version = query_params.get("version", settings.REST_FRAMEWORK["DEFAULT_VERSION"]) - - if version not in ["1.0", "2.0"]: - version = settings.REST_FRAMEWORK["DEFAULT_VERSION"] - - if version == "1.0": - return _paginate_page(query_params, user) - elif version == "2.0": - return _paginate_cursor(query_params, user) - - def _get_int(query_params, field, default, max_value=None, min_value=None): """Helper function to convert a parameter to an integer""" try: @@ -575,31 +559,6 @@ def _get_int(query_params, field, default, max_value=None, min_value=None): return default -def _paginate_page(query_params, user): - """Emulate the Django Rest Framework page number pagination style""" - - if user.is_authenticated: - max_page_size = PageNumberPagination.max_page_size - max_page = PageNumberPagination.auth_page_limit - else: - max_page_size = settings.SOLR_ANON_MAX_ROWS - max_page = PageNumberPagination.anon_page_limit - - rows = _get_int( - query_params, - PageNumberPagination.page_size_query_param, - PageNumberPagination.page_size, - max_value=max_page_size, - ) - page = _get_int(query_params, PageNumberPagination.page_query_param, 1, min_value=1) - if page > max_page: - raise ValueError( - f"The selected `page` value of {page} is over the limit of {max_page}" - ) - start = (page - 1) * rows - return {"rows": rows, "start": start}, {"page": page, "rows": rows} - - def _paginate_cursor(query_params, user): """Emulate the Django Rest Framework cursor pagination style""" @@ -616,36 +575,20 @@ def _paginate_cursor(query_params, user): CursorPagination.page_size, max_value=max_page_size, ) - return {"rows": rows, "cursorMark": cursor}, {} + return {"rows": rows, "cursorMark": cursor} -def _format_response(results, query_params, user, escaped, page_data): +def _format_response(results, query_params, user, escaped): """Emulate the Django Rest Framework response format""" base_url = f"{settings.DOCCLOUD_API_URL}/api/documents/search/" query_params = query_params.copy() - if "page" in page_data: - page = page_data["page"] - per_page = page_data["rows"] - max_page = math.ceil(results.hits / per_page) - if page < max_page: - query_params["page"] = page + 1 - next_url = f"{base_url}?{query_params.urlencode()}" - else: - next_url = None - - if page > 1: - query_params["page"] = page - 1 - previous_url = f"{base_url}?{query_params.urlencode()}" - else: - previous_url = None + if query_params.get("cursor", "*") == results.nextCursorMark: + next_url = None else: - if query_params.get("cursor", "*") == results.nextCursorMark: - next_url = None - else: - query_params["cursor"] = results.nextCursorMark - next_url = f"{base_url}?{query_params.urlencode()}" - previous_url = None + query_params["cursor"] = results.nextCursorMark + next_url = f"{base_url}?{query_params.urlencode()}" + previous_url = None expands = query_params.get("expand", "").split(",") count = results.hits diff --git a/documentcloud/documents/views.py b/documentcloud/documents/views.py index 7ac9639a..365a7682 100644 --- a/documentcloud/documents/views.py +++ b/documentcloud/documents/views.py @@ -1334,8 +1334,10 @@ def search(self, request): status=status.HTTP_503_SERVICE_UNAVAILABLE, ) except ValueError as exc: + # luqum's ParseError subclasses ValueError + # so malformed filter values land here return Response( - {"error": exc.args[0], "code": "max_page"}, + {"error": exc.args[0], "code": "invalid_query"}, status=status.HTTP_400_BAD_REQUEST, ) else: diff --git a/documentcloud/projects/views.py b/documentcloud/projects/views.py index d01f34e4..684689e1 100644 --- a/documentcloud/projects/views.py +++ b/documentcloud/projects/views.py @@ -19,7 +19,7 @@ # DocumentCloud from documentcloud.core.filters import ModelMultipleChoiceFilter -from documentcloud.core.pagination import VersionedCountPagination +from documentcloud.core.pagination import CursorCountPagination from documentcloud.core.permissions import ( DjangoObjectPermissionsOrAnonReadOnly, ProjectPermissions, @@ -441,7 +441,7 @@ class ProjectMembershipViewSet(BulkModelMixin, FlexFieldsModelViewSet): f"document.{e}" for e in DocumentViewSet.permit_list_expands ] permission_classes = (DjangoObjectPermissionsOrAnonReadOnly | ProjectPermissions,) - pagination_class = VersionedCountPagination + pagination_class = CursorCountPagination filter_backends = [OrderingFilter, DjangoFilterBackend] ordering_fields = ["created_at"] ordering = ["-document__pk"]