New policy organization:

* course roots live in roots/{url_name}.xml
   - one is linked from course.xml

* policies live in policies/url_name.json
  - loaded based on course url_name

* Updated to pass policy through into xml parsing, so it takes effect
  before descriptor constructors are called.

* Update toy test course to new structure, fix up tests

common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py

@@ -112,8 +112,8 @@ class TestMongoModuleStore(object):
         should_work = (
             ("i4x://edX/toy/video/Welcome",
              ("edX/toy/2012_Fall", "Overview", "Welcome", None)),
-            ("i4x://edX/toy/html/toylab",
-             ("edX/toy/2012_Fall", "Overview", "Toy_Videos", None)),
+            ("i4x://edX/toy/chapter/Overview",
+             ("edX/toy/2012_Fall", "Overview", None, None)),
             )
         for location, expected in should_work:
             assert_equals(path_to_location(self.store, location), expected)

common/lib/xmodule/xmodule/modulestore/xml.py

@@ -31,7 +31,8 @@ def clean_out_mako_templating(xml_string):
     return xml_string
 
 class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
-    def __init__(self, xmlstore, org, course, course_dir, error_tracker, **kwargs):
+    def __init__(self, xmlstore, org, course, course_dir,
+                 policy, error_tracker, **kwargs):
         """
         A class that handles loading from xml.  Does some munging to ensure that
         all elements have unique slugs.
@@ -97,7 +98,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
         MakoDescriptorSystem.__init__(self, load_item, resources_fs,
                                       error_tracker, render_template, **kwargs)
         XMLParsingSystem.__init__(self, load_item, resources_fs,
-                                  error_tracker, process_xml, **kwargs)
+                                  error_tracker, process_xml, policy, **kwargs)
 
 
 class XMLModuleStore(ModuleStoreBase):
@@ -184,6 +185,7 @@ class XMLModuleStore(ModuleStoreBase):
         if not os.path.exists(policy_path):
             return {}
         try:
+            log.debug("Loading policy from {}".format(policy_path))
             with open(policy_path) as f:
                 return json.load(f)
         except (IOError, ValueError) as err:
@@ -232,19 +234,16 @@ class XMLModuleStore(ModuleStoreBase):
                 tracker(msg)
                 course = course_dir
 
-            system = ImportSystem(self, org, course, course_dir, tracker)
+            url_name = course_data.get('url_name')
+            if url_name:
+                policy_path = self.data_dir / course_dir / 'policies' / '{}.json'.format(url_name)
+                policy = self.load_policy(policy_path, tracker)
+            else:
+                policy = {}
 
-            policy_path = self.data_dir / course_dir / 'policy.json'
-            policy = self.load_policy(policy_path, tracker)
-            # Special case -- need to change the url_name of the course before
-            # it gets loaded, so its location and other fields are right
-            if 'course_url_name' in policy:
-                new_url_name = policy['course_url_name']
-                log.info("changing course url_name to {}".format(new_url_name))
-                course_data.set('url_name', new_url_name)
+            system = ImportSystem(self, org, course, course_dir, policy, tracker)
 
             course_descriptor = system.process_xml(etree.tostring(course_data))
-            XModuleDescriptor.apply_policy(course_descriptor, policy)
 
             # NOTE: The descriptors end up loading somewhat bottom up, which
             # breaks metadata inheritance via get_children().  Instead

common/lib/xmodule/xmodule/tests/test_import.py

@@ -42,9 +42,9 @@ class DummySystem(XMLParsingSystem):
             descriptor.get_children()
             return descriptor
 
-
+        policy = {}
         XMLParsingSystem.__init__(self, load_item, self.resources_fs,
-                                  self.errorlog.tracker, process_xml)
+                                  self.errorlog.tracker, process_xml, policy)
 
     def render_template(self, template, context):
             raise Exception("Shouldn't be called")

common/lib/xmodule/xmodule/x_module.py

@@ -303,10 +303,6 @@ def policy_key(location):
     Get the key for a location in a policy file.  (Since the policy file is
     specific to a course, it doesn't need the full location url).
     """
-    # Special case--we need to be able to override the url_name on a course,
-    # so special case where we look for the course descriptor
-    if location.category == 'course':
-        return 'course'
     return '{cat}/{name}'.format(cat=location.category, name=location.name)
 
 
