Skip to content

Commit f7f9508

Browse files
fix: ensure navigation sidebar serves fresh data after course publish (#38880)
Replace course_version in the sidebar cache key with a block_structure_version derived from BlockStructureModel.data_version. course_version changes eagerly on publish, causing a cache miss before the block structure is rebuilt — poisoning the cache with stale data for 1 hour. block_structure_version only changes after the async rebuild completes, so cache misses only occur when fresh data is available. This is a backport of #38785. (cherry picked from commit 929815f) Co-authored-by: Taylor Payne <wgu.taylor.payne@gmail.com>
1 parent 4d2e220 commit f7f9508

3 files changed

Lines changed: 99 additions & 3 deletions

File tree

lms/djangoapps/course_home_api/outline/tests/test_view.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
from lms.djangoapps.course_home_api.toggles import COURSE_HOME_SEND_COURSE_PROGRESS_ANALYTICS_FOR_STUDENT
2424
from lms.djangoapps.course_home_api.tests.utils import BaseCourseHomeTests
2525
from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
26+
from openedx.core.djangoapps.content.block_structure.api import update_course_in_cache
2627
from openedx.core.djangoapps.content.course_overviews.models import CourseOverview
2728
from openedx.core.djangoapps.content.learning_sequences.api import replace_course_outline
2829
from openedx.core.djangoapps.content.learning_sequences.data import CourseOutlineData, CourseVisibility
@@ -900,3 +901,55 @@ def test_vertical_icon_determined_by_icon_class(self):
900901
response = self.client.get(reverse('course-home:course-navigation', args=[self.course.id]))
901902
vertical_data = response.data['blocks'][str(self.vertical.location)]
902903
assert vertical_data['icon'] == 'video'
904+
905+
def test_navigation_does_not_cache_stale_data_after_publish(self):
906+
"""
907+
Regression test: after the block structure rebuild task completes,
908+
the navigation sidebar should serve fresh data.
909+
910+
This simulates a production scenario where:
911+
1. A unit is deleted and the course is auto-published
912+
2. The block structure rebuild Celery task is queued with a delay (30s by default)
913+
3. A learner hits the navigation endpoint during that 30s window
914+
4. The rebuild task completes (bumping block_structure_version)
915+
5. Another request arrives
916+
917+
Without the fix, step 3 caches stale data under a key that step 5
918+
also hits (because course_version changed eagerly). With the fix,
919+
the cache key uses block_structure_version which only changes when
920+
the rebuild completes, so step 5 gets a cache miss and fresh data.
921+
"""
922+
self.add_blocks_to_course()
923+
CourseEnrollment.enroll(self.user, self.course.id, CourseMode.VERIFIED)
924+
925+
# First request — populates both block structure and navigation cache
926+
response = self.client.get(self.url)
927+
assert response.status_code == 200
928+
sequential_data = response.data['blocks'][str(self.sequential.location)]
929+
assert str(self.vertical.location) in sequential_data['children']
930+
931+
# Delete the vertical directly in the modulestore. Signals are disabled
932+
# in ModuleStoreTestCase, so the block structure cache is now stale —
933+
# mirroring the 30s window in production before the rebuild task runs.
934+
self.store.delete_item(self.vertical.location, self.user.id)
935+
update_outline_from_modulestore(self.course.id)
936+
937+
# Request during the stale window — served from the pre-delete cache
938+
# (block_structure_version hasn't changed yet, so same cache key).
939+
response = self.client.get(self.url)
940+
assert response.status_code == 200
941+
942+
# The vertical is still in the cache, even though it has been deleted
943+
sequential_data = response.data['blocks'][str(self.sequential.location)]
944+
assert str(self.vertical.location) in sequential_data['children']
945+
946+
# Now simulate the block structure rebuild task completing.
947+
# This bumps block_structure_version → new cache key on next request.
948+
update_course_in_cache(self.course.id)
949+
950+
# Next request has a new cache key (version bumped) → cache miss →
951+
# fresh data built from updated block structure.
952+
response = self.client.get(self.url)
953+
assert response.status_code == 200
954+
sequential_data = response.data['blocks'][str(self.sequential.location)]
955+
assert str(self.vertical.location) not in sequential_data['children']

lms/djangoapps/course_home_api/outline/views.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
from lms.djangoapps.courseware.views.views import get_cert_data
4747
from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory
4848
from lms.djangoapps.utils import OptimizelyClient
49+
from openedx.core.djangoapps.content.block_structure.api import get_block_structure_version
4950
from openedx.core.djangoapps.content.learning_sequences.api import get_user_course_outline
5051
from openedx.core.djangoapps.content.course_overviews.api import get_course_overview_or_404
5152
from openedx.core.djangoapps.course_groups.cohorts import get_cohort
@@ -423,7 +424,7 @@ class CourseNavigationBlocksView(RetrieveAPIView):
423424

424425
serializer_class = CourseBlockSerializer
425426
COURSE_BLOCKS_CACHE_KEY_TEMPLATE = (
426-
'course_sidebar_blocks_{course_key_string}_{course_version}_{user_id}_{user_cohort_id}'
427+
'course_sidebar_blocks_{course_key_string}_{block_structure_version}_{user_id}_{user_cohort_id}'
427428
'_{enrollment_mode}_{allow_public}_{allow_public_outline}_{is_masquerading}'
428429
)
429430
COURSE_BLOCKS_CACHE_TIMEOUT = 60 * 60 # 1 hour
@@ -458,7 +459,7 @@ def get(self, request, *args, **kwargs):
458459

459460
cache_key = self.COURSE_BLOCKS_CACHE_KEY_TEMPLATE.format(
460461
course_key_string=course_key_string,
461-
course_version=str(course.course_version),
462+
block_structure_version=get_block_structure_version(course_key),
462463
user_id=request.user.id,
463464
enrollment_mode=getattr(enrollment, 'mode', ''),
464465
user_cohort_id=getattr(user_cohort, 'id', ''),

openedx/core/djangoapps/content/block_structure/api.py

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,47 @@
88
from xmodule.modulestore.django import modulestore
99

1010
from .manager import BlockStructureManager
11+
from .models import BlockStructureModel
12+
13+
BLOCK_STRUCTURE_VERSION_KEY = 'block_structure_version:{}'
14+
15+
16+
def get_block_structure_version(course_key):
17+
"""
18+
Returns the current block structure version for the given course.
19+
This version corresponds to the data_version stored in BlockStructureModel
20+
and changes each time the block structure cache is rebuilt.
21+
22+
Reads from cache first; on a miss, falls back to the database
23+
without populating the cache. The cache is populated exclusively by
24+
_update_block_structure_version after a successful rebuild, which
25+
prevents readers from accidentally caching a stale version during
26+
a concurrent rebuild.
27+
"""
28+
cache_key = BLOCK_STRUCTURE_VERSION_KEY.format(course_key)
29+
version = cache.get(cache_key)
30+
if version is None:
31+
try:
32+
course_usage_key = modulestore().make_course_usage_key(course_key)
33+
block_structure_model = BlockStructureModel.objects.get(data_usage_key=course_usage_key)
34+
version = str(block_structure_model.data_version or '')
35+
except BlockStructureModel.DoesNotExist:
36+
version = ''
37+
return version
38+
39+
40+
def _update_block_structure_version(course_key):
41+
"""
42+
Reads the current data_version from BlockStructureModel and updates
43+
the cached block structure version key.
44+
"""
45+
try:
46+
course_usage_key = modulestore().make_course_usage_key(course_key)
47+
block_structure_model = BlockStructureModel.objects.get(data_usage_key=course_usage_key)
48+
version = str(block_structure_model.data_version or '')
49+
except BlockStructureModel.DoesNotExist:
50+
version = ''
51+
cache.set(BLOCK_STRUCTURE_VERSION_KEY.format(course_key), version, timeout=None)
1152

1253

1354
def get_course_in_cache(course_key):
@@ -29,7 +70,8 @@ def update_course_in_cache(course_key):
2970
block_structure.updated_collected function that updates the block
3071
structure in the cache for the given course_key.
3172
"""
32-
return get_block_structure_manager(course_key).update_collected_if_needed()
73+
get_block_structure_manager(course_key).update_collected_if_needed()
74+
_update_block_structure_version(course_key)
3375

3476

3577
def clear_course_from_cache(course_key):

0 commit comments

Comments
 (0)