diff --git a/core/common/mixins.py b/core/common/mixins.py index 927f3c16..04760a76 100644 --- a/core/common/mixins.py +++ b/core/common/mixins.py @@ -8,7 +8,7 @@ from django.core.exceptions import ValidationError from django.core.paginator import Paginator from django.db import transaction, models -from django.db.models import Q, F, QuerySet +from django.db.models import Q, F, QuerySet, prefetch_related_objects from django.http import HttpResponseForbidden, Http404 from django.shortcuts import redirect from django.urls import resolve, Resolver404 @@ -644,9 +644,143 @@ class SourceChildMixin(ChecksumModel): _counted = models.BooleanField(default=True, null=True, blank=True) _index = models.BooleanField(default=True) + # relations the list serializers read on every row, prefetched once per page by load_list_fields + LIST_PREFETCHES = () + # relations named like the serializer field that reads them; prefetched by load_list_fields when it's serialized + FIELD_PREFETCHES = () + # serializer fields that read latest_source_version + LATEST_SOURCE_VERSION_FIELDS = ('latest_source_version', ) + class Meta: abstract = True + @classmethod + def load_list_fields(cls, instances, fields, collection=None): + """Loads, for a whole page at once, what the serialized fields would otherwise query row by row.""" + fields = set(fields) + prefetch_related_objects(instances, *cls.LIST_PREFETCHES, *fields.intersection(cls.FIELD_PREFETCHES)) + loaders = { + 'version_url': cls.load_latest_version_uris, + 'previous_version_url': cls.load_previous_version_uris, + 'source_versions': cls.load_source_versions, + 'collection_versions': cls.load_collection_versions, + } + for field, loader in loaders.items(): + if field in fields: + loader(instances) + + reads_latest_source_version = fields & set(cls.LATEST_SOURCE_VERSION_FIELDS) + if collection: # there latest_source_version shows the collection's resolved version instead + reads_latest_source_version.discard('latest_source_version') + if fields & {'references', 'latest_source_version'}: + cls.load_collection_references(instances, collection) + if 'latest_source_version' in fields: + cls.load_collection_source_versions(instances, collection) + if reads_latest_source_version: + cls.load_latest_source_versions(instances) + + @classmethod + def load_latest_version_uris(cls, instances): + """Sets each HEAD row's _latest_version_uri (version_url) from one query for the page.""" + heads = [instance.id for instance in instances if instance.is_versioned_object] + latest_version_uris = dict(cls.objects.filter( + versioned_object_id__in=heads, is_active=True, is_latest_version=True + ).exclude(id=F('versioned_object_id')).order_by('versioned_object_id', '-created_at').distinct( + 'versioned_object_id').values_list('versioned_object_id', 'uri')) + for instance in instances: + instance._latest_version_uri = latest_version_uris.get(instance.id) # pylint: disable=protected-access + + @classmethod + def load_source_versions(cls, instances): + """Sets each row's _source_versions: the uris of the released source versions holding it.""" + key = f'{cls._meta.model_name}_id' + uris = {} + for row_id, uri in cls.sources.through.objects.filter( + **{f'{key}__in': [instance.id for instance in instances]} + ).exclude(source__version=HEAD).order_by('source__created_at').values_list(key, 'source__uri'): + uris.setdefault(row_id, []).append(uri) + for instance in instances: + instance._source_versions = uris.get(instance.id, []) # pylint: disable=protected-access + + @classmethod + def load_collection_versions(cls, instances): + """Sets each row's _collection_versions: the uris of the collection versions whose expansions hold it.""" + key = f'{cls._meta.model_name}_id' + uris = {} + for row_id, uri in cls.expansion_set.through.objects.filter( + **{f'{key}__in': [instance.id for instance in instances]} + ).exclude(expansion__collection_version__version=HEAD).values_list(key, 'expansion__collection_version__uri'): + uris.setdefault(row_id, set()).add(uri) + for instance in instances: + instance._collection_versions = sorted(uris.get(instance.id, ())) # pylint: disable=protected-access + + @classmethod + def load_latest_source_versions(cls, instances): + """Sets each row's _latest_source_version: the latest released source version holding it.""" + key = f'{cls._meta.model_name}_id' + latest_source_version_ids = dict(cls.sources.through.objects.filter( + **{f'{key}__in': [instance.id for instance in instances]} + ).exclude(source__version=HEAD).order_by(key, '-source__created_at').distinct(key).values_list( + key, 'source_id')) + source_versions = cls._meta.get_field('sources').related_model.objects.in_bulk( + set(latest_source_version_ids.values())) + for instance in instances: + # pylint: disable=protected-access + instance._latest_source_version = source_versions.get(latest_source_version_ids.get(instance.id)) + + @classmethod + def load_previous_version_uris(cls, instances): + """Sets each row's _prev_version_uri, as prev_version_uri would query it, from one query for the page.""" + siblings = {} + for row_id, versioned_object_id, created_at, uri in cls.objects.filter( + versioned_object_id__in={instance.versioned_object_id for instance in instances}, is_active=True + ).exclude(id=F('versioned_object_id')).values_list('id', 'versioned_object_id', 'created_at', 'uri'): + siblings.setdefault(versioned_object_id, []).append((created_at, row_id, uri)) + for instance in instances: + previous = [ + (created_at, uri) for created_at, row_id, uri in siblings.get(instance.versioned_object_id, []) + if row_id != instance.id and created_at <= instance.created_at + ] + # pylint: disable=protected-access + instance._prev_version_uri = max(previous, key=lambda version: version[0])[1] if previous else None + + @classmethod + def load_collection_references(cls, instances, collection): + """Sets each row's _collection_reference_ids: the ids of its references in that collection.""" + key = f'{cls._meta.model_name}_id' + reference_ids = {} + for row_id, reference_id in cls.references.through.objects.filter( + **{f'{key}__in': [instance.id for instance in instances]}, reference__collection_id=collection.id + ).order_by('reference_id').values_list(key, 'reference_id'): + reference_ids.setdefault(row_id, []).append(reference_id) + for instance in instances: + instance._collection_reference_ids = reference_ids.get(instance.id, []) # pylint: disable=protected-access + + def get_collection_reference_ids(self, collection): + if hasattr(self, '_collection_reference_ids'): # loaded for a whole page by load_list_fields + return self._collection_reference_ids + return list(self.collection_references(collection).order_by('id').values_list('id', flat=True)) + + @classmethod + def load_collection_source_versions(cls, instances, collection): + """ + Sets each row's _collection_source_version: the newest source version its references in this collection + resolved to, as TBv3 shows a collection's rows, or None when they resolved to HEAD or to no source version. + """ + reference_ids = {instance.id: instance.get_collection_reference_ids(collection) for instance in instances} + references = cls._meta.get_field('references').related_model.objects.in_bulk( + {reference_id for ids in reference_ids.values() for reference_id in ids}) + + resolved = {} + for instance in instances: + versions = compact( + references[reference_id]._get_resolved_system_version(resolved) # pylint: disable=protected-access + for reference_id in reference_ids[instance.id] + ) + newest = max(versions, key=lambda version: version.created_at, default=None) + # pylint: disable=protected-access + instance._collection_source_version = None if get(newest, 'version') == HEAD else newest + def calculate_uri(self): uri = self.parent.uri + self.resource_type.lower() + 's/' + str(self.mnemonic) + '/' if not self.is_head: @@ -660,9 +794,9 @@ def is_in_latest_source_version(self): @property def latest_source_version(self): - if self.is_in_latest_source_version: - return self._cached_latest_source_version - return None + if hasattr(self, '_latest_source_version'): # loaded for the whole page by load_list_fields + return self._latest_source_version + return self.sources.exclude(version=HEAD).order_by('-created_at').first() @cached_property def _cached_latest_source_version(self): @@ -736,12 +870,16 @@ def get_filters_for_criterion(params, prefix=None): @property def source_versions(self): - return self.sources.exclude(version=HEAD).values_list('uri', flat=True) + if hasattr(self, '_source_versions'): # loaded for a whole page by load_list_fields + return self._source_versions + return list(self.sources.exclude(version=HEAD).order_by('created_at').values_list('uri', flat=True)) @property def collection_versions(self): - return set(self.expansion_set.exclude( - collection_version__version=HEAD).values_list('collection_version__uri', flat=True)) + if hasattr(self, '_collection_versions'): # loaded for a whole page by load_list_fields + return self._collection_versions + return sorted(set(self.expansion_set.exclude( + collection_version__version=HEAD).values_list('collection_version__uri', flat=True))) @property def versions(self): @@ -755,7 +893,7 @@ def is_versioned_object(self): @property def version_url(self): if self.is_versioned_object: - return self.get_latest_version().uri + return getattr(self, '_latest_version_uri', None) or self.get_latest_version().uri return self.uri @property @@ -916,8 +1054,7 @@ def save(self, *args, force_insert=False, force_update=False, using=None, update self.save(update_fields=['_counted']) def collection_references_uris(self, collection): - ids = self.collection_references(collection).values_list('id', flat=True) - return [f"{collection.uri}references/{_id}/" for _id in ids] + return [f"{collection.uri}references/{_id}/" for _id in self.get_collection_reference_ids(collection)] def collection_references(self, collection): return self.references.filter(collection=collection) diff --git a/core/common/models.py b/core/common/models.py index 4a54fbde..782cde90 100644 --- a/core/common/models.py +++ b/core/common/models.py @@ -683,6 +683,8 @@ def prev_version(self): @property def prev_version_uri(self): + if hasattr(self, '_prev_version_uri'): # loaded for a whole page by SourceChildMixin.load_list_fields + return self._prev_version_uri return get(self, 'prev_version.uri') @property diff --git a/core/common/serializers.py b/core/common/serializers.py index bc00f63e..a2c7bb20 100644 --- a/core/common/serializers.py +++ b/core/common/serializers.py @@ -1,6 +1,7 @@ +from django.db.models.manager import BaseManager from pydash import get from rest_framework.fields import CharField, JSONField, SerializerMethodField, FloatField, ChoiceField -from rest_framework.serializers import Serializer, Field, ValidationError, ModelSerializer +from rest_framework.serializers import Serializer, Field, ValidationError, ModelSerializer, ListSerializer from core import settings from core.code_systems.constants import RESOURCE_TYPE as CODE_SYSTEM_RESOURCE_TYPE @@ -225,6 +226,38 @@ def get_search_highlight(obj): return cleaned_highlight +def get_collection_in_scope(context): + """The collection (version) a concept or mapping view is scoped to, when the view has loaded it.""" + if 'collection' in get(context, 'view.kwargs', {}): + instance = get(context, 'request.instance') + if get(instance, 'resource_type') == 'Collection': + return instance + return None + + +class SourceVersionFieldMixin: + """ + latest_source_version for concepts and mappings: the latest released source version holding the row or, listed + in a collection, the source version the row's references there resolved to (ocl_issues#2770). + """ + def get_latest_source_version(self, obj): + collection = get_collection_in_scope(self.context) + if collection: + if not hasattr(obj, '_collection_source_version'): + obj.load_collection_source_versions([obj], collection) + return get(obj, '_collection_source_version.version') + return get(obj.latest_source_version, 'version') + + +class SourceChildPageSerializer(ListSerializer): # pylint: disable=abstract-method + """Serializes a page of concepts or mappings, loading the per-row lookups for the whole page first.""" + def to_representation(self, data): + instances = list(data.all() if isinstance(data, BaseManager) else data) + self.child.Meta.model.load_list_fields( + instances, self.child.fields.keys(), get_collection_in_scope(self.context)) + return super().to_representation(instances) + + class AbstractResourceSerializer(ModelSerializer): search_meta = SerializerMethodField() diff --git a/core/concepts/models.py b/core/concepts/models.py index 5628da87..6cd51547 100644 --- a/core/concepts/models.py +++ b/core/concepts/models.py @@ -220,6 +220,10 @@ class HierarchicalConcepts(models.Model): class Concept(ConceptValidationMixin, SourceChildMixin, VersionedModel): # pylint: disable=too-many-public-methods + LIST_PREFETCHES = ('parent', 'created_by', 'updated_by', 'names') + FIELD_PREFETCHES = ('descriptions', ) + LATEST_SOURCE_VERSION_FIELDS = ('latest_source_version', 'property') # property: get_parent_source_version + class Meta: db_table = 'concepts' unique_together = ('mnemonic', 'version', 'parent') @@ -979,11 +983,9 @@ def __clone_locales(locales): def is_existing_in_parent(self): return self.parent.concepts_set.filter(mnemonic__exact=self.mnemonic).exists() - @property - def latest_source_version(self): - return self.sources.exclude(version=HEAD).order_by('-created_at').first() - def get_source_version_before_creation(self): + if hasattr(self, '_latest_source_version') and not self._latest_source_version: + return None # load_list_fields found no released version holding this row return self.sources.exclude(version=HEAD).filter( created_at__lte=self.created_at).order_by('-created_at').first() diff --git a/core/concepts/serializers.py b/core/concepts/serializers.py index 5e8229b9..6c77557e 100644 --- a/core/concepts/serializers.py +++ b/core/concepts/serializers.py @@ -9,7 +9,8 @@ INCLUDE_CHILD_CONCEPT_URLS, HEAD, INCLUDE_SUMMARY, INCLUDE_VERBOSE_REFERENCES, VERBOSE_PARAM, \ INCLUDE_RETIRED_PARAM, ACCESS_TYPE_NONE from core.common.fields import EncodedDecodedCharField -from core.common.serializers import AbstractResourceSerializer +from core.common.serializers import AbstractResourceSerializer, SourceChildPageSerializer, \ + SourceVersionFieldMixin, get_collection_in_scope from core.common.utils import to_parent_uri_from_kwargs, get_truthy_values from core.concepts.models import Concept, ConceptName @@ -135,7 +136,7 @@ def to_representation(self, instance): # used to be to_native return ret -class ConceptAbstractSerializer(AbstractResourceSerializer): +class ConceptAbstractSerializer(SourceVersionFieldMixin, AbstractResourceSerializer): uuid = CharField(source='id', read_only=True) mappings = SerializerMethodField() mappings_payload = ListField(child=JSONField(), write_only=True, required=False, allow_empty=True) @@ -204,7 +205,7 @@ def __init__(self, *args, **kwargs): # pylint: disable=too-many-branches self.fields.pop('references', None) if get(params, 'onlyParentLess') not in TRUTHY and get(params, 'onlyHierarchyRoot') not in TRUTHY: self.fields.pop('has_children', None) - if 'collection' in self.view_kwargs: + if 'collection' in self.view_kwargs and not get_collection_in_scope(get(kwargs, 'context')): self.fields.pop('latest_source_version', None) except: # pylint: disable=bare-except pass @@ -317,12 +318,12 @@ class ConceptListSerializer(ConceptAbstractSerializer): version_created_by = DateTimeField(source='created_by.username', read_only=True) version_updated_on = DateTimeField(source='updated_at', read_only=True) version_updated_by = DateTimeField(source='updated_by.username', read_only=True) - latest_source_version = CharField( - source='latest_source_version.version', allow_null=True, allow_blank=True, read_only=True, required=False) + latest_source_version = SerializerMethodField() property = JSONField(source='summary_properties', read_only=True) class Meta: model = Concept + list_serializer_class = SourceChildPageSerializer fields = ConceptAbstractSerializer.Meta.fields + ( 'uuid', 'id', 'external_id', 'concept_class', 'datatype', 'url', 'retired', 'source', 'owner', 'owner_type', 'owner_url', 'display_name', 'display_locale', 'version', 'update_comment', @@ -339,6 +340,7 @@ class ConceptVersionListSerializer(ConceptListSerializer): class Meta: model = Concept + list_serializer_class = SourceChildPageSerializer fields = ConceptListSerializer.Meta.fields + ( 'previous_version_url', 'source_versions', 'collection_versions' ) @@ -355,7 +357,7 @@ def __init__(self, *args, **kwargs): self.fields.pop('source_versions', None) if not self.include_collection_versions: self.fields.pop('collection_versions', None) - if 'collection' in self.view_kwargs: + if 'collection' in self.view_kwargs and not get_collection_in_scope(get(kwargs, 'context')): self.fields.pop('latest_source_version', None) except: # pylint: disable=bare-except pass @@ -479,12 +481,12 @@ class ConceptDetailSerializer(ConceptAbstractSerializer): url = CharField(required=False, source='versioned_object_url') updated_by = DateTimeField(source='updated_by.username', read_only=True) created_by = DateTimeField(source='created_by.username', read_only=True) - latest_source_version = CharField( - source='latest_source_version.version', allow_null=True, allow_blank=True, read_only=True, required=False) + latest_source_version = SerializerMethodField() property = JSONField(source='properties', read_only=True) class Meta: model = Concept + list_serializer_class = SourceChildPageSerializer fields = ConceptAbstractSerializer.Meta.fields + ( 'id', 'external_id', 'concept_class', 'datatype', 'url', 'retired', 'source', 'owner', 'owner_type', 'owner_url', 'display_name', 'display_locale', 'names', 'descriptions', @@ -573,7 +575,7 @@ def get_child_concept_urls(obj): return list(uris) -class ConceptVersionDetailSerializer(ModelSerializer): +class ConceptVersionDetailSerializer(SourceVersionFieldMixin, ModelSerializer): type = CharField(source='resource_type') uuid = CharField(source='id') id = EncodedDecodedCharField(source='mnemonic') @@ -600,8 +602,7 @@ class ConceptVersionDetailSerializer(ModelSerializer): source_versions = ListField(read_only=True) collection_versions = ListField(read_only=True) references = SerializerMethodField() - latest_source_version = CharField( - source='latest_source_version.version', allow_null=True, allow_blank=True, read_only=True, required=False) + latest_source_version = SerializerMethodField() def __init__(self, *args, **kwargs): request = get(kwargs, 'context.request') @@ -630,7 +631,7 @@ def __init__(self, *args, **kwargs): self.fields.pop('parent_concept_urls') if not get(request, 'instance'): self.fields.pop('references', None) - if 'collection' in self.view_kwargs: + if 'collection' in self.view_kwargs and not get_collection_in_scope(get(kwargs, 'context')): self.fields.pop('latest_source_version', None) except: # pylint: disable=bare-except pass @@ -639,6 +640,7 @@ def __init__(self, *args, **kwargs): class Meta: model = Concept + list_serializer_class = SourceChildPageSerializer fields = ( 'type', 'uuid', 'id', 'external_id', 'concept_class', 'datatype', 'display_name', 'display_locale', 'names', 'descriptions', 'extras', 'retired', 'source', 'source_url', 'owner', 'owner_name', 'owner_url', diff --git a/core/integration_tests/test_utils.py b/core/integration_tests/test_utils.py index a4ee2161..f5b23c72 100644 --- a/core/integration_tests/test_utils.py +++ b/core/integration_tests/test_utils.py @@ -1,8 +1,16 @@ import json import os +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 jsonpath_ng import parse +from core.collections.tests.factories import OrganizationCollectionFactory, ExpansionFactory +from core.sources.tests.factories import OrganizationSourceFactory + def load_json(file, parent_dir=None): if parent_dir: @@ -33,3 +41,90 @@ def ignore_json_paths(self, json_input, json_response, paths): for path in paths: self.update(json_input, path, None) self.update(json_response, path, None) + + +class SourceChildListQueryCountMixin: + """ + A page of concepts or mappings costs the same number of queries whatever its size, and serializes as row by row + did. Set model, relation ('concepts' or 'mappings'), list_serializer and detail_serializer; setUp creates 4 rows + in self.source and calls set_released_versions. + """ + model = None + relation = None + list_serializer = None + detail_serializer = None + + def get_page(self, url, limit, params=''): + with CaptureQueriesContext(connection) as context: + response = self.client.get(f'{url}?limit={limit}{params}', format='json') + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data), limit) + return response, len(context.captured_queries) + + def assert_page(self, url, serializer_class, params=''): + """Same query count for a page of 2 and of 4; same output as serializing each row on its own.""" + _, small_page_queries = self.get_page(url, 2, params) + response, page_queries = self.get_page(url, 4, params) + self.assertEqual(page_queries, small_page_queries) + + context = {'request': response.renderer_context['request'], 'view': response.renderer_context['view']} + rows = [ + self.model.objects.get(versioned_object_id=row['versioned_object_id'], version=row['version']) + for row in response.data + ] + self.assertEqual(response.data, [serializer_class(row, context=context).data for row in rows]) + return response.data, rows + + def set_released_versions(self, rows): + """ + Newest first by updated_at in creation order, so a page of 2 and a page of 4 hold the same kinds of rows. + v1 holds every latest version row, v2 every other one; both hold the newest head as well. + """ + self.heads = list(rows.filter(id=F('versioned_object_id')).order_by('id')) + now = timezone.now() + for age, row in enumerate(rows.order_by('-id')): + self.model.objects.filter(id=row.id).update(updated_at=now - timedelta(minutes=age)) + versions = list(rows.exclude(id=F('versioned_object_id')).filter(is_latest_version=True).order_by('id')) + self.v1 = OrganizationSourceFactory( + mnemonic=self.source.mnemonic, organization=self.source.organization, version='v1') + self.v2 = OrganizationSourceFactory( + mnemonic=self.source.mnemonic, organization=self.source.organization, version='v2') + getattr(self.v1, self.relation).add(*versions, self.heads[-1]) + getattr(self.v2, self.relation).add(*versions[::2], self.heads[-1]) + self.in_v2 = {row.id for row in versions[::2]} | {self.heads[-1].id} + + def test_head_list(self): + url = getattr(self.source, f'{self.relation}_url') + data, rows = self.assert_page(url, self.list_serializer) + self.assert_page(url, self.detail_serializer, '&verbose=true') + + for row, instance in zip(data, rows): + self.assertEqual(row['version_url'], instance.get_latest_version().uri) + self.assertEqual(row['latest_source_version'], 'v2' if instance.id == self.heads[-1].id else None) + + def test_version_list(self): + url = getattr(self.v1, f'{self.relation}_url') + data, rows = self.assert_page(url, self.list_serializer) + self.assert_page(url, self.detail_serializer, '&verbose=true') + + for row, instance in zip(data, rows): + self.assertEqual(row['latest_source_version'], 'v2' if instance.id in self.in_v2 else 'v1') + + def test_collection_list(self): + """In a collection, rows show the source version their references resolved to (v1), not the latest one.""" + collection = OrganizationCollectionFactory() + expansion = ExpansionFactory(collection_version=collection) + collection.expansion_uri = expansion.uri + collection.save() + collection.add_expressions( + {'expressions': [f'{self.v1.uri}{self.relation}/{head.mnemonic}/' for head in self.heads]}, + collection.created_by) + + url = getattr(collection, f'{self.relation}_url') + data, _ = self.assert_page(url, self.model.get_serializer_class(version=True)) + self.assert_page(url, self.model.get_serializer_class(verbose=True, version=True), '&verbose=true') + self.assertEqual({row['latest_source_version'] for row in data}, {'v1'}) + + def test_global_list(self): + self.assert_page(f'/{self.relation}/', self.list_serializer) + self.assert_page(f'/{self.relation}/', self.detail_serializer, '&verbose=true') diff --git a/core/integration_tests/tests_concepts.py b/core/integration_tests/tests_concepts.py index 2ef60d9d..eda91604 100644 --- a/core/integration_tests/tests_concepts.py +++ b/core/integration_tests/tests_concepts.py @@ -4,9 +4,13 @@ from celery.states import PENDING from django.core.cache import cache +from django.db import connection from django.db.models import F from django.test import override_settings +from django.test.utils import CaptureQueriesContext from mock import ANY +from rest_framework.request import Request +from rest_framework.test import APIRequestFactory from core.bundles.models import Bundle from core.sources.models import CloneLimitExceeded @@ -16,7 +20,9 @@ from core.concepts.constants import CONCEPT_HARD_DELETE_REQUIRES_HEAD_ONLY from core.concepts.documents import ConceptDocument from core.concepts.models import Concept +from core.concepts.serializers import ConceptListSerializer, ConceptDetailSerializer from core.concepts.tests.factories import ConceptFactory, ConceptNameFactory, ConceptDescriptionFactory +from core.integration_tests.test_utils import SourceChildListQueryCountMixin from core.mappings.models import Mapping from core.mappings.tests.factories import MappingFactory from core.orgs.models import Organization @@ -4175,3 +4181,49 @@ def test_post_success(self, bundle_clone_mock): self.concept, self.concept.parent, self.clone_to_source, self.user, ANY, False, resource_budget=100, **parameters # no group: the preview per-call budget (ocl_online#230) ) + + +class ConceptListQueryCountTest(SourceChildListQueryCountMixin, OCLAPITestCase): + """A page of concepts costs the same number of queries whatever its size, and serializes as row by row did.""" + model = Concept + relation = 'concepts' + list_serializer = ConceptListSerializer + detail_serializer = ConceptDetailSerializer + + def setUp(self): + super().setUp() + self.source = OrganizationSourceFactory() + for _ in range(4): + ConceptFactory( + parent=self.source, + names=[ + ConceptNameFactory.build(locale='en', locale_preferred=True), ConceptNameFactory.build(locale='fr') + ], + descriptions=[ConceptDescriptionFactory.build(locale='en')]) + concepts = Concept.objects.filter(parent=self.source) + for concept in concepts: # a GET calculates and saves missing checksums, one row at a time + concept.set_checksums() + for locale in [*concept.names.all(), *concept.descriptions.all()]: + locale.set_checksums() + self.set_released_versions(concepts) + + def test_revisions_in_no_released_version(self): + """Search results can hold revisions saved since the last release: no per-row lookup for those either.""" + self.v1.concepts.clear() + self.v2.concepts.clear() + revisions = list(Concept.objects.filter(parent=self.source, is_latest_version=True).exclude( + id=F('versioned_object_id')).order_by('id')) + context = {'request': Request(APIRequestFactory().get('/'))} + + def serialize(rows): # fetched as search results are: by id, names prefetched + rows = list( + Concept.objects.filter(id__in=[row.id for row in rows]).prefetch_related('names').order_by('id')) + with CaptureQueriesContext(connection) as queries: + data = ConceptListSerializer(rows, many=True, context=context).data + return data, len(queries.captured_queries) + + _, small_page_queries = serialize(revisions[:2]) + data, page_queries = serialize(revisions) + self.assertEqual(page_queries, small_page_queries) + self.assertEqual(data, [ConceptListSerializer(row, context=context).data for row in revisions]) + self.assertEqual({row['latest_source_version'] for row in data}, {None}) diff --git a/core/integration_tests/tests_mappings.py b/core/integration_tests/tests_mappings.py index 0c9b5e38..db7e9a02 100644 --- a/core/integration_tests/tests_mappings.py +++ b/core/integration_tests/tests_mappings.py @@ -5,9 +5,11 @@ from core.collections.tests.factories import OrganizationCollectionFactory, ExpansionFactory from core.common.tests import OCLAPITestCase from core.concepts.tests.factories import ConceptFactory, ConceptNameFactory +from core.integration_tests.test_utils import SourceChildListQueryCountMixin from core.mappings.constants import SAME_AS from core.mappings.documents import MappingDocument from core.mappings.models import Mapping +from core.mappings.serializers import MappingListSerializer, MappingDetailSerializer from core.mappings.tests.factories import MappingFactory from core.orgs.tests.factories import OrganizationFactory from core.sources.tests.factories import UserSourceFactory, OrganizationSourceFactory @@ -1076,3 +1078,31 @@ def test_repo_version_mappings_with_search_are_not_cached(self): self.assertIsNone(cache.get(body_key)) self.assertIsNone(cache.get(headers_key)) self.assertIsNone(cache.get(body_key + '?q=123')) + + +class MappingListQueryCountTest(SourceChildListQueryCountMixin, OCLAPITestCase): + """A page of mappings costs the same number of queries whatever its size, and serializes as row by row did.""" + model = Mapping + relation = 'mappings' + list_serializer = MappingListSerializer + detail_serializer = MappingDetailSerializer + + def setUp(self): + super().setUp() + self.source = OrganizationSourceFactory() + target = OrganizationSourceFactory() + for index in range(4): # external, internal, external, internal + from_concept = ConceptFactory(parent=self.source, names=[ConceptNameFactory.build(locale_preferred=True)]) + if index % 2: + to_concept = ConceptFactory(parent=target, names=[ConceptNameFactory.build(locale_preferred=True)]) + MappingFactory( + parent=self.source, from_concept=from_concept, from_source=self.source, to_concept=to_concept, + to_source=target) + else: + MappingFactory( + parent=self.source, from_concept=from_concept, from_source=self.source, to_concept=None, + to_source=target, to_concept_code=f'X{index}', to_source_url=target.uri) + mappings = Mapping.objects.filter(parent=self.source) + for mapping in mappings: + mapping.set_checksums() # a GET calculates and saves missing checksums, one row at a time + self.set_released_versions(mappings) diff --git a/core/mappings/models.py b/core/mappings/models.py index 90d57654..e183d837 100644 --- a/core/mappings/models.py +++ b/core/mappings/models.py @@ -20,6 +20,12 @@ class Mapping(MappingValidationMixin, SourceChildMixin, VersionedModel): + LIST_PREFETCHES = ( + 'from_concept__parent', 'to_concept__parent', 'from_source__organization', 'from_source__user', + 'to_source__organization', 'to_source__user', 'created_by', 'updated_by', 'from_concept__names', + 'to_concept__names' + ) + class Meta: db_table = 'mappings' unique_together = ('mnemonic', 'version', 'parent') @@ -414,10 +420,6 @@ def get_source(url): def is_existing_in_parent(self): return self.parent.mappings_set.filter(mnemonic__exact=self.mnemonic).exists() - @property - def latest_source_version(self): - return self.sources.exclude(version=HEAD).order_by('-created_at').first() - @classmethod def create_new_version_for(cls, instance, data, user, cache=None): instance.populate_fields_from_relations(data, cache=cache, user=user) diff --git a/core/mappings/serializers.py b/core/mappings/serializers.py index 74a67405..df6d17cd 100644 --- a/core/mappings/serializers.py +++ b/core/mappings/serializers.py @@ -7,7 +7,8 @@ MAPPING_LOOKUP_TO_CONCEPT, MAPPING_LOOKUP_FROM_SOURCE, MAPPING_LOOKUP_TO_SOURCE, INCLUDE_EXTRAS_PARAM, \ INCLUDE_SOURCE_VERSIONS, INCLUDE_COLLECTION_VERSIONS, INCLUDE_VERBOSE_REFERENCES from core.common.fields import EncodedDecodedCharField -from core.common.serializers import AbstractResourceSerializer +from core.common.serializers import AbstractResourceSerializer, SourceChildPageSerializer, \ + SourceVersionFieldMixin, get_collection_in_scope from core.common.utils import get_truthy_values from core.concepts.serializers import ConceptDetailSerializer from core.mappings.models import Mapping @@ -17,7 +18,7 @@ TRUTHY = get_truthy_values() -class AbstractMappingSerializer(AbstractResourceSerializer): +class AbstractMappingSerializer(SourceVersionFieldMixin, AbstractResourceSerializer): from_concept = SerializerMethodField() to_concept = SerializerMethodField() from_source = SourceListSerializer() @@ -64,7 +65,7 @@ def __init__(self, *args, **kwargs): if not get(request, 'instance'): self.fields.pop('references', None) - if 'collection' in self.view_kwargs: + if 'collection' in self.view_kwargs and not get_collection_in_scope(get(kwargs, 'context')): self.fields.pop('latest_source_version', None) super().__init__(*args, **kwargs) @@ -119,11 +120,11 @@ class MappingListSerializer(AbstractMappingSerializer): sort_weight = FloatField(required=False, allow_null=True) version_updated_on = DateTimeField(source='updated_at', read_only=True) version_updated_by = DateTimeField(source='updated_by.username', read_only=True) - latest_source_version = CharField( - source='latest_source_version.version', allow_null=True, allow_blank=True, read_only=True, required=False) + latest_source_version = SerializerMethodField() class Meta: model = Mapping + list_serializer_class = SourceChildPageSerializer fields = AbstractMappingSerializer.Meta.fields + ( 'external_id', 'retired', 'map_type', 'source', 'owner', 'owner_type', 'from_concept_code', 'from_concept_name', 'from_concept_url', @@ -145,6 +146,7 @@ class MappingVersionListSerializer(MappingListSerializer): class Meta: model = Mapping + list_serializer_class = SourceChildPageSerializer fields = MappingListSerializer.Meta.fields + ( 'previous_version_url', 'source_versions', 'collection_versions' ) @@ -247,11 +249,11 @@ class MappingDetailSerializer(MappingListSerializer): to_source = SourceDetailSerializer() created_on = DateTimeField(source='created_at', read_only=True) updated_on = DateTimeField(source='updated_at', read_only=True) - latest_source_version = CharField( - source='latest_source_version.version', allow_null=True, allow_blank=True, read_only=True, required=False) + latest_source_version = SerializerMethodField() class Meta: model = Mapping + list_serializer_class = SourceChildPageSerializer fields = MappingListSerializer.Meta.fields + ( 'type', 'uuid', 'extras', 'created_on', 'updated_on', 'created_by', 'updated_by', 'parent_id', 'public_can_view', 'latest_source_version' @@ -322,6 +324,7 @@ class MappingVersionDetailSerializer(MappingDetailSerializer): class Meta: model = Mapping + list_serializer_class = SourceChildPageSerializer fields = MappingDetailSerializer.Meta.fields + ( 'previous_version_url', 'source_versions', 'collection_versions', )