diff --git a/core/common/serializers.py b/core/common/serializers.py index 14ec15fc..d4a66712 100644 --- a/core/common/serializers.py +++ b/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) diff --git a/core/concepts/serializers.py b/core/concepts/serializers.py index 9a9fc849..38011909 100644 --- a/core/concepts/serializers.py +++ b/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(',')) diff --git a/core/concepts/tests/tests.py b/core/concepts/tests/tests.py index 5a006798..d41462ed 100644 --- a/core/concepts/tests/tests.py +++ b/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()) diff --git a/core/integration_tests/tests_concepts.py b/core/integration_tests/tests_concepts.py index 9b7f9a77..58f90ae7 100644 --- a/core/integration_tests/tests_concepts.py +++ b/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)