Move inheritance logic out into a separate file in the modulestore

cms/djangoapps/contentstore/tests/factories.py

 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()])

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

cms/djangoapps/models/settings/course_details.py

 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.

common/lib/xmodule/xmodule/modulestore/inheritance.py

+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

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

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):

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
-
-        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
+        location: Something Location-like that identifies this xmodule
 
-            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):

common/lib/xmodule/xmodule/xml_module.py

 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)

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