diff --git a/cms/djangoapps/contentstore/tests/factories.py b/cms/djangoapps/contentstore/tests/factories.py index 3274477098..24897bf795 100644 --- a/cms/djangoapps/contentstore/tests/factories.py +++ b/cms/djangoapps/contentstore/tests/factories.py @@ -1,6 +1,7 @@ from factory import Factory from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore +from xmodule.modulestore.inheritance import own_metadata from time import gmtime from uuid import uuid4 from xmodule.timeparse import stringify_time @@ -48,7 +49,7 @@ class XModuleCourseFactory(Factory): {"type": "progress", "name": "Progress"}] # Update the data in the mongo datastore - store.update_metadata(new_course.location.url(), new_course.own_metadata) + store.update_metadata(new_course.location.url(), own_metadata(new_course)) return new_course @@ -95,7 +96,7 @@ class XModuleItemFactory(Factory): if display_name is not None: new_item.metadata['display_name'] = display_name - store.update_metadata(new_item.location.url(), new_item.own_metadata) + store.update_metadata(new_item.location.url(), own_metadata(new_item)) if new_item.location.category not in DETACHED_CATEGORIES: store.update_children(parent_location, parent.definition.get('children', []) + [new_item.location.url()]) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index bf706f2996..22b46dc4be 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -28,6 +28,7 @@ from django.conf import settings from xmodule.modulestore import Location from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError +from xmodule.modulestore.inheritance import own_metadata from xmodule.x_module import ModuleSystem from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import exc_info_to_str @@ -712,7 +713,7 @@ def clone_item(request): if display_name is not None: new_item.metadata['display_name'] = display_name - get_modulestore(template).update_metadata(new_item.location.url(), new_item.own_metadata) + get_modulestore(template).update_metadata(new_item.location.url(), own_metadata(new_item)) if new_item.location.category not in DETACHED_CATEGORIES: get_modulestore(parent.location).update_children(parent_location, parent.definition.get('children', []) + [new_item.location.url()]) @@ -1231,7 +1232,7 @@ def initialize_course_tabs(course): {"type": "wiki", "name": "Wiki"}, {"type": "progress", "name": "Progress"}] - modulestore('direct').update_metadata(course.location.url(), course.own_metadata) + modulestore('direct').update_metadata(course.location.url(), own_metadata(new_course)) @ensure_csrf_cookie @login_required diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py index 2bb9d98be7..59cadf6962 100644 --- a/cms/djangoapps/models/settings/course_details.py +++ b/cms/djangoapps/models/settings/course_details.py @@ -1,6 +1,7 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore import Location from xmodule.modulestore.exceptions import ItemNotFoundError +from xmodule.modulestore.inheritance import own_metadata import json from json.encoder import JSONEncoder import time @@ -117,7 +118,7 @@ class CourseDetails: descriptor.enrollment_end = converted if dirty: - get_modulestore(course_location).update_metadata(course_location, descriptor.metadata) + get_modulestore(course_location).update_metadata(course_location, own_metadata(descriptor)) # NOTE: below auto writes to the db w/o verifying that any of the fields actually changed # to make faster, could compare against db or could have client send over a list of which fields changed. diff --git a/common/lib/xmodule/xmodule/modulestore/inheritance.py b/common/lib/xmodule/xmodule/modulestore/inheritance.py new file mode 100644 index 0000000000..ca88aab8d8 --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/inheritance.py @@ -0,0 +1,48 @@ +from xmodule.model import Scope + + +def compute_inherited_metadata(descriptor): + """Given a descriptor, traverse all of its descendants and do metadata + inheritance. Should be called on a CourseDescriptor after importing a + course. + + NOTE: This means that there is no such thing as lazy loading at the + moment--this accesses all the children.""" + for child in descriptor.get_children(): + inherit_metadata(child, descriptor._model_data) + compute_inherited_metadata(child) + + +def inherit_metadata(descriptor, model_data): + """ + Updates this module with metadata inherited from a containing module. + Only metadata specified in self.inheritable_metadata will + be inherited + """ + if not hasattr(descriptor, '_inherited_metadata'): + setattr(descriptor, '_inherited_metadata', set()) + + # Set all inheritable metadata from kwargs that are + # in self.inheritable_metadata and aren't already set in metadata + for attr in descriptor.inheritable_metadata: + if attr not in descriptor._model_data and attr in model_data: + descriptor._inherited_metadata.add(attr) + descriptor._model_data[attr] = model_data[attr] + + +def own_metadata(module): + """ + Return a dictionary that contains only non-inherited field keys, + mapped to their values + """ + inherited_metadata = getattr(module, '_inherited_metadata', {}) + metadata = {} + for field in module.fields + module.lms.fields: + # Only save metadata that wasn't inherited + if (field.scope == Scope.settings and + field.name not in inherited_metadata and + field.name in module._model_data): + + metadata[field.name] = module._model_data[field.name] + + return metadata diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 72c9093bbf..ed1bfdb30f 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -22,6 +22,7 @@ from xmodule.html_module import HtmlDescriptor from . import ModuleStoreBase, Location from .exceptions import ItemNotFoundError +from .inheritance import compute_inherited_metadata edx_xml_parser = etree.XMLParser(dtd_validation=False, load_dtd=False, remove_comments=True, remove_blank_text=True) @@ -421,30 +422,6 @@ class XMLModuleStore(ModuleStoreBase): # 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. - def compute_inherited_metadata(descriptor): - """Given a descriptor, traverse all of its descendants and do metadata - inheritance. Should be called on a CourseDescriptor after importing a - course. - - NOTE: This means that there is no such thing as lazy loading at the - moment--this accesses all the children.""" - for child in descriptor.get_children(): - inherit_metadata(child, descriptor._model_data) - compute_inherited_metadata(child) - - def inherit_metadata(descriptor, model_data): - """ - Updates this module with metadata inherited from a containing module. - Only metadata specified in self.inheritable_metadata will - be inherited - """ - # Set all inheritable metadata from kwargs that are - # in self.inheritable_metadata and aren't already set in metadata - for attr in descriptor.inheritable_metadata: - if attr not in descriptor._model_data and attr in model_data: - descriptor._inherited_metadata.add(attr) - descriptor._model_data[attr] = model_data[attr] - compute_inherited_metadata(course_descriptor) # now import all pieces of course_info which is expected to be stored diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index bbbb60d8ed..356ca772f4 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -8,7 +8,7 @@ from .xml import XMLModuleStore from .exceptions import DuplicateItemError from xmodule.modulestore import Location from xmodule.contentstore.content import StaticContent, XASSET_SRCREF_PREFIX -from xmodule.model import Scope +from .inheritance import own_metadata log = logging.getLogger(__name__) @@ -143,15 +143,7 @@ def import_module_from_xml(modulestore, static_content_store, course_data_path, if module.has_children: modulestore.update_children(module.location, module.children) - metadata = {} - for field in module.fields + module.lms.fields: - # Only save metadata that wasn't inherited - if (field.scope == Scope.settings and - field.name not in module._inherited_metadata and - field.name in module._model_data): - - metadata[field.name] = module._model_data[field.name] - modulestore.update_metadata(module.location, metadata) + modulestore.update_metadata(module.location, own_metadata(module)) def import_course_from_xml(modulestore, static_content_store, course_data_path, module, target_location_namespace=None): diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index a05a191e5f..43d6f58a16 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -114,43 +114,12 @@ class XModule(HTMLSnippet): location: Something Location-like that identifies this xmodule - definition: A dictionary containing 'data' and 'children'. Both are - optional - - 'data': is JSON-like (string, dictionary, list, bool, or None, - optionally nested). - - This defines all of the data necessary for a problem to display - that is intrinsic to the problem. It should not include any - data that would vary between two courses using the same problem - (due dates, grading policy, randomization, etc.) - - 'children': is a list of Location-like values for child modules that - this module depends on - descriptor: the XModuleDescriptor that this module is an instance of. TODO (vshnayder): remove the definition parameter and location--they can come from the descriptor. - instance_state: A string of serialized json that contains the state of - this module for current student accessing the system, or None if - no state has been saved - - shared_state: A string of serialized json that contains the state that - is shared between this module and any modules of the same type with - the same shared_state_key. This state is only shared per-student, - not across different students - - kwargs: Optional arguments. Subclasses should always accept kwargs and - pass them to the parent class constructor. - - Current known uses of kwargs: - - metadata: SCAFFOLDING - This dictionary will be split into - several different types of metadata in the future (course - policy, modification history, etc). A dictionary containing - data that specifies information that is particular to a - problem in the context of a course + model_data: A dictionary-like object that maps field names to values + for those fields. ''' self.system = system self.location = Location(location) @@ -357,31 +326,10 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates): system: A DescriptorSystem for interacting with external resources - definition: A dict containing `data` and `children` representing the - problem definition + location: Something Location-like that identifies this xmodule - Current arguments passed in kwargs: - - location: A xmodule.modulestore.Location object indicating the name - and ownership of this problem - - shared_state_key: The key to use for sharing StudentModules with - other modules of this type - - metadata: A dictionary containing the following optional keys: - goals: A list of strings of learning goals associated with this - module - url_name: The name to use for this module in urls and other places - where a unique name is needed. - format: The format of this module ('Homework', 'Lab', etc) - graded (bool): Whether this module is should be graded or not - start (string): The date for which this module will be available - due (string): The due date for this module - graceperiod (string): The amount of grace period to allow when - enforcing the due date - showanswer (string): When to show answers for this module - rerandomize (string): When to generate a newly randomized - instance of the module data + model_data: A dictionary-like object that maps field names to values + for those fields. """ self.system = system self.location = Location(location) @@ -390,7 +338,6 @@ class XModuleDescriptor(Plugin, HTMLSnippet, ResourceTemplates): self._model_data = model_data self._child_instances = None - self._inherited_metadata = set() self._child_instances = None def get_children(self): diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 754d9b523e..d19fc26157 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -1,5 +1,6 @@ from xmodule.x_module import (XModuleDescriptor, policy_key) from xmodule.modulestore import Location +from xmodule.modulestore.inheritance import own_metadata from lxml import etree import json import copy @@ -368,10 +369,10 @@ class XmlDescriptor(XModuleDescriptor): (Possible format conversion through an AttrMap). """ attr_map = self.xml_attribute_map.get(attr, AttrMap()) - return attr_map.to_xml(self.own_metadata[attr]) + return attr_map.to_xml(self._model_data[attr]) # Add the non-inherited metadata - for attr in sorted(self.own_metadata): + for attr in sorted(own_metadata(self)): # don't want e.g. data_dir if attr not in self.metadata_to_strip and attr not in self.metadata_to_export_to_policy: val = val_for_xml(attr) diff --git a/lms/djangoapps/courseware/management/commands/metadata_to_json.py b/lms/djangoapps/courseware/management/commands/metadata_to_json.py index 8ef0dee7b3..3d399ef115 100644 --- a/lms/djangoapps/courseware/management/commands/metadata_to_json.py +++ b/lms/djangoapps/courseware/management/commands/metadata_to_json.py @@ -51,7 +51,7 @@ def node_metadata(node): 'start', 'due', 'graded', 'hide_from_toc', 'ispublic', 'xqa_key') - orig = node.own_metadata + orig = own_metadata(node) d = {k: orig[k] for k in to_export if k in orig} return d