diff --git a/core/common/mixins.py b/core/common/mixins.py index e628b9d8..927f3c16 100644 --- a/core/common/mixins.py +++ b/core/common/mixins.py @@ -43,7 +43,7 @@ class CustomPaginator: def __init__( # pylint: disable=too-many-arguments self, request, total_count, queryset, page_size, is_sliced=False, max_score=None, search_scores=None, - highlights=None + highlights=None, page_by_ids=False ): self.request = request self.queryset = queryset @@ -55,7 +55,12 @@ def __init__( # pylint: disable=too-many-arguments bottom = (self.page_number - 1) * self.page_size top = bottom + self.page_size top = min(top, self.total) - self.queryset = self.queryset[bottom:top] + if page_by_ids and isinstance(self.queryset, QuerySet): + # select the page's ids first, so joins (select_related) run for the page's rows only, + # not for every row a deep page's OFFSET skips + self.queryset = self.queryset.filter(id__in=self.queryset.values('id')[bottom:top]) + else: + self.queryset = self.queryset[bottom:top] if isinstance(self.queryset, QuerySet): self.queryset.count = None self.paginator = Paginator(self.queryset, self.page_size) @@ -247,7 +252,8 @@ def list(self, request, *args, **kwargs): # pylint:disable=too-many-locals,too- paginator = CustomPaginator( request=request, queryset=sorted_list, page_size=self.limit, total_count=self.total_count, is_sliced=self.is_sliced(), max_score=get(self, '_max_score'), - search_scores=get(self, '_scores'), highlights=get(self, '_highlights') + search_scores=get(self, '_scores'), highlights=get(self, '_highlights'), + page_by_ids=self.should_page_by_ids() ) headers = paginator.headers results = paginator.current_page_results @@ -265,6 +271,9 @@ def list(self, request, *args, **kwargs): # pylint:disable=too-many-locals,too- response['num_found'] = len(sorted_list) return response + def should_page_by_ids(self): # see CustomPaginator + return False + def serialize_list(self, results, paginator=None): result_dict = self.get_serializer(results, many=True).data if self.should_include_facets(): diff --git a/core/concepts/views.py b/core/concepts/views.py index 01c8af7e..711c22dd 100644 --- a/core/concepts/views.py +++ b/core/concepts/views.py @@ -174,6 +174,9 @@ def get_serializer_class(self): return ConceptListSerializer + def should_page_by_ids(self): + return 'source' in self.kwargs and bool(get(self, 'parent_resource.is_head')) + def is_sliced(self): result = super().is_sliced() if result: diff --git a/core/integration_tests/tests_list_pages.py b/core/integration_tests/tests_list_pages.py new file mode 100644 index 00000000..f21a53ed --- /dev/null +++ b/core/integration_tests/tests_list_pages.py @@ -0,0 +1,61 @@ +from datetime import timedelta + +from django.db import connection +from django.db.models import F +from django.test.utils import CaptureQueriesContext +from django.utils import timezone + +from core.common.tests import OCLAPITestCase +from core.concepts.models import Concept +from core.concepts.tests.factories import ConceptFactory, ConceptNameFactory +from core.mappings.models import Mapping +from core.mappings.tests.factories import MappingFactory +from core.sources.tests.factories import OrganizationSourceFactory + + +class SourceHeadListPagesTest(OCLAPITestCase): + """ + A source's HEAD concept and mapping lists select a page's ids first (CustomPaginator page_by_ids), so the page + query's joins cover the page's rows only, not every row a deep page's OFFSET skips. + """ + def setUp(self): + super().setUp() + self.source = OrganizationSourceFactory() + + def concept(): + return ConceptFactory(parent=self.source, names=[ConceptNameFactory.build(locale_preferred=True)]) + + for _ in range(5): + MappingFactory(parent=self.source, from_concept=concept(), to_concept=concept()) + now = timezone.now() + for model in (Concept, Mapping): + for minutes, row in enumerate(model.objects.filter(parent=self.source, id=F('versioned_object_id'))): + model.objects.filter(id=row.id).update(updated_at=now - timedelta(minutes=minutes)) + + def assert_pages(self, url, table, total): + everything = self.client.get(f'{url}?limit={total}', format='json') + self.assertEqual(len(everything.data), total) + + rows = [] + for page in range(1, total // 4 + 2): + with CaptureQueriesContext(connection) as context: + response = self.client.get(f'{url}?limit=4&page={page}', format='json') + self.assertEqual(response.status_code, 200) + self.assertEqual(response['num_found'], str(total)) + rows += response.data + + page_queries = [ + query['sql'] for query in context.captured_queries + if query['sql'].startswith('SELECT') and f'FROM "{table}"' in query['sql'] and ' JOIN ' in query['sql'] + ] + self.assertEqual(len(page_queries), 1) + self.assertIn(f'"{table}"."id" IN (SELECT', page_queries[0]) + self.assertNotIn('LIMIT', page_queries[0].rsplit(')', 1)[-1]) # paged inside the ids subquery + + self.assertEqual(rows, everything.data) + + def test_mappings(self): + self.assert_pages(self.source.mappings_url, 'mappings', 5) + + def test_concepts(self): + self.assert_pages(self.source.concepts_url, 'concepts', 10) diff --git a/core/mappings/views.py b/core/mappings/views.py index 264db9de..e02c8aef 100644 --- a/core/mappings/views.py +++ b/core/mappings/views.py @@ -68,6 +68,9 @@ def get_serializer_class(self): def apply_filters(self, queryset): return queryset + def should_page_by_ids(self): + return 'source' in self.kwargs and bool(get(self, 'parent_resource.is_head')) + def is_sliced(self): result = super().is_sliced() if result: