OpenConceptLab/ocl_online#470 | Cap concept, mapping and reference lists at LIST_MAX_LIMIT rows per page

core/collections/views.py

@@ -351,6 +351,7 @@ def get(self, request, *args, **kwargs):
 
 
 class CollectionReferenceConceptsView(CollectionReferenceAbstractResourcesView):
+    is_page_size_capped = True
     is_searchable = True
     document_model = ConceptDocument
     es_fields = Concept.es_fields
@@ -365,6 +366,7 @@ def get_queryset(self):
 
 
 class CollectionReferenceMappingsView(CollectionReferenceAbstractResourcesView):
+    is_page_size_capped = True
     is_searchable = True
     document_model = MappingDocument
     es_fields = Mapping.es_fields
@@ -428,6 +430,8 @@ class CollectionReferencesView(
     CollectionBaseView, ConceptDictionaryUpdateMixin, RetrieveAPIView, DestroyAPIView, ListWithHeadersMixin,
     TaskMixin, CollectionReferencesMixin
 ):
+    is_page_size_capped = True
+
     def get_serializer_class(self):
         if self.is_verbose():
             return CollectionReferenceDetailSerializer
@@ -666,6 +670,8 @@ def post(self, request, **kwargs):  # pylint: disable=unused-argument
 
 
 class CollectionVersionReferencesView(CollectionVersionBaseView, ListWithHeadersMixin, CollectionReferencesMixin):
+    is_page_size_capped = True
+
     def get_serializer_class(self):
         if self.is_verbose():
             return CollectionReferenceDetailSerializer
@@ -986,6 +992,7 @@ def get(self, request, *args, **kwargs):
 
 
 class CollectionVersionExpansionConceptsView(CollectionVersionExpansionChildrenView):
+    is_page_size_capped = True
     is_searchable = True
     document_model = ConceptDocument
     es_fields = Concept.es_fields
@@ -1001,6 +1008,7 @@ def get_queryset(self):
 
 
 class CollectionVersionExpansionMappingsView(CollectionVersionExpansionChildrenView):
+    is_page_size_capped = True
     is_searchable = True
     document_model = MappingDocument
     es_fields = Mapping.es_fields
@@ -1036,6 +1044,7 @@ class ExpansionMappingsIndexView(ExpansionResourcesIndexView):
 
 
 class CollectionVersionConceptsView(CollectionBaseView, ListWithHeadersMixin):
+    is_page_size_capped = True
     is_searchable = True
     document_model = ConceptDocument
     es_fields = Concept.es_fields
@@ -1112,6 +1121,8 @@ def get_object(self, queryset=None):
 
 
 class CollectionVersionConceptMappingsView(CollectionVersionConceptRetrieveView, ListWithHeadersMixin):
+    is_page_size_capped = True
+
     def get_queryset(self):
         queryset = self.apply_filters(super().get_queryset())
         expansion = self.get_expansion()
@@ -1136,6 +1147,7 @@ def get(self, request, *args, **kwargs):
 
 
 class CollectionVersionMappingsView(CollectionBaseView, ListWithHeadersMixin):
+    is_page_size_capped = True
     is_searchable = True
     document_model = MappingDocument
     es_fields = Mapping.es_fields

core/common/mixins.py

@@ -29,7 +29,6 @@
     INCLUDE_PARENT_CONCEPTS, INCLUDE_CHILD_CONCEPTS
 from core.common.permissions import HasPrivateAccess, HasOwnership, CanViewConceptDictionary, \
     CanViewConceptDictionaryVersion, CanEditConceptDictionary
-from core.users.constants import LIST_UNPAGINATED_PERMISSION
 from .checksums import ChecksumModel
 from .exceptions import Http403, UnpaginatedListLimitReached
 from .utils import write_csv_to_s3, get_query_params_from_url_string, compact_dict_by_values, \
