From 66397c35b9ab04667c088bf2898f1b59551fbfc5 Mon Sep 17 00:00:00 2001 From: Nimisha Asthagiri Date: Fri, 15 Jan 2016 12:41:48 -0500 Subject: [PATCH] Refactor Block Cache to separate Collect and Transform phases --- lms/djangoapps/course_api/blocks/api.py | 34 ++- .../course_api/blocks/tests/test_api.py | 4 +- .../course_api/blocks/tests/test_forms.py | 3 +- .../blocks/tests/test_serializers.py | 10 +- .../course_api/blocks/tests/test_views.py | 5 +- .../blocks/transformers/block_counts.py | 2 +- .../blocks/transformers/block_depth.py | 2 +- .../blocks/transformers/blocks_api.py | 2 +- .../blocks/transformers/navigation.py | 2 +- .../blocks/transformers/proctored_exam.py | 2 +- .../blocks/transformers/student_view.py | 2 +- .../transformers/tests/test_block_counts.py | 2 +- .../transformers/tests/test_block_depth.py | 6 +- .../transformers/tests/test_navigation.py | 8 +- .../transformers/tests/test_proctored_exam.py | 10 +- .../transformers/tests/test_student_view.py | 2 +- lms/djangoapps/course_blocks/api.py | 71 +++++-- .../course_blocks/tests/test_utils.py | 33 +++ .../transformers/library_content.py | 2 +- .../course_blocks/transformers/split_test.py | 2 +- .../course_blocks/transformers/start_date.py | 2 +- .../transformers/tests/test_helpers.py | 140 ++++++++----- .../tests/test_library_content.py | 22 +- .../transformers/tests/test_split_test.py | 20 +- .../transformers/tests/test_start_date.py | 5 +- .../tests/test_user_partitions.py | 8 +- .../transformers/tests/test_visibility.py | 6 +- .../transformers/user_partitions.py | 9 +- .../course_blocks/transformers/visibility.py | 2 +- lms/envs/dev.py | 5 - lms/envs/test.py | 13 +- openedx/core/lib/block_cache/block_cache.py | 90 -------- .../block_cache/block_structure_factory.py | 197 ------------------ .../lib/block_cache/tests/test_block_cache.py | 115 ---------- .../tests/test_block_structure_factory.py | 114 ---------- .../__init__.py | 2 +- .../block_structure.py | 16 +- openedx/core/lib/block_structure/cache.py | 123 +++++++++++ .../exceptions.py | 0 openedx/core/lib/block_structure/factory.py | 84 ++++++++ openedx/core/lib/block_structure/manager.py | 96 +++++++++ .../tests/__init__.py | 0 .../tests/test_block_structure.py | 10 +- .../lib/block_structure/tests/test_cache.py | 49 +++++ .../lib/block_structure/tests/test_factory.py | 54 +++++ .../lib/block_structure/tests/test_manager.py | 145 +++++++++++++ .../tests/test_transformer_registry.py | 15 +- .../tests/test_transformers.py | 73 +++++++ .../tests/test_utils.py | 47 +++-- .../transformer.py | 4 +- .../transformer_registry.py | 8 +- .../core/lib/block_structure/transformers.py | 107 ++++++++++ 52 files changed, 1043 insertions(+), 742 deletions(-) create mode 100644 lms/djangoapps/course_blocks/tests/test_utils.py delete mode 100644 openedx/core/lib/block_cache/block_cache.py delete mode 100644 openedx/core/lib/block_cache/block_structure_factory.py delete mode 100644 openedx/core/lib/block_cache/tests/test_block_cache.py delete mode 100644 openedx/core/lib/block_cache/tests/test_block_structure_factory.py rename openedx/core/lib/{block_cache => block_structure}/__init__.py (97%) rename openedx/core/lib/{block_cache => block_structure}/block_structure.py (97%) create mode 100644 openedx/core/lib/block_structure/cache.py rename openedx/core/lib/{block_cache => block_structure}/exceptions.py (100%) create mode 100644 openedx/core/lib/block_structure/factory.py create mode 100644 openedx/core/lib/block_structure/manager.py rename openedx/core/lib/{block_cache => block_structure}/tests/__init__.py (100%) rename openedx/core/lib/{block_cache => block_structure}/tests/test_block_structure.py (95%) create mode 100644 openedx/core/lib/block_structure/tests/test_cache.py create mode 100644 openedx/core/lib/block_structure/tests/test_factory.py create mode 100644 openedx/core/lib/block_structure/tests/test_manager.py rename openedx/core/lib/{block_cache => block_structure}/tests/test_transformer_registry.py (75%) create mode 100644 openedx/core/lib/block_structure/tests/test_transformers.py rename openedx/core/lib/{block_cache => block_structure}/tests/test_utils.py (85%) rename openedx/core/lib/{block_cache => block_structure}/transformer.py (97%) rename openedx/core/lib/{block_cache => block_structure}/transformer_registry.py (86%) create mode 100644 openedx/core/lib/block_structure/transformers.py diff --git a/lms/djangoapps/course_api/blocks/api.py b/lms/djangoapps/course_api/blocks/api.py index f87b5fc987..0adff9c75e 100644 --- a/lms/djangoapps/course_api/blocks/api.py +++ b/lms/djangoapps/course_api/blocks/api.py @@ -2,10 +2,12 @@ API function for retrieving course blocks data """ +from lms.djangoapps.course_blocks.api import get_course_blocks, COURSE_BLOCK_ACCESS_TRANSFORMERS +from openedx.core.lib.block_structure.transformers import BlockStructureTransformers + from .transformers.blocks_api import BlocksAPITransformer from .transformers.proctored_exam import ProctoredExamTransformer from .serializers import BlockSerializer, BlockDictSerializer -from lms.djangoapps.course_blocks.api import get_course_blocks, COURSE_BLOCK_ACCESS_TRANSFORMERS def get_blocks( @@ -17,7 +19,7 @@ def get_blocks( requested_fields=None, block_counts=None, student_view_data=None, - return_type='dict' + return_type='dict', ): """ Return a serialized representation of the course blocks. @@ -43,25 +45,21 @@ def get_blocks( return_type (string): Possible values are 'dict' or 'list'. Indicates the format for returning the blocks. """ - # construct BlocksAPITransformer - blocks_api_transformer = BlocksAPITransformer( - block_counts, - student_view_data, - depth, - nav_depth - ) - - # list of transformers to apply, adding user-specific ones if user is provided - transformers = [] + # create ordered list of transformers, adding BlocksAPITransformer at end. + transformers = BlockStructureTransformers() if user is not None: transformers += COURSE_BLOCK_ACCESS_TRANSFORMERS + [ProctoredExamTransformer()] - transformers += [blocks_api_transformer] + transformers += [ + BlocksAPITransformer( + block_counts, + student_view_data, + depth, + nav_depth + ) + ] - blocks = get_course_blocks( - user, - usage_key, - transformers=transformers, - ) + # transform + blocks = get_course_blocks(user, usage_key, transformers) # serialize serializer_context = { diff --git a/lms/djangoapps/course_api/blocks/tests/test_api.py b/lms/djangoapps/course_api/blocks/tests/test_api.py index 35015c4409..fb6bfa91ef 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_api.py +++ b/lms/djangoapps/course_api/blocks/tests/test_api.py @@ -3,6 +3,8 @@ Tests for Blocks api.py """ from django.test.client import RequestFactory + +from course_blocks.tests.helpers import EnableTransformerRegistryMixin from student.tests.factories import UserFactory from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase @@ -11,7 +13,7 @@ from xmodule.modulestore.tests.factories import SampleCourseFactory from ..api import get_blocks -class TestGetBlocks(SharedModuleStoreTestCase): +class TestGetBlocks(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): """ Tests for the get_blocks function """ diff --git a/lms/djangoapps/course_api/blocks/tests/test_forms.py b/lms/djangoapps/course_api/blocks/tests/test_forms.py index 5c1b01e598..78f94e4b71 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_forms.py +++ b/lms/djangoapps/course_api/blocks/tests/test_forms.py @@ -6,6 +6,7 @@ from django.http import Http404, QueryDict from urllib import urlencode from rest_framework.exceptions import PermissionDenied +from course_blocks.tests.helpers import EnableTransformerRegistryMixin from opaque_keys.edx.locator import CourseLocator from openedx.core.djangoapps.util.test_forms import FormTestMixin from student.models import CourseEnrollment @@ -17,7 +18,7 @@ from ..forms import BlockListGetForm @ddt.ddt -class TestBlockListGetForm(FormTestMixin, SharedModuleStoreTestCase): +class TestBlockListGetForm(EnableTransformerRegistryMixin, FormTestMixin, SharedModuleStoreTestCase): """ Tests for BlockListGetForm """ diff --git a/lms/djangoapps/course_api/blocks/tests/test_serializers.py b/lms/djangoapps/course_api/blocks/tests/test_serializers.py index 010b112e6c..f3a8691c28 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_serializers.py +++ b/lms/djangoapps/course_api/blocks/tests/test_serializers.py @@ -3,6 +3,8 @@ Tests for Course Blocks serializers """ from mock import MagicMock +from course_blocks.tests.helpers import EnableTransformerRegistryMixin +from openedx.core.lib.block_structure.transformers import BlockStructureTransformers from student.tests.factories import UserFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import ToyCourseFactory @@ -10,10 +12,10 @@ from lms.djangoapps.course_blocks.api import get_course_blocks, COURSE_BLOCK_ACC from ..transformers.blocks_api import BlocksAPITransformer from ..serializers import BlockSerializer, BlockDictSerializer -from .test_utils import deserialize_usage_key +from .helpers import deserialize_usage_key -class TestBlockSerializerBase(SharedModuleStoreTestCase): +class TestBlockSerializerBase(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): """ Base class for testing BlockSerializer and BlockDictSerializer """ @@ -33,8 +35,8 @@ class TestBlockSerializerBase(SharedModuleStoreTestCase): ) self.block_structure = get_course_blocks( self.user, - root_block_usage_key=self.course.location, - transformers=COURSE_BLOCK_ACCESS_TRANSFORMERS + [blocks_api_transformer], + self.course.location, + BlockStructureTransformers(COURSE_BLOCK_ACCESS_TRANSFORMERS + [blocks_api_transformer]), ) self.serializer_context = { 'request': MagicMock(), diff --git a/lms/djangoapps/course_api/blocks/tests/test_views.py b/lms/djangoapps/course_api/blocks/tests/test_views.py index 7c9bbcd981..700077baef 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_views.py +++ b/lms/djangoapps/course_api/blocks/tests/test_views.py @@ -7,16 +7,17 @@ from string import join from urllib import urlencode from urlparse import urlunparse +from course_blocks.tests.helpers import EnableTransformerRegistryMixin from opaque_keys.edx.locator import CourseLocator from student.models import CourseEnrollment from student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase from xmodule.modulestore.tests.factories import ToyCourseFactory -from .test_utils import deserialize_usage_key +from .helpers import deserialize_usage_key -class TestBlocksView(SharedModuleStoreTestCase): +class TestBlocksView(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): """ Test class for BlocksView """ diff --git a/lms/djangoapps/course_api/blocks/transformers/block_counts.py b/lms/djangoapps/course_api/blocks/transformers/block_counts.py index 35fecf8ed5..2bab0f912e 100644 --- a/lms/djangoapps/course_api/blocks/transformers/block_counts.py +++ b/lms/djangoapps/course_api/blocks/transformers/block_counts.py @@ -1,7 +1,7 @@ """ Block Counts Transformer """ -from openedx.core.lib.block_cache.transformer import BlockStructureTransformer +from openedx.core.lib.block_structure.transformer import BlockStructureTransformer class BlockCountsTransformer(BlockStructureTransformer): diff --git a/lms/djangoapps/course_api/blocks/transformers/block_depth.py b/lms/djangoapps/course_api/blocks/transformers/block_depth.py index 88b021b466..6f7342e5f5 100644 --- a/lms/djangoapps/course_api/blocks/transformers/block_depth.py +++ b/lms/djangoapps/course_api/blocks/transformers/block_depth.py @@ -1,7 +1,7 @@ """ Block Depth Transformer """ -from openedx.core.lib.block_cache.transformer import BlockStructureTransformer +from openedx.core.lib.block_structure.transformer import BlockStructureTransformer class BlockDepthTransformer(BlockStructureTransformer): diff --git a/lms/djangoapps/course_api/blocks/transformers/blocks_api.py b/lms/djangoapps/course_api/blocks/transformers/blocks_api.py index 94b7c5d3d1..799c3618c1 100644 --- a/lms/djangoapps/course_api/blocks/transformers/blocks_api.py +++ b/lms/djangoapps/course_api/blocks/transformers/blocks_api.py @@ -1,7 +1,7 @@ """ Blocks API Transformer """ -from openedx.core.lib.block_cache.transformer import BlockStructureTransformer +from openedx.core.lib.block_structure.transformer import BlockStructureTransformer from .block_counts import BlockCountsTransformer from .block_depth import BlockDepthTransformer from .navigation import BlockNavigationTransformer diff --git a/lms/djangoapps/course_api/blocks/transformers/navigation.py b/lms/djangoapps/course_api/blocks/transformers/navigation.py index 6dd34df5db..7fad43e8b6 100644 --- a/lms/djangoapps/course_api/blocks/transformers/navigation.py +++ b/lms/djangoapps/course_api/blocks/transformers/navigation.py @@ -1,7 +1,7 @@ """ TODO """ -from openedx.core.lib.block_cache.transformer import BlockStructureTransformer +from openedx.core.lib.block_structure.transformer import BlockStructureTransformer from .block_depth import BlockDepthTransformer diff --git a/lms/djangoapps/course_api/blocks/transformers/proctored_exam.py b/lms/djangoapps/course_api/blocks/transformers/proctored_exam.py index 80a46e570a..777390848d 100644 --- a/lms/djangoapps/course_api/blocks/transformers/proctored_exam.py +++ b/lms/djangoapps/course_api/blocks/transformers/proctored_exam.py @@ -6,7 +6,7 @@ from django.conf import settings from edx_proctoring.api import get_attempt_status_summary from edx_proctoring.models import ProctoredExamStudentAttemptStatus -from openedx.core.lib.block_cache.transformer import BlockStructureTransformer +from openedx.core.lib.block_structure.transformer import BlockStructureTransformer class ProctoredExamTransformer(BlockStructureTransformer): diff --git a/lms/djangoapps/course_api/blocks/transformers/student_view.py b/lms/djangoapps/course_api/blocks/transformers/student_view.py index 820be4d4b9..d9da16f72e 100644 --- a/lms/djangoapps/course_api/blocks/transformers/student_view.py +++ b/lms/djangoapps/course_api/blocks/transformers/student_view.py @@ -1,7 +1,7 @@ """ Student View Transformer """ -from openedx.core.lib.block_cache.transformer import BlockStructureTransformer +from openedx.core.lib.block_structure.transformer import BlockStructureTransformer class StudentViewTransformer(BlockStructureTransformer): diff --git a/lms/djangoapps/course_api/blocks/transformers/tests/test_block_counts.py b/lms/djangoapps/course_api/blocks/transformers/tests/test_block_counts.py index 1bdc66d799..20b6e76b6e 100644 --- a/lms/djangoapps/course_api/blocks/transformers/tests/test_block_counts.py +++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_block_counts.py @@ -3,7 +3,7 @@ Tests for BlockCountsTransformer. """ # pylint: disable=protected-access -from openedx.core.lib.block_cache.block_structure_factory import BlockStructureFactory +from openedx.core.lib.block_structure.factory import BlockStructureFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import SampleCourseFactory diff --git a/lms/djangoapps/course_api/blocks/transformers/tests/test_block_depth.py b/lms/djangoapps/course_api/blocks/transformers/tests/test_block_depth.py index a4cf85641a..6c4f4c8695 100644 --- a/lms/djangoapps/course_api/blocks/transformers/tests/test_block_depth.py +++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_block_depth.py @@ -7,8 +7,8 @@ Tests for BlockDepthTransformer. import ddt from unittest import TestCase -from openedx.core.lib.block_cache.tests.test_utils import ChildrenMapTestMixin -from openedx.core.lib.block_cache.block_structure import BlockStructureModulestoreData +from openedx.core.lib.block_structure.tests.helpers import ChildrenMapTestMixin +from openedx.core.lib.block_structure.block_structure import BlockStructureModulestoreData from ..block_depth import BlockDepthTransformer @@ -34,7 +34,7 @@ class BlockDepthTransformerTestCase(TestCase, ChildrenMapTestMixin): ) @ddt.unpack def test_block_depth(self, block_depth, children_map, transformed_children_map, missing_blocks): - block_structure = self.create_block_structure(BlockStructureModulestoreData, children_map) + block_structure = self.create_block_structure(children_map, BlockStructureModulestoreData) BlockDepthTransformer(block_depth).transform(usage_info=None, block_structure=block_structure) block_structure._prune_unreachable() self.assert_block_structure(block_structure, transformed_children_map, missing_blocks) diff --git a/lms/djangoapps/course_api/blocks/transformers/tests/test_navigation.py b/lms/djangoapps/course_api/blocks/transformers/tests/test_navigation.py index bd860ca980..2de954edc0 100644 --- a/lms/djangoapps/course_api/blocks/transformers/tests/test_navigation.py +++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_navigation.py @@ -7,9 +7,9 @@ from unittest import TestCase from lms.djangoapps.course_api.blocks.transformers.block_depth import BlockDepthTransformer from lms.djangoapps.course_api.blocks.transformers.navigation import BlockNavigationTransformer -from openedx.core.lib.block_cache.tests.test_utils import ChildrenMapTestMixin -from openedx.core.lib.block_cache.block_structure import BlockStructureModulestoreData -from openedx.core.lib.block_cache.block_structure_factory import BlockStructureFactory +from openedx.core.lib.block_structure.tests.helpers import ChildrenMapTestMixin +from openedx.core.lib.block_structure.block_structure import BlockStructureModulestoreData +from openedx.core.lib.block_structure.factory import BlockStructureFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import SampleCourseFactory from xmodule.modulestore import ModuleStoreEnum @@ -46,7 +46,7 @@ class BlockNavigationTransformerTestCase(TestCase, ChildrenMapTestMixin): @ddt.unpack def test_navigation(self, depth, nav_depth, children_map, expected_nav_map): - block_structure = self.create_block_structure(BlockStructureModulestoreData, children_map) + block_structure = self.create_block_structure(children_map, BlockStructureModulestoreData) BlockDepthTransformer(depth).transform(usage_info=None, block_structure=block_structure) BlockNavigationTransformer(nav_depth).transform(usage_info=None, block_structure=block_structure) block_structure._prune_unreachable() diff --git a/lms/djangoapps/course_api/blocks/transformers/tests/test_proctored_exam.py b/lms/djangoapps/course_api/blocks/transformers/tests/test_proctored_exam.py index 486a671836..84dac71a89 100644 --- a/lms/djangoapps/course_api/blocks/transformers/tests/test_proctored_exam.py +++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_proctored_exam.py @@ -12,7 +12,7 @@ from edx_proctoring.api import ( from edx_proctoring.models import ProctoredExamStudentAttemptStatus from edx_proctoring.runtime import set_runtime_service from edx_proctoring.tests.test_services import MockCreditService -from lms.djangoapps.course_blocks.transformers.tests.test_helpers import CourseStructureTestCase +from lms.djangoapps.course_blocks.transformers.tests.helpers import CourseStructureTestCase from student.tests.factories import CourseEnrollmentFactory from ..proctored_exam import ProctoredExamTransformer @@ -25,6 +25,8 @@ class ProctoredExamTransformerTestCase(CourseStructureTestCase): """ Test behavior of ProctoredExamTransformer """ + TRANSFORMER_CLASS_TO_TEST = ProctoredExamTransformer + def setUp(self): """ Setup course structure and create user for split test transformer test. @@ -41,8 +43,6 @@ class ProctoredExamTransformerTestCase(CourseStructureTestCase): # Enroll user in course. CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) - self.transformer = ProctoredExamTransformer() - def setup_proctored_exam(self, block, attempt_status, user_id): """ Test helper to configure the given block as a proctored exam. @@ -123,7 +123,7 @@ class ProctoredExamTransformerTestCase(CourseStructureTestCase): block_structure = get_course_blocks( self.user, self.course.location, - transformers={self.transformer}, + self.transformers, ) self.assertEqual( set(block_structure.get_block_keys()), @@ -163,7 +163,7 @@ class ProctoredExamTransformerTestCase(CourseStructureTestCase): block_structure = get_course_blocks( self.user, self.course.location, - transformers={self.transformer}, + self.transformers, ) self.assertEqual( set(block_structure.get_block_keys()), diff --git a/lms/djangoapps/course_api/blocks/transformers/tests/test_student_view.py b/lms/djangoapps/course_api/blocks/transformers/tests/test_student_view.py index b3864c036a..63665bcf14 100644 --- a/lms/djangoapps/course_api/blocks/transformers/tests/test_student_view.py +++ b/lms/djangoapps/course_api/blocks/transformers/tests/test_student_view.py @@ -4,7 +4,7 @@ Tests for StudentViewTransformer. # pylint: disable=protected-access -from openedx.core.lib.block_cache.block_structure_factory import BlockStructureFactory +from openedx.core.lib.block_structure.factory import BlockStructureFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import ToyCourseFactory diff --git a/lms/djangoapps/course_blocks/api.py b/lms/djangoapps/course_blocks/api.py index 995a251b67..b41aeb1452 100644 --- a/lms/djangoapps/course_blocks/api.py +++ b/lms/djangoapps/course_blocks/api.py @@ -3,8 +3,8 @@ API entry point to the course_blocks app with top-level get_course_blocks and clear_course_from_cache functions. """ from django.core.cache import cache - -from openedx.core.lib.block_cache.block_cache import get_blocks, clear_block_cache +from openedx.core.lib.block_structure.manager import BlockStructureManager +from openedx.core.lib.block_structure.transformers import BlockStructureTransformers from xmodule.modulestore.django import modulestore from .transformers import ( @@ -29,11 +29,11 @@ COURSE_BLOCK_ACCESS_TRANSFORMERS = [ def get_course_blocks( user, root_block_usage_key, - transformers=None + transformers=None, ): """ A higher order function implemented on top of the - block_cache.get_blocks function returning a transformed block + block_structure.get_blocks function returning a transformed block structure for the given user starting at root_block_usage_key. Note: The current implementation requires the root_block_usage_key @@ -50,7 +50,7 @@ def get_course_blocks( root_block_usage_key (UsageKey) - The usage_key for the root of the block structure that is being accessed. - transformers ([BlockStructureTransformer]) - The list of + transformers (BlockStructureTransformers) - A collection of transformers whose transform methods are to be called. If None, COURSE_BLOCK_ACCESS_TRANSFORMERS is used. @@ -63,8 +63,7 @@ def get_course_blocks( exactly equivalent to the blocks that the given user has access. """ - store = modulestore() - if root_block_usage_key != store.make_course_usage_key(root_block_usage_key.course_key): + if root_block_usage_key != modulestore().make_course_usage_key(root_block_usage_key.course_key): # Enforce this check for now until MA-1604 is implemented. # Otherwise, callers will get incorrect block data after a # new version of the course is published, since @@ -72,26 +71,60 @@ def get_course_blocks( # structures starting at the root block of the course. raise NotImplementedError - return get_blocks( - cache, - store, - CourseUsageInfo(root_block_usage_key.course_key, user), - root_block_usage_key, - COURSE_BLOCK_ACCESS_TRANSFORMERS if transformers is None else transformers, - ) + if not transformers: + transformers = BlockStructureTransformers(COURSE_BLOCK_ACCESS_TRANSFORMERS) + transformers.usage_info = CourseUsageInfo(root_block_usage_key.course_key, user) + + return _get_block_structure_manager(root_block_usage_key.course_key).get_transformed(transformers) + + +def get_course_in_cache(course_key): + """ + A higher order function implemented on top of the + block_structure.get_collected function that returns the block + structure in the cache for the given course_key. + + Returns: + BlockStructureBlockData - The collected block structure, + starting at root_block_usage_key. + """ + return _get_block_structure_manager(course_key).get_collected() + + +def update_course_in_cache(course_key): + """ + A higher order function implemented on top of the + block_structure.updated_collected function that updates the block + structure in the cache for the given course_key. + """ + return _get_block_structure_manager(course_key).update_collected() def clear_course_from_cache(course_key): """ A higher order function implemented on top of the - block_cache.clear_block_cache function that clears the block - structure from the cache for the block structure starting at the - root block of the course for the given course_key. + block_structure.clear_block_cache function that clears the block + structure from the cache for the given course_key. Note: See Note in get_course_blocks. Even after MA-1604 is implemented, this implementation should still be valid since the entire block structure of the course is cached, even though arbitrary access to an intermediate block will be supported. """ - course_usage_key = modulestore().make_course_usage_key(course_key) - return clear_block_cache(cache, course_usage_key) + _get_block_structure_manager(course_key).clear() + + +def _get_block_structure_manager(course_key): + """ + Returns the manager for managing Block Structures for the given course. + """ + store = modulestore() + course_usage_key = store.make_course_usage_key(course_key) + return BlockStructureManager(course_usage_key, store, _get_cache()) + + +def _get_cache(): + """ + Returns the storage for caching Block Structures. + """ + return cache diff --git a/lms/djangoapps/course_blocks/tests/test_utils.py b/lms/djangoapps/course_blocks/tests/test_utils.py new file mode 100644 index 0000000000..820b93986b --- /dev/null +++ b/lms/djangoapps/course_blocks/tests/test_utils.py @@ -0,0 +1,33 @@ +""" +Helpers for Course Blocks tests. +""" + +from openedx.core.lib.block_structure.cache import BlockStructureCache +from openedx.core.lib.block_structure.transformer_registry import TransformerRegistry +from ..api import _get_cache + + +class EnableTransformerRegistryMixin(object): + """ + Mixin that enables the TransformerRegistry to USE_PLUGIN_MANAGER for + finding registered transformers. USE_PLUGIN_MANAGER is set to False + for LMS unit tests to speed up performance of the unit tests, so all + registered transformers in the platform do not need to be collected. + This Mixin is expected to be used by Tests for integration testing + with all registered transformers. + """ + def setUp(self, **kwargs): + super(EnableTransformerRegistryMixin, self).setUp(**kwargs) + TransformerRegistry.USE_PLUGIN_MANAGER = True + + def tearDown(self): + super(EnableTransformerRegistryMixin, self).tearDown() + TransformerRegistry.USE_PLUGIN_MANAGER = False + + +def is_course_in_block_structure_cache(course_key, store): + """ + Returns whether the given course is in the Block Structure cache. + """ + course_usage_key = store.make_course_usage_key(course_key) + return BlockStructureCache(_get_cache()).get(course_usage_key) is not None diff --git a/lms/djangoapps/course_blocks/transformers/library_content.py b/lms/djangoapps/course_blocks/transformers/library_content.py index 48a7ce74c1..3cc420fb45 100644 --- a/lms/djangoapps/course_blocks/transformers/library_content.py +++ b/lms/djangoapps/course_blocks/transformers/library_content.py @@ -3,7 +3,7 @@ Content Library Transformer. """ import json from courseware.models import StudentModule -from openedx.core.lib.block_cache.transformer import BlockStructureTransformer +from openedx.core.lib.block_structure.transformer import BlockStructureTransformer from xmodule.library_content_module import LibraryContentModule from xmodule.modulestore.django import modulestore from eventtracking import tracker diff --git a/lms/djangoapps/course_blocks/transformers/split_test.py b/lms/djangoapps/course_blocks/transformers/split_test.py index 306c20893f..1142640227 100644 --- a/lms/djangoapps/course_blocks/transformers/split_test.py +++ b/lms/djangoapps/course_blocks/transformers/split_test.py @@ -1,7 +1,7 @@ """ Split Test Block Transformer """ -from openedx.core.lib.block_cache.transformer import BlockStructureTransformer +from openedx.core.lib.block_structure.transformer import BlockStructureTransformer class SplitTestTransformer(BlockStructureTransformer): diff --git a/lms/djangoapps/course_blocks/transformers/start_date.py b/lms/djangoapps/course_blocks/transformers/start_date.py index 2838931943..09bdd09380 100644 --- a/lms/djangoapps/course_blocks/transformers/start_date.py +++ b/lms/djangoapps/course_blocks/transformers/start_date.py @@ -1,7 +1,7 @@ """ Start Date Transformer implementation. """ -from openedx.core.lib.block_cache.transformer import BlockStructureTransformer +from openedx.core.lib.block_structure.transformer import BlockStructureTransformer from lms.djangoapps.courseware.access_utils import check_start_date from xmodule.course_metadata_utils import DEFAULT_START_DATE diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_helpers.py b/lms/djangoapps/course_blocks/transformers/tests/test_helpers.py index 1d049c8d25..cc6e9ffaab 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_helpers.py +++ b/lms/djangoapps/course_blocks/transformers/tests/test_helpers.py @@ -1,8 +1,11 @@ """ Test helpers for testing course block transformers. """ +from mock import patch from course_modes.models import CourseMode +from openedx.core.lib.block_structure.transformers import BlockStructureTransformers from student.tests.factories import CourseEnrollmentFactory, UserFactory +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -11,7 +14,25 @@ from lms.djangoapps.courseware.access import has_access from ...api import get_course_blocks -class CourseStructureTestCase(ModuleStoreTestCase): +class TransformerRegistryTestMixin(object): + """ + Mixin that overrides the TransformerRegistry so that it returns + TRANSFORMER_CLASS_TO_TEST as a registered transformer. + """ + def setUp(self): + super(TransformerRegistryTestMixin, self).setUp() + self.patcher = patch( + 'openedx.core.lib.block_structure.transformer_registry.TransformerRegistry.get_registered_transformers' + ) + mock_registry = self.patcher.start() + mock_registry.return_value = {self.TRANSFORMER_CLASS_TO_TEST} + self.transformers = BlockStructureTransformers([self.TRANSFORMER_CLASS_TO_TEST()]) + + def tearDown(self): + self.patcher.stop() + + +class CourseStructureTestCase(TransformerRegistryTestMixin, ModuleStoreTestCase): """ Helper for test cases that need to build course structures. """ @@ -157,6 +178,8 @@ class CourseStructureTestCase(ModuleStoreTestCase): for block_hierarchy in course_hierarchy: self.add_parents(block_hierarchy, block_map) + publish_course(block_map['course']) + return block_map def get_block_key_set(self, blocks, *refs): @@ -170,7 +193,7 @@ class CourseStructureTestCase(ModuleStoreTestCase): return set([xblock.location for xblock in xblocks]) -class BlockParentsMapTestCase(ModuleStoreTestCase): +class BlockParentsMapTestCase(TransformerRegistryTestMixin, ModuleStoreTestCase): """ Test helper class for creating a test course of a graph of vertical blocks based on a parents_map. @@ -203,7 +226,6 @@ class BlockParentsMapTestCase(ModuleStoreTestCase): if i == 0: continue # course already created - # create the block as a vertical self.xblock_keys.append( ItemFactory.create( parent=self.get_block(parents_index[0]), @@ -252,66 +274,23 @@ class BlockParentsMapTestCase(ModuleStoreTestCase): transformers result and the current implementation of has_access. - transformers (BlockStructureTransformer): An optional list - of transformer that are to be executed. If not + transformers (BlockStructureTransformers): An optional collection + of transformers that are to be executed. If not provided, the default value used by get_course_blocks is used. """ - def check_results(user, expected_accessible_blocks, blocks_with_differing_access): - """ - Verifies the results of transforming the blocks in the - course for the given user. - """ - - self.client.login(username=user.username, password=self.password) - block_structure = get_course_blocks(user, self.course.location, transformers=transformers) - - # Enumerate through all the blocks that were created in the - # course - for i, xblock_key in enumerate(self.xblock_keys): - - # verify existence of the block - block_structure_result = block_structure.has_block(xblock_key) - has_access_result = bool(has_access(user, 'load', self.get_block(i), course_key=self.course.id)) - - # compare with expected value - self.assertEquals( - block_structure_result, - i in expected_accessible_blocks, - "block_structure return value {0} not equal to expected value for block {1} for user {2}".format( - block_structure_result, i, user.username - ) - ) - - # compare with has_access result - if i in blocks_with_differing_access: - self.assertNotEqual( - block_structure_result, - has_access_result, - "block structure ({0}) & has_access ({1}) results are equal for block {2} for user {3}".format( - block_structure_result, has_access_result, i, user.username - ) - ) - else: - self.assertEquals( - block_structure_result, - has_access_result, - "block structure ({0}) & has_access ({1}) results not equal for block {2} for user {3}".format( - block_structure_result, has_access_result, i, user.username - ) - ) - - self.client.logout() + publish_course(self.course) # verify given test user has access to expected blocks - check_results( + self._check_results( test_user, expected_user_accessible_blocks, - blocks_with_differing_access + blocks_with_differing_access, + transformers, ) # verify staff has access to all blocks - check_results(self.staff, set(range(len(self.parents_map))), {}) + self._check_results(self.staff, set(range(len(self.parents_map))), {}, transformers) def get_block(self, block_index): """ @@ -320,12 +299,65 @@ class BlockParentsMapTestCase(ModuleStoreTestCase): """ return modulestore().get_item(self.xblock_keys[block_index]) + def _check_results(self, user, expected_accessible_blocks, blocks_with_differing_access, transformers): + """ + Verifies the results of transforming the blocks in the + course for the given user. + """ + + self.client.login(username=user.username, password=self.password) + block_structure = get_course_blocks(user, self.course.location, transformers) + + for i, xblock_key in enumerate(self.xblock_keys): + + # compute access results of the block + block_structure_result = block_structure.has_block(xblock_key) + has_access_result = bool(has_access(user, 'load', self.get_block(i), course_key=self.course.id)) + + # compare with expected value + self.assertEquals( + block_structure_result, + i in expected_accessible_blocks, + "block_structure return value {0} not equal to expected value for block {1} for user {2}".format( + block_structure_result, i, user.username + ) + ) + + # compare with has_access_result + if i in blocks_with_differing_access: + self.assertNotEqual( + block_structure_result, + has_access_result, + "block structure ({0}) & has_access ({1}) results are equal for block {2} for user {3}".format( + block_structure_result, has_access_result, i, user.username + ) + ) + else: + self.assertEquals( + block_structure_result, + has_access_result, + "block structure ({0}) & has_access ({1}) results not equal for block {2} for user {3}".format( + block_structure_result, has_access_result, i, user.username + ) + ) + + self.client.logout() + def update_block(block): """ Helper method to update the block in the modulestore """ - return modulestore().update_item(block, 'test_user') + return modulestore().update_item(block, ModuleStoreEnum.UserID.test) + + +def publish_course(course): + """ + Helper method to publish the course (from draft to publish branch) + """ + store = modulestore() + with store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, course.id): + store.publish(course.location, ModuleStoreEnum.UserID.test) def create_location(org, course, run, block_type, block_id): 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 b712ef33d4..fe56928109 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_library_content.py +++ b/lms/djangoapps/course_blocks/transformers/tests/test_library_content.py @@ -4,9 +4,11 @@ Tests for ContentLibraryTransformer. import mock from student.tests.factories import CourseEnrollmentFactory -from course_blocks.transformers.library_content import ContentLibraryTransformer -from course_blocks.api import get_course_blocks, clear_course_from_cache -from lms.djangoapps.course_blocks.transformers.tests.test_helpers import CourseStructureTestCase +from openedx.core.lib.block_structure.transformers import BlockStructureTransformers + +from ...api import get_course_blocks, clear_course_from_cache +from ..library_content import ContentLibraryTransformer +from .helpers import CourseStructureTestCase class MockedModule(object): @@ -24,6 +26,7 @@ class ContentLibraryTransformerTestCase(CourseStructureTestCase): """ ContentLibraryTransformer Test """ + TRANSFORMER_CLASS_TO_TEST = ContentLibraryTransformer def setUp(self): """ @@ -40,9 +43,6 @@ class ContentLibraryTransformerTestCase(CourseStructureTestCase): # Enroll user in course. CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) - self.selected_module = MockedModule('{"selected": [["vertical", "vertical_vertical2"]]}') - self.transformer = ContentLibraryTransformer() - def get_course_hierarchy(self): """ Get a course hierarchy to test with. @@ -116,7 +116,7 @@ class ContentLibraryTransformerTestCase(CourseStructureTestCase): raw_block_structure = get_course_blocks( self.user, self.course.location, - transformers={} + transformers=BlockStructureTransformers(), ) self.assertEqual(len(list(raw_block_structure.get_block_keys())), len(self.blocks)) @@ -124,7 +124,7 @@ class ContentLibraryTransformerTestCase(CourseStructureTestCase): trans_block_structure = get_course_blocks( self.user, self.course.location, - transformers={self.transformer} + self.transformers, ) # Should dynamically assign a block to student @@ -141,14 +141,14 @@ class ContentLibraryTransformerTestCase(CourseStructureTestCase): # Check course structure again, with mocked selected modules for a user. with mock.patch( - 'course_blocks.transformers.library_content.ContentLibraryTransformer._get_student_module', - return_value=self.selected_module + 'lms.djangoapps.course_blocks.transformers.library_content.ContentLibraryTransformer._get_student_module', + return_value=MockedModule('{"selected": [["vertical", "vertical_vertical2"]]}'), ): clear_course_from_cache(self.course.id) trans_block_structure = get_course_blocks( self.user, self.course.location, - transformers={self.transformer} + self.transformers, ) self.assertEqual( set(trans_block_structure.get_block_keys()), 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 861fbf2e41..0a1850eda1 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_split_test.py +++ b/lms/djangoapps/course_blocks/transformers/tests/test_split_test.py @@ -11,7 +11,7 @@ from xmodule.modulestore.tests.factories import check_mongo_calls, check_mongo_c from ...api import get_course_blocks from ..user_partitions import UserPartitionTransformer, _get_user_partition_groups -from .test_helpers import CourseStructureTestCase, create_location +from .helpers import CourseStructureTestCase, create_location @ddt.ddt @@ -20,6 +20,7 @@ class SplitTestTransformerTestCase(CourseStructureTestCase): SplitTestTransformer Test """ TEST_PARTITION_ID = 0 + TRANSFORMER_CLASS_TO_TEST = UserPartitionTransformer def setUp(self): """ @@ -47,8 +48,6 @@ class SplitTestTransformerTestCase(CourseStructureTestCase): # Enroll user in course. CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) - self.transformer = UserPartitionTransformer() - def get_course_hierarchy(self): """ Get a course hierarchy to test with. @@ -193,7 +192,7 @@ class SplitTestTransformerTestCase(CourseStructureTestCase): block_structure1 = get_course_blocks( self.user, self.course.location, - transformers={self.transformer}, + self.transformers, ) self.assertEqual( set(block_structure1.get_block_keys()), @@ -208,17 +207,16 @@ class SplitTestTransformerTestCase(CourseStructureTestCase): self.assertEquals(len(user_groups), 1) # calling twice should result in the same block set - with check_mongo_calls_range(min_finds=1): - block_structure1 = get_course_blocks( - self.user, - self.course.location, - transformers={self.transformer}, - ) + block_structure1 = get_course_blocks( + self.user, + self.course.location, + self.transformers, + ) with check_mongo_calls(0): block_structure2 = get_course_blocks( self.user, self.course.location, - transformers={self.transformer}, + self.transformers, ) self.assertEqual( set(block_structure1.get_block_keys()), diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_start_date.py b/lms/djangoapps/course_blocks/transformers/tests/test_start_date.py index 88d1d744d9..572b2c1fa3 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_start_date.py +++ b/lms/djangoapps/course_blocks/transformers/tests/test_start_date.py @@ -8,7 +8,7 @@ from mock import patch from courseware.tests.factories import BetaTesterFactory from ..start_date import StartDateTransformer, DEFAULT_START_DATE -from .test_helpers import BlockParentsMapTestCase, update_block +from .helpers import BlockParentsMapTestCase, update_block @ddt.ddt @@ -18,6 +18,7 @@ class StartDateTransformerTestCase(BlockParentsMapTestCase): """ STUDENT = 1 BETA_USER = 2 + TRANSFORMER_CLASS_TO_TEST = StartDateTransformer class StartDateType(object): """ @@ -114,5 +115,5 @@ class StartDateTransformerTestCase(BlockParentsMapTestCase): self.beta_user if user_type == self.BETA_USER else self.student, expected_student_visible_blocks, blocks_with_differing_student_access, - [StartDateTransformer()], + self.transformers, ) diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py b/lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py index c10bca4289..5e9931ce91 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py +++ b/lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py @@ -15,13 +15,15 @@ from xmodule.modulestore.tests.factories import CourseFactory from ...api import get_course_blocks from ..user_partitions import UserPartitionTransformer, _MergedGroupAccess -from .test_helpers import CourseStructureTestCase, update_block +from .helpers import CourseStructureTestCase, update_block class UserPartitionTestMixin(object): """ Helper Mixin for testing user partitions. """ + TRANSFORMER_CLASS_TO_TEST = UserPartitionTransformer + def setup_groups_partitions(self, num_user_partitions=1, num_groups=4): """ Sets up groups and user partitions for testing. @@ -90,8 +92,6 @@ class UserPartitionTransformerTestCase(UserPartitionTestMixin, CourseStructureTe # Set up cohorts. self.setup_cohorts(self.course) - self.transformer = UserPartitionTransformer() - def get_course_hierarchy(self): """ Returns a course hierarchy to test with. @@ -204,7 +204,7 @@ class UserPartitionTransformerTestCase(UserPartitionTestMixin, CourseStructureTe trans_block_structure = get_course_blocks( self.user, self.course.location, - transformers={self.transformer} + self.transformers, ) self.assertSetEqual( set(trans_block_structure.get_block_keys()), diff --git a/lms/djangoapps/course_blocks/transformers/tests/test_visibility.py b/lms/djangoapps/course_blocks/transformers/tests/test_visibility.py index c6841e707a..1f9c7a6e43 100644 --- a/lms/djangoapps/course_blocks/transformers/tests/test_visibility.py +++ b/lms/djangoapps/course_blocks/transformers/tests/test_visibility.py @@ -4,7 +4,7 @@ Tests for VisibilityTransformer. import ddt from ..visibility import VisibilityTransformer -from .test_helpers import BlockParentsMapTestCase, update_block +from .helpers import BlockParentsMapTestCase, update_block @ddt.ddt @@ -12,6 +12,8 @@ class VisibilityTransformerTestCase(BlockParentsMapTestCase): """ VisibilityTransformer Test """ + TRANSFORMER_CLASS_TO_TEST = VisibilityTransformer + # Following test cases are based on BlockParentsMapTestCase.parents_map @ddt.data( ({}, {0, 1, 2, 3, 4, 5, 6}, {}), @@ -39,5 +41,5 @@ class VisibilityTransformerTestCase(BlockParentsMapTestCase): self.student, expected_visible_blocks, blocks_with_differing_access, - [VisibilityTransformer()], + self.transformers, ) diff --git a/lms/djangoapps/course_blocks/transformers/user_partitions.py b/lms/djangoapps/course_blocks/transformers/user_partitions.py index 91ef4f9918..86d8eec833 100644 --- a/lms/djangoapps/course_blocks/transformers/user_partitions.py +++ b/lms/djangoapps/course_blocks/transformers/user_partitions.py @@ -1,7 +1,7 @@ """ User Partitions Transformer """ -from openedx.core.lib.block_cache.transformer import BlockStructureTransformer +from openedx.core.lib.block_structure.transformer import BlockStructureTransformer from .split_test import SplitTestTransformer from .utils import get_field_on_block @@ -66,12 +66,7 @@ class UserPartitionTransformer(BlockStructureTransformer): def transform(self, usage_info, block_structure): """ - Mutates block_structure and block_data based on the given - usage_info. - - Arguments: - usage_info (object) - block_structure (BlockStructureCollectedData) + Mutates block_structure based on the given usage_info. """ SplitTestTransformer().transform(usage_info, block_structure) diff --git a/lms/djangoapps/course_blocks/transformers/visibility.py b/lms/djangoapps/course_blocks/transformers/visibility.py index cde91c1ad3..2c6d3c1730 100644 --- a/lms/djangoapps/course_blocks/transformers/visibility.py +++ b/lms/djangoapps/course_blocks/transformers/visibility.py @@ -1,7 +1,7 @@ """ Visibility Transformer implementation. """ -from openedx.core.lib.block_cache.transformer import BlockStructureTransformer +from openedx.core.lib.block_structure.transformer import BlockStructureTransformer class VisibilityTransformer(BlockStructureTransformer): diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 4124df3ba3..34d54b206a 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -87,11 +87,6 @@ CACHES = { 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', 'LOCATION': 'edx_course_structure_mem_cache', }, - 'lms.course_blocks': { - 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', - 'KEY_FUNCTION': 'util.memcache.safe_key', - 'LOCATION': 'lms_course_blocks_cache', - }, } diff --git a/lms/envs/test.py b/lms/envs/test.py index 429d5c0507..d3b5a7a6db 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -226,14 +226,6 @@ CACHES = { 'course_structure_cache': { 'BACKEND': 'django.core.cache.backends.dummy.DummyCache', }, - 'block_cache': { - 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', - 'LOCATION': 'edx_location_block_cache', - }, - 'lms.course_blocks': { - 'BACKEND': 'django.core.cache.backends.locmem.LocMemCache', - 'LOCATION': 'edx_location_course_blocks', - }, } # Dummy secret key for dev @@ -571,3 +563,8 @@ JWT_AUTH.update({ 'JWT_ISSUER': 'https://test-provider/oauth2', 'JWT_AUDIENCE': 'test-key', }) + +# Disable the use of the plugin manager in the transformer registry for +# better performant unit tests. +from openedx.core.lib.block_structure.transformer_registry import TransformerRegistry +TransformerRegistry.USE_PLUGIN_MANAGER = False diff --git a/openedx/core/lib/block_cache/block_cache.py b/openedx/core/lib/block_cache/block_cache.py deleted file mode 100644 index 7914cec3e3..0000000000 --- a/openedx/core/lib/block_cache/block_cache.py +++ /dev/null @@ -1,90 +0,0 @@ -""" -Top-level module for the Block Cache framework with higher order -functions for getting and clearing cached blocks. -""" -from .block_structure_factory import BlockStructureFactory -from .exceptions import TransformerException -from .transformer_registry import TransformerRegistry - - -def get_blocks(cache, modulestore, usage_info, root_block_usage_key, transformers): - """ - Top-level function in the Block Cache framework that manages - the cache (populating it and updating it when needed), calls the - transformers as appropriate (collect and transform methods), and - accessing the modulestore when needed (at cache miss). - - Arguments: - cache (django.core.cache.backends.base.BaseCache) - The - cache to use for storing/retrieving the block structure's - collected data. - - modulestore (ModuleStoreRead) - The modulestore that - contains the data for the xBlock objects corresponding to - the block structure. - - usage_info (any negotiated type) - A usage-specific object - that is forwarded to all requested Transformers in order - to apply a usage-specific transform. For example, an - instance of usage_info would contain a user object for - which the transform should be applied. - - root_block_usage_key (UsageKey) - The usage_key for the root - of the block structure that is being accessed. - - transformers ([BlockStructureTransformer]) - The list of - transformers whose transform methods are to be called. - This list should be a subset of the list of registered - transformers in the Transformer Registry. - - Returns: - BlockStructureBlockData - A transformed block structure, - starting at root_block_usage_key, that has undergone the - transform methods in the given transformers with the - given usage_info. - """ - - # Verify that all requested transformers are registered in the - # Transformer Registry. - unregistered_transformers = TransformerRegistry.find_unregistered(transformers) - if unregistered_transformers: - raise TransformerException( - "The following requested transformers are not registered: {}".format(unregistered_transformers) - ) - - # Load the cached block structure. - root_block_structure = BlockStructureFactory.create_from_cache(root_block_usage_key, cache, transformers) - - # On cache miss, execute the collect phase and update the cache. - if not root_block_structure: - - # Create the block structure from the modulestore. - root_block_structure = BlockStructureFactory.create_from_modulestore(root_block_usage_key, modulestore) - - # Collect data from each registered transformer. - for transformer in TransformerRegistry.get_registered_transformers(): - root_block_structure._add_transformer(transformer) # pylint: disable=protected-access - transformer.collect(root_block_structure) - - # Collect all fields that were requested by the transformers. - root_block_structure._collect_requested_xblock_fields() # pylint: disable=protected-access - - # Cache this information. - BlockStructureFactory.serialize_to_cache(root_block_structure, cache) - - # Execute requested transforms on block structure. - for transformer in transformers: - transformer.transform(usage_info, root_block_structure) - - # Prune the block structure to remove any unreachable blocks. - root_block_structure._prune_unreachable() # pylint: disable=protected-access - - return root_block_structure - - -def clear_block_cache(cache, root_block_usage_key): - """ - Removes the block structure associated with the given root block - key. - """ - BlockStructureFactory.remove_from_cache(root_block_usage_key, cache) diff --git a/openedx/core/lib/block_cache/block_structure_factory.py b/openedx/core/lib/block_cache/block_structure_factory.py deleted file mode 100644 index d927de0f34..0000000000 --- a/openedx/core/lib/block_cache/block_structure_factory.py +++ /dev/null @@ -1,197 +0,0 @@ -""" -Module for factory class for BlockStructure objects. -""" -# pylint: disable=protected-access -from logging import getLogger - -from openedx.core.lib.cache_utils import zpickle, zunpickle - -from .block_structure import BlockStructureBlockData, BlockStructureModulestoreData - - -logger = getLogger(__name__) # pylint: disable=C0103 - - -class BlockStructureFactory(object): - """ - Factory class for BlockStructure objects. - """ - @classmethod - def create_from_modulestore(cls, root_block_usage_key, modulestore): - """ - Creates and returns a block structure from the modulestore - starting at the given root_block_usage_key. - - Arguments: - root_block_usage_key (UsageKey) - The usage_key for the root - of the block structure that is to be created. - - modulestore (ModuleStoreRead) - The modulestore that - contains the data for the xBlocks within the block - structure starting at root_block_usage_key. - - Returns: - BlockStructureModulestoreData - The created block structure - with instantiated xBlocks from the given modulestore - starting at root_block_usage_key. - """ - # Create block structure. - block_structure = BlockStructureModulestoreData(root_block_usage_key) - - # Create internal set of blocks visited to use when recursing. - blocks_visited = set() - - def build_block_structure(xblock): - """ - Recursively update the block structure with the given xBlock - and its descendants. - """ - # Check if the xblock was already visited (can happen in - # DAGs). - if xblock.location in blocks_visited: - return - - # Add the xBlock. - blocks_visited.add(xblock.location) - block_structure._add_xblock(xblock.location, xblock) - - # Add relations with its children and recurse. - for child in xblock.get_children(): - block_structure._add_relation(xblock.location, child.location) - build_block_structure(child) - - root_xblock = modulestore.get_item(root_block_usage_key, depth=None) - build_block_structure(root_xblock) - return block_structure - - @classmethod - def serialize_to_cache(cls, block_structure, cache): - """ - Store a compressed and pickled serialization of the given - block structure into the given cache. - - The key in the cache is 'root.key.'. - The data stored in the cache includes the structure's - block relations, transformer data, and block data. - - Arguments: - block_structure (BlockStructure) - The block structure - that is to be serialized to the given cache. - - cache (django.core.cache.backends.base.BaseCache) - The - cache into which cacheable data of the block structure - is to be serialized. - """ - data_to_cache = ( - block_structure._block_relations, - block_structure._transformer_data, - block_structure._block_data_map - ) - zp_data_to_cache = zpickle(data_to_cache) - cache.set( - cls._encode_root_cache_key(block_structure.root_block_usage_key), - zp_data_to_cache - ) - logger.debug( - "Wrote BlockStructure %s to cache, size: %s", - block_structure.root_block_usage_key, - len(zp_data_to_cache), - ) - - @classmethod - def create_from_cache(cls, root_block_usage_key, cache, transformers): - """ - Deserializes and returns the block structure starting at - root_block_usage_key from the given cache, if it's found in the cache. - - The given root_block_usage_key must equate the root_block_usage_key - previously passed to serialize_to_cache. - - Arguments: - root_block_usage_key (UsageKey) - The usage_key for the root - of the block structure that is to be deserialized from - the given cache. - - cache (django.core.cache.backends.base.BaseCache) - The - cache from which the block structure is to be - deserialized. - - transformers ([BlockStructureTransformer]) - A list of - transformers for which the block structure will be - transformed. - - Returns: - BlockStructure - The deserialized block structure starting - at root_block_usage_key, if found in the cache. - - NoneType - If the root_block_usage_key is not found in the cache - or if the cached data is outdated for one or more of the - given transformers. - """ - - # Find root_block_usage_key in the cache. - zp_data_from_cache = cache.get(cls._encode_root_cache_key(root_block_usage_key)) - if not zp_data_from_cache: - logger.debug( - "BlockStructure %r not found in the cache.", - root_block_usage_key, - ) - return None - else: - logger.debug( - "Read BlockStructure %r from cache, size: %s", - root_block_usage_key, - len(zp_data_from_cache), - ) - - # Deserialize and construct the block structure. - block_relations, transformer_data, block_data_map = zunpickle(zp_data_from_cache) - block_structure = BlockStructureBlockData(root_block_usage_key) - block_structure._block_relations = block_relations - block_structure._transformer_data = transformer_data - block_structure._block_data_map = block_data_map - - # Verify that the cached data for all the given transformers are - # for their latest versions. - outdated_transformers = {} - for transformer in transformers: - cached_transformer_version = block_structure._get_transformer_data_version(transformer) - if transformer.VERSION != cached_transformer_version: - outdated_transformers[transformer.name()] = "version: {}, cached: {}".format( - transformer.VERSION, - cached_transformer_version, - ) - if outdated_transformers: - logger.info( - "Collected data for the following transformers are outdated:\n%s.", - '\n'.join([t_name + ": " + t_value for t_name, t_value in outdated_transformers.iteritems()]), - ) - return None - - return block_structure - - @classmethod - def remove_from_cache(cls, root_block_usage_key, cache): - """ - Removes the block structure for the given root_block_usage_key - from the given cache. - - Arguments: - root_block_usage_key (UsageKey) - The usage_key for the root - of the block structure that is to be removed from - the given cache. - - cache (django.core.cache.backends.base.BaseCache) - The - cache from which the block structure is to be - removed. - """ - cache.delete(cls._encode_root_cache_key(root_block_usage_key)) - # TODO also remove all block data? - - @classmethod - def _encode_root_cache_key(cls, root_block_usage_key): - """ - Returns the cache key to use for storing the block structure - for the given root_block_usage_key. - """ - return "root.key." + unicode(root_block_usage_key) diff --git a/openedx/core/lib/block_cache/tests/test_block_cache.py b/openedx/core/lib/block_cache/tests/test_block_cache.py deleted file mode 100644 index ad98b76581..0000000000 --- a/openedx/core/lib/block_cache/tests/test_block_cache.py +++ /dev/null @@ -1,115 +0,0 @@ -""" -Tests for block_cache.py -""" - -from django.core.cache import get_cache -from django.conf import settings -from mock import patch -from unittest import TestCase, skipUnless - -from ..block_cache import get_blocks -from ..exceptions import TransformerException -from .test_utils import ( - MockModulestoreFactory, MockCache, MockTransformer, ChildrenMapTestMixin -) - - -@patch('openedx.core.lib.block_cache.transformer_registry.TransformerRegistry.get_available_plugins') -class TestBlockCache(TestCase, ChildrenMapTestMixin): - """ - Test class for block cache functionality. - """ - - class TestTransformer1(MockTransformer): - """ - Test Transformer class. - """ - @classmethod - def block_key(cls): - """ - Returns the dictionary key for transformer block data. - """ - return 't1.key1' - - @classmethod - def block_val(cls, block_key): - """ - Returns the dictionary value for transformer block data for - the block identified by the given block key. - """ - return 't1.val1.' + unicode(block_key) - - @classmethod - def collect(cls, block_structure): - """ - Sets transformer block data for each block in the structure - as it is visited using topological traversal. - """ - for block_key in block_structure.topological_traversal(): - block_structure.set_transformer_block_field( - block_key, cls, cls.block_key(), cls.block_val(block_key) - ) - - def transform(self, usage_info, block_structure): - """ - Verifies the transformer block data set for each block - in the structure. - """ - def assert_collected_value(block_key): - """ - Verifies the transformer block data for the given - block equates the value stored in the collect method. - """ - assert ( - block_structure.get_transformer_block_field( - block_key, - self, - self.block_key() - ) == self.block_val(block_key) - ) - - for block_key in block_structure.topological_traversal(): - assert_collected_value(block_key) - - def setUp(self): - super(TestBlockCache, self).setUp() - self.children_map = self.SIMPLE_CHILDREN_MAP - self.usage_info = None - self.mock_cache = MockCache() - self.modulestore = MockModulestoreFactory.create(self.children_map) - self.transformers = [self.TestTransformer1()] - - def test_get_blocks(self, mock_available_transforms): - mock_available_transforms.return_value = {transformer.name(): transformer for transformer in self.transformers} - block_structure = get_blocks( - self.mock_cache, self.modulestore, self.usage_info, root_block_usage_key=0, transformers=self.transformers - ) - self.assert_block_structure(block_structure, self.children_map) - - def test_unregistered_transformers(self, mock_available_transforms): - mock_available_transforms.return_value = {} - with self.assertRaisesRegexp(TransformerException, "requested transformers are not registered"): - get_blocks( - self.mock_cache, - self.modulestore, - self.usage_info, - root_block_usage_key=0, - transformers=self.transformers, - ) - - @skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') - def test_block_caching(self, mock_available_transforms): - mock_available_transforms.return_value = {transformer.name(): transformer for transformer in self.transformers} - - cache = get_cache('block_cache') - - for iteration in range(2): - self.modulestore.get_items_call_count = 0 - block_structure = get_blocks( - cache, self.modulestore, self.usage_info, root_block_usage_key=0, transformers=self.transformers - ) - self.assert_block_structure(block_structure, self.children_map) - if iteration == 0: - self.assertGreater(self.modulestore.get_items_call_count, 0) - else: - self.assertEquals(self.modulestore.get_items_call_count, 0) diff --git a/openedx/core/lib/block_cache/tests/test_block_structure_factory.py b/openedx/core/lib/block_cache/tests/test_block_structure_factory.py deleted file mode 100644 index e7649bb93a..0000000000 --- a/openedx/core/lib/block_cache/tests/test_block_structure_factory.py +++ /dev/null @@ -1,114 +0,0 @@ -""" -Tests for block_structure_factory.py -""" -# pylint: disable=protected-access -from mock import patch -from unittest import TestCase - -from ..block_structure_factory import BlockStructureFactory -from .test_utils import ( - MockCache, MockModulestoreFactory, MockTransformer, ChildrenMapTestMixin -) - - -class TestBlockStructureFactory(TestCase, ChildrenMapTestMixin): - """ - Tests for BlockStructureFactory - """ - def setUp(self): - super(TestBlockStructureFactory, self).setUp() - self.children_map = self.SIMPLE_CHILDREN_MAP - self.modulestore = MockModulestoreFactory.create(self.children_map) - - self.block_structure = BlockStructureFactory.create_from_modulestore( - root_block_usage_key=0, modulestore=self.modulestore - ) - - self.transformers = [MockTransformer] - mock_registry = patch( - 'openedx.core.lib.block_cache.transformer_registry.TransformerRegistry.get_available_plugins' - ) - mock_registry.return_value = {transformer.name(): transformer for transformer in self.transformers} - self.addCleanup(mock_registry.stop) - mock_registry.start() - - def add_transformers(self): - """ - Add each registered transformer to the block structure. - Mimic collection by setting test transformer block data. - """ - for transformer in self.transformers: - self.block_structure._add_transformer(transformer) - self.block_structure.set_transformer_block_field( - usage_key=0, transformer=transformer, key='test', value='{} val'.format(transformer.name()) - ) - - def test_create_from_modulestore(self): - self.assert_block_structure(self.block_structure, self.children_map) - - def test_not_in_cache(self): - cache = MockCache() - - self.assertIsNone( - BlockStructureFactory.create_from_cache( - root_block_usage_key=0, - cache=cache, - transformers=self.transformers, - ) - ) - - def test_uncollected_transformers(self): - cache = MockCache() - - # serialize the structure to cache, but without collecting any transformer data - BlockStructureFactory.serialize_to_cache(self.block_structure, cache) - - with patch('openedx.core.lib.block_cache.block_structure_factory.logger.info') as mock_logger: - # cached data does not have collected information for all registered transformers - self.assertIsNone( - BlockStructureFactory.create_from_cache( - root_block_usage_key=0, - cache=cache, - transformers=self.transformers, - ) - ) - self.assertTrue(mock_logger.called) - - def test_cache(self): - cache = MockCache() - - # collect transformer data - self.add_transformers() - - # serialize to cache - BlockStructureFactory.serialize_to_cache(self.block_structure, cache) - - # test re-create from cache - self.modulestore.get_items_call_count = 0 - from_cache_block_structure = BlockStructureFactory.create_from_cache( - root_block_usage_key=0, - cache=cache, - transformers=self.transformers, - ) - self.assertIsNotNone(from_cache_block_structure) - self.assert_block_structure(from_cache_block_structure, self.children_map) - self.assertEquals(self.modulestore.get_items_call_count, 0) - - def test_remove_from_cache(self): - cache = MockCache() - - # collect transformer data - self.add_transformers() - - # serialize to cache - BlockStructureFactory.serialize_to_cache(self.block_structure, cache) - - # remove from cache - BlockStructureFactory.remove_from_cache(root_block_usage_key=0, cache=cache) - self.assertIsNone( - BlockStructureFactory.create_from_cache( - root_block_usage_key=0, - cache=cache, - transformers=self.transformers - ) - ) diff --git a/openedx/core/lib/block_cache/__init__.py b/openedx/core/lib/block_structure/__init__.py similarity index 97% rename from openedx/core/lib/block_cache/__init__.py rename to openedx/core/lib/block_structure/__init__.py index 4cc1cb613e..f77cf27f04 100644 --- a/openedx/core/lib/block_cache/__init__.py +++ b/openedx/core/lib/block_structure/__init__.py @@ -1,5 +1,5 @@ """ -The block_cache django app provides an extensible framework for caching +The block_structure django app provides an extensible framework for caching data of block structures from the modulestore. Dual-Phase. The framework is meant to be used in 2 phases. diff --git a/openedx/core/lib/block_cache/block_structure.py b/openedx/core/lib/block_structure/block_structure.py similarity index 97% rename from openedx/core/lib/block_cache/block_structure.py rename to openedx/core/lib/block_structure/block_structure.py index e297cf7f48..caef8fb1ba 100644 --- a/openedx/core/lib/block_cache/block_structure.py +++ b/openedx/core/lib/block_structure/block_structure.py @@ -177,7 +177,7 @@ class BlockStructure(object): ) #--- Internal methods ---# - # To be used within the block_cache framework or by tests. + # To be used within the block_structure framework or by tests. def _prune_unreachable(self): """ @@ -488,18 +488,8 @@ class BlockStructureBlockData(BlockStructure): for _ in self.topological_traversal(filter_func=filter_func, **kwargs): pass - def get_block_keys(self): - """ - Returns the block keys in the block structure. - - Returns: - iterator(UsageKey) - An iterator of the usage - keys of all the blocks in the block structure. - """ - return self._block_relations.iterkeys() - #--- Internal methods ---# - # To be used within the block_cache framework or by tests. + # To be used within the block_structure framework or by tests. def _get_transformer_data_version(self, transformer): """ @@ -571,7 +561,7 @@ class BlockStructureModulestoreData(BlockStructureBlockData): return self._xblock_map[usage_key] #--- Internal methods ---# - # To be used within the block_cache framework or by tests. + # To be used within the block_structure framework or by tests. def _add_xblock(self, usage_key, xblock): """ diff --git a/openedx/core/lib/block_structure/cache.py b/openedx/core/lib/block_structure/cache.py new file mode 100644 index 0000000000..cbce9729e3 --- /dev/null +++ b/openedx/core/lib/block_structure/cache.py @@ -0,0 +1,123 @@ +""" +Module for the Cache class for BlockStructure objects. +""" +# pylint: disable=protected-access +from logging import getLogger + +from openedx.core.lib.cache_utils import zpickle, zunpickle + +from .block_structure import BlockStructureModulestoreData + + +logger = getLogger(__name__) # pylint: disable=C0103 + + +class BlockStructureCache(object): + """ + Cache for BlockStructure objects. + """ + def __init__(self, cache): + """ + Arguments: + cache (django.core.cache.backends.base.BaseCache) - The + cache into which cacheable data of the block structure + is to be serialized. + """ + self._cache = cache + + def add(self, block_structure): + """ + Store a compressed and pickled serialization of the given + block structure into the given cache. + + The key in the cache is 'root.key.'. + The data stored in the cache includes the structure's + block relations, transformer data, and block data. + + Arguments: + block_structure (BlockStructure) - The block structure + that is to be serialized to the given cache. + """ + data_to_cache = ( + block_structure._block_relations, + block_structure._transformer_data, + block_structure._block_data_map + ) + zp_data_to_cache = zpickle(data_to_cache) + self._cache.set( + self._encode_root_cache_key(block_structure.root_block_usage_key), + zp_data_to_cache + ) + logger.debug( + "Wrote BlockStructure %s to cache, size: %s", + block_structure.root_block_usage_key, + len(zp_data_to_cache), + ) + + def get(self, root_block_usage_key): + """ + Deserializes and returns the block structure starting at + root_block_usage_key from the given cache, if it's found in the cache. + + The given root_block_usage_key must equate the root_block_usage_key + previously passed to serialize_to_cache. + + Arguments: + root_block_usage_key (UsageKey) - The usage_key for the root + of the block structure that is to be deserialized from + the given cache. + + Returns: + BlockStructure - The deserialized block structure starting + at root_block_usage_key, if found in the cache. + + NoneType - If the root_block_usage_key is not found in the cache. + """ + + # Find root_block_usage_key in the cache. + zp_data_from_cache = self._cache.get(self._encode_root_cache_key(root_block_usage_key)) + if not zp_data_from_cache: + logger.debug( + "Did not find BlockStructure %r in the cache.", + root_block_usage_key, + ) + return None + else: + logger.debug( + "Read BlockStructure %r from cache, size: %s", + root_block_usage_key, + len(zp_data_from_cache), + ) + + # Deserialize and construct the block structure. + block_relations, transformer_data, block_data_map = zunpickle(zp_data_from_cache) + block_structure = BlockStructureModulestoreData(root_block_usage_key) + block_structure._block_relations = block_relations + block_structure._transformer_data = transformer_data + block_structure._block_data_map = block_data_map + + return block_structure + + def delete(self, root_block_usage_key): + """ + Deletes the block structure for the given root_block_usage_key + from the given cache. + + Arguments: + root_block_usage_key (UsageKey) - The usage_key for the root + of the block structure that is to be removed from + the cache. + """ + self._cache.delete(self._encode_root_cache_key(root_block_usage_key)) + logger.debug( + "Deleted BlockStructure %r from the cache.", + root_block_usage_key, + ) + + @classmethod + def _encode_root_cache_key(cls, root_block_usage_key): + """ + Returns the cache key to use for storing the block structure + for the given root_block_usage_key. + """ + return "root.key." + unicode(root_block_usage_key) diff --git a/openedx/core/lib/block_cache/exceptions.py b/openedx/core/lib/block_structure/exceptions.py similarity index 100% rename from openedx/core/lib/block_cache/exceptions.py rename to openedx/core/lib/block_structure/exceptions.py diff --git a/openedx/core/lib/block_structure/factory.py b/openedx/core/lib/block_structure/factory.py new file mode 100644 index 0000000000..7a2b253334 --- /dev/null +++ b/openedx/core/lib/block_structure/factory.py @@ -0,0 +1,84 @@ +""" +Module for factory class for BlockStructure objects. +""" +from .block_structure import BlockStructureModulestoreData + + +class BlockStructureFactory(object): + """ + Factory class for BlockStructure objects. + """ + @classmethod + def create_from_modulestore(cls, root_block_usage_key, modulestore): + """ + Creates and returns a block structure from the modulestore + starting at the given root_block_usage_key. + + Arguments: + root_block_usage_key (UsageKey) - The usage_key for the root + of the block structure that is to be created. + + modulestore (ModuleStoreRead) - The modulestore that + contains the data for the xBlocks within the block + structure starting at root_block_usage_key. + + Returns: + BlockStructureModulestoreData - The created block structure + with instantiated xBlocks from the given modulestore + starting at root_block_usage_key. + + Raises: + xmodule.modulestore.exceptions.ItemNotFoundError if a block for + root_block_usage_key is not found in the modulestore. + """ + block_structure = BlockStructureModulestoreData(root_block_usage_key) + blocks_visited = set() + + def build_block_structure(xblock): + """ + Recursively update the block structure with the given xBlock + and its descendants. + """ + # Check if the xblock was already visited (can happen in + # DAGs). + if xblock.location in blocks_visited: + return + + # Add the xBlock. + blocks_visited.add(xblock.location) + block_structure._add_xblock(xblock.location, xblock) # pylint: disable=protected-access + + # Add relations with its children and recurse. + for child in xblock.get_children(): + block_structure._add_relation(xblock.location, child.location) # pylint: disable=protected-access + build_block_structure(child) + + root_xblock = modulestore.get_item(root_block_usage_key, depth=None) + build_block_structure(root_xblock) + return block_structure + + @classmethod + def create_from_cache(cls, root_block_usage_key, block_structure_cache): + """ + Deserializes and returns the block structure starting at + root_block_usage_key from the given cache, if it's found in the cache. + + The given root_block_usage_key must equate the root_block_usage_key + previously passed to serialize_to_cache. + + Arguments: + root_block_usage_key (UsageKey) - The usage_key for the root + of the block structure that is to be deserialized from + the given cache. + + block_structure_cache (BlockStructureCache) - The + cache from which the block structure is to be + deserialized. + + Returns: + BlockStructure - The deserialized block structure starting + at root_block_usage_key, if found in the cache. + + NoneType - If the root_block_usage_key is not found in the cache. + """ + return block_structure_cache.get(root_block_usage_key) diff --git a/openedx/core/lib/block_structure/manager.py b/openedx/core/lib/block_structure/manager.py new file mode 100644 index 0000000000..7711a0c98f --- /dev/null +++ b/openedx/core/lib/block_structure/manager.py @@ -0,0 +1,96 @@ +""" +Top-level module for the Block Structure framework with a class for managing +BlockStructures. +""" +from .factory import BlockStructureFactory +from .cache import BlockStructureCache +from .transformers import BlockStructureTransformers + + +class BlockStructureManager(object): + """ + Top-level class for managing Block Structures. + """ + + def __init__(self, root_block_usage_key, modulestore, cache): + """ + Arguments: + root_block_usage_key (UsageKey) - The usage_key for the root + of the block structure that is being accessed. + + modulestore (ModuleStoreRead) - The modulestore that + contains the data for the xBlock objects corresponding to + the block structure. + + cache (django.core.cache.backends.base.BaseCache) - The + cache to use for storing/retrieving the block structure's + collected data. + """ + self.root_block_usage_key = root_block_usage_key + self.modulestore = modulestore + self.block_structure_cache = BlockStructureCache(cache) + + def get_transformed(self, transformers): + """ + Returns the transformed Block Structure for the root_block_usage_key, + getting block data from the cache and modulestore, as needed. + + Details: Same as the get_collected method, except the transformers' + transform methods are also called. + + Arguments: + transformers (BlockStructureTransformers) - Collection of + transformers to apply. + + Returns: + BlockStructureBlockData - A transformed block structure, + starting at self.root_block_usage_key. + """ + block_structure = self.get_collected() + transformers.transform(block_structure) + return block_structure + + def get_collected(self): + """ + Returns the collected Block Structure for the root_block_usage_key, + getting block data from the cache and modulestore, as needed. + + Details: The cache is updated if needed (if outdated or empty), + the modulestore is accessed if needed (at cache miss), and the + transformers data is collected if needed. + + Returns: + BlockStructureBlockData - A collected block structure, + starting at root_block_usage_key, with collected data + from each registered transformer. + """ + block_structure = BlockStructureFactory.create_from_cache( + self.root_block_usage_key, + self.block_structure_cache + ) + cache_miss = block_structure is None + if cache_miss or BlockStructureTransformers.is_collected_outdated(block_structure): + 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 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. + """ + self.clear() + self.get_collected() + + def clear(self): + """ + Removes cached data for the block structure associated with the given + root block key. + """ + self.block_structure_cache.delete(self.root_block_usage_key) diff --git a/openedx/core/lib/block_cache/tests/__init__.py b/openedx/core/lib/block_structure/tests/__init__.py similarity index 100% rename from openedx/core/lib/block_cache/tests/__init__.py rename to openedx/core/lib/block_structure/tests/__init__.py diff --git a/openedx/core/lib/block_cache/tests/test_block_structure.py b/openedx/core/lib/block_structure/tests/test_block_structure.py similarity index 95% rename from openedx/core/lib/block_cache/tests/test_block_structure.py rename to openedx/core/lib/block_structure/tests/test_block_structure.py index 994c758d87..78f47b2327 100644 --- a/openedx/core/lib/block_cache/tests/test_block_structure.py +++ b/openedx/core/lib/block_structure/tests/test_block_structure.py @@ -10,9 +10,9 @@ from unittest import TestCase from openedx.core.lib.graph_traversals import traverse_post_order -from ..block_structure import BlockStructure, BlockStructureModulestoreData, BlockStructureBlockData +from ..block_structure import BlockStructure, BlockStructureModulestoreData from ..exceptions import TransformerException -from .test_utils import MockXBlock, MockTransformer, ChildrenMapTestMixin +from .helpers import MockXBlock, MockTransformer, ChildrenMapTestMixin @ddt.ddt @@ -27,7 +27,7 @@ class TestBlockStructure(TestCase, ChildrenMapTestMixin): ChildrenMapTestMixin.DAG_CHILDREN_MAP, ) def test_relations(self, children_map): - block_structure = self.create_block_structure(BlockStructure, children_map) + block_structure = self.create_block_structure(children_map, BlockStructure) # get_children for parent, children in enumerate(children_map): @@ -167,7 +167,7 @@ class TestBlockStructureData(TestCase, ChildrenMapTestMixin): return ### create structure - block_structure = self.create_block_structure(BlockStructureBlockData, children_map) + block_structure = self.create_block_structure(children_map) parents_map = self.get_parents_map(children_map) ### verify blocks pre-exist @@ -213,6 +213,6 @@ class TestBlockStructureData(TestCase, ChildrenMapTestMixin): self.assert_block_structure(block_structure, pruned_children_map, missing_blocks) def test_remove_block_if(self): - block_structure = self.create_block_structure(BlockStructureBlockData, ChildrenMapTestMixin.LINEAR_CHILDREN_MAP) + block_structure = self.create_block_structure(ChildrenMapTestMixin.LINEAR_CHILDREN_MAP) block_structure.remove_block_if(lambda block: block == 2) self.assert_block_structure(block_structure, [[1], [], [], []], missing_blocks=[2]) diff --git a/openedx/core/lib/block_structure/tests/test_cache.py b/openedx/core/lib/block_structure/tests/test_cache.py new file mode 100644 index 0000000000..6654cbab9e --- /dev/null +++ b/openedx/core/lib/block_structure/tests/test_cache.py @@ -0,0 +1,49 @@ +""" +Tests for block_structure/cache.py +""" +from unittest import TestCase + +from ..cache import BlockStructureCache +from .helpers import ChildrenMapTestMixin, MockCache, MockTransformer + + +class TestBlockStructureCache(ChildrenMapTestMixin, TestCase): + """ + Tests for BlockStructureFactory + """ + def setUp(self): + super(TestBlockStructureCache, self).setUp() + self.children_map = self.SIMPLE_CHILDREN_MAP + self.block_structure = self.create_block_structure(self.children_map) + self.cache = BlockStructureCache(MockCache()) + + def add_transformers(self): + """ + Add each registered transformer to the block structure. + Mimic collection by setting test transformer block data. + """ + for transformer in [MockTransformer]: + self.block_structure._add_transformer(transformer) # pylint: disable=protected-access + self.block_structure.set_transformer_block_field( + usage_key=0, transformer=transformer, key='test', value='{} val'.format(transformer.name()) + ) + + def test_add(self): + self.add_transformers() + self.cache.add(self.block_structure) + cached_value = self.cache.get(self.block_structure.root_block_usage_key) + self.assertIsNotNone(cached_value) + self.assert_block_structure(cached_value, self.children_map) + + def test_get_none(self): + self.assertIsNone( + self.cache.get(self.block_structure.root_block_usage_key) + ) + + def test_delete(self): + self.add_transformers() + self.cache.add(self.block_structure) + self.cache.delete(self.block_structure.root_block_usage_key) + self.assertIsNone( + self.cache.get(self.block_structure.root_block_usage_key) + ) diff --git a/openedx/core/lib/block_structure/tests/test_factory.py b/openedx/core/lib/block_structure/tests/test_factory.py new file mode 100644 index 0000000000..c7c6e69c3d --- /dev/null +++ b/openedx/core/lib/block_structure/tests/test_factory.py @@ -0,0 +1,54 @@ +""" +Tests for block_structure_factory.py +""" +from unittest import TestCase +from xmodule.modulestore.exceptions import ItemNotFoundError + +from ..cache import BlockStructureCache +from ..factory import BlockStructureFactory +from .helpers import ( + MockCache, MockModulestoreFactory, ChildrenMapTestMixin +) + + +class TestBlockStructureFactory(TestCase, ChildrenMapTestMixin): + """ + Tests for BlockStructureFactory + """ + def setUp(self): + super(TestBlockStructureFactory, self).setUp() + self.children_map = self.SIMPLE_CHILDREN_MAP + self.modulestore = MockModulestoreFactory.create(self.children_map) + + def test_from_modulestore(self): + block_structure = BlockStructureFactory.create_from_modulestore( + root_block_usage_key=0, modulestore=self.modulestore + ) + self.assert_block_structure(block_structure, self.children_map) + + def test_from_modulestore_fail(self): + with self.assertRaises(ItemNotFoundError): + BlockStructureFactory.create_from_modulestore( + root_block_usage_key=len(self.children_map) + 1, + modulestore=self.modulestore, + ) + + def test_from_cache(self): + cache = BlockStructureCache(MockCache()) + block_structure = self.create_block_structure(self.children_map) + cache.add(block_structure) + from_cache_block_structure = BlockStructureFactory.create_from_cache( + block_structure.root_block_usage_key, + cache, + ) + self.assertIsNotNone(from_cache_block_structure) + self.assert_block_structure(from_cache_block_structure, self.children_map) + + def test_from_cache_none(self): + cache = BlockStructureCache(MockCache()) + self.assertIsNone( + BlockStructureFactory.create_from_cache( + root_block_usage_key=0, + block_structure_cache=cache, + ) + ) diff --git a/openedx/core/lib/block_structure/tests/test_manager.py b/openedx/core/lib/block_structure/tests/test_manager.py new file mode 100644 index 0000000000..e0abc45b61 --- /dev/null +++ b/openedx/core/lib/block_structure/tests/test_manager.py @@ -0,0 +1,145 @@ +""" +Tests for manager.py +""" +from unittest import TestCase + +from ..manager import BlockStructureManager +from ..transformers import BlockStructureTransformers +from .helpers import ( + MockModulestoreFactory, MockCache, MockTransformer, ChildrenMapTestMixin, mock_registered_transformers +) + + +class TestTransformer1(MockTransformer): + """ + Test Transformer class with basic functionality to verify collected and + transformed data. + """ + collect_data_key = 't1.collect' + transform_data_key = 't1.transform' + collect_call_count = 0 + + @classmethod + def collect(cls, block_structure): + """ + Collects block data for the block structure. + """ + cls._set_block_values(block_structure, cls.collect_data_key) + cls.collect_call_count += 1 + + def transform(self, usage_info, block_structure): + """ + Transforms the block structure. + """ + self._set_block_values(block_structure, self.transform_data_key) + + @classmethod + def assert_collected(cls, block_structure): + """ + Asserts data was collected for the block structure. + """ + cls._assert_block_values(block_structure, cls.collect_data_key) + + @classmethod + def assert_transformed(cls, block_structure): + """ + Asserts the block structure was transformed. + """ + cls._assert_block_values(block_structure, cls.transform_data_key) + + @classmethod + def _set_block_values(cls, block_structure, data_key): + """ + Sets a value for each block in the given structure, using the given + data key. + """ + for block_key in block_structure.topological_traversal(): + block_structure.set_transformer_block_field( + block_key, cls, data_key, cls._create_block_value(block_key, data_key) + ) + + @classmethod + def _assert_block_values(cls, block_structure, data_key): + """ + Verifies the value for each block in the given structure, for the given + data key. + """ + for block_key in block_structure.topological_traversal(): + assert ( + block_structure.get_transformer_block_field( + block_key, + cls, + data_key, + ) == cls._create_block_value(block_key, data_key) + ) + + @classmethod + def _create_block_value(cls, block_key, data_key): + """ + Returns a unique deterministic value for the given block key + and data key. + """ + return data_key + 't1.val1.' + unicode(block_key) + + +class TestBlockStructureManager(TestCase, ChildrenMapTestMixin): + """ + Test class for BlockStructureManager. + """ + def setUp(self): + super(TestBlockStructureManager, self).setUp() + + TestTransformer1.collect_call_count = 0 + self.registered_transformers = [TestTransformer1()] + with mock_registered_transformers(self.registered_transformers): + self.transformers = BlockStructureTransformers(self.registered_transformers) + + self.children_map = self.SIMPLE_CHILDREN_MAP + self.modulestore = MockModulestoreFactory.create(self.children_map) + self.cache = MockCache() + self.bs_manager = BlockStructureManager( + root_block_usage_key=0, + modulestore=self.modulestore, + cache=self.cache, + ) + + def collect_and_verify(self, expect_modulestore_called, expect_cache_updated): + """ + Calls the manager's get_collected method and verifies its result + and behavior. + """ + self.modulestore.get_items_call_count = 0 + self.cache.set_call_count = 0 + with mock_registered_transformers(self.registered_transformers): + block_structure = self.bs_manager.get_collected() + self.assert_block_structure(block_structure, self.children_map) + TestTransformer1.assert_collected(block_structure) + if expect_modulestore_called: + self.assertGreater(self.modulestore.get_items_call_count, 0) + else: + self.assertEquals(self.modulestore.get_items_call_count, 0) + self.assertEquals(self.cache.set_call_count, 1 if expect_cache_updated else 0) + + def test_get_transformed(self): + with mock_registered_transformers(self.registered_transformers): + block_structure = self.bs_manager.get_transformed(self.transformers) + self.assert_block_structure(block_structure, self.children_map) + TestTransformer1.assert_collected(block_structure) + TestTransformer1.assert_transformed(block_structure) + + 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) + self.assertEquals(TestTransformer1.collect_call_count, 1) + + def test_get_collected_outdated_data(self): + self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) + TestTransformer1.VERSION += 1 + self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) + self.assertEquals(TestTransformer1.collect_call_count, 2) + + def test_clear(self): + self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) + self.bs_manager.clear() + self.collect_and_verify(expect_modulestore_called=True, expect_cache_updated=True) + self.assertEquals(TestTransformer1.collect_call_count, 2) diff --git a/openedx/core/lib/block_cache/tests/test_transformer_registry.py b/openedx/core/lib/block_structure/tests/test_transformer_registry.py similarity index 75% rename from openedx/core/lib/block_cache/tests/test_transformer_registry.py rename to openedx/core/lib/block_structure/tests/test_transformer_registry.py index fc7beeb3dc..008ed8f3bc 100644 --- a/openedx/core/lib/block_cache/tests/test_transformer_registry.py +++ b/openedx/core/lib/block_structure/tests/test_transformer_registry.py @@ -3,11 +3,10 @@ Tests for transformer_registry.py """ import ddt -from mock import patch from unittest import TestCase from ..transformer_registry import TransformerRegistry -from .test_utils import MockTransformer +from .helpers import MockTransformer, mock_registered_transformers class TestTransformer1(MockTransformer): @@ -55,14 +54,8 @@ class TransformerRegistryTestCase(TestCase): @ddt.unpack def test_find_unregistered(self, transformers, expected_unregistered): - with ( - patch('openedx.core.lib.block_cache.transformer_registry.TransformerRegistry.get_available_plugins') - ) as mock_registry: - mock_registry.return_value = { - transformer.name(): transformer - for transformer in [TestTransformer1, TestTransformer2] - } - + with mock_registered_transformers([TestTransformer1, TestTransformer2]): self.assertSetEqual( - TransformerRegistry.find_unregistered(transformers), set(expected_unregistered) + TransformerRegistry.find_unregistered(transformers), + set(expected_unregistered), ) diff --git a/openedx/core/lib/block_structure/tests/test_transformers.py b/openedx/core/lib/block_structure/tests/test_transformers.py new file mode 100644 index 0000000000..2b77ab16e3 --- /dev/null +++ b/openedx/core/lib/block_structure/tests/test_transformers.py @@ -0,0 +1,73 @@ +""" +Tests for transformers.py +""" +from mock import MagicMock, patch +from unittest import TestCase + +from ..block_structure import BlockStructureModulestoreData +from ..exceptions import TransformerException +from ..transformers import BlockStructureTransformers +from .helpers import ( + ChildrenMapTestMixin, MockTransformer, mock_registered_transformers +) + + +class TestBlockStructureTransformers(ChildrenMapTestMixin, TestCase): + """ + Test class for testing BlockStructureTransformers + """ + class UnregisteredTransformer(MockTransformer): + """ + Mock transformer that is not registered. + """ + pass + + def setUp(self): + super(TestBlockStructureTransformers, self).setUp() + self.transformers = BlockStructureTransformers(usage_info=MagicMock()) + self.registered_transformers = [MockTransformer] + + def add_mock_transformer(self): + """ + Adds the registered transformers to the self.transformers collection. + """ + with mock_registered_transformers(self.registered_transformers): + self.transformers += self.registered_transformers + + def test_add_registered(self): + self.add_mock_transformer() + self.assertIn(MockTransformer, self.transformers._transformers) # pylint: disable=protected-access + + def test_add_unregistered(self): + with self.assertRaises(TransformerException): + self.transformers += [self.UnregisteredTransformer] + + self.assertEquals(self.transformers._transformers, []) # pylint: disable=protected-access + + def test_collect(self): + with mock_registered_transformers(self.registered_transformers): + with patch( + 'openedx.core.lib.block_structure.tests.helpers.MockTransformer.collect' + ) as mock_collect_call: + self.transformers.collect(block_structure=MagicMock()) + self.assertTrue(mock_collect_call.called) + + def test_transform(self): + self.add_mock_transformer() + + with patch( + 'openedx.core.lib.block_structure.tests.helpers.MockTransformer.transform' + ) as mock_transform_call: + self.transformers.transform(block_structure=MagicMock()) + self.assertTrue(mock_transform_call.called) + + def test_is_collected_outdated(self): + block_structure = self.create_block_structure( + self.SIMPLE_CHILDREN_MAP, + BlockStructureModulestoreData + ) + + with mock_registered_transformers(self.registered_transformers): + self.assertTrue(self.transformers.is_collected_outdated(block_structure)) + self.transformers.collect(block_structure) + self.assertFalse(self.transformers.is_collected_outdated(block_structure)) diff --git a/openedx/core/lib/block_cache/tests/test_utils.py b/openedx/core/lib/block_structure/tests/test_utils.py similarity index 85% rename from openedx/core/lib/block_cache/tests/test_utils.py rename to openedx/core/lib/block_structure/tests/test_utils.py index 114673a0fe..c37001774e 100644 --- a/openedx/core/lib/block_cache/tests/test_utils.py +++ b/openedx/core/lib/block_structure/tests/test_utils.py @@ -1,7 +1,11 @@ """ -Common utilities for tests in block_cache module +Common utilities for tests in block_structure module """ -# pylint: disable=protected-access +from contextlib import contextmanager +from mock import patch +from xmodule.modulestore.exceptions import ItemNotFoundError + +from ..block_structure import BlockStructureBlockData from ..transformer import BlockStructureTransformer @@ -55,9 +59,14 @@ class MockModulestore(object): """ Returns the mock XBlock (MockXBlock) associated with the given block_key. + + Raises ItemNotFoundError if the item is not found. """ self.get_items_call_count += 1 - return self.blocks.get(block_key) + item = self.blocks.get(block_key) + if not item: + raise ItemNotFoundError + return item class MockCache(object): @@ -68,11 +77,13 @@ class MockCache(object): def __init__(self): # An in-memory map of cache keys to cache values. self.map = {} + self.set_call_count = 0 def set(self, key, val): """ Associates the given key with the given value in the cache. """ + self.set_call_count += 1 self.map[key] = val def get(self, key, default=None): @@ -82,20 +93,6 @@ class MockCache(object): """ return self.map.get(key, default) - def set_many(self, map_): - """ - For each dictionary entry in the given map, updates the cache - with that entry. - """ - for key, val in map_.iteritems(): - self.set(key, val) - - def get_many(self, keys): - """ - Returns a dictionary of entries for each key found in the cache. - """ - return {key: self.map[key] for key in keys if key in self.map} - def delete(self, key): """ Deletes the given key from the cache. @@ -141,6 +138,18 @@ class MockTransformer(BlockStructureTransformer): pass +@contextmanager +def mock_registered_transformers(transformers): + """ + Context manager for mocking the transformer registry to return the given transformers. + """ + with patch( + 'openedx.core.lib.block_structure.transformer_registry.TransformerRegistry.get_registered_transformers' + ) as mock_available_transforms: + mock_available_transforms.return_value = {transformer for transformer in transformers} + yield + + class ChildrenMapTestMixin(object): """ A Test Mixin with utility methods for testing with block structures @@ -172,7 +181,7 @@ class ChildrenMapTestMixin(object): # 5 6 DAG_CHILDREN_MAP = [[1, 2], [3], [3, 4], [5, 6], [], [], []] - def create_block_structure(self, block_structure_cls, children_map): + def create_block_structure(self, children_map, block_structure_cls=BlockStructureBlockData): """ Factory method for creating and returning a block structure for the given children_map. @@ -183,7 +192,7 @@ class ChildrenMapTestMixin(object): # _add_relation for parent, children in enumerate(children_map): for child in children: - block_structure._add_relation(parent, child) + block_structure._add_relation(parent, child) # pylint: disable=protected-access return block_structure def get_parents_map(self, children_map): diff --git a/openedx/core/lib/block_cache/transformer.py b/openedx/core/lib/block_structure/transformer.py similarity index 97% rename from openedx/core/lib/block_cache/transformer.py rename to openedx/core/lib/block_structure/transformer.py index 1e175be17a..54904e9dbf 100644 --- a/openedx/core/lib/block_cache/transformer.py +++ b/openedx/core/lib/block_structure/transformer.py @@ -14,7 +14,7 @@ class BlockStructureTransformer(object): # attribute. While the value for the base class is set to 0, # the value for each concrete transformer should be 1 or higher. # - # A transformer's version attribute is used by the block_cache + # A transformer's version attribute is used by the block_structure # framework in order to determine whether any collected data for a # transformer is outdated. When a transformer's data is collected # and cached, it's version number at the time of collection is @@ -115,7 +115,7 @@ class BlockStructureTransformer(object): Arguments: usage_info (any negotiated type) - A usage-specific object - that is passed to the block_cache and forwarded to all + that is passed to the block_structure and forwarded to all requested Transformers in order to apply a usage-specific transform. For example, an instance of usage_info would contain a user object for which the diff --git a/openedx/core/lib/block_cache/transformer_registry.py b/openedx/core/lib/block_structure/transformer_registry.py similarity index 86% rename from openedx/core/lib/block_cache/transformer_registry.py rename to openedx/core/lib/block_structure/transformer_registry.py index a24ba72214..98547357b6 100644 --- a/openedx/core/lib/block_cache/transformer_registry.py +++ b/openedx/core/lib/block_structure/transformer_registry.py @@ -14,6 +14,7 @@ class TransformerRegistry(PluginManager): `BlockStructureTransformer`. """ NAMESPACE = 'openedx.block_structure_transformer' + USE_PLUGIN_MANAGER = True @classmethod def get_registered_transformers(cls): @@ -24,7 +25,10 @@ class TransformerRegistry(PluginManager): {BlockStructureTransformer} - All transformers that are registered with the platform's PluginManager. """ - return set(cls.get_available_plugins().itervalues()) + if cls.USE_PLUGIN_MANAGER: + return set(cls.get_available_plugins().itervalues()) + else: + return set() @classmethod def find_unregistered(cls, transformers): @@ -38,7 +42,7 @@ class TransformerRegistry(PluginManager): transformers to check in the registry. Returns: - [string] - The names of a subset of the given + set([string]) - Set of names of a subset of the given transformers that weren't found in the registry. """ registered_transformer_names = set(reg_trans.name() for reg_trans in cls.get_registered_transformers()) diff --git a/openedx/core/lib/block_structure/transformers.py b/openedx/core/lib/block_structure/transformers.py new file mode 100644 index 0000000000..493dcebde2 --- /dev/null +++ b/openedx/core/lib/block_structure/transformers.py @@ -0,0 +1,107 @@ +""" +Module for a collection of BlockStructureTransformers. +""" +from logging import getLogger + +from .exceptions import TransformerException +from .transformer_registry import TransformerRegistry + + +logger = getLogger(__name__) # pylint: disable=C0103 + + +class BlockStructureTransformers(object): + """ + The BlockStructureTransformers class encapsulates an ordered list of block + structure transformers. It uses the Transformer Registry to verify the + the registration status of added transformers and to collect their data. + It provides aggregate functionality for collection and ordered + transformation of the transformers. + + Clients are expected to access the list of transformers through the + class' interface rather than directly. + """ + def __init__(self, transformers=None, usage_info=None): + """ + Arguments: + transformers ([BlockStructureTransformer]) - List of transformers + to add to the collection. + + usage_info (any negotiated type) - A usage-specific object + that is passed to the block_structure and forwarded to all + requested Transformers in order to apply a + usage-specific transform. For example, an instance of + usage_info would contain a user object for which the + transform should be applied. + + Raises: + TransformerException - if any transformer is not registered in the + Transformer Registry. + """ + self.usage_info = usage_info + self._transformers = [] + if transformers: + self.__iadd__(transformers) + + def __iadd__(self, transformers): + """ + Adds the given transformers to the collection. + + Args: + transformers ([BlockStructureTransformer]) - List of transformers + to add to the collection. + + Raises: + TransformerException - if any transformer is not registered in the + Transformer Registry. + """ + unregistered_transformers = TransformerRegistry.find_unregistered(transformers) + if unregistered_transformers: + raise TransformerException( + "The following requested transformers are not registered: {}".format(unregistered_transformers) + ) + + self._transformers.extend(transformers) + return self + + @classmethod + def collect(cls, block_structure): + """ + Collects data for each registered transformer. + """ + for transformer in TransformerRegistry.get_registered_transformers(): + block_structure._add_transformer(transformer) # pylint: disable=protected-access + transformer.collect(block_structure) + + # Collect all fields that were requested by the transformers. + block_structure._collect_requested_xblock_fields() # pylint: disable=protected-access + + def transform(self, block_structure): + """ + The given block structure is transformed by each transformer in the + collection, in the order that the transformers were added. + """ + for transformer in self._transformers: + transformer.transform(self.usage_info, block_structure) + + # Prune the block structure to remove any unreachable blocks. + block_structure._prune_unreachable() # pylint: disable=protected-access + + @classmethod + def is_collected_outdated(cls, block_structure): + """ + Returns whether the collected data in the block structure is outdated. + """ + outdated_transformers = [] + for transformer in TransformerRegistry.get_registered_transformers(): + version_in_block_structure = block_structure._get_transformer_data_version(transformer) # pylint: disable=protected-access + if transformer.VERSION != version_in_block_structure: + outdated_transformers.append(transformer) + + if outdated_transformers: + logger.debug( + "Collected Block Structure data for the following transformers is outdated: '%s'.", + [(transformer.name(), transformer.VERSION) for transformer in outdated_transformers], + ) + + return bool(outdated_transformers)