diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index f738fd09bf..65f7097cad 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -17,7 +17,7 @@ from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import SampleCourseFactory, check_mongo_calls from xmodule.modulestore.tests.sample_courses import BlockInfo 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, waffle_switch +from openedx.core.djangoapps.content.block_structure.config import STORAGE_BACKING_FOR_CACHE from ..api import get_blocks @@ -230,7 +230,7 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase): ) @ddt.unpack def test_query_counts_cached(self, store_type, with_storage_backing): - with override_waffle_switch(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=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, @@ -247,7 +247,7 @@ class TestGetBlocksQueryCounts(TestGetBlocksQueryCountsBase): @ddt.unpack def test_query_counts_uncached(self, store_type_tuple, with_storage_backing): store_type, expected_mongo_queries = store_type_tuple - with override_waffle_switch(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=with_storage_backing): + with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): course = self._create_course(store_type) clear_course_from_cache(course.id) diff --git a/openedx/core/djangoapps/content/block_structure/config/__init__.py b/openedx/core/djangoapps/content/block_structure/config/__init__.py index 8657512d25..754c2b17be 100644 --- a/openedx/core/djangoapps/content/block_structure/config/__init__.py +++ b/openedx/core/djangoapps/content/block_structure/config/__init__.py @@ -2,37 +2,32 @@ 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.__future__ import WaffleSwitch - -from edx_toggles.toggles import LegacyWaffleSwitch, LegacyWaffleSwitchNamespace from openedx.core.lib.cache_utils import request_cached from .models import BlockStructureConfiguration -# Namespace -WAFFLE_NAMESPACE = u'block_structure' - # Switches -INVALIDATE_CACHE_ON_PUBLISH = u'invalidate_cache_on_publish' -STORAGE_BACKING_FOR_CACHE = u'storage_backing_for_cache' -RAISE_ERROR_WHEN_NOT_FOUND = u'raise_error_when_not_found' +INVALIDATE_CACHE_ON_PUBLISH = WaffleSwitch( + "block_structure.invalidate_cache_on_publish", __name__ +) +STORAGE_BACKING_FOR_CACHE = WaffleSwitch( + "block_structure.storage_backing_for_cache", __name__ +) +RAISE_ERROR_WHEN_NOT_FOUND = WaffleSwitch( + "block_structure.raise_error_when_not_found", __name__ +) -def waffle(): +def enable_storage_backing_for_cache_in_request(): """ - Returns the namespaced and cached Waffle class for BlockStructures. + 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. """ - return LegacyWaffleSwitchNamespace(name=WAFFLE_NAMESPACE, log_prefix=u'BlockStructure: ') - - -def waffle_switch(name): - """ - Return the waffle switch associated to this namespace. - - WARNING: do not replicate this pattern. Instead of declaring waffle switch names as strings, you should create - LegacyWaffleSwitch objects as top-level constants. - """ - return LegacyWaffleSwitch(waffle(), name, module_name=__name__) + # pylint: disable=protected-access + STORAGE_BACKING_FOR_CACHE._cached_switches[STORAGE_BACKING_FOR_CACHE.name] = True @request_cached() 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 1ee189a528..45f50d72cf 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 @@ -7,13 +7,12 @@ import logging import six from django.core.management.base import BaseCommand -import six from six import text_type 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 STORAGE_BACKING_FOR_CACHE, waffle +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, @@ -133,7 +132,7 @@ class Command(BaseCommand): Generates course blocks for the given course_keys per the given options. """ if options.get('with_storage'): - waffle().set_request_cache_with_short_name(STORAGE_BACKING_FOR_CACHE, True) + enable_storage_backing_for_cache_in_request() for course_key in course_keys: try: diff --git a/openedx/core/djangoapps/content/block_structure/manager.py b/openedx/core/djangoapps/content/block_structure/manager.py index 1623d208c2..63aa2fe3a6 100644 --- a/openedx/core/djangoapps/content/block_structure/manager.py +++ b/openedx/core/djangoapps/content/block_structure/manager.py @@ -102,10 +102,9 @@ class BlockStructureManager(object): BlockStructureTransformers.verify_versions(block_structure) except (BlockStructureNotFound, TransformerDataIncompatible): - if config.waffle().is_enabled(config.RAISE_ERROR_WHEN_NOT_FOUND): + if config.RAISE_ERROR_WHEN_NOT_FOUND.is_enabled(): raise - else: - block_structure = self._update_collected() + block_structure = self._update_collected() return block_structure diff --git a/openedx/core/djangoapps/content/block_structure/signals.py b/openedx/core/djangoapps/content/block_structure/signals.py index 43fe00113a..1e01fc7a13 100644 --- a/openedx/core/djangoapps/content/block_structure/signals.py +++ b/openedx/core/djangoapps/content/block_structure/signals.py @@ -29,7 +29,7 @@ def update_block_structure_on_course_publish(sender, course_key, **kwargs): # p if isinstance(course_key, LibraryLocator): return - if config.waffle().is_enabled(config.INVALIDATE_CACHE_ON_PUBLISH): + if config.INVALIDATE_CACHE_ON_PUBLISH.is_enabled(): try: clear_course_from_cache(course_key) except BlockStructureNotFound: diff --git a/openedx/core/djangoapps/content/block_structure/store.py b/openedx/core/djangoapps/content/block_structure/store.py index 28e76dc4a0..df1a23144b 100644 --- a/openedx/core/djangoapps/content/block_structure/store.py +++ b/openedx/core/djangoapps/content/block_structure/store.py @@ -7,8 +7,8 @@ Module for the Storage of BlockStructure objects. from logging import getLogger import six - from django.utils.encoding import python_2_unicode_compatible + from openedx.core.lib.cache_utils import zpickle, zunpickle from . import config @@ -28,6 +28,7 @@ class StubModel(object): By using this stub, we eliminate the need for extra conditional statements in the code. """ + def __init__(self, root_block_usage_key): self.data_usage_key = root_block_usage_key @@ -45,6 +46,7 @@ class BlockStructureStore(object): """ Storage for BlockStructure objects. """ + def __init__(self, cache): """ Arguments: @@ -121,7 +123,7 @@ class BlockStructureStore(object): Returns whether the data in storage for the given key is already up-to-date with the version in the given modulestore. """ - if _is_storage_backing_enabled(): + 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) @@ -135,7 +137,7 @@ class BlockStructureStore(object): """ Returns the model associated with the given key. """ - if _is_storage_backing_enabled(): + if config.STORAGE_BACKING_FOR_CACHE.is_enabled(): return BlockStructureModel.get(root_block_usage_key) else: return StubModel(root_block_usage_key) @@ -145,7 +147,7 @@ class BlockStructureStore(object): Updates or creates the model for the given block_structure and serialized_data. """ - if _is_storage_backing_enabled(): + 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, @@ -187,7 +189,7 @@ class BlockStructureStore(object): Raises: BlockStructureNotFound if not found. """ - if not _is_storage_backing_enabled(): + if not config.STORAGE_BACKING_FOR_CACHE.is_enabled(): raise BlockStructureNotFound(bs_model.data_usage_key) return bs_model.get_serialized_data() @@ -229,14 +231,12 @@ class BlockStructureStore(object): Returns the cache key to use for the given BlockStructureModel or StubModel. """ - if _is_storage_backing_enabled(): + if config.STORAGE_BACKING_FOR_CACHE.is_enabled(): return six.text_type(bs_model) - - else: - return "v{version}.root.key.{root_usage_key}".format( - version=six.text_type(BlockStructureBlockData.VERSION), - root_usage_key=six.text_type(bs_model.data_usage_key), - ) + return "v{version}.root.key.{root_usage_key}".format( + version=six.text_type(BlockStructureBlockData.VERSION), + root_usage_key=six.text_type(bs_model.data_usage_key), + ) @staticmethod def _version_data_of_block(root_block): @@ -260,10 +260,3 @@ class BlockStructureStore(object): field_name: getattr(bs_model, field_name, None) for field_name in BlockStructureModel.VERSION_FIELDS } - - -def _is_storage_backing_enabled(): - """ - Returns whether storage backing for Block Structures is enabled. - """ - return config.waffle().is_enabled(config.STORAGE_BACKING_FOR_CACHE) diff --git a/openedx/core/djangoapps/content/block_structure/tasks.py b/openedx/core/djangoapps/content/block_structure/tasks.py index 50f8519a2a..970f48e4cd 100644 --- a/openedx/core/djangoapps/content/block_structure/tasks.py +++ b/openedx/core/djangoapps/content/block_structure/tasks.py @@ -14,7 +14,7 @@ from opaque_keys.edx.keys import CourseKey from capa.responsetypes import LoncapaProblemError from openedx.core.djangoapps.content.block_structure import api -from openedx.core.djangoapps.content.block_structure.config import STORAGE_BACKING_FOR_CACHE, waffle +from openedx.core.djangoapps.content.block_structure.config import enable_storage_backing_for_cache_in_request from xmodule.modulestore.exceptions import ItemNotFoundError log = logging.getLogger('edx.celery.task') @@ -63,7 +63,7 @@ def _update_course_in_cache(self, **kwargs): Updates the course blocks (mongo -> BlockStructure) for the specified course. """ if kwargs.get('with_storage'): - waffle().set_request_cache_with_short_name(STORAGE_BACKING_FOR_CACHE, True) + enable_storage_backing_for_cache_in_request() _call_and_retry_if_needed(self, api.update_course_in_cache, **kwargs) @@ -94,7 +94,7 @@ 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'): - waffle().set_request_cache_with_short_name(STORAGE_BACKING_FOR_CACHE, True) + 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_manager.py b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py index cfdf68fb1a..1c0d921426 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_manager.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_manager.py @@ -9,7 +9,7 @@ from django.test import TestCase from edx_toggles.toggles.testutils import override_waffle_switch from ..block_structure import BlockStructureBlockData -from ..config import RAISE_ERROR_WHEN_NOT_FOUND, STORAGE_BACKING_FOR_CACHE, waffle_switch +from ..config import RAISE_ERROR_WHEN_NOT_FOUND, STORAGE_BACKING_FOR_CACHE from ..exceptions import BlockStructureNotFound, UsageKeyNotInBlockStructure from ..manager import BlockStructureManager from ..transformers import BlockStructureTransformers @@ -179,14 +179,14 @@ class TestBlockStructureManager(UsageKeyFactoryMixin, ChildrenMapTestMixin, Test assert TestTransformer1.collect_call_count == 1 def test_get_collected_error_raised(self): - with override_waffle_switch(waffle_switch(RAISE_ERROR_WHEN_NOT_FOUND), active=True): + with override_waffle_switch(RAISE_ERROR_WHEN_NOT_FOUND, active=True): with mock_registered_transformers(self.registered_transformers): with self.assertRaises(BlockStructureNotFound): self.bs_manager.get_collected() @ddt.data(True, False) def test_update_collected_if_needed(self, with_storage_backing): - with override_waffle_switch(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=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 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 ca47fb2eeb..99e3db6a61 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_signals.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_signals.py @@ -12,7 +12,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory from ..api import get_block_structure_manager -from ..config import INVALIDATE_CACHE_ON_PUBLISH, waffle_switch +from ..config import INVALIDATE_CACHE_ON_PUBLISH from ..signals import update_block_structure_on_course_publish from .helpers import is_course_in_block_structure_cache @@ -56,7 +56,7 @@ class CourseBlocksSignalTest(ModuleStoreTestCase): def test_cache_invalidation(self, invalidate_cache_enabled, mock_bs_manager_clear): test_display_name = "Jedi 101" - with override_waffle_switch(waffle_switch(INVALIDATE_CACHE_ON_PUBLISH), active=invalidate_cache_enabled): + with override_waffle_switch(INVALIDATE_CACHE_ON_PUBLISH, active=invalidate_cache_enabled): self.course.display_name = test_display_name self.store.update_item(self.course, self.user.id) 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 85772578db..ef35a70139 100644 --- a/openedx/core/djangoapps/content/block_structure/tests/test_store.py +++ b/openedx/core/djangoapps/content/block_structure/tests/test_store.py @@ -8,7 +8,7 @@ from edx_toggles.toggles.testutils import override_waffle_switch from openedx.core.djangolib.testing.utils import CacheIsolationTestCase -from ..config import STORAGE_BACKING_FOR_CACHE, waffle_switch +from ..config import STORAGE_BACKING_FOR_CACHE from ..config.models import BlockStructureConfiguration from ..exceptions import BlockStructureNotFound from ..store import BlockStructureStore @@ -48,13 +48,13 @@ class TestBlockStructureStore(UsageKeyFactoryMixin, ChildrenMapTestMixin, CacheI @ddt.data(True, False) def test_get_none(self, with_storage_backing): - with override_waffle_switch(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=with_storage_backing): + with override_waffle_switch(STORAGE_BACKING_FOR_CACHE, active=with_storage_backing): with self.assertRaises(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(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=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) self.assertIsNotNone(stored_value) @@ -62,7 +62,7 @@ class TestBlockStructureStore(UsageKeyFactoryMixin, ChildrenMapTestMixin, CacheI @ddt.data(True, False) def test_delete(self, with_storage_backing): - with override_waffle_switch(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=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 self.assertRaises(BlockStructureNotFound): @@ -75,7 +75,7 @@ class TestBlockStructureStore(UsageKeyFactoryMixin, ChildrenMapTestMixin, CacheI self.store.get(self.block_structure.root_block_usage_key) def test_uncached_with_storage(self): - with override_waffle_switch(waffle_switch(STORAGE_BACKING_FOR_CACHE), active=True): + 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) diff --git a/openedx/core/djangoapps/coursegraph/management/commands/tests/test_dump_to_neo4j.py b/openedx/core/djangoapps/coursegraph/management/commands/tests/test_dump_to_neo4j.py index 7431a90392..febacf0819 100644 --- a/openedx/core/djangoapps/coursegraph/management/commands/tests/test_dump_to_neo4j.py +++ b/openedx/core/djangoapps/coursegraph/management/commands/tests/test_dump_to_neo4j.py @@ -500,9 +500,7 @@ class TestModuleStoreSerializer(TestDumpToNeo4jCommandBase): self.assertEqual(len(submitted), len(self.course_strings)) # simulate one of the courses being published - with override_waffle_switch( - block_structure_config.waffle_switch(block_structure_config.STORAGE_BACKING_FOR_CACHE), True - ): + with override_waffle_switch(block_structure_config.STORAGE_BACKING_FOR_CACHE, True): update_block_structure_on_course_publish(None, self.course.id) # make sure only the published course was dumped