@@ -339,17 +338,17 @@ def only_search_stats(self):
     def should_compress(self):
         """
         `Compress: true` returns the whole list, unpaginated (a zip). Anyone may, for up to MAX_UNPAGINATED_RESULTS
-        results; bigger lists need `users.list_unpaginated` (ocl_online#230).
+        results (a capped list: settings.LIST_MAX_LIMIT); bigger lists need `users.list_unpaginated`.
         """
         if self.request.META.get(HTTP_COMPRESS_HEADER, False) not in TRUTHY:
             return False
-        user = self.request.user
-        if user.is_staff or user.has_perm(LIST_UNPAGINATED_PERMISSION):
+        if self.can_list_unpaginated():
             return True
         total = self.total_count or (
             self.object_list.count() if isinstance(self.object_list, QuerySet) else len(self.object_list))
-        if total > MAX_UNPAGINATED_RESULTS:
-            raise UnpaginatedListLimitReached(MAX_UNPAGINATED_RESULTS, total)
+        max_results = settings.LIST_MAX_LIMIT if get(self, 'is_page_size_capped') else MAX_UNPAGINATED_RESULTS
+        if total > max_results:
+            raise UnpaginatedListLimitReached(max_results, total)
         return True
 
     def get_object_ids(self):

core/common/serializers.py

@@ -8,7 +8,7 @@
 from core.common.constants import INCLUDE_CONCEPTS_PARAM, INCLUDE_MAPPINGS_PARAM, LIMIT_PARAM, OFFSET_PARAM, \
     INCLUDE_VERBOSE_REFERENCES, INCLUDE_SEARCH_META_PARAM, ACCESS_TYPE_CHOICES, ACCESS_TYPE_VIEW
 from core.common.feeds import DEFAULT_LIMIT
-from core.common.utils import to_int, get_truthy_values, normalize_public_access
+from core.common.utils import to_int, get_truthy_values, normalize_public_access, cap_page_size
 from core.concept_maps.constants import RESOURCE_TYPE as CONCEPT_MAP_RESOURCE_TYPE
 from core.orgs.models import Organization
 from core.users.models import UserProfile
@@ -323,6 +323,8 @@ def __init__(self, *args, **kwargs):
             self.include_mappings = self.query_params.get(INCLUDE_MAPPINGS_PARAM) in TRUTHY
             self.include_references = self.query_params.get(INCLUDE_VERBOSE_REFERENCES) in TRUTHY
             self.limit = to_int(self.query_params.get(LIMIT_PARAM), DEFAULT_LIMIT)
+            if self.include_concepts or self.include_mappings or self.include_references:
+                self.limit = cap_page_size(self.limit)
             self.offset = to_int(self.query_params.get(OFFSET_PARAM), 0)
         try:
             if not self.include_concepts:

core/common/tests.py

@@ -62,7 +62,7 @@
     split_list_by_condition, is_zip_file, get_date_range_label, get_prev_month, from_string_to_date, get_end_of_month,
     get_start_of_month, es_id_in, web_url, get_queue_task_names, get_resource_class_from_resource_uri, encode_string,
     to_parent_kwargs_from_uri, reverse_resource, reverse_resource_version, write_export_file, queue_bulk_import,
-    get_bulk_import_celery_once_lock_key, generic_sort, get_embeddings, encode_texts, get_lm_model)
+    get_bulk_import_celery_once_lock_key, generic_sort, get_embeddings, encode_texts, get_lm_model, cap_page_size)
 from core.concepts.documents import ConceptDocument
 from core.concepts.models import Concept
 from core.mappings.documents import MappingDocument
@@ -982,6 +982,15 @@ def test_nested_dict_values(self):
             [1, 'foobar', 1, 'foobar', [{'a': 1}, {'b': 'foobar'}]]
         )
 
+    def test_cap_page_size(self):
+        self.assertEqual(cap_page_size('1000'), 100)
+        self.assertEqual(cap_page_size('100'), 100)
+        self.assertEqual(cap_page_size(40), 40)
+        self.assertEqual(cap_page_size(None), 25)
+        self.assertEqual(cap_page_size('0'), 25)
+        with override_settings(LIST_MAX_LIMIT=1):
+            self.assertEqual(cap_page_size('10'), 1)
+
     def test_chunks(self):
         self.assertEqual(list(chunks([], 1000)), [])
         self.assertEqual(list(chunks([1, 2, 3, 4], 3)), [[1, 2, 3], [4]])

