Skip to content

Commit 83330d7

Browse files
Cache leaderboard data to improve page load performance (#148)
* Initial plan * Implement leaderboard caching with cache invalidation Co-authored-by: alexeygrigorev <875246+alexeygrigorev@users.noreply.github.com> * Add test for cache invalidation and fix test cache clearing Co-authored-by: alexeygrigorev <875246+alexeygrigorev@users.noreply.github.com> * Address code review feedback: cache dictionaries instead of model instances Co-authored-by: alexeygrigorev <875246+alexeygrigorev@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: alexeygrigorev <875246+alexeygrigorev@users.noreply.github.com>
1 parent e2c50ff commit 83330d7

5 files changed

Lines changed: 84 additions & 9 deletions

File tree

course_management/settings.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,14 @@
186186

187187
VERSION = os.getenv("VERSION", "N/A")
188188

189+
# Cache configuration
190+
CACHES = {
191+
"default": {
192+
"BACKEND": "django.core.cache.backends.locmem.LocMemCache",
193+
"LOCATION": "default-cache",
194+
}
195+
}
196+
189197
# authentication
190198

191199
AUTHENTICATION_BACKENDS = (

courses/scoring.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from django.db.models import Sum, Count
1010

1111
from django.db import transaction
12+
from django.core.cache import cache
1213

1314

1415
from .models import (
@@ -345,6 +346,11 @@ def update_leaderboard(course: Course):
345346
["total_score", "position_on_leaderboard"],
346347
)
347348

349+
# Invalidate the leaderboard cache
350+
cache_key = f"leaderboard:{course.id}"
351+
cache.delete(cache_key)
352+
logger.info(f"Invalidated cache for leaderboard of course {course.id}")
353+
348354
t1 = time()
349355
logger.info(f"Updated leaderboard in {(t1 - t0):.2f} seconds")
350356

courses/tests/test_course.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from django.test import TestCase, Client
22
from django.urls import reverse
33
from django.utils import timezone
4+
from django.core.cache import cache
45

56
from courses.models import (
67
User,
@@ -27,6 +28,9 @@
2728

2829
class CourseDetailViewTests(TestCase):
2930
def setUp(self):
31+
# Clear cache before each test to ensure fresh state
32+
cache.clear()
33+
3034
self.client = Client()
3135

3236
self.user = User.objects.create_user(**credentials)
@@ -313,7 +317,7 @@ def test_leaderboard_order(self):
313317
self.enrollment.display_name,
314318
]
315319

316-
actual_order = [e.display_name for e in enrollments]
320+
actual_order = [e['display_name'] for e in enrollments]
317321

318322
self.assertEqual(actual_order, expected_order)
319323

@@ -349,13 +353,13 @@ def test_new_enrollment_at_the_end_of_leaderboard(self):
349353
e5.display_name,
350354
]
351355

352-
actual_order = [e.display_name for e in enrollments]
356+
actual_order = [e['display_name'] for e in enrollments]
353357

354358
self.assertEqual(actual_order, expected_order)
355359

356360
expected_positions = [1, 2, 3, 4, None, None]
357361
actual_positions = [
358-
e.position_on_leaderboard for e in enrollments
362+
e['position_on_leaderboard'] for e in enrollments
359363
]
360364
self.assertEqual(actual_positions, expected_positions)
361365

@@ -390,7 +394,7 @@ def test_not_enrolled_yet_but_leaderboard_displays(self):
390394

391395
# Verify the order is correct
392396
expected_order = ["e1", "e2", "e3", "e4", "e5"]
393-
actual_order = [e.display_name for e in enrollments]
397+
actual_order = [e['display_name'] for e in enrollments]
394398
self.assertEqual(actual_order, expected_order)
395399

396400
def test_not_enrolled_but_can_edit_details(self):

courses/tests/test_leaderboard.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
from django.test import TestCase
44
from django.utils import timezone
55
from datetime import timedelta
6+
from django.core.cache import cache
67

78
from courses.models import (
89
Course,
@@ -58,6 +59,9 @@ def submit_homework(self, homework, enrollment, score):
5859
)
5960

6061
def setUp(self):
62+
# Clear cache before each test to ensure fresh state
63+
cache.clear()
64+
6165
self.course = Course.objects.create(
6266
slug="test-course",
6367
title="Test Course",
@@ -125,3 +129,29 @@ def test_leaderboard(self):
125129
self.assertEqual(enrollment.position_on_leaderboard, rank)
126130

127131
self.assertEqual(enrollment.total_score, score)
132+
133+
def test_leaderboard_cache_invalidation(self):
134+
"""Test that leaderboard cache is invalidated when update_leaderboard is called"""
135+
# Create some test data
136+
enrollment1 = self.create_student("student1")
137+
enrollment2 = self.create_student("student2")
138+
139+
homework = self.create_homework(1)
140+
self.submit_homework(homework, enrollment1, score=100)
141+
self.submit_homework(homework, enrollment2, score=50)
142+
143+
# Update leaderboard (this should populate the cache)
144+
update_leaderboard(self.course)
145+
146+
# Check the cache key
147+
cache_key = f"leaderboard:{self.course.id}"
148+
149+
# Manually set a value in cache to verify it gets invalidated
150+
cache.set(cache_key, "test_value", 3600)
151+
self.assertEqual(cache.get(cache_key), "test_value")
152+
153+
# Update leaderboard again (this should invalidate the cache)
154+
update_leaderboard(self.course)
155+
156+
# Cache should be invalidated (None)
157+
self.assertIsNone(cache.get(cache_key))

courses/views/course.py

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from django.db.models.functions import Coalesce
1515

1616
from django.contrib.auth.decorators import login_required
17+
from django.core.cache import cache
1718

1819
from courses.models import (
1920
Course,
@@ -258,13 +259,39 @@ def leaderboard_view(request, course_slug: str):
258259
except Enrollment.DoesNotExist:
259260
pass
260261

261-
enrollments = Enrollment.objects.filter(course=course).order_by(
262-
Coalesce("position_on_leaderboard", Value(999999)),
263-
"id",
264-
)
262+
# Try to get enrollments from cache
263+
cache_key = f"leaderboard:{course.id}"
264+
enrollments_data = cache.get(cache_key)
265+
266+
if enrollments_data is None:
267+
logger.info(f"Cache miss for leaderboard of course {course.slug}")
268+
# Cache miss, fetch from database
269+
enrollments = list(
270+
Enrollment.objects.filter(course=course)
271+
.select_related('student')
272+
.order_by(
273+
Coalesce("position_on_leaderboard", Value(999999)),
274+
"id",
275+
)
276+
)
277+
# Store as list of dictionaries to avoid stale model instances
278+
enrollments_data = [
279+
{
280+
'id': e.id,
281+
'display_name': e.display_name,
282+
'total_score': e.total_score,
283+
'position_on_leaderboard': e.position_on_leaderboard,
284+
}
285+
for e in enrollments
286+
]
287+
# Cache for 1 hour (3600 seconds)
288+
# The cache will be invalidated when leaderboard is recalculated
289+
cache.set(cache_key, enrollments_data, 3600)
290+
else:
291+
logger.info(f"Cache hit for leaderboard of course {course.slug}")
265292

266293
context = {
267-
"enrollments": enrollments,
294+
"enrollments": enrollments_data,
268295
"course": course,
269296
"current_student_enrollment": current_student_enrollment,
270297
"current_student_enrollment_id": current_student_enrollment_id,

0 commit comments

Comments
 (0)