Merge pull request #1511 from MITx/feature/cdodge/metadata-inheritence-crawling

Feature/cdodge/metadata inheritence crawling

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -264,6 +264,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         self.assertContains(resp, '/c4x/edX/full/asset/handouts_schematic_tutorial.pdf')
 
 
+
 class ContentStoreTest(ModuleStoreTestCase):
     """
     Tests for the CMS ContentStore application.
@@ -422,6 +423,52 @@ class ContentStoreTest(ModuleStoreTestCase):
         self.assertNotIn('markdown', problem.editable_metadata_fields, "Markdown slipped into the editable metadata fields")
 
 
+    def test_metadata_inheritance(self):
+        import_from_xml(modulestore(), 'common/test/data/', ['full'])
+
+        ms = modulestore('direct')
+        course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None]))
+
+        verticals = ms.get_items(['i4x', 'edX', 'full', 'vertical', None, None])
+
+        # let's assert on the metadata_inheritance on an existing vertical
+        for vertical in verticals:
+            self.assertIn('xqa_key', vertical.metadata)
+            self.assertEqual(course.metadata['xqa_key'], vertical.metadata['xqa_key'])
+
+        self.assertGreater(len(verticals), 0)
+
+        new_component_location = Location('i4x', 'edX', 'full', 'html', 'new_component')
+        source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Empty')
+        
+        # crate a new module and add it as a child to a vertical
+        ms.clone_item(source_template_location, new_component_location)
+        parent = verticals[0]
+        ms.update_children(parent.location, parent.definition.get('children', []) + [new_component_location.url()])
+
+        # flush the cache
+        ms.get_cached_metadata_inheritance_tree(new_component_location, -1)
+        new_module = ms.get_item(new_component_location)
+
+        # check for grace period definition which should be defined at the course level
+        self.assertIn('graceperiod', new_module.metadata)
+
+        self.assertEqual(course.metadata['graceperiod'], new_module.metadata['graceperiod'])
+
+        #
+        # now let's define an override at the leaf node level
+        #
+        new_module.metadata['graceperiod'] = '1 day'
+        ms.update_metadata(new_module.location, new_module.metadata)
+
+        # flush the cache and refetch
+        ms.get_cached_metadata_inheritance_tree(new_component_location, -1)
+        new_module = ms.get_item(new_component_location)
+
+        self.assertIn('graceperiod', new_module.metadata)
+        self.assertEqual('1 day', new_module.metadata['graceperiod'])
+
+
 class TemplateTestCase(ModuleStoreTestCase):
 
     def test_template_cleanup(self):        

common/lib/xmodule/xmodule/mako_module.py

@@ -44,5 +44,6 @@ class MakoModuleDescriptor(XModuleDescriptor):
     # cdodge: encapsulate a means to expose "editable" metadata fields (i.e. not internal system metadata)
     @property
     def editable_metadata_fields(self):
-        subset = [name for name in self.metadata.keys() if name not in self.system_metadata_fields]
+        subset = [name for name in self.metadata.keys() if name not in self.system_metadata_fields and 
+            name not in self._inherited_metadata]
         return subset

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

@@ -1,11 +1,13 @@
 import pymongo
 import sys
 import logging
+import copy
 
 from bson.son import SON
 from fs.osfs import OSFS
 from itertools import repeat
 from path import path
+from datetime import datetime, timedelta
 
 from importlib import import_module
 from xmodule.errortracker import null_error_tracker, exc_info_to_str
@@ -27,9 +29,11 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
     """
     A system that has a cache of module json that it will use to load modules
     from, with a backup of calling to the underlying modulestore for more data
+    TODO (cdodge) when the 'split module store' work has been completed we can remove all
+    references to metadata_inheritance_tree
     """
     def __init__(self, modulestore, module_data, default_class, resources_fs,
-                 error_tracker, render_template):
+                 error_tracker, render_template, metadata_inheritance_tree = None):
         """
         modulestore: the module store that can be used to retrieve additional modules
 
@@ -54,6 +58,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
         # cdodge: other Systems have a course_id attribute defined. To keep things consistent, let's
         # define an attribute here as well, even though it's None
         self.course_id = None
+        self.metadata_inheritance_tree = metadata_inheritance_tree
 
     def load_item(self, location):
         location = Location(location)
@@ -61,11 +66,13 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
         if json_data is None:
             return self.modulestore.get_item(location)
         else:
-            # TODO (vshnayder): metadata inheritance is somewhat broken because mongo, doesn't
-            # always load an entire course.  We're punting on this until after launch, and then
-            # will build a proper course policy framework.
+            # load the module and apply the inherited metadata
             try:
