perf: request cache get_schedule_for_user

CHANGELOG.rst

@@ -14,6 +14,10 @@ Change Log
 Unreleased
 ~~~~~~~~~~
 
+[2.5.1] - 2024-08-06
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+* Reduce schedule queries by using a request cache for get_schedule_for_user.
+
 [2.5.0] - 2024-04-02
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 * Add support for Python 3.11. Dropped django32 support.

edx_when/__init__.py

@@ -2,4 +2,4 @@
 Central source of course block dates for the LMS.
 """
 
-__version__ = '2.5.0'
+__version__ = '2.5.1'

edx_when/api.py

@@ -193,7 +193,7 @@ def get_dates_for_course(
             user_id = user.id if not user.is_anonymous else ''
 
     if schedule is None and user is not None and user_id != '':
-        schedule = get_schedule_for_user(user_id, course_id)
+        schedule = get_schedule_for_user(user_id, course_id, use_cached=use_cached)
 
     # Construct the cache key, incorporating all parameters which would cause a different
     # query set to be returned.

edx_when/utils.py

@@ -2,6 +2,7 @@
 Utility functions to use across edx-when.
 """
 from django.core.exceptions import ObjectDoesNotExist
+from edx_django_utils.cache.utils import RequestCache
 
 try:
     from openedx.core.djangoapps.schedules.models import Schedule
@@ -10,13 +11,34 @@
     Schedule = None
 
 
-def get_schedule_for_user(user_id, course_key):
+def get_schedule_for_user(user_id, course_key, use_cached=True):
     """
     Return the schedule for the user in the course or None if it does not exist or the Schedule model is undefined.
     """
-    if Schedule:
-        try:
-            return Schedule.objects.get(enrollment__user__id=user_id, enrollment__course__id=course_key)
-        except ObjectDoesNotExist:
-            pass
-    return None
+    # If Schedule is not defined, there's nothing to query, so return None. This
+    # hackiness is happening because the Schedule model is in edx-platform at
+    # the moment.
+    if not Schedule:
+        return None
+
+    # This is intentionally a RequestCache and not a TieredCache–that way it's
+    # just a local memory reference, and we don't have to worry about the
+    # complications that can come with pickling model objects.
+    cache = RequestCache('edx-when')
+    cache_key = f"get_schedule_for_user::{user_id}::{course_key}"
+    if use_cached:
+        cache_response = cache.get_cached_response(cache_key)
+        if cache_response.is_found:
+            return cache_response.value
+
+    try:
+        schedule = Schedule.objects.get(
+            enrollment__user__id=user_id,
+            enrollment__course__id=course_key,
+        )
+    except ObjectDoesNotExist:
+        schedule = None
+
+    cache.set(cache_key, schedule)
+
+    return schedule

tests/test_api.py

@@ -10,7 +10,7 @@
 from django.contrib import auth
 from django.test import TestCase
 from django.urls import reverse
-from edx_django_utils.cache.utils import TieredCache
+from edx_django_utils.cache.utils import RequestCache, TieredCache
 from opaque_keys.edx.locator import CourseLocator
 
 from edx_when import api, models
@@ -54,8 +54,11 @@ def setUp(self):
         relative_dates_patcher = patch('edx_when.api._are_relative_dates_enabled', return_value=True)
         relative_dates_patcher.start()
         self.addCleanup(relative_dates_patcher.stop)
-        self.addCleanup(TieredCache.dangerous_clear_all_tiers)
+        self.addCleanup(self._clear_caches)
 
+    @staticmethod
+    def _clear_caches():
+        RequestCache.clear_all_namespaces()
         TieredCache.dangerous_clear_all_tiers()
 
     @patch('edx_when.api.Schedule', DummySchedule)
@@ -274,7 +277,7 @@ def test_set_user_override(self, initial_date, override_date, expected_date):
         api.set_dates_for_course(str(block_id.course_key), items)
 
         api.set_date_for_block(block_id.course_key, block_id, 'due', override_date, user=self.user)
-        TieredCache.dangerous_clear_all_tiers()
+        self._clear_caches()
         retrieved = api.get_dates_for_course(block_id.course_key, user=self.user.id)
         assert len(retrieved) == NUM_OVERRIDES
         assert retrieved[block_id, 'due'] == expected_date
@@ -308,7 +311,7 @@ def test_set_date_for_block(self, initial_date, override_date, expected_date):
 
         api.set_dates_for_course(str(block_id.course_key), items)
         api.set_date_for_block(block_id.course_key, block_id, 'due', override_date)
-        TieredCache.dangerous_clear_all_tiers()
+        self._clear_caches()
         retrieved = api.get_dates_for_course(block_id.course_key, user=self.user.id)
         assert len(retrieved) == NUM_OVERRIDES
         assert retrieved[block_id, 'due'] == expected_date
@@ -329,13 +332,13 @@ def test_remove_user_override(self, initial_date, override_date, expected_date):
         api.set_dates_for_course(str(block_id.course_key), items)
 
         api.set_date_for_block(block_id.course_key, block_id, 'due', override_date, user=self.user)
-        TieredCache.dangerous_clear_all_tiers()
+        self._clear_caches()
         retrieved = api.get_dates_for_course(block_id.course_key, user=self.user.id)
         assert len(retrieved) == NUM_OVERRIDES
         assert retrieved[block_id, 'due'] == expected_date
 
         api.set_date_for_block(block_id.course_key, block_id, 'due', None, user=self.user)
-        TieredCache.dangerous_clear_all_tiers()
+        self._clear_caches()
         retrieved = api.get_dates_for_course(block_id.course_key, user=self.user.id)
         assert len(retrieved) == NUM_OVERRIDES
         if isinstance(initial_date, timedelta):
@@ -484,7 +487,7 @@ def test_relative_date_past_cutoff_date(self):
         ]
         assert api.get_dates_for_course(course_key, schedule=self.schedule) == dict(dates)
 
-        TieredCache.dangerous_clear_all_tiers()
+        self._clear_caches()
 
         # Now set schedule start date too close to the end date and verify that we no longer get due dates
         self.schedule.created = datetime(2019, 4, 15)
@@ -526,16 +529,15 @@ def test_get_dates_for_course_query_counts(self, has_schedule, pass_user_object,
                 course_id=self.course.id, user=user, schedule=schedule
             )
 
-        # Second time, the request cache eliminates all querying (sometimes)...
-        # If a schedule is not provided, we will get the schedule to avoid caching outdated dates
-        with self.assertNumQueries(0 if schedule else 1):
+        # Second time, the request cache eliminates all querying...
+        with self.assertNumQueries(0):
             cached_dates = api.get_dates_for_course(
                 course_id=self.course.id, user=user, schedule=schedule
             )
             assert dates == cached_dates
 
         # Now wipe all cache tiers...
-        TieredCache.dangerous_clear_all_tiers()
+        self._clear_caches()
 
         # No cached values - so will do *all* queries again.
         with self.assertNumQueries(query_count):