core/common/utils.py

@@ -27,7 +27,7 @@
 from rest_framework.utils import encoders
 
 from core.common.constants import UPDATED_SINCE_PARAM, BULK_IMPORT_QUEUES_COUNT, CURRENT_USER, REQUEST_URL, \
-    TEMP_PREFIX, HEAD, EXPORT_TIME_EXTRA, RETIRED_ACCESS_TYPE_EDIT, ACCESS_TYPE_VIEW
+    TEMP_PREFIX, HEAD, EXPORT_TIME_EXTRA, RETIRED_ACCESS_TYPE_EDIT, ACCESS_TYPE_VIEW, LIST_DEFAULT_LIMIT
 from core.settings import EXPORT_SERVICE
 
 
@@ -928,6 +928,11 @@ def to_int(value, default_value):
         return default_value
 
 
+def cap_page_size(limit):
+    """`limit` as an int, at most settings.LIST_MAX_LIMIT: it keeps one page of a heavy list a short request."""
+    return min(to_int(limit, LIST_DEFAULT_LIMIT), settings.LIST_MAX_LIMIT)
+
+
 def parse_id(value):
     # Plain ASCII digits within bigint range only: '²'.isdigit() is True but int('²') raises (a 500).
     if isinstance(value, int) and not isinstance(value, bool):

core/common/views.py

@@ -26,18 +26,18 @@
     LIMIT_PARAM, NOT_FOUND, MUST_SPECIFY_EXTRA_PARAM_IN_BODY, INCLUDE_RETIRED_PARAM, VERBOSE_PARAM, HEAD, LATEST, \
     BRIEF_PARAM, ES_REQUEST_TIMEOUT, INCLUDE_INACTIVE, FHIR_LIMIT_PARAM, RAW_PARAM, SEARCH_MAP_CODES_PARAM, \
     INCLUDE_SEARCH_META_PARAM, EXCLUDE_FUZZY_SEARCH_PARAM, EXCLUDE_WILDCARD_SEARCH_PARAM, UPDATED_BY_USERNAME_PARAM, \
-    CANONICAL_URL_REQUEST_PARAM, CHECKSUMS_PARAM, ACCESS_TYPE_NONE
+    CANONICAL_URL_REQUEST_PARAM, CHECKSUMS_PARAM, ACCESS_TYPE_NONE, HTTP_COMPRESS_HEADER
 from core.common.exceptions import Http400
 from core.common.mixins import PathWalkerMixin
 from core.common.search import CustomESSearch, EXACT_OWNER_FIELD
 from core.common.serializers import RootSerializer
 from core.common.swagger_parameters import all_resource_query_param
 from core.common.throttling import ThrottleUtil
 from core.common.utils import compact_dict_by_values, to_snake_case, parse_updated_since_param, \
-    to_int, get_falsy_values, get_truthy_values, format_url_for_search
+    to_int, get_falsy_values, get_truthy_values, format_url_for_search, cap_page_size
 from core.concepts.permissions import CanViewParentDictionary, CanEditParentDictionary
 from core.orgs.constants import ORG_OBJECT_TYPE
-from core.users.constants import USER_OBJECT_TYPE
+from core.users.constants import USER_OBJECT_TYPE, LIST_UNPAGINATED_PERMISSION
 
 TRUTHY = get_truthy_values()
 
@@ -52,6 +52,7 @@ class BaseAPIView(generics.GenericAPIView, PathWalkerMixin):
     pk_field = 'mnemonic'
     user_is_self = False
     is_searchable = False
+    is_page_size_capped = False  # concept, mapping and reference lists: `limit` is capped at settings.LIST_MAX_LIMIT
     limit = LIST_DEFAULT_LIMIT
     default_filters = {}
     sort_asc_param = 'sortAsc'