-                return XModuleDescriptor.load_from_json(json_data, self, self.default_class)
+                module = XModuleDescriptor.load_from_json(json_data, self, self.default_class)
+                if self.metadata_inheritance_tree is not None:
+                    metadata_to_inherit = self.metadata_inheritance_tree.get('parent_metadata', {}).get(location.url(),{})
+                    module.inherit_metadata(metadata_to_inherit)
+                return module
             except:
                 return ErrorDescriptor.from_json(
                     json_data,
@@ -142,6 +149,82 @@ class MongoModuleStore(ModuleStoreBase):
         self.fs_root = path(fs_root)
         self.error_tracker = error_tracker
         self.render_template = render_template
+        self.metadata_inheritance_cache = {}
+
+    def get_metadata_inheritance_tree(self, location):
+        '''
+        TODO (cdodge) This method can be deleted when the 'split module store' work has been completed
+        '''
+        
+        # get all collections in the course, this query should not return any leaf nodes
+        query = { '_id.org' : location.org,
+                '_id.course' : location.course,
+                '_id.revision' : None,
+                'definition.children':{'$ne': []}
+                }
+        # we just want the Location, children, and metadata
+        record_filter = {'_id':1,'definition.children':1,'metadata':1}
+
+        # call out to the DB
+        resultset = self.collection.find(query, record_filter)
+
+        results_by_url = {}
+        root = None
+
+        # now go through the results and order them by the location url
+        for result in resultset:
+            location = Location(result['_id'])
+            results_by_url[location.url()] = result
+            if location.category == 'course':
+                root = location.url()
+
+        # now traverse the tree and compute down the inherited metadata
+        metadata_to_inherit = {}
+        def _compute_inherited_metadata(url):
+            my_metadata = results_by_url[url]['metadata']
+            for key in my_metadata.keys():
+                if key not in XModuleDescriptor.inheritable_metadata:
+                    del my_metadata[key]
+            results_by_url[url]['metadata'] = my_metadata
+
+            # go through all the children and recurse, but only if we have
+            # in the result set. Remember results will not contain leaf nodes
+            for child in results_by_url[url].get('definition',{}).get('children',[]):
+                if child in results_by_url:
+                    new_child_metadata = copy.deepcopy(my_metadata)
+                    new_child_metadata.update(results_by_url[child]['metadata'])
+                    results_by_url[child]['metadata'] = new_child_metadata
+                    metadata_to_inherit[child] = new_child_metadata
+                    _compute_inherited_metadata(child)
+                else:
+                    # this is likely a leaf node, so let's record what metadata we need to inherit
+                    metadata_to_inherit[child] = my_metadata
+        
+        if root is not None:
+            _compute_inherited_metadata(root)
+
+        cache = {'parent_metadata': metadata_to_inherit, 
+            'timestamp' : datetime.now()}
+
+        return cache
+
+    def get_cached_metadata_inheritance_tree(self, location, max_age_allowed):
+        '''
+        TODO (cdodge) This method can be deleted when the 'split module store' work has been completed
+        '''
+        cache_name = '{0}/{1}'.format(location.org, location.course)
+        cache = self.metadata_inheritance_cache.get(cache_name,{'parent_metadata': {}, 
+            'timestamp': datetime.now() - timedelta(hours=1)})
+        age = (datetime.now() - cache['timestamp'])
+
+        if age.seconds >= max_age_allowed:
+            logging.debug('loading entire inheritance tree for {0}'.format(cache_name))
+            cache = self.get_metadata_inheritance_tree(location)
+            self.metadata_inheritance_cache[cache_name] = cache
+
+        return cache
+
+
 
     def _clean_item_data(self, item):
         """
@@ -196,6 +279,8 @@ class MongoModuleStore(ModuleStoreBase):
 
         resource_fs = OSFS(root)
 
+        # TODO (cdodge): When the 'split module store' work has been completed, we should remove
+        # the 'metadata_inheritance_tree' parameter
         system = CachingDescriptorSystem(
             self,
             data_cache,
@@ -203,6 +288,7 @@ class MongoModuleStore(ModuleStoreBase):
             resource_fs,
             self.error_tracker,
             self.render_template,
+            metadata_inheritance_tree = self.get_cached_metadata_inheritance_tree(Location(item['location']), 60)
         )
         return system.load_item(item['location'])
 
@@ -261,11 +347,11 @@ class MongoModuleStore(ModuleStoreBase):
             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.
-
         """
         location = Location.ensure_fully_specified(location)
         item = self._find_one(location)
-        return self._load_items([item], depth)[0]
+        module = self._load_items([item], depth)[0]
+        return module
 
     def get_instance(self, course_id, location, depth=0):
         """
@@ -285,7 +371,8 @@ class MongoModuleStore(ModuleStoreBase):
             sort=[('revision', pymongo.ASCENDING)],
         )
 
-        return self._load_items(list(items), depth)
+        modules = self._load_items(list(items), depth)
+        return modules
 
     def clone_item(self, source, location):
         """
@@ -313,7 +400,7 @@ class MongoModuleStore(ModuleStoreBase):
             raise DuplicateItemError(location)
 
 
-    def get_course_for_item(self, location):
+    def get_course_for_item(self, location, depth=0):
         '''
         VS[compat]
         cdodge: for a given Xmodule, return the course that it belongs to
@@ -327,7 +414,7 @@ class MongoModuleStore(ModuleStoreBase):
         # know the 'name' parameter in this context, so we have
         # to assume there's only one item in this query even though we are not specifying a name
         course_search_location = ['i4x', location.org, location.course, 'course', None]
-        courses = self.get_items(course_search_location)
+        courses = self.get_items(course_search_location, depth=depth)
 
         # make sure we found exactly one match on this above course search
         found_cnt = len(courses)

common/lib/xmodule/xmodule/x_module.py

@@ -411,7 +411,6 @@ class ResourceTemplates(object):
 
         return templates
 
-
 class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates):
     """
     An XModuleDescriptor is a specification for an element of a course. This
@@ -585,11 +584,11 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates):
     def inherit_metadata(self, metadata):
         """
         Updates this module with metadata inherited from a containing module.
-        Only metadata specified in self.inheritable_metadata will
+        Only metadata specified in inheritable_metadata will
         be inherited
         """
         # Set all inheritable metadata from kwargs that are
-        # in self.inheritable_metadata and aren't already set in metadata
+        # in inheritable_metadata and aren't already set in metadata
         for attr in self.inheritable_metadata:
             if attr not in self.metadata and attr in metadata:
                 self._inherited_metadata.add(attr)