Allow MongoModuleStore to use an existing CachingDescriptorSystem.

Prior to this change, if a CachingDescriptorSystem was prepopulated
with XBlock field data (`module_data`), then it would be used as
the DescriptorSystem for blocks loaded through it. On the other hand,
if it had to fetch new data from the modulestore, then a new
CachingDescriptorSystem would be created by the modulestore, and used
by the newly returned object.

This commit allows the modulestore get_item and get_items methods
to accept an existing CachingDescriptorSystem to augment with data
from the backing mongo store, and to use as the DescriptorSystem
for newly loaded XBlocks.

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -693,7 +693,7 @@ class ModuleStoreRead(ModuleStoreAssetBase):
         pass
 
     @abstractmethod
-    def get_item(self, usage_key, depth=0, **kwargs):
+    def get_item(self, usage_key, depth=0, using_descriptor_system=None, **kwargs):
         """
         Returns an XModuleDescriptor instance for the item at location.
 

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -218,10 +218,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin):
         assert isinstance(location, UsageKey)
         json_data = self.module_data.get(location)
         if json_data is None:
-            module = self.modulestore.get_item(location)
-            if module is not None:
-                # update our own cache after going to the DB to get cache miss
-                self.module_data.update(module.runtime.module_data)
+            module = self.modulestore.get_item(location, using_descriptor_system=self)
             return module
         else:
             # load the module and apply the inherited metadata
@@ -855,9 +852,19 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
 
         return data
 
-    def _load_item(self, course_key, item, data_cache, apply_cached_metadata=True):
+    def _load_item(self, course_key, item, data_cache, apply_cached_metadata=True, using_descriptor_system=None):
         """
         Load an XModuleDescriptor from item, using the children stored in data_cache
+
+        Arguments:
+            course_key (CourseKey): which course to load from
+            item (UsageKey): which xblock to load
+            data_cache (dict): A dictionary mapping from UsageKeys to xblock field data
+                (this is the xblock data loaded from the database)
+            apply_cached_metadata (bool): Whether to use the cached metadata for inheritance
+                purposes.
+            using_descriptor_system (CachingDescriptorSystem): The existing CachingDescriptorSystem
+                to add data to, and to load the XBlocks from.
         """
         course_key = self.fill_in_run(course_key)
         location = Location._from_deprecated_son(item['location'], course_key.run)
@@ -869,32 +876,38 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
         if apply_cached_metadata:
             cached_metadata = self._get_cached_metadata_inheritance_tree(course_key)
 
-        services = {}
-        if self.i18n_service:
-            services["i18n"] = self.i18n_service
-
-        if self.fs_service:
-            services["fs"] = self.fs_service
-
-        if self.user_service:
-            services["user"] = self.user_service
-
-        system = CachingDescriptorSystem(
-            modulestore=self,
-            course_key=course_key,
-            module_data=data_cache,
-            default_class=self.default_class,
-            resources_fs=resource_fs,
-            error_tracker=self.error_tracker,
-            render_template=self.render_template,
-            cached_metadata=cached_metadata,
-            mixins=self.xblock_mixins,
-            select=self.xblock_select,
-            services=services,
-        )
+        if using_descriptor_system is None:
+            services = {}
+            if self.i18n_service:
+                services["i18n"] = self.i18n_service
+
+            if self.fs_service:
+                services["fs"] = self.fs_service
+
+            if self.user_service:
+                services["user"] = self.user_service
+
+            system = CachingDescriptorSystem(
+                modulestore=self,
+                course_key=course_key,
+                module_data=data_cache,
+                default_class=self.default_class,
+                resources_fs=resource_fs,
+                error_tracker=self.error_tracker,
+                render_template=self.render_template,
+                cached_metadata=cached_metadata,
+                mixins=self.xblock_mixins,
+                select=self.xblock_select,
+                services=services,
+            )
+        else:
+            system = using_descriptor_system
+            system.module_data.update(data_cache)
+            system.cached_metadata.update(cached_metadata)
+
         return system.load_item(location)
 
-    def _load_items(self, course_key, items, depth=0):
+    def _load_items(self, course_key, items, depth=0, using_descriptor_system=None):
         """
         Load a list of xmodules from the data in items, with children cached up
         to specified depth
@@ -906,8 +919,11 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
         # bother with the metadata inheritance
         return [
             self._load_item(
-                course_key, item, data_cache,
-                apply_cached_metadata=(item['location']['category'] != 'course' or depth != 0)
+                course_key,
+                item,
+                data_cache,
+                apply_cached_metadata=(item['location']['category'] != 'course' or depth != 0),
+                using_descriptor_system=using_descriptor_system
             )
             for item in items
         ]
@@ -1006,7 +1022,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
         except ItemNotFoundError:
             return False
 
-    def get_item(self, usage_key, depth=0):
+    def get_item(self, usage_key, depth=0, using_descriptor_system=None):
         """
         Returns an XModuleDescriptor instance for the item at location.
 
@@ -1015,14 +1031,22 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
         If no object is found at that location, raises
             xmodule.modulestore.exceptions.ItemNotFoundError
 
-        usage_key: a :class:`.UsageKey` instance
-        depth (int): An argument that some module stores may use to prefetch
-            descendents of the queried modules for more efficient results later
-            in the request. The depth is counted in the number of
-            calls to get_children() to cache. None indicates to cache all descendents.
+        Arguments:
+            usage_key: a :class:`.UsageKey` instance
+            depth (int): An argument that some module stores may use to prefetch
+                descendents of the queried modules for more efficient results later
+                in the request. The depth is counted in the number of
+                calls to get_children() to cache. None indicates to cache all descendents.
+            using_descriptor_system (CachingDescriptorSystem): The existing CachingDescriptorSystem
+                to add data to, and to load the XBlocks from.
         """
         item = self._find_one(usage_key)
-        module = self._load_items(usage_key.course_key, [item], depth)[0]
+        module = self._load_items(
+            usage_key.course_key,
+            [item],
+            depth,
+            using_descriptor_system=using_descriptor_system
+        )[0]
         return module
 
     @staticmethod
@@ -1054,6 +1078,7 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
             content=None,
             key_revision=MongoRevisionKey.published,
             qualifiers=None,
+            using_descriptor_system=None,
             **kwargs
     ):
         """
@@ -1085,6 +1110,8 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
                 For this modulestore, ``name`` is a commonly provided key (Location based stores)
                 This modulestore does not allow searching dates by comparison or edited_by, previous_version,
                 update_version info.
+            using_descriptor_system (CachingDescriptorSystem): The existing CachingDescriptorSystem
+                to add data to, and to load the XBlocks from.
         """
         qualifiers = qualifiers.copy() if qualifiers else {}  # copy the qualifiers (destructively manipulated here)
         query = self._course_key_to_son(course_id)
@@ -1106,7 +1133,11 @@ class MongoModuleStore(ModuleStoreDraftAndPublished, ModuleStoreWriteBase, Mongo
             sort=[SORT_REVISION_FAVOR_DRAFT],
         )
 
-        modules = self._load_items(course_id, list(items))
+        modules = self._load_items(
+            course_id,
+            list(items),
+            using_descriptor_system=using_descriptor_system
+        )
         return modules
 
     def create_course(self, org, course, run, user_id, fields=None, **kwargs):