diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index 84a76fd77312..e0bc9fcc9558 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -21,7 +21,6 @@ CoursewareSearchIndexer, LibrarySearchIndexer, ) -from cms.djangoapps.contentstore.utils import drop_course_sidebar_blocks_cache from common.djangoapps.track.event_transaction_utils import get_event_transaction_id, get_event_transaction_type from common.djangoapps.util.block_utils import yield_dynamic_block_descendants from lms.djangoapps.grades.api import task_compute_all_grades_for_course @@ -142,7 +141,6 @@ def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable= # register special exams asynchronously after the data is ready course_key_str = str(course_key) transaction.on_commit(lambda: update_special_exams_and_publish.delay(course_key_str)) - drop_course_sidebar_blocks_cache(course_key_str) if key_supports_outlines(course_key): # Push the course outline to learning_sequences asynchronously. diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 1a69a11c2e88..d14944436a7e 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -8,11 +8,10 @@ from collections import defaultdict from contextlib import contextmanager from datetime import datetime, timezone -from urllib.parse import quote_plus, unquote +from urllib.parse import quote_plus from uuid import uuid4 from django.conf import settings -from django.core.cache import cache from django.core.exceptions import ValidationError from django.urls import reverse from django.utils import translation @@ -2240,10 +2239,8 @@ def send_course_update_notification(course_key, content, user): def get_xblock_validation_messages(xblock): """ Retrieves validation messages for a given xblock. - Args: xblock: The xblock object to validate. - Returns: list: A list of validation error messages. """ @@ -2254,11 +2251,9 @@ def get_xblock_validation_messages(xblock): def get_xblock_render_error(request, xblock): """ Checks if there are any rendering errors for a given block and return these. - Args: request: WSGI request object xblock: The xblock object to rendering. - Returns: str: Error message which happened while rendering of xblock. """ @@ -2292,37 +2287,3 @@ def get_xblock_render_context(request, block): return str(exc) return "" - - -def drop_course_sidebar_blocks_cache(course_id: str): - """ - Drop the course sidebar blocks cache for the given course. - """ - cache_key_prefix = f"course_sidebar_blocks_{course_id}" - cache_keys = get_cache_keys(cache_key_prefix) - - cache.delete_many(cache_keys) - - -def get_cache_keys(cache_key_prefix): - """ - Get all cache keys for the given cache key prefix. - - LocMemCache does not have a keys method, so we need to iterate over the cache - and manually filter out the keys that match the given prefix. - """ - cache_backend = settings.CACHES['default']['BACKEND'] - if cache_backend == 'django_redis.cache.RedisCache': - yield cache.iter_keys(f"{cache_key_prefix}*") - elif cache_backend == 'django.core.cache.backends.locmem.LocMemCache': - for key in cache._cache.keys(): # pylint: disable=protected-access - try: - decoded_key = unquote(key.split(':', 2)[-1], encoding='utf-8') - except IndexError: - continue - - if decoded_key.startswith(cache_key_prefix): - yield decoded_key - else: - log.error(f"Unsupported cache backend: {cache_backend}") - yield diff --git a/lms/djangoapps/course_home_api/outline/serializers.py b/lms/djangoapps/course_home_api/outline/serializers.py index 5db6110619b6..b5e2a488b78c 100644 --- a/lms/djangoapps/course_home_api/outline/serializers.py +++ b/lms/djangoapps/course_home_api/outline/serializers.py @@ -85,6 +85,7 @@ def get_vertical_icon_class(block): for higher_class in icon_call_priority: if higher_class in child_classes: new_class = higher_class + return new_class diff --git a/lms/djangoapps/course_home_api/outline/tests/test_view.py b/lms/djangoapps/course_home_api/outline/tests/test_view.py index 797cf4cca442..4f40d9a80192 100644 --- a/lms/djangoapps/course_home_api/outline/tests/test_view.py +++ b/lms/djangoapps/course_home_api/outline/tests/test_view.py @@ -467,7 +467,7 @@ def __init__(self, *args, **kwargs): def setUp(self): super().setUp() - self.url = reverse('course-home:course-sidebar-blocks', args=[self.course.id]) + self.url = reverse('course-home:course-navigation', args=[self.course.id]) def update_course_and_overview(self): """ @@ -573,7 +573,7 @@ def test_get_unknown_course(self): """ Test that the API returns a 404 when the course is not found. """ - url = reverse('course-home:course-sidebar-blocks', args=['course-v1:unknown+course+2T2020']) + url = reverse('course-home:course-navigation', args=['course-v1:unknown+course+2T2020']) response = self.client.get(url) assert response.status_code == 404 @@ -624,7 +624,7 @@ def test_proctored_exam(self, mock_summary): 'suggested_icon': 'fa-foo-bar', } - url = reverse('course-home:course-sidebar-blocks', args=[course.id]) + url = reverse('course-home:course-navigation', args=[course.id]) response = self.client.get(url) assert response.status_code == 200 @@ -715,7 +715,7 @@ def test_empty_blocks_complete(self): """ self.add_blocks_to_course() CourseEnrollment.enroll(self.user, self.course.id) - url = reverse('course-home:course-sidebar-blocks', args=[self.course.id]) + url = reverse('course-home:course-navigation', args=[self.course.id]) response = self.client.get(url) assert response.status_code == 200 @@ -731,7 +731,7 @@ def test_blocks_complete_with_problem(self, problem_complete): CourseEnrollment.enroll(self.user, self.course.id) self.create_completion(problem, int(problem_complete)) - response = self.client.get(reverse('course-home:course-sidebar-blocks', args=[self.course.id])) + response = self.client.get(reverse('course-home:course-navigation', args=[self.course.id])) sequence_data = response.data['blocks'][str(self.sequential.location)] vertical_data = response.data['blocks'][str(self.vertical.location)] @@ -750,7 +750,7 @@ def test_blocks_completion_stat(self): CourseEnrollment.enroll(self.user, self.course.id) self.create_completion(completed_problem, 1) self.create_completion(uncompleted_problem, 0) - response = self.client.get(reverse('course-home:course-sidebar-blocks', args=[self.course.id])) + response = self.client.get(reverse('course-home:course-navigation', args=[self.course.id])) expected_sequence_completion_stat = { 'completion': 0, @@ -779,7 +779,7 @@ def test_blocks_completion_stat_all_problem_completed(self): CourseEnrollment.enroll(self.user, self.course.id) self.create_completion(problem1, 1) self.create_completion(problem2, 1) - response = self.client.get(reverse('course-home:course-sidebar-blocks', args=[self.course.id])) + response = self.client.get(reverse('course-home:course-navigation', args=[self.course.id])) expected_sequence_completion_stat = { 'completion': 1, diff --git a/lms/djangoapps/course_home_api/outline/views.py b/lms/djangoapps/course_home_api/outline/views.py index 8bf6be9d01a4..1d02261dbc0c 100644 --- a/lms/djangoapps/course_home_api/outline/views.py +++ b/lms/djangoapps/course_home_api/outline/views.py @@ -40,7 +40,7 @@ from lms.djangoapps.courseware.courses import get_course_date_blocks, get_course_info_section from lms.djangoapps.courseware.date_summary import TodaysDate from lms.djangoapps.courseware.masquerade import is_masquerading, setup_masquerade -from lms.djangoapps.courseware.toggles import courseware_mfe_navigation_sidebar_blocks_caching_is_enabled +from lms.djangoapps.courseware.toggles import courseware_disable_navigation_sidebar_blocks_caching from lms.djangoapps.courseware.views.views import get_cert_data from lms.djangoapps.grades.course_grade_factory import CourseGradeFactory from lms.djangoapps.utils import OptimizelyClient @@ -381,12 +381,12 @@ def finalize_response(self, request, response, *args, **kwargs): return expose_header('Date', response) -class CourseSidebarBlocksView(RetrieveAPIView): +class CourseNavigationBlocksView(RetrieveAPIView): """ **Use Cases** Request details for the sidebar navigation of the course. **Example Requests** - GET api/course_home/v1/sidebar/{course_key} + GET api/course_home/v1/navigation/{course_key} **Response Values** For a good 200 response, the response will include: blocks: List of serialized Course Block objects. Each serialization has the following fields: @@ -416,8 +416,8 @@ class CourseSidebarBlocksView(RetrieveAPIView): serializer_class = CourseBlockSerializer COURSE_BLOCKS_CACHE_KEY_TEMPLATE = ( - 'course_sidebar_blocks_{course_key_string}_{user_id}_{user_cohort_id}' - '_{allow_public}_{allow_public_outline}_{is_masquerading}' + 'course_sidebar_blocks_{course_key_string}_{course_version}_{user_id}_{user_cohort_id}' + '_{enrollment_mode}_{allow_public}_{allow_public_outline}_{is_masquerading}' ) COURSE_BLOCKS_CACHE_TIMEOUT = 60 * 60 # 1 hour @@ -445,22 +445,25 @@ def get(self, request, *args, **kwargs): enrollment = CourseEnrollment.get_enrollment(request.user, course_key) try: - user_cohort = get_cohort(request.user, course_key) + user_cohort = get_cohort(request.user, course_key, use_cached=True) except ValueError: user_cohort = None cache_key = self.COURSE_BLOCKS_CACHE_KEY_TEMPLATE.format( course_key_string=course_key_string, + course_version=str(course.course_version), user_id=request.user.id, + enrollment_mode=getattr(enrollment, 'mode', ''), user_cohort_id=getattr(user_cohort, 'id', ''), allow_public=allow_public, allow_public_outline=allow_public_outline, is_masquerading=user_is_masquerading, ) - if courseware_mfe_navigation_sidebar_blocks_caching_is_enabled(): - course_blocks = cache.get(cache_key) - else: + + if navigation_sidebar_caching_is_disabled := courseware_disable_navigation_sidebar_blocks_caching(): course_blocks = None + else: + course_blocks = cache.get(cache_key) if not course_blocks: if getattr(enrollment, 'is_active', False) or bool(staff_access): @@ -468,10 +471,10 @@ def get(self, request, *args, **kwargs): elif allow_public_outline or allow_public or user_is_masquerading: course_blocks = get_course_outline_block_tree(request, course_key_string, None) - if courseware_mfe_navigation_sidebar_blocks_caching_is_enabled(): + if not navigation_sidebar_caching_is_disabled: cache.set(cache_key, course_blocks, self.COURSE_BLOCKS_CACHE_TIMEOUT) - course_blocks = self.filter_unavailable_blocks(course_blocks, course_key) + course_blocks = self.filter_inaccessible_blocks(course_blocks, course_key) if course_blocks: course_blocks = self.mark_complete_recursive(course_blocks) @@ -486,19 +489,19 @@ def get(self, request, *args, **kwargs): return Response(serializer.data) - def filter_unavailable_blocks(self, course_blocks, course_key): + def filter_inaccessible_blocks(self, course_blocks, course_key): """ - Filter out sections and subsections that are not available to the current user. + Filter out sections and subsections that are not accessible to the current user. """ if course_blocks: user_course_outline = get_user_course_outline(course_key, self.request.user, datetime.now(tz=timezone.utc)) course_sections = course_blocks.get('children', []) - course_blocks['children'] = self.get_available_sections(user_course_outline, course_sections) + course_blocks['children'] = self.get_accessible_sections(user_course_outline, course_sections) for section_data in course_sections: - section_data['children'] = self.get_available_sequences( + section_data['children'] = self.get_accessible_sequences( user_course_outline, - section_data.get('children', ['completion']) + section_data.get('children', []) ) accessible_sequence_ids = {str(usage_key) for usage_key in user_course_outline.accessible_sequences} for sequence_data in section_data['children']: @@ -550,9 +553,9 @@ def get_completable_children(block): return [child for child in block.get('children', []) if child['type'] != 'discussion'] @staticmethod - def get_available_sections(user_course_outline, course_sections): + def get_accessible_sections(user_course_outline, course_sections): """ - Filter out sections that are not available to the user. + Filter out sections that are not accessible to the user. """ available_section_ids = set(map(lambda section: str(section.usage_key), user_course_outline.sections)) return list(filter( @@ -560,9 +563,9 @@ def get_available_sections(user_course_outline, course_sections): )) @staticmethod - def get_available_sequences(user_course_outline, course_sequences): + def get_accessible_sequences(user_course_outline, course_sequences): """ - Filter out sequences that are not available to the user. + Filter out sequences that are not accessible to the user. """ available_sequence_ids = set(map(str, user_course_outline.sequences)) diff --git a/lms/djangoapps/course_home_api/urls.py b/lms/djangoapps/course_home_api/urls.py index 62e356d34710..2ce9903b6031 100644 --- a/lms/djangoapps/course_home_api/urls.py +++ b/lms/djangoapps/course_home_api/urls.py @@ -9,7 +9,7 @@ from lms.djangoapps.course_home_api.course_metadata.views import CourseHomeMetadataView from lms.djangoapps.course_home_api.dates.views import DatesTabView from lms.djangoapps.course_home_api.outline.views import ( - CourseSidebarBlocksView, + CourseNavigationBlocksView, OutlineTabView, dismiss_welcome_message, save_course_goal, @@ -49,9 +49,9 @@ name='outline-tab' ), re_path( - fr'sidebar/{settings.COURSE_KEY_PATTERN}', - CourseSidebarBlocksView.as_view(), - name='course-sidebar-blocks' + fr'navigation/{settings.COURSE_KEY_PATTERN}', + CourseNavigationBlocksView.as_view(), + name='course-navigation' ), re_path( r'dismiss_welcome_message', diff --git a/lms/djangoapps/courseware/toggles.py b/lms/djangoapps/courseware/toggles.py index ba319ca98928..4aadfd3e93ed 100644 --- a/lms/djangoapps/courseware/toggles.py +++ b/lms/djangoapps/courseware/toggles.py @@ -71,14 +71,16 @@ # .. toggle_name: courseware.navigation_sidebar_blocks_caching # .. toggle_implementation: WaffleFlag # .. toggle_default: False -# .. toggle_description: Enable caching of navigation sidebar blocks on Learning MFE to improve performance. +# .. toggle_description: Disable caching of navigation sidebar blocks on Learning MFE. +# It can be used when caching the structure of large courses for a large number of users +# at the same time can overload the cache storage (memcache or redis). # .. toggle_use_cases: temporary # .. toggle_creation_date: 2024-03-21 # .. toggle_target_removal_date: None -# .. toggle_tickets: AXIMST-682 +# .. toggle_tickets: FC-0056 # .. toggle_warning: None. -COURSEWARE_MICROFRONTEND_NAVIGATION_SIDEBAR_BLOCKS_CACHING_ENABLED = CourseWaffleFlag( - f'{WAFFLE_FLAG_NAMESPACE}.navigation_sidebar_blocks_caching', __name__ +COURSEWARE_MICROFRONTEND_NAVIGATION_SIDEBAR_BLOCKS_DISABLE_CACHING = CourseWaffleFlag( + f'{WAFFLE_FLAG_NAMESPACE}.disable_navigation_sidebar_blocks_caching', __name__ ) # .. toggle_name: courseware.disable_navigation_sidebar @@ -228,8 +230,8 @@ def courseware_show_default_right_sidebar_is_enabled(course_key=None): return COURSEWARE_SHOW_DEFAULT_RIGHT_SIDEBAR.is_enabled(course_key) -def courseware_mfe_navigation_sidebar_blocks_caching_is_enabled(course_key=None): +def courseware_disable_navigation_sidebar_blocks_caching(course_key=None): """ - Return whether the courseware.navigation_sidebar_blocks_caching flag is on. + Return whether the courseware.disable_navigation_sidebar_blocks_caching flag is on. """ - return COURSEWARE_MICROFRONTEND_NAVIGATION_SIDEBAR_BLOCKS_CACHING_ENABLED.is_enabled(course_key) + return COURSEWARE_MICROFRONTEND_NAVIGATION_SIDEBAR_BLOCKS_DISABLE_CACHING.is_enabled(course_key)