do portable link rewriting on import and add test to confirm it

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -720,6 +720,17 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         resp = self.client.get('http://localhost:8001/c4x/CDX/123123/asset/&images_circuits_Lab7Solution2.png')
         self.assertEqual(resp.status_code, 400)
 
+    def test_rewrite_nonportable_links_on_import(self):
+        module_store = modulestore('direct')
+        content_store = contentstore()
+
+        import_from_xml(module_store, 'common/test/data/', ['toy'], static_content_store=content_store)
+
+        html_module_location = Location(['i4x', 'edX', 'toy', 'html', 'nonportable'])
+        html_module = module_store.get_instance('edX/toy/2012_Fall', html_module_location)
+
+        self.assertIn('/static/foo.jpg', html_module.data)
+
     def test_delete_course(self):
         """
         This test will import a course, make a draft item, and delete it. This will also assert that the

common/lib/xmodule/xmodule/modulestore/store_utilities.py

+import re
 from xmodule.contentstore.content import StaticContent
 from xmodule.modulestore import Location
 from xmodule.modulestore.mongo import MongoModuleStore
 from xmodule.modulestore.inheritance import own_metadata
+from static_replace import _url_replace_regex
 
 import logging
 
 
+def convert_to_portable_links(source_course_id, text):
+    """
+    Does a regex replace on non-portable links:
+         /c4x/<org>/<course>/asset/<name> -> /static/<name>
+         /jump_to/i4x://<org>/<course>/<category>/<name> -> /jump_to_id/<id>
+    """
+
+    def portable_asset_link_subtitution(match):
+        quote = match.group('quote')
+        rest = match.group('rest')
+        return "".join([quote, '/static/'+rest, quote])
+
+    org, course, run = source_course_id.split("/")
+    course_location = Location(['i4x', org, course, 'course', run])
+
+    c4x_link_base = '{0}/'.format(StaticContent.get_base_url_path_for_course_assets(course_location))
+    text = re.sub(_url_replace_regex(c4x_link_base), portable_asset_link_subtitution, text)
+
+    return text
+
+
 def _clone_modules(modulestore, modules, dest_location):
     for module in modules:
         original_loc = Location(module.location)

common/lib/xmodule/xmodule/modulestore/xml_importer.py

@@ -10,6 +10,7 @@ from xmodule.modulestore import Location
 from xmodule.contentstore.content import StaticContent
 from .inheritance import own_metadata
 from xmodule.errortracker import make_error_tracker
+from .store_utilities import convert_to_portable_links
 
 log = logging.getLogger(__name__)
 
@@ -60,54 +61,6 @@ def import_static_content(modules, course_loc, course_data_path, static_content_
     return remap_dict
 
 
-def import_module_from_xml(modulestore, static_content_store, course_data_path, module, target_location_namespace=None, verbose=False):
-    # remap module to the new namespace
-    if target_location_namespace is not None:
-        # This looks a bit wonky as we need to also change the 'name' of the imported course to be what
-        # the caller passed in
-        if module.location.category != 'course':
-            module.location = module.location._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
-                                                       course=target_location_namespace.course)
-        else:
-            module.location = module.location._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
-                                                       course=target_location_namespace.course, name=target_location_namespace.name)
-
-        # then remap children pointers since they too will be re-namespaced
-        if module.has_children:
-            children_locs = module.children
-            new_locs = []
-            for child in children_locs:
-                child_loc = Location(child)
-                new_child_loc = child_loc._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
-                                                   course=target_location_namespace.course)
-
-                new_locs.append(new_child_loc.url())
-
-            module.children = new_locs
-
-    if hasattr(module, 'data'):
-        modulestore.update_item(module.location, module.data)
-
-    if module.has_children:
-        modulestore.update_children(module.location, module.children)
-
-    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, verbose=False):
-    # cdodge: more hacks (what else). Seems like we have a problem when importing a course (like 6.002) which
-    # does not have any tabs defined in the policy file. The import goes fine and then displays fine in LMS,
-    # but if someone tries to add a new tab in the CMS, then the LMS barfs because it expects that -
-    # if there is *any* tabs - then there at least needs to be some predefined ones
-    if module.tabs is None or len(module.tabs) == 0:
-        module.tabs = [{"type": "courseware"},
-                       {"type": "course_info", "name": "Course Info"},
-                       {"type": "discussion", "name": "Discussion"},
-                       {"type": "wiki", "name": "Wiki"}]  # note, add 'progress' when we can support it on Edge
-
-    import_module_from_xml(modulestore, static_content_store, course_data_path, module, target_location_namespace, verbose=verbose)
-
-
 def import_from_xml(store, data_dir, course_dirs=None,
                     default_class='xmodule.raw_module.RawDescriptor',
                     load_error_modules=True, static_content_store=None, target_location_namespace=None,
@@ -175,7 +128,7 @@ def import_from_xml(store, data_dir, course_dirs=None,
                                        {"type": "discussion", "name": "Discussion"},
                                        {"type": "wiki", "name": "Wiki"}]  # note, add 'progress' when we can support it on Edge
 
-                    import_module(module, store, course_data_path, static_content_store)
+                    import_module(module, store, course_data_path, static_content_store, course_location)
 
                     course_items.append(module)
 
@@ -202,12 +155,12 @@ def import_from_xml(store, data_dir, course_dirs=None,
                 if verbose:
                     log.debug('importing module location {0}'.format(module.location))
 
-                import_module(module, store, course_data_path, static_content_store)
+                import_module(module, store, course_data_path, static_content_store, course_location)
 
             # now import any 'draft' items
             if draft_store is not None:
                 import_course_draft(xml_module_store, store, draft_store, course_data_path,
-                                    static_content_store, target_location_namespace if target_location_namespace is not None
+                                    static_content_store, course_location, target_location_namespace if target_location_namespace is not None
                                     else course_location)
 
         finally:
@@ -220,7 +173,7 @@ def import_from_xml(store, data_dir, course_dirs=None,
     return xml_module_store, course_items
 
 
-def import_module(module, store, course_data_path, static_content_store, allow_not_found=False):
+def import_module(module, store, course_data_path, static_content_store, source_course_location, allow_not_found=False):
     content = {}
     for field in module.fields:
         if field.scope != Scope.content:
@@ -237,6 +190,11 @@ def import_module(module, store, course_data_path, static_content_store, allow_n
     else:
         module_data = content
 
+    if isinstance(module_data, basestring):
+        # we want to convert all 'non-portable' links in the module_data (if it is a string) to
+        # portable strings (e.g. /static/)
+        module_data = convert_to_portable_links(source_course_location.course_id, module_data)
+
     if allow_not_found:
         store.update_item(module.location, module_data, allow_not_found=allow_not_found)
     else:
@@ -250,7 +208,7 @@ def import_module(module, store, course_data_path, static_content_store, allow_n
     store.update_metadata(module.location, dict(own_metadata(module)))
 
 
-def import_course_draft(xml_module_store, store, draft_store, course_data_path, static_content_store, target_location_namespace):
+def import_course_draft(xml_module_store, store, draft_store, course_data_path, static_content_store, source_location_namespace, target_location_namespace):
     '''
     This will import all the content inside of the 'drafts' folder, if it exists
     NOTE: This is not a full course import, basically in our current application only verticals (and downwards)
@@ -307,7 +265,7 @@ def import_course_draft(xml_module_store, store, draft_store, course_data_path,
                             del module.xml_attributes['parent_sequential_url']
                             del module.xml_attributes['index_in_children_list']
 
-                        import_module(module, draft_store, course_data_path, static_content_store, allow_not_found=True)
+                        import_module(module, draft_store, course_data_path, static_content_store, source_location_namespace, allow_not_found=True)
                         for child in module.get_children():
                             _import_module(child)
 
@@ -524,3 +482,58 @@ def perform_xlint(data_dir, course_dirs,
         print "This course can be imported successfully."
 
     return err_cnt
+
+
+#
+# UNSURE IF THIS IS UNUSED CODE - IF SO NEEDS TO BE PRUNED. TO BE INVESTIGATED.
+#
+def import_module_from_xml(modulestore, static_content_store, course_data_path, module, target_location_namespace=None, verbose=False):
+    # remap module to the new namespace
+    if target_location_namespace is not None:
+        # This looks a bit wonky as we need to also change the 'name' of the imported course to be what
+        # the caller passed in
+        if module.location.category != 'course':
+            module.location = module.location._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
+                                                       course=target_location_namespace.course)
+        else:
+            module.location = module.location._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
+                                                       course=target_location_namespace.course, name=target_location_namespace.name)
+
+        # then remap children pointers since they too will be re-namespaced
+        if module.has_children:
+            children_locs = module.children
+            new_locs = []
+            for child in children_locs:
+                child_loc = Location(child)
+                new_child_loc = child_loc._replace(tag=target_location_namespace.tag, org=target_location_namespace.org,
+                                                   course=target_location_namespace.course)
+
+                new_locs.append(new_child_loc.url())
+
+            module.children = new_locs
+
+    if hasattr(module, 'data'):
+        modulestore.update_item(module.location, module.data)
+
+    if module.has_children:
+        modulestore.update_children(module.location, module.children)
+
+    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, verbose=False):
+    # CDODGE: Is this unused code (along with import_module_from_xml)? I can't find any references to it. If so, then
+    # we need to delete this apparently duplicate code.
+
+    # cdodge: more hacks (what else). Seems like we have a problem when importing a course (like 6.002) which
+    # does not have any tabs defined in the policy file. The import goes fine and then displays fine in LMS,
+    # but if someone tries to add a new tab in the CMS, then the LMS barfs because it expects that -
+    # if there is *any* tabs - then there at least needs to be some predefined ones
+    if module.tabs is None or len(module.tabs) == 0:
+        module.tabs = [{"type": "courseware"},
+                       {"type": "course_info", "name": "Course Info"},
+                       {"type": "discussion", "name": "Discussion"},
+                       {"type": "wiki", "name": "Wiki"}]  # note, add 'progress' when we can support it on Edge
+
+    import_module_from_xml(modulestore, static_content_store, course_data_path, module, target_location_namespace, verbose=verbose)
+

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

@@ -5,6 +5,7 @@
       <html url_name="secret:toylab"/>
       <html url_name="toyjumpto"/>
       <html url_name="toyhtml"/>
+      <html url_name="nonportable"/>
       <video url_name="Video_Resources" youtube_id_1_0="1bK-WdDi6Qw" display_name="Video Resources"/>
     </videosequence>
     <video url_name="Welcome" youtube_id_1_0="p2Q6BrNhdh8" display_name="Welcome"/>

common/test/data/toy/html/nonportable.html

+<a href="/c4x/edX/toy/asset/foo.jpg">link</a>

common/test/data/toy/html/nonportable.xml

+<html filename="nonportable.html"/>
\ No newline at end of file