Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions apps/events/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ def get_absolute_url(self):
return reverse("events:eventlist_location", kwargs={"calendar_slug": self.calendar.slug, "pk": self.pk})


class EventManager(models.Manager):
"""Custom manager for querying events by time boundaries."""
class EventQuerySet(models.QuerySet):
"""Queryset for events, with the time boundaries and eager loading the views need."""

def for_datetime(self, dt=None):
"""Return events occurring after the given datetime."""
Expand All @@ -115,6 +115,13 @@ def until_datetime(self, dt=None):
dt = timezone.now() if dt is None else convert_dt_to_aware(dt)
return self.filter(Q(occurring_rule__dt_end__lt=dt) | Q(recurring_rules__begin__lt=dt))

def with_related(self):
"""Fetch what the event templates read for each row, so rendering does not query per event."""
return self.select_related("occurring_rule", "venue", "calendar").prefetch_related("recurring_rules")


EventManager = models.Manager.from_queryset(EventQuerySet)


class Event(ContentManageable):
"""A Python community event such as a conference, sprint, or meetup."""
Expand Down Expand Up @@ -186,7 +193,9 @@ def next_time(self):
if occurring_rule and occurring_rule.dt_start > now:
occurring_start = (occurring_rule.dt_start, occurring_rule)

rrules = self.recurring_rules.filter(finish__gt=now)
# Filter in Python so a prefetched `recurring_rules` cache is reused; calling
# .filter() on the related manager would issue a query for every event.
rrules = [rule for rule in self.recurring_rules.all() if rule.finish > now]
Comment thread
jefftriplett marked this conversation as resolved.
recurring_starts = [(rule.dt_start, rule) for rule in rrules if rule.dt_start is not None]
recurring_starts.sort(key=itemgetter(0))

Expand Down Expand Up @@ -230,7 +239,8 @@ def previous_time(self):
if occurring_rule and occurring_rule.dt_end < now:
occurring_end = (occurring_rule.dt_end, occurring_rule)

rrules = self.recurring_rules.filter(begin__lt=now)
# Filter in Python so a prefetched `recurring_rules` cache is reused.
rrules = [rule for rule in self.recurring_rules.all() if rule.begin < now]
recurring_ends = [(rule.dt_end, rule) for rule in rrules if rule.dt_end is not None]
recurring_ends.sort(key=itemgetter(0), reverse=True)

Expand Down
2 changes: 1 addition & 1 deletion apps/events/templatetags/events.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
@register.simple_tag
def get_events_upcoming(limit=5, only_featured=False):
"""Return upcoming events, optionally filtered to featured only."""
qs = Event.objects.for_datetime(timezone.now()).order_by("occurring_rule__dt_start")
qs = Event.objects.for_datetime(timezone.now()).with_related().order_by("occurring_rule__dt_start")
if only_featured:
qs = qs.filter(featured=True)
return qs[:limit]
58 changes: 58 additions & 0 deletions apps/events/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

from django.contrib.auth import get_user_model
from django.core import mail
from django.db import connection
from django.test import TestCase
from django.test.utils import CaptureQueriesContext
from django.urls import reverse, reverse_lazy
from django.utils import timezone

Expand Down Expand Up @@ -292,3 +294,59 @@ def test_badheadererror(self):
messages = list(response.context["messages"])
self.assertEqual(len(messages), 1)
self.assertEqual(messages[0].message, "Invalid header found.")


class EventHomepageQueryCountTests(TestCase):
"""The events homepage must not run queries per event. See #3125."""

@classmethod
def setUpTestData(cls):
cls.user = get_user_model().objects.create_user(username="query-count", password="password")
cls.calendar = Calendar.objects.create(creator=cls.user, slug="query-count-calendar")
now = timezone.now()
for index in range(10):
past = Event.objects.create(title=f"Past {index}", creator=cls.user, calendar=cls.calendar)
OccurringRule.objects.create(
event=past,
dt_start=now - datetime.timedelta(days=index + 10),
dt_end=now - datetime.timedelta(days=index + 9),
)
upcoming = Event.objects.create(title=f"Upcoming {index}", creator=cls.user, calendar=cls.calendar)
OccurringRule.objects.create(
event=upcoming,
dt_start=now + datetime.timedelta(days=index + 1),
dt_end=now + datetime.timedelta(days=index + 2),
)
recurring = Event.objects.create(title=f"Recurring {index}", creator=cls.user, calendar=cls.calendar)
RecurringRule.objects.create(
event=recurring,
begin=now - datetime.timedelta(days=1),
finish=now + datetime.timedelta(days=30),
)

def _homepage_query_count(self):
with CaptureQueriesContext(connection) as queries:
response = self.client.get(reverse("events:events"))
self.assertEqual(response.status_code, 200)
return len(queries)

def test_query_count_does_not_grow_with_the_number_of_events(self):
"""Adding events must not add queries, otherwise the page is querying per row again."""
before = self._homepage_query_count()

