Merge pull request #14358 from edx/beryl/bs_cache_invalidation

Block Structure: Don't invalidate immediately upon course publish

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

@@ -5,21 +5,24 @@ from django.conf import settings
 from django.dispatch.dispatcher import receiver
 
 from xmodule.modulestore.django import SignalHandler
+from waffle import switch_is_active
 
 from .api import clear_course_from_cache
 from .tasks import update_course_in_cache
 
 
+INVALIDATE_CACHE_ON_PUBLISH_SWITCH = 'block_structure_invalidate_cache_on_publish'
+
+
 @receiver(SignalHandler.course_published)
 def _listen_for_course_publish(sender, course_key, **kwargs):  # pylint: disable=unused-argument
     """
     Catches the signal that a course has been published in the module
     store and creates/updates the corresponding cache entry.
     """
-    clear_course_from_cache(course_key)
+    if switch_is_active(INVALIDATE_CACHE_ON_PUBLISH_SWITCH):
+        clear_course_from_cache(course_key)
 
-    # The countdown=0 kwarg ensures the call occurs after the signal emitter
-    # has finished all operations.
     update_course_in_cache.apply_async(
         [unicode(course_key)],
         countdown=settings.BLOCK_STRUCTURES_SETTINGS['BLOCK_STRUCTURES_COURSE_PUBLISH_TASK_DELAY'],

openedx/core/djangoapps/content/block_structure/tests/test_signals.py

 """
 Unit tests for the Course Blocks signals
 """
+import ddt
+from mock import patch
+from waffle.testutils import override_switch
 
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory
 
 from ..api import get_block_structure_manager
+from ..signals import INVALIDATE_CACHE_ON_PUBLISH_SWITCH
 from .helpers import is_course_in_block_structure_cache
 
 
+@ddt.ddt
 class CourseBlocksSignalTest(ModuleStoreTestCase):
     """
     Tests for the Course Blocks signal
@@ -41,6 +46,17 @@ class CourseBlocksSignalTest(ModuleStoreTestCase):
             updated_block_structure.get_xblock_field(self.course_usage_key, 'display_name')
         )
 
+    @ddt.data(True, False)
+    @patch('openedx.core.lib.block_structure.manager.BlockStructureManager.clear')
+    def test_cache_invalidation(self, invalidate_cache_enabled, mock_bs_manager_clear):
+        test_display_name = "Jedi 101"
+
+        with override_switch(INVALIDATE_CACHE_ON_PUBLISH_SWITCH, active=invalidate_cache_enabled):
+            self.course.display_name = test_display_name
+            self.store.update_item(self.course, self.user.id)
+
+        self.assertEquals(mock_bs_manager_clear.called, invalidate_cache_enabled)
+
     def test_course_delete(self):
         bs_manager = get_block_structure_manager(self.course.id)
         self.assertIsNotNone(bs_manager.get_collected())

openedx/core/lib/block_structure/manager.py

@@ -95,24 +95,24 @@ class BlockStructureManager(object):
         )
         cache_miss = block_structure is None
         if cache_miss or BlockStructureTransformers.is_collected_outdated(block_structure):
-            with self._bulk_operations():
-                block_structure = BlockStructureFactory.create_from_modulestore(
-                    self.root_block_usage_key,
-                    self.modulestore
-                )
-                BlockStructureTransformers.collect(block_structure)
-                self.block_structure_cache.add(block_structure)
+            block_structure = self.update_collected()
         return block_structure
 
     def update_collected(self):
         """
         Updates the collected Block Structure for the root_block_usage_key.
 
-        Details: The cache is cleared and updated by collecting transformers
-        data from the modulestore.
+        Details: The cache is updated by collecting transformers data from
+        the modulestore.
         """
-        self.clear()
-        self.get_collected()
+        with self._bulk_operations():
+            block_structure = BlockStructureFactory.create_from_modulestore(
+                self.root_block_usage_key,
+                self.modulestore,
+            )
+            BlockStructureTransformers.collect(block_structure)
+            self.block_structure_cache.add(block_structure)
+            return block_structure
 
     def clear(self):
         """

openedx/core/lib/celery/routers.py

@@ -33,13 +33,6 @@ class AlternateEnvironmentRouter(object):
         If None is returned from this method, default routing logic is used.
         """
         alternate_env = self.alternate_env_tasks.get(task, None)
-        if 'update_course_in_cache' in task:
-            log.info("TNL-5408: task={task}, args={args}, alternate_env={alt_env}, queues={queues}".format(
-                task=task,
-                args=args,
-                alt_env=alternate_env,
-                queues=getattr(settings, 'CELERY_QUEUES', []).keys()
-            ))
         if alternate_env:
             return self.ensure_queue_env(alternate_env)
         return None