@@ -130,6 +131,20 @@ def initialize(self, request, path_info_segment, **kwargs):  # pylint: disable=u
         params = request.query_params.dict()
         self.validate_pagination_params(request, params)
         self.limit = params.get(LIMIT_PARAM, None) or params.get(FHIR_LIMIT_PARAM, None) or LIST_DEFAULT_LIMIT
+        self.limit = self.cap_limit(self.limit)
+
+    def cap_limit(self, limit):
+        """`limit`, capped on a capped list, except for checksums and for a `Compress` by someone allowed any size,
+         whose results must not be pre-sliced to one page."""
+        if not self.is_page_size_capped or (self.is_brief() and self.is_checksums()) or (
+                self.request.META.get(HTTP_COMPRESS_HEADER, False) in TRUTHY and self.can_list_unpaginated()):
+            return limit
+        return cap_page_size(limit)
+
+    def can_list_unpaginated(self):
+        """Staff, or `users.list_unpaginated`: `Compress` returns any size of list."""
+        user = self.request.user
+        return user.is_staff or user.has_perm(LIST_UNPAGINATED_PERMISSION)
 
     @staticmethod
     def validate_pagination_params(request, params):
@@ -1205,10 +1220,9 @@ def __get_params(self):
     def __set_params(self):
         self.params = self.__get_params()
         if self.params:
-            self.limit = to_int(
-                CSV_DEFAULT_LIMIT if self.params.get('csv') else self.params.get(LIMIT_PARAM),
-                LIST_DEFAULT_LIMIT
-            )
+            # `limit` as initialize read (and capped) it; `_count` isn't read here
+            limit = self.limit if self.params.get(LIMIT_PARAM) else self.cap_limit(LIST_DEFAULT_LIMIT)
+            self.limit = to_int(CSV_DEFAULT_LIMIT if self.params.get('csv') else limit, LIST_DEFAULT_LIMIT)
 
 
 class SourceChildExtrasBaseView:

core/concept_maps/views.py

@@ -75,6 +75,7 @@ def get_detail_serializer(self, obj):
 
 class ConceptMapTranslateView(MappingListView):
     serializer_class = ConceptMapParametersSerializer
+    is_page_size_capped = False  # FHIR paging (`_count`) is decided apart from the list cap
 
     def get_permissions(self):
         return [CanAccessParentDictionary(), CanViewConceptDictionary()]

core/concepts/views.py

@@ -24,7 +24,7 @@
 from core.collections.documents import CollectionDocument
 from core.common import ERRBIT_LOGGER
 from core.common.constants import (
-    HEAD, INCLUDE_INVERSE_MAPPINGS_PARAM, INCLUDE_RETIRED_PARAM, ACCESS_TYPE_NONE, LIMIT_PARAM, LIST_DEFAULT_LIMIT)
+    HEAD, INCLUDE_INVERSE_MAPPINGS_PARAM, INCLUDE_RETIRED_PARAM, ACCESS_TYPE_NONE, LIST_DEFAULT_LIMIT)
 from core.common.exceptions import Http400, Http403, Http409
 from core.common.mixins import ListWithHeadersMixin, ConceptDictionaryMixin
 from core.capabilities.constants import CAPABILITY_EXCEEDED_ERROR_CODE, CAPABILITY_NOT_ENTITLED_ERROR_CODE, \
@@ -176,6 +176,7 @@ def get_queryset(self):
 
 
 class ConceptListView(ConceptBaseView, ListWithHeadersMixin, CreateModelMixin):
+    is_page_size_capped = True
     serializer_class = ConceptListSerializer
 
     def get_permissions(self):
@@ -222,7 +223,7 @@ def get_queryset(self):
                 queryset = Concept.apply_attribute_based_filters(
                     parent.concepts_set, self.params).filter(is_active=True)
             else:
-                limit = to_int(self.params.get(LIMIT_PARAM), LIST_DEFAULT_LIMIT)
+                limit = to_int(self.limit, LIST_DEFAULT_LIMIT)  # capped, so later pages start where the first ended
                 page = to_int(self.params.get('page'), 1)
                 offset = (page - 1) * limit
                 through_qs = Concept.sources.through.objects.filter(source_id=parent.id)
