fixup! block_cache fix pickling issue.

openedx/core/lib/block_cache/block_structure.py

@@ -13,18 +13,19 @@ logger = getLogger(__name__)  # pylint: disable=C0103
 TRANSFORMER_VERSION_KEY = '_version'
 
 
+class BlockRelations(object):
+    def __init__(self):
+        self.parents = []
+        self.children = []
+
+
 class BlockStructure(object):
     """
     A class to encapsulate a structure of blocks, a directed acyclic graph of blocks.
     """
-    class BlockRelations(object):
-        def __init__(self):
-            self.parents = []
-            self.children = []
-
     def __init__(self, root_block_key):
         self.root_block_key = root_block_key
-        self._block_relations = defaultdict(self.BlockRelations)
+        self._block_relations = defaultdict(BlockRelations)
         self._add_block(self._block_relations, root_block_key)
 
     def __iter__(self):
@@ -62,7 +63,7 @@ class BlockStructure(object):
 
     def prune(self):
         # create a new block relations map with only those blocks that are still linked
-        pruned_block_relations = defaultdict(self.BlockRelations)
+        pruned_block_relations = defaultdict(BlockRelations)
         old_block_relations = self._block_relations
 
         def do_for_each_block(block_key):
@@ -86,23 +87,24 @@ class BlockStructure(object):
         _ = block_relations[block_key]
 
 
+class BlockData(object):
+    def __init__(self):
+        # dictionary mapping xblock field names to their values.
+        self._xblock_fields = {}
+
+        # dictionary mapping transformers' IDs to their collected data.
+        self._transformer_data = defaultdict(dict)
+
+
 class BlockStructureBlockData(BlockStructure):
     """
     A sub-class of BlockStructure that encapsulates data captured about the blocks.
     """
-    class BlockData(object):
-        def __init__(self):
-            # dictionary mapping xblock field names to their values.
-            self._xblock_fields = {}
-
-            # dictionary mapping transformers' IDs to their collected data.
-            self._transformer_data = defaultdict(dict)
-
     def __init__(self, root_block_key):
         super(BlockStructureBlockData, self).__init__(root_block_key)
 
         # dictionary mapping usage keys to BlockData
-        self._block_data_map = defaultdict(self.BlockData)
+        self._block_data_map = defaultdict(BlockData)
 
         # dictionary mapping transformer IDs to block-structure-wide transformer data
         self._transformer_data = defaultdict(dict)
@@ -247,14 +249,22 @@ class BlockStructureFactory(object):
             block_structure._block_relations = block_relations
             block_structure._transformer_data = transformer_data
 
-            if all(
-                transformer.VERSION == block_structure.get_transformer_data_version(transformer)
-                for transformer in BlockStructureTransformers.get_registered_transformers()
-            ):
+            transformer_issues = {}
+            for transformer in BlockStructureTransformers.get_registered_transformers():
+                cached_transformer_version = block_structure.get_transformer_data_version(transformer)
+                if transformer.VERSION != cached_transformer_version:
+                    transformer_issues[transformer.name()] = "version: {}, cached: {}".format(
+                        transformer.VERSION,
+                        cached_transformer_version,
+                    )
+
+            if not transformer_issues:
                 block_structure._block_data_map = cache.get_many(block_relations.iterkeys())
                 return block_structure
             else:
-                logger.info("Collected data for transformer is outdated.")
+                logger.info(
+                    "Collected data for the following transformers have issues:\n{}."
+                ).format('\n'.join([t_name + ": " + t_value for t_name, t_value in transformer_issues.iteritems()]))
 
         return None
 

openedx/core/lib/block_cache/tests/test_block_cache.py

@@ -3,8 +3,9 @@ Tests for block_cache.py
 """
 
 from django.core.cache import get_cache
-from mock import patch, DEFAULT
+from mock import patch
 from unittest import TestCase
+
 from .test_utils import (
     MockModulestoreFactory, MockCache, MockUserInfo, MockTransformer, ChildrenMapTestMixin
 )
@@ -81,5 +82,5 @@ class TestBlockCache(TestCase, ChildrenMapTestMixin):
             self.assert_block_structure(block_structure, self.children_map)
             if iteration == 0:
                 self.assertTrue(self.modulestore.get_items_call_count > 0)
-            # else:  TODO - debug issue with pickling
-            #     self.assertEquals(self.modulestore.get_items_call_count, 0)
+            else:
+                self.assertEquals(self.modulestore.get_items_call_count, 0)

openedx/core/lib/block_cache/tests/test_block_structure.py

@@ -225,37 +225,60 @@ class TestBlockStructureFactory(TestCase, ChildrenMapTestMixin):
     """
     Tests for BlockStructureFactory
     """
-    def test_factory_methods(self):
-        children_map = self.SIMPLE_CHILDREN_MAP
-        modulestore = MockModulestoreFactory.create(children_map)
-        cache = MockCache()
+    def setUp(self):
+        super(TestBlockStructureFactory, self).setUp()
+        self.children_map = self.SIMPLE_CHILDREN_MAP
+        self.modulestore = MockModulestoreFactory.create(self.children_map)
 
-        # test create from modulestore
-        block_structure = BlockStructureFactory.create_from_modulestore(root_block_key=0, modulestore=modulestore)
-        self.assert_block_structure(block_structure, children_map)
+        self.block_structure = BlockStructureFactory.create_from_modulestore(
+            root_block_key=0, modulestore=self.modulestore
+        )
 
-        # test not in cache
-        self.assertIsNone(BlockStructureFactory.create_from_cache(root_block_key=0, cache=cache))
+    def add_transformers(self):
+        """
+        Add each registered transformer to the block structure.
+        """
+        for transformer in BlockStructureTransformers.get_registered_transformers():
+            self.block_structure.add_transformer(transformer)
+            self.block_structure.set_transformer_block_data(
+                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)
 
-        # test transformers outdated
-        BlockStructureFactory.serialize_to_cache(block_structure, cache)
+    def test_uncollected_transformers(self):
+        cache = MockCache()
+
+        BlockStructureFactory.serialize_to_cache(self.block_structure, cache)
         with patch('openedx.core.lib.block_cache.block_structure.logger.info') as mock_logger:
+            # cached data does not have collected information for all registered transformers
             self.assertIsNone(BlockStructureFactory.create_from_cache(root_block_key=0, cache=cache))
             self.assertTrue(mock_logger.called)
 
-        # update transformers
-        for transformer in BlockStructureTransformers.get_registered_transformers():
-            block_structure.add_transformer(transformer)
-            block_structure.set_transformer_block_data(
-                usage_key=0, transformer=transformer, key='test', value='test.val'
-            )
-        BlockStructureFactory.serialize_to_cache(block_structure, cache)
+    def test_not_in_cache(self):
+        cache = MockCache()
+
+        self.assertIsNone(BlockStructureFactory.create_from_cache(root_block_key=0, cache=cache))
+
+    def test_cache(self):
+        cache = MockCache()
+        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_key=0, cache=cache)
         self.assertIsNotNone(from_cache_block_structure)
-        self.assert_block_structure(from_cache_block_structure, children_map)
+        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()
+        self.add_transformers()
 
-        # test remove from cache
+        BlockStructureFactory.serialize_to_cache(self.block_structure, cache)
         BlockStructureFactory.remove_from_cache(root_block_key=0, cache=cache)
         self.assertIsNone(BlockStructureFactory.create_from_cache(root_block_key=0, cache=cache))