Commit 67949f8a by Victor Shnayder

Fix metadata inheritance

* with xml datastore, re-do all inheritance once the whole course is loaded
parent 310788ca
...@@ -55,6 +55,9 @@ class CachingDescriptorSystem(MakoDescriptorSystem): ...@@ -55,6 +55,9 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
if json_data is None: if json_data is None:
return self.modulestore.get_item(location) return self.modulestore.get_item(location)
else: 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.
return XModuleDescriptor.load_from_json(json_data, self, self.default_class) return XModuleDescriptor.load_from_json(json_data, self, self.default_class)
......
...@@ -213,6 +213,12 @@ class XMLModuleStore(ModuleStoreBase): ...@@ -213,6 +213,12 @@ class XMLModuleStore(ModuleStoreBase):
system = ImportSystem(self, org, course, course_dir, tracker) system = ImportSystem(self, org, course, course_dir, tracker)
course_descriptor = system.process_xml(etree.tostring(course_data)) course_descriptor = system.process_xml(etree.tostring(course_data))
# NOTE: The descriptors end up loading somewhat bottom up, which
# breaks metadata inheritance via get_children(). Instead
# (actually, in addition to, for now), we do a final inheritance pass
# after we have the course descriptor.
XModuleDescriptor.compute_inherited_metadata(course_descriptor)
log.debug('========> Done with course import from {0}'.format(course_dir)) log.debug('========> Done with course import from {0}'.format(course_dir))
return course_descriptor return course_descriptor
......
...@@ -8,8 +8,11 @@ from xmodule.x_module import XMLParsingSystem, XModuleDescriptor ...@@ -8,8 +8,11 @@ from xmodule.x_module import XMLParsingSystem, XModuleDescriptor
from xmodule.xml_module import is_pointer_tag from xmodule.xml_module import is_pointer_tag
from xmodule.errortracker import make_error_tracker from xmodule.errortracker import make_error_tracker
from xmodule.modulestore import Location from xmodule.modulestore import Location
from xmodule.modulestore.xml import XMLModuleStore
from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.exceptions import ItemNotFoundError
from .test_export import DATA_DIR
ORG = 'test_org' ORG = 'test_org'
COURSE = 'test_course' COURSE = 'test_course'
...@@ -185,3 +188,22 @@ class ImportTestCase(unittest.TestCase): ...@@ -185,3 +188,22 @@ class ImportTestCase(unittest.TestCase):
chapter_xml = etree.fromstring(f.read()) chapter_xml = etree.fromstring(f.read())
self.assertEqual(chapter_xml.tag, 'chapter') self.assertEqual(chapter_xml.tag, 'chapter')
self.assertFalse('graceperiod' in chapter_xml.attrib) self.assertFalse('graceperiod' in chapter_xml.attrib)
def test_metadata_inherit(self):
"""Make sure that metadata is inherited properly"""
print "Starting import"
initial_import = XMLModuleStore(DATA_DIR, eager=True, course_dirs=['toy'])
courses = initial_import.get_courses()
self.assertEquals(len(courses), 1)
course = courses[0]
def check_for_key(key, node):
"recursive check for presence of key"
print "Checking {}".format(node.location.url())
self.assertTrue(key in node.metadata)
for c in node.get_children():
check_for_key(key, c)
check_for_key('graceperiod', course)
...@@ -403,6 +403,21 @@ class XModuleDescriptor(Plugin, HTMLSnippet): ...@@ -403,6 +403,21 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
return dict((k,v) for k,v in self.metadata.items() return dict((k,v) for k,v in self.metadata.items()
if k not in self._inherited_metadata) if k not in self._inherited_metadata)
@staticmethod
def compute_inherited_metadata(course):
"""Given a course descriptor, traverse the entire course tree and do
metadata inheritance. Should be called after importing a course.
NOTE: This means that there is no such thing as lazy loading at the
moment--this accesses all the children."""
def do_inherit(node):
for c in node.get_children():
c.inherit_metadata(node.metadata)
do_inherit(c)
do_inherit(course)
def inherit_metadata(self, metadata): def inherit_metadata(self, metadata):
""" """
Updates this module with metadata inherited from a containing module. Updates this module with metadata inherited from a containing module.
...@@ -415,6 +430,7 @@ class XModuleDescriptor(Plugin, HTMLSnippet): ...@@ -415,6 +430,7 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
if attr not in self.metadata and attr in metadata: if attr not in self.metadata and attr in metadata:
self._inherited_metadata.add(attr) self._inherited_metadata.add(attr)
self.metadata[attr] = metadata[attr] self.metadata[attr] = metadata[attr]
print "final self.metadata: {}".format(self.metadata)
def get_children(self): def get_children(self):
"""Returns a list of XModuleDescriptor instances for the children of """Returns a list of XModuleDescriptor instances for the children of
...@@ -423,6 +439,9 @@ class XModuleDescriptor(Plugin, HTMLSnippet): ...@@ -423,6 +439,9 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
self._child_instances = [] self._child_instances = []
for child_loc in self.definition.get('children', []): for child_loc in self.definition.get('children', []):
child = self.system.load_item(child_loc) child = self.system.load_item(child_loc)
# TODO (vshnayder): this should go away once we have
# proper inheritance support in mongo. The xml
# datastore does all inheritance on course load.
child.inherit_metadata(self.metadata) child.inherit_metadata(self.metadata)
self._child_instances.append(child) self._child_instances.append(child)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment