Merge pull request #35185 from openedx/feanil/blockstructure_cache

feat!: Drop the block_structure.storage_backing_for_cache WaffleSwitch

cms/djangoapps/contentstore/management/commands/import.py

@@ -8,7 +8,7 @@
 from openedx.core.djangoapps.django_comment_common.utils import are_permissions_roles_seeded, seed_permissions_roles
 from xmodule.contentstore.django import contentstore  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.modulestore import ModuleStoreEnum  # lint-amnesty, pylint: disable=wrong-import-order
-from xmodule.modulestore.django import modulestore  # lint-amnesty, pylint: disable=wrong-import-order
+from xmodule.modulestore.django import SignalHandler, modulestore  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.modulestore.xml_importer import import_course_from_xml  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.util.sandboxing import DEFAULT_PYTHON_LIB_FILENAME  # lint-amnesty, pylint: disable=wrong-import-order
 
@@ -73,6 +73,10 @@ def handle(self, *args, **options):
 
         for course in course_items:
             course_id = course.id
+            # Importing is an act of publishing so send the course published signal.
+            SignalHandler.course_published.send_robust(sender=self, course_key=course_id)
+
+            # Seed forum permission roles if we need to.
             if not are_permissions_roles_seeded(course_id):
                 self.stdout.write(f'Seeding forum roles for course {course_id}\n')
                 seed_permissions_roles(course_id)

lms/djangoapps/ccx/tests/test_field_override_performance.py

@@ -234,7 +234,7 @@ class TestFieldOverrideSplitPerformance(FieldOverridePerformanceTestCase):
     __test__ = True
 
     # TODO: decrease query count as part of REVO-28
-    QUERY_COUNT = 33
+    QUERY_COUNT = 34
 
     TEST_DATA = {
         ('no_overrides', 1, True, False): (QUERY_COUNT, 2),

lms/djangoapps/course_api/blocks/tests/test_api.py

@@ -3,16 +3,13 @@
 """
 
 
-from itertools import product
 from unittest.mock import patch
 
 import ddt
 from django.test.client import RequestFactory
-from edx_toggles.toggles.testutils import override_waffle_switch
 
 from common.djangoapps.student.tests.factories import UserFactory
 from openedx.core.djangoapps.content.block_structure.api import clear_course_from_cache
-from openedx.core.djangoapps.content.block_structure.config import STORAGE_BACKING_FOR_CACHE
 from xmodule.modulestore import ModuleStoreEnum  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.modulestore.tests.factories import SampleCourseFactory, check_mongo_calls  # lint-amnesty, pylint: disable=wrong-import-order
@@ -209,34 +206,25 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase):
     Tests query counts for the get_blocks function.
     """
 
-    @ddt.data(
-        *product(
-            (ModuleStoreEnum.Type.split, ),
-            (True, False),
+    @ddt.data(ModuleStoreEnum.Type.split)
+    def test_query_counts_cached(self, store_type):
+        course = self._create_course(store_type)
+        self._get_blocks(
+            course,
+            expected_mongo_queries=0,
+            expected_sql_queries=14,
         )
-    )
-    @ddt.unpack
-    def test_query_counts_cached(self, store_type, with_storage_backing):
-        with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing):
-            course = self._create_course(store_type)
-            self._get_blocks(
-                course,
-                expected_mongo_queries=0,
-                expected_sql_queries=14 if with_storage_backing else 13,
-            )
 
     @ddt.data(
-        (ModuleStoreEnum.Type.split, 2, True, 24),
-        (ModuleStoreEnum.Type.split, 2, False, 14),
+        (ModuleStoreEnum.Type.split, 2, 24),
     )
     @ddt.unpack
-    def test_query_counts_uncached(self, store_type, expected_mongo_queries, with_storage_backing, num_sql_queries):
-        with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing):
-            course = self._create_course(store_type)
-            clear_course_from_cache(course.id)
-
-            self._get_blocks(
-                course,
-                expected_mongo_queries,
-                expected_sql_queries=num_sql_queries,
-            )
+    def test_query_counts_uncached(self, store_type, expected_mongo_queries, num_sql_queries):
+        course = self._create_course(store_type)
+        clear_course_from_cache(course.id)
+
+        self._get_blocks(
+            course,
+            expected_mongo_queries,
+            expected_sql_queries=num_sql_queries,
+        )

lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py

@@ -13,6 +13,7 @@
 from lms.djangoapps.course_blocks.api import get_course_blocks
 from lms.djangoapps.course_blocks.transformers.tests.helpers import CourseStructureTestCase
 from lms.djangoapps.gating import api as lms_gating_api
+import openedx.core.djangoapps.content.block_structure.api as bs_api
 from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers
 from openedx.core.djangoapps.course_apps.toggles import EXAMS_IDA
 from openedx.core.lib.gating import api as gating_api
@@ -166,7 +167,11 @@ def test_gated(self, gated_block_ref, gating_block_ref, expected_blocks_before_c
         self.course.enable_subsection_gating = True
         self.setup_gated_section(self.blocks[gated_block_ref], self.blocks[gating_block_ref])
 
-        with self.assertNumQueries(5):
+        # Cache the course blocks so that they don't need to be generated when we're trying to
+        # get data back.  This would happen as a part of publishing in a production system.
+        bs_api.update_course_in_cache(self.course.id)
+
+        with self.assertNumQueries(4):
             self.get_blocks_and_check_against_expected(self.user, expected_blocks_before_completion)
 
         # clear the request cache to simulate a new request
@@ -179,7 +184,7 @@ def test_gated(self, gated_block_ref, gating_block_ref, expected_blocks_before_c
             self.user,
         )
 
-        with self.assertNumQueries(6):
+        with self.assertNumQueries(4):
             self.get_blocks_and_check_against_expected(self.user, self.ALL_BLOCKS_EXCEPT_SPECIAL)
 
     def test_staff_access(self):

lms/djangoapps/course_blocks/transformers/tests/test_library_content.py

@@ -8,6 +8,7 @@
 from openedx.core.djangoapps.content.block_structure.api import clear_course_from_cache
 from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers
 
+import openedx.core.djangoapps.content.block_structure.api as bs_api
 from ...api import get_course_blocks
 from ..library_content import ContentLibraryOrderTransformer, ContentLibraryTransformer
 from .helpers import CourseStructureTestCase
@@ -41,6 +42,8 @@ def setUp(self):
         self.course_hierarchy = self.get_course_hierarchy()
         self.blocks = self.build_course(self.course_hierarchy)
         self.course = self.blocks['course']
+        # Do this manually because publish signals are not fired by default in tests.
+        bs_api.update_course_in_cache(self.course.id)
         clear_course_from_cache(self.course.id)
 
         # Enroll user in course.
@@ -122,6 +125,7 @@ def test_content_library(self):
         )
         assert len(list(raw_block_structure.get_block_keys())) == len(self.blocks)
 
+        bs_api.update_course_in_cache(self.course.id)
         clear_course_from_cache(self.course.id)
         trans_block_structure = get_course_blocks(
             self.user,
@@ -175,6 +179,7 @@ def setUp(self):
         self.course_hierarchy = self.get_course_hierarchy()
         self.blocks = self.build_course(self.course_hierarchy)
         self.course = self.blocks['course']
+        bs_api.update_course_in_cache(self.course.id)
         clear_course_from_cache(self.course.id)
 
         # Enroll user in course.

lms/djangoapps/courseware/tests/test_views.py

@@ -1472,8 +1472,8 @@ def test_view_certificate_link(self):
             self.assertContains(resp, "earned a certificate for this course.")
 
     @ddt.data(
-        (True, 53),
-        (False, 53),
+        (True, 54),
+        (False, 54),
     )
     @ddt.unpack
     def test_progress_queries_paced_courses(self, self_paced, query_count):
@@ -1488,13 +1488,13 @@ def test_progress_queries(self):
         ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1))
         self.setup_course()
         with self.assertNumQueries(
-            53, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST
+            54, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST
         ), check_mongo_calls(2):
             self._get_progress_page()
 
         for _ in range(2):
             with self.assertNumQueries(
-                38, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST
+                39, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST
             ), check_mongo_calls(2):
                 self._get_progress_page()
 

lms/djangoapps/grades/rest_api/v1/tests/test_gradebook_views.py

@@ -22,6 +22,7 @@
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory
 
+import openedx.core.djangoapps.content.block_structure.api as bs_api
 from common.djangoapps.course_modes.models import CourseMode
 from common.djangoapps.student.roles import (
     CourseBetaTesterRole,
@@ -2228,6 +2229,9 @@ def test_get_override_for_unreleased_block(self):
             display_name='Unreleased Section',
         )
 
+        # We need to update the course in the cache after we create the new block.
+        bs_api.update_course_in_cache(self.course_data.course_key)
+
         resp = self.client.get(
             self.get_url(subsection_id=unreleased_subsection.location)
         )

lms/djangoapps/grades/tests/integration/test_events.py

@@ -7,6 +7,7 @@
 
 from crum import set_current_request
 
+import openedx.core.djangoapps.content.block_structure.api as bs_api
 from xmodule.capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory
 from common.djangoapps.student.models import CourseEnrollment
 from common.djangoapps.student.tests.factories import UserFactory
@@ -76,44 +77,45 @@ def setUp(self):
         CourseEnrollment.enroll(self.student, self.course.id)
         self.instructor = UserFactory.create(is_staff=True, username='test_instructor', password=self.TEST_PASSWORD)
         self.refresh_course()
+        # Since this doesn't happen automatically and we don't want to run all the publish signal handlers
+        # Just make sure we have the latest version of the course in cache before we test the problem.
+        bs_api.update_course_in_cache(self.course.id)
 
     @patch('lms.djangoapps.grades.events.tracker')
     def test_submit_answer(self, events_tracker):
         self.submit_question_answer('p1', {'2_1': 'choice_choice_2'})
-        course = self.store.get_course(self.course.id, depth=0)
 
         event_transaction_id = events_tracker.emit.mock_calls[0][1][1]['event_transaction_id']
-        events_tracker.emit.assert_has_calls(
-            [
-                mock_call(
-                    events.PROBLEM_SUBMITTED_EVENT_TYPE,
-                    {
-                        'user_id': str(self.student.id),
-                        'event_transaction_id': event_transaction_id,
-                        'event_transaction_type': events.PROBLEM_SUBMITTED_EVENT_TYPE,
-                        'course_id': str(self.course.id),
-                        'problem_id': str(self.problem.location),
-                        'weighted_earned': 2.0,
-                        'weighted_possible': 2.0,
-                    },
-                ),
-                mock_call(
-                    events.COURSE_GRADE_CALCULATED,
-                    {
-                        'course_version': str(course.course_version),
-                        'percent_grade': 0.02,
-                        'grading_policy_hash': 'ChVp0lHGQGCevD0t4njna/C44zQ=',
-                        'user_id': str(self.student.id),
-                        'letter_grade': '',
-                        'event_transaction_id': event_transaction_id,
-                        'event_transaction_type': events.PROBLEM_SUBMITTED_EVENT_TYPE,
-                        'course_id': str(self.course.id),
-                        'course_edited_timestamp': str(course.subtree_edited_on),
-                    }
-                ),
-            ],
-            any_order=True,
-        )
+        expected_calls = [
+            mock_call(
+                events.PROBLEM_SUBMITTED_EVENT_TYPE,
+                {
+                    'user_id': str(self.student.id),
+                    'event_transaction_id': event_transaction_id,
+                    'event_transaction_type': events.PROBLEM_SUBMITTED_EVENT_TYPE,
+                    'course_id': str(self.course.id),
+                    'problem_id': str(self.problem.location),
+                    'weighted_earned': 2.0,
+                    'weighted_possible': 2.0,
+                },
+            ),
+            mock_call(
+                events.COURSE_GRADE_CALCULATED,
+                {
+                    'course_version': str(self.course.course_version),
+                    'percent_grade': 0.02,
+                    'grading_policy_hash': 'ChVp0lHGQGCevD0t4njna/C44zQ=',
+                    'user_id': str(self.student.id),
+                    'letter_grade': '',
+                    'event_transaction_id': event_transaction_id,
+                    'event_transaction_type': events.PROBLEM_SUBMITTED_EVENT_TYPE,
+                    'course_id': str(self.course.id),
+                    'course_edited_timestamp': str(self.course.subtree_edited_on),
+                }
+            ),
+        ]
+
+        events_tracker.emit.assert_has_calls(expected_calls, any_order=True)
 
     @ddt.data(True, False)
     def test_delete_student_state(self, emit_signals):

lms/djangoapps/grades/tests/test_course_grade_factory.py

@@ -68,35 +68,35 @@ def _assert_section_order(course_grade):
                 self.sequence2.display_name
             ]
 
-        with self.assertNumQueries(4), mock_get_score(1, 2):
+        with self.assertNumQueries(5), mock_get_score(1, 2):
             _assert_read(expected_pass=False, expected_percent=0)  # start off with grade of 0
 
-        num_queries = 42
+        num_queries = 43
         with self.assertNumQueries(num_queries), mock_get_score(1, 2):
             grade_factory.update(self.request.user, self.course, force_update_subsections=True)
 
-        with self.assertNumQueries(3):
+        with self.assertNumQueries(4):
             _assert_read(expected_pass=True, expected_percent=0.5)  # updated to grade of .5
 
-        num_queries = 6
+        num_queries = 7
         with self.assertNumQueries(num_queries), mock_get_score(1, 4):
             grade_factory.update(self.request.user, self.course, force_update_subsections=False)
 
-        with self.assertNumQueries(3):
+        with self.assertNumQueries(4):
             _assert_read(expected_pass=True, expected_percent=0.5)  # NOT updated to grade of .25
 
-        num_queries = 18
+        num_queries = 19
         with self.assertNumQueries(num_queries), mock_get_score(2, 2):
             grade_factory.update(self.request.user, self.course, force_update_subsections=True)
 
-        with self.assertNumQueries(3):
+        with self.assertNumQueries(4):
             _assert_read(expected_pass=True, expected_percent=1.0)  # updated to grade of 1.0
 
-        num_queries = 28
+        num_queries = 29
         with self.assertNumQueries(num_queries), mock_get_score(0, 0):  # the subsection now is worth zero
             grade_factory.update(self.request.user, self.course, force_update_subsections=True)
 
-        with self.assertNumQueries(3):
+        with self.assertNumQueries(4):
             _assert_read(expected_pass=False, expected_percent=0.0)  # updated to grade of 0.0
 
     @ddt.data((True, False))
@@ -286,7 +286,7 @@ def test_grading_exception(self, mock_course_grade):
             else mock_course_grade.return_value
             for student in self.students
         ]
-        with self.assertNumQueries(11):
+        with self.assertNumQueries(20):
             all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students)
         assert {student: str(all_errors[student]) for student in all_errors} == {
             student3: 'Error for student3.',

lms/djangoapps/grades/tests/test_tasks.py

@@ -156,8 +156,8 @@ def test_block_structure_created_only_once(self):
             assert mock_block_structure_create.call_count == 1
 
     @ddt.data(
-        (ModuleStoreEnum.Type.split, 2, 41, True),
-        (ModuleStoreEnum.Type.split, 2, 41, False),
+        (ModuleStoreEnum.Type.split, 2, 42, True),
+        (ModuleStoreEnum.Type.split, 2, 42, False),
     )
     @ddt.unpack
     def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections):
@@ -168,7 +168,7 @@ def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, creat
                     self._apply_recalculate_subsection_grade()
 
     @ddt.data(
-        (ModuleStoreEnum.Type.split, 2, 41),
+        (ModuleStoreEnum.Type.split, 2, 42),
     )
     @ddt.unpack
     def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls):
@@ -255,7 +255,7 @@ def test_problem_block_with_restricted_access(self, mock_subsection_signal):
         UserPartition.scheme_extensions = None
 
     @ddt.data(
-        (ModuleStoreEnum.Type.split, 2, 41),
+        (ModuleStoreEnum.Type.split, 2, 42),
     )
     @ddt.unpack
     def test_persistent_grades_on_course(self, default_store, num_mongo_queries, num_sql_queries):

lms/djangoapps/grades/tests/test_transformer.py

@@ -13,6 +13,7 @@
 from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
 from xmodule.modulestore.tests.factories import check_mongo_calls_range
 
+import openedx.core.djangoapps.content.block_structure.api as bs_api
 from common.djangoapps.student.tests.factories import UserFactory
 from lms.djangoapps.course_blocks.api import get_course_blocks
 from lms.djangoapps.course_blocks.transformers.tests.helpers import CourseStructureTestCase
@@ -462,6 +463,7 @@ def test_modulestore_performance(self, store_type, max_mongo_calls, min_mongo_ca
             )
         with self.store.default_store(store_type):
             blocks = self.build_course(course)
+        bs_api.update_course_in_cache(blocks['course'].id)
         clear_course_from_cache(blocks['course'].id)
         with check_mongo_calls_range(max_mongo_calls, min_mongo_calls):
             get_course_blocks(self.student, blocks['course'].location, self.transformers)