diff --git a/jsapp/js/api/models/assetsDataListParams.ts b/jsapp/js/api/models/assetsDataListParams.ts index 6dab66dbac..c0fa71d6c6 100644 --- a/jsapp/js/api/models/assetsDataListParams.ts +++ b/jsapp/js/api/models/assetsDataListParams.ts @@ -20,5 +20,5 @@ export type AssetsDataListParams = { /** * The initial index from which to return the results. */ - start?: number + offset?: number } diff --git a/jsapp/js/api/models/assetsHooksLogsListParams.ts b/jsapp/js/api/models/assetsHooksLogsListParams.ts index f46c10e840..e008578145 100644 --- a/jsapp/js/api/models/assetsHooksLogsListParams.ts +++ b/jsapp/js/api/models/assetsHooksLogsListParams.ts @@ -14,9 +14,13 @@ import type { AssetsHooksLogsListStatus } from './assetsHooksLogsListStatus' export type AssetsHooksLogsListParams = { end?: string /** - * A page number within the paginated result set. + * Number of results to return per page. */ - page?: number + limit?: number + /** + * The initial index from which to return the results. + */ + offset?: number start?: string /** * * `0` - Failed diff --git a/kobo/apps/audit_log/views.py b/kobo/apps/audit_log/views.py index a5d8daab3b..36c2f7c6a9 100644 --- a/kobo/apps/audit_log/views.py +++ b/kobo/apps/audit_log/views.py @@ -13,7 +13,7 @@ ImportExportStatusChoices, ProjectHistoryLogExportTask, ) -from kpi.paginators import FastPagination, NoCountPagination, Paginated +from kpi.paginators import DefaultPagination from kpi.permissions import IsAuthenticated from kpi.renderers import BasicHTMLRenderer from kpi.tasks import export_task_in_background @@ -90,7 +90,7 @@ class AuditLogViewSet(mixins.ListModelMixin, viewsets.GenericViewSet): 'model_name__icontains', 'metadata__icontains', ] - pagination_class = NoCountPagination + pagination_class = DefaultPagination.custom_class(no_count=True) @extend_schema_view( @@ -146,7 +146,7 @@ class AccessLogViewSet(AuditLogViewSet): permission_classes = (IsAuthenticated,) filter_backends = (AccessLogPermissionsFilter,) serializer_class = AccessLogSerializer - pagination_class = Paginated + pagination_class = DefaultPagination @extend_schema_view( @@ -300,7 +300,7 @@ class ProjectHistoryLogViewSet( model = ProjectHistoryLog permission_classes = (ViewProjectHistoryLogsPermission,) lookup_field = 'uid' - pagination_class = FastPagination + pagination_class = DefaultPagination.custom_class(fast_count=True) def get_queryset(self): return self.model.objects.filter(metadata__asset_uid=self.asset_uid).order_by( diff --git a/kobo/apps/hook/views/v2/hook_log.py b/kobo/apps/hook/views/v2/hook_log.py index d542167a88..af2af5af9c 100644 --- a/kobo/apps/hook/views/v2/hook_log.py +++ b/kobo/apps/hook/views/v2/hook_log.py @@ -12,7 +12,7 @@ from kobo.apps.hook.models.hook_log import HookLog from kobo.apps.hook.schema_extensions.v2.hooks.logs.serializers import LogsRetryResponse from kobo.apps.hook.serializers.v2.hook_log import HookLogSerializer -from kpi.paginators import TinyPaginated +from kpi.paginators import DefaultPagination from kpi.permissions import AssetEditorSubmissionViewerPermission from kpi.utils.schema_extensions.markdown import read_md from kpi.utils.schema_extensions.response import open_api_200_ok_response @@ -89,7 +89,7 @@ class HookLogViewSet(AssetNestedObjectViewsetMixin, lookup_url_kwarg = 'uid_log' serializer_class = HookLogSerializer permission_classes = (AssetEditorSubmissionViewerPermission,) - pagination_class = TinyPaginated + pagination_class = DefaultPagination.custom_class(page_size=50) filter_backends = (DjangoFilterBackend,) filterset_class = HookLogFilter diff --git a/kobo/apps/project_views/views.py b/kobo/apps/project_views/views.py index d82ccfa07c..2286a767f0 100644 --- a/kobo/apps/project_views/views.py +++ b/kobo/apps/project_views/views.py @@ -15,7 +15,7 @@ from kpi.mixins.asset import AssetViewSetListMixin from kpi.mixins.object_permission import ObjectPermissionViewSetMixin from kpi.models import Asset, ProjectViewExportTask -from kpi.paginators import FastPagination +from kpi.paginators import DefaultPagination from kpi.permissions import IsAuthenticated from kpi.serializers.v2.asset import AssetMetadataListSerializer from kpi.serializers.v2.user import UserListSerializer @@ -144,7 +144,7 @@ def get_queryset(self, *args, **kwargs): detail=True, methods=['GET'], filter_backends=[SearchFilter, AssetOrderingFilter], - pagination_class=FastPagination, + pagination_class=DefaultPagination.custom_class(fast_count=True), ) def assets(self, request, uid_project_view): if not user_has_view_perms(request.user, uid_project_view): diff --git a/kobo/apps/user_reports/views.py b/kobo/apps/user_reports/views.py index ca873a8147..20ab5189fe 100644 --- a/kobo/apps/user_reports/views.py +++ b/kobo/apps/user_reports/views.py @@ -9,7 +9,7 @@ from kobo.apps.user_reports.models import UserReports from kobo.apps.user_reports.seralizers import UserReportsSerializer from kpi.filters import SearchFilter -from kpi.paginators import NoCountPagination +from kpi.paginators import DefaultPagination from kpi.permissions import IsAuthenticated from kpi.schema_extensions.v2.user_reports.serializers import UserReportsListResponse from kpi.utils.schema_extensions.markdown import read_md @@ -40,7 +40,7 @@ class UserReportsViewSet(mixins.ListModelMixin, viewsets.GenericViewSet): queryset = UserReports.objects.all() serializer_class = UserReportsSerializer - pagination_class = NoCountPagination + pagination_class = DefaultPagination.custom_class(no_count=True) permission_classes = (IsAuthenticated, SuperUserPermission) filter_backends = [DjangoFilterBackend, OrderingFilter, SearchFilter] diff --git a/kobo/settings/base.py b/kobo/settings/base.py index 5370a99380..0d6853da84 100644 --- a/kobo/settings/base.py +++ b/kobo/settings/base.py @@ -1003,7 +1003,7 @@ def __init__(self, *args, **kwargs): REST_FRAMEWORK = { 'URL_FIELD_NAME': 'url', - 'DEFAULT_PAGINATION_CLASS': 'kpi.paginators.Paginated', + 'DEFAULT_PAGINATION_CLASS': 'kpi.paginators.DefaultPagination', 'PAGE_SIZE': 100, 'DEFAULT_AUTHENTICATION_CLASSES': [ # SessionAuthentication and BasicAuthentication would be included by diff --git a/kpi/paginators.py b/kpi/paginators.py index 1fe5e40a9c..3f2daafc73 100644 --- a/kpi/paginators.py +++ b/kpi/paginators.py @@ -1,30 +1,164 @@ +import contextlib from collections import OrderedDict from typing import Union from django.conf import settings from django.db.models.query import QuerySet from django_request_cache import cache_for_request -from rest_framework.pagination import ( - LimitOffsetPagination, - PageNumberPagination, -) +from rest_framework.pagination import LimitOffsetPagination, _positive_int from rest_framework.response import Response from rest_framework.reverse import reverse_lazy from rest_framework.serializers import SerializerMethodField -from rest_framework.utils.urls import replace_query_param +from rest_framework.utils.urls import remove_query_param, replace_query_param -class Paginated(LimitOffsetPagination): +class DefaultPagination(LimitOffsetPagination): """ + Default pagination class for API views, it can be customized via the + custom_class class method. It can take `offset`/`start` & `limit` parameters as + well as `page` number. + Adds 'root' to the wrapping response object. """ root = SerializerMethodField('get_parent_url', read_only=True) + page_query_param = 'page' + page_size = settings.REST_FRAMEWORK['PAGE_SIZE'] + page_size_query_param = None + no_count = False + fast_count = False + + def get_count(self, queryset): + """ + This custom function raises NotImplementedError if self.no_count equals True. + If fast_count is enabled, it does a faster counting for DISTINCT queries on + large tables. It only looks at the primary key field, avoiding expensive + DISTINCTs comparing several fields. This may not work for queries with lots of + joins, especially with one-to-many or many-to-many type relationships. + If fast_count is disabled, it runs the parent class get_count method. + """ + if self.no_count: + raise NotImplementedError( + 'DefaultPagination class with attribute no_count=True' + ) + if self.fast_count: + if queryset.query.distinct: + return queryset.only('pk').count() + + return super().get_count(queryset) def get_parent_url(self, obj): return reverse_lazy('api-root', request=self.context.get('request')) + def get_limit(self, request): + if self.limit_query_param: + with contextlib.suppress(KeyError, ValueError): + return _positive_int( + request.query_params[self.limit_query_param], + strict=True, + cutoff=self.max_limit, + ) + + return None + + def get_offset(self, request): + with contextlib.suppress(ValueError, TypeError): + offset = ( + request.query_params.get('offset') + or request.query_params.get('start') + or request.query_params.get(self.offset_query_param) + ) + return _positive_int(offset) + + return None + + def get_page_number(self, request): + try: + return _positive_int(request.query_params.get(self.page_query_param)) + except (ValueError, TypeError): + return None + + def get_page_size(self, request): + if self.page_size_query_param: + with contextlib.suppress(KeyError, ValueError): + return _positive_int( + request.query_params[self.page_size_query_param], + strict=True, + cutoff=self.max_page_size, + ) + return self.page_size + + def get_paginated_response_schema(self, schema): + response_schema = super().get_paginated_response_schema(schema) + if self.no_count: + response_schema['required'].remove('count') + del response_schema['properties']['count'] + return response_schema + + def get_paginated_response(self, data): + if self.no_count: + return Response( + { + 'next': self.get_next_link(), + 'previous': self.get_previous_link(), + 'results': data, + } + ) + + return super().get_paginated_response(data) + + def get_next_link(self): + if self.no_count: + if not self.has_next: + return None + url = self.request.build_absolute_uri() + url = replace_query_param(url, self.limit_query_param, self.limit) + offset = self.offset + self.limit + return replace_query_param(url, self.offset_query_param, offset) + + url = super().get_next_link() + return remove_query_param(url, self.page_query_param) + + def get_previous_link(self): + url = super().get_previous_link() + url = remove_query_param(url, self.page_query_param) + return url + + @classmethod + def custom_class(cls, **kwargs): + class_name = kwargs.pop('class_name', 'CustomPagination') + return type(class_name, (cls,), kwargs) + + def paginate_queryset(self, queryset, request, view=None): + self.request = request + self.limit = self.get_limit(request) + self.offset = self.get_offset(request) + page_number = self.get_page_number(request) + + if page_number and not self.limit and not self.offset: + self.offset = (page_number - 1) * self.get_page_size(request) + self.limit = self.get_page_size(request) + + if not self.offset: + self.offset = 0 + if not self.limit: + self.limit = self.default_limit + + if self.no_count: + items = list(queryset[self.offset:(self.offset + self.limit + 1)]) + self.has_next = len(items) > self.limit + return items[: self.limit] + + self.count = self.get_count(queryset) + if self.count > self.limit and self.template is not None: + self.display_page_controls = True -class AssetPagination(Paginated): + if self.count == 0 or self.offset > self.count: + return [] + + return list(queryset[self.offset:(self.offset + self.limit)]) + + +class AssetPagination(DefaultPagination): def get_paginated_response(self, data, metadata): @@ -119,79 +253,3 @@ def get_paginated_response_schema(self, schema): 'results': schema, } } - - -class DataPagination(LimitOffsetPagination): - """ - Pagination class for submissions. - """ - default_limit = 100 - offset_query_param = 'start' - max_limit = settings.SUBMISSION_LIST_LIMIT - - -class FastPagination(Paginated): - """ - Pagination class optimized for faster counting for DISTINCT queries on large tables. - - This class overrides the get_count() method to only look at the primary key field, avoiding expensive DISTINCTs - comparing several fields. This may not work for queries with lots of joins, especially with one-to-many or - many-to-many type relationships. - """ - - def get_count(self, queryset): - if queryset.query.distinct: - return queryset.only('pk').count() - return super().get_count(queryset) - - -class NoCountPagination(Paginated): - """ - Omits the 'count' field to avoid expensive COUNT(*) queries. - """ - - def get_paginated_response_schema(self, schema): - response_schema = super().get_paginated_response_schema(schema) - response_schema['required'].remove('count') - del response_schema['properties']['count'] - return response_schema - - def get_paginated_response(self, data): - return Response( - { - 'next': self.get_next_link(), - 'previous': self.get_previous_link(), - 'results': data, - } - ) - - def paginate_queryset(self, queryset, request, view=None): - self.request = request - - self.limit = self.get_limit(request) - if self.limit is None: - return None - - self.offset = self.get_offset(request) - - # Peek one item beyond the current page to see if a next page exists - items = list(queryset[self.offset:self.offset + self.limit + 1]) - self.has_next = len(items) > self.limit - return items[:self.limit] - - def get_next_link(self): - if not self.has_next: - return None - - url = self.request.build_absolute_uri() - url = replace_query_param(url, self.limit_query_param, self.limit) - - offset = self.offset + self.limit - return replace_query_param(url, self.offset_query_param, offset) - - -class TinyPaginated(PageNumberPagination): - """ - Same as Paginated with a small page size - """ - page_size = 50 diff --git a/kpi/tests/test_pagination.py b/kpi/tests/test_pagination.py new file mode 100644 index 0000000000..1b918eb061 --- /dev/null +++ b/kpi/tests/test_pagination.py @@ -0,0 +1,46 @@ +from django.test import TestCase +from rest_framework import generics, serializers +from rest_framework.test import APIRequestFactory + +from kpi.paginators import DefaultPagination + + +class PassThroughSerializer(serializers.BaseSerializer): + def to_representation(self, item): + return item + + +class TestDefaultPagination(TestCase): + def setUp(self): + self.request_factory = APIRequestFactory() + + def test_custom_pagination_attributes(self): + CustomClass = DefaultPagination.custom_class( + page_size=123, custom_parameter='abc' + ) + assert CustomClass.custom_parameter == 'abc' + assert CustomClass.page_size == 123 + + def test_with_start_offset(self): + self.view = generics.ListAPIView.as_view( + serializer_class=PassThroughSerializer, + queryset=range(1, 101), + pagination_class=DefaultPagination.custom_class(default_limit=5), + ) + request = self.request_factory.get('/', {'start': 20}) + response = self.view(request) + assert response.data['results'] == [21, 22, 23, 24, 25] + + request = self.request_factory.get('/', {'offset': 30}) + response = self.view(request) + assert response.data['results'] == [31, 32, 33, 34, 35] + + def test_pagination_with_page(self): + self.view = generics.ListAPIView.as_view( + serializer_class=PassThroughSerializer, + queryset=range(1, 101), + pagination_class=DefaultPagination.custom_class(page_size=5), + ) + request = self.request_factory.get('/', {'page': 2}) + response = self.view(request) + assert response.data['results'] == [6, 7, 8, 9, 10] diff --git a/kpi/views/v2/data.py b/kpi/views/v2/data.py index 6fe1301d5a..c2fcd19ef2 100644 --- a/kpi/views/v2/data.py +++ b/kpi/views/v2/data.py @@ -36,7 +36,7 @@ ObjectDeploymentDoesNotExist, ) from kpi.models import Asset -from kpi.paginators import DataPagination +from kpi.paginators import DefaultPagination from kpi.permissions import ( DuplicateSubmissionPermission, EditLinkSubmissionPermission, @@ -202,7 +202,10 @@ class DataViewSet( SubmissionXMLRenderer, ) permission_classes = (SubmissionPermission,) - pagination_class = DataPagination + pagination_class = DefaultPagination.custom_class( + default_limit=100, + max_limit=settings.SUBMISSION_LIST_LIMIT, + ) log_type = AuditType.PROJECT_HISTORY logged_fields = [] diff --git a/static/openapi/schema_v2.json b/static/openapi/schema_v2.json index 135c1977c4..05df0a5122 100644 --- a/static/openapi/schema_v2.json +++ b/static/openapi/schema_v2.json @@ -1907,7 +1907,7 @@ } }, { - "name": "start", + "name": "offset", "required": false, "in": "query", "description": "The initial index from which to return the results.", @@ -5496,10 +5496,19 @@ } }, { - "name": "page", + "name": "limit", "required": false, "in": "query", - "description": "A page number within the paginated result set.", + "description": "Number of results to return per page.", + "schema": { + "type": "integer" + } + }, + { + "name": "offset", + "required": false, + "in": "query", + "description": "The initial index from which to return the results.", "schema": { "type": "integer" } @@ -16899,13 +16908,13 @@ "type": "string", "nullable": true, "format": "uri", - "example": "http://api.example.org/accounts/?start=400&limit=100" + "example": "http://api.example.org/accounts/?offset=400&limit=100" }, "previous": { "type": "string", "nullable": true, "format": "uri", - "example": "http://api.example.org/accounts/?start=200&limit=100" + "example": "http://api.example.org/accounts/?offset=200&limit=100" }, "results": { "type": "array", @@ -17116,13 +17125,13 @@ "type": "string", "nullable": true, "format": "uri", - "example": "http://api.example.org/accounts/?page=4" + "example": "http://api.example.org/accounts/?offset=400&limit=100" }, "previous": { "type": "string", "nullable": true, "format": "uri", - "example": "http://api.example.org/accounts/?page=2" + "example": "http://api.example.org/accounts/?offset=200&limit=100" }, "results": { "type": "array", diff --git a/static/openapi/schema_v2.yaml b/static/openapi/schema_v2.yaml index 63fea9f35d..6354eefc40 100644 --- a/static/openapi/schema_v2.yaml +++ b/static/openapi/schema_v2.yaml @@ -1400,7 +1400,7 @@ paths: description: Number of results to return per page. schema: type: integer - - name: start + - name: offset required: false in: query description: The initial index from which to return the results. @@ -4139,10 +4139,16 @@ paths: schema: type: string format: date-time - - name: page + - name: limit required: false in: query - description: A page number within the paginated result set. + description: Number of results to return per page. + schema: + type: integer + - name: offset + required: false + in: query + description: The initial index from which to return the results. schema: type: integer - in: query @@ -12200,12 +12206,12 @@ components: type: string nullable: true format: uri - example: http://api.example.org/accounts/?start=400&limit=100 + example: http://api.example.org/accounts/?offset=400&limit=100 previous: type: string nullable: true format: uri - example: http://api.example.org/accounts/?start=200&limit=100 + example: http://api.example.org/accounts/?offset=200&limit=100 results: type: array items: @@ -12361,12 +12367,12 @@ components: type: string nullable: true format: uri - example: http://api.example.org/accounts/?page=4 + example: http://api.example.org/accounts/?offset=400&limit=100 previous: type: string nullable: true format: uri - example: http://api.example.org/accounts/?page=2 + example: http://api.example.org/accounts/?offset=200&limit=100 results: type: array items: