Merge pull request #2413 from edx/bugfix/ichuang/do-not-import-tilde-files

Skip tilde files when importing

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

@@ -1,16 +1,37 @@
+"""
+Tests around our XML modulestore, including importing
+well-formed and not-well-formed XML.
+"""
 import os.path
+import unittest
+from glob import glob
+from mock import patch
 
 from nose.tools import assert_raises, assert_equals  # pylint: disable=E0611
 
 from xmodule.course_module import CourseDescriptor
 from xmodule.modulestore.xml import XMLModuleStore
-from xmodule.modulestore import XML_MODULESTORE_TYPE
+from xmodule.modulestore import Location, XML_MODULESTORE_TYPE
 
 from .test_modulestore import check_path_to_location
 from xmodule.tests import DATA_DIR
 
 
-class TestXMLModuleStore(object):
+def glob_tildes_at_end(path):
+    """
+    A wrapper for the `glob.glob` function, but it always returns
+    files that end in a tilde (~) at the end of the list of results.
+    """
+    result = glob(path)
+    with_tildes = [f for f in result if f.endswith("~")]
+    no_tildes = [f for f in result if not f.endswith("~")]
+    return no_tildes + with_tildes
+
+
+class TestXMLModuleStore(unittest.TestCase):
+    """
+    Test around the XML modulestore
+    """
     def test_path_to_location(self):
         """Make sure that path_to_location works properly"""
 
@@ -42,3 +63,18 @@ class TestXMLModuleStore(object):
         location = CourseDescriptor.id_to_location("edX/toy/2012_Fall")
         errors = modulestore.get_item_errors(location)
         assert errors == []
+
+    @patch("xmodule.modulestore.xml.glob.glob", side_effect=glob_tildes_at_end)
+    def test_tilde_files_ignored(self, _fake_glob):
+        modulestore = XMLModuleStore(DATA_DIR, course_dirs=['tilde'], load_error_modules=False)
+        course_module = modulestore.modules['edX/tilde/2012_Fall']
+        about_location = Location({
+            'tag': 'i4x',
+            'org': 'edX',
+            'course': 'tilde',
+            'category': 'about',
+            'name': 'index',
+        })
+        about_module = course_module[about_location]
+        self.assertIn("GREEN", about_module.data)
+        self.assertNotIn("RED", about_module.data)

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

@@ -405,8 +405,9 @@ class XMLModuleStore(ModuleStoreReadBase):
         try:
             course_descriptor = self.load_course(course_dir, errorlog.tracker)
         except Exception as e:
-            msg = "ERROR: Failed to load course '{0}': {1}".format(course_dir.encode("utf-8"),
-                    unicode(e))
+            msg = "ERROR: Failed to load course '{0}': {1}".format(
+                course_dir.encode("utf-8"), unicode(e)
+            )
             log.exception(msg)
             errorlog.tracker(msg)
 
@@ -568,6 +569,9 @@ class XMLModuleStore(ModuleStoreReadBase):
             if not os.path.isfile(filepath):
                 continue
 
+            if filepath.endswith('~'):  # skip *~ files
+                continue
+
             with open(filepath) as f:
                 try:
                     html = f.read().decode('utf-8')

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

@@ -39,6 +39,12 @@ def import_static_content(
         for filename in filenames:
 
             content_path = os.path.join(dirname, filename)
+
+            if filename.endswith('~'):
+                if verbose:
+                    log.debug('skipping static content %s...', content_path)
+                continue
+
             if verbose:
                 log.debug('importing static content %s...', content_path)
 

common/lib/xmodule/xmodule/tests/test_import_static.py

@@ -0,0 +1,23 @@
+"""
+Tests that check that we ignore the appropriate files when importing courses.
+"""
+import unittest
+from mock import Mock
+from xmodule.modulestore import Location
+from xmodule.modulestore.xml_importer import import_static_content
+from xmodule.tests import DATA_DIR
+
+
+class IgnoredFilesTestCase(unittest.TestCase):
+    "Tests for ignored files"
+    def test_ignore_tilde_static_files(self):
+        course_dir = DATA_DIR / "tilde"
+        loc = Location("edX", "tilde", "Fall_2012")
+        content_store = Mock()
+        content_store.generate_thumbnail.return_value = ("content", "location")
+        import_static_content(Mock(), Mock(), course_dir, content_store, loc)
+        saved_static_content = [call[0][0] for call in content_store.save.call_args_list]
+        name_val = {sc.name: sc.data for sc in saved_static_content}
+        self.assertIn("example.txt", name_val)
+        self.assertNotIn("example.txt~", name_val)
+        self.assertIn("GREEN", name_val["example.txt"])

common/test/data/tilde/README.md

@@ -0,0 +1,9 @@
+DO NOT DELETE TILDE FILES
+
+This course simulates an export that has been edited by hand, where the editor's
+text editor has left behind some backup files (about/index.html~ and
+static/example.txt~). Normally, we do not commit files that end with tildes to
+the repository, for precisely this reason -- they are backup files, and do
+not belong with the content. However, in this case, we *need* these backup files
+to be committed to the repository, so that we can exercise logic in the codebase
+that checks for these sort of editor backup files and skips them on export.

common/test/data/tilde/about/index.html

@@ -0,0 +1 @@
+GREEN

common/test/data/tilde/about/index.html~

@@ -0,0 +1 @@
+RED

common/test/data/tilde/course.xml

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

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

@@ -0,0 +1,2 @@
+<course>
+</course>

common/test/data/tilde/static/example.txt

@@ -0,0 +1 @@
+GREEN

common/test/data/tilde/static/example.txt~

@@ -0,0 +1 @@
+RED