now = timezone.now()
extra = Event.objects.bulk_create(
[Event(title=f"Extra {index}", creator=self.user, calendar=self.calendar) for index in range(50)]
)
OccurringRule.objects.bulk_create(
[
OccurringRule(
event=event,
dt_start=now + datetime.timedelta(days=index + 100),
dt_end=now + datetime.timedelta(days=index + 101),
)
for index, event in enumerate(extra)
]
)

self.assertLessEqual(self._homepage_query_count(), before)
54 changes: 40 additions & 14 deletions apps/events/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from django.contrib import messages
from django.core.mail import BadHeaderError
from django.db.models import QuerySet
from django.shortcuts import get_object_or_404, redirect
from django.urls import reverse_lazy
from django.utils import timezone
Expand All @@ -15,6 +16,18 @@
from pydotorg.mixins import LoginRequiredMixin


def next_start(event, default):
"""Return when an event next starts, computing the `next_time` property only once."""
next_time = event.next_time
return next_time.dt_start if next_time else default


def previous_start(event, default):
"""Return when an event last started, computing the `previous_time` property only once."""
previous_time = event.previous_time
return previous_time.dt_start if previous_time else default


class CalendarList(ListView):
"""List all available event calendars."""

Expand Down Expand Up @@ -49,28 +62,31 @@ class EventHomepage(ListView):

template_name = "events/event_list.html"

def get_queryset(self) -> Event:
def get_queryset(self) -> QuerySet[Event]:
"""Queryset to return all events, ordered by START date."""
return Event.objects.all().order_by("occurring_rule__dt_start")
return Event.objects.with_related().order_by("occurring_rule__dt_start")

def get_context_data(self, **kwargs: dict) -> dict:
"""Add more ctx, specifically events that are happening now, just missed, and upcoming."""
context = super().get_context_data(**kwargs)
now = timezone.now()

# past events, most recent first
past_events = list(Event.objects.until_datetime(timezone.now()))
past_events.sort(key=lambda e: e.previous_time.dt_start if e.previous_time else timezone.now(), reverse=True)
past_events = list(Event.objects.until_datetime(now).with_related())
past_events.sort(key=lambda event: previous_start(event, now), reverse=True)
context["events_just_missed"] = past_events[:2]

# upcoming events, soonest first
upcoming = list(Event.objects.for_datetime(timezone.now()))
upcoming.sort(key=lambda e: e.next_time.dt_start if e.next_time else timezone.now())
upcoming = list(Event.objects.for_datetime(now).with_related())
upcoming.sort(key=lambda event: next_start(event, now))
context["upcoming_events"] = upcoming

# right now, soonest first
context["events_now"] = Event.objects.filter(
occurring_rule__dt_start__lte=timezone.now(), occurring_rule__dt_end__gte=timezone.now()
).order_by("occurring_rule__dt_start")[:2]
context["events_now"] = (
Event.objects.filter(occurring_rule__dt_start__lte=now, occurring_rule__dt_end__gte=now)
.with_related()
.order_by("occurring_rule__dt_start")[:2]
)
return context


Expand All @@ -81,7 +97,7 @@ class EventDetail(DetailView):

def get_queryset(self):
"""Return events with related data prefetched."""
return super().get_queryset().select_related()
return super().get_queryset().with_related()

def get_context_data(self, **kwargs):
"""Add 7/30/90/365-day date windows for the next occurrence."""
Expand All @@ -107,6 +123,7 @@ def get_queryset(self):
return (
Event.objects.for_datetime(timezone.now())
.filter(calendar__slug=self.kwargs["calendar_slug"])
.with_related()
.order_by("occurring_rule__dt_start")
)

Expand All @@ -115,10 +132,11 @@ def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)

# today's events, most recent first
now = timezone.now()
today_events = list(
Event.objects.until_datetime(timezone.now()).filter(calendar__slug=self.kwargs["calendar_slug"])
Event.objects.until_datetime(now).filter(calendar__slug=self.kwargs["calendar_slug"]).with_related()
)
today_events.sort(key=lambda e: e.previous_time.dt_start if e.previous_time else timezone.now(), reverse=True)
today_events.sort(key=lambda event: previous_start(event, now), reverse=True)
context["events_today"] = today_events[:2]
context["calendar"] = get_object_or_404(Calendar, slug=self.kwargs["calendar_slug"])
context["upcoming_events"] = context["object_list"]
Expand All @@ -133,7 +151,11 @@ class PastEventList(EventList):

def get_queryset(self):
"""Return past events for the calendar specified in the URL."""
return Event.objects.until_datetime(timezone.now()).filter(calendar__slug=self.kwargs["calendar_slug"])
return (
Event.objects.until_datetime(timezone.now())
.filter(calendar__slug=self.kwargs["calendar_slug"])
.with_related()
)


class EventListByDate(EventList):
Expand All @@ -148,7 +170,11 @@ def get_object(self):

def get_queryset(self):
"""Return events on or after the specified date."""
return Event.objects.for_datetime(self.get_object()).filter(calendar__slug=self.kwargs["calendar_slug"])
return (
Event.objects.for_datetime(self.get_object())
.filter(calendar__slug=self.kwargs["calendar_slug"])
.with_related()
)


class EventListByCategory(EventList):
Expand Down
Loading