Revert "feat: [FC-0092] Optimize Course Info Blocks API"

lms/djangoapps/course_api/blocks/api.py

@@ -1,7 +1,7 @@
 """
 API function for retrieving course blocks data
 """
-from edx_django_utils.cache import RequestCache
+
 
 import lms.djangoapps.course_blocks.api as course_blocks_api
 from lms.djangoapps.course_blocks.transformers.access_denied_filter import AccessDeniedMessageFilterTransformer
@@ -14,7 +14,6 @@
 from .toggles import HIDE_ACCESS_DENIALS_FLAG
 from .transformers.blocks_api import BlocksAPITransformer
 from .transformers.milestones import MilestonesAndSpecialExamsTransformer
-from .utils import COURSE_API_REQUEST_CACHE_NAMESPACE, REUSABLE_BLOCKS_CACHE_KEY
 
 
 def get_blocks(
@@ -30,7 +29,6 @@ def get_blocks(
         block_types_filter=None,
         hide_access_denials=False,
         allow_start_dates_in_future=False,
-        cache_with_future_dates=False,
 ):
     """
     Return a serialized representation of the course blocks.
@@ -63,7 +61,6 @@ def get_blocks(
         allow_start_dates_in_future (bool): When True, will allow blocks to be
             returned that can bypass the StartDateTransformer's filter to show
             blocks with start dates in the future.
-        cache_with_future_dates (bool): When True, will use the block caching logic using RequestCache
     """
 
     if HIDE_ACCESS_DENIALS_FLAG.is_enabled():
@@ -121,10 +118,6 @@ def get_blocks(
         ),
     ]
 
-    if cache_with_future_dates:
-        # Include future dates such that get_course_assignments can reuse the block structure from RequestCache
-        allow_start_dates_in_future = True
-
     # transform
     blocks = course_blocks_api.get_course_blocks(
         user,
@@ -135,19 +128,6 @@ def get_blocks(
         include_has_scheduled_content=include_has_scheduled_content
     )
 
-    if cache_with_future_dates:
-        # Store a copy of the transformed, but still unfiltered, course blocks in RequestCache to be reused
-        # wherever possible for optimization. Copying is required to make sure the cached structure is not mutated
-        # by the filtering below.
-        request_cache = RequestCache(COURSE_API_REQUEST_CACHE_NAMESPACE)
-        request_cache.set(REUSABLE_BLOCKS_CACHE_KEY, blocks.copy())
-
-        # Since we included blocks with future start dates in our block structure,
-        # we need to include the 'start' field to filter out such blocks before returning the response.
-        # If 'start' field is not requested, it will be removed from the response.
-        requested_fields = set(requested_fields)
-        requested_fields.add('start')
-
     # filter blocks by types
     if block_types_filter:
         block_keys_to_remove = []
@@ -162,7 +142,7 @@ def get_blocks(
     serializer_context = {
         'request': request,
         'block_structure': blocks,
-        'requested_fields': requested_fields,
+        'requested_fields': requested_fields or [],
     }
 
     if return_type == 'dict':

lms/djangoapps/course_api/blocks/utils.py

@@ -1,7 +1,6 @@
 """
    Utils for Blocks
 """
-from edx_django_utils.cache import RequestCache
 from rest_framework.utils.serializer_helpers import ReturnList
 
 from openedx.core.djangoapps.discussions.models import (
@@ -10,10 +9,6 @@
 )
 
 
-COURSE_API_REQUEST_CACHE_NAMESPACE = "course_api"
-REUSABLE_BLOCKS_CACHE_KEY = "reusable_transformed_blocks"
-
-
 def filter_discussion_xblocks_from_response(response, course_key):
     """
     Removes discussion xblocks if discussion provider is openedx.
@@ -68,18 +63,3 @@ def filter_discussion_xblocks_from_response(response, course_key):
         response.data['blocks'] = filtered_blocks
 
     return response
-
-
-def get_cached_transformed_blocks():
-    """
-    Helper function to get an unfiltered course structure from RequestCache,
-    including blocks with start dates in the future.
-
-    Caution: For performance reasons, the function returns the structure object itself, not its copy.
-    This means the retrieved structure is supposed to be read-only and should not be mutated by consumers.
-    """
-    request_cache = RequestCache(COURSE_API_REQUEST_CACHE_NAMESPACE)
-    cached_response = request_cache.get_cached_response(REUSABLE_BLOCKS_CACHE_KEY)
-    reusable_transformed_blocks = cached_response.value if cached_response.is_found else None
-
-    return reusable_transformed_blocks

lms/djangoapps/course_api/blocks/views.py

@@ -2,7 +2,6 @@
 CourseBlocks API views
 """
 
-from datetime import datetime, timezone
 
 from django.core.exceptions import ValidationError
 from django.db import transaction
@@ -238,7 +237,6 @@ def list(self, request, usage_key_string, hide_access_denials=False):  # pylint:
                     params.cleaned_data['return_type'],
                     params.cleaned_data.get('block_types_filter', None),
                     hide_access_denials=hide_access_denials,
-                    cache_with_future_dates=True
                 )
             )
             # If the username is an empty string, and not None, then we are requesting
@@ -341,50 +339,9 @@ def list(self, request, hide_access_denials=False):  # pylint: disable=arguments
         if not root:
             raise ValidationError(f"Unable to find course block in '{course_key_string}'")
 
-        # Earlier we included blocks with future start dates in the collected/cached block structure.
-        # Now we need to emulate allow_start_dates_in_future=False by removing any such blocks.
-        include_start = "start" in request.query_params['requested_fields']
-        self.remove_future_blocks(course_blocks, include_start)
-
         recurse_mark_complete(root, course_blocks)
         return response
 
-    @staticmethod
-    def remove_future_blocks(course_blocks, include_start: bool):
-        """
-        Mutates course_blocks in place:
-        - removes blocks whose 'start' is in the future
-        - also removes references to them from parents' 'children' lists
-        - removes 'start' key from all blocks if it wasn't requested
-        """
-        if not course_blocks:
-            return course_blocks
-
-        now = datetime.now(timezone.utc)
-
-        # 1. Collect IDs of blocks to remove
-        to_remove = set()
-        for block_id, block in course_blocks.items():
-            get_field = block.get if include_start else block.pop
-            start = get_field("start")
-            if start and start > now:
-                to_remove.add(block_id)
-
-        if not to_remove:
-            return course_blocks
-
-        # 2. Remove the blocks themselves
-        for block_id in to_remove:
-            course_blocks.pop(block_id, None)
-
-        # 3. Clean up children lists
-        for block in course_blocks.values():
-            children = block.get("children")
-            if children:
-                block["children"] = [cid for cid in children if cid not in to_remove]
-
-        return course_blocks
-
 
 @method_decorator(transaction.non_atomic_requests, name='dispatch')
 @view_auth_classes(is_authenticated=False)

lms/djangoapps/courseware/courses.py

@@ -26,7 +26,6 @@
 from common.djangoapps.static_replace import replace_static_urls
 from common.djangoapps.util.date_utils import strftime_localized
 from lms.djangoapps import branding
