OpenConceptLab/ocl_online#456 | Load a page's concept mappings once for includeMappings concept lists (#949)

* OpenConceptLab/ocl_online#456 | Load a page's concept mappings once for includeMappings concept lists

* OpenConceptLab/ocl_online#456 | fixing pylint

* OpenConceptLab/ocl_online#456 | API-level tests for batched includeMappings across concept list endpoints

Author: snyaggarwal

core/common/serializers.py

@@ -262,6 +262,9 @@ def to_representation(self, data):
         self.child.Meta.model.load_list_fields(
             instances, self.child.fields.keys(), get_collection_in_scope(self.context),
             get(self.context, 'request.user') or get(self.context, 'user'))
+        preload_page = getattr(self.child, 'preload_page', None)
+        if preload_page:
+            preload_page(instances)
         return super().to_representation(instances)
 
 

core/concepts/serializers.py

@@ -1,3 +1,4 @@
+from django.db.models import F
 from pydash import get, compact
 from rest_framework.fields import CharField, DateTimeField, BooleanField, URLField, JSONField, SerializerMethodField, \
     UUIDField, ListField, IntegerField, ReadOnlyField
@@ -234,10 +235,52 @@ def get_references(self, obj):
             return obj.collection_references_uris(collection)
         return None
 
-    def get_mappings(self, obj):
+    def mapping_serializer_class(self):
+        from core.mappings.serializers import MappingDetailSerializer, MappingMinimalSerializer
+        request = get(self, 'context.request')
+        is_mapping_brief = request.query_params.get('mappingBrief', None) in TRUTHY
+        return MappingMinimalSerializer if is_mapping_brief else MappingDetailSerializer
+
+    def preloads_direct_mappings(self):
+        return 'mappings' in self.fields and self.include_direct_mappings and not (
+            self.include_indirect_mappings or 'collection' in self.view_kwargs)
+
+    def preload_page(self, instances):
+        """Loads the direct mappings of a whole page of concepts in one go, instead of per concept."""
+        request = get(self, 'context.request')
+        if not (instances and request and self.preloads_direct_mappings()):
+            return
+        from core.mappings.models import Mapping
+        params = request.query_params
+        concept_ids = {
+            concept.id: set(compact([
+                concept.id, concept.versioned_object_id, get(concept.get_latest_version(), 'id')]))
+            for concept in instances
+        }
+        mappings = Mapping.objects.filter(
+            parent_id__in={concept.parent_id for concept in instances},
+            from_concept_id__in=set().union(*concept_ids.values()), id=F('versioned_object_id'))
+        if map_types := compact((params.get('mapTypes', '') or '').split(',')):
+            mappings = mappings.filter(map_type__in=map_types)
+        if target_repo_urls := compact((params.get('targetRepoUrls', '') or '').split(',')):
+            mappings = mappings.filter(to_source_url__in=target_repo_urls)
+        if params.get(INCLUDE_RETIRED_PARAM, None) not in TRUTHY:
+            mappings = mappings.filter(retired=False)
+        mappings = list(mappings.order_by('id'))
+        Mapping.load_list_fields(mappings, self.mapping_serializer_class()().fields.keys(), None, request.user)
+        for concept in instances:
+            concept._preloaded_mappings = [  # pylint: disable=protected-access
+                mapping for mapping in mappings
+                if mapping.parent_id == concept.parent_id and mapping.from_concept_id in concept_ids[concept.id]]
+
+    def get_mappings(self, obj):  # pylint: disable=too-many-locals
         from core.mappings.serializers import MappingDetailSerializer, MappingMinimalSerializer
         context = get(self, 'context')
         request = get(context, 'request')
+        preloaded = getattr(obj, '_preloaded_mappings', None)
+        if preloaded is not None and self.preloads_direct_mappings():
+            child = self.mapping_serializer_class()()
+            return [child.to_representation(mapping) for mapping in preloaded]
         is_mapping_brief = request.query_params.get('mappingBrief', None) in TRUTHY
         map_types = compact((request.query_params.get('mapTypes', '') or '').split(','))
         target_repo_urls = compact((request.query_params.get('targetRepoUrls', '') or '').split(','))

core/concepts/tests/tests.py

@@ -6,7 +6,8 @@
 from elasticsearch_dsl.utils import AttrDict
 from django.contrib.auth.models import Group
 from django.core.exceptions import ValidationError
-from django.db import IntegrityError
+from django.db import IntegrityError, connection
+from django.test.utils import CaptureQueriesContext
 from django.http import Http404, QueryDict
 from django.test import override_settings
 from pydash import omit
@@ -3116,6 +3117,26 @@ def test_get_mappings_direct_with_map_types_and_target_repo_urls_filters(self):
         result = serializer.get_mappings(concept1)
         self.assertEqual(result, [])
 
+    def test_list_serializer_loads_page_mappings_once(self):
+        source = OrganizationSourceFactory()
+        concepts = [ConceptFactory(parent=source) for _ in range(6)]
+        for concept in concepts[:5]:
+            MappingFactory(from_concept=concept, to_concept=concepts[5], parent=source, map_type='Same As')
+            MappingFactory(from_concept=concept, to_concept=concepts[5], parent=source, map_type='Other')
+        query = 'includeMappings=true&mapTypes=Same As,Other'
+        context = self.build_context(query)
+
+        rows = [ConceptListSerializer(concept, context=context).data['mappings'] for concept in concepts]
+        with CaptureQueriesContext(connection) as small:
+            page = ConceptListSerializer(concepts[:2], many=True, context=context).data
+        with CaptureQueriesContext(connection) as large:
+            page = ConceptListSerializer(concepts, many=True, context=context).data
+
+        by_id = lambda mappings: sorted(mappings, key=lambda mapping: mapping['uuid'])  # noqa: E731
+        self.assertEqual([by_id(row['mappings']) for row in page], [by_id(row) for row in rows])
+        self.assertEqual(len(rows[0]), 2)
+        self.assertLess(len(large) - len(small), 4 * 4)
+
     def test_get_mappings_returns_empty_list_by_default(self):
         concept = ConceptFactory()
         serializer = ConceptListSerializer(context=self.build_context())

