diff --git a/website/admin/person_admin.py b/website/admin/person_admin.py index ebd432f7..0923181f 100644 --- a/website/admin/person_admin.py +++ b/website/admin/person_admin.py @@ -1,12 +1,13 @@ from django import forms from django.contrib import admin from django.core.files import File -from website.models import Position, Person, ProjectRole +from website.models import Position, Person, ProjectRole, Publication, Talk from website.models.position import Title from website.models.person import PERSON_THUMBNAIL_SIZE from easy_thumbnails.exceptions import InvalidImageFormatError # for handling invalid images from website.admin_list_filters import PositionRoleListFilter, PositionTitleListFilter -from website.admin.utils import get_active_professors_queryset, get_active_mentors_queryset +from website.admin.utils import (get_active_professors_queryset, get_active_mentors_queryset, + related_count_subquery) from image_cropping import ImageCroppingMixin from image_cropping.widgets import EasterEggCropImageWidget import website.utils.fileutils as ml_fileutils @@ -179,22 +180,67 @@ def save_model(self, request, obj, form, change): # The list display lets us control what is shown in the default persons table at Home > Website > People # info on displaying multiple entries comes from http://stackoverflow.com/questions/9164610/custom-columns-using-django-admin - list_display = ('get_full_name', 'get_display_thumbnail', 'get_current_title', 'get_current_role', 'is_active', - 'get_start_date', 'get_cur_pos_start_date', 'get_end_date', 'recent_projects', 'get_project_count', 'get_pub_count', - 'get_talk_count', 'display_time_current_position', 'display_total_time_as_member') + # The count columns (project_count / pub_count / talk_count) read annotations + # set in get_queryset() rather than the per-row model count methods (#1346). + list_display = ('get_full_name', 'get_display_thumbnail', 'get_current_title', 'get_current_role', 'is_active', + 'get_start_date', 'get_cur_pos_start_date', 'get_end_date', 'recent_projects', 'project_count', 'pub_count', + 'talk_count', 'display_time_current_position', 'display_total_time_as_member') list_filter = (PositionRoleListFilter, PositionTitleListFilter) + # The changelist renders ~14 columns of position/count data per person; cap + # the page so the per-row thumbnail filesystem check stays bounded (#1346). + list_per_page = 50 + + def get_queryset(self, request): + """Make the People changelist issue a roughly constant number of queries + regardless of how many people are listed (the #1346 perf audit). + + - ``position_set`` is prefetched because nearly every column ("current + title/role", dates, durations, is_active) and the Role filter funnel + through ``Person.get_latest_position``, which now reads this prefetch + cache instead of issuing ``.latest()`` per row. + - ``projectrole_set__project`` backs :meth:`recent_projects`. + - the three ``_*_count`` annotations back the sortable count columns, + replacing three per-row ``COUNT(*)`` queries with scalar subqueries. + """ + return (super().get_queryset(request) + .prefetch_related('position_set', 'projectrole_set__project') + .annotate( + _project_count=related_count_subquery(ProjectRole, 'person'), + _pub_count=related_count_subquery(Publication, 'authors'), + _talk_count=related_count_subquery(Talk, 'authors'), + )) + def recent_projects(self, obj): - # Get the three most recent projects for this person based on start_date - recent_projects = (ProjectRole.objects.filter(person=obj) - .order_by('-start_date')[:3]) - - # Return the project names as a comma-separated string - return ', '.join([str(project.project) for project in recent_projects]) + """The person's three most recent project roles (by start_date), as a + comma-separated list of project names. Reads ``obj.projectrole_set.all()`` + (prefetched with its ``project`` in :meth:`get_queryset`) and sorts in + Python, so it adds no per-row queries on the changelist.""" + roles = sorted(obj.projectrole_set.all(), + key=lambda role: role.start_date, reverse=True)[:3] + return ', '.join(str(role.project) for role in roles) recent_projects.short_description = 'Recent Projects' # Sets column name in admin interface + def project_count(self, obj): + """Number of project roles (annotated in get_queryset; sortable).""" + return obj._project_count + project_count.short_description = 'Projects' + project_count.admin_order_field = '_project_count' + + def pub_count(self, obj): + """Number of publications authored (annotated in get_queryset; sortable).""" + return obj._pub_count + pub_count.short_description = 'Pubs' + pub_count.admin_order_field = '_pub_count' + + def talk_count(self, obj): + """Number of talks given (annotated in get_queryset; sortable).""" + return obj._talk_count + talk_count.short_description = 'Talks' + talk_count.admin_order_field = '_talk_count' + def display_time_current_position(self, obj): """Displays the time in the current position""" duration = obj.get_time_in_current_position diff --git a/website/admin/project_admin.py b/website/admin/project_admin.py index fa15e5f3..28b5d77e 100644 --- a/website/admin/project_admin.py +++ b/website/admin/project_admin.py @@ -1,6 +1,7 @@ from django.contrib import admin from django.contrib.admin import widgets -from website.models import Project, Banner, Photo, ProjectRole, Grant +from website.models import (Project, Banner, Photo, ProjectRole, Grant, + Publication, Talk, Video, Person) from website.models.project import PROJECT_THUMBNAIL_SIZE from website.admin_list_filters import ActiveProjectsFilter from image_cropping import ImageCroppingMixin @@ -9,9 +10,27 @@ from easy_thumbnails.files import get_thumbnailer # for generating thumbnails import os # for checking if thumbnail file exists from django import forms -from django.db.models import F +from django.db.models import F, Q, Count, OuterRef, Subquery, IntegerField, Value +from django.db.models.functions import Greatest, Coalesce +from website.admin.utils import related_count_subquery from website.admin.admin_site import ml_admin_site + +def _contributor_count_subquery(): + """Distinct count of people who either hold a ProjectRole on the project OR + are an author on one of its publications — matching Project.get_contributors, + which unions those two sets (#1346). One correlated subquery: count distinct + Person ids matching either relation for the outer project, grouped by a + constant so it returns a single scalar.""" + people = (Person.objects + .filter(Q(projectrole__project=OuterRef('pk')) | + Q(publication__projects=OuterRef('pk'))) + .order_by() + .values(_grp=Value(1)) # group by a constant -> one row + .annotate(_c=Count('pk', distinct=True)) + .values('_c')[:1]) + return Coalesce(Subquery(people, output_field=IntegerField()), 0) + class ProjectRoleInline(admin.TabularInline): model = ProjectRole extra = 1 # Number of extra forms displayed @@ -53,11 +72,16 @@ class ProjectAdmin(ImageCroppingMixin, admin.ModelAdmin): # The list display lets us control what is shown in the Project table at Home > Website > Project # info on displaying multiple entries comes from http://stackoverflow.com/questions/9164610/custom-columns-using-django-admin + # The count / most-recent-artifact columns read annotations set in + # get_queryset() (sortable) rather than the per-row model methods (#1346). list_display = ('name', 'is_visible', 'get_display_thumbnail', 'start_date', 'end_date', 'has_ended', - 'get_contributor_count', 'get_people_count', - 'get_current_member_count', 'get_past_member_count', - 'get_most_recent_artifact_date', 'get_most_recent_artifact_type', - 'get_publication_count', 'get_video_count', 'get_talk_count', 'get_banner_count') + 'contributor_count', 'people_count', + 'current_member_count', 'past_member_count', + 'most_recent_artifact_date', 'most_recent_artifact_type', + 'pub_count', 'video_count', 'talk_count', 'banner_count') + + # Bounds the per-row gallery-image filesystem check on the changelist (#1346). + list_per_page = 50 fieldsets = [ (None, {'fields': ['name', 'short_name', 'is_visible']}), @@ -68,6 +92,109 @@ class ProjectAdmin(ImageCroppingMixin, admin.ModelAdmin): list_filter = (ActiveProjectsFilter, 'is_visible') + def get_queryset(self, request): + """Collapse the Project changelist's ~10 per-row count/aggregate columns + into a single query (#1346). Each count is an independent scalar subquery + (see related_count_subquery), so the joins don't multiply each other; the + three most-recent-artifact dates are scalar subqueries combined into one + sortable date. With the default 100 projects this turns ~1,000+ queries + into a handful. + + The model methods (get_publication_count, get_most_recent_artifact, ...) + are left intact for the public site / detail views; only the changelist + columns are repointed at these annotations. + """ + def latest_artifact_date(model): + # Most recent artifact date for this project, as a scalar subquery. + return Subquery( + model.objects.filter(projects=OuterRef('pk')) + .order_by('-date').values('date')[:1] + ) + + return (super().get_queryset(request).annotate( + _pub_count=related_count_subquery(Publication, 'projects'), + _talk_count=related_count_subquery(Talk, 'projects'), + _video_count=related_count_subquery(Video, 'projects'), + _banner_count=related_count_subquery(Banner, 'project'), + _people_count=related_count_subquery( + ProjectRole, 'project', count_field='person', distinct=True), + _current_member_count=related_count_subquery( + ProjectRole, 'project', count_field='person', distinct=True, + extra_filter=Q(end_date__isnull=True)), + _past_member_count=related_count_subquery( + ProjectRole, 'project', count_field='person', distinct=True, + extra_filter=Q(end_date__isnull=False)), + _contributor_count=_contributor_count_subquery(), + _pub_max_date=latest_artifact_date(Publication), + _talk_max_date=latest_artifact_date(Talk), + _video_max_date=latest_artifact_date(Video), + ).annotate( + # Greatest ignores NULLs on Postgres, so this is the max of whichever + # artifact types exist (NULL only when the project has none). + _recent_artifact_date=Greatest( + '_pub_max_date', '_talk_max_date', '_video_max_date'), + )) + + # --- Annotation-backed changelist columns (sortable; see get_queryset) --- + + def pub_count(self, obj): + return obj._pub_count + pub_count.short_description = 'Pubs' + pub_count.admin_order_field = '_pub_count' + + def video_count(self, obj): + return obj._video_count + video_count.short_description = 'Videos' + video_count.admin_order_field = '_video_count' + + def talk_count(self, obj): + return obj._talk_count + talk_count.short_description = 'Talks' + talk_count.admin_order_field = '_talk_count' + + def banner_count(self, obj): + return obj._banner_count + banner_count.short_description = 'Banners' + banner_count.admin_order_field = '_banner_count' + + def people_count(self, obj): + return obj._people_count + people_count.short_description = 'Num People' + people_count.admin_order_field = '_people_count' + + def current_member_count(self, obj): + return obj._current_member_count + current_member_count.short_description = 'Num Current Members' + current_member_count.admin_order_field = '_current_member_count' + + def past_member_count(self, obj): + return obj._past_member_count + past_member_count.short_description = 'Num Past Members' + past_member_count.admin_order_field = '_past_member_count' + + def contributor_count(self, obj): + return obj._contributor_count + contributor_count.short_description = 'Contributors' + contributor_count.admin_order_field = '_contributor_count' + + def most_recent_artifact_date(self, obj): + return obj._recent_artifact_date + most_recent_artifact_date.short_description = 'Most Recent Artifact Date' + most_recent_artifact_date.admin_order_field = '_recent_artifact_date' + + def most_recent_artifact_type(self, obj): + """Type ('Publication' / 'Talk' / 'Video') of the most recent artifact, + derived from the three annotated max-dates. Ties resolve in the same + order as Project.get_most_recent_artifact (pub, then talk, then video).""" + candidates = [('Publication', obj._pub_max_date), + ('Talk', obj._talk_max_date), + ('Video', obj._video_max_date)] + candidates = [(label, d) for label, d in candidates if d is not None] + if not candidates: + return None + return max(candidates, key=lambda c: c[1])[0] + most_recent_artifact_type.short_description = 'Most Recent Artifact Type' + def get_display_thumbnail(self, obj): if obj.gallery_image and os.path.isfile(obj.gallery_image.path): # Use easy_thumbnails to generate a thumbnail diff --git a/website/admin/utils.py b/website/admin/utils.py index 7c68a7fd..c4a334a3 100644 --- a/website/admin/utils.py +++ b/website/admin/utils.py @@ -9,13 +9,49 @@ person_admin.py and position_admin.py. """ -from django.db.models import Q, Case, When, Value, IntegerField +from django.db.models import Q, Case, When, Value, IntegerField, Count, OuterRef, Subquery +from django.db.models.functions import Coalesce from django.utils import timezone from website.models import Person, Position from website.models.position import Title +def related_count_subquery(model, fk_lookup, *, count_field='pk', distinct=False, + extra_filter=None): + """Build a correlated-subquery annotation that counts related ``model`` rows + for each outer row, for use in a ``ModelAdmin.get_queryset()`` override. + + Why a subquery instead of ``annotate(Count(...))``: stacking several + ``Count()`` annotations over different multi-valued relations in one query + makes the joins multiply rows (a cartesian blow-up) and, without care, wrong + counts. Each count here is an independent scalar subquery, so N count columns + cost N cheap correlated subqueries in a single changelist query — not a + growing pile of joins, and not one query per row. + + Args: + model: the related model to count (e.g. ``Publication``). + fk_lookup: the field on ``model`` pointing back to the outer row + (e.g. ``'projects'`` for ``Publication.projects``, ``'person'`` for + ``ProjectRole.person``). ``OuterRef('pk')`` is matched against it. + count_field: field to count; defaults to ``'pk'`` (i.e. count rows). + distinct: count distinct values of ``count_field`` (e.g. distinct people). + extra_filter: an optional ``Q`` further constraining the related rows. + + Returns: + An expression resolving to an int (0 when there are no related rows). + """ + qs = model.objects.filter(**{fk_lookup: OuterRef('pk')}) + if extra_filter is not None: + qs = qs.filter(extra_filter) + # Group by the outer key so the subquery returns exactly one row (the count). + qs = (qs.order_by() + .values(fk_lookup) + .annotate(_c=Count(count_field, distinct=distinct)) + .values('_c')[:1]) + return Coalesce(Subquery(qs, output_field=IntegerField()), 0) + + def get_active_professors_queryset(prioritized_name=Person.DIRECTOR_NAME): """ Build a queryset of currently active professors, optionally prioritizing a specific person. diff --git a/website/admin_list_filters/position_role_list_filter.py b/website/admin_list_filters/position_role_list_filter.py index 3c7bae73..b1b166cd 100644 --- a/website/admin_list_filters/position_role_list_filter.py +++ b/website/admin_list_filters/position_role_list_filter.py @@ -59,8 +59,12 @@ def queryset(self, request, queryset): # Compare the requested value (either True or False) # to decide how to filter the queryset. + # prefetch_related('position_set') so the membership checks below + # (is_current_member, is_alumni_member, ...) — which all read the + # person's positions — resolve from one prefetched query instead of + # ~2 queries per person on every People changelist load. filtered_person_ids = [] - for person in Person.objects.all(): + for person in Person.objects.all().prefetch_related('position_set'): if person.is_current_member is True and self.value() is None: filtered_person_ids.append(person.id) elif person.is_alumni_member is True and person.is_current_member is False and self.value() == "past_member": diff --git a/website/models/person.py b/website/models/person.py index f671117c..43ab3b6b 100644 --- a/website/models/person.py +++ b/website/models/person.py @@ -361,10 +361,24 @@ def has_started(self): return latest_position.has_started() def get_total_time_in_role(self, role): - """Returns the total time as in the specified role across all positions as a DurationField""" - duration = ExpressionWrapper(Coalesce(F('end_date'), date.today()) - F('start_date'), output_field=fields.DurationField()) - total_time_in_role = self.position_set.filter(role=role).aggregate(total=Sum(duration))['total'] - return total_time_in_role + """Returns the total time in the specified role across all positions as a + timedelta, or None if the person never held that role. + + Sums ``(end_date or today) - start_date`` over the person's positions in + this role, iterating ``self.position_set.all()`` in Python so a + prefetched ``position_set`` (admin changelist) needs no extra query. This + matches the prior DB aggregate, which also returned None when no + positions matched the role. + """ + today = date.today() + total = None + for position in self.position_set.all(): + if position.role != role: + continue + end_date = position.end_date if position.end_date is not None else today + duration = end_date - position.start_date + total = duration if total is None else total + duration + return total get_total_time_in_role.short_description = "Total Time In Role" @@ -400,10 +414,16 @@ def is_alumni_member(self): That is, they may have started as high school students then left the lab then returned as a grad student. A cached property.""" - # we use Django’s Q objects to construct a complex query. We check if there exists any position - # where the role was Position.MEMBER and the end date is less than the current time (i.e., in the past). - # The exists() method returns True if such a position exists, and False otherwise. - return self.position_set.filter(Q(role=Role.MEMBER) & Q(end_date__lt=timezone.now())).exists() + # True if any MEMBER position has an end_date on or before today (i.e. it + # has ended). Evaluated in Python over self.position_set.all() so a + # prefetched position_set (admin changelist / Role filter) avoids a + # per-row query; matches the prior DB filter (end_date < now()), where a + # same-day end_date also counted as ended. + today = timezone.now().date() + return any(position.role == Role.MEMBER + and position.end_date is not None + and position.end_date <= today + for position in self.position_set.all()) @cached_property def is_current_collaborator(self): @@ -466,19 +486,33 @@ def get_earliest_position_in_role(self, role, contiguous_constraint=True): @cached_property def get_earliest_position(self): - """Gets the earliest Position for the person or None if none exists. A cached property.""" - if self.position_set.exists() is False: + """Gets the earliest Position for the person or None if none exists. A cached property. + + Picks the min-start_date position from ``self.position_set.all()`` in + Python rather than issuing an ``.earliest()`` query. This lets a caller + that has ``prefetch_related('position_set')`` (e.g. the admin People + changelist) resolve this with zero extra queries. Positions-per-person is + tiny, so the non-prefetched path is unaffected (one query loads them). + """ + positions = self.position_set.all() + if not positions: return None - else: - return self.position_set.earliest('start_date') - + return min(positions, key=lambda position: position.start_date) + @cached_property def get_latest_position(self): - """Gets the latest Position for the person or None if none exists. A cached property.""" - if self.position_set.exists() is False: + """Gets the latest Position for the person or None if none exists. A cached property. + + See :meth:`get_earliest_position`: picks the max-start_date position in + Python so a prefetched ``position_set`` avoids a per-row ``.latest()`` + query. Nearly every other "current ..." property funnels through here, so + this is the single biggest lever for the admin People changelist's query + count. + """ + positions = self.position_set.all() + if not positions: return None - else: - return self.position_set.latest('start_date') + return max(positions, key=lambda position: position.start_date) @cached_property def get_start_date(self): diff --git a/website/tests/test_admin_perf.py b/website/tests/test_admin_perf.py new file mode 100644 index 00000000..b0ab4df5 --- /dev/null +++ b/website/tests/test_admin_perf.py @@ -0,0 +1,249 @@ +""" +Phase 2 of the admin changelist audit (#1346): N+1 query fixes on the Project +and Person changelists. + +Two kinds of coverage: + 1. Correctness — the new get_queryset() annotations must return exactly what + the original per-row model methods returned (get_publication_count, + get_most_recent_artifact, get_contributor_count, the Person counts, etc.), + and the rewritten prefetch-friendly Person model methods + (get_latest_position / get_earliest_position / get_total_time_in_role / + is_alumni_member) must be behavior-preserving. + 2. The actual regression — the changelist must issue a roughly constant number + of queries regardless of how many rows are listed. We render the real admin + ChangeList (filters + every column cell) for N rows, then for 2N rows, and + assert the query count doesn't grow. Before the fix these pages were + ~O(rows); a regression would make 2N cost more than N. +""" + +from datetime import date, timedelta + +from django.contrib.auth import get_user_model +from django.contrib.admin.templatetags.admin_list import items_for_result +from django.db import connection +from django.test import RequestFactory +from django.test.utils import CaptureQueriesContext + +from website.models import (Person, Position, ProjectRole, Project, Banner) +from website.models.position import Role, Title +from website.admin.admin_site import ml_admin_site +from website.admin.person_admin import PersonAdmin +from website.admin.project_admin import ProjectAdmin +from website.tests.base import DatabaseTestCase + + +class _AdminPerfBase(DatabaseTestCase): + @classmethod + def setUpTestData(cls): + cls.superuser = get_user_model().objects.create_superuser( + username="perfadmin", email="perf@example.com", password="x") + + def setUp(self): + self.rf = RequestFactory() + + def _request(self, params=None): + request = self.rf.get('/', params or {}) + request.user = self.superuser + return request + + def _render_changelist(self, model_admin, params): + """Render the real admin ChangeList (filters + every column cell) once.""" + request = self._request(params) + cl = model_admin.get_changelist_instance(request) + cl.get_results(request) + for obj in cl.result_list: + list(items_for_result(cl, obj, None)) + + def _steady_state_query_count(self, model_admin, params): + """Number of DB queries to render the changelist *in steady state*. + + We render once first to warm easy_thumbnails' per-image first-render + cache (an image's thumbnail metadata is written on first generation, a + pre-existing per-row cost that is out of scope for this N+1 fix and does + zero DB work once cached), then measure the second render. What remains + is the position/count machinery this phase optimized — which must be flat + as rows grow. + """ + self._render_changelist(model_admin, params) # warm thumbnail cache + with CaptureQueriesContext(connection) as ctx: + self._render_changelist(model_admin, params) + return len(ctx.captured_queries) + + +class PersonModelMethodTests(_AdminPerfBase): + """The prefetch-friendly rewrites are behavior-preserving.""" + + def _reload(self, person): + # Re-fetch to clear cached_property values computed during creation. + return Person.objects.get(pk=person.pk) + + def test_latest_and_earliest_position(self): + person = self.make_person() + old = Position.objects.create(person=person, start_date=date(2018, 1, 1), + end_date=date(2019, 1, 1), role=Role.MEMBER, + title=Title.MS_STUDENT) + new = Position.objects.create(person=person, start_date=date(2021, 1, 1), + end_date=None, role=Role.MEMBER, + title=Title.PHD_STUDENT) + person = self._reload(person) + self.assertEqual(person.get_latest_position, new) + self.assertEqual(person.get_earliest_position, old) + + def test_no_positions_returns_none(self): + person = self._reload(self.make_person()) + self.assertIsNone(person.get_latest_position) + self.assertIsNone(person.get_earliest_position) + + def test_total_time_in_role_and_alumni(self): + person = self.make_person() + Position.objects.create(person=person, start_date=date(2015, 1, 1), + end_date=date(2017, 1, 1), role=Role.MEMBER, + title=Title.MS_STUDENT) + person = self._reload(person) + self.assertTrue(person.is_alumni_member) + self.assertEqual(person.get_total_time_as_member, + date(2017, 1, 1) - date(2015, 1, 1)) + + def test_current_member_is_not_alumni(self): + person = self.make_person() + Position.objects.create(person=person, start_date=date(2022, 1, 1), + end_date=None, role=Role.MEMBER, + title=Title.PHD_STUDENT) + person = self._reload(person) + self.assertFalse(person.is_alumni_member) + + def test_total_time_in_role_none_when_role_never_held(self): + person = self.make_person() + Position.objects.create(person=person, start_date=date(2022, 1, 1), + end_date=None, role=Role.COLLABORATOR, + title=Title.UNKNOWN) + person = self._reload(person) + self.assertIsNone(person.get_total_time_as_member) + + +class PersonAdminAnnotationTests(_AdminPerfBase): + """PersonAdmin count annotations equal the original model count methods.""" + + def test_counts_match_model(self): + person = self.make_person(first_name="Auth", last_name="Or") + project = self.make_project(name="PA Proj", is_visible=True) + ProjectRole.objects.create(person=person, project=project, + start_date=date(2020, 1, 1)) + pub = self.make_publication(title="PA Pub", year=2021) + pub.authors.add(person) + talk = self.make_talk(title="PA Talk", year=2022) + talk.authors.add(person) + + admin = PersonAdmin(Person, ml_admin_site) + annotated = admin.get_queryset(self._request()).get(pk=person.pk) + model_person = Person.objects.get(pk=person.pk) + self.assertEqual(admin.project_count(annotated), model_person.get_project_count) + self.assertEqual(admin.pub_count(annotated), model_person.get_pub_count) + self.assertEqual(admin.talk_count(annotated), model_person.get_talk_count) + + def test_changelist_query_count_is_flat(self): + admin = PersonAdmin(Person, ml_admin_site) + + def make_people(start, end): + for i in range(start, end): + p = self.make_person(first_name=f"P{i}", last_name=f"L{i}") + Position.objects.create(person=p, start_date=date(2021, 1, 1), + end_date=None, role=Role.MEMBER, + title=Title.PHD_STUDENT) + Position.objects.create(person=p, start_date=date(2018, 1, 1), + end_date=date(2019, 1, 1), role=Role.MEMBER, + title=Title.MS_STUDENT) + proj = self.make_project(name=f"P{i} Proj", short_name=f"p{i}proj") + ProjectRole.objects.create(person=p, project=proj, + start_date=date(2021, 2, 1)) + + params = {'position_role': 'all'} + make_people(0, 4) + n1 = self._steady_state_query_count(admin, params) + make_people(4, 12) + n2 = self._steady_state_query_count(admin, params) + self.assertEqual(n1, n2, + f"People changelist N+1: {n1} -> {n2} queries as rows grew 4 -> 12") + + +class ProjectAdminAnnotationTests(_AdminPerfBase): + """ProjectAdmin annotations equal the original per-row model methods.""" + + def test_counts_and_most_recent_match_model(self): + project = self.make_project(name="PX", is_visible=True) + a = self.make_person(first_name="A", last_name="A") + b = self.make_person(first_name="B", last_name="B") + # a has a current and a past role (distinct-person counts must dedupe). + ProjectRole.objects.create(person=a, project=project, + start_date=date(2020, 1, 1), end_date=None) + ProjectRole.objects.create(person=a, project=project, + start_date=date(2019, 1, 1), end_date=date(2019, 6, 1)) + # b is a publication author but holds no role -> contributors = {a, b}. + pub = self.make_publication(title="PX Pub", year=2021) + pub.projects.add(project) + pub.authors.add(b) + talk = self.make_talk(title="PX Talk", year=2023) + talk.projects.add(project) + video = self.make_video(title="PX Video", year=2022) + video.projects.add(project) + Banner.objects.create(project=project, title="PX Banner") + + admin = ProjectAdmin(Project, ml_admin_site) + ann = admin.get_queryset(self._request()).get(pk=project.pk) + model_project = Project.objects.get(pk=project.pk) + + self.assertEqual(admin.pub_count(ann), model_project.get_publication_count()) + self.assertEqual(admin.talk_count(ann), model_project.get_talk_count()) + self.assertEqual(admin.video_count(ann), model_project.get_video_count()) + self.assertEqual(admin.banner_count(ann), model_project.get_banner_count()) + self.assertEqual(admin.people_count(ann), model_project.get_people_count()) + self.assertEqual(admin.current_member_count(ann), + model_project.get_current_member_count()) + self.assertEqual(admin.past_member_count(ann), + model_project.get_past_member_count()) + self.assertEqual(admin.contributor_count(ann), + model_project.get_contributor_count()) + self.assertEqual(admin.most_recent_artifact_date(ann), + model_project.get_most_recent_artifact_date()) + self.assertEqual(admin.most_recent_artifact_type(ann), + model_project.get_most_recent_artifact_type()) + # Sanity-check the actual values, not just agreement with the model. + self.assertEqual(admin.contributor_count(ann), 2) + self.assertEqual(admin.people_count(ann), 1) + self.assertEqual(admin.most_recent_artifact_type(ann), 'Talk') + + def test_most_recent_artifact_single_type_and_empty(self): + admin = ProjectAdmin(Project, ml_admin_site) + + solo = self.make_project(name="Solo", is_visible=True) + pub = self.make_publication(title="Solo Pub", year=2020) + pub.projects.add(solo) + ann = admin.get_queryset(self._request()).get(pk=solo.pk) + # Exercises Greatest() ignoring the NULL talk/video dates (Postgres). + self.assertEqual(admin.most_recent_artifact_type(ann), 'Publication') + self.assertEqual(admin.most_recent_artifact_date(ann), date(2020, 1, 1)) + + empty = self.make_project(name="Empty", is_visible=True) + ann_empty = admin.get_queryset(self._request()).get(pk=empty.pk) + self.assertIsNone(admin.most_recent_artifact_type(ann_empty)) + self.assertIsNone(admin.most_recent_artifact_date(ann_empty)) + + def test_changelist_query_count_is_flat(self): + admin = ProjectAdmin(Project, ml_admin_site) + + def make_projects(start, end): + for i in range(start, end): + proj = self.make_project(name=f"Proj {i}", short_name=f"proj{i}", + is_visible=True) + person = self.make_person(first_name=f"PP{i}", last_name=f"LL{i}") + ProjectRole.objects.create(person=person, project=proj, + start_date=date(2021, 1, 1)) + Banner.objects.create(project=proj, title=f"B{i}") + + params = {'active_project_status': 'All'} + make_projects(0, 4) + n1 = self._steady_state_query_count(admin, params) + make_projects(4, 12) + n2 = self._steady_state_query_count(admin, params) + self.assertEqual(n1, n2, + f"Project changelist N+1: {n1} -> {n2} queries as rows grew 4 -> 12")