-from lms.djangoapps.course_api.blocks.utils import get_cached_transformed_blocks
 from lms.djangoapps.course_blocks.api import get_course_blocks
 from lms.djangoapps.courseware.access import has_access
 from lms.djangoapps.courseware.access_response import (
@@ -637,10 +636,7 @@ def get_course_assignments(course_key, user, include_access=False, include_witho
 
     store = modulestore()
     course_usage_key = store.make_course_usage_key(course_key)
-
-    block_data = get_cached_transformed_blocks() or get_course_blocks(
-        user, course_usage_key, allow_start_dates_in_future=True, include_completion=True
-    )
+    block_data = get_course_blocks(user, course_usage_key, allow_start_dates_in_future=True, include_completion=True)
 
     now = datetime.now(pytz.UTC)
     assignments = []

lms/djangoapps/grades/course_data.py

@@ -2,12 +2,12 @@
 Code used to get and cache the requested course-data
 """
 
+
 from lms.djangoapps.course_blocks.api import get_course_blocks
 from openedx.core.djangoapps.content.block_structure.api import get_block_structure_manager
 from xmodule.modulestore.django import modulestore  # lint-amnesty, pylint: disable=wrong-import-order
 
 from .transformer import GradesTransformer
-from ..course_api.blocks.utils import get_cached_transformed_blocks
 
 
 class CourseData:
@@ -56,11 +56,7 @@ def location(self):  # lint-amnesty, pylint: disable=missing-function-docstring
     @property
     def structure(self):  # lint-amnesty, pylint: disable=missing-function-docstring
         if self._structure is None:
-            # The get_course_blocks function proved to be a major time sink during a request at "blocks/".
-            # This caching logic helps improve the response time by getting a copy of the already transformed, but still
-            # unfiltered, course blocks from RequestCache and thus reducing the number of times that
-            # the get_course_blocks function is called.
-            self._structure = get_cached_transformed_blocks() or get_course_blocks(
+            self._structure = get_course_blocks(
                 self.user,
                 self.location,
                 collected_block_structure=self._collected_block_structure,

lms/djangoapps/mobile_api/tests/test_course_info_views.py

@@ -432,78 +432,6 @@ def test_extend_sequential_info_with_assignment_progress_for_other_types(self, b
         for block_info in response.data['blocks'].values():
             self.assertNotEqual('assignment_progress', block_info)
 
-    def test_response_keys(self):
-        response = self.verify_response(url=self.url)
-        data = response.data
-
-        expected_top_level_keys = {
-            'blocks',
-            'certificate',
-            'course_about',
-            'course_access_details',
-            'course_handouts',
-            'course_modes',
-            'course_progress',
-            'course_sharing_utm_parameters',
-            'course_updates',
-            'deprecate_youtube',
-            'discussion_url',
-            'end',
-            'enrollment_details',
-            'id',
-            'is_self_paced',
-            'media',
-            'name',
-            'number',
-            'org',
-            'org_logo',
-            'root',
-            'start',
-            'start_display',
-            'start_type'
-        }
-        expected_course_access_keys = {
-            "has_unmet_prerequisites",
-            "is_too_early",
-            "is_staff",
-            "audit_access_expires",
-            "courseware_access"
-        }
-        expected_courseware_access_keys = {
-            "has_access",
-            "error_code",
-            "developer_message",
-            "user_message",
-            "additional_context_user_message",
-            "user_fragment"
-        }
-        expected_enrollment_details_keys = {"created", "mode", "is_active", "upgrade_deadline"}
-        expected_media_keys = {"image"}
-        expected_image_keys = {"raw", "small", "large"}
-        expected_course_sharing_keys = {"facebook", "twitter"}
-        expected_course_modes_keys = {"slug", "sku", "android_sku", "ios_sku", "min_price"}
-        expected_course_progress_keys = {"total_assignments_count", "assignments_completed"}
-
-        self.assertSetEqual(set(data), expected_top_level_keys)
-        self.assertSetEqual(set(data["course_access_details"]), expected_course_access_keys)
-        self.assertSetEqual(set(data["course_access_details"]["courseware_access"]), expected_courseware_access_keys)
-        self.assertSetEqual(set(data["enrollment_details"]), expected_enrollment_details_keys)
-        self.assertSetEqual(set(data["media"]), expected_media_keys)
-        self.assertSetEqual(set(data["media"]["image"]), expected_image_keys)
-        self.assertSetEqual(set(data["course_sharing_utm_parameters"]), expected_course_sharing_keys)
-        self.assertSetEqual(set(data["course_modes"][0]), expected_course_modes_keys)
-        self.assertSetEqual(set(data["course_progress"]), expected_course_progress_keys)
-
-    def test_block_count_depends_on_depth_in_request_params(self):
-        response_depth_zero = self.verify_response(url=self.url, params={'depth': 0})
-        response_depth_one = self.verify_response(url=self.url, params={'depth': 1})
-        blocks_depth_zero = [block for block in self.store.get_items(self.course_key) if block.category == "course"]
-        blocks_depth_one = [
-            block for block in self.store.get_items(self.course_key) if block.category in ("course", "chapter")
-        ]
-        self.assertEqual(len(response_depth_zero.data["blocks"]), len(blocks_depth_zero))
-        self.assertEqual(len(response_depth_one.data["blocks"]), len(blocks_depth_one))
-
 
 class TestCourseEnrollmentDetailsView(MobileAPITestCase, MilestonesTestCaseMixin):  # lint-amnesty, pylint: disable=test-inherits-tests
     """