@@ -429,23 +425,6 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
                     if k not in self._inherited_metadata)
 
 
-    @staticmethod
-    def apply_policy(node, policy):
-        """
-        Given a descriptor, traverse all its descendants and update its metadata
-        with the policy.
-
-        Notes:
-          - this does not propagate inherited metadata.  The caller should
-            call compute_inherited_metadata after applying the policy.
-          - metadata specified in the policy overrides metadata in the xml
-        """
-        k = policy_key(node.location)
-        if k in policy:
-            node.metadata.update(policy[k])
-        for c in node.get_children():
-            XModuleDescriptor.apply_policy(c, policy)
-
     @staticmethod
     def compute_inherited_metadata(node):
         """Given a descriptor, traverse all of its descendants and do metadata
@@ -701,16 +680,19 @@ class DescriptorSystem(object):
 
 
 class XMLParsingSystem(DescriptorSystem):
-    def __init__(self, load_item, resources_fs, error_tracker, process_xml, **kwargs):
+    def __init__(self, load_item, resources_fs, error_tracker, process_xml, policy, **kwargs):
         """
         load_item, resources_fs, error_tracker: see DescriptorSystem
 
+        policy: a policy dictionary for overriding xml metadata
+
         process_xml: Takes an xml string, and returns a XModuleDescriptor
             created from that xml
         """
         DescriptorSystem.__init__(self, load_item, resources_fs, error_tracker,
                                   **kwargs)
         self.process_xml = process_xml
+        self.policy = policy
 
 
 class ModuleSystem(object):

common/lib/xmodule/xmodule/xml_module.py

@@ -1,4 +1,4 @@
-from xmodule.x_module import XModuleDescriptor
+from xmodule.x_module import (XModuleDescriptor, policy_key)
 from xmodule.modulestore import Location
 from lxml import etree
 import json
@@ -270,6 +270,11 @@ class XmlDescriptor(XModuleDescriptor):
                 log.debug('Error %s in loading metadata %s' % (err,dmdata))
                 metadata['definition_metadata_err'] = str(err)
 
+        # Set/override any metadata specified by policy
+        k = policy_key(location)
+        if k in system.policy:
+            metadata.update(system.policy[k])
+
         return cls(
             system,
             definition,

common/test/data/toy/course.xml

@@ -1,9 +0,0 @@
-<course name="Toy Course" org="edX" course="toy" graceperiod="1 day 5 hours 59 minutes 59 seconds" slug="2012_Fall" start="2015-07-17T12:00">
-  <chapter name="Overview">
-    <videosequence format="Lecture Sequence" name="Toy Videos">
-      <html name="toylab" filename="toylab"/>
-      <video name="Video Resources" youtube="1.0:1bK-WdDi6Qw"/>
-    </videosequence>
-    <video name="Welcome" youtube="1.0:p2Q6BrNhdh8"/>
-  </chapter>
-</course>

common/test/data/toy/course.xml

@@ -0,0 +1 @@
+roots/2012_Fall.xml

common/test/data/toy/course/2012_Fall.xml

@@ -0,0 +1,9 @@
+<course>
+  <chapter url_name="Overview">
+    <videosequence url_name="Toy_Videos">
+      <html url_name="toylab"/>
+      <video url_name="Video_Resources" youtube="1.0:1bK-WdDi6Qw"/>
+    </videosequence>
+    <video url_name="Welcome" youtube="1.0:p2Q6BrNhdh8"/>
+  </chapter>
+</course>

common/test/data/toy/policies/2012_Fall.json

@@ -0,0 +1,23 @@
+{
+    "course/2012_Fall": {
+        "graceperiod": "2 days 5 hours 59 minutes 59 seconds",
+        "start": "2015-07-17T12:00",
+        "display_name": "Toy Course"
+    },
+    "chapter/Overview": {
+        "display_name": "Overview"
+    },
+    "videosequence/Toy_Videos": {
+        "display_name": "Toy Videos",
+        "format": "Lecture Sequence"
+    },
+    "html/toylab": {
+        "display_name": "Toy lab"
+    },
+    "video/Video_Resources": {
+        "display_name": "Video Resources"
+    },
+    "video/Welcome": {
+        "display_name": "Welcome"
+    }
+}

common/test/data/toy/roots/2012_Fall.xml

@@ -0,0 +1 @@
+<course org="edX" course="toy" url_name="2012_Fall"/>