core/integration_tests/tests_concepts.py

@@ -28,7 +28,7 @@
 from core.concepts.documents import ConceptDocument
 from core.concepts.models import Concept
 from core.concepts.search import ConceptFuzzySearch
-from core.concepts.serializers import ConceptListSerializer, ConceptDetailSerializer
+from core.concepts.serializers import ConceptListSerializer, ConceptDetailSerializer, ConceptAbstractSerializer
 from core.concepts.tests.factories import ConceptFactory, ConceptNameFactory, ConceptDescriptionFactory
 from core.concepts.views import MetadataToConceptsListView
 from core.integration_tests.test_utils import SourceChildListQueryCountMixin
@@ -4628,3 +4628,79 @@ def test_post_429_refused_by_capacity_limit(self, reranker_mock):
         self.assertEqual(response.status_code, 429)
         self.assertEqual(response.data['error_code'], 'capacity_exceeded')
         reranker_mock.assert_not_called()
+
+
+class ConceptListIncludeMappingsTest(OCLAPITestCase):
+    def setUp(self):
+        super().setUp()
+        self.source = OrganizationSourceFactory()
+        self.other_source = OrganizationSourceFactory()
+        self.concepts = [ConceptFactory(parent=self.source) for _ in range(3)]
+        self.other_concept = ConceptFactory(parent=self.other_source)
+        self.mappings = {}
+        for concept in self.concepts:
+            self.mappings[concept.mnemonic] = {
+                MappingFactory(
+                    parent=self.source, from_concept=concept, to_concept=self.concepts[0], map_type=f'type{index}'
+                ).id for index in range(2)
+            }
+        self.mappings[self.other_concept.mnemonic] = {MappingFactory(
+            parent=self.other_source, from_concept=self.other_concept, to_concept=self.other_concept).id}
+
+    def mappings_by_concept(self, url):
+        response = self.client.get(url + ('&' if '?' in url else '?') + 'verbose=true&includeMappings=true')
+        self.assertEqual(response.status_code, 200)
+        return {row['id']: sorted(mapping['uuid'] for mapping in row['mappings']) for row in response.data}
+
+    def assert_same_with_and_without_batching(self, url, expected=None):
+        batched = self.mappings_by_concept(url)
+        with patch.object(ConceptAbstractSerializer, 'preload_page', lambda *args: None):
+            per_concept = self.mappings_by_concept(url)
+        self.assertEqual(batched, per_concept)
+        if expected is not None:
+            self.assertEqual(
+                batched, {mnemonic: sorted(str(_id) for _id in ids) for mnemonic, ids in expected.items()})
+
+    def test_source_head_list(self):
+        self.assert_same_with_and_without_batching(
+            self.source.uri + 'concepts/', {c.mnemonic: self.mappings[c.mnemonic] for c in self.concepts})
+
+    def test_global_list_across_sources(self):
+        batched = self.mappings_by_concept('/concepts/')
+        self.assertEqual(len(batched), 4)
+        self.assert_same_with_and_without_batching('/concepts/', {
+            mnemonic: ids for mnemonic, ids in self.mappings.items()})
+
+    def test_source_version_list(self):
+        version = OrganizationSourceFactory(
+            organization=self.source.organization, mnemonic=self.source.mnemonic, version='v1')
+        for concept in self.concepts:
+            version.concepts.add(concept.get_latest_version())
+        self.assert_same_with_and_without_batching(version.uri + 'concepts/')
+        self.assertTrue(any(self.mappings_by_concept(version.uri + 'concepts/').values()))
+
+    def test_concept_versions_list(self):
+        self.assert_same_with_and_without_batching(
+            self.concepts[1].versions_url, {self.concepts[1].mnemonic: self.mappings[self.concepts[1].mnemonic]})
+
+    def test_collection_list_ignores_mappings_outside_expansion(self):
+        collection = OrganizationCollectionFactory()
+        expansion = ExpansionFactory(collection_version=collection)
+        collection.expansion_uri = expansion.uri
+        collection.save()
+        concept = self.concepts[0]
+        kept = Mapping.objects.filter(parent=self.source, from_concept=concept).first()
+        expansion.concepts.add(concept.get_latest_version())
+        expansion.mappings.add(kept)
+
+        url = collection.uri + 'HEAD/concepts/'
+        self.assert_same_with_and_without_batching(url)
+        shown = {uuid for uuids in self.mappings_by_concept(url).values() for uuid in uuids}
+        self.assertLessEqual(shown, {str(kept.id)})
+
+    def test_inverse_mappings_are_unchanged(self):
+        url = self.source.uri + 'concepts/?verbose=true&includeInverseMappings=true'
+        batched = self.client.get(url).data
+        with patch.object(ConceptAbstractSerializer, 'preload_page', lambda *args: None):
+            per_concept = self.client.get(url).data
+        self.assertEqual(batched, per_concept)