@@ -602,6 +603,7 @@ def get_clone_to_source(self):
 
 
 class ConceptChildrenView(ConceptBaseView, ListWithHeadersMixin):
+    is_page_size_capped = True
     is_searchable = False  # a search would list the repo's concepts, not this concept's hierarchy
     serializer_class = ConceptChildrenSerializer
     default_qs_sort_attr = 'mnemonic'
@@ -622,6 +624,7 @@ def get(self, request, *args, **kwargs):
 
 
 class ConceptParentsView(ConceptBaseView, ListWithHeadersMixin):
+    is_page_size_capped = True
     is_searchable = False  # a search would list the repo's concepts, not this concept's hierarchy
     serializer_class = ConceptParentsSerializer
 
@@ -666,6 +669,7 @@ def update(self, request, *args, **kwargs):
 
 
 class ConceptVersionsView(ConceptBaseView, ConceptDictionaryMixin, ListWithHeadersMixin):
+    is_page_size_capped = True
     permission_classes = (CanViewParentDictionary,)
 
     def get_queryset(self):
@@ -688,6 +692,7 @@ def get(self, request, *args, **kwargs):
 
 
 class ConceptMappingsView(ConceptBaseView, ListWithHeadersMixin):
+    is_page_size_capped = True
     serializer_class = MappingListSerializer
     permission_classes = (CanViewParentDictionary,)
     default_qs_sort_attr = ['map_type', 'sort_weight']

core/integration_tests/tests_collections.py

@@ -3,6 +3,7 @@
 
 from celery_once import AlreadyQueued
 from django.core.files.uploadedfile import SimpleUploadedFile
+from django.test import override_settings
 from mock import patch, Mock, ANY
 from mock.mock import PropertyMock
 from rest_framework.exceptions import ErrorDetail
@@ -502,6 +503,26 @@ def test_get_200_with_resources(self):
         self.assertEqual(len(response.data['mappings']), 1)
         self.assertEqual(len(response.data['references']), 1)
 
+    @override_settings(LIST_MAX_LIMIT=1)
+    def test_get_200_with_resources_limit_capped(self):
+        coll = OrganizationCollectionFactory(mnemonic='coll1')
+        expansion = ExpansionFactory(collection_version=coll)
+        coll.expansion_uri = expansion.uri
+        coll.save()
+        expansion.concepts.add(ConceptFactory(), ConceptFactory())
+        expansion.mappings.add(MappingFactory(), MappingFactory())
+
+        response = self.client.get(
+            coll.uri + '?includeConcepts=true&includeMappings=true&limit=1000', format='json')
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(response.data['concepts']), 1)
+        self.assertEqual(len(response.data['mappings']), 1)
+
+        response = self.client.get(coll.uri + 'concepts/?limit=1000', format='json')
+        self.assertEqual(len(response.data), 1)
+        self.assertEqual(response['num_found'], '2')
+
     def test_get_404(self):
         response = self.client.get(
             '/orgs/foobar/collections/coll1/',
@@ -674,6 +695,20 @@ def test_get_200(self):
         self.assertEqual(response.status_code, 200)
         self.assertEqual(len(response.data), 0)
 
+    @override_settings(LIST_MAX_LIMIT=1)
+    def test_get_200_limit_capped(self):
+        CollectionReference(expression='/foo/bar', collection=self.collection).save()
+
+        response = self.client.get(self.collection.uri + 'references/?limit=1000', format='json')
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(response.data), 1)
+        self.assertEqual(response['num_found'], '2')
+        self.assertEqual(response['pages'], '2')
+
+        response = self.client.get(self.collection.uri + 'references/?limit=1000&page=2', format='json')
+        self.assertEqual(len(response.data), 1)
+
     def test_get_200_with_resolved_repo_versions(self):
         duplicate_reference = CollectionReference(
             expression=f'{self.concept.parent.uri}concepts/duplicate/',