diff --git a/core/collections/views.py b/core/collections/views.py index e45e6b8d..9285a82f 100644 --- a/core/collections/views.py +++ b/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 diff --git a/core/common/mixins.py b/core/common/mixins.py index 57ea324e..0aa13dc7 100644 --- a/core/common/mixins.py +++ b/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): diff --git a/core/common/serializers.py b/core/common/serializers.py index 868b6c87..4c8336a6 100644 --- a/core/common/serializers.py +++ b/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: diff --git a/core/common/tests.py b/core/common/tests.py index 64cc4a4c..f5c19451 100644 --- a/core/common/tests.py +++ b/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]]) diff --git a/core/common/utils.py b/core/common/utils.py index 5ebdce68..40cc846c 100644 --- a/core/common/utils.py +++ b/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): diff --git a/core/common/views.py b/core/common/views.py index d7d845cc..8c937bb3 100644 --- a/core/common/views.py +++ b/core/common/views.py @@ -26,7 +26,7 @@ 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 @@ -34,10 +34,10 @@ 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: diff --git a/core/concept_maps/views.py b/core/concept_maps/views.py index e1aa117f..abbae140 100644 --- a/core/concept_maps/views.py +++ b/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()] diff --git a/core/concepts/views.py b/core/concepts/views.py index b9997980..64fceff9 100644 --- a/core/concepts/views.py +++ b/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'] diff --git a/core/integration_tests/tests_collections.py b/core/integration_tests/tests_collections.py index a5ab6aec..92099d7d 100644 --- a/core/integration_tests/tests_collections.py +++ b/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/', diff --git a/core/integration_tests/tests_concepts.py b/core/integration_tests/tests_concepts.py index 87215f9c..dc04c0a3 100644 --- a/core/integration_tests/tests_concepts.py +++ b/core/integration_tests/tests_concepts.py @@ -1,4 +1,6 @@ +import json import uuid +import zipfile from unittest.mock import patch import factory @@ -3257,6 +3259,85 @@ def test_facets_with_latest_released_repo_search(self): self.assertFalse(class_b_facet[2]) self.assertEqual([x for x in response.data['facets']['fields']['conceptClass'] if x[0] == 'classA'], []) + def follow_pages(self, url, **headers): + ids, pages = [], 0 + while url: + response = self.client.get(url, **headers) + self.assertEqual(response.status_code, 200) + self.assertLessEqual(len(response.data), 1) + self.assertEqual(int(response['num_returned']), len(response.data)) + ids += [concept['id'] for concept in response.data] + pages += 1 + url = response.get('next') + return sorted(ids), pages + + @override_settings(LIST_MAX_LIMIT=1) + def test_page_size_capped(self): + response = self.client.get(self.source.concepts_url + '?limit=1000') + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data), 1) + self.assertEqual(response['num_found'], '2') + self.assertEqual(response['pages'], '2') + self.assertIn('limit=1000', response['next']) + self.assertEqual(self.follow_pages(self.source.concepts_url + '?limit=1000'), (['MyConcept1', 'MyConcept2'], 2)) + + # without a limit too, the default page is under the max + self.assertEqual(self.follow_pages(self.source.concepts_url), (['MyConcept1', 'MyConcept2'], 2)) + response = self.client.get('/concepts/', HTTP_AUTHORIZATION='Token ' + self.random_user.get_token()) + self.assertEqual(len(response.data), 1) + + # search (ES) and the global list are capped the same way + response = self.client.get(self.source.concepts_url + '?q=MyConcept&limit=1000') + self.assertLessEqual(len(response.data), 1) + self.assertEqual(response['pages'], response['num_found']) # pages of one row + response = self.client.get('/concepts/?limit=1000', HTTP_AUTHORIZATION='Token ' + self.random_user.get_token()) + self.assertEqual(len(response.data), 1) + + # a released version: later pages start where the first ended, not at the raw limit + self.source_v1.concepts.add(self.concept1.get_latest_version()) + self.assertEqual( + self.follow_pages(self.source.uri + 'v1/concepts/?limit=1000'), (['MyConcept1', 'MyConcept2'], 2)) + + response = self.client.get(self.concept1.uri + 'versions/?limit=1000') + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data), 1) + + def test_page_size_up_to_max_unchanged(self): + response = self.client.get(self.source.concepts_url + '?limit=1') + self.assertEqual(len(response.data), 1) + self.assertEqual(response['pages'], '2') + + response = self.client.get(self.source.concepts_url + '?limit=100') + self.assertEqual(len(response.data), 2) + self.assertEqual(response['num_returned'], '2') + + response = self.client.get(self.source.concepts_url) + self.assertEqual(len(response.data), 2) + + @override_settings(LIST_MAX_LIMIT=1) + def test_page_size_brief_checksums_not_capped(self): + response = self.client.get(self.source.concepts_url + '?limit=1000&brief=true&checksums=true') + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data), 2) + + @override_settings(LIST_MAX_LIMIT=1) + def test_compress_over_max_page_size_needs_permission(self): + response = self.client.get( + self.source.concepts_url, HTTP_COMPRESS='true', HTTP_AUTHORIZATION='Token ' + self.random_user.get_token()) + self.assertEqual(response.status_code, 403) + self.assertEqual(response.data['error_code'], 'list_unpaginated_limit_reached') + self.assertEqual(response.data['limit'], 1) + self.assertEqual(response.data['requested'], 2) + + # someone allowed any size gets the whole list, not one capped page of it + self.source_v1.concepts.add(self.concept1.get_latest_version()) + for url in (self.source.concepts_url + '?q=MyConcept&limit=1000', self.source.uri + 'v1/concepts/?limit=1000'): + response = self.client.get(url, HTTP_COMPRESS='true', HTTP_AUTHORIZATION='Token ' + self.token) + self.assertEqual(response.status_code, 200, url) + content = json.loads( + zipfile.ZipFile(response.rendered_content.filelike).read('export.json').decode('utf-8')) + self.assertEqual(sorted(concept['id'] for concept in content), ['MyConcept1', 'MyConcept2'], url) + @override_settings(CACHES={'default': {'BACKEND': 'django.core.cache.backends.locmem.LocMemCache'}}) class ConceptListCacheTest(OCLAPITestCase): diff --git a/core/integration_tests/tests_mappings.py b/core/integration_tests/tests_mappings.py index db7e9a02..46c82581 100644 --- a/core/integration_tests/tests_mappings.py +++ b/core/integration_tests/tests_mappings.py @@ -23,6 +23,23 @@ def setUp(self): self.user = UserProfileFactory() self.token = self.user.get_token() + @override_settings(LIST_MAX_LIMIT=1) + def test_page_size_capped(self): + mapping = MappingFactory() + MappingFactory(parent=mapping.parent) + MappingDocument().update(mapping.parent.mappings_set.all()) + + for url in (mapping.parent.mappings_url, '/mappings/'): + response = self.client.get(url + '?limit=1000') + self.assertEqual(response.status_code, 200, url) + self.assertEqual(len(response.data), 1, url) + self.assertEqual(response['num_returned'], '1', url) + self.assertEqual(response['pages'], '2', url) + + response = self.client.get(mapping.parent.mappings_url + '?limit=1000&page=2') + self.assertEqual(len(response.data), 1) + self.assertNotIn('next', response) + def test_get_200(self): response = self.client.get('/mappings/', format='json') diff --git a/core/integration_tests/tests_orgs.py b/core/integration_tests/tests_orgs.py index 2021219d..b606f4b4 100644 --- a/core/integration_tests/tests_orgs.py +++ b/core/integration_tests/tests_orgs.py @@ -2,6 +2,7 @@ import time from django.db import connection +from django.test import override_settings from mock import patch from mock.mock import Mock, ANY from rest_framework.exceptions import ErrorDetail @@ -983,6 +984,16 @@ def test_without_header_unaffected(self): response = self.client.get('/orgs/', HTTP_AUTHORIZATION='Token ' + UserProfileFactory().get_token()) self.assertEqual(response.status_code, 200) + @override_settings(LIST_MAX_LIMIT=1) + def test_uncapped_list_ignores_max_page_size(self): + token = 'Token ' + UserProfileFactory().get_token() + response = self.client.get('/orgs/', HTTP_COMPRESS='true', HTTP_AUTHORIZATION=token) + self.assertEqual(response.status_code, 200) + + response = self.client.get('/orgs/?limit=1000', HTTP_AUTHORIZATION=token) + self.assertEqual(response.status_code, 200) + self.assertGreater(len(response.data), 1) + class PaginationParamsValidationTest(OCLAPITestCase): def setUp(self): diff --git a/core/mappings/views.py b/core/mappings/views.py index 4a058a97..24d3c381 100644 --- a/core/mappings/views.py +++ b/core/mappings/views.py @@ -8,7 +8,7 @@ from rest_framework.permissions import IsAdminUser from rest_framework.response import Response -from core.common.constants import HEAD, ACCESS_TYPE_NONE, LIMIT_PARAM, LIST_DEFAULT_LIMIT +from core.common.constants import HEAD, ACCESS_TYPE_NONE, LIST_DEFAULT_LIMIT from core.common.exceptions import Http400 from core.common.mixins import ListWithHeadersMixin, ConceptDictionaryMixin from core.common.swagger_parameters import ( @@ -44,6 +44,7 @@ def get_queryset(self): class MappingListView(MappingBaseView, ListWithHeadersMixin, CreateModelMixin): + is_page_size_capped = True serializer_class = MappingListSerializer def get_permissions(self): @@ -90,7 +91,7 @@ def get_queryset(self): queryset = Mapping.apply_attribute_based_filters( queryset, 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 = Mapping.sources.through.objects.filter(source_id=parent.id) @@ -313,6 +314,7 @@ def update(self, request, *args, **kwargs): class MappingVersionsView(MappingBaseView, ConceptDictionaryMixin, ListWithHeadersMixin): + is_page_size_capped = True permission_classes = (CanViewParentDictionary,) def get_queryset(self): diff --git a/core/settings.py b/core/settings.py index ef027266..36991dfc 100644 --- a/core/settings.py +++ b/core/settings.py @@ -708,6 +708,8 @@ def get_set_from_env(name): # Postgres statement_timeout for a paginated list's queries, so a list that can't finish frees its worker; 0 is off LIST_QUERY_TIMEOUT_MS = int(os.environ.get('LIST_QUERY_TIMEOUT_MS', 30000)) +# Largest page a concept, mapping or reference list serves, so one page stays a short request +LIST_MAX_LIMIT = int(os.environ.get('LIST_MAX_LIMIT', 100)) ANALYTICS_API = os.environ.get('ANALYTICS_API', 'http://host.docker.internal:8002') if ANALYTICS_API: