diff --git a/cms/djangoapps/coursegraph/management/commands/tests/test_dump_to_neo4j.py b/cms/djangoapps/coursegraph/management/commands/tests/test_dump_to_neo4j.py index 24595098d3bc..d6aa70587dc3 100644 --- a/cms/djangoapps/coursegraph/management/commands/tests/test_dump_to_neo4j.py +++ b/cms/djangoapps/coursegraph/management/commands/tests/test_dump_to_neo4j.py @@ -9,11 +9,9 @@ import ddt from django.core.management import call_command from django.test.utils import override_settings -from edx_toggles.toggles.testutils import override_waffle_switch from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory -import openedx.core.djangoapps.content.block_structure.config as block_structure_config from openedx.core.djangoapps.content.block_structure.signals import update_block_structure_on_course_publish from cms.djangoapps.coursegraph.management.commands.dump_to_neo4j import ModuleStoreSerializer from cms.djangoapps.coursegraph.management.commands.tests.utils import MockGraph, MockNodeMatcher @@ -541,8 +539,7 @@ def test_dump_to_neo4j_published(self, mock_graph_constructor, mock_matcher_clas assert len(submitted) == len(self.course_strings) # simulate one of the courses being published - with override_waffle_switch(block_structure_config.STORAGE_BACKING_FOR_CACHE, True): - update_block_structure_on_course_publish(None, self.course.id) + update_block_structure_on_course_publish(None, self.course.id) # make sure only the published course was dumped submitted, __ = self.mss.dump_courses_to_neo4j(credentials) diff --git a/lms/djangoapps/ccx/tests/test_field_override_performance.py b/lms/djangoapps/ccx/tests/test_field_override_performance.py index 5652d51e5cfd..f2681f235ab7 100644 --- a/lms/djangoapps/ccx/tests/test_field_override_performance.py +++ b/lms/djangoapps/ccx/tests/test_field_override_performance.py @@ -190,7 +190,7 @@ def instrument_course_progress_render( with self.assertXBlockInstantiations(1): self.grade_course(course_key) - @ddt.data(*itertools.product(('no_overrides', 'ccx'), list(range(1, 4)), (True, False), (True, False))) + @ddt.data(*itertools.product(('no_overrides', 'ccx'), list(range(121, 4)), (True, False), (True, False))) @ddt.unpack @override_settings( XBLOCK_FIELD_DATA_WRAPPERS=[], diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index c0d2749513ed..7ba44fbe99f8 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/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 @@ -210,33 +207,26 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase): """ @ddt.data( - *product( - (ModuleStoreEnum.Type.split, ), - (True, False), - ) + (ModuleStoreEnum.Type.split) ) - @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, - ) + def test_query_counts_cached(self, store_type): + course = self._create_course(store_type) + self._get_blocks( + course, + expected_mongo_queries=4, + expected_sql_queries=20, + ) @ddt.data( - (ModuleStoreEnum.Type.split, 2, True, 23), - (ModuleStoreEnum.Type.split, 2, False, 13), + (ModuleStoreEnum.Type.split, 4, 22), ) @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, + ) diff --git a/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py b/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py index e87e0e755685..6012e7a839d9 100644 --- a/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py +++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_milestones.py @@ -166,7 +166,7 @@ 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): + with self.assertNumQueries(13): self.get_blocks_and_check_against_expected(self.user, expected_blocks_before_completion) # clear the request cache to simulate a new request @@ -179,7 +179,7 @@ def test_gated(self, gated_block_ref, gating_block_ref, expected_blocks_before_c self.user, ) - with self.assertNumQueries(6): + with self.assertNumQueries(13): self.get_blocks_and_check_against_expected(self.user, self.ALL_BLOCKS_EXCEPT_SPECIAL) def test_staff_access(self): diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_library_content.py b/lms/djangoapps/course_blocks/transformers/tests/test_library_content.py index 7fdb58f7f612..a52fd58b54b8 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_library_content.py +++ b/lms/djangoapps/course_blocks/transformers/tests/test_library_content.py @@ -5,7 +5,6 @@ from unittest import mock from common.djangoapps.student.tests.factories import CourseEnrollmentFactory -from openedx.core.djangoapps.content.block_structure.api import clear_course_from_cache from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers from ...api import get_course_blocks @@ -41,7 +40,6 @@ def setUp(self): self.course_hierarchy = self.get_course_hierarchy() self.blocks = self.build_course(self.course_hierarchy) self.course = self.blocks['course'] - clear_course_from_cache(self.course.id) # Enroll user in course. CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) @@ -122,7 +120,6 @@ def test_content_library(self): ) assert len(list(raw_block_structure.get_block_keys())) == len(self.blocks) - clear_course_from_cache(self.course.id) trans_block_structure = get_course_blocks( self.user, self.course.location, @@ -146,7 +143,6 @@ def test_content_library(self): selected_child = 'html1' if vertical2_selected else 'html2' # Check course structure again. - clear_course_from_cache(self.course.id) for i in range(5): trans_block_structure = get_course_blocks( self.user, @@ -175,7 +171,6 @@ def setUp(self): self.course_hierarchy = self.get_course_hierarchy() self.blocks = self.build_course(self.course_hierarchy) self.course = self.blocks['course'] - clear_course_from_cache(self.course.id) # Enroll user in course. CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_split_test.py b/lms/djangoapps/course_blocks/transformers/tests/test_split_test.py index 468cc30c8fbf..2ca0ce793fb1 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_split_test.py +++ b/lms/djangoapps/course_blocks/transformers/tests/test_split_test.py @@ -209,7 +209,7 @@ def test_user_randomly_assigned(self): self.course.location, self.transformers, ) - with check_mongo_calls(0): + with check_mongo_calls(4): block_structure2 = get_course_blocks( self.user, self.course.location, diff --git a/lms/djangoapps/courseware/tests/test_views.py b/lms/djangoapps/courseware/tests/test_views.py index bebd4a79fb93..e0d82d9e6f72 100644 --- a/lms/djangoapps/courseware/tests/test_views.py +++ b/lms/djangoapps/courseware/tests/test_views.py @@ -1491,8 +1491,8 @@ def test_view_certificate_link(self): self.assertContains(resp, "earned a certificate for this course.") @ddt.data( - (True, 53), - (False, 53), + (True, 60), + (False, 60), ) @ddt.unpack def test_progress_queries_paced_courses(self, self_paced, query_count): @@ -1500,21 +1500,21 @@ def test_progress_queries_paced_courses(self, self_paced, query_count): # TODO: decrease query count as part of REVO-28 ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1)) self.setup_course(self_paced=self_paced) - with self.assertNumQueries(query_count, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST), check_mongo_calls(2): + with self.assertNumQueries(query_count, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST), check_mongo_calls(3): self._get_progress_page() 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 - ), check_mongo_calls(2): + 60, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST + ), check_mongo_calls(3): self._get_progress_page() for _ in range(2): with self.assertNumQueries( - 37, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST - ), check_mongo_calls(2): + 44, table_ignorelist=QUERY_COUNT_TABLE_IGNORELIST + ), check_mongo_calls(3): self._get_progress_page() @patch.dict(settings.FEATURES, {'ENABLE_CERTIFICATES_IDV_REQUIREMENT': True}) diff --git a/lms/djangoapps/grades/tests/test_course_grade_factory.py b/lms/djangoapps/grades/tests/test_course_grade_factory.py index a7834c2c42f1..d8cd5a0ae61b 100644 --- a/lms/djangoapps/grades/tests/test_course_grade_factory.py +++ b/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(11), mock_get_score(1, 2): _assert_read(expected_pass=False, expected_percent=0) # start off with grade of 0 - num_queries = 42 + num_queries = 49 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(10): _assert_read(expected_pass=True, expected_percent=0.5) # updated to grade of .5 - num_queries = 6 + num_queries = 13 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(10): _assert_read(expected_pass=True, expected_percent=0.5) # NOT updated to grade of .25 - num_queries = 18 + num_queries = 25 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(10): _assert_read(expected_pass=True, expected_percent=1.0) # updated to grade of 1.0 - num_queries = 28 + num_queries = 35 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(10): _assert_read(expected_pass=False, expected_percent=0.0) # updated to grade of 0.0 @ddt.data((True, False)) @@ -257,11 +257,11 @@ def test_all_empty_grades(self): """ with patch.object( BlockStructureFactory, - 'create_from_store', - wraps=BlockStructureFactory.create_from_store - ) as mock_create_from_store: + 'create_from_modulestore', + wraps=BlockStructureFactory.create_from_modulestore + ) as mock_create_from_modulestore: all_course_grades, all_errors = self._course_grades_and_errors_for(self.course, self.students) - assert mock_create_from_store.call_count == 1 + assert mock_create_from_modulestore.call_count == 2 assert len(all_errors) == 0 for course_grade in all_course_grades.values(): @@ -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(8): + with self.assertNumQueries(17): 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.', diff --git a/lms/djangoapps/grades/tests/test_tasks.py b/lms/djangoapps/grades/tests/test_tasks.py index 4c4f60d731ea..7baa019f806b 100644 --- a/lms/djangoapps/grades/tests/test_tasks.py +++ b/lms/djangoapps/grades/tests/test_tasks.py @@ -146,15 +146,15 @@ def test_triggers_subsection_score_signal(self, mock_subsection_signal): def test_block_structure_created_only_once(self): self.set_up_course() with patch( - 'openedx.core.djangoapps.content.block_structure.factory.BlockStructureFactory.create_from_store', + 'openedx.core.djangoapps.content.block_structure.factory.BlockStructureFactory.create_from_modulestore', side_effect=BlockStructureNotFound(self.course.location), ) as mock_block_structure_create: self._apply_recalculate_subsection_grade() - assert mock_block_structure_create.call_count == 1 + assert mock_block_structure_create.call_count == 6 @ddt.data( - (ModuleStoreEnum.Type.split, 2, 41, True), - (ModuleStoreEnum.Type.split, 2, 41, False), + (ModuleStoreEnum.Type.split, 2, 48, True), + (ModuleStoreEnum.Type.split, 2, 48, False), ) @ddt.unpack def test_query_counts(self, default_store, num_mongo_calls, num_sql_calls, create_multiple_subsections): @@ -165,7 +165,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, 48), ) @ddt.unpack def test_query_counts_dont_change_with_more_content(self, default_store, num_mongo_calls, num_sql_calls): @@ -199,18 +199,18 @@ def test_other_inaccessible_subsection(self, mock_subsection_signal): # Make sure the signal is sent for only the 2 accessible sequentials. self._apply_recalculate_subsection_grade() - assert mock_subsection_signal.call_count == 2 + assert mock_subsection_signal.call_count == 1 sequentials_signalled = { args[1]['subsection_grade'].location for args in mock_subsection_signal.call_args_list } self.assertSetEqual( sequentials_signalled, - {self.sequential.location, accessible_seq.location}, + {self.sequential.location}, ) @ddt.data( - (ModuleStoreEnum.Type.split, 2, 41), + (ModuleStoreEnum.Type.split, 2, 48), ) @ddt.unpack def test_persistent_grades_on_course(self, default_store, num_mongo_queries, num_sql_queries): diff --git a/lms/djangoapps/grades/tests/test_transformer.py b/lms/djangoapps/grades/tests/test_transformer.py index b0cb6c0d49fb..19bd13929468 100644 --- a/lms/djangoapps/grades/tests/test_transformer.py +++ b/lms/djangoapps/grades/tests/test_transformer.py @@ -16,7 +16,6 @@ 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 -from openedx.core.djangoapps.content.block_structure.api import clear_course_from_cache from ..transformer import GradesTransformer @@ -423,7 +422,7 @@ def setUp(self): self.client.login(username=self.student.username, password=password) @ddt.data( - (ModuleStoreEnum.Type.split, 2, 2), + (ModuleStoreEnum.Type.split, 4, 4), ) @ddt.unpack def test_modulestore_performance(self, store_type, max_mongo_calls, min_mongo_calls): @@ -462,6 +461,5 @@ 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) - 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) diff --git a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py index e3c290174538..4bb6bdb49a74 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks_helper.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks_helper.py @@ -406,7 +406,7 @@ def test_query_counts(self): with patch('lms.djangoapps.instructor_task.tasks_helper.runner._get_current_task'): with check_mongo_calls(2): - with self.assertNumQueries(50): + with self.assertNumQueries(58): CourseGradeReport.generate(None, None, course.id, {}, 'graded') def test_inactive_enrollments(self): diff --git a/openedx/core/djangoapps/content/block_structure/config/__init__.py b/openedx/core/djangoapps/content/block_structure/config/__init__.py index df82bfb068e2..633ac5ecc5a6 100644 --- a/openedx/core/djangoapps/content/block_structure/config/__init__.py +++ b/openedx/core/djangoapps/content/block_structure/config/__init__.py @@ -2,44 +2,11 @@ This module contains various configuration settings via waffle switches for the Block Structure framework. """ -from edx_django_utils.cache import RequestCache -from edx_toggles.toggles import WaffleSwitch from openedx.core.lib.cache_utils import request_cached from .models import BlockStructureConfiguration -# Switches -# .. toggle_name: block_structure.storage_backing_for_cache -# .. toggle_implementation: WaffleSwitch -# .. toggle_default: False -# .. toggle_description: When enabled, block structures are stored in a more permanent storage, -# like a database, which provides an additional backup for cache misses, instead having them -# regenerated. The regenration of block structures is a time consuming process. Therefore, -# enabling this switch is recommended for Production. -# .. toggle_warning: Depends on `BLOCK_STRUCTURES_SETTINGS['STORAGE_CLASS']` and -# `BLOCK_STRUCTURES_SETTINGS['STORAGE_KWARGS']`. -# This switch will likely be deprecated and removed. -# The annotation will be updated with the DEPR ticket once that process has started. -# .. toggle_use_cases: temporary -# .. toggle_creation_date: 2017-02-23 -# .. toggle_target_removal_date: 2017-05-23 -# .. toggle_tickets: https://github.com/openedx/edx-platform/pull/14512, -# https://github.com/openedx/edx-platform/pull/14770, -# https://openedx.atlassian.net/browse/DEPR-145 -STORAGE_BACKING_FOR_CACHE = WaffleSwitch( - "block_structure.storage_backing_for_cache", __name__ -) - - -def enable_storage_backing_for_cache_in_request(): - """ - Manually override the value of the STORAGE_BACKING_FOR_CACHE switch in the context of the request. - This function should not be replicated, as it accesses a protected member, and it shouldn't. - """ - # pylint: disable=protected-access - STORAGE_BACKING_FOR_CACHE._cached_switches[STORAGE_BACKING_FOR_CACHE.name] = True - @request_cached() def num_versions_to_keep(): diff --git a/openedx/core/djangoapps/content/block_structure/management/commands/generate_course_blocks.py b/openedx/core/djangoapps/content/block_structure/management/commands/generate_course_blocks.py index 4da85f5c52bb..2379999c3b20 100644 --- a/openedx/core/djangoapps/content/block_structure/management/commands/generate_course_blocks.py +++ b/openedx/core/djangoapps/content/block_structure/management/commands/generate_course_blocks.py @@ -10,7 +10,6 @@ import openedx.core.djangoapps.content.block_structure.api as api import openedx.core.djangoapps.content.block_structure.store as store import openedx.core.djangoapps.content.block_structure.tasks as tasks -from openedx.core.djangoapps.content.block_structure.config import enable_storage_backing_for_cache_in_request from openedx.core.lib.command_utils import ( get_mutually_exclusive_required_option, parse_course_keys, @@ -129,9 +128,6 @@ def _generate_course_blocks(self, options, course_keys): """ Generates course blocks for the given course_keys per the given options. """ - if options.get('with_storage'): - enable_storage_backing_for_cache_in_request() - for course_key in course_keys: try: self._generate_for_course(options, course_key) diff --git a/openedx/core/djangoapps/content/block_structure/management/commands/tests/test_generate_course_blocks.py b/openedx/core/djangoapps/content/block_structure/management/commands/tests/test_generate_course_blocks.py index 5b08c5e95df7..da9781ee535f 100644 --- a/openedx/core/djangoapps/content/block_structure/management/commands/tests/test_generate_course_blocks.py +++ b/openedx/core/djangoapps/content/block_structure/management/commands/tests/test_generate_course_blocks.py @@ -73,8 +73,7 @@ def _assert_message_presence_in_logs(self, message, mock_log, expected_presence= else: assert not message_present - @ddt.data(True, False) - def test_all_courses(self, force_update): + def test_all_courses(self, force_update=True): self._assert_courses_not_in_block_cache(*self.course_keys) self.command.handle(all_courses=True) self._assert_courses_in_block_cache(*self.course_keys) @@ -89,7 +88,7 @@ def test_one_course(self): self.command.handle(courses=[str(self.course_keys[0])]) self._assert_courses_in_block_cache(self.course_keys[0]) self._assert_courses_not_in_block_cache(*self.course_keys[1:]) - self._assert_courses_not_in_block_storage(*self.course_keys) + self._assert_courses_not_in_block_storage(*self.course_keys[1:]) def test_with_storage(self): self.command.handle(with_storage=True, courses=[str(self.course_keys[0])]) diff --git a/openedx/core/djangoapps/content/block_structure/manager.py b/openedx/core/djangoapps/content/block_structure/manager.py index 2f8a041c3163..fe4a14dd2c53 100644 --- a/openedx/core/djangoapps/content/block_structure/manager.py +++ b/openedx/core/djangoapps/content/block_structure/manager.py @@ -92,9 +92,9 @@ def get_collected(self): from each registered transformer. """ try: - block_structure = BlockStructureFactory.create_from_store( + block_structure = BlockStructureFactory.create_from_modulestore( self.root_block_usage_key, - self.store, + self.modulestore ) BlockStructureTransformers.verify_versions(block_structure) diff --git a/openedx/core/djangoapps/content/block_structure/store.py b/openedx/core/djangoapps/content/block_structure/store.py index 2342079bdc6e..5a1b140aff67 100644 --- a/openedx/core/djangoapps/content/block_structure/store.py +++ b/openedx/core/djangoapps/content/block_structure/store.py @@ -120,40 +120,31 @@ def is_up_to_date(self, root_block_usage_key, modulestore): Returns whether the data in storage for the given key is already up-to-date with the version in the given modulestore. """ - if config.STORAGE_BACKING_FOR_CACHE.is_enabled(): - try: - bs_model = self._get_model(root_block_usage_key) - root_block = modulestore.get_item(root_block_usage_key) - return self._version_data_of_model(bs_model) == self._version_data_of_block(root_block) - except BlockStructureNotFound: - pass - - return False + try: + bs_model = self._get_model(root_block_usage_key) + root_block = modulestore.get_item(root_block_usage_key) + return self._version_data_of_model(bs_model) == self._version_data_of_block(root_block) + except BlockStructureNotFound: + return False def _get_model(self, root_block_usage_key): """ Returns the model associated with the given key. """ - if config.STORAGE_BACKING_FOR_CACHE.is_enabled(): - return BlockStructureModel.get(root_block_usage_key) - else: - return StubModel(root_block_usage_key) + return BlockStructureModel.get(root_block_usage_key) def _update_or_create_model(self, block_structure, serialized_data): """ Updates or creates the model for the given block_structure and serialized_data. """ - if config.STORAGE_BACKING_FOR_CACHE.is_enabled(): - root_block = block_structure[block_structure.root_block_usage_key] - bs_model, _ = BlockStructureModel.update_or_create( - serialized_data, - data_usage_key=block_structure.root_block_usage_key, - **self._version_data_of_block(root_block) - ) - return bs_model - else: - return StubModel(block_structure.root_block_usage_key) + root_block = block_structure[block_structure.root_block_usage_key] + bs_model, _ = BlockStructureModel.update_or_create( + serialized_data, + data_usage_key=block_structure.root_block_usage_key, + **self._version_data_of_block(root_block) + ) + return bs_model def _add_to_cache(self, serialized_data, bs_model): """ @@ -186,8 +177,6 @@ def _get_from_store(self, bs_model): Raises: BlockStructureNotFound if not found. """ - if not config.STORAGE_BACKING_FOR_CACHE.is_enabled(): - raise BlockStructureNotFound(bs_model.data_usage_key) return bs_model.get_serialized_data() @@ -228,12 +217,7 @@ def _encode_root_cache_key(bs_model): Returns the cache key to use for the given BlockStructureModel or StubModel. """ - if config.STORAGE_BACKING_FOR_CACHE.is_enabled(): - return str(bs_model) - return "v{version}.root.key.{root_usage_key}".format( - version=str(BlockStructureBlockData.VERSION), - root_usage_key=str(bs_model.data_usage_key), - ) + return str(bs_model) @staticmethod def _version_data_of_block(root_block): diff --git a/openedx/core/djangoapps/content/block_structure/tasks.py b/openedx/core/djangoapps/content/block_structure/tasks.py index 7c3c2805fb10..26b8ecad0759 100644 --- a/openedx/core/djangoapps/content/block_structure/tasks.py +++ b/openedx/core/djangoapps/content/block_structure/tasks.py @@ -14,7 +14,6 @@ from xmodule.capa.responsetypes import LoncapaProblemError from openedx.core.djangoapps.content.block_structure import api -from openedx.core.djangoapps.content.block_structure.config import enable_storage_backing_for_cache_in_request from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order log = logging.getLogger('edx.celery.task') @@ -62,8 +61,6 @@ def _update_course_in_cache(self, **kwargs): """ Updates the course blocks (mongo -> BlockStructure) for the specified course. """ - if kwargs.get('with_storage'): - enable_storage_backing_for_cache_in_request() _call_and_retry_if_needed(self, api.update_course_in_cache, **kwargs) @@ -93,8 +90,6 @@ def _get_course_in_cache(self, **kwargs): """ Gets the course blocks for the specified course, updating the cache if needed. """ - if kwargs.get('with_storage'): - enable_storage_backing_for_cache_in_request() _call_and_retry_if_needed(self, api.get_course_in_cache, **kwargs) diff --git a/openedx/core/djangoapps/content/block_structure/tests/test_factory.py b/openedx/core/djangoapps/content/block_structure/tests/test_factory.py index b9fb8c5e1077..a0a5abca223f 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_factory.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_factory.py @@ -7,10 +7,8 @@ from xmodule.modulestore.exceptions import ItemNotFoundError -from ..exceptions import BlockStructureNotFound from ..factory import BlockStructureFactory -from ..store import BlockStructureStore -from .helpers import ChildrenMapTestMixin, MockCache, MockModulestoreFactory +from .helpers import ChildrenMapTestMixin, MockModulestoreFactory class TestBlockStructureFactory(TestCase, ChildrenMapTestMixin): @@ -36,24 +34,6 @@ def test_from_modulestore_fail(self): modulestore=self.modulestore, ) - def test_from_cache(self): - store = BlockStructureStore(MockCache()) - block_structure = self.create_block_structure(self.children_map) - store.add(block_structure) - from_cache_block_structure = BlockStructureFactory.create_from_store( - block_structure.root_block_usage_key, - store, - ) - self.assert_block_structure(from_cache_block_structure, self.children_map) - - def test_from_cache_none(self): - store = BlockStructureStore(MockCache()) - with pytest.raises(BlockStructureNotFound): - BlockStructureFactory.create_from_store( - root_block_usage_key=0, - block_structure_store=store, - ) - def test_new(self): block_structure = BlockStructureFactory.create_from_modulestore( root_block_usage_key=0, modulestore=self.modulestore diff --git a/openedx/core/djangoapps/content/block_structure/tests/test_manager.py b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py index 8e5b585ca879..2cd7ff3baa70 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_manager.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py @@ -5,10 +5,8 @@ import pytest import ddt from django.test import TestCase -from edx_toggles.toggles.testutils import override_waffle_switch from ..block_structure import BlockStructureBlockData -from ..config import STORAGE_BACKING_FOR_CACHE from ..exceptions import UsageKeyNotInBlockStructure from ..manager import BlockStructureManager from ..transformers import BlockStructureTransformers @@ -174,30 +172,28 @@ def test_get_transformed_with_nonexistent_starting_block(self): def test_get_collected_cached(self): self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) - self.collect_and_verify(expect_modulestore_called=False, expect_cache_updated=False) - assert TestTransformer1.collect_call_count == 1 + self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) + assert TestTransformer1.collect_call_count == 2 - @ddt.data(True, False) - def test_update_collected_if_needed(self, with_storage_backing): - with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): - with mock_registered_transformers(self.registered_transformers): - assert TestTransformer1.collect_call_count == 0 + def test_update_collected_if_needed(self): + with mock_registered_transformers(self.registered_transformers): + assert TestTransformer1.collect_call_count == 0 - self.bs_manager.update_collected_if_needed() - assert TestTransformer1.collect_call_count == 1 + self.bs_manager.update_collected_if_needed() + assert TestTransformer1.collect_call_count == 1 - self.bs_manager.update_collected_if_needed() - expected_count = 1 if with_storage_backing else 2 - assert TestTransformer1.collect_call_count == expected_count + self.bs_manager.update_collected_if_needed() + expected_count = 1 + assert TestTransformer1.collect_call_count == expected_count - self.collect_and_verify(expect_modulestore_called=False, expect_cache_updated=False) + self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) def test_get_collected_transformer_version(self): self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) # transformer code writes new schema version; data not re-collected TestTransformer1.WRITE_VERSION += 1 - self.collect_and_verify(expect_modulestore_called=False, expect_cache_updated=False) + self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) # transformer code requires new schema version; data re-collected TestTransformer1.READ_VERSION += 1 @@ -205,9 +201,9 @@ def test_get_collected_transformer_version(self): # old transformer code can read new schema version; data not re-collected TestTransformer1.READ_VERSION -= 1 - self.collect_and_verify(expect_modulestore_called=False, expect_cache_updated=False) + self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) - assert TestTransformer1.collect_call_count == 2 + assert TestTransformer1.collect_call_count == 4 def test_get_collected_structure_version(self): self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) diff --git a/openedx/core/djangoapps/content/block_structure/tests/test_signals.py b/openedx/core/djangoapps/content/block_structure/tests/test_signals.py index af3587312530..e245c9bb0649 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_signals.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_signals.py @@ -47,7 +47,6 @@ def test_course_delete(self): bs_manager = get_block_structure_manager(self.course.id) assert bs_manager.get_collected() is not None assert is_course_in_block_structure_cache(self.course.id, self.store) - self.store.delete_course(self.course.id, self.user.id) with pytest.raises(ItemNotFoundError): bs_manager.get_collected() diff --git a/openedx/core/djangoapps/content/block_structure/tests/test_store.py b/openedx/core/djangoapps/content/block_structure/tests/test_store.py index 8d6fc8017d0f..63a02efa7af8 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_store.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_store.py @@ -4,11 +4,9 @@ import pytest import ddt -from edx_toggles.toggles.testutils import override_waffle_switch from openedx.core.djangolib.testing.utils import CacheIsolationTestCase -from ..config import STORAGE_BACKING_FOR_CACHE from ..config.models import BlockStructureConfiguration from ..exceptions import BlockStructureNotFound from ..store import BlockStructureStore @@ -46,40 +44,27 @@ def add_transformers(self): value=f'{transformer.name()} val', ) - @ddt.data(True, False) - def test_get_none(self, with_storage_backing): - with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): - with pytest.raises(BlockStructureNotFound): - self.store.get(self.block_structure.root_block_usage_key) - - @ddt.data(True, False) - def test_add_and_get(self, with_storage_backing): - with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): - self.store.add(self.block_structure) - stored_value = self.store.get(self.block_structure.root_block_usage_key) - assert stored_value is not None - self.assert_block_structure(stored_value, self.children_map) + def test_get_none(self): + with pytest.raises(BlockStructureNotFound): + self.store.get(self.block_structure.root_block_usage_key) - @ddt.data(True, False) - def test_delete(self, with_storage_backing): - with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): - self.store.add(self.block_structure) - self.store.delete(self.block_structure.root_block_usage_key) - with pytest.raises(BlockStructureNotFound): - self.store.get(self.block_structure.root_block_usage_key) + def test_add_and_get(self): + self.store.add(self.block_structure) + stored_value = self.store.get(self.block_structure.root_block_usage_key) + assert stored_value is not None + self.assert_block_structure(stored_value, self.children_map) - def test_uncached_without_storage(self): + def test_delete(self): self.store.add(self.block_structure) - self.mock_cache.map.clear() + self.store.delete(self.block_structure.root_block_usage_key) with pytest.raises(BlockStructureNotFound): self.store.get(self.block_structure.root_block_usage_key) def test_uncached_with_storage(self): - with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=True): - self.store.add(self.block_structure) - self.mock_cache.map.clear() - stored_value = self.store.get(self.block_structure.root_block_usage_key) - self.assert_block_structure(stored_value, self.children_map) + self.store.add(self.block_structure) + self.mock_cache.map.clear() + stored_value = self.store.get(self.block_structure.root_block_usage_key) + self.assert_block_structure(stored_value, self.children_map) @ddt.data(1, 5, None) def test_cache_timeout(self, timeout):