diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 0fccc2498b..0447254cfb 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -176,7 +176,7 @@ def load_preview_state(request, preview_id, location): def save_preview_state(request, preview_id, location, instance_state, shared_state): """ - Load the state of a preview module to the request + Save the state of a preview module to the request preview_id (str): An identifier specifying which preview this module is used for location: The Location of the module to dispatch to diff --git a/cms/envs/dev.py b/cms/envs/dev.py index 4098263829..c5e1cf5689 100644 --- a/cms/envs/dev.py +++ b/cms/envs/dev.py @@ -2,13 +2,18 @@ This config file runs the simplest dev environment""" from .common import * +from .logsettings import get_logger_config import logging import sys -logging.basicConfig(stream=sys.stdout, ) DEBUG = True TEMPLATE_DEBUG = DEBUG +LOGGING = get_logger_config(ENV_ROOT / "log", + logging_env="dev", + tracking_filename="tracking.log", + debug=True) + MODULESTORE = { 'default': { diff --git a/cms/envs/logsettings.py b/cms/envs/logsettings.py index 31130e33c6..3683314d02 100644 --- a/cms/envs/logsettings.py +++ b/cms/envs/logsettings.py @@ -3,19 +3,19 @@ import os.path import platform import sys -def get_logger_config(log_dir, - logging_env="no_env", +def get_logger_config(log_dir, + logging_env="no_env", tracking_filename=None, syslog_addr=None, debug=False): """Return the appropriate logging config dictionary. You should assign the - result of this to the LOGGING var in your settings. The reason it's done + result of this to the LOGGING var in your settings. The reason it's done this way instead of registering directly is because I didn't want to worry - about resetting the logging state if this is called multiple times when + about resetting the logging state if this is called multiple times when settings are extended.""" # If we're given an explicit place to put tracking logs, we do that (say for - # debugging). However, logging is not safe for multiple processes hitting + # debugging). However, logging is not safe for multiple processes hitting # the same file. So if it's left blank, we dynamically create the filename # based on the PID of this worker process. if tracking_filename: @@ -33,6 +33,7 @@ def get_logger_config(log_dir, return { 'version': 1, + 'disable_existing_loggers': False, 'formatters' : { 'standard' : { 'format' : '%(asctime)s %(levelname)s %(process)d [%(name)s] %(filename)s:%(lineno)d - %(message)s', diff --git a/common/lib/xmodule/tests/test_export.py b/common/lib/xmodule/tests/test_export.py index 97da2c4fe5..eacf8352be 100644 --- a/common/lib/xmodule/tests/test_export.py +++ b/common/lib/xmodule/tests/test_export.py @@ -1,5 +1,6 @@ from xmodule.modulestore.xml import XMLModuleStore from nose.tools import assert_equals +from nose import SkipTest from tempfile import mkdtemp from fs.osfs import OSFS @@ -26,3 +27,10 @@ def check_export_roundtrip(data_dir): for location in initial_import.modules.keys(): print "Checking", location assert_equals(initial_import.modules[location], second_import.modules[location]) + + +def test_toy_roundtrip(): + dir = "" + # TODO: add paths and make this run. + raise SkipTest() + check_export_roundtrip(dir) diff --git a/common/lib/xmodule/tests/test_import.py b/common/lib/xmodule/tests/test_import.py index 34ace135a3..6a407fe189 100644 --- a/common/lib/xmodule/tests/test_import.py +++ b/common/lib/xmodule/tests/test_import.py @@ -1,36 +1,61 @@ from path import path import unittest +from fs.memoryfs import MemoryFS from lxml import etree from xmodule.x_module import XMLParsingSystem, XModuleDescriptor -from xmodule.errortracker import null_error_tracker +from xmodule.errortracker import make_error_tracker from xmodule.modulestore import Location +from xmodule.modulestore.exceptions import ItemNotFoundError + +ORG = 'test_org' +COURSE = 'test_course' + +class DummySystem(XMLParsingSystem): + def __init__(self): + + self.modules = {} + self.resources_fs = MemoryFS() + self.errorlog = make_error_tracker() + + def load_item(loc): + loc = Location(loc) + if loc in self.modules: + return self.modules[loc] + + print "modules: " + print self.modules + raise ItemNotFoundError("Can't find item at loc: {0}".format(loc)) + + def process_xml(xml): + print "loading {0}".format(xml) + descriptor = XModuleDescriptor.load_from_xml(xml, self, ORG, COURSE, None) + # Need to save module so we can find it later + self.modules[descriptor.location] = descriptor + + # always eager + descriptor.get_children() + return descriptor + + + XMLParsingSystem.__init__(self, load_item, self.resources_fs, + self.errorlog.tracker, process_xml) + + def render_template(self, template, context): + raise Exception("Shouldn't be called") + + + class ImportTestCase(unittest.TestCase): '''Make sure module imports work properly, including for malformed inputs''' + @staticmethod def get_system(): '''Get a dummy system''' - # Shouldn't need any system params, because the initial parse should fail - def load_item(loc): - raise Exception("Shouldn't be called") - - resources_fs = None - - def process_xml(xml): - raise Exception("Shouldn't be called") - - - def render_template(template, context): - raise Exception("Shouldn't be called") - - system = XMLParsingSystem(load_item, resources_fs, - null_error_tracker, process_xml) - system.render_template = render_template - - return system + return DummySystem() def test_fallback(self): '''Make sure that malformed xml loads as an ErrorDescriptor.''' @@ -85,3 +110,30 @@ class ImportTestCase(unittest.TestCase): xml_out = etree.fromstring(xml_str_out) self.assertEqual(xml_out.tag, 'sequential') + def test_metadata_inherit(self): + """Make sure metadata inherits properly""" + system = self.get_system() + v = "1 hour" + start_xml = ''' + + Two houses, ... + '''.format(grace=v) + descriptor = XModuleDescriptor.load_from_xml(start_xml, system, + 'org', 'course') + + print "Errors: {0}".format(system.errorlog.errors) + print descriptor, descriptor.metadata + self.assertEqual(descriptor.metadata['graceperiod'], v) + + # Check that the child inherits correctly + child = descriptor.get_children()[0] + self.assertEqual(child.metadata['graceperiod'], v) + + # Now export and see if the chapter tag has a graceperiod attribute + resource_fs = MemoryFS() + exported_xml = descriptor.export_to_xml(resource_fs) + print "Exported xml:", exported_xml + root = etree.fromstring(exported_xml) + chapter_tag = root[0] + self.assertEqual(chapter_tag.tag, 'chapter') + self.assertFalse('graceperiod' in chapter_tag.attrib) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index e7f9c9ce0a..d6540023e8 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -41,6 +41,9 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): self.used_slugs = set() def process_xml(xml): + """Takes an xml string, and returns a XModuleDescriptor created from + that xml. + """ try: # VS[compat] # TODO (cpennington): Remove this once all fall 2012 courses diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index 578ade95fe..891db7e994 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -35,6 +35,8 @@ def import_from_xml(store, data_dir, course_dirs=None, eager=True, store.update_item(module.location, module.definition['data']) if 'children' in module.definition: store.update_children(module.location, module.definition['children']) - store.update_metadata(module.location, dict(module.metadata)) + # NOTE: It's important to use own_metadata here to avoid writing + # inherited metadata everywhere. + store.update_metadata(module.location, dict(module.own_metadata)) return module_store diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 2a1769bbd7..360f1b07d0 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -358,6 +358,14 @@ class XModuleDescriptor(Plugin, HTMLSnippet): self._child_instances = None self._inherited_metadata = set() + @property + def own_metadata(self): + """ + Return the metadata that is not inherited, but was defined on this module. + """ + return dict((k,v) for k,v in self.metadata.items() + if k not in self._inherited_metadata) + def inherit_metadata(self, metadata): """ Updates this module with metadata inherited from a containing module. diff --git a/lms/djangoapps/courseware/management/commands/clean_xml.py b/lms/djangoapps/courseware/management/commands/clean_xml.py index 29ce246637..1773c3d21f 100644 --- a/lms/djangoapps/courseware/management/commands/clean_xml.py +++ b/lms/djangoapps/courseware/management/commands/clean_xml.py @@ -53,14 +53,12 @@ def import_with_checks(course_dir, verbose=True): data_dir = course_dir.dirname() course_dirs = [course_dir.basename()] - (error_tracker, errors) = make_error_tracker() # No default class--want to complain if it doesn't find plugins for any # module. modulestore = XMLModuleStore(data_dir, default_class=None, eager=True, - course_dirs=course_dirs, - error_tracker=error_tracker) + course_dirs=course_dirs) def str_of_err(tpl): (msg, exc_info) = tpl @@ -71,6 +69,15 @@ def import_with_checks(course_dir, verbose=True): return '{msg}\n{exc}'.format(msg=msg, exc=exc_str) courses = modulestore.get_courses() + + n = len(courses) + if n != 1: + print 'ERROR: Expect exactly 1 course. Loaded {n}: {lst}'.format( + n=n, lst=courses) + return (False, None) + + course = courses[0] + errors = modulestore.get_item_errors(course.location) if len(errors) != 0: all_ok = False print '\n' @@ -80,13 +87,6 @@ def import_with_checks(course_dir, verbose=True): print "=" * 40 print '\n' - n = len(courses) - if n != 1: - print 'ERROR: Expect exactly 1 course. Loaded {n}: {lst}'.format( - n=n, lst=courses) - return (False, None) - - course = courses[0] #print course validators = (